Message ID | 20190515164814.258898-3-dianders@chromium.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | spi: A better solution for cros_ec_spi reliability | expand |
Hi, On 15/5/19 18:48, Douglas Anderson wrote: > Right now the only way to get the SPI pumping thread bumped up to > realtime priority is for the controller to request it. However it may > be that the controller works fine with the normal priority but > communication to a particular SPI device on the bus needs realtime > priority. > > Let's add a way for devices to request realtime priority when they set > themselves up. > > NOTE: this will just affect the priority of transfers that end up on > the SPI core's pumping thread. In many cases transfers happen in the > context of the caller so if you need realtime priority for all > transfers you should ensure the calling context is also realtime > priority. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Guenter Roeck <groeck@chromium.org> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Thanks, Enric > --- > > Changes in v4: None > Changes in v3: > - SPI core change now like patch v1 patch #2 (with name "rt"). > > Changes in v2: > - Now only force transfers to the thread for devices that want it. > - Squashed patch #1 and #2 together. > - Renamed variable to "force_rt_transfers". > > drivers/spi/spi.c | 36 ++++++++++++++++++++++++++++++------ > include/linux/spi/spi.h | 2 ++ > 2 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 8eb7460dd744..466984796dd9 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1364,10 +1364,32 @@ static void spi_pump_messages(struct kthread_work *work) > __spi_pump_messages(ctlr, true); > } > > -static int spi_init_queue(struct spi_controller *ctlr) > +/** > + * spi_set_thread_rt - set the controller to pump at realtime priority > + * @ctlr: controller to boost priority of > + * > + * This can be called because the controller requested realtime priority > + * (by setting the ->rt value before calling spi_register_controller()) or > + * because a device on the bus said that its transfers needed realtime > + * priority. > + * > + * NOTE: at the moment if any device on a bus says it needs realtime then > + * the thread will be at realtime priority for all transfers on that > + * controller. If this eventually becomes a problem we may see if we can > + * find a way to boost the priority only temporarily during relevant > + * transfers. > + */ > +static void spi_set_thread_rt(struct spi_controller *ctlr) > { > struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 }; > > + dev_info(&ctlr->dev, > + "will run message pump with realtime priority\n"); > + sched_setscheduler(ctlr->kworker_task, SCHED_FIFO, ¶m); > +} > + > +static int spi_init_queue(struct spi_controller *ctlr) > +{ > ctlr->running = false; > ctlr->busy = false; > > @@ -1387,11 +1409,8 @@ static int spi_init_queue(struct spi_controller *ctlr) > * request and the scheduling of the message pump thread. Without this > * setting the message pump thread will remain at default priority. > */ > - if (ctlr->rt) { > - dev_info(&ctlr->dev, > - "will run message pump with realtime priority\n"); > - sched_setscheduler(ctlr->kworker_task, SCHED_FIFO, ¶m); > - } > + if (ctlr->rt) > + spi_set_thread_rt(ctlr); > > return 0; > } > @@ -2982,6 +3001,11 @@ int spi_setup(struct spi_device *spi) > > spi_set_cs(spi, false); > > + if (spi->rt && !spi->controller->rt) { > + spi->controller->rt = true; > + spi_set_thread_rt(spi->controller); > + } > + > dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n", > (int) (spi->mode & (SPI_CPOL | SPI_CPHA)), > (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "", > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 053abd22ad31..15505c2485d6 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -109,6 +109,7 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats, > * This may be changed by the device's driver, or left at the > * default (0) indicating protocol words are eight bit bytes. > * The spi_transfer.bits_per_word can override this for each transfer. > + * @rt: Make the pump thread real time priority. > * @irq: Negative, or the number passed to request_irq() to receive > * interrupts from this device. > * @controller_state: Controller's runtime state > @@ -143,6 +144,7 @@ struct spi_device { > u32 max_speed_hz; > u8 chip_select; > u8 bits_per_word; > + bool rt; > u32 mode; > #define SPI_CPHA 0x01 /* clock phase */ > #define SPI_CPOL 0x02 /* clock polarity */ >
On Wed, May 15, 2019 at 09:48:12AM -0700, Douglas Anderson wrote: > Right now the only way to get the SPI pumping thread bumped up to > realtime priority is for the controller to request it. However it may > be that the controller works fine with the normal priority but > communication to a particular SPI device on the bus needs realtime > priority. The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9: Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-rt-pump for you to fetch changes up to 924b5867e7bd6a6a98014f0517b747465b108011: spi: Allow SPI devices to request the pumping thread be realtime (2019-05-23 14:44:02 +0100) ---------------------------------------------------------------- spi: Allow setting pump to RT priority ---------------------------------------------------------------- Douglas Anderson (1): spi: Allow SPI devices to request the pumping thread be realtime drivers/spi/spi.c | 36 ++++++++++++++++++++++++++++++------ include/linux/spi/spi.h | 2 ++ 2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 8eb7460dd744..466984796dd9 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1364,10 +1364,32 @@ static void spi_pump_messages(struct kthread_work *work) __spi_pump_messages(ctlr, true); } -static int spi_init_queue(struct spi_controller *ctlr) +/** + * spi_set_thread_rt - set the controller to pump at realtime priority + * @ctlr: controller to boost priority of + * + * This can be called because the controller requested realtime priority + * (by setting the ->rt value before calling spi_register_controller()) or + * because a device on the bus said that its transfers needed realtime + * priority. + * + * NOTE: at the moment if any device on a bus says it needs realtime then + * the thread will be at realtime priority for all transfers on that + * controller. If this eventually becomes a problem we may see if we can + * find a way to boost the priority only temporarily during relevant + * transfers. + */ +static void spi_set_thread_rt(struct spi_controller *ctlr) { struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 }; + dev_info(&ctlr->dev, + "will run message pump with realtime priority\n"); + sched_setscheduler(ctlr->kworker_task, SCHED_FIFO, ¶m); +} + +static int spi_init_queue(struct spi_controller *ctlr) +{ ctlr->running = false; ctlr->busy = false; @@ -1387,11 +1409,8 @@ static int spi_init_queue(struct spi_controller *ctlr) * request and the scheduling of the message pump thread. Without this * setting the message pump thread will remain at default priority. */ - if (ctlr->rt) { - dev_info(&ctlr->dev, - "will run message pump with realtime priority\n"); - sched_setscheduler(ctlr->kworker_task, SCHED_FIFO, ¶m); - } + if (ctlr->rt) + spi_set_thread_rt(ctlr); return 0; } @@ -2982,6 +3001,11 @@ int spi_setup(struct spi_device *spi) spi_set_cs(spi, false); + if (spi->rt && !spi->controller->rt) { + spi->controller->rt = true; + spi_set_thread_rt(spi->controller); + } + dev_dbg(&spi->dev, "setup mode %d, %s%s%s%s%u bits/w, %u Hz max --> %d\n", (int) (spi->mode & (SPI_CPOL | SPI_CPHA)), (spi->mode & SPI_CS_HIGH) ? "cs_high, " : "", diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 053abd22ad31..15505c2485d6 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -109,6 +109,7 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats, * This may be changed by the device's driver, or left at the * default (0) indicating protocol words are eight bit bytes. * The spi_transfer.bits_per_word can override this for each transfer. + * @rt: Make the pump thread real time priority. * @irq: Negative, or the number passed to request_irq() to receive * interrupts from this device. * @controller_state: Controller's runtime state @@ -143,6 +144,7 @@ struct spi_device { u32 max_speed_hz; u8 chip_select; u8 bits_per_word; + bool rt; u32 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */