devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/2] Add dt_compat field to struct gpio_chip
       [not found] <20110407163322.465935232@gmail.com>
@ 2011-04-07 16:39 ` Domenico Andreoli
       [not found]   ` <20110407163930.GB17498-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
  2011-04-07 16:39 ` [patch 2/2] Add GPIO DT support to s3c24xx Domenico Andreoli
  1 sibling, 1 reply; 8+ messages in thread
From: Domenico Andreoli @ 2011-04-07 16:39 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: gpiolib-add-dt_compat-to-gpio_chip --]
[-- Type: text/plain, Size: 1268 bytes --]

From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This new field allows easy creation of GPIO chips in base of struct arrays.

Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
 drivers/of/gpio.c          |    3 +++
 include/asm-generic/gpio.h |    1 +
 2 files changed, 4 insertions(+)

Index: b/drivers/of/gpio.c
===================================================================
--- a/drivers/of/gpio.c	2011-04-07 18:19:20.000000000 +0200
+++ b/drivers/of/gpio.c	2011-04-07 18:20:31.000000000 +0200
@@ -212,6 +212,9 @@
 
 void of_gpiochip_add(struct gpio_chip *chip)
 {
+	if ((!chip->of_node) && (chip->dt_compat))
+		chip->of_node = of_find_compatible_node(NULL, NULL, chip->dt_compat);
+
 	if ((!chip->of_node) && (chip->dev))
 		chip->of_node = chip->dev->of_node;
 
Index: b/include/asm-generic/gpio.h
===================================================================
--- a/include/asm-generic/gpio.h	2011-04-07 18:19:20.000000000 +0200
+++ b/include/asm-generic/gpio.h	2011-04-07 18:19:30.000000000 +0200
@@ -129,6 +129,7 @@
 	int of_gpio_n_cells;
 	int (*of_xlate)(struct gpio_chip *gc, struct device_node *np,
 		        const void *gpio_spec, u32 *flags);
+	const char *dt_compat;
 #endif
 };

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

* [patch 2/2] Add GPIO DT support to s3c24xx
       [not found] <20110407163322.465935232@gmail.com>
  2011-04-07 16:39 ` [patch 1/2] Add dt_compat field to struct gpio_chip Domenico Andreoli
@ 2011-04-07 16:39 ` Domenico Andreoli
  1 sibling, 0 replies; 8+ messages in thread
From: Domenico Andreoli @ 2011-04-07 16:39 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: s3c24xx-gpio-add-dt-support --]
[-- Type: text/plain, Size: 3140 bytes --]

From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add DT compat strings to the GPIO chips registerd by s3c24xx SOCs.

Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
 arch/arm/plat-s3c24xx/gpiolib.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Index: b/arch/arm/plat-s3c24xx/gpiolib.c
===================================================================
--- a/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-07 18:16:09.000000000 +0200
+++ b/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-07 18:20:41.000000000 +0200
@@ -94,6 +94,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOA",
 			.ngpio			= 24,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-a",
+#endif
 			.direction_input	= s3c24xx_gpiolib_banka_input,
 			.direction_output	= s3c24xx_gpiolib_banka_output,
 		},
@@ -106,6 +109,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOB",
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-b",
+#endif
 		},
 	},
 	[2] = {
@@ -116,6 +122,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOC",
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-c",
+#endif
 		},
 	},
 	[3] = {
@@ -126,6 +135,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOD",
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-d",
+#endif
 		},
 	},
 	[4] = {
@@ -136,6 +148,9 @@
 			.label			= "GPIOE",
 			.owner			= THIS_MODULE,
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-e",
+#endif
 		},
 	},
 	[5] = {
@@ -147,6 +162,9 @@
 			.label			= "GPIOF",
 			.ngpio			= 8,
 			.to_irq			= s3c24xx_gpiolib_bankf_toirq,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-f",
+#endif
 		},
 	},
 	[6] = {
@@ -159,6 +177,9 @@
 			.label			= "GPIOG",
 			.ngpio			= 16,
 			.to_irq			= samsung_gpiolib_to_irq,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-g",
+#endif
 		},
 	}, {
 		.base	= S3C2410_GPHCON,
@@ -168,6 +189,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOH",
 			.ngpio			= 11,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2410-gpio-h",
+#endif
 		},
 	},
 		/* GPIOS for the S3C2443 and later devices. */
@@ -179,6 +203,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOJ",
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2440-gpio-j",
+#endif
 		},
 	}, {
 		.base	= S3C2443_GPKCON,
@@ -188,6 +215,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOK",
 			.ngpio			= 16,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2443-gpio-k",
+#endif
 		},
 	}, {
 		.base	= S3C2443_GPLCON,
@@ -197,6 +227,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOL",
 			.ngpio			= 15,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2443-gpio-l",
+#endif
 		},
 	}, {
 		.base	= S3C2443_GPMCON,
@@ -206,6 +239,9 @@
 			.owner			= THIS_MODULE,
 			.label			= "GPIOM",
 			.ngpio			= 2,
+#if defined(CONFIG_OF_GPIO)
+			.dt_compat		= "samsung,s3c2443-gpio-m",
+#endif
 		},
 	},
 };

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

* Re: [patch 1/2] Add dt_compat field to struct gpio_chip
       [not found]   ` <20110407163930.GB17498-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
