diff mbox series

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

Message ID 20231026152316.2729575-5-estl@gmx.net (mailing list archive)
State New, archived
Headers show
Series Add tCLQV parameter to tweak SPI timings | expand

Commit Message

Eberhard Stoll Oct. 26, 2023, 3:23 p.m. UTC 
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(+)

--
2.25.1

Comments

kernel test robot Oct. 26, 2023, 8:03 p.m. UTC | #1 
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
Frieder Schrempf Oct. 27, 2023, 6:51 a.m. UTC | #2 
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...
diff mbox series

Patch

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)) {