[PATCH v1 1/3] ARM: pxa/lubbock: Replace custom ->cs_control() by GPIO lookup table

[PATCH v1 1/3] ARM: pxa/lubbock: Replace custom ->cs_control() by GPIO lookup table

[Andy Shevchenko]

SPI PXA2xx driver supports GPIO chipselect by querying for known
GPIO connection ID. Replace custom ->cs_control() by GPIO table,
so the driver will use generic approach on this platform.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/arm/mach-pxa/lubbock.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-pxa/lubbock.c b/arch/arm/mach-pxa/lubbock.c
index 742d18a1f7dc..e2411971422d 100644
--- a/arch/arm/mach-pxa/lubbock.c
+++ b/arch/arm/mach-pxa/lubbock.c
@@ -211,16 +211,17 @@ static struct ads7846_platform_data ads_info = {
 	// .y_plate_ohms		= 500,	/* GUESS! */
 };
 
-static void ads7846_cs(u32 command)
-{
-	static const unsigned	TS_nCS = 1 << 11;
-	lubbock_set_misc_wr(TS_nCS, (command == PXA2XX_CS_ASSERT) ? 0 : TS_nCS);
-}
+static struct gpiod_lookup_table ads7846_cs_gpios = {
+	.dev_id		= "ads7846",
+	.table		= {
+		GPIO_LOOKUP("lubbock", 11, "cs", GPIO_ACTIVE_LOW),
+		{}
+	},
+};
 
 static struct pxa2xx_spi_chip ads_hw = {
 	.tx_threshold		= 1,
 	.rx_threshold		= 2,
-	.cs_control		= ads7846_cs,
 };
 
 static struct spi_board_info spi_board_info[] __initdata = { {
@@ -512,6 +513,8 @@ static void __init lubbock_init(void)
 	lubbock_flash_data[flashboot].name = "boot-rom";
 	(void) platform_add_devices(devices, ARRAY_SIZE(devices));
 
+	gpiod_add_lookup_table(&ads7846_cs_gpios);
+
 	pxa2xx_set_spi_info(1, &pxa_ssp_master_info);
 	spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info));
 }
-- 
2.33.0

[PATCH v1 2/3] spi: pxa2xx: Get rid of unused ->cs_control()

[Andy Shevchenko]

Since the last user of the custom ->cs_control() gone, we may get rid of
this legacy API completely.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/spi/pxa2xx.rst   | 29 +++--------------------------
 arch/arm/mach-pxa/stargate2.c  |  2 +-
 drivers/spi/spi-pxa2xx.c       | 18 ------------------
 drivers/spi/spi-pxa2xx.h       |  3 ---
 include/linux/spi/pxa2xx_spi.h |  4 ----
 5 files changed, 4 insertions(+), 52 deletions(-)

diff --git a/Documentation/spi/pxa2xx.rst b/Documentation/spi/pxa2xx.rst
index 6312968acfe9..dfc7673ed15d 100644
--- a/Documentation/spi/pxa2xx.rst
+++ b/Documentation/spi/pxa2xx.rst
@@ -102,7 +102,7 @@ device. All fields are optional.
 	u8 dma_burst_size;
 	u32 timeout;
 	u8 enable_loopback;
-	void (*cs_control)(u32 command);
+	int gpio_cs;
   };
 
 The "pxa2xx_spi_chip.tx_threshold" and "pxa2xx_spi_chip.rx_threshold" fields are
@@ -133,11 +133,6 @@ into internal loopback mode.  In this mode the SSP controller internally
 connects the SSPTX pin to the SSPRX pin.  This is useful for initial setup
 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 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 a descriptor)
@@ -152,30 +147,12 @@ field. Below is a sample configuration using the PXA255 NSSP.
 
 ::
 
