Skip to contentSkip to conversation

spi: pxa2xx: Convert to use SPI core GPIO handling

PATCH In review Andy Shevchenko 2 participants 11 messages May 17, 2021 – May 18, 2021

Subsystems (3)
SPI PXA2xx/PXA3xx support Intel LPSS (Low Power SubSystem) drivers
lore
Share & tools
lore

Andy Shevchenko

author reviewer
#0 lore raw

cover spi: pxa2xx: Convert to use SPI core GPIO handling

Convert the PXA2xx SPI host controller driver to use SPI core GPIO handling. On top of that a few additional cleanups.

Has been tested on Intel Merrifield with SPI ADC and TFT display connected.

Andy Shevchenko (9):
spi: pxa2xx: Propagate firmware node to the child SPI controller
device
spi: pxa2xx: Switch to use SPI core GPIO (descriptor) CS handling
spi: pxa2xx: Switch to use SPI core GPIO (legacy) CS handling
spi: pxa2xx: Drop duplicate chip_select in struct chip_data
spi: pxa2xx: Drop unneeded '!= 0' comparisons
spi: pxa2xx: Fix printf() specifiers
spi: pxa2xx: Fix style of and typos in the comments and messages
spi: pxa2xx: Update documentation to point out that it's outdated
spi: pxa2xx: Use predefined mask when programming FIFO thresholds

 Documentation/spi/pxa2xx.rst   |  58 +++++-----
 drivers/spi/spi-pxa2xx-dma.c   |   4 +-
 drivers/spi/spi-pxa2xx-pci.c   |   5 +-
 drivers/spi/spi-pxa2xx.c       | 203 +++++++++++++--------------------
 drivers/spi/spi-pxa2xx.h       |  16 +--
 include/linux/pxa2xx_ssp.h     |   9 +-
 include/linux/spi/pxa2xx_spi.h |  12 +-
 7 files changed, 135 insertions(+), 172 deletions(-)
Signature · 1 line
2.30.2
author reviewer
#1 lore raw

PATCH 1/9 spi: pxa2xx: Propagate firmware node to the child SPI controller device

SPI core may utilize properties and resources provided by the parent device. Propagate firmware node to the child SPI controller device for that.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-pxa2xx.c +3 -1
index 5985b39e2dd6..38eef9033468 100644
@@ -1718,7 +1718,9 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	drv_data->controller_info = platform_info;
 	drv_data->ssp = ssp;
 
-	controller->dev.of_node = pdev->dev.of_node;
+	controller->dev.of_node = dev->of_node;
+	controller->dev.fwnode = dev->fwnode;
+
 	/* the spi->mode bits understood by this driver: */
 	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LOOP;
Signature · 1 line
2.30.2
author reviewer
#2 lore raw

PATCH 2/9 spi: pxa2xx: Switch to use SPI core GPIO (descriptor) CS handling

In reply to Andy Shevchenko

SPI core has been already providing the GPIO CS handling. Use it.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 51 +++-------------------------------------
 drivers/spi/spi-pxa2xx.h |  3 ---
 2 files changed, 3 insertions(+), 51 deletions(-)
drivers/spi/spi-pxa2xx.c +3 -48
index 38eef9033468..d19cea4ed946 100644
@@ -1198,25 +1198,12 @@ static int pxa2xx_spi_unprepare_transfer(struct spi_controller *controller)
 static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 		    struct pxa2xx_spi_chip *chip_info)
 {
-	struct driver_data *drv_data =
-		spi_controller_get_devdata(spi->controller);
 	struct gpio_desc *gpiod;
 	int err = 0;
 
 	if (chip == NULL)
 		return 0;
 
-	if (drv_data->cs_gpiods) {
-		gpiod = drv_data->cs_gpiods[spi->chip_select];
-		if (gpiod) {
-			chip->gpiod_cs = gpiod;
-			chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
-			gpiod_set_value(gpiod, chip->gpio_cs_inverted);
-		}
-
-		return 0;
-	}
-
 	if (chip_info == NULL)
 		return 0;
 

@@ -1430,8 +1417,7 @@ static void cleanup(struct spi_device *spi)
 	if (!chip)
 		return;
 
-	if (drv_data->ssp_type != CE4100_SSP && !drv_data->cs_gpiods &&
-	    chip->gpiod_cs)
+	if (drv_data->ssp_type != CE4100_SSP && chip->gpiod_cs)
 		gpiod_put(chip->gpiod_cs);
 
 	kfree(chip);

@@ -1682,7 +1668,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	struct driver_data *drv_data;
 	struct ssp_device *ssp;
 	const struct lpss_config *config;
-	int status, count;
+	int status;
 	u32 tmp;
 
 	platform_info = dev_get_platdata(dev);

@@ -1861,38 +1847,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 		}
 	}
 	controller->num_chipselect = platform_info->num_chipselect;