@ 2011-04-07 17:17     ` Grant Likely
       [not found]       ` <20110407171704.GC2455-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2011-04-07 17:17 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Domenico Andreoli

On Thu, Apr 07, 2011 at 06:39:30PM +0200, Domenico Andreoli wrote:
> From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This new field allows easy creation of GPIO chips in base of struct arrays.
> 
> Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> ---
>  drivers/of/gpio.c          |    3 +++
>  include/asm-generic/gpio.h |    1 +
>  2 files changed, 4 insertions(+)
> 
> Index: b/drivers/of/gpio.c
> ===================================================================
> --- a/drivers/of/gpio.c	2011-04-07 18:19:20.000000000 +0200
> +++ b/drivers/of/gpio.c	2011-04-07 18:20:31.000000000 +0200
> @@ -212,6 +212,9 @@
>  
>  void of_gpiochip_add(struct gpio_chip *chip)
>  {
> +	if ((!chip->of_node) && (chip->dt_compat))
> +		chip->of_node = of_find_compatible_node(NULL, NULL, chip->dt_compat);
> +

Hi Domenico,

Thanks for looking at this.

However, I think there is a better way to solve this problem,

>From what I can tell, this patch attempts to adapt to the way that
arch/arm/plat-samsung/gpio.c registers gpios with the kernel.  Each
platform seems to have its own static table of gpio banks which are
registered without any regard to the Linux device model.  It works,
but it isn't really the way things should be done.

The design of gpiolib right now is such that the of_node pointer
should be known *before* of_gpiochip_add() gets called.  This is very
important because the code that registers the gpio is the only place
that can truly know which node is actually associated with the gpio.

To solve your problem, the best solution would be to rework
arch/arm/plat-samsung-gpio.c to properly use the driver model and
register platform_devices which can be attached to dt nodes.  This
change should probably be done anyway, even ignoring the dt needs, but
I'm not going to force you to make this change to get dt support added
for samsung gpios.

Alternately, what you should do is make sure that the chip->of_node
pointer is correctly populated before calling gpiochip_add(), possibly
in s3c_gpiolib_add().

Also, be careful about the way that 'compatible' is being used.
Remember that compatible describes the /interface/ to a device, but
not the /instance/.  Many systems have multiple instances of the same
device, and compatible doesn't provide any help for differentiating
between them.  Typically, when trying to find a specific instance of a
device, it should be resolved with a property in the /aliases node, or
it should be matched up against the resolved base address of the
device.

Cheers,
g.

>  	if ((!chip->of_node) && (chip->dev))
>  		chip->of_node = chip->dev->of_node;
>  
> Index: b/include/asm-generic/gpio.h
> ===================================================================
> --- a/include/asm-generic/gpio.h	2011-04-07 18:19:20.000000000 +0200
> +++ b/include/asm-generic/gpio.h	2011-04-07 18:19:30.000000000 +0200
> @@ -129,6 +129,7 @@
>  	int of_gpio_n_cells;
>  	int (*of_xlate)(struct gpio_chip *gc, struct device_node *np,
>  		        const void *gpio_spec, u32 *flags);
> +	const char *dt_compat;
>  #endif
>  };
>  
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

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

