[PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of
its content is being used solely internally to SPI subsystem
(PXA2xx drivers). Hence this refactoring series with the additional
win of getting rid of legacy documentation.

Changelog v2:
- dropped applied patches
- added patch to amend dependencies (Mark)
- amended the second patch accordingly (Mark)
- elaborated purpose of the patch 6 in the commit message (Mark)

Cc: Arnd Bergmann <arnd@arndb.de>

Andy Shevchenko (9):
  spi: pxa2xx: Narrow the Kconfig option visibility
  spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
  spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
  spi: pxa2xx: Skip SSP initialization if it's done elsewhere
  spi: pxa2xx: Allow number of chip select pins to be read from property
  spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties
  spi: pxa2xx: Move contents of linux/spi/pxa2xx_spi.h to a local one
  spi: pxa2xx: Remove outdated documentation
  spi: pxa2xx: Don't use "proxy" headers

 Documentation/spi/pxa2xx.rst   | 208 ---------------------------------
 arch/arm/mach-pxa/spitz.c      |  25 ++--
 drivers/spi/Kconfig            |   5 +-
 drivers/spi/spi-pxa2xx-dma.c   |  11 +-
 drivers/spi/spi-pxa2xx-pci.c   |  10 +-
 drivers/spi/spi-pxa2xx.c       |  99 ++++++++++------
 drivers/spi/spi-pxa2xx.h       |  39 ++++++-
 include/linux/spi/pxa2xx_spi.h |  48 --------
 8 files changed, 133 insertions(+), 312 deletions(-)
 delete mode 100644 Documentation/spi/pxa2xx.rst
 delete mode 100644 include/linux/spi/pxa2xx_spi.h

-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 1/9] spi: pxa2xx: Narrow the Kconfig option visibility

[Andy Shevchenko]

The PCI || ACPI dependency is the historical part of the x86 support.
Narrow the Kconfig option visibility by limiting this dependency to x86.

The drop of x86 for PCI case had happened in the commit 2b49ebda39d6
("spi/pxa2xx: allow building on a 64-bit kernel"), while the ACPI
was specifically added for Intel Lynx Point in the commit a3496855d9f1
("spi/pxa2xx: add support for Lynxpoint SPI controllers").

Note that X86 covers both 32- and 64-bit variants.

Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 26ba34a4100b..c1bb1895009c 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -817,7 +817,7 @@ config SPI_PPC4xx
 
 config SPI_PXA2XX
 	tristate "PXA2xx SSP SPI master"
-	depends on ARCH_PXA || ARCH_MMP || PCI || ACPI || COMPILE_TEST
+	depends on ARCH_PXA || ARCH_MMP || (X86 && (PCI || ACPI)) || COMPILE_TEST
 	select PXA_SSP if ARCH_PXA || ARCH_MMP
 	help
 	  This enables using a PXA2xx or Sodaville SSP port as a SPI master
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 2/9] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()

[Andy Shevchenko]

Drop rather useless use of ACPI_PTR() and of_match_ptr().
It also removes the necessity to be dependent of.h inclusion.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 1d43346b4436..75d208087748 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -19,7 +19,6 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
@@ -1725,7 +1724,6 @@ static const struct dev_pm_ops pxa2xx_spi_pm_ops = {
 	RUNTIME_PM_OPS(pxa2xx_spi_runtime_suspend, pxa2xx_spi_runtime_resume, NULL)
 };
 
-#ifdef CONFIG_ACPI
 static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
 	{ "80860F0E", LPSS_BYT_SSP },
 	{ "8086228E", LPSS_BSW_SSP },
@@ -1736,9 +1734,8 @@ static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);
-#endif
 
-static const struct of_device_id pxa2xx_spi_of_match[] __maybe_unused = {
+static const struct of_device_id pxa2xx_spi_of_match[] = {
 	{ .compatible = "marvell,mmp2-ssp", .data = (void *)MMP2_SSP },
 	{}
 };
@@ -1748,8 +1745,8 @@ static struct platform_driver driver = {
 	.driver = {
 		.name	= "pxa2xx-spi",
 		.pm	= pm_ptr(&pxa2xx_spi_pm_ops),
-		.acpi_match_table = ACPI_PTR(pxa2xx_spi_acpi_match),
-		.of_match_table = of_match_ptr(pxa2xx_spi_of_match),
+		.acpi_match_table = pxa2xx_spi_acpi_match,
+		.of_match_table = pxa2xx_spi_of_match,
 	},
 	.probe = pxa2xx_spi_probe,
 	.remove_new = pxa2xx_spi_remove,
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 3/9] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper

[Andy Shevchenko]

Refactor pxa2xx_spi_init_pdata() by extracting a new
pxa2xx_spi_init_ssp() helper which makes code less
twisted. It will be easier to continue refactoring for
a new coming modification.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c | 66 +++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 75d208087748..e7072727c25c 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1314,19 +1314,50 @@ static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
 	return param == chan->device->dev;
 }
 
