| Message ID | 20190324175002.28969-10-kernel@martin.sperl.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: bcm2835aux: bug fixes and improvements | expand |
Hi Martin, Am 24.03.19 um 18:50 schrieb kernel@martin.sperl.org: > From: Martin Sperl <kernel@martin.sperl.org> > > Under some circumstances the default 30 us polling limit is not optimal > and may lead to long delays because we are waiting on an interrupt. > with this patch we have the possibility to influence this policy. > > So make this limit (in us) configurable via a module parameters > (but also modifyable via /sys/modules/...) > > Signed-off-by: Martin Sperl <kernel@martin.sperl.org> > > --- > Changelog: > V1 -> V2: remove the dependency on a different patchset focused on > making cs_change delay configurable > > --- > drivers/spi/spi-bcm2835aux.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c > index d2b58060b333..df065108122b 100644 > --- a/drivers/spi/spi-bcm2835aux.c > +++ b/drivers/spi/spi-bcm2835aux.c > @@ -37,6 +37,12 @@ > #include <linux/spi/spi.h> > #include <linux/spinlock.h> > > +/* define polling limits */ > +unsigned int polling_limit_us = 30; > +module_param(polling_limit_us, uint, 0664); > +MODULE_PARM_DESC(polling_limit_us, > + "time in us to run a transfer in polling mode\n"); > + could you please document the case polling_limit_us = 0 ?
> On 25.03.2019, at 10:44, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Martin, > > Am 24.03.19 um 18:50 schrieb kernel@martin.sperl.org: >> From: Martin Sperl <kernel@martin.sperl.org> >> >> +/* define polling limits */ >> +unsigned int polling_limit_us = 30; >> +module_param(polling_limit_us, uint, 0664); >> +MODULE_PARM_DESC(polling_limit_us, >> + "time in us to run a transfer in polling mode\n"); >> + > could you please document the case polling_limit_us = 0 ? Means no polling under any circumstances - I can add it as a comment in V3. martin
Am 25.03.19 um 10:49 schrieb kernel@martin.sperl.org: >> On 25.03.2019, at 10:44, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> >> Hi Martin, >> >> Am 24.03.19 um 18:50 schrieb kernel@martin.sperl.org: >>> From: Martin Sperl <kernel@martin.sperl.org> >>> >>> +/* define polling limits */ >>> +unsigned int polling_limit_us = 30; >>> +module_param(polling_limit_us, uint, 0664); >>> +MODULE_PARM_DESC(polling_limit_us, >>> + "time in us to run a transfer in polling mode\n"); >>> + >> could you please document the case polling_limit_us = 0 ? > Means no polling under any circumstances - I can add it as a comment in V3. I meant the parameter description, because users shouldn't be forced to look at the source code. > > martin > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index d2b58060b333..df065108122b 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -37,6 +37,12 @@ #include <linux/spi/spi.h> #include <linux/spinlock.h> +/* define polling limits */ +unsigned int polling_limit_us = 30; +module_param(polling_limit_us, uint, 0664); +MODULE_PARM_DESC(polling_limit_us, + "time in us to run a transfer in polling mode\n"); + /* * spi register defines * @@ -89,10 +95,6 @@ #define BCM2835_AUX_SPI_STAT_BUSY 0x00000040 #define BCM2835_AUX_SPI_STAT_BITCOUNT 0x0000003F -/* timeout values */ -#define BCM2835_AUX_SPI_POLLING_LIMIT_US 30 -#define BCM2835_AUX_SPI_POLLING_JIFFIES 2 - struct bcm2835aux_spi { void __iomem *regs; struct clk *clk; @@ -325,8 +327,8 @@ static int bcm2835aux_spi_transfer_one_poll(struct spi_master *master, bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL1, bs->cntl[1]); bcm2835aux_wr(bs, BCM2835_AUX_SPI_CNTL0, bs->cntl[0]); - /* set the timeout */ - timeout = jiffies + BCM2835_AUX_SPI_POLLING_JIFFIES; + /* set the timeout to at least 2 jiffies */ + timeout = jiffies + 2 + HZ * polling_limit_us / 1000000; /* loop until finished the transfer */ while (bs->rx_len) { @@ -356,8 +358,8 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, struct spi_transfer *tfr) { struct bcm2835aux_spi *bs = spi_master_get_devdata(master); - unsigned long spi_hz, clk_hz, speed; - unsigned long spi_used_hz; + unsigned long spi_hz, clk_hz, speed, spi_used_hz; + unsigned long hz_per_byte, byte_limit; /* calculate the registers to handle * @@ -401,14 +403,15 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, * of Hz per byte per polling limit. E.g., we can transfer 1 byte in * 30 µs per 300,000 Hz of bus clock. */ -#define HZ_PER_BYTE ((9 * 1000000) / BCM2835_AUX_SPI_POLLING_LIMIT_US) + hz_per_byte = polling_limit_us ? (9 * 1000000) / polling_limit_us : 0; + byte_limit = hz_per_byte ? spi_used_hz / hz_per_byte : 1; + /* run in polling mode for short transfers */ - if (tfr->len < spi_used_hz / HZ_PER_BYTE) + if (tfr->len < byte_limit) return bcm2835aux_spi_transfer_one_poll(master, spi, tfr); /* run in interrupt mode for all others */ return bcm2835aux_spi_transfer_one_irq(master, spi, tfr); -#undef HZ_PER_BYTE } static int bcm2835aux_spi_prepare_message(struct spi_master *master,