[PATCH] Added gpios property for SPI slaves

[PATCH] Added gpios property for SPI slaves

[Wolfgang Ocker]

SPI slave devices require the specification of a chip select address.
This patch allows that specification using the gpios property. The reg
property remains supported.

Signed-off-by: Wolfgang Ocker <weo@reccoware.de>
---

--- linux-2.6.27.3/drivers/of/of_spi.c.of_spi_gpio	2008-10-22 23:38:01.000000000 +0200
+++ linux-2.6.27.3/drivers/of/of_spi.c.of_spi_cshigh	2008-10-24 21:36:39.000000000 +0200
@@ -10,6 +10,7 @@
 #include <linux/device.h>
 #include <linux/spi/spi.h>
 #include <linux/of_spi.h>
+#include <linux/of_gpio.h>
 
 /**
  * of_register_spi_devices - Register child devices onto the SPI bus
@@ -24,6 +25,7 @@ void of_register_spi_devices(struct spi_
 	struct spi_device *spi;
 	struct device_node *nc;
 	const u32 *prop;
+	int cs_addr;
 	int rc;
 	int len;
 
@@ -48,13 +50,20 @@ void of_register_spi_devices(struct spi_
 
 		/* Device address */
 		prop = of_get_property(nc, "reg", &len);
-		if (!prop || len < sizeof(*prop)) {
-			dev_err(&master->dev, "%s has no 'reg' property\n",
-				nc->full_name);
-			spi_dev_put(spi);
-			continue;
+		if (prop && len >= sizeof(*prop)) {
+			cs_addr = *prop;
+		}
+		else {
+			cs_addr = of_get_gpio(nc, 0);
+			if (cs_addr < 0) {
+				dev_err(&master->dev, 
+					"%s has no 'reg' nor gpios property\n",
+					nc->full_name);
+				spi_dev_put(spi);
+				continue;
+			}
 		}
-		spi->chip_select = *prop;
+		spi->chip_select = cs_addr;
 
 		/* Mode (clock phase/polarity/etc.) */
 		if (of_find_property(nc, "spi-cpha", NULL))
--- linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_gpio	2008-10-22 23:38:01.000000000 +0200
+++ linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_cshigh	2008-10-24 21:57:16.000000000 +0200
@@ -1909,7 +1909,9 @@ platforms are moved over to use the flat
 
     SPI slave nodes must be children of the SPI master node and can
     contain the following properties.
-    - reg             - (required) chip select address of device.
+    - reg             - chip select address of device.
+    - gpios           - chip select address of device (alternatively).
+			one of reg and gpios is required.
     - compatible      - (required) name of SPI device following generic names
 			recommended practice
     - spi-max-frequency - (required) Maximum SPI clocking speed of device in Hz
@@ -1936,7 +1938,7 @@ platforms are moved over to use the flat
 			codec@1 {
 				compatible = "ti,tlv320aic26";
 				spi-max-frequency = <100000>;
-				reg = <1>;
+				gpios = <&GPIO1 3>;
 			};
 		};
 

Re: [PATCH] Added gpios property for SPI slaves

[Anton Vorontsov]

On Fri, Oct 24, 2008 at 10:08:59PM +0200, Wolfgang Ocker wrote:
> SPI slave devices require the specification of a chip select address.
> This patch allows that specification using the gpios property. The reg
> property remains supported.
> 
> Signed-off-by: Wolfgang Ocker <weo@reccoware.de>
> ---
> 
> --- linux-2.6.27.3/drivers/of/of_spi.c.of_spi_gpio	2008-10-22 23:38:01.000000000 +0200
> +++ linux-2.6.27.3/drivers/of/of_spi.c.of_spi_cshigh	2008-10-24 21:36:39.000000000 +0200
[...]
> --- linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_gpio	2008-10-22 23:38:01.000000000 +0200
> +++ linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_cshigh	2008-10-24 21:57:16.000000000 +0200
> @@ -1909,7 +1909,9 @@ platforms are moved over to use the flat
>  
>      SPI slave nodes must be children of the SPI master node and can
>      contain the following properties.
> -    - reg             - (required) chip select address of device.
> +    - reg             - chip select address of device.
> +    - gpios           - chip select address of device (alternatively).
> +			one of reg and gpios is required.
>      - compatible      - (required) name of SPI device following generic names
>  			recommended practice
>      - spi-max-frequency - (required) Maximum SPI clocking speed of device in Hz
> @@ -1936,7 +1938,7 @@ platforms are moved over to use the flat
>  			codec@1 {
>  				compatible = "ti,tlv320aic26";
>  				spi-max-frequency = <100000>;
> -				reg = <1>;
> +				gpios = <&GPIO1 3>;
>  			};
>  		};

