Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650168 > unrolled thread

[RFC PATCH 0/5] spi: moving to struct gpio_desc

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/5] spi: moving to struct gpio_desc Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-05-25 06:40 +0200
    [RFC PATCH 3/5] ARM: imx: add gpiod_lookup_table for spi chip-selects Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-05-25 06:40 +0200
    [RFC PATCH 4/5] spi: core: convert spi_master to use gpio_desc Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-05-25 06:40 +0200
    [RFC PATCH 5/5] ARM: ep93xx: remove chipselect from ep93xx_spi_info Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-05-25 06:40 +0200
    Re: [RFC PATCH 0/5] spi: moving to struct gpio_desc Mark Brown <broonie@kernel.org> - 2017-05-29 15:40 +0200

#1650168 — [RFC PATCH 0/5] spi: moving to struct gpio_desc

This is my attempt to move spi over to using struct gpio_desc. I've
stopped at converting struct spi_master to gather some feedback.

ep93xx wasn't as hard as I'd expected so I'm pretty happy with those
changes. imx on the other hand has an annoying habit of conflating the GPIO
and native chip-select so I'm pretty sure that's broken.

I've compile tested ep93xx_defconfig and multi_v5_defconfig.

Looking ahead to converting struct spi_device there are a number of drivers
that re-use cs_gpio to reference the native chip-select so that still poses
issues I don't have a plan to solve. Happily most of the changes are deleting
code that requests the GPIO since that's now taken care of in core.

Chris Packham (5):
  spi: use gpio_desc instead of numeric gpio
  ARM: ep93xx: add gpiod_lookup_table for spi chip-selects
  ARM: imx: add gpiod_lookup_table for spi chip-selects
  spi: core: convert spi_master to use gpio_desc
  ARM: ep93xx: remove chipselect from ep93xx_spi_info

 arch/arm/mach-ep93xx/edb93xx.c           | 15 +++++++++++----
 arch/arm/mach-ep93xx/simone.c            | 14 ++++++++++----
 arch/arm/mach-ep93xx/vision_ep9307.c     | 20 ++++++++++++++------
 arch/arm/mach-imx/mach-mx27_3ds.c        | 21 +++++++++++++++++++++
 arch/arm/mach-imx/mach-pca100.c          | 13 +++++++++++++
 drivers/spi/spi-ep93xx.c                 | 18 ++++++++----------
 drivers/spi/spi-imx.c                    | 25 +++++++++----------------
 drivers/spi/spi-mt65xx.c                 | 13 -------------
 drivers/spi/spi.c                        | 29 +++++++++++++++++++++--------
 include/linux/platform_data/spi-ep93xx.h |  4 +---
 include/linux/spi/spi.h                  |  2 +-
 11 files changed, 109 insertions(+), 65 deletions(-)

-- 
2.13.0

[toc] | [next] | [standalone]

#1650170 — [RFC PATCH 3/5] ARM: imx: add gpiod_lookup_table for spi chip-selects

This is a preparatory step which will allow the conversion of
spi-imx.c to gpiod.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/mach-imx/mach-mx27_3ds.c | 21 +++++++++++++++++++++
 arch/arm/mach-imx/mach-pca100.c   | 13 +++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/arch/arm/mach-imx/mach-mx27_3ds.c b/arch/arm/mach-imx/mach-mx27_3ds.c
index 45e16bd7e2f2..46d67be124f0 100644
--- a/arch/arm/mach-imx/mach-mx27_3ds.c
+++ b/arch/arm/mach-imx/mach-mx27_3ds.c
@@ -22,6 +22,7 @@
 
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/irq.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/ulpi.h>
@@ -313,6 +314,15 @@ static struct imx_ssi_platform_data mx27_3ds_ssi_pdata = {
 };
 
 /* SPI */