-
-	count = gpiod_count(&pdev->dev, "cs");
-	if (count > 0) {
-		int i;
-
-		controller->num_chipselect = max_t(int, count,
-			controller->num_chipselect);
-
-		drv_data->cs_gpiods = devm_kcalloc(&pdev->dev,
-			controller->num_chipselect, sizeof(struct gpio_desc *),
-			GFP_KERNEL);
-		if (!drv_data->cs_gpiods) {
-			status = -ENOMEM;
-			goto out_error_clock_enabled;
-		}
-
-		for (i = 0; i < controller->num_chipselect; i++) {
-			struct gpio_desc *gpiod;
-
-			gpiod = devm_gpiod_get_index(dev, "cs", i, GPIOD_ASIS);
-			if (IS_ERR(gpiod)) {
-				/* Means use native chip select */
-				if (PTR_ERR(gpiod) == -ENOENT)
-					continue;
-
-				status = PTR_ERR(gpiod);
-				goto out_error_clock_enabled;
-			} else {
-				drv_data->cs_gpiods[i] = gpiod;
-			}
-		}
-	}
+	controller->use_gpio_descriptors = true;
 
 	if (platform_info->is_slave) {
 		drv_data->gpiod_ready = devm_gpiod_get_optional(dev,
drivers/spi/spi-pxa2xx.h +0 -3
index 5c6a5e0f249e..672dfd4863cc 100644
@@ -53,9 +53,6 @@ struct driver_data {
 
 	void __iomem *lpss_base;
 
-	/* GPIOs for chip selects */
-	struct gpio_desc **cs_gpiods;
-
 	/* Optional slave FIFO ready signal */
 	struct gpio_desc *gpiod_ready;
 };
Signature · 1 line
2.30.2
author reviewer
#3 lore raw

PATCH 3/9 spi: pxa2xx: Switch to use SPI core GPIO (legacy) CS handling

In reply to Andy Shevchenko

SPI core has been already providing the GPIO CS handling. Use it.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 59 ++++++++++++++++++----------------------
 drivers/spi/spi-pxa2xx.h | 12 ++++----
 2 files changed, 32 insertions(+), 39 deletions(-)
drivers/spi/spi-pxa2xx.c +27 -32
index d19cea4ed946..1a0bcd3bac1f 100644
@@ -441,11 +441,6 @@ static void cs_assert(struct spi_device *spi)
 		return;
 	}
 
-	if (chip->gpiod_cs) {
-		gpiod_set_value(chip->gpiod_cs, chip->gpio_cs_inverted);
-		return;
-	}
-
 	if (is_lpss_ssp(drv_data))
 		lpss_ssp_cs_control(spi, true);
 }

@@ -471,11 +466,6 @@ static void cs_deassert(struct spi_device *spi)
 		return;
 	}
 
-	if (chip->gpiod_cs) {
-		gpiod_set_value(chip->gpiod_cs, !chip->gpio_cs_inverted);
-		return;
-	}
-
 	if (is_lpss_ssp(drv_data))
 		lpss_ssp_cs_control(spi, false);
 }

@@ -1195,11 +1185,19 @@ static int pxa2xx_spi_unprepare_transfer(struct spi_controller *controller)
 	return 0;
 }
 