+static int
+pxa2xx_spi_init_ssp(struct platform_device *pdev, struct ssp_device *ssp, enum pxa_ssp_type type)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int status;
+	u64 uid;
+
+	ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(ssp->mmio_base))
+		return PTR_ERR(ssp->mmio_base);
+
+	ssp->phys_base = res->start;
+
+	ssp->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(ssp->clk))
+		return PTR_ERR(ssp->clk);
+
+	ssp->irq = platform_get_irq(pdev, 0);
+	if (ssp->irq < 0)
+		return ssp->irq;
+
+	ssp->type = type;
+	ssp->dev = dev;
+
+	status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
+	if (status)
+		ssp->port_id = -1;
+	else
+		ssp->port_id = uid;
+
+	return 0;
+}
+
 static struct pxa2xx_spi_controller *
 pxa2xx_spi_init_pdata(struct platform_device *pdev)
 {
 	struct pxa2xx_spi_controller *pdata;
 	struct device *dev = &pdev->dev;
 	struct device *parent = dev->parent;
-	struct ssp_device *ssp;
-	struct resource *res;
 	enum pxa_ssp_type type = SSP_UNDEFINED;
 	const void *match;
 	bool is_lpss_priv;
 	int status;
-	u64 uid;
 
 	is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
 
@@ -1351,14 +1382,6 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
-	ssp = &pdata->ssp;
-
-	ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
-	if (IS_ERR(ssp->mmio_base))
-		return ERR_CAST(ssp->mmio_base);
-
-	ssp->phys_base = res->start;
-
 	/* Platforms with iDMA 64-bit */
 	if (is_lpss_priv) {
 		pdata->tx_param = parent;
@@ -1366,28 +1389,15 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 		pdata->dma_filter = pxa2xx_spi_idma_filter;
 	}
 
-	ssp->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(ssp->clk))
-		return ERR_CAST(ssp->clk);
-
-	ssp->irq = platform_get_irq(pdev, 0);
-	if (ssp->irq < 0)
-		return ERR_PTR(ssp->irq);
-
-	ssp->type = type;
-	ssp->dev = dev;
-
-	status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
-	if (status)
-		ssp->port_id = -1;
-	else
-		ssp->port_id = uid;
-
 	pdata->is_target = device_property_read_bool(dev, "spi-slave");
 	pdata->num_chipselect = 1;
 	pdata->enable_dma = true;
 	pdata->dma_burst_size = 1;
 
+	status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
+	if (status)
+		return ERR_PTR(status);
+
 	return pdata;
 }
 
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 4/9] spi: pxa2xx: Skip SSP initialization if it's done elsewhere

[Andy Shevchenko]

If SSP has been enumerated elsewhere, skip its initialization
in pxa2xx_spi_init_pdata().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index e7072727c25c..b01a18c89b6b 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1355,6 +1355,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device *parent = dev->parent;
 	enum pxa_ssp_type type = SSP_UNDEFINED;
+	struct ssp_device *ssp = NULL;
 	const void *match;
 	bool is_lpss_priv;
 	int status;
@@ -1372,6 +1373,10 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 			return ERR_PTR(status);
 
 		type = (enum pxa_ssp_type)value;
+	} else {
+		ssp = pxa_ssp_request(pdev->id, pdev->name);
+		if (ssp)
+			type = ssp->type;
 	}
 
 	/* Validate the SSP type correctness */
@@ -1394,6 +1399,10 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 	pdata->enable_dma = true;
 	pdata->dma_burst_size = 1;
 
+	/* If SSP has been already enumerated, use it */
+	if (ssp)
+		return pdata;
+
 	status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
 	if (status)
 		return ERR_PTR(status);
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 5/9] spi: pxa2xx: Allow number of chip select pins to be read from property

[Andy Shevchenko]

In some cases the number of the chip select pins might come from
the device property. Allow driver to use it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b01a18c89b6b..f4435c39d096 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1358,6 +1358,7 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 	struct ssp_device *ssp = NULL;
 	const void *match;
 	bool is_lpss_priv;
+	u32 num_cs = 1;
 	int status;
 
 	is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
@@ -1394,8 +1395,11 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
 		pdata->dma_filter = pxa2xx_spi_idma_filter;
 	}
 
+	/* Read number of chip select pins, if provided */
+	device_property_read_u32(dev, "num-cs", &num_cs);
+
+	pdata->num_chipselect = num_cs;
 	pdata->is_target = device_property_read_bool(dev, "spi-slave");
