* [PATCH 0/4] Add tCLQV parameter to tweak SPI timings @ 2023-10-26 15:23 Eberhard Stoll 2023-10-26 15:23 ` [PATCH 1/4] spi: Add parameter for clock to rx delay Eberhard Stoll 2023-10-26 15:23 ` [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment Eberhard Stoll 0 siblings, 2 replies; 10+ messages in thread From: Eberhard Stoll @ 2023-10-26 15:23 UTC (permalink / raw) To: Han Xu, linux-kernel, linux-mtd, linux-spi, Mark Brown, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra Cc: Eberhard Stoll, Amit Kumar Mahapatra, Amit Kumar Mahapatra via Alsa-devel, Andy Shevchenko, Cédric Le Goater, Christophe JAILLET, Chuanhong Guo, Dhruva Gole, Dmitry Rokosov, Frieder Schrempf, Geert Uytterhoeven, Greg Kroah-Hartman, Krishna Yarlagadda, Mario Kicherer, Martin Kurbanov, Olivier Maignial, Rob Herring, Serge Semin, Uwe Kleine-König, William Zhang, Yang Yingliang From: Eberhard Stoll <eberhard.stoll@kontron.de> Hi All, this patch series adds parameters to tweak SPI timings for some (Q)SPI devices. For example it optimizes the support for the operation of a Winbond W25N02KV SPI NAND chip on NXP i.mx6 ul(l) devices. The Winbond W25N02KV SPI NAND has the characteristic to introduce a delay between spi clock and the masters spi receive data (called tCLQV value in data sheet). This chip can operate with a maximum SPI clock of 104MHz. Disregarding the requred tCLQV value of 7ns for this chip reduces the possible spi clock to approx. 70MHz. To support the full bandwith of this chip, the tCLQV parameter has to be supported in SPI framework and SPI controller and the SPI NAND chip needs this parameter in the device configuration data. Also other devices can improve their operating SPI clock performance with this setting if they also has significant tCLQV values and the SPI controller will support it. This patch series adds support the tCLQV parameter in: - SPI framework - SPI NAND framework - NXP i.mx6 QSPI controller - Winbond W25N02KV SPI NAND device Eberhard Stoll (4): spi: Add parameter for clock to rx delay mtd: spinand: Add support for clock to rx delay setting mtd: spinand: winbond: Add rx sample delay for W25N02KV spi: spi-fsl-qspi: Add support for rx data sample point adjustment drivers/mtd/nand/spi/core.c | 2 + drivers/mtd/nand/spi/winbond.c | 3 +- drivers/spi/spi-fsl-qspi.c | 80 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spinand.h | 5 +++ include/linux/spi/spi.h | 3 ++ 5 files changed, 92 insertions(+), 1 deletion(-) base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1 -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] spi: Add parameter for clock to rx delay 2023-10-26 15:23 [PATCH 0/4] Add tCLQV parameter to tweak SPI timings Eberhard Stoll @ 2023-10-26 15:23 ` Eberhard Stoll 2023-10-26 22:56 ` Miquel Raynal 2023-10-26 15:23 ` [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment Eberhard Stoll 1 sibling, 1 reply; 10+ messages in thread From: Eberhard Stoll @ 2023-10-26 15:23 UTC (permalink / raw) To: linux-kernel, linux-spi, Mark Brown Cc: Eberhard Stoll, Frieder Schrempf, Amit Kumar Mahapatra, Andy Shevchenko, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Miquel Raynal, Yang Yingliang From: Eberhard Stoll <eberhard.stoll@kontron.de> For spi devices the master clock output defines the sampling point for receive data input stream (rising or falling edge). The receive data stream from the device is delayed in relation to the master clock output. For some devices this delay is larger than one half clock period, which is normally the sampling point for receive data. In this case receive data is sampled too early and the device fails to operate. In consequence the spi clock has to be reduced to match the delay characteristics and this reduces performance and is therefore not recommended. Some spi controllers implement a 'clock to receive data delay' compensation which shifts the receive sampling point. So we need a property to set this value for each spi device. Add a parameter 'rx_sample_delay_ns' to enable setting the clock to rx data delay for each spi device separately. The 'clock to receive data delay' value is often referenced as tCLQV in spi device data sheets. Signed-off-by: Eberhard Stoll <eberhard.stoll@kontron.de> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> --- include/linux/spi/spi.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 7f8b478fdeb3..14622d47f44f 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -166,6 +166,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg, * @cs_inactive: delay to be introduced by the controller after CS is * deasserted. If @cs_change_delay is used from @spi_transfer, then the * two delays will be added up. + * @rx_sample_delay_ns: spi clk to spi rx data delay * @pcpu_statistics: statistics for the spi_device * * A @spi_device is used to interchange data between an SPI slave @@ -219,6 +220,8 @@ struct spi_device { struct spi_delay cs_setup; struct spi_delay cs_hold; struct spi_delay cs_inactive; + /* Transfer characteristics */ + u32 rx_sample_delay_ns; /* Clock to RX data delay */ /* The statistics */ struct spi_statistics __percpu *pcpu_statistics; -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] spi: Add parameter for clock to rx delay 2023-10-26 15:23 ` [PATCH 1/4] spi: Add parameter for clock to rx delay Eberhard Stoll @ 2023-10-26 22:56 ` Miquel Raynal 2023-10-27 8:38 ` AW: " Stoll, Eberhard 0 siblings, 1 reply; 10+ messages in thread From: Miquel Raynal @ 2023-10-26 22:56 UTC (permalink / raw) To: Eberhard Stoll Cc: linux-kernel, linux-spi, Mark Brown, Eberhard Stoll, Frieder Schrempf, Amit Kumar Mahapatra, Andy Shevchenko, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Yang Yingliang Hello, estl@gmx.net wrote on Thu, 26 Oct 2023 17:23:02 +0200: > From: Eberhard Stoll <eberhard.stoll@kontron.de> > > For spi devices the master clock output defines the sampling point > for receive data input stream (rising or falling edge). The receive > data stream from the device is delayed in relation to the master > clock output. > > For some devices this delay is larger than one half clock period, Can you be more specific? I am wondering how big the need is. > which is normally the sampling point for receive data. In this case > receive data is sampled too early and the device fails to operate. > In consequence the spi clock has to be reduced to match the delay > characteristics and this reduces performance and is therefore not > recommended. > > Some spi controllers implement a 'clock to receive data delay' > compensation which shifts the receive sampling point. So we need > a property to set this value for each spi device. What if the spi controller does not support this feature? Shall we add a capability? Shall we refuse to probe if the controller is not capable of sampling at the right moment? > Add a parameter 'rx_sample_delay_ns' to enable setting the clock > to rx data delay for each spi device separately. > > The 'clock to receive data delay' value is often referenced as tCLQV > in spi device data sheets. > > Signed-off-by: Eberhard Stoll <eberhard.stoll@kontron.de> > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > --- > include/linux/spi/spi.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 7f8b478fdeb3..14622d47f44f 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -166,6 +166,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg, > * @cs_inactive: delay to be introduced by the controller after CS is > * deasserted. If @cs_change_delay is used from @spi_transfer, then the > * two delays will be added up. > + * @rx_sample_delay_ns: spi clk to spi rx data delay > * @pcpu_statistics: statistics for the spi_device > * > * A @spi_device is used to interchange data between an SPI slave > @@ -219,6 +220,8 @@ struct spi_device { > struct spi_delay cs_setup; > struct spi_delay cs_hold; > struct spi_delay cs_inactive; > + /* Transfer characteristics */ > + u32 rx_sample_delay_ns; /* Clock to RX data delay */ > > /* The statistics */ > struct spi_statistics __percpu *pcpu_statistics; > -- > 2.25.1 > Thanks, Miquèl ^ permalink raw reply [flat|nested] 10+ messages in thread
* AW: [PATCH 1/4] spi: Add parameter for clock to rx delay 2023-10-26 22:56 ` Miquel Raynal @ 2023-10-27 8:38 ` Stoll, Eberhard 2023-10-27 11:46 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Stoll, Eberhard @ 2023-10-27 8:38 UTC (permalink / raw) To: Miquel Raynal, Eberhard Stoll Cc: linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Mark Brown, Schrempf, Frieder, Amit Kumar Mahapatra, Andy Shevchenko, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Yang Yingliang Hello, > > For spi devices the master clock output defines the sampling point > > for receive data input stream (rising or falling edge). The receive > > data stream from the device is delayed in relation to the master > > clock output. > > > > For some devices this delay is larger than one half clock period, > > Can you be more specific? I am wondering how big the need is. In our case it's a QSPI NAND chip (Winbond W25N02KV). This device can operate at 104MHz SPI clock. But it also has a tCLQV value of 7ns. The tCLQV value limits the SPI clock speed for this device to 2x7ns (if it is not adjustable in the SPI controller) which is approximately 70MHz. Without the ability to set the tCLQV value, the SPI clock has to be limited to 70MHz in device tree for this bus. In our case the Winbond W25N02KV chip is a replacement of an older chip. The older chip can operate at 104MHz and does not have the tCLQV restrictions as this new one. The new chip is mostly is better than the data sheet and meet the timing requirements for 104MHz. But on higher temperatures devices fail. In device tree QSPI NAND chips are configured by a compatible property of 'spi-nand'. The mtd layer detects the real device and fetches the properties of this device from the appropriate driver. So for our case (boards containing the old and new chip) we well have to reduce the SPI clock for the entire QSPI bus to 70MHz, even for the elder chips which can operate well also with 104MHz. > > > which is normally the sampling point for receive data. In this case > > receive data is sampled too early and the device fails to operate. > > In consequence the spi clock has to be reduced to match the delay > > characteristics and this reduces performance and is therefore not > > recommended. > > > > Some spi controllers implement a 'clock to receive data delay' > > compensation which shifts the receive sampling point. So we need > > a property to set this value for each spi device. > > What if the spi controller does not support this feature? Shall we add > a capability? Shall we refuse to probe if the controller is not capable > of sampling at the right moment? > Refuse to probe is not necessary IMHO. The device can operate well even with controllers which do not implement the tCLQV functionality. The SPI clock has simply to be reduced and all works well. In this case not the maximum SPI clock frequency of the device limits the SPI bus clock, but the tCLQV value! IMHO it's the responsibility of the writer of the device tree configuration. For SPI controllers which do not support this setting, the SPI framework could check whether the max SPI clock frequency of the device or the tCLQV value limits the SPI bus speed and adjust it appropriately. For our case this seems a little bit of overkill. With 'discoverable' devices on the SPI bus like SPI NAND chips, the 'max_speed_hz' in 'struct spi_device' is no more really device specific, but more like chip select specific. The real chips 'max_speed_hz' data sheet value could then e.g. be propagated from the discovered chips SPI device driver to the frameworks 'chip select specific' 'max_speed_hz' property. We could introduce a 'probe_speed_hz' setting and maybe many other things ... ... but IMHO this would be far too much of overkill (at least currently) ... Thanks, Eberhard ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: AW: [PATCH 1/4] spi: Add parameter for clock to rx delay 2023-10-27 8:38 ` AW: " Stoll, Eberhard @ 2023-10-27 11:46 ` Andy Shevchenko [not found] ` <ZTvbFc+kFMotVUkh@finisterre.sirena.org.uk> 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2023-10-27 11:46 UTC (permalink / raw) To: Stoll, Eberhard Cc: Miquel Raynal, Eberhard Stoll, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Mark Brown, Schrempf, Frieder, Amit Kumar Mahapatra, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Yang Yingliang On Fri, Oct 27, 2023 at 08:38:54AM +0000, Stoll, Eberhard wrote: ... > > Can you be more specific? I am wondering how big the need is. > > In our case it's a QSPI NAND chip (Winbond W25N02KV). This device > can operate at 104MHz SPI clock. But it also has a tCLQV value of 7ns. > The tCLQV value limits the SPI clock speed for this device to 2x7ns > (if it is not adjustable in the SPI controller) which is approximately > 70MHz. > > Without the ability to set the tCLQV value, the SPI clock has to be > limited to 70MHz in device tree for this bus. > > In our case the Winbond W25N02KV chip is a replacement of an > older chip. The older chip can operate at 104MHz and does not > have the tCLQV restrictions as this new one. > The new chip is mostly is better than the data sheet and meet the > timing requirements for 104MHz. But on higher temperatures > devices fail. > > In device tree QSPI NAND chips are configured by a compatible > property of 'spi-nand'. The mtd layer detects the real device > and fetches the properties of this device from the appropriate > driver. > > So for our case (boards containing the old and new chip) we well > have to reduce the SPI clock for the entire QSPI bus to 70MHz, even > for the elder chips which can operate well also with 104MHz. So, to me sounds like device tree source issue. I.e. you need to provide different DT(b)s depending on the platform (and how it should be). The cleanest solution (as I see not the first time people I trying quirks like this to be part of the subsystems / drivers) is to make DT core (OF) to have conditionals or boot-time modifications allowed. This, what you are doing, does not scale and smells like an ugly hack. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <ZTvbFc+kFMotVUkh@finisterre.sirena.org.uk>]
* Re: AW: [PATCH 1/4] spi: Add parameter for clock to rx delay [not found] ` <ZTvbFc+kFMotVUkh@finisterre.sirena.org.uk> @ 2023-10-30 8:48 ` Andy Shevchenko 2023-10-30 9:28 ` AW: " Stoll, Eberhard 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2023-10-30 8:48 UTC (permalink / raw) To: Mark Brown, Rob Herring, Frank Rowand Cc: Stoll, Eberhard, Miquel Raynal, Eberhard Stoll, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Schrempf, Frieder, Amit Kumar Mahapatra, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Yang Yingliang On Fri, Oct 27, 2023 at 04:45:25PM +0100, Mark Brown wrote: > On Fri, Oct 27, 2023 at 02:46:42PM +0300, Andy Shevchenko wrote: > > > So, to me sounds like device tree source issue. I.e. you need to provide > > different DT(b)s depending on the platform (and how it should be). > > The cleanest solution (as I see not the first time people I trying quirks like > > this to be part of the subsystems / drivers) is to make DT core (OF) to have > > conditionals or boot-time modifications allowed. > > > This, what you are doing, does not scale and smells like an ugly hack. > > No, this seems like an entirely reasonable thing to have - it's just a > property of the device, we don't need to add a DT property for it, and > the maximum speed that the device can run at is going to vary depending > on the ability of the controller to control the sampling point. > > As people have been saying there's a particularly clear case for this > with SPI flash which is probed at runtime and is readily substituted at > the hardware level. So, then the question is what does DT _actually_ describes? If we have an autoprobe of something that doesn't work on platform A and works on platform B, shouldn't these platforms have to have distinguishable DTs? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* AW: AW: [PATCH 1/4] spi: Add parameter for clock to rx delay 2023-10-30 8:48 ` Andy Shevchenko @ 2023-10-30 9:28 ` Stoll, Eberhard 0 siblings, 0 replies; 10+ messages in thread From: Stoll, Eberhard @ 2023-10-30 9:28 UTC (permalink / raw) To: Andy Shevchenko, Mark Brown, Rob Herring, Frank Rowand Cc: Miquel Raynal, Eberhard Stoll, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, Schrempf, Frieder, Amit Kumar Mahapatra, Christophe JAILLET, Geert Uytterhoeven, Krishna Yarlagadda, Leonard Göhrs, Yang Yingliang > So, then the question is what does DT _actually_ describes? > If we have an autoprobe of something that doesn't work on platform A and > works > on platform B, shouldn't these platforms have to have distinguishable DTs? Yes it's one possibility to deal with it. But I think the first choice should be to improve the autoprobe function to work properly on all platforms. This offers the most advantage for all. If this doesn't work, the DT approach should be the fallback choice. Improving the autoprobe function in this way seems realistic. So it's currently the way we should go. Kind regards Eberhard ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment 2023-10-26 15:23 [PATCH 0/4] Add tCLQV parameter to tweak SPI timings Eberhard Stoll 2023-10-26 15:23 ` [PATCH 1/4] spi: Add parameter for clock to rx delay Eberhard Stoll @ 2023-10-26 15:23 ` Eberhard Stoll 2023-10-26 20:03 ` kernel test robot 1 sibling, 1 reply; 10+ messages in thread From: Eberhard Stoll @ 2023-10-26 15:23 UTC (permalink / raw) To: Han Xu, linux-kernel, linux-spi, Mark Brown Cc: Eberhard Stoll, Frieder Schrempf, Amit Kumar Mahapatra, Amit Kumar Mahapatra via Alsa-devel, Michal Simek, Rob Herring, Uwe Kleine-König, Yang Yingliang From: Eberhard Stoll <eberhard.stoll@kontron.de> This qspi controller supports shifting the spi rx data sampling point to compensate line or spi device response delays. It enables fast spi data transfers even for devices which have a noticeable delay in the rx data stream. Add support for the SMPR sampling functionality Signed-off-by: Eberhard Stoll <eberhard.stoll@kontron.de> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> --- drivers/spi/spi-fsl-qspi.c | 80 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c index 79bac30e79af..68801e08f997 100644 --- a/drivers/spi/spi-fsl-qspi.c +++ b/drivers/spi/spi-fsl-qspi.c @@ -274,6 +274,12 @@ struct fsl_qspi { int selected; }; +struct fsl_qspi_chip_data { + u32 rx_sample_delay_ns; + unsigned long rate; + u32 smpr_sampling; +}; + static inline int needs_swap_endian(struct fsl_qspi *q) { return q->devtype_data->quirks & QUADSPI_QUIRK_SWAP_ENDIAN; @@ -522,14 +528,63 @@ static void fsl_qspi_invalidate(struct fsl_qspi *q) qspi_writel(q, reg, q->iobase + QUADSPI_MCR); } +static void fsl_qspi_update_smpr_sampling(struct fsl_qspi *q, u32 smpr) +{ + void __iomem *base = q->iobase; + u32 reg; + + /* Disable the module */ + qspi_writel(q, QUADSPI_MCR_MDIS_MASK | QUADSPI_MCR_RESERVED_MASK, + base + QUADSPI_MCR); + + reg = qspi_readl(q, base + QUADSPI_SMPR) & + ~(QUADSPI_SMPR_FSPHS_MASK | QUADSPI_SMPR_FSDLY_MASK); + qspi_writel(q, reg | smpr, base + QUADSPI_SMPR); + + /* Enable the module */ + qspi_writel(q, QUADSPI_MCR_RESERVED_MASK | QUADSPI_MCR_END_CFG_MASK, + base + QUADSPI_MCR); +} + +const char *sampling_mode[] = { "N1", "I1", "N2", "I2"}; + static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi) { unsigned long rate = spi->max_speed_hz; int ret; + struct fsl_qspi_chip_data *chip = spi_get_ctldata(spi); + const char *sampling_ident = sampling_mode[0]; + + if (chip->rx_sample_delay_ns != spi->rx_sample_delay_ns | + chip->rate != rate) { + chip->rx_sample_delay_ns = spi->rx_sample_delay_ns; + chip->rate = rate; + + chip->smpr_sampling = + (2 * spi->rx_sample_delay_ns * (rate >> 10)) / (1000000000 >> 10); + dev_dbg(q->dev, "smpr_sampling = %u (delay %u ns)\n", + chip->smpr_sampling, spi->rx_sample_delay_ns); + + if (chip->smpr_sampling > 3) { + dev_err(q->dev, "rx sample delay for device %s exceeds hw capabilities! Clamp value to maximum setting.\n", + dev_name(&spi->dev)); + chip->smpr_sampling = 3; + sampling_ident = "(I2 clamped to max)"; + } else { + sampling_ident = sampling_mode[chip->smpr_sampling]; + } + + chip->smpr_sampling <<= 5; + dev_info(q->dev, "sampling point %s at %lu kHz used for device %s\n", + sampling_ident, rate / 1000, dev_name(&spi->dev)); + fsl_qspi_update_smpr_sampling(q, chip->smpr_sampling); + } if (q->selected == spi_get_chipselect(spi, 0)) return; + fsl_qspi_update_smpr_sampling(q, chip->smpr_sampling); + if (needs_4x_clock(q)) rate *= 4; @@ -839,6 +894,28 @@ static const struct spi_controller_mem_ops fsl_qspi_mem_ops = { .get_name = fsl_qspi_get_name, }; +static int fsl_qspi_setup(struct spi_device *spi) +{ + struct fsl_qspi_chip_data *chip = spi_get_ctldata(spi); + + if (!chip) { + chip = kzalloc(sizeof(*chip), GFP_KERNEL); + if (!chip) + return -ENOMEM; + spi_set_ctldata(spi, chip); + } + + return 0; +} + +static void fsl_qspi_cleanup(struct spi_device *spi) +{ + struct fsl_qspi_chip_data *chip = spi_get_ctldata(spi); + + kfree(chip); + spi_set_ctldata(spi, NULL); +} + static int fsl_qspi_probe(struct platform_device *pdev) { struct spi_controller *ctlr; @@ -865,6 +942,9 @@ static int fsl_qspi_probe(struct platform_device *pdev) platform_set_drvdata(pdev, q); + ctlr->setup = fsl_qspi_setup; + ctlr->cleanup = fsl_qspi_cleanup; + /* find the resources */ q->iobase = devm_platform_ioremap_resource_byname(pdev, "QuadSPI"); if (IS_ERR(q->iobase)) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment 2023-10-26 15:23 ` [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment Eberhard Stoll @ 2023-10-26 20:03 ` kernel test robot 2023-10-27 6:51 ` Frieder Schrempf 0 siblings, 1 reply; 10+ messages in thread From: kernel test robot @ 2023-10-26 20:03 UTC (permalink / raw) To: Eberhard Stoll, Han Xu, linux-kernel, linux-spi, Mark Brown Cc: oe-kbuild-all, Eberhard Stoll, Frieder Schrempf, Amit Kumar Mahapatra, Amit Kumar Mahapatra via Alsa-devel, Michal Simek, Rob Herring, Uwe Kleine-König, Yang Yingliang Hi Eberhard, kernel test robot noticed the following build warnings: [auto build test WARNING on 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1] url: https://github.com/intel-lab-lkp/linux/commits/Eberhard-Stoll/spi-Add-parameter-for-clock-to-rx-delay/20231026-232547 base: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1 patch link: https://lore.kernel.org/r/20231026152316.2729575-5-estl%40gmx.net patch subject: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231027/202310270332.mcbckKCr-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310270332.mcbckKCr-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310270332.mcbckKCr-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/spi/spi-fsl-qspi.c: In function 'fsl_qspi_select_mem': >> drivers/spi/spi-fsl-qspi.c:558:38: warning: suggest parentheses around comparison in operand of '|' [-Wparentheses] 558 | if (chip->rx_sample_delay_ns != spi->rx_sample_delay_ns | | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ vim +558 drivers/spi/spi-fsl-qspi.c 550 551 static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi) 552 { 553 unsigned long rate = spi->max_speed_hz; 554 int ret; 555 struct fsl_qspi_chip_data *chip = spi_get_ctldata(spi); 556 const char *sampling_ident = sampling_mode[0]; 557 > 558 if (chip->rx_sample_delay_ns != spi->rx_sample_delay_ns | 559 chip->rate != rate) { 560 chip->rx_sample_delay_ns = spi->rx_sample_delay_ns; 561 chip->rate = rate; 562 563 chip->smpr_sampling = 564 (2 * spi->rx_sample_delay_ns * (rate >> 10)) / (1000000000 >> 10); 565 dev_dbg(q->dev, "smpr_sampling = %u (delay %u ns)\n", 566 chip->smpr_sampling, spi->rx_sample_delay_ns); 567 568 if (chip->smpr_sampling > 3) { 569 dev_err(q->dev, "rx sample delay for device %s exceeds hw capabilities! Clamp value to maximum setting.\n", 570 dev_name(&spi->dev)); 571 chip->smpr_sampling = 3; 572 sampling_ident = "(I2 clamped to max)"; 573 } else { 574 sampling_ident = sampling_mode[chip->smpr_sampling]; 575 } 576 577 chip->smpr_sampling <<= 5; 578 dev_info(q->dev, "sampling point %s at %lu kHz used for device %s\n", 579 sampling_ident, rate / 1000, dev_name(&spi->dev)); 580 fsl_qspi_update_smpr_sampling(q, chip->smpr_sampling); 581 } 582 583 if (q->selected == spi_get_chipselect(spi, 0)) 584 return; 585 586 fsl_qspi_update_smpr_sampling(q, chip->smpr_sampling); 587 588 if (needs_4x_clock(q)) 589 rate *= 4; 590 591 fsl_qspi_clk_disable_unprep(q); 592 593 ret = clk_set_rate(q->clk, rate); 594 if (ret) 595 return; 596 597 ret = fsl_qspi_clk_prep_enable(q); 598 if (ret) 599 return; 600 601 q->selected = spi_get_chipselect(spi, 0); 602 603 fsl_qspi_invalidate(q); 604 } 605 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment 2023-10-26 20:03 ` kernel test robot @ 2023-10-27 6:51 ` Frieder Schrempf 0 siblings, 0 replies; 10+ messages in thread From: Frieder Schrempf @ 2023-10-27 6:51 UTC (permalink / raw) To: kernel test robot, Eberhard Stoll, Han Xu, linux-kernel, linux-spi, Mark Brown Cc: oe-kbuild-all, Eberhard Stoll, Amit Kumar Mahapatra, Amit Kumar Mahapatra via Alsa-devel, Michal Simek, Rob Herring, Uwe Kleine-König, Yang Yingliang On 26.10.23 22:03, kernel test robot wrote: > Hi Eberhard, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1] > > url: https://github.com/intel-lab-lkp/linux/commits/Eberhard-Stoll/spi-Add-parameter-for-clock-to-rx-delay/20231026-232547 > base: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1 > patch link: https://lore.kernel.org/r/20231026152316.2729575-5-estl%40gmx.net > patch subject: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment > config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231027/202310270332.mcbckKCr-lkp@intel.com/config) > compiler: m68k-linux-gcc (GCC) 13.2.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310270332.mcbckKCr-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202310270332.mcbckKCr-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > drivers/spi/spi-fsl-qspi.c: In function 'fsl_qspi_select_mem': >>> drivers/spi/spi-fsl-qspi.c:558:38: warning: suggest parentheses around comparison in operand of '|' [-Wparentheses] > 558 | if (chip->rx_sample_delay_ns != spi->rx_sample_delay_ns | > | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ IIRC, when I prepared the patches for sending "checkpatch.pl --strict" suggested to remove the parentheses here. Seems a bit inconsistent... ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-30 9:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-26 15:23 [PATCH 0/4] Add tCLQV parameter to tweak SPI timings Eberhard Stoll
2023-10-26 15:23 ` [PATCH 1/4] spi: Add parameter for clock to rx delay Eberhard Stoll
2023-10-26 22:56 ` Miquel Raynal
2023-10-27 8:38 ` AW: " Stoll, Eberhard
2023-10-27 11:46 ` Andy Shevchenko
[not found] ` <ZTvbFc+kFMotVUkh@finisterre.sirena.org.uk>
2023-10-30 8:48 ` Andy Shevchenko
2023-10-30 9:28 ` AW: " Stoll, Eberhard
2023-10-26 15:23 ` [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment Eberhard Stoll
2023-10-26 20:03 ` kernel test robot
2023-10-27 6:51 ` Frieder Schrempf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox