[PATCH RESEND 0/4] spi: sh-msiof: Add DT support

[PATCH RESEND 0/4] spi: sh-msiof: Add DT support

[Bastian Hecht]

Patch resend from 21st september, this time cover letter included. Added ARM-ML
and SH-ML as recipients too.

The first patch contains a minor cleanup that becomes possible due to older
changes in the SuperH code.

In the 2nd patch we add the SH-Mobile chips as possible architecture requirement
for the MSIOF driver.

Then we come to the core of the patchset: DeviceTree support for sh-msiof in
patch 3 and patch 4.

There are minimal changes to the first publication to the patchset like changing
a comment string, so I just resend it as v1 again as there are no comments to the
original patchset.

Bastian Hecht (4):
  spi: sh-msiof: Remove unneeded clock name
  spi: Add SH Mobile series as dependency to MSIOF controller
  spi: sh-msiof: Add device tree parsing to driver
  devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc

 Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++
 drivers/spi/Kconfig                                |    4 +-
 drivers/spi/spi-sh-msiof.c                         |   60 ++++++++++++++++++--
 3 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt

-- 
1.7.5.4

[PATCH 1/4] spi: sh-msiof: Remove unneeded clock name

[Bastian Hecht]

clk_get() no longer needs a character string for associating the right
clock as this is done via the device struct now.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/spi/spi-sh-msiof.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 1f466bc..96358d0 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -597,7 +597,6 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	struct resource	*r;
 	struct spi_master *master;
 	struct sh_msiof_spi_priv *p;
-	char clk_name[16];
 	int i;
 	int ret;
 
@@ -614,10 +613,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	p->info = pdev->dev.platform_data;
 	init_completion(&p->done);
 
-	snprintf(clk_name, sizeof(clk_name), "msiof%d", pdev->id);
-	p->clk = clk_get(&pdev->dev, clk_name);
+	p->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(p->clk)) {
-		dev_err(&pdev->dev, "cannot get clock \"%s\"\n", clk_name);
+		dev_err(&pdev->dev, "cannot get clock\n");
 		ret = PTR_ERR(p->clk);
 		goto err1;
 	}
-- 
1.7.5.4

Re: [PATCH 1/4] spi: sh-msiof: Remove unneeded clock name

[Grant Likely]

On Wed,  7 Nov 2012 12:40:04 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> clk_get() no longer needs a character string for associating the right
> clock as this is done via the device struct now.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-sh-msiof.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 1f466bc..96358d0 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -597,7 +597,6 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>  	struct resource	*r;
>  	struct spi_master *master;
>  	struct sh_msiof_spi_priv *p;
> -	char clk_name[16];
>  	int i;
>  	int ret;
>  
> @@ -614,10 +613,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
>  	p->info = pdev->dev.platform_data;
>  	init_completion(&p->done);
>  
> -	snprintf(clk_name, sizeof(clk_name), "msiof%d", pdev->id);
> -	p->clk = clk_get(&pdev->dev, clk_name);
> +	p->clk = clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(p->clk)) {
> -		dev_err(&pdev->dev, "cannot get clock \"%s\"\n", clk_name);
> +		dev_err(&pdev->dev, "cannot get clock\n");
>  		ret = PTR_ERR(p->clk);
>  		goto err1;
>  	}
> -- 
> 1.7.5.4
> 
> 
> ------------------------------------------------------------------------------
> LogMeIn Central: Instant, anywhere, Remote PC access and management.
> Stay in control, update software, and manage PCs from one command center
> Diagnose problems and improve visibility into emerging IT issues
> Automate, monitor and manage. Do more in less time with Central
> http://p.sf.net/sfu/logmein12331_d2d
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

[PATCH 2/4] spi: Add SH Mobile series as dependency to MSIOF controller

[Bastian Hecht]

The MSIOF hardware block is used in the SH Mobile series as well, so we
add it here.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/spi/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 1acae35..d0c0ae6 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -341,10 +341,10 @@ config SPI_SC18IS602
 
 config SPI_SH_MSIOF
 	tristate "SuperH MSIOF SPI controller"
-	depends on SUPERH && HAVE_CLK
+	depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
 	select SPI_BITBANG
 	help