-	pdata->num_chipselect = 1;
 	pdata->enable_dma = true;
 	pdata->dma_burst_size = 1;
 
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 6/9] spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties

[Andy Shevchenko]

Since driver can parse num-cs device property, replace platform data
with this new approach. This pursues the following objectives:

- getting rid of the public header that barely used outside of
  the SPI subsystem (more specifically the SPI PXA2xx drivers)

- making a trampoline for the driver to support non-default number
  of the chip select pins in case the original code is going to be
  converted to Device Tree model

It's not expected to have more users in board files except this one.

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

diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 318402ad685e..3c5f5a3cb480 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -18,10 +18,10 @@
 #include <linux/i2c.h>
 #include <linux/platform_data/i2c-pxa.h>
 #include <linux/platform_data/pca953x.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/ads7846.h>
 #include <linux/spi/corgi_lcd.h>
-#include <linux/spi/pxa2xx_spi.h>
 #include <linux/mtd/sharpsl.h>
 #include <linux/mtd/physmap.h>
 #include <linux/input-event-codes.h>
@@ -569,10 +569,6 @@ static struct spi_board_info spitz_spi_devices[] = {
 	},
 };
 
-static struct pxa2xx_spi_controller spitz_spi_info = {
-	.num_chipselect	= 3,
-};
-
 static struct gpiod_lookup_table spitz_spi_gpio_table = {
 	.dev_id = "spi2",
 	.table = {
@@ -583,10 +579,20 @@ static struct gpiod_lookup_table spitz_spi_gpio_table = {
 	},
 };
 
+static const struct property_entry spitz_spi_properties[] = {
+	PROPERTY_ENTRY_U32("num-cs", 3),
+	{ }
+};
+
+static const struct software_node spitz_spi_node = {
+	.properties = spitz_spi_properties,
+};
+
 static void __init spitz_spi_init(void)
 {
 	struct platform_device *pd;
 	int id = 2;
+	int err;
 
 	if (machine_is_akita())
 		gpiod_add_lookup_table(&akita_lcdcon_gpio_table);
@@ -601,8 +607,13 @@ static void __init spitz_spi_init(void)
 	if (pd == NULL) {
 		pr_err("pxa2xx-spi: failed to allocate device id %d\n", id);
 	} else {
-		pd->dev.platform_data = &spitz_spi_info;
-		platform_device_add(pd);
+		err = device_add_software_node(&pd->dev, &spitz_spi_node);
+		if (err) {
+			platform_device_put(pd);
+			pr_err("pxa2xx-spi: failed to add software node\n");
+		} else {
+			platform_device_add(pd);
+		}
 	}
 
 	spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices));
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 7/9] spi: pxa2xx: Move contents of linux/spi/pxa2xx_spi.h to a local one

[Andy Shevchenko]

There is no user of the linux/spi/pxa2xx_spi.h. Move its contents
to the drivers/spi/spi-pxa2xx.h.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-dma.c   |  1 -
 drivers/spi/spi-pxa2xx-pci.c   |  4 +--
 drivers/spi/spi-pxa2xx.c       |  1 -
 drivers/spi/spi-pxa2xx.h       | 36 ++++++++++++++++++++++++-
 include/linux/spi/pxa2xx_spi.h | 48 ----------------------------------
 5 files changed, 37 insertions(+), 53 deletions(-)
 delete mode 100644 include/linux/spi/pxa2xx_spi.h

diff --git a/drivers/spi/spi-pxa2xx-dma.c b/drivers/spi/spi-pxa2xx-dma.c
index be563f0dd03a..26416ced6505 100644
--- a/drivers/spi/spi-pxa2xx-dma.c
+++ b/drivers/spi/spi-pxa2xx-dma.c
@@ -12,7 +12,6 @@
 #include <linux/scatterlist.h>
 #include <linux/sizes.h>
 
-#include <linux/spi/pxa2xx_spi.h>
 #include <linux/spi/spi.h>
 
 #include "spi-pxa2xx.h"
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 861b21c63504..e11a613bc340 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -10,11 +10,11 @@
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 
-#include <linux/spi/pxa2xx_spi.h>
-
 #include <linux/dmaengine.h>
 #include <linux/platform_data/dma-dw.h>
 
+#include "spi-pxa2xx.h"
+
 #define PCI_DEVICE_ID_INTEL_QUARK_X1000		0x0935
 #define PCI_DEVICE_ID_INTEL_BYT			0x0f0e
 #define PCI_DEVICE_ID_INTEL_MRFLD		0x1194
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index f4435c39d096..e22d9d29c7e9 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -24,7 +24,6 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
-#include <linux/spi/pxa2xx_spi.h>
 #include <linux/spi/spi.h>
 
 #include "spi-pxa2xx.h"
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 45cdbbc71c4b..08296729ea80 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -7,6 +7,7 @@
 #ifndef SPI_PXA2XX_H
 #define SPI_PXA2XX_H
 
