[PATCH] SPI: Unify simple platform data for some controllers

[PATCH] SPI: Unify simple platform data for some controllers

[Alexander Shiyan]

This patch unifies simple pdata for some SPI controllers where only
array of GPIOs is used for define chipselects.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/mach-imx/devices/devices-common.h      |  4 ++--
 arch/arm/mach-imx/devices/platform-spi_imx.c    |  2 +-
 arch/arm/mach-imx/eukrea_mbimx27-baseboard.c    |  2 +-
 arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c  |  2 +-
 arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c  |  2 +-
 arch/arm/mach-imx/hardware.h                    | 11 ++++++++++
 arch/arm/mach-imx/mach-cpuimx51sd.c             |  2 +-
 arch/arm/mach-imx/mach-mx27_3ds.c               |  4 ++--
 arch/arm/mach-imx/mach-mx31_3ds.c               |  4 ++--
 arch/arm/mach-imx/mach-mx31lilly.c              |  4 ++--
 arch/arm/mach-imx/mach-mx31lite.c               |  2 +-
 arch/arm/mach-imx/mach-mx31moboard.c            |  4 ++--
 arch/arm/mach-imx/mach-mx51_babbage.c           |  2 +-
 arch/arm/mach-imx/mach-pca100.c                 |  2 +-
 arch/arm/mach-imx/mach-pcm037_eet.c             |  2 +-
 arch/arm/mach-imx/mach-pcm038.c                 |  2 +-
 arch/arm/mach-imx/mx31lite-db.c                 |  2 +-
 drivers/spi/spi-clps711x.c                      |  4 ++--
 drivers/spi/spi-imx.c                           |  4 ++--
 include/linux/platform_data/spi-clps711x.h      | 21 -------------------
 include/linux/platform_data/spi-generic-pdata.h | 14 +++++++++++++
 include/linux/platform_data/spi-imx.h           | 27 -------------------------
 22 files changed, 50 insertions(+), 73 deletions(-)
 delete mode 100644 include/linux/platform_data/spi-clps711x.h
 create mode 100644 include/linux/platform_data/spi-generic-pdata.h
 delete mode 100644 include/linux/platform_data/spi-imx.h

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 453e20b..2f32b79 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -328,7 +328,7 @@ struct platform_device *__init imx_add_sdhci_esdhc_imx(
 		const struct imx_sdhci_esdhc_imx_data *data,
 		const struct esdhc_platform_data *pdata);
 
-#include <linux/platform_data/spi-imx.h>
+#include <linux/platform_data/spi-generic-pdata.h>
 struct imx_spi_imx_data {
 	const char *devid;
 	int id;
@@ -338,7 +338,7 @@ struct imx_spi_imx_data {
 };
 struct platform_device *__init imx_add_spi_imx(
 		const struct imx_spi_imx_data *data,
-		const struct spi_imx_master *pdata);
+		const struct spi_generic_pdata *pdata);
 
 struct platform_device *imx_add_imx_dma(char *name, resource_size_t iobase,
 					int irq, int irq_err);