+static struct gpiod_lookup_table spi1_cs_gpio_table = {
+	.dev_id = "spi.0",
+	.table = {
+		GPIO_LOOKUP("gpio-mxc", SPI1_SS0,
+			    "spi-cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static int spi1_chipselect[] = {SPI1_SS0};
 
 static const struct spi_imx_master spi1_pdata __initconst = {
@@ -320,6 +330,15 @@ static const struct spi_imx_master spi1_pdata __initconst = {
 	.num_chipselect	= ARRAY_SIZE(spi1_chipselect),
 };
 
+static struct gpiod_lookup_table spi2_cs_gpio_table = {
+	.dev_id = "spi.1",
+	.table = {
+		GPIO_LOOKUP("gpio-mxc", SPI2_SS0,
+			    "spi-cs", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static int spi2_chipselect[] = {SPI2_SS0};
 
 static const struct spi_imx_master spi2_pdata __initconst = {
@@ -398,7 +417,9 @@ static void __init mx27pdk_init(void)
 	imx27_add_imx_keypad(&mx27_3ds_keymap_data);
 	imx27_add_imx2_wdt();
 
+	gpiod_add_lookup_table(&spi2_cs_gpio_table);
 	imx27_add_spi_imx1(&spi2_pdata);
+	gpiod_add_lookup_table(&spi1_cs_gpio_table);
 	imx27_add_spi_imx0(&spi1_pdata);
 
 	imx27_add_imx_i2c(0, &mx27_3ds_i2c0_data);
diff --git a/arch/arm/mach-imx/mach-pca100.c b/arch/arm/mach-imx/mach-pca100.c
index ed675863655b..8a2b5860f327 100644
--- a/arch/arm/mach-imx/mach-pca100.c
+++ b/arch/arm/mach-imx/mach-pca100.c
@@ -27,6 +27,7 @@
 #include <linux/irq.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/ulpi.h>
 
@@ -202,6 +203,17 @@ static struct spi_board_info pca100_spi_board_info[] __initdata = {
 	},
 };
 
+static struct gpiod_lookup_table pca100_spi_cs_gpio_table = {
+	.dev_id = "spi.0",
+	.table = {
+		GPIO_LOOKUP_IDX("gpio-mxc", SPI1_SS0,
+				"spi-cs", 0, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-mxc", SPI1_SS1,
+				"spi-cs", 1, GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static int pca100_spi_cs[] = {SPI1_SS0, SPI1_SS1};
 
 static const struct spi_imx_master pca100_spi0_data __initconst = {
@@ -376,6 +388,7 @@ static void __init pca100_init(void)
 	mxc_gpio_mode(GPIO_PORTD | 27 | GPIO_GPIO | GPIO_IN);
 	spi_register_board_info(pca100_spi_board_info,
 				ARRAY_SIZE(pca100_spi_board_info));
+	gpiod_add_lookup_table(&pca100_spi_cs_gpio_table);
 	imx27_add_spi_imx0(&pca100_spi0_data);
 
 	imx27_add_imx_fb(&pca100_fb_data);
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1650171 — [RFC PATCH 4/5] spi: core: convert spi_master to use gpio_desc

Instead of numeric gpios make struct spi_master hold an array of struct
gpio_desc. For now struct spi_device still maintains a numeric gpio
which will be updated in a subsequent change.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/spi/spi-ep93xx.c | 18 ++++++++----------
 drivers/spi/spi-imx.c    | 25 +++++++++----------------
 drivers/spi/spi-mt65xx.c | 13 -------------
 drivers/spi/spi.c        | 24 +++++++++++++++++-------
 include/linux/spi/spi.h  |  2 +-
 5 files changed, 35 insertions(+), 47 deletions(-)

diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index b5d766064b7b..8ff795e65b38 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -816,7 +816,7 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 
 	master->num_chipselect = info->num_chipselect;
 	master->cs_gpios = devm_kzalloc(&master->dev,
-					sizeof(int) * master->num_chipselect,
+					sizeof(*master->cs_gpios) * master->num_chipselect,
 					GFP_KERNEL);
 	if (!master->cs_gpios) {
 		error = -ENOMEM;
@@ -824,19 +824,17 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0; i < master->num_chipselect; i++) {
-		master->cs_gpios[i] = info->chipselect[i];
+		struct gpio_desc *cs;
 
-		if (!gpio_is_valid(master->cs_gpios[i]))
-			continue;
-
-		error = devm_gpio_request_one(&pdev->dev, master->cs_gpios[i],
-					      GPIOF_OUT_INIT_HIGH,
-					      "ep93xx-spi");
-		if (error) {
+		cs = devm_gpiod_get_index(&pdev->dev, "spi-cs", i,
+					  GPIOD_OUT_HIGH);
+		if (IS_ERR(cs)) {
 			dev_err(&pdev->dev, "could not request cs gpio %d\n",
-				master->cs_gpios[i]);
+				i);
+			error = PTR_ERR(cs);
 			goto fail_release_master;
 		}
+		master->cs_gpios[i] = cs;
 	}
 
 	platform_set_drvdata(pdev, master);
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 19b30cf7d2b7..efbf03ac7cf1 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1360,12 +1360,18 @@ static int spi_imx_probe(struct platform_device *pdev)
 	if (mxc_platform_info) {
 		master->num_chipselect = mxc_platform_info->num_chipselect;
 		master->cs_gpios = devm_kzalloc(&master->dev,
-			sizeof(int) * master->num_chipselect, GFP_KERNEL);
+			sizeof(*master->cs_gpios) * master->num_chipselect, GFP_KERNEL);
 		if (!master->cs_gpios)
 			return -ENOMEM;
 
-		for (i = 0; i < master->num_chipselect; i++)
-			master->cs_gpios[i] = mxc_platform_info->chipselect[i];
+		for (i = 0; i < master->num_chipselect; i++) {
+			struct gpio_desc *cs;
+
+			cs = devm_gpiod_get_index(&pdev->dev, "spi-cs", i,
+						  GPIOD_OUT_HIGH);
+			if (!IS_ERR(cs))
+				master->cs_gpios[i] = cs;
+		}
  	}
 
 	spi_imx->bitbang.chipselect = spi_imx_chipselect;
@@ -1456,19 +1462,6 @@ static int spi_imx_probe(struct platform_device *pdev)
 		goto out_clk_put;
 	}
 
-	for (i = 0; i < master->num_chipselect; i++) {
-		if (!gpio_is_valid(master->cs_gpios[i]))
-			continue;
-
-		ret = devm_gpio_request(&pdev->dev, master->cs_gpios[i],
-					DRIVER_NAME);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't get CS GPIO %i\n",
-				master->cs_gpios[i]);
-			goto out_clk_put;
-		}
-	}
-
 	dev_info(&pdev->dev, "probed\n");
 
 	clk_disable(spi_imx->clk_ipg);
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 278867a31950..36e0d865fd02 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -681,19 +681,6 @@ static int mtk_spi_probe(struct platform_device *pdev)
 			ret = -EINVAL;
 			goto err_disable_runtime_pm;
 		}
-
-		if (master->cs_gpios) {
-			for (i = 0; i < master->num_chipselect; i++) {
-				ret = devm_gpio_request(&pdev->dev,
-							master->cs_gpios[i],
-							dev_name(&pdev->dev));
-				if (ret) {
-					dev_err(&pdev->dev,
-						"can't get CS GPIO %i\n", i);
-					goto err_disable_runtime_pm;
-				}
-			}
-		}
 	}
 
 	return 0;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b39c0f9956dd..8be01520b02e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -40,6 +40,7 @@
 #include <linux/ioport.h>
 #include <linux/acpi.h>
 #include <linux/highmem.h>
+#include <linux/gpio/consumer.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/spi.h>
@@ -531,8 +532,10 @@ int spi_add_device(struct spi_device *spi)
 		goto done;
 	}
 
-	if (master->cs_gpios)
-		spi->cs_gpio = master->cs_gpios[spi->chip_select];
+	if (master->cs_gpios[spi->chip_select])
+		spi->cs_gpio = desc_to_gpio(master->cs_gpios[spi->chip_select]);
+	else
+		spi->cs_gpio = -ENOENT;
 
 	/* Drivers may modify this initial i/o setup, but will
 	 * normally rely on the device being setup.  Devices
@@ -1878,7 +1881,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 #ifdef CONFIG_OF
 static int of_spi_register_master(struct spi_master *master)
 {
-	int nb, i, *cs;
+	int nb, i;
+	struct gpio_desc **cs;
 	struct device_node *np = master->dev.of_node;
 
 	if (!np)
@@ -1894,7 +1898,7 @@ static int of_spi_register_master(struct spi_master *master)
 		return nb;
 
 	cs = devm_kzalloc(&master->dev,
-			  sizeof(int) * master->num_chipselect,
+			  sizeof(*master->cs_gpios) * master->num_chipselect,
 			  GFP_KERNEL);
 	master->cs_gpios = cs;
 
@@ -1902,10 +1906,16 @@ static int of_spi_register_master(struct spi_master *master)
 		return -ENOMEM;
 
 	for (i = 0; i < master->num_chipselect; i++)
-		cs[i] = -ENOENT;
+		cs[i] = NULL;
+
+	for (i = 0; i < nb; i++) {
+		struct gpio_desc *gpio;
 
-	for (i = 0; i < nb; i++)
-		cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+		gpio = devm_gpiod_get_index(&master->dev, "cs", i,
+					    GPIOD_OUT_HIGH);
+		if (!IS_ERR(gpio))
+			cs[i] = gpio;
+	}
 
 	return 0;
 }
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 935bd2854ff1..46960ca62d5b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -556,7 +556,7 @@ struct spi_master {
 			   struct spi_message *message);
 
 	/* gpio chip select */
-	int			*cs_gpios;
+	struct gpio_desc	**cs_gpios;
 
 	/* statistics */
 	struct spi_statistics	statistics;
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1650172 — [RFC PATCH 5/5] ARM: ep93xx: remove chipselect from ep93xx_spi_info

Now that the driver has been updated to use gpiod there is no need to
have platform data to define the SPI chipselects. We still need to
define the number of chipselects used.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/mach-ep93xx/edb93xx.c           | 7 +------
 arch/arm/mach-ep93xx/simone.c            | 7 +------
 arch/arm/mach-ep93xx/vision_ep9307.c     | 9 +--------
 include/linux/platform_data/spi-ep93xx.h | 4 +---
 4 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c
index 9042adfe03de..b8d354fc27f5 100644
--- a/arch/arm/mach-ep93xx/edb93xx.c
+++ b/arch/arm/mach-ep93xx/edb93xx.c
@@ -127,13 +127,8 @@ static struct gpiod_lookup_table edb93xx_gpios_table = {
 	},
 };
 
-static int edb93xx_spi_chipselects[] __initdata = {
-	EP93XX_GPIO_LINE_EGPIO6,
-};
-
 static struct ep93xx_spi_info edb93xx_spi_info __initdata = {
-	.chipselect	= edb93xx_spi_chipselects,
-	.num_chipselect	= ARRAY_SIZE(edb93xx_spi_chipselects),
+	.num_chipselect	= 1,
 };
 
 static void __init edb93xx_register_spi(void)
diff --git a/arch/arm/mach-ep93xx/simone.c b/arch/arm/mach-ep93xx/simone.c
index f297a7a89ed9..cb6bc2e1670a 100644
--- a/arch/arm/mach-ep93xx/simone.c
+++ b/arch/arm/mach-ep93xx/simone.c
@@ -129,13 +129,8 @@ static struct gpiod_lookup_table simone_gpios_table = {
 	},
 };
 
-static int simone_spi_chipselects[] __initdata = {
-	EP93XX_GPIO_LINE_EGPIO1,
-};
-
 static struct ep93xx_spi_info simone_spi_info __initdata = {
-	.chipselect	= simone_spi_chipselects,
-	.num_chipselect	= ARRAY_SIZE(simone_spi_chipselects),
+	.num_chipselect	= 1,
 	.use_dma = 1,
 };
 
diff --git a/arch/arm/mach-ep93xx/vision_ep9307.c b/arch/arm/mach-ep93xx/vision_ep9307.c
index 3d9c66bbf2af..494c55080472 100644
--- a/arch/arm/mach-ep93xx/vision_ep9307.c
+++ b/arch/arm/mach-ep93xx/vision_ep9307.c
@@ -256,15 +256,8 @@ static struct gpiod_lookup_table vision_gpios_table = {
 	},
 };
 
-static int vision_spi_chipselects[] __initdata = {
-	EP93XX_GPIO_LINE_EGPIO6,
-	EP93XX_GPIO_LINE_EGPIO7,
-	EP93XX_GPIO_LINE_G(2),
-};
-
 static struct ep93xx_spi_info vision_spi_master __initdata = {
-	.chipselect	= vision_spi_chipselects,
-	.num_chipselect	= ARRAY_SIZE(vision_spi_chipselects),
+	.num_chipselect	= 3,
 	.use_dma	= 1,
 };
 
diff --git a/include/linux/platform_data/spi-ep93xx.h b/include/linux/platform_data/spi-ep93xx.h
index 171a271c2cbd..efcf33eff851 100644
--- a/include/linux/platform_data/spi-ep93xx.h
+++ b/include/linux/platform_data/spi-ep93xx.h
@@ -5,12 +5,10 @@ struct spi_device;
 
 /**
  * struct ep93xx_spi_info - EP93xx specific SPI descriptor
- * @chipselect: array of gpio numbers to use as chip selects
- * @num_chipselect: ARRAY_SIZE(chipselect)
+ * @num_chipselect: number chip selects supported
  * @use_dma: use DMA for the transfers
  */
 struct ep93xx_spi_info {
-	int	*chipselect;
 	int	num_chipselect;
 	bool	use_dma;
 };
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1652533

[Multipart message — attachments visible in raw view] — view raw

On Thu, May 25, 2017 at 04:30:38PM +1200, Chris Packham wrote:

> ep93xx wasn't as hard as I'd expected so I'm pretty happy with those
> changes. imx on the other hand has an annoying habit of conflating the GPIO
> and native chip-select so I'm pretty sure that's broken.

So, I was just looking at the list archives and I found that there's a
series that Hartley posted back in February modernizing the ep93xx
driver which didn't get copied to me so it's not being reviewed.  Can
you take a look and see if that helps with your updates to the driver?
The subject for the cover letter is "spi: spi-ep93xx: cleanup and update
driver to modern API".

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web