+#include <linux/dmaengine.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/types.h>
@@ -15,7 +16,40 @@
 #include <linux/pxa2xx_ssp.h>
 
 struct gpio_desc;
-struct pxa2xx_spi_controller;
+
+/*
+ * The platform data for SSP controller devices
+ * (resides in device.platform_data).
+ */
+struct pxa2xx_spi_controller {
+	u8 num_chipselect;
+	u8 enable_dma;
+	u8 dma_burst_size;
+	bool is_target;
+
+	/* DMA engine specific config */
+	dma_filter_fn dma_filter;
+	void *tx_param;
+	void *rx_param;
+
+	/* For non-PXA arches */
+	struct ssp_device ssp;
+};
+
+/*
+ * The controller specific data for SPI target 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;
+	u8 tx_hi_threshold;
+	u8 rx_threshold;
+	u8 dma_burst_size;
+	u32 timeout;
+};
+
 struct spi_controller;
 struct spi_device;
 struct spi_transfer;
diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h
deleted file mode 100644
index e5a4a045fb67..000000000000
--- a/include/linux/spi/pxa2xx_spi.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2005 Stephen Street / StreetFire Sound Labs
- */
-#ifndef __LINUX_SPI_PXA2XX_SPI_H
-#define __LINUX_SPI_PXA2XX_SPI_H
-
-#include <linux/dmaengine.h>
-#include <linux/types.h>
-
-#include <linux/pxa2xx_ssp.h>
-
-struct dma_chan;
-
-/*
- * The platform data for SSP controller devices
- * (resides in device.platform_data).
- */
-struct pxa2xx_spi_controller {
-	u8 num_chipselect;
-	u8 enable_dma;
-	u8 dma_burst_size;
-	bool is_target;
-
-	/* DMA engine specific config */
-	dma_filter_fn dma_filter;
-	void *tx_param;
-	void *rx_param;
-
-	/* For non-PXA arches */
-	struct ssp_device ssp;
-};
-
-/*
- * The controller specific data for SPI target 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;
-	u8 tx_hi_threshold;
-	u8 rx_threshold;
-	u8 dma_burst_size;
-	u32 timeout;
-};
-
-#endif	/* __LINUX_SPI_PXA2XX_SPI_H */
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 8/9] spi: pxa2xx: Remove outdated documentation

[Andy Shevchenko]

The documentation is referring to the legacy enumeration of the SPI
host controllers and target devices. It has nothing to do with the
modern way, which is the only supported in kernel right now. Hence,
remove outdated documentation file.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/spi/pxa2xx.rst | 208 -----------------------------------
 drivers/spi/Kconfig          |   3 +-
 2 files changed, 1 insertion(+), 210 deletions(-)
 delete mode 100644 Documentation/spi/pxa2xx.rst