* Re: [patch 1/2] Add dt_compat field to struct gpio_chip
       [not found]       ` <20110407171704.GC2455-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
@ 2011-04-07 20:41         ` Domenico Andreoli
       [not found]           ` <BANLkTi=WE_vOUAi8T60ESAH2kua4uB-_HA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Domenico Andreoli @ 2011-04-07 20:41 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Apr 7, 2011 at 7:17 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>
> On Thu, Apr 07, 2011 at 06:39:30PM +0200, Domenico Andreoli wrote:
> > From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >
> > This new field allows easy creation of GPIO chips in base of struct arrays.
> >
> > Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >
> > ---
> >  drivers/of/gpio.c          |    3 +++
> >  include/asm-generic/gpio.h |    1 +
> >  2 files changed, 4 insertions(+)
> >
> > Index: b/drivers/of/gpio.c
> > ===================================================================
> > --- a/drivers/of/gpio.c       2011-04-07 18:19:20.000000000 +0200
> > +++ b/drivers/of/gpio.c       2011-04-07 18:20:31.000000000 +0200
> > @@ -212,6 +212,9 @@
> >
> >  void of_gpiochip_add(struct gpio_chip *chip)
> >  {
> > +     if ((!chip->of_node) && (chip->dt_compat))
> > +             chip->of_node = of_find_compatible_node(NULL, NULL, chip->dt_compat);
> > +
>
> Hi Domenico,

Hi Grant,

> From what I can tell, this patch attempts to adapt to the way that
> arch/arm/plat-samsung/gpio.c registers gpios with the kernel.  Each
> platform seems to have its own static table of gpio banks which are
> registered without any regard to the Linux device model.  It works,
> but it isn't really the way things should be done.

About the independent models of the ARM platforms, I can't say
anything. I only wanted to explore the DT concepts and import them on
the s3c24xx. While this could look only as the latest individual
attempt, the DT interface is not and cannot be of any harm if ARM
deploys it a little more.

> The design of gpiolib right now is such that the of_node pointer
> should be known *before* of_gpiochip_add() gets called.  This is very
> important because the code that registers the gpio is the only place
> that can truly know which node is actually associated with the gpio.

This explains why a so simple patch has never been proposed before.

> To solve your problem, the best solution would be to rework
> arch/arm/plat-samsung-gpio.c to properly use the driver model and
> register platform_devices which can be attached to dt nodes.  This
> change should probably be done anyway, even ignoring the dt needs, but
> I'm not going to force you to make this change to get dt support added
> for samsung gpios.

Indeed my plan is to not rework it, not in the next future, not before
I switched all its users to DT ;)

I'd like to follow the path towards a pure dts platform definition so
the next step would be to DT-fy something like the s3c
sdi/sdhci/whatever, which already uses some GPIO. Then something else
like audio or network and so on. I would be very glad to arrive sooner
or later to a s3c24xx FDT.

> Alternately, what you should do is make sure that the chip->of_node
> pointer is correctly populated before calling gpiochip_add(), possibly
> in s3c_gpiolib_add().

I already restored it to the previous state, with .dt_compat in struct
s3c_gpio_chip and the the call of of_find_compatible_node() in
s3c_gpiolib_add(), this way it stays out of gpiolib.

> Also, be careful about the way that 'compatible' is being used.
> Remember that compatible describes the /interface/ to a device, but
> not the /instance/.  Many systems have multiple instances of the same
> device, and compatible doesn't provide any help for differentiating
> between them.

I needed some clarification here, I already suspected that my way to
encode banks in the compatible property was somehow fishy. And I
needed a working patch to get some attention before digging further..
;)

> Typically, when trying to find a specific instance of a
> device, it should be resolved with a property in the /aliases node, or
> it should be matched up against the resolved base address of the
> device.

This is what I'll try next. This alone seems to imply the rework you
said above, let's see.

> Cheers,
> g.

Thank you.

cheers,
Domenico

-----[ Domenico Andreoli, aka cavok
 --[ http://www.dandreoli.com/gpgkey.asc
   ---[ 3A0F 2F80 F79C 678A 8936  4FEE 0677 9033 A20E BC50

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

* Re: [patch 1/2] Add dt_compat field to struct gpio_chip
       [not found]           ` <BANLkTi=WE_vOUAi8T60ESAH2kua4uB-_HA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-04-08  0:17             ` Grant Likely
  0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2011-04-08  0:17 UTC (permalink / raw)
  To: Domenico Andreoli
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Apr 7, 2011 at 1:41 PM, Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Apr 7, 2011 at 7:17 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>>
>> On Thu, Apr 07, 2011 at 06:39:30PM +0200, Domenico Andreoli wrote:
>> > From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >
>> > This new field allows easy creation of GPIO chips in base of struct arrays.
>> >
>> > Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >
>> > ---
>> >  drivers/of/gpio.c          |    3 +++
>> >  include/asm-generic/gpio.h |    1 +
>> >  2 files changed, 4 insertions(+)
>> >
>> > Index: b/drivers/of/gpio.c
>> > ===================================================================
>> > --- a/drivers/of/gpio.c       2011-04-07 18:19:20.000000000 +0200
>> > +++ b/drivers/of/gpio.c       2011-04-07 18:20:31.000000000 +0200
>> > @@ -212,6 +212,9 @@
>> >
>> >  void of_gpiochip_add(struct gpio_chip *chip)
>> >  {
>> > +     if ((!chip->of_node) && (chip->dt_compat))
>> > +             chip->of_node = of_find_compatible_node(NULL, NULL, chip->dt_compat);
>> > +
>>
>> Hi Domenico,
>
> Hi Grant,
>
>> From what I can tell, this patch attempts to adapt to the way that
>> arch/arm/plat-samsung/gpio.c registers gpios with the kernel.  Each
>> platform seems to have its own static table of gpio banks which are
>> registered without any regard to the Linux device model.  It works,
>> but it isn't really the way things should be done.
>
> About the independent models of the ARM platforms, I can't say
> anything. I only wanted to explore the DT concepts and import them on
> the s3c24xx. While this could look only as the latest individual
> attempt, the DT interface is not and cannot be of any harm if ARM
> deploys it a little more.
>
>> The design of gpiolib right now is such that the of_node pointer
>> should be known *before* of_gpiochip_add() gets called.  This is very
>> important because the code that registers the gpio is the only place
>> that can truly know which node is actually associated with the gpio.
>
> This explains why a so simple patch has never been proposed before.
>
>> To solve your problem, the best solution would be to rework
>> arch/arm/plat-samsung-gpio.c to properly use the driver model and
>> register platform_devices which can be attached to dt nodes.  This
>> change should probably be done anyway, even ignoring the dt needs, but
>> I'm not going to force you to make this change to get dt support added
>> for samsung gpios.
>
> Indeed my plan is to not rework it, not in the next future, not before
> I switched all its users to DT ;)
>
> I'd like to follow the path towards a pure dts platform definition so
> the next step would be to DT-fy something like the s3c
> sdi/sdhci/whatever, which already uses some GPIO. Then something else
> like audio or network and so on. I would be very glad to arrive sooner
> or later to a s3c24xx FDT.
>
>> Alternately, what you should do is make sure that the chip->of_node
>> pointer is correctly populated before calling gpiochip_add(), possibly
>> in s3c_gpiolib_add().
>
> I already restored it to the previous state, with .dt_compat in struct
> s3c_gpio_chip and the the call of of_find_compatible_node() in
> s3c_gpiolib_add(), this way it stays out of gpiolib.
>
>> Also, be careful about the way that 'compatible' is being used.
>> Remember that compatible describes the /interface/ to a device, but
>> not the /instance/.  Many systems have multiple instances of the same
>> device, and compatible doesn't provide any help for differentiating
>> between them.
>
> I needed some clarification here, I already suspected that my way to
> encode banks in the compatible property was somehow fishy. And I
> needed a working patch to get some attention before digging further..
> ;)
>
>> Typically, when trying to find a specific instance of a
>> device, it should be resolved with a property in the /aliases node, or
>> it should be matched up against the resolved base address of the
>> device.
>
> This is what I'll try next. This alone seems to imply the rework you
> said above, let's see.

It doesn't actually depend on the rework.  You already have the
register information in s3c gpio.  You could use the same compatible
value for all, and then add another comparison against the base
address to figure out exactly which one is the correct one.

g.

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

* [patch 2/2] Add GPIO DT support to s3c24xx
       [not found] <20110410152735.338798127@gmail.com>
@ 2011-04-10 15:33 ` Domenico Andreoli
       [not found]   ` <20110410153316.GC27088-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Domenico Andreoli @ 2011-04-10 15:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: s3c24xx-gpio-add-dt-support --]
[-- Type: text/plain, Size: 2090 bytes --]

From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Assign proper OF node (= with matching physical base address) to each
s3c24xx GPIO chip.

Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
 arch/arm/plat-s3c24xx/gpiolib.c |   45 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Index: b/arch/arm/plat-s3c24xx/gpiolib.c
===================================================================
--- a/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-10 16:24:59.000000000 +0200
+++ b/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-10 17:12:32.000000000 +0200
@@ -19,6 +19,8 @@
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <plat/gpio-core.h>
 #include <plat/gpio-cfg.h>
@@ -97,6 +99,9 @@
 			.direction_input	= s3c24xx_gpiolib_banka_input,
 			.direction_output	= s3c24xx_gpiolib_banka_output,
 		},
+#ifdef CONFIG_OF_GPIO
+		.dt_compat = "samsung,s3c2410-gpio-a",
+#endif
 	},
 	[1] = {
 		.base	= S3C2410_GPBCON,
@@ -210,6 +215,45 @@
 	},
 };
 
+#ifdef CONFIG_OF_GPIO
+static int s3c24xx_of_base_match(struct s3c_gpio_chip *chip, struct device_node *dn)
+{
+	const u32 *addrp;
+	u64 addr;
+
+	addrp = of_get_address(dn, 0, 0, NULL);
+	if (!addrp)
+		return 0;
+
+	addr = of_translate_address(dn, addrp);
+	if (addr == OF_BAD_ADDR)
+		return 0;
+
+	return chip->base == (addr - S3C24XX_PA_GPIO + S3C24XX_VA_GPIO);
+}
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+	struct device_node *dn;
+
+	if (!chip->dt_compat)
+		chip->dt_compat = "samsung,s3c2410-gpio";
+
+	for_each_compatible_node(dn, NULL, chip->dt_compat) {
+		if (s3c24xx_of_base_match(chip, dn)) {
+			chip->chip.of_node = dn;
+			break;
+		}
+	}
+}
+
+#else
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+}
+
+#endif
 
 static __init int s3c24xx_gpiolib_init(void)
 {
@@ -220,6 +264,7 @@
 		if (!chip->config)
 			chip->config = &s3c24xx_gpiocfg_default;
 
+		s3c24xx_attach_of_node(chip);
 		s3c_gpiolib_add(chip);
 	}

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

* Re: [patch 2/2] Add GPIO DT support to s3c24xx
       [not found]   ` <20110410153316.GC27088-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
