linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave
@ 2012-01-30 15:27 Jean-Christophe PLAGNIOL-VILLARD
  2012-01-30 15:27 ` [PATCH 2/3] spi/atmel: add DT support Jean-Christophe PLAGNIOL-VILLARD
       [not found] ` <1327937271-23668-1-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
  0 siblings, 2 replies; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-30 15:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: spi-devel-general, devicetree-discuss, Jean-Christophe PLAGNIOL-VILLARD

This will allow to use gpio for chip select with no modification in the
driver binding

When use the ncs-gpio, the gpio number will be passed via the controller_data
and the number of chip select will automatically increased.

When a spi master have only gpio chip select and is probe via dt check the
number of chip select only when adding slave.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: devicetree-discuss@lists.ozlabs.org
Cc: spi-devel-general@lists.sourceforge.net
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |    9 ++++++-
 drivers/of/of_spi.c                               |   27 ++++++++++++++------
 drivers/spi/spi.c                                 |    2 +-
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index e782add..1dccf35 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,9 +21,16 @@ assumption that board specific platform code will be used to manage
 chip selects.  Individual drivers can define additional properties to
 support describing the chip select layout.
 
+If a gpio a specified to the SPI slave and no hardware chip select is present
+the reg property #address-cells and #size-cells are not needed.
+
+When use the ncs-gpio the gpio number will be passed via the controller_data
+and the number of chip select will automatically increased.
+
 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             - (required if no ncs-gpio) chip select address of device.
+- ncs-gpio        - (required if no reg) chip select gpio of device.
 - compatible      - (required) name of SPI device following generic names
     		recommended practice
 - spi-max-frequency - (required) Maximum SPI clocking speed of device in Hz
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index 6dbc074..0d41407 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -12,6 +12,7 @@
 #include <linux/spi/spi.h>
 #include <linux/of_irq.h>
 #include <linux/of_spi.h>