diff --git a/Documentation/spi/pxa2xx.rst b/Documentation/spi/pxa2xx.rst
deleted file mode 100644
index 33d2ad95901e..000000000000
--- a/Documentation/spi/pxa2xx.rst
+++ /dev/null
@@ -1,208 +0,0 @@
-==============================
-PXA2xx SPI on SSP driver HOWTO
-==============================
-
-This a mini HOWTO on the pxa2xx_spi driver. The driver turns a PXA2xx
-synchronous serial port into an SPI host controller
-(see Documentation/spi/spi-summary.rst). The driver has the following features
-
-- Support for any PXA2xx and compatible SSP.
-- SSP PIO and SSP DMA data transfers.
-- External and Internal (SSPFRM) chip selects.
-- Per peripheral device (chip) configuration.
-- Full suspend, freeze, resume support.
-
-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 host controllers
----------------------------------
-Typically, for a legacy platform, an SPI host controller is defined in the
-arch/.../mach-*/board-*.c as a "platform device". The host controller configuration
-is passed to the driver via a table found in include/linux/spi/pxa2xx_spi.h::
-
-  struct pxa2xx_spi_controller {
-	u8 num_chipselect;
-	u8 enable_dma;
-	...
-  };
-
-The "pxa2xx_spi_controller.num_chipselect" field is used to determine the number of
-peripheral devices (chips) attached to this SPI host controller.
-
-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 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 HOST SAMPLE
-----------------
-Below is a sample configuration using the PXA255 NSSP for a legacy platform::
-
-  static struct resource pxa_spi_nssp_resources[] = {
-	[0] = {
-		.start	= __PREG(SSCR0_P(2)), /* Start address of NSSP */
-		.end	= __PREG(SSCR0_P(2)) + 0x2c, /* Range of registers */
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= IRQ_NSSP, /* NSSP IRQ */
-		.end	= IRQ_NSSP,
-		.flags	= IORESOURCE_IRQ,
-	},
-  };
-
-  static struct pxa2xx_spi_controller pxa_nssp_controller_info = {
-	.num_chipselect = 1, /* Matches the number of chips attached to NSSP */
-	.enable_dma = 1, /* Enables NSSP DMA */
-  };
-
-  static struct platform_device pxa_spi_nssp = {
-	.name = "pxa2xx-spi", /* MUST BE THIS VALUE, so device match driver */
-	.id = 2, /* Bus number, MUST MATCH SSP number 1..n */
-	.resource = pxa_spi_nssp_resources,
-	.num_resources = ARRAY_SIZE(pxa_spi_nssp_resources),
-	.dev = {
-		.platform_data = &pxa_nssp_controller_info, /* Passed to driver */
-	},
-  };
-
-  static struct platform_device *devices[] __initdata = {
-	&pxa_spi_nssp,
-  };
-
-  static void __init board_init(void)
-  {
-	(void)platform_add_device(devices, ARRAY_SIZE(devices));
-  }
-
-Declaring peripheral devices
-----------------------------
-Typically, for a legacy platform, each SPI peripheral device (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 peripheral device (chip) attached to the PXA2xx must provide specific chip configuration
-information via the structure "pxa2xx_spi_chip" found in
-"include/linux/spi/pxa2xx_spi.h". The PXA2xx host controller driver will use
-the configuration whenever the driver communicates with the peripheral
-device. All fields are optional.
-
-::
-
-  struct pxa2xx_spi_chip {
-	u8 tx_threshold;
-	u8 rx_threshold;
-	u8 dma_burst_size;
-	u32 timeout;
-  };
-
-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
-performance of pxa2xx_spi driver and misconfiguration will result in rx
-FIFO overruns (especially in PIO mode transfers). Good default values are::
-
-	.tx_threshold = 8,
-	.rx_threshold = 8,
-
-The range is 1 to 16 where zero indicates "use default".
-
-The "pxa2xx_spi_chip.dma_burst_size" field is used to configure PXA2xx DMA
-engine and is related the "spi_device.bits_per_word" field.  Read and understand
-the PXA2xx "Developer Manual" sections on the DMA controller and SSP Controllers
-to determine the correct value. An SSP configured for byte-wide transfers would
-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
-dependent on the SPI bus speed ("spi_board_info.max_speed_hz") and the specific
-peripheral device. Please note that the PXA2xx SSP 1 does not support trailing byte
-timeouts and must busy-wait any trailing bytes.
-
-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)
-to accommodate these chips.
-
-
-NSSP PERIPHERAL SAMPLE
-----------------------
-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.
-
-::
-
-  static struct pxa2xx_spi_chip cs8415a_chip_info = {
-	.tx_threshold = 8, /* SSP hardware FIFO threshold */
-	.rx_threshold = 8, /* SSP hardware FIFO threshold */
-	.dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
-	.timeout = 235, /* See Intel documentation */
-  };
-
-  static struct pxa2xx_spi_chip cs8405a_chip_info = {
-	.tx_threshold = 8, /* SSP hardware FIFO threshold */
-	.rx_threshold = 8, /* SSP hardware FIFO threshold */
-	.dma_burst_size = 8, /* Byte wide transfers used so 8 byte bursts */
-	.timeout = 235, /* See Intel documentation */
-  };
-
-  static struct spi_board_info streetracer_spi_board_info[] __initdata = {
-	{
-		.modalias = "cs8415a", /* Name of spi_driver for this device */
-		.max_speed_hz = 3686400, /* Run SSP as fast a possible */
-		.bus_num = 2, /* Framework bus number */
-		.chip_select = 0, /* Framework chip select */
-		.platform_data = NULL; /* No spi_driver specific config */
-		.controller_data = &cs8415a_chip_info, /* Host controller config */
-		.irq = STREETRACER_APCI_IRQ, /* Peripheral device interrupt */
-	},
-	{
-		.modalias = "cs8405a", /* Name of spi_driver for this device */
-		.max_speed_hz = 3686400, /* Run SSP as fast a possible */
-		.bus_num = 2, /* Framework bus number */
-		.chip_select = 1, /* Framework chip select */
-		.controller_data = &cs8405a_chip_info, /* Host controller config */
-		.irq = STREETRACER_APCI_IRQ, /* Peripheral device interrupt */
-	},
-  };
-
-  static void __init streetracer_init(void)
-  {
-	spi_register_board_info(streetracer_spi_board_info,
-				ARRAY_SIZE(streetracer_spi_board_info));
-  }
-
-
-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.
-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
-a per "spi_transfer" basis::
-
-  if spi_message.len > 65536 then
-	print "rate limited" warning
-	use PIO transfers
-
-  if enable_dma and the size is in the range [DMA burst size..65536] then
-	use streaming DMA mode
-
-  otherwise
-	use PIO transfer
-
-THANKS TO
----------
-David Brownell and others for mentoring the development of this driver.
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index c1bb1895009c..11d7ac8aed4a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -821,8 +821,7 @@ config SPI_PXA2XX
 	select PXA_SSP if ARCH_PXA || ARCH_MMP
 	help
 	  This enables using a PXA2xx or Sodaville SSP port as a SPI master