@ 2011-04-11  6:47     ` Grant Likely
  0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2011-04-11  6:47 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Domenico Andreoli

On Sun, Apr 10, 2011 at 05:33:16PM +0200, Domenico Andreoli wrote:
> From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Assign proper OF node (= with matching physical base address) to each
> s3c24xx GPIO chip.
> 
> Signed-off-by: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Looks pretty good.  Minor comments below.

g.

> 
> ---
>  arch/arm/plat-s3c24xx/gpiolib.c |   45 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> Index: b/arch/arm/plat-s3c24xx/gpiolib.c
> ===================================================================
> --- a/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-10 16:24:59.000000000 +0200
> +++ b/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-10 17:12:32.000000000 +0200
> @@ -19,6 +19,8 @@
>  #include <linux/ioport.h>
>  #include <linux/io.h>
>  #include <linux/gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>  
>  #include <plat/gpio-core.h>
>  #include <plat/gpio-cfg.h>
> @@ -97,6 +99,9 @@
>  			.direction_input	= s3c24xx_gpiolib_banka_input,
>  			.direction_output	= s3c24xx_gpiolib_banka_output,
>  		},
> +#ifdef CONFIG_OF_GPIO
> +		.dt_compat = "samsung,s3c2410-gpio-a",
> +#endif

The '-a' bit I suspect is wrong.  Aren't all the gpio banks of the same type?

>  	},
>  	[1] = {
>  		.base	= S3C2410_GPBCON,
> @@ -210,6 +215,45 @@
>  	},
>  };
>  
> +#ifdef CONFIG_OF_GPIO
> +static int s3c24xx_of_base_match(struct s3c_gpio_chip *chip, struct device_node *dn)
> +{
> +	const u32 *addrp;
> +	u64 addr;
> +
> +	addrp = of_get_address(dn, 0, 0, NULL);
> +	if (!addrp)
> +		return 0;
> +
> +	addr = of_translate_address(dn, addrp);
> +	if (addr == OF_BAD_ADDR)
> +		return 0;

Use of_address_to_resource() instead.

> +
> +	return chip->base == (addr - S3C24XX_PA_GPIO + S3C24XX_VA_GPIO);
> +}
> +
> +static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
> +{
> +	struct device_node *dn;
> +
> +	if (!chip->dt_compat)
> +		chip->dt_compat = "samsung,s3c2410-gpio";
> +
> +	for_each_compatible_node(dn, NULL, chip->dt_compat) {
> +		if (s3c24xx_of_base_match(chip, dn)) {
> +			chip->chip.of_node = dn;
> +			break;
> +		}
> +	}
> +}
> +
> +#else
> +
> +static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
> +{
> +}
> +
> +#endif
>  
>  static __init int s3c24xx_gpiolib_init(void)
>  {
> @@ -220,6 +264,7 @@
>  		if (!chip->config)
>  			chip->config = &s3c24xx_gpiocfg_default;
>  
> +		s3c24xx_attach_of_node(chip);
>  		s3c_gpiolib_add(chip);
>  	}
>  
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

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

* [PATCH 2/2] Add GPIO DT support to s3c24xx
       [not found] <20110421130711.920096092@gmail.com>
@ 2011-04-21 13:17 ` Domenico Andreoli
  0 siblings, 0 replies; 8+ messages in thread