diff --git a/arch/arm/mach-imx/devices/platform-spi_imx.c b/arch/arm/mach-imx/devices/platform-spi_imx.c
index 8880bcb..f33a49f 100644
--- a/arch/arm/mach-imx/devices/platform-spi_imx.c
+++ b/arch/arm/mach-imx/devices/platform-spi_imx.c
@@ -108,7 +108,7 @@ const struct imx_spi_imx_data imx53_ecspi_data[] __initconst = {
 
 struct platform_device *__init imx_add_spi_imx(
 		const struct imx_spi_imx_data *data,
-		const struct spi_imx_master *pdata)
+		const struct spi_generic_pdata *pdata)
 {
 	struct resource res[] = {
 		{
diff --git a/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c b/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
index b2f08bf..40d7c7c 100644
--- a/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
+++ b/arch/arm/mach-imx/eukrea_mbimx27-baseboard.c
@@ -273,7 +273,7 @@ static struct spi_board_info __maybe_unused
 
 static int eukrea_mbimx27_spi_cs[] = {GPIO_PORTD | 28};
 
-static const struct spi_imx_master eukrea_mbimx27_spi0_data __initconst = {
+static const struct spi_generic_pdata eukrea_mbimx27_spi0_data __initconst = {
 	.chipselect	= eukrea_mbimx27_spi_cs,
 	.num_chipselect = ARRAY_SIZE(eukrea_mbimx27_spi_cs),
 };
diff --git a/arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c b/arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c
index e2b70f4c..ab65190 100644
--- a/arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c
+++ b/arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c
@@ -258,7 +258,7 @@ static struct spi_board_info eukrea_mbimxsd25_spi_board_info[] __initdata = {
 
 static int eukrea_mbimxsd25_spi_cs[] = {GPIO_SPI1_SS0, GPIO_SPI1_SS1};
 
-static const struct spi_imx_master eukrea_mbimxsd25_spi0_data __initconst = {
+static const struct spi_generic_pdata eukrea_mbimxsd25_spi0_data __initconst = {
 	.chipselect     = eukrea_mbimxsd25_spi_cs,
 	.num_chipselect = ARRAY_SIZE(eukrea_mbimxsd25_spi_cs),
 };
diff --git a/arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c b/arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c
index 5a2d5ef..e6ffb7d 100644
--- a/arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c
+++ b/arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c
@@ -264,7 +264,7 @@ static struct spi_board_info eukrea_mbimxsd35_spi_board_info[] __initdata = {
 
 static int eukrea_mbimxsd35_spi_cs[] = {GPIO_SPI1_SS0, GPIO_SPI1_SS1};
 
-static const struct spi_imx_master eukrea_mbimxsd35_spi0_data __initconst = {
+static const struct spi_generic_pdata eukrea_mbimxsd35_spi0_data __initconst = {
 	.chipselect     = eukrea_mbimxsd35_spi_cs,
 	.num_chipselect = ARRAY_SIZE(eukrea_mbimxsd35_spi_cs),
 };
diff --git a/arch/arm/mach-imx/hardware.h b/arch/arm/mach-imx/hardware.h
index a3b0b04..d3b5a59 100644
--- a/arch/arm/mach-imx/hardware.h
+++ b/arch/arm/mach-imx/hardware.h
@@ -125,4 +125,15 @@
 /* range e.g. GPIO_1_5 is gpio 5 under linux */
 #define IMX_GPIO_NR(bank, nr)		(((bank) - 1) * 32 + (nr))
 
+/* Macro for using internal CSPI chipselects. Normally you */
+/* want to use gpio based chip selects as the CSPI module tries to be */
+/* intelligent about when to assert the chipselect: The CSPI module */
+/* deasserts the chipselect once it runs out of input data. The other */
+/* problem that it is not possible to mix between high active and low */
+/* active chipselects on one single bus using the internal chipselects. */
+/* Unfortunately Freescale decided to put some chipselects on dedicated */
+/* pins which are not usable as gpios, so we have to support the internal */
+/* chipselects. */
+#define MXC_SPI_CS(no)			((no) - 32)
+
 #endif /* __ASM_ARCH_MXC_HARDWARE_H__ */
diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
index 9b5ddf5..7950151 100644
--- a/arch/arm/mach-imx/mach-cpuimx51sd.c
+++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
@@ -264,7 +264,7 @@ static int cpuimx51sd_spi1_cs[] = {
 	CAN_NCS,
 };
 
-static const struct spi_imx_master cpuimx51sd_ecspi1_pdata __initconst = {
+static const struct spi_generic_pdata cpuimx51sd_ecspi1_pdata __initconst = {
 	.chipselect	= cpuimx51sd_spi1_cs,
 	.num_chipselect	= ARRAY_SIZE(cpuimx51sd_spi1_cs),
 };
diff --git a/arch/arm/mach-imx/mach-mx27_3ds.c b/arch/arm/mach-imx/mach-mx27_3ds.c
index 25b3e4c..2e7cf1c 100644
--- a/arch/arm/mach-imx/mach-mx27_3ds.c
+++ b/arch/arm/mach-imx/mach-mx27_3ds.c
@@ -357,14 +357,14 @@ static struct imx_ssi_platform_data mx27_3ds_ssi_pdata = {
 /* SPI */
 static int spi1_chipselect[] = {SPI1_SS0};
 
-static const struct spi_imx_master spi1_pdata __initconst = {
+static const struct spi_generic_pdata spi1_pdata __initconst = {
 	.chipselect	= spi1_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi1_chipselect),
 };
 
 static int spi2_chipselect[] = {SPI2_SS0};
 
-static const struct spi_imx_master spi2_pdata __initconst = {
+static const struct spi_generic_pdata spi2_pdata __initconst = {
 	.chipselect	= spi2_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi2_chipselect),
 };
diff --git a/arch/arm/mach-imx/mach-mx31_3ds.c b/arch/arm/mach-imx/mach-mx31_3ds.c
index 1ed9161..2ac1974 100644
--- a/arch/arm/mach-imx/mach-mx31_3ds.c
+++ b/arch/arm/mach-imx/mach-mx31_3ds.c
@@ -510,7 +510,7 @@ static int spi0_internal_chipselect[] = {
 	MXC_SPI_CS(2),
 };
 
-static const struct spi_imx_master spi0_pdata __initconst = {
+static const struct spi_generic_pdata spi0_pdata __initconst = {
 	.chipselect	= spi0_internal_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi0_internal_chipselect),
 };
@@ -520,7 +520,7 @@ static int spi1_internal_chipselect[] = {
 	MXC_SPI_CS(2),
 };
 
-static const struct spi_imx_master spi1_pdata __initconst = {
+static const struct spi_generic_pdata spi1_pdata __initconst = {
 	.chipselect	= spi1_internal_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi1_internal_chipselect),
 };
diff --git a/arch/arm/mach-imx/mach-mx31lilly.c b/arch/arm/mach-imx/mach-mx31lilly.c
index 832b1e2..426f14a 100644
--- a/arch/arm/mach-imx/mach-mx31lilly.c
+++ b/arch/arm/mach-imx/mach-mx31lilly.c
@@ -211,12 +211,12 @@ static int spi_internal_chipselect[] = {
 	MXC_SPI_CS(2),
 };
 
-static const struct spi_imx_master spi0_pdata __initconst = {
+static const struct spi_generic_pdata spi0_pdata __initconst = {
 	.chipselect = spi_internal_chipselect,
 	.num_chipselect = ARRAY_SIZE(spi_internal_chipselect),
 };
 
-static const struct spi_imx_master spi1_pdata __initconst = {
+static const struct spi_generic_pdata spi1_pdata __initconst = {
 	.chipselect = spi_internal_chipselect,
 	.num_chipselect = ARRAY_SIZE(spi_internal_chipselect),
 };
diff --git a/arch/arm/mach-imx/mach-mx31lite.c b/arch/arm/mach-imx/mach-mx31lite.c
index bea0729..12f1479 100644
--- a/arch/arm/mach-imx/mach-mx31lite.c
+++ b/arch/arm/mach-imx/mach-mx31lite.c
@@ -106,7 +106,7 @@ static int spi_internal_chipselect[] = {
 	MXC_SPI_CS(0),
 };
 
-static const struct spi_imx_master spi1_pdata __initconst = {
+static const struct spi_generic_pdata spi1_pdata __initconst = {
 	.chipselect	= spi_internal_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi_internal_chipselect),
 };
diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index dae4cd7..cf03cc6 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -168,7 +168,7 @@ static int moboard_spi1_cs[] = {
 	MXC_SPI_CS(2),
 };
 
-static const struct spi_imx_master moboard_spi1_pdata __initconst = {
+static const struct spi_generic_pdata moboard_spi1_pdata __initconst = {
 	.chipselect	= moboard_spi1_cs,
 	.num_chipselect	= ARRAY_SIZE(moboard_spi1_cs),
 };
@@ -316,7 +316,7 @@ static int moboard_spi2_cs[] = {
 	MXC_SPI_CS(1),
 };
 
-static const struct spi_imx_master moboard_spi2_pdata __initconst = {
+static const struct spi_generic_pdata moboard_spi2_pdata __initconst = {
 	.chipselect	= moboard_spi2_cs,
 	.num_chipselect	= ARRAY_SIZE(moboard_spi2_cs),
 };
diff --git a/arch/arm/mach-imx/mach-mx51_babbage.c b/arch/arm/mach-imx/mach-mx51_babbage.c
index f3d264a..a01c76a 100644
--- a/arch/arm/mach-imx/mach-mx51_babbage.c
+++ b/arch/arm/mach-imx/mach-mx51_babbage.c
@@ -336,7 +336,7 @@ static int mx51_babbage_spi_cs[] = {
 	BABBAGE_ECSPI1_CS1,
 };
 
-static const struct spi_imx_master mx51_babbage_spi_pdata __initconst = {
+static const struct spi_generic_pdata mx51_babbage_spi_pdata __initconst = {
 	.chipselect     = mx51_babbage_spi_cs,
 	.num_chipselect = ARRAY_SIZE(mx51_babbage_spi_cs),
 };
diff --git a/arch/arm/mach-imx/mach-pca100.c b/arch/arm/mach-imx/mach-pca100.c
index b8b15bb..47580c4 100644
--- a/arch/arm/mach-imx/mach-pca100.c
+++ b/arch/arm/mach-imx/mach-pca100.c
@@ -203,7 +203,7 @@ static struct spi_board_info pca100_spi_board_info[] __initdata = {
 
 static int pca100_spi_cs[] = {SPI1_SS0, SPI1_SS1};
 
-static const struct spi_imx_master pca100_spi0_data __initconst = {
+static const struct spi_generic_pdata pca100_spi0_data __initconst = {
 	.chipselect	= pca100_spi_cs,
 	.num_chipselect = ARRAY_SIZE(pca100_spi_cs),
 };
diff --git a/arch/arm/mach-imx/mach-pcm037_eet.c b/arch/arm/mach-imx/mach-pcm037_eet.c
index 8fd8255..3a6cbcd 100644
--- a/arch/arm/mach-imx/mach-pcm037_eet.c
+++ b/arch/arm/mach-imx/mach-pcm037_eet.c
@@ -58,7 +58,7 @@ static struct spi_board_info pcm037_spi_dev[] = {
 /* Platform Data for MXC CSPI */
 static int pcm037_spi1_cs[] = {MXC_SPI_CS(1), IOMUX_TO_GPIO(MX31_PIN_KEY_COL7)};
 
-static const struct spi_imx_master pcm037_spi1_pdata __initconst = {
+static const struct spi_generic_pdata pcm037_spi1_pdata __initconst = {
 	.chipselect = pcm037_spi1_cs,
 	.num_chipselect = ARRAY_SIZE(pcm037_spi1_cs),
 };
diff --git a/arch/arm/mach-imx/mach-pcm038.c b/arch/arm/mach-imx/mach-pcm038.c
index e805ac2..8c034d8 100644
--- a/arch/arm/mach-imx/mach-pcm038.c
+++ b/arch/arm/mach-imx/mach-pcm038.c
@@ -204,7 +204,7 @@ static struct i2c_board_info pcm038_i2c_devices[] = {
 
 static int pcm038_spi_cs[] = {GPIO_PORTD + 28};
 
-static const struct spi_imx_master pcm038_spi0_data __initconst = {
+static const struct spi_generic_pdata pcm038_spi0_data __initconst = {
 	.chipselect = pcm038_spi_cs,
 	.num_chipselect = ARRAY_SIZE(pcm038_spi_cs),
 };
diff --git a/arch/arm/mach-imx/mx31lite-db.c b/arch/arm/mach-imx/mx31lite-db.c
index 5a160b7..4b5f3b1 100644
--- a/arch/arm/mach-imx/mx31lite-db.c
+++ b/arch/arm/mach-imx/mx31lite-db.c
@@ -154,7 +154,7 @@ static int spi_internal_chipselect[] = {
 	MXC_SPI_CS(2),
 };
 
-static const struct spi_imx_master spi0_pdata __initconst = {
+static const struct spi_generic_pdata spi0_pdata __initconst = {
 	.chipselect	= spi_internal_chipselect,
 	.num_chipselect	= ARRAY_SIZE(spi_internal_chipselect),
 };
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 6859a02..0cd0f00 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -18,7 +18,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
-#include <linux/platform_data/spi-clps711x.h>
+#include <linux/platform_data/spi-generic-pdata.h>
 
 #include <mach/hardware.h>
 
@@ -168,7 +168,7 @@ static int spi_clps711x_probe(struct platform_device *pdev)
 	int i, ret;
 	struct spi_master *master;
 	struct spi_clps711x_data *hw;
-	struct spi_clps711x_pdata *pdata = dev_get_platdata(&pdev->dev);
+	struct spi_generic_pdata *pdata = dev_get_platdata(&pdev->dev);
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "No platform data supplied\n");
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 1e99b57..ad93086 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -38,7 +38,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 
-#include <linux/platform_data/spi-imx.h>
+#include <linux/platform_data/spi-generic-pdata.h>
 
 #define DRIVER_NAME "spi_imx"
 
@@ -754,7 +754,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(spi_imx_dt_ids, &pdev->dev);
-	struct spi_imx_master *mxc_platform_info =
+	struct spi_generic_pdata *mxc_platform_info =
 			dev_get_platdata(&pdev->dev);
 	struct spi_master *master;
 	struct spi_imx_data *spi_imx;
diff --git a/include/linux/platform_data/spi-clps711x.h b/include/linux/platform_data/spi-clps711x.h
deleted file mode 100644
index 301956e..0000000
--- a/include/linux/platform_data/spi-clps711x.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- *  CLPS711X SPI bus driver definitions
- *
- *  Copyright (C) 2012 Alexander Shiyan <shc_work@mail.ru>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef ____LINUX_PLATFORM_DATA_SPI_CLPS711X_H
-#define ____LINUX_PLATFORM_DATA_SPI_CLPS711X_H
-
-/* Board specific platform_data */
-struct spi_clps711x_pdata {
-	int *chipselect;	/* Array of GPIO-numbers */
-	int num_chipselect;	/* Total count of GPIOs */
-};
-
-#endif
diff --git a/include/linux/platform_data/spi-generic-pdata.h b/include/linux/platform_data/spi-generic-pdata.h
new file mode 100644
index 0000000..6c45a55
--- /dev/null
+++ b/include/linux/platform_data/spi-generic-pdata.h
@@ -0,0 +1,14 @@
+#ifndef _LINUX_PLATFORM_DATA_SPI_GENERIC_PDATA_H
+#define _LINUX_PLATFORM_DATA_SPI_GENERIC_PDATA_H
+
+/*
+ * struct spi_generic_pdata - device.platform_data for SPI controller devices.
+ * @chipselect: Array of chipselects for this master.
+ * @num_chipselect: ARRAY_SIZE(chipselect)
+ */
+struct spi_generic_pdata {
+	int	*chipselect;
+	int	num_chipselect;
+};
+
+#endif
diff --git a/include/linux/platform_data/spi-imx.h b/include/linux/platform_data/spi-imx.h
deleted file mode 100644
index 08be445..0000000
--- a/include/linux/platform_data/spi-imx.h
+++ /dev/null
@@ -1,27 +0,0 @@
-
-#ifndef __MACH_SPI_H_
-#define __MACH_SPI_H_
-
-/*
- * struct spi_imx_master - device.platform_data for SPI controller devices.
- * @chipselect: Array of chipselects for this master. Numbers >= 0 mean gpio
- *              pins, numbers < 0 mean internal CSPI chipselects according
- *              to MXC_SPI_CS(). Normally you want to use gpio based chip
- *              selects as the CSPI module tries to be intelligent about
- *              when to assert the chipselect: The CSPI module deasserts the
- *              chipselect once it runs out of input data. The other problem
- *              is that it is not possible to mix between high active and low
- *              active chipselects on one single bus using the internal
- *              chipselects. Unfortunately Freescale decided to put some
- *              chipselects on dedicated pins which are not usable as gpios,
- *              so we have to support the internal chipselects.
- * @num_chipselect: ARRAY_SIZE(chipselect)
- */
-struct spi_imx_master {
-	int	*chipselect;
-	int	num_chipselect;
-};
-
-#define MXC_SPI_CS(no)	((no) - 32)
-
-#endif /* __MACH_SPI_H_*/
-- 
1.8.1.5

[PATCH] SPI: Unify simple platform data for some controllers

[Arnd Bergmann]

On Wednesday 29 May 2013 10:42:22 Alexander Shiyan wrote:
> This patch unifies simple pdata for some SPI controllers where only
> array of GPIOs is used for define chipselects.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>

Wouldn't it be better to kill off the platform_data for these drivers
and pass the gpio number through spi->controller_data as the
spi_gpio driver does?

	Arnd

Re: [PATCH] SPI: Unify simple platform data for some controllers

[Alexander Shiyan]

> On Wednesday 29 May 2013 10:42:22 Alexander Shiyan wrote:
> > This patch unifies simple pdata for some SPI controllers where only
> > array of GPIOs is used for define chipselects.
> > 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> 
> Wouldn't it be better to kill off the platform_data for these drivers
> and pass the gpio number through spi->controller_data as the
> spi_gpio driver does?

Interesting way. Just one question remains, for using not-dt variant,
how  we should specify num_chipselect parameter for master?
I see only way for specify maximum constant in the driver.

---

[PATCH] SPI: Unify simple platform data for some controllers

[Mark Brown]

On Wed, May 29, 2013 at 12:49:13PM +0400, Alexander Shiyan wrote:
> > On Wednesday 29 May 2013 10:42:22 Alexander Shiyan wrote:

> > Wouldn't it be better to kill off the platform_data for these drivers
> > and pass the gpio number through spi->controller_data as the
> > spi_gpio driver does?

> Interesting way. Just one question remains, for using not-dt variant,
> how  we should specify num_chipselect parameter for master?
> I see only way for specify maximum constant in the driver.

We should fix that to be dynamic, having a fixed number is not sensible
when GPIOs are usable for chip select.  We probably also want to make
GPIO chip select support a standard thing that's available with every
driver rather than something the driver has to know about but that's a
separate bit of work.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130529/a1431524/attachment.sig>

[PATCH] SPI: Unify simple platform data for some controllers

[Arnd Bergmann]

On Wednesday 29 May 2013, Mark Brown wrote:
> On Wed, May 29, 2013 at 12:49:13PM +0400, Alexander Shiyan wrote:
> > > On Wednesday 29 May 2013 10:42:22 Alexander Shiyan wrote:
> 
> > > Wouldn't it be better to kill off the platform_data for these drivers
> > > and pass the gpio number through spi->controller_data as the
> > > spi_gpio driver does?
> 
> > Interesting way. Just one question remains, for using not-dt variant,
> > how  we should specify num_chipselect parameter for master?
> > I see only way for specify maximum constant in the driver.
> 
> We should fix that to be dynamic, having a fixed number is not sensible
> when GPIOs are usable for chip select. 

Agreed (not that I have much of a say on this matter).

> We probably also want to make
> GPIO chip select support a standard thing that's available with every
> driver rather than something the driver has to know about but that's a
> separate bit of work.

That actually seems simpler than doing it in just one driver: if
we add a cs_gpio field to spi_board_info, spi_new_device could
just copy that information into the new spi_device instead of
taking it from master->cs_gpios.

	Arnd

[PATCH] SPI: Unify simple platform data for some controllers

[Mark Brown]

On Wed, May 29, 2013 at 03:26:34PM +0200, Arnd Bergmann wrote:
> On Wednesday 29 May 2013, Mark Brown wrote:

> > We probably also want to make
> > GPIO chip select support a standard thing that's available with every
> > driver rather than something the driver has to know about but that's a
> > separate bit of work.

> That actually seems simpler than doing it in just one driver: if
> we add a cs_gpio field to spi_board_info, spi_new_device could
> just copy that information into the new spi_device instead of
> taking it from master->cs_gpios.

Yeah, that bit of it is fine and straightforward.  It's slightly more
work to factor the chip select handling out so that we can implement the
handling at the appropriate moments in the flow due to the fact that
it's embedded in the message transfer functions but totally doable.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130529/73ebb588/attachment-0001.sig>

Re: [PATCH] SPI: Unify simple platform data for some controllers

[Alexander Shiyan]

> On Wed, May 29, 2013 at 03:26:34PM +0200, Arnd Bergmann wrote:
> > On Wednesday 29 May 2013, Mark Brown wrote:
> 
> > > We probably also want to make
> > > GPIO chip select support a standard thing that's available with every
> > > driver rather than something the driver has to know about but that's a
> > > separate bit of work.
> 
> > That actually seems simpler than doing it in just one driver: if
> > we add a cs_gpio field to spi_board_info, spi_new_device could
> > just copy that information into the new spi_device instead of
> > taking it from master->cs_gpios.
> 
> Yeah, that bit of it is fine and straightforward.  It's slightly more
> work to factor the chip select handling out so that we can implement the
> handling at the appropriate moments in the flow due to the fact that
> it's embedded in the message transfer functions but totally doable.

Initial try to simplify i.MX SPI driver I will send in 5 min.
Patches is untested, so it is only RFC. Platform data still present in
driver but patch migrate driver to using dynamic counter of
chipselects for dt-case. Is it true way now? Comments are welcome.
Thanks.

---

[PATCH] SPI: Unify simple platform data for some controllers

[Arnd Bergmann]

On Wednesday 29 May 2013 21:05:59 Alexander Shiyan wrote:
> > On Wed, May 29, 2013 at 03:26:34PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 29 May 2013, Mark Brown wrote:
> > 
> > > > We probably also want to make
> > > > GPIO chip select support a standard thing that's available with every
> > > > driver rather than something the driver has to know about but that's a
> > > > separate bit of work.
> > 
> > > That actually seems simpler than doing it in just one driver: if
> > > we add a cs_gpio field to spi_board_info, spi_new_device could
> > > just copy that information into the new spi_device instead of
> > > taking it from master->cs_gpios.
> > 
> > Yeah, that bit of it is fine and straightforward.  It's slightly more
> > work to factor the chip select handling out so that we can implement the
> > handling at the appropriate moments in the flow due to the fact that
> > it's embedded in the message transfer functions but totally doable.
> 
> Initial try to simplify i.MX SPI driver I will send in 5 min.
> Patches is untested, so it is only RFC. Platform data still present in
> driver but patch migrate driver to using dynamic counter of
> chipselects for dt-case. Is it true way now? Comments are welcome.

The plan for i.MX is to move to DT-only in the long run, but
AFAIK a lot of people still rely on board files and they are
not trivial to convert without access to test hardware.

Any new i.MX hardware should certainly be DT-only.

Shawn can probably comment better on the time line.

	Arnd

[PATCH] SPI: Unify simple platform data for some controllers

[Shawn Guo]

On Wed, May 29, 2013 at 08:10:05PM +0200, Arnd Bergmann wrote:
> The plan for i.MX is to move to DT-only in the long run, but
> AFAIK a lot of people still rely on board files and they are
> not trivial to convert without access to test hardware.
> 
> Any new i.MX hardware should certainly be DT-only.
> 
> Shawn can probably comment better on the time line.
> 
It's progressing slowly, and I can not really commit a time line because
there are too many board files to be converted, and more importantly
I haven't seen too much effort from board owners on this work.

Shawn

[RFC 1/3] SPI: imx: Convert to devm_* API

[Shawn Guo]

On Wed, May 29, 2013 at 09:10:28PM +0400, Alexander Shiyan wrote:
> Use devm_* functions for the driver.
> This ensures more consistent error values and simplifies error paths.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/spi/spi-imx.c | 79 ++++++++++++++-------------------------------------
>  1 file changed, 21 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 0befeeb..1cd4473 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -85,7 +85,6 @@ struct spi_imx_data {
>  
>  	struct completion xfer_done;
>  	void __iomem *base;
> -	int irq;
>  	struct clk *clk_per;
>  	struct clk *clk_ipg;
>  	unsigned long spi_clk;
> @@ -761,7 +760,7 @@ static int spi_imx_probe(struct platform_device *pdev)
>  	struct spi_imx_data *spi_imx;
>  	struct resource *res;
>  	struct pinctrl *pinctrl;
> -	int i, ret, num_cs;
> +	int i, irq, ret, num_cs;
>  
>  	if (!np && !mxc_platform_info) {
>  		dev_err(&pdev->dev, "can't get the platform data\n");
> @@ -798,7 +797,8 @@ static int spi_imx_probe(struct platform_device *pdev)
>  		if (!gpio_is_valid(cs_gpio))
>  			continue;
>  
> -		ret = gpio_request(spi_imx->chipselect[i], DRIVER_NAME);
> +		ret = devm_gpio_request(&pdev->dev, spi_imx->chipselect[i],
> +					DRIVER_NAME);
>  		if (ret) {
>  			dev_err(&pdev->dev, "can't get cs gpios\n");
>  			goto out_gpio_free;
> @@ -818,52 +818,41 @@ static int spi_imx_probe(struct platform_device *pdev)
>  		(struct spi_imx_devtype_data *) pdev->id_entry->driver_data;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(&pdev->dev, "can't get platform resource\n");
> -		ret = -ENOMEM;
> +	spi_imx->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(spi_imx->base)) {
> +		ret = PTR_ERR(spi_imx->base);
>  		goto out_gpio_free;
>  	}
>  
> -	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
> -		dev_err(&pdev->dev, "request_mem_region failed\n");
> -		ret = -EBUSY;
> -		goto out_gpio_free;
> -	}
> -
> -	spi_imx->base = ioremap(res->start, resource_size(res));
> -	if (!spi_imx->base) {
> -		ret = -EINVAL;
> -		goto out_release_mem;
> -	}
> -
> -	spi_imx->irq = platform_get_irq(pdev, 0);
> -	if (spi_imx->irq < 0) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
>  		ret = -EINVAL;
> -		goto out_iounmap;
> +		goto out_gpio_free;
>  	}
>  
> -	ret = request_irq(spi_imx->irq, spi_imx_isr, 0, DRIVER_NAME, spi_imx);
> +	ret = devm_request_irq(&pdev->dev, irq, spi_imx_isr, 0, DRIVER_NAME,
> +			       spi_imx);
>  	if (ret) {
> -		dev_err(&pdev->dev, "can't get irq%d: %d\n", spi_imx->irq, ret);
> -		goto out_iounmap;
> +		dev_err(&pdev->dev, "can't get irq%d: %d\n", irq, ret);
> +		goto out_gpio_free;
>  	}
>  
>  	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
>  	if (IS_ERR(pinctrl)) {
>  		ret = PTR_ERR(pinctrl);
> -		goto out_free_irq;
> +		goto out_gpio_free;
>  	}
>  
>  	spi_imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
>  	if (IS_ERR(spi_imx->clk_ipg)) {
>  		ret = PTR_ERR(spi_imx->clk_ipg);
> -		goto out_free_irq;
> +		goto out_gpio_free;
>  	}
>  
>  	spi_imx->clk_per = devm_clk_get(&pdev->dev, "per");
>  	if (IS_ERR(spi_imx->clk_per)) {
>  		ret = PTR_ERR(spi_imx->clk_per);
> -		goto out_free_irq;
> +		goto out_gpio_free;
>  	}
>  
>  	clk_prepare_enable(spi_imx->clk_per);
> @@ -877,60 +866,34 @@ static int spi_imx_probe(struct platform_device *pdev)
>  
>  	master->dev.of_node = pdev->dev.of_node;
>  	ret = spi_bitbang_start(&spi_imx->bitbang);

---8<-----
> -	if (ret) {
> -		dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
> -		goto out_clk_put;
> -	}
> -
> -	dev_info(&pdev->dev, "probed\n");
> +	if (!ret)
> +		return 0;
>  
> -	return ret;
> +	dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
>  
> -out_clk_put:
--->8------

I do not understand why above changes are necessary.

>  	clk_disable_unprepare(spi_imx->clk_per);
>  	clk_disable_unprepare(spi_imx->clk_ipg);
> -out_free_irq:
> -	free_irq(spi_imx->irq, spi_imx);
> -out_iounmap:
> -	iounmap(spi_imx->base);
> -out_release_mem:
> -	release_mem_region(res->start, resource_size(res));
> +
>  out_gpio_free:
> -	while (--i >= 0) {
> -		if (gpio_is_valid(spi_imx->chipselect[i]))
> -			gpio_free(spi_imx->chipselect[i]);
> -	}

With the change, label out_gpio_free becomes improper, and we should
probably rename it.

Shawn

>  	spi_master_put(master);
>  	kfree(master);
> -	platform_set_drvdata(pdev, NULL);
> +
>  	return ret;
>  }
>  
>  static int spi_imx_remove(struct platform_device *pdev)
>  {
>  	struct spi_master *master = platform_get_drvdata(pdev);
> -	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
> -	int i;
>  
>  	spi_bitbang_stop(&spi_imx->bitbang);
>  
>  	writel(0, spi_imx->base + MXC_CSPICTRL);
>  	clk_disable_unprepare(spi_imx->clk_per);
>  	clk_disable_unprepare(spi_imx->clk_ipg);
> -	free_irq(spi_imx->irq, spi_imx);
> -	iounmap(spi_imx->base);
> -
> -	for (i = 0; i < master->num_chipselect; i++)
> -		if (gpio_is_valid(spi_imx->chipselect[i]))
> -			gpio_free(spi_imx->chipselect[i]);
>  
>  	spi_master_put(master);
>  
> -	release_mem_region(res->start, resource_size(res));
> -
> -	platform_set_drvdata(pdev, NULL);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.1.5
> 

[RFC 2/3] SPI: imx: Using SPI-master "cs_gpios" field for array of chipselects instead of private

[Shawn Guo]

On Wed, May 29, 2013 at 09:10:29PM +0400, Alexander Shiyan wrote:
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

> ---
>  drivers/spi/spi-imx.c | 49 +++++++++++++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 22 deletions(-)

[RFC 2/3] SPI: imx: Using SPI-master "cs_gpios" field for array of chipselects instead of private

[Shawn Guo]

On Wed, May 29, 2013 at 09:10:29PM +0400, Alexander Shiyan wrote:
> @@ -356,11 +356,12 @@ static void __maybe_unused mx31_trigger(struct spi_imx_data *spi_imx)
>  	writel(reg, spi_imx->base + MXC_CSPICTRL);
>  }
>  
> -static int __maybe_unused mx31_config(struct spi_imx_data *spi_imx,
> +static int __maybe_unused mx31_config(struct spi_device *spi,
>  		struct spi_imx_config *config)
>  {
> +	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
>  	unsigned int reg = MX31_CSPICTRL_ENABLE | MX31_CSPICTRL_MASTER;
> -	int cs = spi_imx->chipselect[config->cs];
> +	int cs = spi->master->cs_gpios[config->cs];

Hmm, just took another look and found it (and all the following) can be
even simpler like:

	int cs = spi->cs_gpio;

Shawn

>  
>  	reg |= spi_imx_clkdiv_2(spi_imx->spi_clk, config->speed_hz) <<
>  		MX31_CSPICTRL_DR_SHIFT;
> @@ -434,11 +435,12 @@ static void __maybe_unused mx21_trigger(struct spi_imx_data *spi_imx)
>  	writel(reg, spi_imx->base + MXC_CSPICTRL);
>  }
>  
> -static int __maybe_unused mx21_config(struct spi_imx_data *spi_imx,
> +static int __maybe_unused mx21_config(struct spi_device *spi,
>  		struct spi_imx_config *config)
>  {
> +	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
>  	unsigned int reg = MX21_CSPICTRL_ENABLE | MX21_CSPICTRL_MASTER;
> -	int cs = spi_imx->chipselect[config->cs];
> +	int cs = spi->master->cs_gpios[config->cs];
>  	unsigned int max = is_imx27_cspi(spi_imx) ? 16 : 18;
>  
>  	reg |= spi_imx_clkdiv_1(spi_imx->spi_clk, config->speed_hz, max) <<
> @@ -501,9 +503,10 @@ static void __maybe_unused mx1_trigger(struct spi_imx_data *spi_imx)
>  	writel(reg, spi_imx->base + MXC_CSPICTRL);
>  }
>  
> -static int __maybe_unused mx1_config(struct spi_imx_data *spi_imx,
> +static int __maybe_unused mx1_config(struct spi_device *spi,
>  		struct spi_imx_config *config)
>  {
> +	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
>  	unsigned int reg = MX1_CSPICTRL_ENABLE | MX1_CSPICTRL_MASTER;
>  
>  	reg |= spi_imx_clkdiv_2(spi_imx->spi_clk, config->speed_hz) <<
> @@ -622,8 +625,7 @@ static const struct of_device_id spi_imx_dt_ids[] = {
>  
>  static void spi_imx_chipselect(struct spi_device *spi, int is_active)
>  {
> -	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
> -	int gpio = spi_imx->chipselect[spi->chip_select];
> +	int gpio = spi->master->cs_gpios[spi->chip_select];
>  	int active = is_active != BITBANG_CS_INACTIVE;
>  	int dev_is_lowactive = !(spi->mode & SPI_CS_HIGH);
>  
> @@ -703,7 +705,7 @@ static int spi_imx_setupxfer(struct spi_device *spi,
>  	} else
>  		BUG();
>  
> -	spi_imx->devtype_data->config(spi_imx, &config);
> +	spi_imx->devtype_data->config(spi, &config);
>  
>  	return 0;
>  }
> @@ -731,8 +733,7 @@ static int spi_imx_transfer(struct spi_device *spi,
>  
>  static int spi_imx_setup(struct spi_device *spi)
>  {
> -	struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
> -	int gpio = spi_imx->chipselect[spi->chip_select];
> +	int gpio = spi->master->cs_gpios[spi->chip_select];
>  
>  	dev_dbg(&spi->dev, "%s: mode %d, %u bpw, %d hz\n", __func__,
>  		 spi->mode, spi->bits_per_word, spi->max_speed_hz);

[RFC 3/3] SPI: imx: Using dynamic count of chipselects provided by of_spi_register_master

[Shawn Guo]

On Wed, May 29, 2013 at 09:10:30PM +0400, Alexander Shiyan wrote:
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../devicetree/bindings/spi/fsl-imx-cspi.txt       |  2 -
>  arch/arm/boot/dts/imx27-apf27dev.dts               |  2 -
>  arch/arm/boot/dts/imx51-apf51dev.dts               |  2 -
>  arch/arm/boot/dts/imx51-babbage.dts                |  1 -
>  arch/arm/boot/dts/imx53-evk.dts                    |  1 -
>  arch/arm/boot/dts/imx53-smd.dts                    |  1 -
>  arch/arm/boot/dts/imx53-tqma53.dtsi                |  2 -
>  arch/arm/boot/dts/imx6q-sabrelite.dts              |  1 -
>  drivers/spi/spi-imx.c                              | 51 ++++++++++------------
>  9 files changed, 22 insertions(+), 41 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> index 4256a6d..069592d 100644
> --- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> +++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
> @@ -5,7 +5,6 @@ Required properties:
>  - compatible : Should be "fsl,<soc>-cspi" or "fsl,<soc>-ecspi"
>  - reg : Offset and length of the register set for the device
>  - interrupts : Should contain CSPI/eCSPI interrupt
> -- fsl,spi-num-chipselects : Contains the number of the chipselect
>  - cs-gpios : Specifies the gpio pins to be used for chipselects.
>  
>  Example:
> @@ -16,7 +15,6 @@ ecspi at 70010000 {
>  	compatible = "fsl,imx51-ecspi";
>  	reg = <0x70010000 0x4000>;
>  	interrupts = <36>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio3 24 0>, /* GPIO3_24 */
>  		   <&gpio3 25 0>; /* GPIO3_25 */
>  };
> diff --git a/arch/arm/boot/dts/imx27-apf27dev.dts b/arch/arm/boot/dts/imx27-apf27dev.dts
> index 66b8e1c..6e71949 100644
> --- a/arch/arm/boot/dts/imx27-apf27dev.dts
> +++ b/arch/arm/boot/dts/imx27-apf27dev.dts
> @@ -38,13 +38,11 @@
>  };
>  
>  &cspi1 {
> -	fsl,spi-num-chipselects = <1>;
>  	cs-gpios = <&gpio4 28 1>;
>  	status = "okay";
>  };
>  
>  &cspi2 {
> -	fsl,spi-num-chipselects = <3>;
>  	cs-gpios = <&gpio4 21 1>, <&gpio4 27 1>,
>  			<&gpio2 17 1>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts b/arch/arm/boot/dts/imx51-apf51dev.dts
> index 123fe84..557b616 100644
> --- a/arch/arm/boot/dts/imx51-apf51dev.dts
> +++ b/arch/arm/boot/dts/imx51-apf51dev.dts
> @@ -40,7 +40,6 @@
>  &ecspi1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio4 24 0>, <&gpio4 25 0>;
>  	status = "okay";
>  };
> @@ -48,7 +47,6 @@
>  &ecspi2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi2_1>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio3 28 1>, <&gpio3 27 1>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 6dd9486..0a6c443 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -89,7 +89,6 @@
>  &ecspi1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio4 24 0>, <&gpio4 25 0>;
>  	status = "okay";
>  
> diff --git a/arch/arm/boot/dts/imx53-evk.dts b/arch/arm/boot/dts/imx53-evk.dts
> index 801fda7..6a9b1e7 100644
> --- a/arch/arm/boot/dts/imx53-evk.dts
> +++ b/arch/arm/boot/dts/imx53-evk.dts
> @@ -43,7 +43,6 @@
>  &ecspi1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>;
>  	status = "okay";
>  
> diff --git a/arch/arm/boot/dts/imx53-smd.dts b/arch/arm/boot/dts/imx53-smd.dts
> index a9b6e10..6d86ae8 100644
> --- a/arch/arm/boot/dts/imx53-smd.dts
> +++ b/arch/arm/boot/dts/imx53-smd.dts
> @@ -63,7 +63,6 @@
>  &ecspi1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> -	fsl,spi-num-chipselects = <2>;
>  	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>;
>  	status = "okay";
>  
> diff --git a/arch/arm/boot/dts/imx53-tqma53.dtsi b/arch/arm/boot/dts/imx53-tqma53.dtsi
> index 38bed3e..bd5a024 100644
> --- a/arch/arm/boot/dts/imx53-tqma53.dtsi
> +++ b/arch/arm/boot/dts/imx53-tqma53.dtsi
> @@ -50,7 +50,6 @@
>  &ecspi1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> -	fsl,spi-num-chipselects = <4>;
>  	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>,
>  		   <&gpio3 24 0>, <&gpio3 25 0>;
>  	status = "disabled";
> @@ -133,7 +132,6 @@
>  &cspi {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_cspi_1>;
> -	fsl,spi-num-chipselects = <3>;
>  	cs-gpios = <&gpio1 18 0>, <&gpio1 19 0>,
>  		   <&gpio1 21 0>;
>  	status = "disabled";
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 6a00066..a47647d 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -66,7 +66,6 @@
>  };
>  
>  &ecspi1 {
> -	fsl,spi-num-chipselects = <1>;
>  	cs-gpios = <&gpio3 19 0>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_ecspi1_1>;
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 3a11b20..5384a42 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -761,53 +761,46 @@ static int spi_imx_probe(struct platform_device *pdev)
>  	struct spi_imx_data *spi_imx;
>  	struct resource *res;
>  	struct pinctrl *pinctrl;
> -	int i, irq, ret, num_cs;
> +	int i, irq, ret;
>  
>  	if (!np && !mxc_platform_info) {
>  		dev_err(&pdev->dev, "can't get the platform data\n");
>  		return -EINVAL;
>  	}
>  
> -	ret = of_property_read_u32(np, "fsl,spi-num-chipselects", &num_cs);
> -	if (ret < 0) {
> -		if (mxc_platform_info)
> -			num_cs = mxc_platform_info->num_chipselect;
> -		else
> -			return ret;
> -	}
> -
>  	master = spi_alloc_master(&pdev->dev, sizeof(struct spi_imx_data));
>  	if (!master)
>  		return -ENOMEM;
>  
> -	master->cs_gpios = devm_kzalloc(&pdev->dev, sizeof(int) * num_cs,
> -					GFP_KERNEL);
> -	if (!master->cs_gpios) {
> -		ret = -ENOMEM;
> -		goto out_master_free;
> +	if (!np && mxc_platform_info) {
> +		master->num_chipselect = mxc_platform_info->num_chipselect;
> +
> +		master->cs_gpios = devm_kzalloc(&pdev->dev, sizeof(int) *
> +						master->num_chipselect,
> +						GFP_KERNEL);
> +		if (!master->cs_gpios) {
> +			ret = -ENOMEM;
> +			goto out_master_free;
> +		}
> +
> +		for (i = 0; i < master->num_chipselect; i++) {
> +			master->cs_gpios[i] = mxc_platform_info->chipselect[i];
> +			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_gpio_free;
> +			}
> +		}
>  	}
>  
>  	platform_set_drvdata(pdev, master);
>  
>  	master->bus_num = pdev->id;
> -	master->num_chipselect = num_cs;
>  
>  	spi_imx = spi_master_get_devdata(master);
>  	spi_imx->bitbang.master = spi_master_get(master);
> -
> -	for (i = 0; i < master->num_chipselect; i++) {
> -		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> -		if (!gpio_is_valid(cs_gpio) && mxc_platform_info)
> -			cs_gpio = mxc_platform_info->chipselect[i];
> -
> -		master->cs_gpios[i] = cs_gpio;
> -		ret = devm_gpio_request(&pdev->dev, cs_gpio, DRIVER_NAME);

It seems that of_spi_register_master() does not request gpio, right?

Shawn

> -		if (ret) {
> -			dev_err(&pdev->dev, "can't get cs gpio %i\n", cs_gpio);
> -			goto out_gpio_free;
> -		}
> -	}
> -
>  	spi_imx->bitbang.chipselect = spi_imx_chipselect;
>  	spi_imx->bitbang.setup_transfer = spi_imx_setupxfer;
>  	spi_imx->bitbang.txrx_bufs = spi_imx_transfer;
> -- 
> 1.8.1.5
> 

[RFC 1/3] SPI: imx: Convert to devm_* API

[Shawn Guo]

On Thu, May 30, 2013 at 07:50:38AM +0400, Alexander Shiyan wrote:
> > On Wed, May 29, 2013 at 09:10:28PM +0400, Alexander Shiyan wrote:
> > > Use devm_* functions for the driver.
> > > This ensures more consistent error values and simplifies error paths.
> > > 
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > ---
> > >  drivers/spi/spi-imx.c | 79 ++++++++++++++-------------------------------------
> > >  1 file changed, 21 insertions(+), 58 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> ...
> > >  	master->dev.of_node = pdev->dev.of_node;
> > >  	ret = spi_bitbang_start(&spi_imx->bitbang);
> > 
> > ---8<-----
> > > -	if (ret) {
> > > -		dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
> > > -		goto out_clk_put;
> > > -	}
> > > -
> > > -	dev_info(&pdev->dev, "probed\n");
> > > +	if (!ret)
> > > +		return 0;
> > >  
> > > -	return ret;
> > > +	dev_err(&pdev->dev, "bitbang start failed with %d\n", ret);
> > >  
> > > -out_clk_put:
> > --->8------
> > 
> > I do not understand why above changes are necessary.
> 
> This just simplifies the error path and removes one "goto".
> 
Sorry, I do not get it and take this as a diff stat churn.

Shawn