-	  controller. The driver can be configured to use any SSP port and
-	  additional documentation can be found a Documentation/spi/pxa2xx.rst.
+	  controller. The driver can be configured to use any SSP port.
 
 config SPI_PXA2XX_PCI
 	def_tristate SPI_PXA2XX && PCI && COMMON_CLK
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 9/9] spi: pxa2xx: Don't use "proxy" headers

[Andy Shevchenko]

Update header inclusions to follow IWYU (Include What You Use)
principle.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-pxa2xx-dma.c | 10 ++++++++--
 drivers/spi/spi-pxa2xx-pci.c |  6 ++++++
 drivers/spi/spi-pxa2xx.c     | 10 +++++++---
 drivers/spi/spi-pxa2xx.h     |  3 +--
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-dma.c b/drivers/spi/spi-pxa2xx-dma.c
index 26416ced6505..751cb0f77b62 100644
--- a/drivers/spi/spi-pxa2xx-dma.c
+++ b/drivers/spi/spi-pxa2xx-dma.c
@@ -6,16 +6,22 @@
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
  */
 
-#include <linux/device.h>
+#include <linux/atomic.h>
+#include <linux/dev_printk.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
+#include <linux/errno.h>
+#include <linux/irqreturn.h>
 #include <linux/scatterlist.h>
-#include <linux/sizes.h>
+#include <linux/string.h>
+#include <linux/types.h>
 
 #include <linux/spi/spi.h>
 
 #include "spi-pxa2xx.h"
 
+struct device;
+
 static void pxa2xx_spi_dma_transfer_complete(struct driver_data *drv_data,
 					     bool error)
 {
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index e11a613bc340..6d2efdb0e95f 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -6,9 +6,15 @@
  * Copyright (C) 2016, 2021 Intel Corporation
  */
 #include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/sprintf.h>
+#include <linux/string.h>
+#include <linux/types.h>
 
 #include <linux/dmaengine.h>
 #include <linux/platform_data/dma-dw.h>
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index e22d9d29c7e9..86d0f1064a45 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -5,24 +5,28 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/atomic.h>
 #include <linux/bitops.h>
+#include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dmaengine.h>
 #include <linux/err.h>
-#include <linux/errno.h>
 #include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/ioport.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/math64.h>
+#include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
+#include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/types.h>
 
 #include <linux/spi/spi.h>
 
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 08296729ea80..f1097c96c50f 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -8,8 +8,7 @@
 #define SPI_PXA2XX_H
 
 #include <linux/dmaengine.h>
-#include <linux/interrupt.h>
-#include <linux/io.h>
+#include <linux/irqreturn.h>
 #include <linux/types.h>
 #include <linux/sizes.h>
 
-- 
2.43.0.rc1.1.gbec44491f096

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Mark Brown]

On Wed, 27 Mar 2024 21:29:19 +0200, Andy Shevchenko wrote:
> As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of
> its content is being used solely internally to SPI subsystem
> (PXA2xx drivers). Hence this refactoring series with the additional
> win of getting rid of legacy documentation.
> 
> Changelog v2:
> - dropped applied patches
> - added patch to amend dependencies (Mark)
> - amended the second patch accordingly (Mark)
> - elaborated purpose of the patch 6 in the commit message (Mark)
> 
> [...]

Applied to

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

Thanks!

[1/9] spi: pxa2xx: Narrow the Kconfig option visibility
      commit: 3af201a405b3e5abee65102b062c309fff68cc0e
[2/9] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
      commit: 9907c475dcab9b269422972577360122129ac84c
[3/9] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
      commit: 7290f1e4075d28ab961df5a454503296fa289271
[4/9] spi: pxa2xx: Skip SSP initialization if it's done elsewhere
      commit: bb77c99ee6d3d704086acf141d3ec92601747809

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

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