Close, but no cigar. Sorry. ;-) The bindings are fine as is, you don't
need to change them.

The scheme should look like this:

spi-controller {
        #address-cells = <1>;
        #size-cells = <0>;

        /* two GPIOs, representing two chip selects: 0 and 1 */
        gpios = <&pio 5 0 &pio 16 0>;

        mmc-slot@0 {
                reg = <0>;
        };

        touchscreen@1 {
                reg = <1>;
        };
}

Notice that "gpios" is spi-controller's property, not spi devices'.
It is truly as hardware works, SPI controllers consists of two units:
I/O, and chip-select machine. Most spi controllers don't have
dedicated chip-select machines, so they use GPIOs.

The spi controller driver should request all the specified gpios,
and then work with chip select numbers. Something like this:

struct spi_controller {
	int *gpios;
	unsigned int num_gpios;
	...
}

int spi_set_chipselect(struct spi_controller *spi, int cs, int active)
{
	/* 
	 * chip-select is not necessary if there is just one device on
	 * the bus, so gpios = <> are not necessary either */
	 */
	if (!spi->num_gpios)
		return 0;

	if (cs > spi->num_gpios)
		return -EINVAL;

	gpio_set_value(spi->gpios[cs], active);
}

...

unsigned int of_num_gpios(struct device_node *node)
{
	unsigned int num = 0;

	while (gpio_is_valid(of_get_gpio(node, num)))
		num++;
	return num;
}

int spi_controller_probe(...)
{
	spi_controller *spi;
...
	spi->num_gpios = of_num_gpios(node);
	if (spi->num_gpios) {
		int i = spi->num_gpios;

		spi->gpios = kzalloc(sizeof(int) * i, GFP_KERNEL);
		if (!spi->gpios)
			return -ENOMEM;

		do
			spi->gpios[i] = of_get_gpio(node, i);
		while (!(i--));
	}
...
}

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH] Added gpios property for SPI slaves

[Grant Likely]

On Fri, Oct 24, 2008 at 3:10 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Fri, Oct 24, 2008 at 10:08:59PM +0200, Wolfgang Ocker wrote:
>> SPI slave devices require the specification of a chip select address.
>> This patch allows that specification using the gpios property. The reg
>> property remains supported.
>>
>> Signed-off-by: Wolfgang Ocker <weo@reccoware.de>
>> ---
>>
>> --- linux-2.6.27.3/drivers/of/of_spi.c.of_spi_gpio    2008-10-22 23:38:01.000000000 +0200
>> +++ linux-2.6.27.3/drivers/of/of_spi.c.of_spi_cshigh  2008-10-24 21:36:39.000000000 +0200
> [...]
>> --- linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_gpio   2008-10-22 23:38:01.000000000 +0200
>> +++ linux-2.6.27.3/Documentation/powerpc/booting-without-of.txt.of_spi_cshigh 2008-10-24 21:57:16.000000000 +0200
>> @@ -1936,7 +1938,7 @@ platforms are moved over to use the flat
>>                       codec@1 {
>>                               compatible = "ti,tlv320aic26";
>>                               spi-max-frequency = <100000>;
>> -                             reg = <1>;
>> +                             gpios = <&GPIO1 3>;
>>                       };
>>               };
>
> Close, but no cigar. Sorry. ;-) The bindings are fine as is, you don't
> need to change them.
>
> The scheme should look like this:
>
> spi-controller {
>        #address-cells = <1>;
>        #size-cells = <0>;
>
>        /* two GPIOs, representing two chip selects: 0 and 1 */
>        gpios = <&pio 5 0 &pio 16 0>;
>
>        mmc-slot@0 {
>                reg = <0>;
>        };
>
>        touchscreen@1 {
>                reg = <1>;
>        };
> }
>
> Notice that "gpios" is spi-controller's property, not spi devices'.
> It is truly as hardware works, SPI controllers consists of two units:
> I/O, and chip-select machine. Most spi controllers don't have
> dedicated chip-select machines, so they use GPIOs.

FWIW, I 100% agree.

g.

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

Re: [PATCH] Added gpios property for SPI slaves

[Wolfgang Ocker]

On Sat, 2008-10-25 at 01:10 +0400, Anton Vorontsov wrote: 
> On Fri, Oct 24, 2008 at 10:08:59PM +0200, Wolfgang Ocker wrote:
> > SPI slave devices require the specification of a chip select address.
> > This patch allows that specification using the gpios property. The reg
> > property remains supported.
> 
> Close, but no cigar. Sorry. ;-) The bindings are fine as is, you don't
> need to change them.

Oh, what a steep learning curve! Anyway, thank you for your guidance and
patience!

Looks like I have to adjust the SPI driver.

Wolfgang