+#include <linux/of_gpio.h>
 
 /**
  * of_register_spi_devices - Register child devices onto the SPI bus
@@ -27,6 +28,7 @@ void of_register_spi_devices(struct spi_master *master)
 	const __be32 *prop;
 	int rc;
 	int len;
+	int ncs_pin;
 
 	if (!master->dev.of_node)
 		return;
@@ -50,15 +52,24 @@ void of_register_spi_devices(struct spi_master *master)
 			continue;
 		}
 
-		/* 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;
+		/* ncs gpio */
+		ncs_pin = of_get_named_gpio(nc, "ncs-gpio", 0);
+
+		if (gpio_is_valid(ncs_pin)) {
+			spi->controller_data = (void *)ncs_pin;
+			spi->chip_select = master->num_chipselect;
+			master->num_chipselect++;
+		} else {
+			/* 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;
+			}
+			spi->chip_select = be32_to_cpup(prop);
 		}
-		spi->chip_select = be32_to_cpup(prop);
 
 		/* Mode (clock phase/polarity/etc.) */
 		if (of_find_property(nc, "spi-cpha", NULL))
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b2ccdea..ccd1a6f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -595,7 +595,7 @@ int spi_register_master(struct spi_master *master)
 	/* even if it's just one always-selected device, there must
 	 * be at least one chipselect
 	 */
-	if (master->num_chipselect == 0)
+	if (!master->dev.of_node && master->num_chipselect == 0)
 		return -EINVAL;
 
 	/* convention:  dynamically assigned bus IDs count down from the max */
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] spi/atmel: add DT support
  2012-01-30 15:27 [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave Jean-Christophe PLAGNIOL-VILLARD
@ 2012-01-30 15:27 ` Jean-Christophe PLAGNIOL-VILLARD
       [not found]   ` <1327937271-23668-2-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
       [not found] ` <1327937271-23668-1-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
  1 sibling, 1 reply; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-30 15:27 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: spi-devel-general, devicetree-discuss, Jean-Christophe PLAGNIOL-VILLARD

the atmel_spi use only gpio for chip select

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: devicetree-discuss@lists.ozlabs.org
Cc: spi-devel-general@lists.sourceforge.net
---
 .../devicetree/bindings/spi/spi_atmel.txt          |    6 ++++++
 drivers/spi/spi-atmel.c                            |   13 ++++++++++++-
 2 files changed, 18 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt

diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt
new file mode 100644
index 0000000..7ec3d8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt
@@ -0,0 +1,6 @@
+Atmel SPI device
+
+Required properties:
+- compatible : should be "atmel,at91rm9200-spi".
+- reg: Address and length of the register set for the device
+- interrupts: Should contain macb interrupt
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 16d6a83..beeaa26 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/spi/spi.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 #include <asm/io.h>
 #include <mach/board.h>
@@ -937,8 +938,9 @@ static int __devinit atmel_spi_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 
+	master->dev.of_node = pdev->dev.of_node;
 	master->bus_num = pdev->id;
-	master->num_chipselect = 4;
+	master->num_chipselect = master->dev.of_node ? 0 : 4;
 	master->setup = atmel_spi_setup;
 	master->transfer = atmel_spi_transfer;
 	master->cleanup = atmel_spi_cleanup;
@@ -1064,11 +1066,20 @@ static int atmel_spi_resume(struct platform_device *pdev)
 #define	atmel_spi_resume	NULL
 #endif
 
+#if defined(CONFIG_OF)
+static const struct of_device_id atmel_spi_dt_ids[] = {
+	{ .compatible = "atmel,at91rm9200-spi" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, atmel_spi_dt_ids);
+#endif
 
 static struct platform_driver atmel_spi_driver = {
 	.driver		= {
 		.name	= "atmel_spi",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(atmel_spi_dt_ids),
 	},
 	.suspend	= atmel_spi_suspend,
 	.resume		= atmel_spi_resume,
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave
       [not found] ` <1327937271-23668-1-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
@ 2012-01-30 18:54   ` Grant Likely
       [not found]     ` <20120130185458.GO28397-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Grant Likely @ 2012-01-30 18:54 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Jan 30, 2012 at 04:27:49PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> This will allow to use gpio for chip select with no modification in the
> driver binding
> 
> When use the ncs-gpio, the gpio number will be passed via the controller_data
> and the number of chip select will automatically increased.
> 
> When a spi master have only gpio chip select and is probe via dt check the
> number of chip select only when adding slave.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt |    9 ++++++-
>  drivers/of/of_spi.c                               |   27 ++++++++++++++------
>  drivers/spi/spi.c                                 |    2 +-
>  3 files changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index e782add..1dccf35 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -21,9 +21,16 @@ assumption that board specific platform code will be used to manage
>  chip selects.  Individual drivers can define additional properties to
>  support describing the chip select layout.
>  
> +If a gpio a specified to the SPI slave and no hardware chip select is present
> +the reg property #address-cells and #size-cells are not needed.
> +
> +When use the ncs-gpio the gpio number will be passed via the controller_data
> +and the number of chip select will automatically increased.
> +
>  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             - (required if no ncs-gpio) chip select address of device.
> +- ncs-gpio        - (required if no reg) chip select gpio of device.

There is already precedence for using gpios for chip selects.  The slave device
nodes remain as they are and it is the responsibility of the spi bus node
to have a gpios property with a list of SS gpios.  The order of the gpios
property should match the 'reg' address numbering of the child node.

I don't like the idea of having multiple bindings for the slave address of
the spi device (reg or ncs-gpio)

What I want to see is generic SS helper code that spi bus drivers
default to and correctly implement parsing of chip select gpios.

I'm not going to apply this patch.

g.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] spi/atmel: add DT support
       [not found]   ` <1327937271-23668-2-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
@ 2012-01-30 18:56     ` Grant Likely
  0 siblings, 0 replies; 5+ messages in thread
From: Grant Likely @ 2012-01-30 18:56 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Jan 30, 2012 at 04:27:50PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> the atmel_spi use only gpio for chip select
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> ---
>  .../devicetree/bindings/spi/spi_atmel.txt          |    6 ++++++
>  drivers/spi/spi-atmel.c                            |   13 ++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt
> new file mode 100644
> index 0000000..7ec3d8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt
> @@ -0,0 +1,6 @@
> +Atmel SPI device
> +
> +Required properties:
> +- compatible : should be "atmel,at91rm9200-spi".
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain macb interrupt
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 16d6a83..beeaa26 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/spi/spi.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>  
>  #include <asm/io.h>
>  #include <mach/board.h>
> @@ -937,8 +938,9 @@ static int __devinit atmel_spi_probe(struct platform_device *pdev)
>  	/* the spi->mode bits understood by this driver: */
>  	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
>  
> +	master->dev.of_node = pdev->dev.of_node;
>  	master->bus_num = pdev->id;
> -	master->num_chipselect = 4;
> +	master->num_chipselect = master->dev.of_node ? 0 : 4;

...continuing on from my comments on patch 1; this code will need to use
of_gpio_count() to find out how many SS lines there are.  Otherwise this patch
looks okay to me.