On Fri, Mar 29, 2024 at 01:29:10AM +0000, Mark Brown wrote:
> On Wed, 27 Mar 2024 21:29:19 +0200, Andy Shevchenko wrote:
> > As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of
> > its content is being used solely internally to SPI subsystem
> > (PXA2xx drivers). Hence this refactoring series with the additional
> > win of getting rid of legacy documentation.
> > 
> > Changelog v2:
> > - dropped applied patches
> > - added patch to amend dependencies (Mark)
> > - amended the second patch accordingly (Mark)
> > - elaborated purpose of the patch 6 in the commit message (Mark)
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thanks!
> 
> [1/9] spi: pxa2xx: Narrow the Kconfig option visibility
>       commit: 3af201a405b3e5abee65102b062c309fff68cc0e
> [2/9] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
>       commit: 9907c475dcab9b269422972577360122129ac84c
> [3/9] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
>       commit: 7290f1e4075d28ab961df5a454503296fa289271
> [4/9] spi: pxa2xx: Skip SSP initialization if it's done elsewhere
>       commit: bb77c99ee6d3d704086acf141d3ec92601747809

Thank you!

Do I need to do anything else to get the rest applied?

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote:

> Do I need to do anything else to get the rest applied?

All the concerns I have with swnodes just being a more complex and less
maintainable way of doing things still stand, I'm not clear that this is
making anything better.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote:
> 
> > Do I need to do anything else to get the rest applied?
> 
> All the concerns I have with swnodes just being a more complex and less
> maintainable way of doing things still stand, I'm not clear that this is
> making anything better.

As I explained before it's not less maintainable than device tree sources.
The only difference is that we don't have validation tool for in-kernel
tables. And I don't see why we need that. The data describes the platforms
and in the very same way may come to the driver from elsewhere.
How would you validate that? It the same as we trust firmware (boot loader)
or not. If we don't than how should we do at all?

Can you point out what the exact aspect is most significant from C language
perspective that we miss after conversion? Type checking? Something else?

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:
> > On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote:
> > 
> > > Do I need to do anything else to get the rest applied?
> > 
> > All the concerns I have with swnodes just being a more complex and less
> > maintainable way of doing things still stand, I'm not clear that this is
> > making anything better.
> 
> As I explained before it's not less maintainable than device tree sources.
> The only difference is that we don't have validation tool for in-kernel
> tables. And I don't see why we need that. The data describes the platforms
> and in the very same way may come to the driver from elsewhere.
> How would you validate that? It the same as we trust firmware (boot loader)
> or not. If we don't than how should we do at all?
> 
> Can you point out what the exact aspect is most significant from C language
> perspective that we miss after conversion? Type checking? Something else?

Also note, after hiding the data structures from that file we open
the door for the much bigger cleanup, and I have patches already precooked
(need a bit of time to test, though).

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Wed, Apr 03, 2024 at 04:41:40PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:

> > > All the concerns I have with swnodes just being a more complex and less
> > > maintainable way of doing things still stand, I'm not clear that this is
> > > making anything better.

> > As I explained before it's not less maintainable than device tree sources.
> > The only difference is that we don't have validation tool for in-kernel
> > tables. And I don't see why we need that. The data describes the platforms
> > and in the very same way may come to the driver from elsewhere.
> > How would you validate that? It the same as we trust firmware (boot loader)
> > or not. If we don't than how should we do at all?

I don't think we should do this at all, all I see from these changes is
effort in reviewing them - as far as I can tell they are a solution in
search of a problem.  DT has some support for validation of the
formatting of the data supplied by the board and offers the potential
for distributing the board description separately to the kernel.

> > Can you point out what the exact aspect is most significant from C language
> > perspective that we miss after conversion? Type checking? Something else?

You're changing the code from supplying data from the board description
via a simple C structure where the compiler offers at least some
validation and where we can just supply directly usable values to an
abstract data structure that's still encoded in C but has less
validation and requires a bunch of code to parse at runtime.  Platform
data is unsurprisingly a good solution to the problem of supplying
platform data.

> Also note, after hiding the data structures from that file we open
> the door for the much bigger cleanup, and I have patches already precooked
> (need a bit of time to test, though).

Perhaps those will motivate a change, as things stand I've not seen what
you're proposing to do here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

