2023-10-26 15:24:48

by Eberhard Stoll

[permalink] [raw]
Subject: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment

From: Eberhard Stoll <[email protected]>

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 <[email protected]>
Signed-off-by: Frieder Schrempf <[email protected]>
---
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


2023-10-26 20:04:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment

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/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-10-27 06:52:18

by Frieder Schrempf

[permalink] [raw]
Subject: Re: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment

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/[email protected]/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> 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...

2023-10-27 16:09:40

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/4] spi: spi-fsl-qspi: Add support for rx data sample point adjustment

On Fri, Oct 27, 2023 at 08:51:49AM +0200, Frieder Schrempf wrote:
> On 26.10.23 22:03, kernel test robot wrote:

> > 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...

Ignore chekpatch, and it does look like as well as parentheses you
should have used || not | here.


Attachments:
(No filename) (724.00 B)
signature.asc (499.00 B)
Download all attachments