-  /* Chip Select control for the CS8415A SPI slave device */
-  static void cs8415a_cs_control(u32 command)
-  {
-	if (command & PXA2XX_CS_ASSERT)
-		GPCR(2) = GPIO_bit(2);
-	else
-		GPSR(2) = GPIO_bit(2);
-  }
-
-  /* Chip Select control for the CS8405A SPI slave device */
-  static void cs8405a_cs_control(u32 command)
-  {
-	if (command & PXA2XX_CS_ASSERT)
-		GPCR(3) = GPIO_bit(3);
-	else
-		GPSR(3) = GPIO_bit(3);
-  }
-
   static struct pxa2xx_spi_chip cs8415a_chip_info = {
 	.tx_threshold = 8, /* SSP hardward FIFO threshold */
 	.rx_threshold = 8, /* SSP hardward FIFO threshold */
 	.dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
 	.timeout = 235, /* See Intel documentation */
-	.cs_control = cs8415a_cs_control, /* Use external chip select */
+	.gpio_cs = 2, /* Use external chip select */
   };
 
   static struct pxa2xx_spi_chip cs8405a_chip_info = {
@@ -183,7 +160,7 @@ field. Below is a sample configuration using the PXA255 NSSP.
 	.rx_threshold = 8, /* SSP hardward FIFO threshold */
 	.dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
 	.timeout = 235, /* See Intel documentation */
-	.cs_control = cs8405a_cs_control, /* Use external chip select */
+	.gpio_cs = 3, /* Use external chip select */
   };
 
   static struct spi_board_info streetracer_spi_board_info[] __initdata = {
diff --git a/arch/arm/mach-pxa/stargate2.c b/arch/arm/mach-pxa/stargate2.c
index 7ad627465768..8ca02ec1d44c 100644
--- a/arch/arm/mach-pxa/stargate2.c
+++ b/arch/arm/mach-pxa/stargate2.c
@@ -347,7 +347,7 @@ static struct pxa2xx_spi_controller pxa_ssp_master_2_info = {
 };
 
 /* An upcoming kernel change will scrap SFRM usage so these
- * drivers have been moved to use gpio's via cs_control */
+ * drivers have been moved to use GPIOs */
 static struct pxa2xx_spi_chip staccel_chip_info = {
 	.tx_threshold = 8,
 	.rx_threshold = 8,
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index ee3297dd532e..24196156c609 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -427,7 +427,6 @@ static void lpss_ssp_cs_control(struct spi_device *spi, bool enable)
 
 static void cs_assert(struct spi_device *spi)
 {
-	struct chip_data *chip = spi_get_ctldata(spi);
 	struct driver_data *drv_data =
 		spi_controller_get_devdata(spi->controller);
 
@@ -436,18 +435,12 @@ static void cs_assert(struct spi_device *spi)
 		return;
 	}
 
-	if (chip->cs_control) {
-		chip->cs_control(PXA2XX_CS_ASSERT);
-		return;
-	}
-
 	if (is_lpss_ssp(drv_data))
 		lpss_ssp_cs_control(spi, true);
 }
 
 static void cs_deassert(struct spi_device *spi)
 {
-	struct chip_data *chip = spi_get_ctldata(spi);
 	struct driver_data *drv_data =
 		spi_controller_get_devdata(spi->controller);
 	unsigned long timeout;
@@ -461,11 +454,6 @@ static void cs_deassert(struct spi_device *spi)
 	       !time_after(jiffies, timeout))
 		cpu_relax();
 
-	if (chip->cs_control) {
-		chip->cs_control(PXA2XX_CS_DEASSERT);
-		return;
-	}
-
 	if (is_lpss_ssp(drv_data))
 		lpss_ssp_cs_control(spi, false);
 }