-	  SPI driver for SuperH MSIOF blocks.
+	  SPI driver for SuperH and SH Mobile MSIOF blocks.
 
 config SPI_SH
 	tristate "SuperH SPI controller"
-- 
1.7.5.4

[PATCH 3/4] spi: sh-msiof: Add device tree parsing to driver

[Bastian Hecht]

This adds the capability to retrieve setup data from the device tree
node. The usage of platform data is still available.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 drivers/spi/spi-sh-msiof.c |   54 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 96358d0..65cd19c 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
@@ -592,6 +593,35 @@ static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
+{
+	struct sh_msiof_spi_info *info;
+	struct device_node *np = dev->of_node;
+	u32 chipselect = 0;
+
+	info = devm_kzalloc(dev, sizeof(struct sh_msiof_spi_info), GFP_KERNEL);
+	if (!info) {
+		dev_err(dev, "failed to allocate setup data\n");
+		return NULL;
+	}
+
+	/* Parse the MSIOF properties */
+	of_property_read_u32(np, "chip-select", &chipselect);
+	of_property_read_u32(np, "tx_fifo_size", &info->tx_fifo_override);
+	of_property_read_u32(np, "rx_fifo_size", &info->rx_fifo_override);
+
+	info->num_chipselect = chipselect;
+
+	return info;
+}
+#else
+static struct sh_msiof_spi_info *sh_msiof_spi_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int sh_msiof_spi_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -610,7 +640,17 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	p = spi_master_get_devdata(master);
 
 	platform_set_drvdata(pdev, p);
-	p->info = pdev->dev.platform_data;
+	if (pdev->dev.of_node)
+		p->info = sh_msiof_spi_parse_dt(&pdev->dev);
+	else
+		p->info = pdev->dev.platform_data;
+
+	if (!p->info) {
+		dev_err(&pdev->dev, "failed to obtain device info\n");
+		ret = -ENXIO;
+		goto err1;
+	}
+
 	init_completion(&p->done);
 
 	p->clk = clk_get(&pdev->dev, NULL);
