[PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info

Subsystems: arm port, the rest, ti davinci machine support

STALE5007d

3 messages, 2 authors, 2012-11-28 · open the first message on its own page

[PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info

From: Vivien Didelot <hidden>
Date: 2012-09-11 00:29:38

Without this patch, da8xx_register_spi() registers the SPI board info,
the SPI controller, and sets its number of chipselect to the size of the
static spi_board_info array. This is bad because a SPI board info may
declare devices for different SPI buses, and because other code can also
call spi_register_board_info() (e.g. a daughter board might provide
additional SPI devices).

This patch removes the spi_register_board_info() call from
da8xx_register_spi(), renames this last one to da8xx_register_spi_bus()
to be more explicit, takes the number of chipselect as a function
parameter, and updates the impacted board-da8{3,5}0-evm.c, and
board-mityomapl138.c files accordingly. It also sets the SPI platform
data static, as it doesn't need to be exported.

Signed-off-by: Vivien Didelot <redacted>
---
 arch/arm/mach-davinci/board-da830-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-da850-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-mityomapl138.c |  9 +++++++--
 arch/arm/mach-davinci/devices-da8xx.c      | 14 +++-----------
 arch/arm/mach-davinci/include/mach/da8xx.h |  3 +--
 5 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 11c3db9..adb8d87 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -652,8 +652,13 @@ static __init void da830_evm_init(void)
 	if (ret)
 		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
 
-	ret = da8xx_register_spi(0, da830evm_spi_info,
-				 ARRAY_SIZE(da830evm_spi_info));
+	ret = spi_register_board_info(da830evm_spi_info,
+				      ARRAY_SIZE(da830evm_spi_info));
+	if (ret)
+		pr_warning("da830_evm_init: spi info registration failed: %d\n",
+			   ret);
+
+	ret = da8xx_register_spi_bus(0, ARRAY_SIZE(da830evm_spi_info));
 	if (ret)
 		pr_warning("da830_evm_init: spi 0 registration failed: %d\n",
 			   ret);
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 6659a90..3b2c9e3 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1374,8 +1374,13 @@ static __init void da850_evm_init(void)
 		pr_warning("da850_evm_init: suspend registration failed: %d\n",
 				ret);
 
-	ret = da8xx_register_spi(1, da850evm_spi_info,
-				 ARRAY_SIZE(da850evm_spi_info));
+	ret = spi_register_board_info(da850evm_spi_info,
+				      ARRAY_SIZE(da850evm_spi_info));
+	if (ret)
+		pr_warning("da850_evm_init: spi info registration failed: %d\n",
+			   ret);
+
+	ret = da8xx_register_spi_bus(1, ARRAY_SIZE(da850evm_spi_info));
 	if (ret)
 		pr_warning("da850_evm_init: spi 1 registration failed: %d\n",
 				ret);
diff --git a/arch/arm/mach-davinci/board-mityomapl138.c b/arch/arm/mach-davinci/board-mityomapl138.c
index 3cfff55..9f92f11 100644
--- a/arch/arm/mach-davinci/board-mityomapl138.c
+++ b/arch/arm/mach-davinci/board-mityomapl138.c
@@ -529,8 +529,13 @@ static void __init mityomapl138_init(void)
 
 	mityomapl138_setup_nand();
 
-	ret = da8xx_register_spi(1, mityomapl138_spi_flash_info,
-			       ARRAY_SIZE(mityomapl138_spi_flash_info));
+	ret = spi_register_board_info(mityomapl138_spi_flash_info,
+				      ARRAY_SIZE(mityomapl138_spi_flash_info));
+	if (ret)
+		pr_warning("spi info registration failed: %d\n", ret);
+
+	ret = da8xx_register_spi_bus(1,
+				     ARRAY_SIZE(mityomapl138_spi_flash_info));
 	if (ret)
 		pr_warning("spi 1 registration failed: %d\n", ret);
 
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index d91ab9e..e86117b 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -792,7 +792,7 @@ static struct resource da8xx_spi1_resources[] = {
 	},
 };
 
-struct davinci_spi_platform_data da8xx_spi_pdata[] = {
+static struct davinci_spi_platform_data da8xx_spi_pdata[] = {
 	[0] = {
 		.version	= SPI_VERSION_2,
 		.intr_line	= 1,
@@ -826,20 +826,12 @@ static struct platform_device da8xx_spi_device[] = {
 	},
 };
 
-int __init da8xx_register_spi(int instance, struct spi_board_info *info,
-			      unsigned len)
+int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect)
 {
-	int ret;
-
 	if (instance < 0 || instance > 1)
 		return -EINVAL;
 
-	ret = spi_register_board_info(info, len);
-	if (ret)
-		pr_warning("%s: failed to register board info for spi %d :"
-			   " %d\n", __func__, instance, ret);
-
-	da8xx_spi_pdata[instance].num_chipselect = len;
+	da8xx_spi_pdata[instance].num_chipselect = num_chipselect;
 
 	if (instance == 1 && cpu_is_davinci_da850()) {
 		da8xx_spi1_resources[0].start = DA850_SPI1_BASE;
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 53ee8a2..aa2d665 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -76,7 +76,7 @@ void __init da850_init(void);
 int da830_register_edma(struct edma_rsv_info *rsv);
 int da850_register_edma(struct edma_rsv_info *rsv[2]);
 int da8xx_register_i2c(int instance, struct davinci_i2c_platform_data *pdata);
-int da8xx_register_spi(int instance, struct spi_board_info *info, unsigned len);
+int da8xx_register_spi_bus(int instance, unsigned num_chipselect);
 int da8xx_register_watchdog(void);
 int da8xx_register_usb20(unsigned mA, unsigned potpgt);
 int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
@@ -96,7 +96,6 @@ extern struct platform_device da8xx_serial_device;
 extern struct emac_platform_data da8xx_emac_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lcd035q3dg01_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lk043t1dg01_pdata;
-extern struct davinci_spi_platform_data da8xx_spi_pdata[];
 
 extern struct platform_device da8xx_emac_device;
 extern struct platform_device da8xx_wdt_device;
-- 
1.7.11.4

Re: [PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info

From: Sekhar Nori <hidden>
Date: 2012-11-28 08:49:44

Hi Vivien,

On 9/11/2012 5:59 AM, Vivien Didelot wrote:
Without this patch, da8xx_register_spi() registers the SPI board info,
the SPI controller, and sets its number of chipselect to the size of the
static spi_board_info array. This is bad because a SPI board info may
declare devices for different SPI buses, and because other code can also
call spi_register_board_info() (e.g. a daughter board might provide
additional SPI devices).

This patch removes the spi_register_board_info() call from
da8xx_register_spi(), renames this last one to da8xx_register_spi_bus()
to be more explicit, takes the number of chipselect as a function
parameter, and updates the impacted board-da8{3,5}0-evm.c, and
board-mityomapl138.c files accordingly. It also sets the SPI platform
data static, as it doesn't need to be exported.

Signed-off-by: Vivien Didelot <redacted>
Sorry about the late reply. It seems I missed seeing this completely
until you reminded Kevin about it.

The patch looks good to me except some minor issues. I made those
changes myself locally since I am the one guilty of getting back on this
so late. See below:
---
 arch/arm/mach-davinci/board-da830-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-da850-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-mityomapl138.c |  9 +++++++--
 arch/arm/mach-davinci/devices-da8xx.c      | 14 +++-----------
 arch/arm/mach-davinci/include/mach/da8xx.h |  3 +--
 5 files changed, 25 insertions(+), 19 deletions(-)
The patch doesn't apply to the latest kernel anymore. I resolved that.
quoted hunk
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 11c3db9..adb8d87 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -652,8 +652,13 @@ static __init void da830_evm_init(void)
 	if (ret)
 		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
 
-	ret = da8xx_register_spi(0, da830evm_spi_info,
-				 ARRAY_SIZE(da830evm_spi_info));
+	ret = spi_register_board_info(da830evm_spi_info,
+				      ARRAY_SIZE(da830evm_spi_info));
+	if (ret)
+		pr_warning("da830_evm_init: spi info registration failed: %d\n",
+			   ret);
Checkpatch asks us to use pr_warn() instead. Fixed that. Also, used
__func__ to print function name. Rob Tivy has patches fixing these all
through this file.

Thanks,
Sekhar

Re: [PATCH] ARM: davinci: da8xx_register_spi() should not register SPI board info

From: Vivien Didelot <hidden>
Date: 2012-11-28 17:10:17

Hi Sekhar,

On Wed, 2012-11-28 at 14:19 +0530, Sekhar Nori wrote:
Hi Vivien,

On 9/11/2012 5:59 AM, Vivien Didelot wrote:
quoted
Without this patch, da8xx_register_spi() registers the SPI board info,
the SPI controller, and sets its number of chipselect to the size of the
static spi_board_info array. This is bad because a SPI board info may
declare devices for different SPI buses, and because other code can also
call spi_register_board_info() (e.g. a daughter board might provide
additional SPI devices).

This patch removes the spi_register_board_info() call from
da8xx_register_spi(), renames this last one to da8xx_register_spi_bus()
to be more explicit, takes the number of chipselect as a function
parameter, and updates the impacted board-da8{3,5}0-evm.c, and
board-mityomapl138.c files accordingly. It also sets the SPI platform
data static, as it doesn't need to be exported.

Signed-off-by: Vivien Didelot <redacted>
Sorry about the late reply. It seems I missed seeing this completely
until you reminded Kevin about it.

The patch looks good to me except some minor issues. I made those
changes myself locally since I am the one guilty of getting back on this
so late. See below:
quoted
---
 arch/arm/mach-davinci/board-da830-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-da850-evm.c    |  9 +++++++--
 arch/arm/mach-davinci/board-mityomapl138.c |  9 +++++++--
 arch/arm/mach-davinci/devices-da8xx.c      | 14 +++-----------
 arch/arm/mach-davinci/include/mach/da8xx.h |  3 +--
 5 files changed, 25 insertions(+), 19 deletions(-)
The patch doesn't apply to the latest kernel anymore. I resolved that.
quoted
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 11c3db9..adb8d87 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -652,8 +652,13 @@ static __init void da830_evm_init(void)
 	if (ret)
 		pr_warning("da830_evm_init: rtc setup failed: %d\n", ret);
 
-	ret = da8xx_register_spi(0, da830evm_spi_info,
-				 ARRAY_SIZE(da830evm_spi_info));
+	ret = spi_register_board_info(da830evm_spi_info,
+				      ARRAY_SIZE(da830evm_spi_info));
+	if (ret)
+		pr_warning("da830_evm_init: spi info registration failed: %d\n",
+			   ret);
Checkpatch asks us to use pr_warn() instead. Fixed that. Also, used
__func__ to print function name. Rob Tivy has patches fixing these all
through this file.

Thanks,
Sekhar
Thank you for considering the patch and for the upstream update!

Cheers,
Vivien
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help