@@ -1204,12 +1192,6 @@ static int setup_cs(struct spi_device *spi, struct chip_data *chip,
 	 */
 	cleanup_cs(spi);
 
-	/* If ->cs_control() is provided, ignore GPIO chip select */
-	if (chip_info->cs_control) {
-		chip->cs_control = chip_info->cs_control;
-		return 0;
-	}
-
 	if (gpio_is_valid(chip_info->gpio_cs)) {
 		int gpio = chip_info->gpio_cs;
 		int err;
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 4d77f4de6eda..45cdbbc71c4b 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -49,7 +49,6 @@ struct driver_data {
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
 	irqreturn_t (*transfer_handler)(struct driver_data *drv_data);
-	void (*cs_control)(u32 command);
 
 	void __iomem *lpss_base;
 
@@ -67,8 +66,6 @@ struct chip_data {
 	u32 threshold;
 	u16 lpss_rx_threshold;
 	u16 lpss_tx_threshold;
-
-	void (*cs_control)(u32 command);
 };
 
 static inline u32 pxa2xx_spi_read(const struct driver_data *drv_data, u32 reg)
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index eaab121ee575..42e06bfbc2a4 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -9,9 +9,6 @@
 
 #include <linux/pxa2xx_ssp.h>
 
-#define PXA2XX_CS_ASSERT (0x01)
-#define PXA2XX_CS_DEASSERT (0x02)
-
 struct dma_chan;
 
 /*
@@ -47,7 +44,6 @@ struct pxa2xx_spi_chip {
 	u32 timeout;
 	u8 enable_loopback;
 	int gpio_cs;
-	void (*cs_control)(u32 command);
 };
 
 #if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
-- 
2.33.0

[PATCH v1 3/3] spi: pxa2xx: Get rid of unused enable_loopback member

[Andy Shevchenko]

There is no user of the enable_loopback member in the struct pxa2xx_spi_chip.
Remote this legacy member completely.

The mentioned in the documentation the testing phase can be performed with
spidev_test tool.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/spi/pxa2xx.rst   | 6 ------
 drivers/spi/spi-pxa2xx.c       | 5 ++---
 include/linux/spi/pxa2xx_spi.h | 1 -
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/Documentation/spi/pxa2xx.rst b/Documentation/spi/pxa2xx.rst
index dfc7673ed15d..6347580826be 100644
--- a/Documentation/spi/pxa2xx.rst
+++ b/Documentation/spi/pxa2xx.rst
@@ -101,7 +101,6 @@ device. All fields are optional.
 	u8 rx_threshold;
 	u8 dma_burst_size;
 	u32 timeout;
-	u8 enable_loopback;
 	int gpio_cs;
   };
 
@@ -128,11 +127,6 @@ 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.
 
-The "pxa2xx_spi_chip.enable_loopback" field is used to place the SSP porting
-into internal loopback mode.  In this mode the SSP controller internally
-connects the SSPTX pin to the SSPRX pin.  This is useful for initial setup
-testing.
-
 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 a descriptor)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 24196156c609..b3186bd0c2a8 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1289,7 +1289,6 @@ static int setup(struct spi_device *spi)
 	chip_info = spi->controller_data;
 
 	/* chip_info isn't always needed */
-	chip->cr1 = 0;
 	if (chip_info) {
 		if (chip_info->timeout)
 			chip->timeout = chip_info->timeout;
@@ -1300,9 +1299,9 @@ static int setup(struct spi_device *spi)
 		if (chip_info->rx_threshold)
 			rx_thres = chip_info->rx_threshold;
 		chip->dma_threshold = 0;
-		if (chip_info->enable_loopback)
-			chip->cr1 = SSCR1_LBM;
 	}
+
+	chip->cr1 = 0;
 	if (spi_controller_is_slave(drv_data->controller)) {
 		chip->cr1 |= SSCR1_SCFR;
 		chip->cr1 |= SSCR1_SCLKDIR;
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
index 42e06bfbc2a4..ca74dce36706 100644
--- a/include/linux/spi/pxa2xx_spi.h
+++ b/include/linux/spi/pxa2xx_spi.h
@@ -42,7 +42,6 @@ struct pxa2xx_spi_chip {
 	u8 rx_threshold;
 	u8 dma_burst_size;
 	u32 timeout;
-	u8 enable_loopback;
 	int gpio_cs;
 };
 
-- 
2.33.0

Re: [PATCH v1 1/3] ARM: pxa/lubbock: Replace custom ->cs_control() by GPIO lookup table

[Mark Brown]

On Tue, 23 Nov 2021 21:27:21 +0200, Andy Shevchenko wrote:
> SPI PXA2xx driver supports GPIO chipselect by querying for known
> GPIO connection ID. Replace custom ->cs_control() by GPIO table,
> so the driver will use generic approach on this platform.
> 
> 

Applied to

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

Thanks!

[1/3] ARM: pxa/lubbock: Replace custom ->cs_control() by GPIO lookup table
      commit: 342e3ce0f6f4691b31b1c7c9c3ae37160c4a82d2
[2/3] spi: pxa2xx: Get rid of unused ->cs_control()
      commit: a9c8f68ce2c37ced2f7a8667eda71b7753ede398
[3/3] spi: pxa2xx: Get rid of unused enable_loopback member
      commit: 8393961c53b31078cfc877bc00eb0f67e1474edd

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