@@ -715,6 +755,17 @@ static int sh_msiof_spi_runtime_nop(struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id sh_msiof_match[] = {
+	{ .compatible = "renesas,sh-msiof", },
+	{ .compatible = "renesas,sh-mobile-msiof", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sh_msiof_match);
+#else
+#define sh_msiof_match NULL
+#endif
+
 static struct dev_pm_ops sh_msiof_spi_dev_pm_ops = {
 	.runtime_suspend = sh_msiof_spi_runtime_nop,
 	.runtime_resume = sh_msiof_spi_runtime_nop,
@@ -727,6 +778,7 @@ static struct platform_driver sh_msiof_spi_drv = {
 		.name		= "spi_sh_msiof",
 		.owner		= THIS_MODULE,
 		.pm		= &sh_msiof_spi_dev_pm_ops,
+		.of_match_table = sh_msiof_match,
 	},
 };
 module_platform_driver(sh_msiof_spi_drv);
-- 
1.7.5.4

Re: [PATCH 3/4] spi: sh-msiof: Add device tree parsing to driver

[Grant Likely]

On Wed,  7 Nov 2012 12:40:06 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> This adds the capability to retrieve setup data from the device tree
> node. The usage of platform data is still available.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>

Aside from the binding issues I've pointed out on patch 4, this change
looks fine. I'll wait for an updated patch before committing.

g.

[PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc

[Bastian Hecht]

Add binding documentation for Renesas' MSIOF SPI controller.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
new file mode 100644
index 0000000..b62312e
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -0,0 +1,12 @@
+Renesas MSIOF spi controller
+
+Required properties:
+- compatible : 	"renesas,sh-msiof" for SuperH or
+		"renesas,sh-mobile-msiof" for SH Mobile series
+- reg : Offset and length of the register set for the device
+- interrupts : interrupt line used by MSIOF
+
+Optional properties:
+- chip_select  : Chip select, defaults to 0
+- tx_fifo_size : Overrides the default tx fifo size given in words
+- rx_fifo_size : Overrides the default rx fifo size given in words
-- 
1.7.5.4

Re: [PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc

[Grant Likely]

On Wed,  7 Nov 2012 12:40:07 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
> Add binding documentation for Renesas' MSIOF SPI controller.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> ---
>  Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
> new file mode 100644
> index 0000000..b62312e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
> @@ -0,0 +1,12 @@
> +Renesas MSIOF spi controller
> +
> +Required properties:
> +- compatible : 	"renesas,sh-msiof" for SuperH or
> +		"renesas,sh-mobile-msiof" for SH Mobile series
> +- reg : Offset and length of the register set for the device
> +- interrupts : interrupt line used by MSIOF
> +
> +Optional properties:
> +- chip_select  : Chip select, defaults to 0

This doesn't make a lot of sense to me. What is this property for? Is
there more than one CS? And if there is, shouldn't the SPI driver be
able to manipulate more than one?

> +- tx_fifo_size : Overrides the default tx fifo size given in words
> +- rx_fifo_size : Overrides the default rx fifo size given in words

Nit: property names should use '-' instead of '_' (by convention) and
custom properties should be prefixed with the manufacturer prefix to
avoid namespace collisions. ie. "renesas,tx-fifo-size.

g.

Re: [PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc

[Grant Likely]

On Thu, Dec 6, 2012 at 2:08 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed,  7 Nov 2012 12:40:07 +0100, Bastian Hecht <hechtb@gmail.com> wrote:
>> Add binding documentation for Renesas' MSIOF SPI controller.
>>
>> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/spi/sh-msiof.txt |   12 ++++++++++++
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/spi/sh-msiof.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> new file mode 100644
>> index 0000000..b62312e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> @@ -0,0 +1,12 @@
>> +Renesas MSIOF spi controller
>> +
>> +Required properties:
>> +- compatible :       "renesas,sh-msiof" for SuperH or
>> +             "renesas,sh-mobile-msiof" for SH Mobile series
>> +- reg : Offset and length of the register set for the device
>> +- interrupts : interrupt line used by MSIOF
>> +
>> +Optional properties:
>> +- chip_select  : Chip select, defaults to 0
>
> This doesn't make a lot of sense to me. What is this property for? Is
> there more than one CS? And if there is, shouldn't the SPI driver be
> able to manipulate more than one?

Oh, wait, from reading the code, is this the number of chip select
lines? If so, then there is a common property for this. This property
should be named "num-cs".

>
>> +- tx_fifo_size : Overrides the default tx fifo size given in words
>> +- rx_fifo_size : Overrides the default rx fifo size given in words
>
> Nit: property names should use '-' instead of '_' (by convention) and
> custom properties should be prefixed with the manufacturer prefix to
> avoid namespace collisions. ie. "renesas,tx-fifo-size.
>
> g.



--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 4/4] devicetree: Add Renesas SH Mobile MSIOF spi controller binding doc

[Bastian Hecht]

Hello Grant,

>>> +Renesas MSIOF spi controller
>>> +
>>> +Required properties:
>>> +- compatible :       "renesas,sh-msiof" for SuperH or
>>> +             "renesas,sh-mobile-msiof" for SH Mobile series
>>> +- reg : Offset and length of the register set for the device
>>> +- interrupts : interrupt line used by MSIOF
>>> +
>>> +Optional properties:
>>> +- chip_select  : Chip select, defaults to 0
>>
>> This doesn't make a lot of sense to me. What is this property for? Is
>> there more than one CS? And if there is, shouldn't the SPI driver be
>> able to manipulate more than one?
>
> Oh, wait, from reading the code, is this the number of chip select
> lines? If so, then there is a common property for this. This property
> should be named "num-cs".

Yes, this value is nowhere used in the driver but just forwarded to
the SPI subsystem as the number of chip selects.

>>
>>> +- tx_fifo_size : Overrides the default tx fifo size given in words
>>> +- rx_fifo_size : Overrides the default rx fifo size given in words
>>
>> Nit: property names should use '-' instead of '_' (by convention) and
>> custom properties should be prefixed with the manufacturer prefix to
>> avoid namespace collisions. ie. "renesas,tx-fifo-size.

Ok sure! Thanks for pointing it out.

I'll post v2 in a second.

Thanks,

 Bastian

>> g.
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.