+static void cleanup_cs(struct spi_device *spi)
+{
+	if (!gpio_is_valid(spi->cs_gpio))
+		return;
+
+	gpio_free(spi->cs_gpio);
+	spi->cs_gpio = -ENOENT;
+}
+
 static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 		    struct pxa2xx_spi_chip *chip_info)
 {
-	struct gpio_desc *gpiod;
-	int err = 0;
+	struct driver_data *drv_data = spi_controller_get_devdata(spi->controller);
 
 	if (chip == NULL)
 		return 0;

@@ -1207,13 +1205,13 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	if (chip_info == NULL)
 		return 0;
 
+	if (drv_data->ssp_type == CE4100_SSP)
+		return 0;
+
 	/* NOTE: setup() can be called multiple times, possibly with
 	 * different chip_info, release previously requested GPIO
 	 */
-	if (chip->gpiod_cs) {
-		gpiod_put(chip->gpiod_cs);
-		chip->gpiod_cs = NULL;
-	}
+	cleanup_cs(spi);
 
 	/* If (*cs_control) is provided, ignore GPIO chip select */
 	if (chip_info->cs_control) {

@@ -1222,21 +1220,25 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	}
 
 	if (gpio_is_valid(chip_info->gpio_cs)) {
-		err = gpio_request(chip_info->gpio_cs, "SPI_CS");
+		int gpio = chip_info->gpio_cs;
+		int err;
+
+		err = gpio_request(gpio, "SPI_CS");
 		if (err) {
-			dev_err(&spi->dev, "failed to request chip select GPIO%d\n",
-				chip_info->gpio_cs);
+			dev_err(&spi->dev, "failed to request chip select GPIO%d\n", gpio);
 			return err;
 		}
 
-		gpiod = gpio_to_desc(chip_info->gpio_cs);
-		chip->gpiod_cs = gpiod;
-		chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
+		err = gpio_direction_output(gpio, !(spi->mode & SPI_CS_HIGH));
+		if (err) {
+			gpio_free(gpio);
+			return err;
+		}
 
-		err = gpiod_direction_output(gpiod, !chip->gpio_cs_inverted);
+		spi->cs_gpio = gpio;
 	}
 
-	return err;
+	return 0;
 }
 
 static int setup(struct spi_device *spi)

@@ -1411,15 +1413,8 @@ static int setup(struct spi_device *spi)
 static void cleanup(struct spi_device *spi)
 {
 	struct chip_data *chip = spi_get_ctldata(spi);
-	struct driver_data *drv_data =
-		spi_controller_get_devdata(spi->controller);
-
-	if (!chip)
-		return;
-
-	if (drv_data->ssp_type != CE4100_SSP && chip->gpiod_cs)
-		gpiod_put(chip->gpiod_cs);
 
+	cleanup_cs(spi);
 	kfree(chip);
 }
drivers/spi/spi-pxa2xx.h +5 -7
index 672dfd4863cc..a91fe6edb275 100644
@@ -62,19 +62,17 @@ struct chip_data {
 	u32 dds_rate;
 	u32 timeout;
 	u8 n_bytes;
+	u8 enable_dma;
 	u32 dma_burst_size;
-	u32 threshold;
 	u32 dma_threshold;
+	u32 threshold;
 	u16 lpss_rx_threshold;
 	u16 lpss_tx_threshold;
-	u8 enable_dma;
-	union {
-		struct gpio_desc *gpiod_cs;
-		unsigned int frm;
-	};
-	int gpio_cs_inverted;
+
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
+
+	unsigned int frm;
 	void (*cs_control)(u32 command);
 };
Signature · 1 line
2.30.2
author reviewer
#4 lore raw

PATCH 4/9 spi: pxa2xx: Drop duplicate chip_select in struct chip_data

In reply to Andy Shevchenko

The struct chip_data had been introduced in order to keep the parameters that may be provided on stack during device allocation. There is no need to duplicate parameters there, which are carried on by SPI device itself.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 4 +---
 drivers/spi/spi-pxa2xx.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)
drivers/spi/spi-pxa2xx.c +1 -3
index 1a0bcd3bac1f..fb80f6013d54 100644
@@ -432,7 +432,7 @@ static void cs_assert(struct spi_device *spi)
 		spi_controller_get_devdata(spi->controller);
 
 	if (drv_data->ssp_type == CE4100_SSP) {
-		pxa2xx_spi_write(drv_data, SSSR, chip->frm);
+		pxa2xx_spi_write(drv_data, SSSR, spi->chip_select);
 		return;
 	}
 