From: Domenico Andreoli @ 2011-04-21 13:17 UTC (permalink / raw)
  To: devicetree-discuss, linux-mmc, spi-devel-general

[-- Attachment #1: s3c24xx-gpio-add-dt-support --]
[-- Type: text/plain, Size: 1709 bytes --]

From: Domenico Andreoli <cavokz@gmail.com>

Assign proper OF node (= with matching physical base address) to each
s3c24xx GPIO chip.

Signed-off-by: Domenico Andreoli <cavokz@gmail.com>

---
 arch/arm/plat-s3c24xx/gpiolib.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Index: b/arch/arm/plat-s3c24xx/gpiolib.c
===================================================================
--- a/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-10 23:28:31.000000000 +0200
+++ b/arch/arm/plat-s3c24xx/gpiolib.c	2011-04-11 11:23:36.000000000 +0200
@@ -19,6 +19,8 @@
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <plat/gpio-core.h>
 #include <plat/gpio-cfg.h>
@@ -210,6 +212,39 @@
 	},
 };
 
+#ifdef CONFIG_OF_GPIO
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+	struct device_node *dn;
+	struct resource res;
+	const char *dt_compat;
+
+	if (chip->base == S3C2410_GPACON)
+		dt_compat = "samsung,s3c2410-gpio-a";
+	else
+		dt_compat = "samsung,s3c2410-gpio";
+
+	for_each_compatible_node(dn, NULL, dt_compat) {
+		if (of_address_to_resource(dn, 0, &res) < 0) {
+			printk(KERN_ERR "%s: unable to translate DT address\n", dn->full_name);
+			continue;
+		}
+
+		if (chip->base == (res.start - S3C24XX_PA_GPIO + S3C24XX_VA_GPIO)) {
+			chip->chip.of_node = dn;
+			break;
+		}
+	}
+}
+
+#else
+
+static void s3c24xx_attach_of_node(struct s3c_gpio_chip *chip)
+{
+}
+
+#endif
 
 static __init int s3c24xx_gpiolib_init(void)
 {
@@ -220,6 +255,7 @@
 		if (!chip->config)
 			chip->config = &s3c24xx_gpiocfg_default;
 
+		s3c24xx_attach_of_node(chip);
 		s3c_gpiolib_add(chip);
 	}
 


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

end of thread, other threads:[~2011-04-21 13:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20110407163322.465935232@gmail.com>
2011-04-07 16:39 ` [patch 1/2] Add dt_compat field to struct gpio_chip Domenico Andreoli
     [not found]   ` <20110407163930.GB17498-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
2011-04-07 17:17     ` Grant Likely
     [not found]       ` <20110407171704.GC2455-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
2011-04-07 20:41         ` Domenico Andreoli
     [not found]           ` <BANLkTi=WE_vOUAi8T60ESAH2kua4uB-_HA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-04-08  0:17             ` Grant Likely
2011-04-07 16:39 ` [patch 2/2] Add GPIO DT support to s3c24xx Domenico Andreoli
     [not found] <20110410152735.338798127@gmail.com>
2011-04-10 15:33 ` Domenico Andreoli
     [not found]   ` <20110410153316.GC27088-iIV0ii4H0r6tG0bUXCXiUA@public.gmane.org>
2011-04-11  6:47     ` Grant Likely
     [not found] <20110421130711.920096092@gmail.com>
2011-04-21 13:17 ` [PATCH " Domenico Andreoli

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).