svn commit: r355492 - in stable/12/sys/arm/ti: . am335x
Ian Lepore
ian at FreeBSD.org
Sat Dec 7 17:17:35 UTC 2019
Author: ian
Date: Sat Dec 7 17:17:34 2019
New Revision: 355492
URL: https://svnweb.freebsd.org/changeset/base/355492
Log:
MFC r352196, r352333, r352342, r353653
r352196:
In am335x_dmtpps, use a spin mutex to interlock between PPS capture and PPS
ioctl(2) handling. This allows doing the pps_event() work in the polling
routine, instead of using a taskqueue task to do that work.
Also, add PNPINFO, and switch to using make_dev_s() to create the cdev.
Using a spin mutex and calling pps_event() from the polling function works
around the situation which requires more than 2 sets of timecounter
timehands in a single-core system to get reliable PPS capture. That problem
would happen when a single-core system is idle in cpu_idle() then gets woken
up with an event timer event which was scheduled to handle a hardclock tick.
That processing path would end up calling tc_windup 3 or 4 times between
when the tc polling function was called and when the taskqueue task would
eventually run, and with only two sets of timehands, the th_generation count
would always be too old to allow the captured PPS data to be used.
r352333:
Include <lock.h>, required to use spinlocks in this code.
r352342:
Make the ti_sysc device quiet. It's an internal utility pseudo-device
that makes the upstream FDT data work right, so we don't need to see a
couple dozen instances of it spam the dmesg at boot time unless it's a
verbose boot.
r353653:
Update some comments; no functional changes. Some historical old comments
in this driver indicate that the SD_CAPA register is write-once and after
being set one time the values in it cannot be changed. That turns out not
to be the case -- the values written to it survive a reset, but they can
be rewritten/changed at any time.
Modified:
stable/12/sys/arm/ti/am335x/am335x_dmtpps.c
stable/12/sys/arm/ti/ti_sdhci.c
stable/12/sys/arm/ti/ti_sysc.c
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/sys/arm/ti/am335x/am335x_dmtpps.c
==============================================================================
--- stable/12/sys/arm/ti/am335x/am335x_dmtpps.c Sat Dec 7 17:14:33 2019 (r355491)
+++ stable/12/sys/arm/ti/am335x/am335x_dmtpps.c Sat Dec 7 17:17:34 2019 (r355492)
@@ -48,11 +48,11 @@ __FBSDID("$FreeBSD$");
#include <sys/bus.h>
#include <sys/conf.h>
#include <sys/kernel.h>
+#include <sys/lock.h>
#include <sys/module.h>
#include <sys/malloc.h>
#include <sys/mutex.h>
#include <sys/rman.h>
-#include <sys/taskqueue.h>
#include <sys/timepps.h>
#include <sys/timetc.h>
#include <machine/bus.h>
@@ -79,7 +79,6 @@ struct dmtpps_softc {
uint32_t tclr; /* Cached TCLR register. */
struct timecounter tc;
int pps_curmode; /* Edge mode now set in hw. */
- struct task pps_task; /* For pps_event handling. */
struct cdev * pps_cdev;
struct pps_state pps_state;
struct mtx pps_mtx;
@@ -93,6 +92,7 @@ static struct ofw_compat_data compat_data[] = {
{"ti,am335x-timer-1ms", 1},
{NULL, 0},
};
+SIMPLEBUS_PNP_INFO(compat_data);
/*
* A table relating pad names to the hardware timer number they can be mux'd to.
@@ -285,48 +285,29 @@ dmtpps_poll(struct timecounter *tc)
* populates it from the current DMT_TCRR register) with the latched
* value from the TCAR1 register.
*
- * There is no locking here, by design. pps_capture() writes into an
- * area of struct pps_state which is read only by pps_event(). The
- * synchronization of access to that area is temporal rather than
- * interlock based... we write in this routine and trigger the task that
- * will read the data, so no simultaneous access can occur.
- *
* Note that we don't have the TCAR interrupt enabled, but the hardware
* still provides the status bits in the "RAW" status register even when
* they're masked from generating an irq. However, when clearing the
* TCAR status to re-arm the capture for the next second, we have to
* write to the IRQ status register, not the RAW register. Quirky.
+ *
+ * We do not need to hold a lock while capturing the pps data, because
+ * it is captured into an area of the pps_state struct which is read
+ * only by pps_event(). We do need to hold a lock while calling
+ * pps_event(), because it manipulates data which is also accessed from
+ * the ioctl(2) context by userland processes.
*/
if (DMTIMER_READ4(sc, DMT_IRQSTATUS_RAW) & DMT_IRQ_TCAR) {
pps_capture(&sc->pps_state);
sc->pps_state.capcount = DMTIMER_READ4(sc, DMT_TCAR1);
DMTIMER_WRITE4(sc, DMT_IRQSTATUS, DMT_IRQ_TCAR);
- taskqueue_enqueue(taskqueue_fast, &sc->pps_task);
+
+ mtx_lock_spin(&sc->pps_mtx);
+ pps_event(&sc->pps_state, PPS_CAPTUREASSERT);
+ mtx_unlock_spin(&sc->pps_mtx);
}
}
-static void
-dmtpps_event(void *arg, int pending)
-{
- struct dmtpps_softc *sc;
-
- sc = arg;
-
- /* This is the task function that gets enqueued by poll_pps. Once the
- * time has been captured by the timecounter polling code which runs in
- * primary interrupt context, the remaining (more expensive) work to
- * process the event is done later in a threaded context.
- *
- * Here there is an interlock that protects the event data in struct
- * pps_state. That data can be accessed at any time from userland via
- * ioctl() calls so we must ensure that there is no read access to
- * partially updated data while pps_event() does its work.
- */
- mtx_lock(&sc->pps_mtx);
- pps_event(&sc->pps_state, PPS_CAPTUREASSERT);
- mtx_unlock(&sc->pps_mtx);
-}
-
static int
dmtpps_open(struct cdev *dev, int flags, int fmt,
struct thread *td)
@@ -374,9 +355,9 @@ dmtpps_ioctl(struct cdev *dev, u_long cmd, caddr_t dat
sc = dev->si_drv1;
/* Let the kernel do the heavy lifting for ioctl. */
- mtx_lock(&sc->pps_mtx);
+ mtx_lock_spin(&sc->pps_mtx);
err = pps_ioctl(cmd, data, &sc->pps_state);
- mtx_unlock(&sc->pps_mtx);
+ mtx_unlock_spin(&sc->pps_mtx);
if (err != 0)
return (err);
@@ -436,6 +417,7 @@ static int
dmtpps_attach(device_t dev)
{
struct dmtpps_softc *sc;
+ struct make_dev_args mda;
clk_ident_t timer_id;
int err, sysclk_freq;
@@ -502,22 +484,27 @@ dmtpps_attach(device_t dev)
* now, just say we can only capture assert events (the positive-going
* edge of the pulse).
*/
- mtx_init(&sc->pps_mtx, "dmtpps", NULL, MTX_DEF);
+ mtx_init(&sc->pps_mtx, "dmtpps", NULL, MTX_SPIN);
+ sc->pps_state.flags = PPSFLAG_MTX_SPIN;
sc->pps_state.ppscap = PPS_CAPTUREASSERT;
sc->pps_state.driver_abi = PPS_ABI_VERSION;
sc->pps_state.driver_mtx = &sc->pps_mtx;
pps_init_abi(&sc->pps_state);
- /*
- * Init the task that does deferred pps_event() processing after
- * the polling routine has captured a pps pulse time.
- */
- TASK_INIT(&sc->pps_task, 0, dmtpps_event, sc);
-
/* Create the PPS cdev. */
- sc->pps_cdev = make_dev(&dmtpps_cdevsw, 0, UID_ROOT, GID_WHEEL, 0600,
- PPS_CDEV_NAME);
- sc->pps_cdev->si_drv1 = sc;
+ make_dev_args_init(&mda);
+ mda.mda_flags = MAKEDEV_WAITOK;
+ mda.mda_devsw = &dmtpps_cdevsw;
+ mda.mda_cr = NULL;
+ mda.mda_uid = UID_ROOT;
+ mda.mda_gid = GID_WHEEL;
+ mda.mda_mode = 0600;
+ mda.mda_unit = device_get_unit(dev);
+ mda.mda_si_drv1 = sc;
+ if ((err = make_dev_s(&mda, &sc->pps_cdev, PPS_CDEV_NAME)) != 0) {
+ device_printf(dev, "Failed to create cdev %s\n", PPS_CDEV_NAME);
+ return (err);
+ }
if (bootverbose)
device_printf(sc->dev, "Using %s for PPS device /dev/%s\n",
Modified: stable/12/sys/arm/ti/ti_sdhci.c
==============================================================================
--- stable/12/sys/arm/ti/ti_sdhci.c Sat Dec 7 17:14:33 2019 (r355491)
+++ stable/12/sys/arm/ti/ti_sdhci.c Sat Dec 7 17:17:34 2019 (r355492)
@@ -481,10 +481,10 @@ ti_sdhci_hw_init(device_t dev)
/*
* The attach() routine has examined fdt data and set flags in
* slot.host.caps to reflect what voltages we can handle. Set those
- * values in the CAPA register. The manual says that these values can
- * only be set once, "before initialization" whatever that means, and
- * that they survive a reset. So maybe doing this will be a no-op if
- * u-boot has already initialized the hardware.
+ * values in the CAPA register. Empirical testing shows that the
+ * values in this register can be overwritten at any time, but the
+ * manual says that these values should only be set once, "before
+ * initialization" whatever that means, and that they survive a reset.
*/
regval = ti_mmchs_read_4(sc, MMCHS_SD_CAPA);
if (sc->slot.host.caps & MMC_OCR_LOW_VOLTAGE)
@@ -528,8 +528,7 @@ ti_sdhci_attach(device_t dev)
* device, and only 1p8v on other devices unless an external transceiver
* is used. The only way we could know about a transceiver is fdt data.
* Note that we have to do this before calling ti_sdhci_hw_init() so
- * that it can set the right values in the CAPA register, which can only
- * be done once and never reset.
+ * that it can set the right values in the CAPA register.
*/
sc->slot.host.caps |= MMC_OCR_LOW_VOLTAGE;
if (sc->mmchs_clk_id == MMC1_CLK || OF_hasprop(node, "ti,dual-volt")) {
Modified: stable/12/sys/arm/ti/ti_sysc.c
==============================================================================
--- stable/12/sys/arm/ti/ti_sysc.c Sat Dec 7 17:14:33 2019 (r355491)
+++ stable/12/sys/arm/ti/ti_sysc.c Sat Dec 7 17:17:34 2019 (r355492)
@@ -71,6 +71,9 @@ ti_sysc_probe(device_t dev)
return (ENXIO);
device_set_desc(dev, "TI SYSC Interconnect");
+ if (!bootverbose)
+ device_quiet(dev);
+
return (BUS_PROBE_DEFAULT);
}
More information about the svn-src-all
mailing list