@@ -1303,8 +1303,6 @@ static int setup(struct spi_device *spi)
 				kfree(chip);
 				return -EINVAL;
 			}
-
-			chip->frm = spi->chip_select;
 		}
 		chip->enable_dma = drv_data->controller_info->enable_dma;
 		chip->timeout = TIMOUT_DFLT;
drivers/spi/spi-pxa2xx.h +0 -1
index a91fe6edb275..db9de46110ad 100644
@@ -72,7 +72,6 @@ struct chip_data {
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
 
-	unsigned int frm;
 	void (*cs_control)(u32 command);
 };
Signature · 1 line
2.30.2
author reviewer
#5 lore raw

PATCH 5/9 spi: pxa2xx: Drop unneeded '!= 0' comparisons

In reply to Andy Shevchenko

In the few places it's redundant to compare against 0. Drop the unneeded comparisons.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/spi/spi-pxa2xx.c +4 -4
index fb80f6013d54..f24851b3c020 100644
@@ -1380,8 +1380,8 @@ static int setup(struct spi_device *spi)
 	}
 
 	chip->cr1 &= ~(SSCR1_SPO | SSCR1_SPH);
-	chip->cr1 |= (((spi->mode & SPI_CPHA) != 0) ? SSCR1_SPH : 0)
-			| (((spi->mode & SPI_CPOL) != 0) ? SSCR1_SPO : 0);
+	chip->cr1 |= ((spi->mode & SPI_CPHA) ? SSCR1_SPH : 0) |
+		     ((spi->mode & SPI_CPOL) ? SSCR1_SPO : 0);
 
 	if (spi->mode & SPI_LOOP)
 		chip->cr1 |= SSCR1_LBM;

@@ -1859,7 +1859,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	/* Register with the SPI framework */
 	platform_set_drvdata(pdev, drv_data);
 	status = spi_register_controller(controller);
-	if (status != 0) {
+	if (status) {
 		dev_err(&pdev->dev, "problem registering spi controller\n");
 		goto out_error_pm_runtime_enabled;
 	}

@@ -1918,7 +1918,7 @@ static int pxa2xx_spi_suspend(struct device *dev)
 	int status;
 
 	status = spi_controller_suspend(drv_data->controller);
-	if (status != 0)
+	if (status)
 		return status;
 
 	pxa_ssp_disable(ssp);
Signature · 1 line
2.30.2
author reviewer
#6 lore raw

PATCH 6/9 spi: pxa2xx: Fix printf() specifiers

In reply to Andy Shevchenko

Instead of explicit casting use proper specifier in one case, and fix specifier signness in another.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/spi/spi-pxa2xx.c +3 -4
index f24851b3c020..f8264771b360 100644
@@ -983,8 +983,8 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *controller,
 
 		/* warn ... we force this to PIO mode */
 		dev_warn_ratelimited(&spi->dev,
-				     "DMA disabled for transfer length %ld greater than %d\n",
-				     (long)transfer->len, MAX_DMA_LEN);
+				     "DMA disabled for transfer length %u greater than %d\n",
+				     transfer->len, MAX_DMA_LEN);
 	}
 
 	/* Setup the transfer state based on the type of transfer */