>  	master->setup = atmel_spi_setup;
>  	master->transfer = atmel_spi_transfer;
>  	master->cleanup = atmel_spi_cleanup;
> @@ -1064,11 +1066,20 @@ static int atmel_spi_resume(struct platform_device *pdev)
>  #define	atmel_spi_resume	NULL
>  #endif
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id atmel_spi_dt_ids[] = {
> +	{ .compatible = "atmel,at91rm9200-spi" },
> +	{ /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, atmel_spi_dt_ids);
> +#endif
>  
>  static struct platform_driver atmel_spi_driver = {
>  	.driver		= {
>  		.name	= "atmel_spi",
>  		.owner	= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(atmel_spi_dt_ids),
>  	},
>  	.suspend	= atmel_spi_suspend,
>  	.resume		= atmel_spi_resume,
> -- 
> 1.7.7
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave
       [not found]     ` <20120130185458.GO28397-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
@ 2012-01-31 11:07       ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-01-31 11:07 UTC (permalink / raw)
  To: Grant Likely
  Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 11:54 Mon 30 Jan     , Grant Likely wrote:
> On Mon, Jan 30, 2012 at 04:27:49PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > This will allow to use gpio for chip select with no modification in the
> > driver binding
> > 
> > When use the ncs-gpio, the gpio number will be passed via the controller_data
> > and the number of chip select will automatically increased.
> > 
> > When a spi master have only gpio chip select and is probe via dt check the
> > number of chip select only when adding slave.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > ---
> >  Documentation/devicetree/bindings/spi/spi-bus.txt |    9 ++++++-
> >  drivers/of/of_spi.c                               |   27 ++++++++++++++------
> >  drivers/spi/spi.c                                 |    2 +-
> >  3 files changed, 28 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> > index e782add..1dccf35 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> > +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> > @@ -21,9 +21,16 @@ assumption that board specific platform code will be used to manage
> >  chip selects.  Individual drivers can define additional properties to
> >  support describing the chip select layout.
> >  
> > +If a gpio a specified to the SPI slave and no hardware chip select is present
> > +the reg property #address-cells and #size-cells are not needed.
> > +
> > +When use the ncs-gpio the gpio number will be passed via the controller_data
> > +and the number of chip select will automatically increased.
> > +
> >  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             - (required if no ncs-gpio) chip select address of device.
> > +- ncs-gpio        - (required if no reg) chip select gpio of device.
> 
> There is already precedence for using gpios for chip selects.  The slave device
> nodes remain as they are and it is the responsibility of the spi bus node
> to have a gpios property with a list of SS gpios.  The order of the gpios
> property should match the 'reg' address numbering of the child node.
> 
> I don't like the idea of having multiple bindings for the slave address of
> the spi device (reg or ncs-gpio)
> 
> What I want to see is generic SS helper code that spi bus drivers
> default to and correctly implement parsing of chip select gpios.
> 
> I'm not going to apply this patch.
I do not want to see any of managment in the driver, I want a generic way to
specify only gpio cs and hw and gpio cs

spi1: spi@fffcc000 {
	ncs-gpios = <&pioB 3 0>;
	status = "okay";
	mmc-slot@0 {
		reg = <0>;
		compatible = "mmc-spi-slot";
		gpios = <&pioC 4 0   /* CD */
			>;
		voltage-ranges = <3200 3400>;
		spi-max-frequency = <20000000>;
	};
};

as on atmel spi we have no HW spi the cs is 0
if there is 3 hw cs the reg will be 3

we have 4 hw chipselect 3 enabled and one gpio chipselect

spi1: spi@fffcc000 {
	ncs-hw = < 0 1 1 1>;
	ncs-gpios = <&pioB 3 0>;
	status = "okay";
	mmc-slot@0 {
		reg = <3>;
		compatible = "mmc-spi-slot";
		gpios = <&pioC 4 0   /* CD */
			>;
		voltage-ranges = <3200 3400>;
		spi-max-frequency = <20000000>;
	};
};

the idea is to pass the ncs_pin in the spi_device so the driver just have
to check if the chipselect is hw or gpio and then use it the right way

Best Regards,
J.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-01-31 11:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-30 15:27 [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave Jean-Christophe PLAGNIOL-VILLARD
2012-01-30 15:27 ` [PATCH 2/3] spi/atmel: add DT support Jean-Christophe PLAGNIOL-VILLARD
     [not found]   ` <1327937271-23668-2-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
2012-01-30 18:56     ` Grant Likely
     [not found] ` <1327937271-23668-1-git-send-email-plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
2012-01-30 18:54   ` [PATCH 1/3] of_spi: add generic binding support to specify ncs gpio in the slave Grant Likely
     [not found]     ` <20120130185458.GO28397-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2012-01-31 11:07       ` Jean-Christophe PLAGNIOL-VILLARD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).