On Wed, Apr 03, 2024 at 03:13:48PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 04:41:40PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:
> 
> > > > All the concerns I have with swnodes just being a more complex and less
> > > > maintainable way of doing things still stand, I'm not clear that this is
> > > > making anything better.
> 
> > > As I explained before it's not less maintainable than device tree sources.
> > > The only difference is that we don't have validation tool for in-kernel
> > > tables. And I don't see why we need that. The data describes the platforms
> > > and in the very same way may come to the driver from elsewhere.
> > > How would you validate that? It the same as we trust firmware (boot loader)
> > > or not. If we don't than how should we do at all?
> 
> I don't think we should do this at all, all I see from these changes is
> effort in reviewing them - as far as I can tell they are a solution in
> search of a problem.  DT has some support for validation of the
> formatting of the data supplied by the board and offers the potential
> for distributing the board description separately to the kernel.
> 
> > > Can you point out what the exact aspect is most significant from C language
> > > perspective that we miss after conversion? Type checking? Something else?
> 
> You're changing the code from supplying data from the board description
> via a simple C structure where the compiler offers at least some
> validation and where we can just supply directly usable values to an
> abstract data structure that's still encoded in C but has less
> validation and requires a bunch of code to parse at runtime.  Platform
> data is unsurprisingly a good solution to the problem of supplying
> platform data.

Linus was long time ago against board files. Yet, we have a few old
(kinda supported) boards left in the tree. The conversion makes the
driver be prepared for the DT conversion when it happens. From maintenance
perspective my patch reduced the code under the maintenance, which reduces
time spent by both contributors and maintainers on this.

AFAIU all what you are moaning about is type checking. Okay, I got
it, but we have a lot of other places with similar approach done,
e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
driver without any platform data being involved and you seems to be fine with
that:

$ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'

I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").

Can you tell how it is different to my proposal?

> > Also note, after hiding the data structures from that file we open
> > the door for the much bigger cleanup, and I have patches already precooked
> > (need a bit of time to test, though).
> 
> Perhaps those will motivate a change, as things stand I've not seen what
> you're proposing to do here.

Okay, let me incorporate those into v3.

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]

On Wed, Apr 03, 2024 at 05:41:30PM +0300, Andy Shevchenko wrote:

> Linus was long time ago against board files. Yet, we have a few old
> (kinda supported) boards left in the tree. The conversion makes the
> driver be prepared for the DT conversion when it happens. From maintenance
> perspective my patch reduced the code under the maintenance, which reduces
> time spent by both contributors and maintainers on this.

> AFAIU all what you are moaning about is type checking. Okay, I got

The type checking is part of it, but it's more a general taste thing
with using swnodes like this.  You've not actually removed the board
file and it's hard to get enthusiastic about the change to the board
file that results, or to see this as a substantial step towards DT
conversion for the platform given the trivialness of the single
property here.  As a general thing I don't want to encourage people to
start randomly converting things to swnode rather than to DT.

> it, but we have a lot of other places with similar approach done,
> e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
> driver without any platform data being involved and you seems to be fine with
> that:

> $ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'

> I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").

> Can you tell how it is different to my proposal?

The main difference with the GPIO lookup tables is that they are
structured data specifically for GPIOs rather than the general purpose
free for all we have with swnode.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH v2 0/9] spi: pxa2xx: Drop linux/spi/pxa2xx_spi.h

[Andy Shevchenko]

On Wed, Apr 03, 2024 at 04:50:00PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 05:41:30PM +0300, Andy Shevchenko wrote:
> 
> > Linus was long time ago against board files. Yet, we have a few old
> > (kinda supported) boards left in the tree. The conversion makes the
> > driver be prepared for the DT conversion when it happens. From maintenance
> > perspective my patch reduced the code under the maintenance, which reduces
> > time spent by both contributors and maintainers on this.
> 
> > AFAIU all what you are moaning about is type checking. Okay, I got
> 
> The type checking is part of it, but it's more a general taste thing
> with using swnodes like this.  You've not actually removed the board
> file and it's hard to get enthusiastic about the change to the board
> file that results, or to see this as a substantial step towards DT
> conversion for the platform given the trivialness of the single
> property here.  As a general thing I don't want to encourage people to
> start randomly converting things to swnode rather than to DT.

Conversion to GPIO lookup tables is also the same in this sense.
But with it in place, the drivers aren't needed to be touched
when the real conversion happens. I agree, that _ideally_ we should
take that shot, but I am not an expert in DT and it will take a lot
for me to get to the shape, besides the fact of the ARM (platform)
specifics, which I'm far from. So, I prefer do step-by-step approach
if one developer can't fulfill the task. I.o.w. perfect is enemy of
good.

> > it, but we have a lot of other places with similar approach done,
> > e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
> > driver without any platform data being involved and you seems to be fine with
> > that:
> 
> > $ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'
> 
> > I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").
> 
> > Can you tell how it is different to my proposal?
> 
> The main difference with the GPIO lookup tables is that they are
> structured data specifically for GPIOs rather than the general purpose
> free for all we have with swnode.

Semantically yes, technically they have all the same issues you pointed out
that swnode has.

Nevertheless, I'm about to send a part 2 of cleanup (I decided not mangle
this series, so it will be on top of this) for you to see how we can go
forward.

And thank you for this discussion.

-- 
With Best Regards,
Andy Shevchenko