@@ -1115,8 +1115,7 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *controller,
 
 		if (tx_level) {
 			/* On MMP2, flipping SSE doesn't to empty TXFIFO. */
-			dev_warn(&spi->dev, "%d bytes of garbage in TXFIFO!\n",
-								tx_level);
+			dev_warn(&spi->dev, "%u bytes of garbage in Tx FIFO!\n", tx_level);
 			if (tx_level > transfer->len)
 				tx_level = transfer->len;
 			drv_data->tx += tx_level;
Signature · 1 line
2.30.2
author reviewer
#7 lore raw

PATCH 7/9 spi: pxa2xx: Fix style of and typos in the comments and messages

In reply to Andy Shevchenko

Fix style of the comments and messages along with typos in them.

While at it, update Intel Copyright year.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx-dma.c   |  4 +-
 drivers/spi/spi-pxa2xx-pci.c   |  5 ++-
 drivers/spi/spi-pxa2xx.c       | 71 ++++++++++++++++++----------------
 drivers/spi/spi-pxa2xx.h       |  2 +-
 include/linux/pxa2xx_ssp.h     |  9 +++--
 include/linux/spi/pxa2xx_spi.h | 12 ++++--
 6 files changed, 58 insertions(+), 45 deletions(-)
drivers/spi/spi-pxa2xx-dma.c +2 -2
index f022d82dcb1b..be563f0dd03a 100644
@@ -2,7 +2,7 @@
 /*
  * PXA2xx SPI DMA engine support.
  *
- * Copyright (C) 2013, Intel Corporation
+ * Copyright (C) 2013, 2021 Intel Corporation
  * Author: Mika Westerberg <[email protected]>
  */
 

@@ -26,7 +26,7 @@ static void pxa2xx_spi_dma_transfer_complete(struct driver_data *drv_data,
 	 * It is possible that one CPU is handling ROR interrupt and other
 	 * just gets DMA completion. Calling pump_transfers() twice for the
 	 * same transfer leads to problems thus we prevent concurrent calls
-	 * by using ->dma_running.
+	 * by using dma_running.
 	 */
 	if (atomic_dec_and_test(&drv_data->dma_running)) {
 		/*
drivers/spi/spi-pxa2xx-pci.c +3 -2
index dce9ade9a4df..9c9992d4f547 100644
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * CE4100's SPI device is more or less the same one as found on PXA
+ * PCI glue driver for SPI PXA2xx compatible controllers.
+ * CE4100's SPI device is more or less the same one as found on PXA.
  *
- * Copyright (C) 2016, Intel Corporation
+ * Copyright (C) 2016, 2021 Intel Corporation
  */
 #include <linux/clk-provider.h>
 #include <linux/module.h>
drivers/spi/spi-pxa2xx.c +38 -33

202 lines not on this page · raw message

drivers/spi/spi-pxa2xx.h +1 -1
index db9de46110ad..9a20fb88e50f 100644
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
  * Copyright (C) 2005 Stephen Street / StreetFire Sound Labs
- * Copyright (C) 2013, Intel Corporation
+ * Copyright (C) 2013, 2021 Intel Corporation
  */
 
 #ifndef SPI_PXA2XX_H
include/linux/pxa2xx_ssp.h +5 -4
index 2b21bc1f3c73..a3fec2de512f 100644
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- *  Copyright (C) 2003 Russell King, All Rights Reserved.
+ * Copyright (C) 2003 Russell King, All Rights Reserved.
  *
  * This driver supports the following PXA CPU/SSP ports:-
  *

@@ -59,7 +59,7 @@ struct device_node;
 /* PXA27x, PXA3xx */
 #define SSCR0_EDSS	BIT(20)		/* Extended data size select */
 #define SSCR0_NCS	BIT(21)		/* Network clock select */
-#define SSCR0_RIM	BIT(22)		/* Receive FIFO overrrun interrupt mask */
+#define SSCR0_RIM	BIT(22)		/* Receive FIFO overrun interrupt mask */
 #define SSCR0_TUM	BIT(23)		/* Transmit FIFO underrun interrupt mask */
 #define SSCR0_FRDC	GENMASK(26, 24)	/* Frame rate divider control (mask) */
 #define SSCR0_SlotsPerFrm(x) (((x) - 1) << 24)	/* Time slots per frame [1..8] */

@@ -126,7 +126,7 @@ struct device_node;
 #define QUARK_X1000_SSCR1_EFWR	BIT(16)		/* Enable FIFO Write/Read */
 #define QUARK_X1000_SSCR1_STRF	BIT(17)		/* Select FIFO or EFWR */
 
-/* extra bits in PXA255, PXA26x and PXA27x SSP ports */
+/* Extra bits in PXA255, PXA26x and PXA27x SSP ports */
 #define SSCR0_TISSP		(1 << 4)	/* TI Sync Serial Protocol */
 #define SSCR0_PSP		(3 << 4)	/* PSP - Programmable Serial Protocol */
 

@@ -222,7 +222,8 @@ enum pxa_ssp_type {
 	CE4100_SSP,
 	MRFLD_SSP,
 	QUARK_X1000_SSP,
-	LPSS_LPT_SSP, /* Keep LPSS types sorted with lpss_platforms[] */
+	/* Keep LPSS types sorted with lpss_platforms[] */
+	LPSS_LPT_SSP,
 	LPSS_BYT_SSP,
 	LPSS_BSW_SSP,
 	LPSS_SPT_SSP,
include/linux/spi/pxa2xx_spi.h +9 -3
index 12ef04d0896d..eaab121ee575 100644
@@ -14,7 +14,10 @@
 
 struct dma_chan;
 
-/* device.platform_data for SSP controller devices */
+/*
+ * The platform data for SSP controller devices
+ * (resides in device.platform_data).
+ */
 struct pxa2xx_spi_controller {
 	u16 num_chipselect;
 	u8 enable_dma;

@@ -30,8 +33,11 @@ struct pxa2xx_spi_controller {
 	struct ssp_device ssp;
 };
 
-/* spi_board_info.controller_data for SPI slave devices,
- * copied to spi_device.platform_data ... mostly for dma tuning
+/*
+ * The controller specific data for SPI slave devices
+ * (resides in spi_board_info.controller_data),
+ * copied to spi_device.platform_data ... mostly for
+ * DMA tuning.
  */
 struct pxa2xx_spi_chip {
 	u8 tx_threshold;
Signature · 1 line
2.30.2
author reviewer
#8 lore raw

PATCH 8/9 spi: pxa2xx: Update documentation to point out that it's outdated

In reply to Andy Shevchenko

Update documentation by pointing out that it's applicable mostly for a legacy platform. While at it, add couple of points with regard to ACPI, Device Tree, and automatic DMA enablement.

Signed-off-by Andy Shevchenko <[email protected]>
---
 Documentation/spi/pxa2xx.rst | 58 ++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 26 deletions(-)
Documentation/spi/pxa2xx.rst +32 -26
index 882d3cc72cc2..6312968acfe9 100644
@@ -2,43 +2,47 @@
 PXA2xx SPI on SSP driver HOWTO
 ==============================
 
-This a mini howto on the pxa2xx_spi driver.  The driver turns a PXA2xx
-synchronous serial port into a SPI master controller
+This a mini HOWTO on the pxa2xx_spi driver. The driver turns a PXA2xx
+synchronous serial port into an SPI master controller
 (see Documentation/spi/spi-summary.rst). The driver has the following features
 
-- Support for any PXA2xx SSP
+- Support for any PXA2xx and compatible SSP.
 - SSP PIO and SSP DMA data transfers.
 - External and Internal (SSPFRM) chip selects.
 - Per slave device (chip) configuration.
 - Full suspend, freeze, resume support.
 
-The driver is built around a "spi_message" fifo serviced by workqueue and a
-tasklet. The workqueue, "pump_messages", drives message fifo and the tasklet
-(pump_transfer) is responsible for queuing SPI transactions and setting up and
-launching the dma/interrupt driven transfers.
+The driver is built around a &struct spi_message FIFO serviced by kernel
+thread. The kernel thread, spi_pump_messages(), drives message FIFO and
+is responsible for queuing SPI transactions and setting up and launching
+the DMA or interrupt driven transfers.
 
 Declaring PXA2xx Master Controllers
 -----------------------------------
-Typically a SPI master is defined in the arch/.../mach-*/board-*.c as a
-"platform device".  The master configuration is passed to the driver via a table
-found in include/linux/spi/pxa2xx_spi.h::
+Typically, for a legacy platform, an SPI master is defined in the
+arch/.../mach-*/board-*.c as a "platform device". The master configuration
+is passed to the driver via a table found in include/linux/spi/pxa2xx_spi.h::
 
   struct pxa2xx_spi_controller {
 	u16 num_chipselect;
 	u8 enable_dma;
+	...
   };
 
 The "pxa2xx_spi_controller.num_chipselect" field is used to determine the number of
 slave device (chips) attached to this SPI master.
 
 The "pxa2xx_spi_controller.enable_dma" field informs the driver that SSP DMA should
-be used.  This caused the driver to acquire two DMA channels: rx_channel and
-tx_channel.  The rx_channel has a higher DMA service priority the tx_channel.
+be used. This caused the driver to acquire two DMA channels: Rx channel and
+Tx channel. The Rx channel has a higher DMA service priority than the Tx channel.
 See the "PXA2xx Developer Manual" section "DMA Controller".
 
+For the new platforms the description of the controller and peripheral devices
+comes from Device Tree or ACPI.
+
 NSSP MASTER SAMPLE
 ------------------
-Below is a sample configuration using the PXA255 NSSP::
+Below is a sample configuration using the PXA255 NSSP for a legacy platform::
 
   static struct resource pxa_spi_nssp_resources[] = {
 	[0] = {

@@ -79,9 +83,10 @@ Below is a sample configuration using the PXA255 NSSP::
 
 Declaring Slave Devices
 -----------------------
-Typically each SPI slave (chip) is defined in the arch/.../mach-*/board-*.c
-using the "spi_board_info" structure found in "linux/spi/spi.h". See
-"Documentation/spi/spi-summary.rst" for additional information.
+Typically, for a legacy platform, each SPI slave (chip) is defined in the
+arch/.../mach-*/board-*.c using the "spi_board_info" structure found in
+"linux/spi/spi.h". See "Documentation/spi/spi-summary.rst" for additional
+information.
 
 Each slave device attached to the PXA must provide slave specific configuration
 information via the structure "pxa2xx_spi_chip" found in

@@ -101,9 +106,9 @@ device. All fields are optional.
   };
 
 The "pxa2xx_spi_chip.tx_threshold" and "pxa2xx_spi_chip.rx_threshold" fields are
-used to configure the SSP hardware fifo.  These fields are critical to the
+used to configure the SSP hardware FIFO. These fields are critical to the
 performance of pxa2xx_spi driver and misconfiguration will result in rx
-fifo overruns (especially in PIO mode transfers). Good default values are::
+FIFO overruns (especially in PIO mode transfers). Good default values are::
 
 	.tx_threshold = 8,
 	.rx_threshold = 8,

@@ -118,7 +123,7 @@ use a value of 8. The driver will determine a reasonable default if
 dma_burst_size == 0.
 
 The "pxa2xx_spi_chip.timeout" fields is used to efficiently handle
-trailing bytes in the SSP receiver fifo.  The correct value for this field is
+trailing bytes in the SSP receiver FIFO. The correct value for this field is
 dependent on the SPI bus speed ("spi_board_info.max_speed_hz") and the specific
 slave device.  Please note that the PXA2xx SSP 1 does not support trailing byte
 timeouts and must busy-wait any trailing bytes.

@@ -131,19 +136,19 @@ testing.
 The "pxa2xx_spi_chip.cs_control" field is used to point to a board specific
 function for asserting/deasserting a slave device chip select.  If the field is
 NULL, the pxa2xx_spi master controller driver assumes that the SSP port is
-configured to use SSPFRM instead.
+configured to use GPIO or SSPFRM instead.
 
 NOTE: the SPI driver cannot control the chip select if SSPFRM is used, so the
 chipselect is dropped after each spi_transfer.  Most devices need chip select
-asserted around the complete message.  Use SSPFRM as a GPIO (through cs_control)
+asserted around the complete message. Use SSPFRM as a GPIO (through a descriptor)
 to accommodate these chips.
 
 
 NSSP SLAVE SAMPLE
 -----------------
-The pxa2xx_spi_chip structure is passed to the pxa2xx_spi driver in the
-"spi_board_info.controller_data" field. Below is a sample configuration using
-the PXA255 NSSP.
+For a legacy platform or in some other cases, the pxa2xx_spi_chip structure
+is passed to the pxa2xx_spi driver in the "spi_board_info.controller_data"
+field. Below is a sample configuration using the PXA255 NSSP.
 
 ::
 

@@ -212,7 +217,9 @@ DMA and PIO I/O Support
 -----------------------
 The pxa2xx_spi driver supports both DMA and interrupt driven PIO message
 transfers.  The driver defaults to PIO mode and DMA transfers must be enabled
-by setting the "enable_dma" flag in the "pxa2xx_spi_controller" structure.  The DMA
+by setting the "enable_dma" flag in the "pxa2xx_spi_controller" structure.
+For the newer platforms, that are known to support DMA, the driver will enable
+it automatically and try it first with a possible fallback to PIO. The DMA
 mode supports both coherent and stream based DMA mappings.
 
 The following logic is used to determine the type of I/O to be used on

@@ -236,5 +243,4 @@ a per "spi_transfer" basis::
 
 THANKS TO
 ---------
-
 David Brownell and others for mentoring the development of this driver.
Signature · 1 line
2.30.2
author reviewer
#9 lore raw

PATCH 9/9 spi: pxa2xx: Use predefined mask when programming FIFO thresholds

In reply to Andy Shevchenko

The predefined mask for threshold modification can be used in case of Intel Merrifield SPI. Replace open-coded value with predefined mask when programming FIFO thresholds.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/spi/spi-pxa2xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/spi/spi-pxa2xx.c +2 -1
index 94b1585de203..bdf9a283efc3 100644
@@ -1083,12 +1083,13 @@ static int pxa2xx_spi_transfer_one(struct spi_controller *controller,
 	}
 
 	if (is_mrfld_ssp(drv_data)) {
+		u32 mask = SFIFOTT_RFT | SFIFOTT_TFT;
 		u32 thresh = 0;
 
 		thresh |= SFIFOTT_RxThresh(chip->lpss_rx_threshold);
 		thresh |= SFIFOTT_TxThresh(chip->lpss_tx_threshold);
 
-		pxa2xx_spi_update(drv_data, SFIFOTT, 0xffffffff, thresh);
+		pxa2xx_spi_update(drv_data, SFIFOTT, mask, thresh);
 	}
 
 	if (is_quark_x1000_ssp(drv_data))
Signature · 1 line
2.30.2
maintainer
#10 lore raw
In reply to Andy Shevchenko
Quoted 17 lines · Andy Shevchenko

Convert the PXA2xx SPI host controller driver to use SPI core GPIO handling. On top of that a few additional cleanups.

Has been tested on Intel Merrifield with SPI ADC and TFT display connected.

Andy Shevchenko (9):
spi: pxa2xx: Propagate firmware node to the child SPI controller
device
spi: pxa2xx: Switch to use SPI core GPIO (descriptor) CS handling
spi: pxa2xx: Switch to use SPI core GPIO (legacy) CS handling
spi: pxa2xx: Drop duplicate chip_select in struct chip_data
spi: pxa2xx: Drop unneeded '!= 0' comparisons
spi: pxa2xx: Fix printf() specifiers
spi: pxa2xx: Fix style of and typos in the comments and messages
spi: pxa2xx: Update documentation to point out that it's outdated
spi: pxa2xx: Use predefined mask when programming FIFO thresholds

Quoted 1 line

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/9] spi: pxa2xx: Propagate firmware node to the child SPI controller device

      commit: 94acf80755c8d8dd066d4f5a2afbdf393f0e2afd
[2/9] spi: pxa2xx: Switch to use SPI core GPIO (descriptor) CS handling
      commit: 778c12e69481d544e6fcfa45b23ae3c5379b5a02
[3/9] spi: pxa2xx: Switch to use SPI core GPIO (legacy) CS handling
      commit: de6926f307e7ff605f3e37f11a4b3cc20c85c365
[4/9] spi: pxa2xx: Drop duplicate chip_select in struct chip_data
      commit: ccd60b2030a04b91977d9fee9e0ec6469ac4cd1b
[5/9] spi: pxa2xx: Drop unneeded '!= 0' comparisons
      commit: eb743ec600be596553bf4e42c85b0bbc65083791
[6/9] spi: pxa2xx: Fix printf() specifiers
      commit: 684a3ac720c3fd4c09ae5aa3d349861bf12dcff6
[7/9] spi: pxa2xx: Fix style of and typos in the comments and messages
      commit: 8083d6b812cac5e38db9c707b41cd478beed4a0c
[8/9] spi: pxa2xx: Update documentation to point out that it's outdated
      commit: f96e6c0ef63b981d295547ef624f4da7c820e097
[9/9] spi: pxa2xx: Use predefined mask when programming FIFO thresholds
      commit: 70252440b2b6337d03f2b95bc475fedbea79072f

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark