* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
@ 2012-08-25 21:53 Heiko Stübner
2012-08-27 4:20 ` Thomas Abraham
2012-08-31 22:52 ` Linus Walleij
0 siblings, 2 replies; 9+ messages in thread
From: Heiko Stübner @ 2012-08-25 21:53 UTC (permalink / raw)
To: linux-arm-kernel
Until now the Exynos-SoC was the only Samsung-SoC supporting the GPIOs
via the device tree. This patch implements dt-support for the
s3c24xx arches.
The controllers contain only 3 cells, as the underlying gpio controller
does not support controlling the drive strength on a gpio level.
Tested with the gpio-keys driver on a s3c2416 based machine.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
.../devicetree/bindings/gpio/gpio-samsung.txt | 38 ++++++++++++
drivers/gpio/gpio-samsung.c | 63 ++++++++++++++++++++
2 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
index 5375625..ce6a7d4 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
@@ -39,3 +39,41 @@ Example:
#gpio-cells = <4>;
gpio-controller;
};
+
+
+Samsung S3C24XX GPIO Controller
+
+Required properties:
+- compatible: Compatible property value should be "samsung,s3c24xx-gpio".
+
+- reg: Physical base address of the controller and length of memory mapped
+ region.
+
+- #gpio-cells: Should be 3. The syntax of the gpio specifier used by client nodes
+ should be the following with values derived from the SoC user manual.
+ <[phandle of the gpio controller node]
+ [pin number within the gpio controller]
+ [mux function]
+ [flags and pull up/down]
+
+ Values for gpio specifier:
+ - Pin number: depending on the controller a number from 0 up to 15.
+ - Flags and Pull Up/Down: 0 - Pull Up/Down Disabled.
+ 1 - Pull Down Enabled.
+ 3 - Pull Up Enabled.
+ Bit 16 (0x00010000) - Input is active low.
+
+- gpio-controller: Specifies that the node is a gpio controller.
+- #address-cells: should be 1.
+- #size-cells: should be 1.
+
+Example:
+
+ gpa: gpio-controller@56000000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "samsung,s3c24xx-gpio";
+ reg = <0x56000000 0x10>;
+ #gpio-cells = <3>;
+ gpio-controller;
+ };
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index a150d2e..80a2817 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -938,6 +938,67 @@ static void __init samsung_gpiolib_add(struct samsung_gpio_chip *chip)
s3c_gpiolib_track(chip);
}
+#if defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF)
+static int s3c24xx_gpio_xlate(struct gpio_chip *gc,
+ const struct of_phandle_args *gpiospec, u32 *flags)
+{
+ unsigned int pin;
+
+ if (WARN_ON(gc->of_gpio_n_cells < 3))
+ return -EINVAL;
+
+ if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
+ return -EINVAL;
+
+ if (gpiospec->args[0] > gc->ngpio)
+ return -EINVAL;
+
+ pin = gc->base + gpiospec->args[0];
+
+ if (s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(gpiospec->args[1])))
+ pr_warn("gpio_xlate: failed to set pin function\n");
+ if (s3c_gpio_setpull(pin, gpiospec->args[2] & 0xffff))
+ pr_warn("gpio_xlate: failed to set pin pull up/down\n");
+
+ if (flags)
+ *flags = gpiospec->args[2] >> 16;
+
+ return gpiospec->args[0];
+}
+
+static const struct of_device_id s3c24xx_gpio_dt_match[] __initdata = {
+ { .compatible = "samsung,s3c24xx-gpio", },
+ {}
+};
+
+static __init void s3c24xx_gpiolib_attach_ofnode(struct samsung_gpio_chip *chip,
+ u64 base, u64 offset)
+{
+ struct gpio_chip *gc = &chip->chip;
+ u64 address;
+
+ if (!of_have_populated_dt())
+ return;
+
+ address = chip->base ? base + ((u32)chip->base & 0xfff) : base + offset;
+ gc->of_node = of_find_matching_node_by_address(NULL,
+ s3c24xx_gpio_dt_match, address);
+ if (!gc->of_node) {
+ pr_info("gpio: device tree node not found for gpio controller"
+ " with base address %08llx\n", address);
+ return;
+ }
+ gc->of_gpio_n_cells = 3;
+ gc->of_xlate = s3c24xx_gpio_xlate;
+}
+#elif defined(CONFIG_PLAT_S3C24XX)
+static __init void s3c24xx_gpiolib_attach_ofnode(struct samsung_gpio_chip *chip,
+ u64 base, u64 offset)
+{
+ return;
+}
+#endif /* defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF) */
+
static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip *chip,
int nr_chips, void __iomem *base)
{
@@ -962,6 +1023,8 @@ static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip *chip,
gc->direction_output = samsung_gpiolib_2bit_output;
samsung_gpiolib_add(chip);
+
+ s3c24xx_gpiolib_attach_ofnode(chip, S3C24XX_PA_GPIO, i * 0x10);
}
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-25 21:53 [PATCH] gpio: samsung: add devicetree init for s3c24xx arches Heiko Stübner
@ 2012-08-27 4:20 ` Thomas Abraham
2012-08-27 8:01 ` Heiko Stübner
2012-08-31 22:52 ` Linus Walleij
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Abraham @ 2012-08-27 4:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Heiko,
On 26 August 2012 03:23, Heiko St?bner <heiko@sntech.de> wrote:
> Until now the Exynos-SoC was the only Samsung-SoC supporting the GPIOs
> via the device tree. This patch implements dt-support for the
> s3c24xx arches.
>
> The controllers contain only 3 cells, as the underlying gpio controller
> does not support controlling the drive strength on a gpio level.
>
> Tested with the gpio-keys driver on a s3c2416 based machine.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> .../devicetree/bindings/gpio/gpio-samsung.txt | 38 ++++++++++++
> drivers/gpio/gpio-samsung.c | 63 ++++++++++++++++++++
> 2 files changed, 101 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> index 5375625..ce6a7d4 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> @@ -39,3 +39,41 @@ Example:
> #gpio-cells = <4>;
> gpio-controller;
> };
> +
> +
> +Samsung S3C24XX GPIO Controller
> +
> +Required properties:
> +- compatible: Compatible property value should be "samsung,s3c24xx-gpio".
This is debatable, but I would choose to be specific here and use
"samsing,s3c2416-gpio". For instance, SoC's in s3c24xx family have
differing value for pull none/down/up.
> +
> +- reg: Physical base address of the controller and length of memory mapped
> + region.
> +
> +- #gpio-cells: Should be 3. The syntax of the gpio specifier used by client nodes
> + should be the following with values derived from the SoC user manual.
> + <[phandle of the gpio controller node]
> + [pin number within the gpio controller]
> + [mux function]
> + [flags and pull up/down]
> +
> + Values for gpio specifier:
> + - Pin number: depending on the controller a number from 0 up to 15.
> + - Flags and Pull Up/Down: 0 - Pull Up/Down Disabled.
> + 1 - Pull Down Enabled.
> + 3 - Pull Up Enabled.
As per s3c2416 user manual, 2 is used for Pull Up and 3 is listed as
reserved value. Any particular reason to use 3 here for pull up and
not 2 as per the user manual?
> + Bit 16 (0x00010000) - Input is active low.
> +
> +- gpio-controller: Specifies that the node is a gpio controller.
> +- #address-cells: should be 1.
> +- #size-cells: should be 1.
It would be informative to add information about the 'mux function'
cell here as well. Specifically, on how to handle the banks that have
an extended GPxSEL register that allow additional pin function
selection.
> +
> +Example:
> +
> + gpa: gpio-controller at 56000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "samsung,s3c24xx-gpio";
> + reg = <0x56000000 0x10>;
> + #gpio-cells = <3>;
> + gpio-controller;
> + };
> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> index a150d2e..80a2817 100644
> --- a/drivers/gpio/gpio-samsung.c
> +++ b/drivers/gpio/gpio-samsung.c
> @@ -938,6 +938,67 @@ static void __init samsung_gpiolib_add(struct samsung_gpio_chip *chip)
> s3c_gpiolib_track(chip);
> }
>
> +#if defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF)
> +static int s3c24xx_gpio_xlate(struct gpio_chip *gc,
> + const struct of_phandle_args *gpiospec, u32 *flags)
> +{
> + unsigned int pin;
> +
> + if (WARN_ON(gc->of_gpio_n_cells < 3))
> + return -EINVAL;
> +
> + if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> + return -EINVAL;
> +
> + if (gpiospec->args[0] > gc->ngpio)
> + return -EINVAL;
> +
> + pin = gc->base + gpiospec->args[0];
> +
> + if (s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(gpiospec->args[1])))
> + pr_warn("gpio_xlate: failed to set pin function\n");
> + if (s3c_gpio_setpull(pin, gpiospec->args[2] & 0xffff))
> + pr_warn("gpio_xlate: failed to set pin pull up/down\n");
> +
> + if (flags)
> + *flags = gpiospec->args[2] >> 16;
> +
> + return gpiospec->args[0];
> +}
> +
> +static const struct of_device_id s3c24xx_gpio_dt_match[] __initdata = {
> + { .compatible = "samsung,s3c24xx-gpio", },
> + {}
> +};
> +
> +static __init void s3c24xx_gpiolib_attach_ofnode(struct samsung_gpio_chip *chip,
> + u64 base, u64 offset)
> +{
> + struct gpio_chip *gc = &chip->chip;
> + u64 address;
> +
> + if (!of_have_populated_dt())
> + return;
> +
> + address = chip->base ? base + ((u32)chip->base & 0xfff) : base + offset;
> + gc->of_node = of_find_matching_node_by_address(NULL,
> + s3c24xx_gpio_dt_match, address);
> + if (!gc->of_node) {
> + pr_info("gpio: device tree node not found for gpio controller"
> + " with base address %08llx\n", address);
> + return;
> + }
> + gc->of_gpio_n_cells = 3;
> + gc->of_xlate = s3c24xx_gpio_xlate;
> +}
> +#elif defined(CONFIG_PLAT_S3C24XX)
> +static __init void s3c24xx_gpiolib_attach_ofnode(struct samsung_gpio_chip *chip,
> + u64 base, u64 offset)
> +{
> + return;
> +}
> +#endif /* defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF) */
> +
> static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip *chip,
> int nr_chips, void __iomem *base)
> {
> @@ -962,6 +1023,8 @@ static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip *chip,
> gc->direction_output = samsung_gpiolib_2bit_output;
>
> samsung_gpiolib_add(chip);
> +
> + s3c24xx_gpiolib_attach_ofnode(chip, S3C24XX_PA_GPIO, i * 0x10);
> }
> }
>
> --
> 1.7.2.3
>
Overall, this is patch looks fine.
Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 4:20 ` Thomas Abraham
@ 2012-08-27 8:01 ` Heiko Stübner
2012-08-27 8:57 ` Vasily Khoruzhick
2012-08-28 4:44 ` Thomas Abraham
0 siblings, 2 replies; 9+ messages in thread
From: Heiko Stübner @ 2012-08-27 8:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
thanks for your review:
Am Montag, 27. August 2012, 06:20:49 schrieb Thomas Abraham:
> Hi Heiko,
>
> On 26 August 2012 03:23, Heiko St?bner <heiko@sntech.de> wrote:
> > Until now the Exynos-SoC was the only Samsung-SoC supporting the GPIOs
> > via the device tree. This patch implements dt-support for the
> > s3c24xx arches.
> >
> > The controllers contain only 3 cells, as the underlying gpio controller
> > does not support controlling the drive strength on a gpio level.
> >
> > Tested with the gpio-keys driver on a s3c2416 based machine.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >
> > .../devicetree/bindings/gpio/gpio-samsung.txt | 38 ++++++++++++
> > drivers/gpio/gpio-samsung.c | 63
> > ++++++++++++++++++++ 2 files changed, 101 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> > b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt index
> > 5375625..ce6a7d4 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
> >
> > @@ -39,3 +39,41 @@ Example:
> > #gpio-cells = <4>;
> > gpio-controller;
> >
> > };
> >
> > +
> > +
> > +Samsung S3C24XX GPIO Controller
> > +
> > +Required properties:
> > +- compatible: Compatible property value should be
> > "samsung,s3c24xx-gpio".
>
> This is debatable, but I would choose to be specific here and use
> "samsing,s3c2416-gpio". For instance, SoC's in s3c24xx family have
> differing value for pull none/down/up.
Hmmm, but the logic to drive the gpio controller is the same for all arches.
The only difference is the number to use for the different pull up/down
settings. So I think the "samsung,s3c24xx-gpio" is ok but the documentation
should simply reflect the different pull settings.
> > +
> > +- reg: Physical base address of the controller and length of memory
> > mapped + region.
> > +
> > +- #gpio-cells: Should be 3. The syntax of the gpio specifier used by
> > client nodes + should be the following with values derived from the SoC
> > user manual. + <[phandle of the gpio controller node]
> > + [pin number within the gpio controller]
> > + [mux function]
> > + [flags and pull up/down]
> > +
> > + Values for gpio specifier:
> > + - Pin number: depending on the controller a number from 0 up to 15.
> > + - Flags and Pull Up/Down: 0 - Pull Up/Down Disabled.
> > + 1 - Pull Down Enabled.
> > + 3 - Pull Up Enabled.
>
> As per s3c2416 user manual, 2 is used for Pull Up and 3 is listed as
> reserved value. Any particular reason to use 3 here for pull up and
> not 2 as per the user manual?
Sorry, copy and paste error :-)
> > + Bit 16 (0x00010000) - Input is active low.
> > +
> > +- gpio-controller: Specifies that the node is a gpio controller.
> > +- #address-cells: should be 1.
> > +- #size-cells: should be 1.
>
> It would be informative to add information about the 'mux function'
> cell here as well. Specifically, on how to handle the banks that have
> an extended GPxSEL register that allow additional pin function
> selection.
Will add a mux function description.
Until now I've never realised the existence of the GPxSEL registers, so thanks
for the pointer :-).
But is this used in the driver at all? All the setcfg/getcfg functions in
gpio-samsung.c only handle the GPxCON registers - I haven't found code to
handle the GPxSEL registers at all. So my guess is that this was never
implemented - or that I'm blind ;-) .
> > +
> > +Example:
> > +
> > + gpa: gpio-controller at 56000000 {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + compatible = "samsung,s3c24xx-gpio";
> > + reg = <0x56000000 0x10>;
> > + #gpio-cells = <3>;
> > + gpio-controller;
> > + };
> > diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> > index a150d2e..80a2817 100644
> > --- a/drivers/gpio/gpio-samsung.c
> > +++ b/drivers/gpio/gpio-samsung.c
> > @@ -938,6 +938,67 @@ static void __init samsung_gpiolib_add(struct
> > samsung_gpio_chip *chip)
> >
> > s3c_gpiolib_track(chip);
> >
> > }
> >
> > +#if defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF)
> > +static int s3c24xx_gpio_xlate(struct gpio_chip *gc,
> > + const struct of_phandle_args *gpiospec, u32
> > *flags) +{
> > + unsigned int pin;
> > +
> > + if (WARN_ON(gc->of_gpio_n_cells < 3))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> > + return -EINVAL;
> > +
> > + if (gpiospec->args[0] > gc->ngpio)
> > + return -EINVAL;
> > +
> > + pin = gc->base + gpiospec->args[0];
> > +
> > + if (s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(gpiospec->args[1])))
> > + pr_warn("gpio_xlate: failed to set pin function\n");
> > + if (s3c_gpio_setpull(pin, gpiospec->args[2] & 0xffff))
> > + pr_warn("gpio_xlate: failed to set pin pull up/down\n");
> > +
> > + if (flags)
> > + *flags = gpiospec->args[2] >> 16;
> > +
> > + return gpiospec->args[0];
> > +}
> > +
> > +static const struct of_device_id s3c24xx_gpio_dt_match[] __initdata = {
> > + { .compatible = "samsung,s3c24xx-gpio", },
> > + {}
> > +};
> > +
> > +static __init void s3c24xx_gpiolib_attach_ofnode(struct
> > samsung_gpio_chip *chip, +
> > u64 base, u64 offset) +{
> > + struct gpio_chip *gc = &chip->chip;
> > + u64 address;
> > +
> > + if (!of_have_populated_dt())
> > + return;
> > +
> > + address = chip->base ? base + ((u32)chip->base & 0xfff) : base +
> > offset; + gc->of_node = of_find_matching_node_by_address(NULL,
> > + s3c24xx_gpio_dt_match, address);
> > + if (!gc->of_node) {
> > + pr_info("gpio: device tree node not found for gpio
> > controller" + " with base address %08llx\n",
> > address); + return;
> > + }
> > + gc->of_gpio_n_cells = 3;
> > + gc->of_xlate = s3c24xx_gpio_xlate;
> > +}
> > +#elif defined(CONFIG_PLAT_S3C24XX)
> > +static __init void s3c24xx_gpiolib_attach_ofnode(struct
> > samsung_gpio_chip *chip, +
> > u64 base, u64 offset) +{
> > + return;
> > +}
> > +#endif /* defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF) */
> > +
> >
> > static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip
> > *chip,
> >
> > int nr_chips, void __iomem
> > *base)
> >
> > {
> >
> > @@ -962,6 +1023,8 @@ static void __init s3c24xx_gpiolib_add_chips(struct
> > samsung_gpio_chip *chip,
> >
> > gc->direction_output =
> > samsung_gpiolib_2bit_output;
> >
> > samsung_gpiolib_add(chip);
> >
> > +
> > + s3c24xx_gpiolib_attach_ofnode(chip, S3C24XX_PA_GPIO, i *
> > 0x10);
> >
> > }
> >
> > }
> >
> > --
> > 1.7.2.3
>
> Overall, this is patch looks fine.
>
> Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>
Heiko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 8:01 ` Heiko Stübner
@ 2012-08-27 8:57 ` Vasily Khoruzhick
2012-08-27 9:44 ` Heiko Stübner
2012-08-28 4:44 ` Thomas Abraham
1 sibling, 1 reply; 9+ messages in thread
From: Vasily Khoruzhick @ 2012-08-27 8:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Mon, Aug 27, 2012 at 11:01 AM, Heiko St?bner <heiko@sntech.de> wrote:
> Hi Thomas,
>
> thanks for your review:
>
> Hmmm, but the logic to drive the gpio controller is the same for all arches.
> The only difference is the number to use for the different pull up/down
> settings. So I think the "samsung,s3c24xx-gpio" is ok but the documentation
> should simply reflect the different pull settings.
>
Also s3c2410 and s3c2440 has different count of GPIOs. And that could
be a problem on machines
with GPIO-expanders (h1940 machine)
>> It would be informative to add information about the 'mux function'
>> cell here as well. Specifically, on how to handle the banks that have
>> an extended GPxSEL register that allow additional pin function
>> selection.
>
> Will add a mux function description.
>
> Until now I've never realised the existence of the GPxSEL registers, so thanks
> for the pointer :-).
>
> But is this used in the driver at all? All the setcfg/getcfg functions in
> gpio-samsung.c only handle the GPxCON registers - I haven't found code to
> handle the GPxSEL registers at all. So my guess is that this was never
> implemented - or that I'm blind ;-) .
There're no GPxSEL registers on s3c2410/s3c2440/s3c2442. Is it
something s3c2412/sec2416 specific?
Regards
Vasily
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 8:57 ` Vasily Khoruzhick
@ 2012-08-27 9:44 ` Heiko Stübner
2012-08-27 9:51 ` Vasily Khoruzhick
0 siblings, 1 reply; 9+ messages in thread
From: Heiko Stübner @ 2012-08-27 9:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi Vasily,
Am Montag, 27. August 2012, 10:57:04 schrieb Vasily Khoruzhick:
> Hi,
>
> On Mon, Aug 27, 2012 at 11:01 AM, Heiko St?bner <heiko@sntech.de> wrote:
> > Hi Thomas,
> >
> > thanks for your review:
> >
> > Hmmm, but the logic to drive the gpio controller is the same for all
> > arches. The only difference is the number to use for the different pull
> > up/down settings. So I think the "samsung,s3c24xx-gpio" is ok but the
> > documentation should simply reflect the different pull settings.
>
> Also s3c2410 and s3c2440 has different count of GPIOs. And that could
> be a problem on machines
> with GPIO-expanders (h1940 machine)
I'm not sure I follow :-) .
The compatible property here only sets the mechanism on how to handle the
gpios defined in the devicetree - here to use the s3c24xx-style. As you can
see in gpio-samsung.c the handling is already unified for all the s3c24xx
architectures.
The definition of what gpio banks exist is then done in the respective
devicetree file for the individual SoC. And of course here one would have
individual definitions, depending on the banks present.
For reference my quite empty s3c2416.dtsi file currently looks like:
/include/ "skeleton.dtsi"
/ {
compatible = "samsung,s3c2416";
cpus {
cpu at 0 {
compatible = "arm,arm926ejs";
};
};
gpio-controllers {
#address-cells = <1>;
#size-cells = <1>;
gpio-controller;
ranges;
gpa: gpio-controller at 56000000 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000000 0x10>;
#gpio-cells = <3>;
};
gpb: gpio-controller at 56000010 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000010 0x10>;
#gpio-cells = <3>;
};
gpc: gpio-controller at 56000020 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000020 0x10>;
#gpio-cells = <3>;
};
gpd: gpio-controller at 56000030 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000030 0x10>;
#gpio-cells = <3>;
};
gpe: gpio-controller at 56000040 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000040 0x10>;
#gpio-cells = <3>;
};
gpf: gpio-controller at 56000050 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000050 0x10>;
#gpio-cells = <3>;
};
gpg: gpio-controller at 56000060 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000060 0x10>;
#gpio-cells = <3>;
};
gph: gpio-controller at 56000070 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000070 0x10>;
#gpio-cells = <3>;
};
/* s3c2443 and later */
gpj: gpio-controller at 560000D0 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x560000D0 0x10>;
#gpio-cells = <3>;
};
gpk: gpio-controller at 560000E0 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x560000E0 0x10>;
#gpio-cells = <3>;
};
gpl: gpio-controller at 560000F0 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x560000F0 0x10>;
#gpio-cells = <3>;
};
gpm: gpio-controller at 56000100 {
compatible = "samsung,s3c24xx-gpio";
reg = <0x56000100 0x10>;
#gpio-cells = <3>;
};
};
};
Other s3c24xx SoCs would of course need to define their own.
> >> It would be informative to add information about the 'mux function'
> >> cell here as well. Specifically, on how to handle the banks that have
> >> an extended GPxSEL register that allow additional pin function
> >> selection.
> >
> > Will add a mux function description.
> >
> > Until now I've never realised the existence of the GPxSEL registers, so
> > thanks for the pointer :-).
> >
> > But is this used in the driver at all? All the setcfg/getcfg functions in
> > gpio-samsung.c only handle the GPxCON registers - I haven't found code to
> > handle the GPxSEL registers at all. So my guess is that this was never
> > implemented - or that I'm blind ;-) .
>
> There're no GPxSEL registers on s3c2410/s3c2440/s3c2442. Is it
> something s3c2412/sec2416 specific?
s3c2412 also does not have it ... it seems entirely specific to
s3c2416/s3c2450 and as written also seems to be unused.
So I would tend to ignore it for now :-) .
Heiko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 9:44 ` Heiko Stübner
@ 2012-08-27 9:51 ` Vasily Khoruzhick
2012-08-27 10:12 ` Heiko Stübner
0 siblings, 1 reply; 9+ messages in thread
From: Vasily Khoruzhick @ 2012-08-27 9:51 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 27, 2012 at 12:44 PM, Heiko St?bner <heiko@sntech.de> wrote:
> Hi Vasily,
>
> I'm not sure I follow :-) .
>
> The compatible property here only sets the mechanism on how to handle the
> gpios defined in the devicetree - here to use the s3c24xx-style. As you can
> see in gpio-samsung.c the handling is already unified for all the s3c24xx
> architectures.
>
> The definition of what gpio banks exist is then done in the respective
> devicetree file for the individual SoC. And of course here one would have
> individual definitions, depending on the banks present.
>
> For reference my quite empty s3c2416.dtsi file currently looks like:
>
> /include/ "skeleton.dtsi"
>
> / {
> compatible = "samsung,s3c2416";
>
> cpus {
> cpu at 0 {
> compatible = "arm,arm926ejs";
> };
> };
>
> gpio-controllers {
> #address-cells = <1>;
> #size-cells = <1>;
> gpio-controller;
> ranges;
>
> gpa: gpio-controller at 56000000 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000000 0x10>;
> #gpio-cells = <3>;
> };
>
> gpb: gpio-controller at 56000010 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000010 0x10>;
> #gpio-cells = <3>;
> };
>
> gpc: gpio-controller at 56000020 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000020 0x10>;
> #gpio-cells = <3>;
> };
>
> gpd: gpio-controller at 56000030 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000030 0x10>;
> #gpio-cells = <3>;
> };
>
> gpe: gpio-controller at 56000040 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000040 0x10>;
> #gpio-cells = <3>;
> };
>
> gpf: gpio-controller at 56000050 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000050 0x10>;
> #gpio-cells = <3>;
> };
>
> gpg: gpio-controller at 56000060 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000060 0x10>;
> #gpio-cells = <3>;
> };
>
> gph: gpio-controller at 56000070 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000070 0x10>;
> #gpio-cells = <3>;
> };
>
> /* s3c2443 and later */
> gpj: gpio-controller at 560000D0 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x560000D0 0x10>;
> #gpio-cells = <3>;
> };
>
> gpk: gpio-controller at 560000E0 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x560000E0 0x10>;
> #gpio-cells = <3>;
> };
>
> gpl: gpio-controller at 560000F0 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x560000F0 0x10>;
> #gpio-cells = <3>;
> };
>
> gpm: gpio-controller at 56000100 {
> compatible = "samsung,s3c24xx-gpio";
> reg = <0x56000100 0x10>;
> #gpio-cells = <3>;
> };
> };
> };
>
> Other s3c24xx SoCs would of course need to define their own.
I see. But how does it handle GPA bank (which is output-only, and
GPACON differs a bit from GP{B-J}CON? And some banks has lower number
(not 16) of GPIOs, like GPH.
Also, what would be value for S3C_GPIO_END?
Regards
Vasily
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 9:51 ` Vasily Khoruzhick
@ 2012-08-27 10:12 ` Heiko Stübner
0 siblings, 0 replies; 9+ messages in thread
From: Heiko Stübner @ 2012-08-27 10:12 UTC (permalink / raw)
To: linux-arm-kernel
Am Montag, 27. August 2012, 11:51:27 schrieb Vasily Khoruzhick:
> On Mon, Aug 27, 2012 at 12:44 PM, Heiko St?bner <heiko@sntech.de> wrote:
> > Hi Vasily,
> >
> > I'm not sure I follow :-) .
> >
> > The compatible property here only sets the mechanism on how to handle the
> > gpios defined in the devicetree - here to use the s3c24xx-style. As you
> > can see in gpio-samsung.c the handling is already unified for all the
> > s3c24xx architectures.
> >
> > The definition of what gpio banks exist is then done in the respective
> > devicetree file for the individual SoC. And of course here one would have
> > individual definitions, depending on the banks present.
> >
> > For reference my quite empty s3c2416.dtsi file currently looks like:
> >
> > /include/ "skeleton.dtsi"
> >
> > / {
> >
> > compatible = "samsung,s3c2416";
> >
> > cpus {
> >
> > cpu at 0 {
> >
> > compatible = "arm,arm926ejs";
> >
> > };
> >
> > };
> >
> > gpio-controllers {
> >
> > #address-cells = <1>;
> > #size-cells = <1>;
> > gpio-controller;
> > ranges;
> >
> > gpa: gpio-controller at 56000000 {
> >
> > compatible = "samsung,s3c24xx-gpio";
> > reg = <0x56000000 0x10>;
> > #gpio-cells = <3>;
> >
> > };
> >
> > gpb: gpio-controller at 56000010 {
> >
> > compatible = "samsung,s3c24xx-gpio";
> > reg = <0x56000010 0x10>;
> > #gpio-cells = <3>;
> >
> > };
[ ... ]
> > };
> >
> > };
> >
> > Other s3c24xx SoCs would of course need to define their own.
>
> I see. But how does it handle GPA bank (which is output-only, and
> GPACON differs a bit from GP{B-J}CON? And some banks has lower number
> (not 16) of GPIOs, like GPH.
>
> Also, what would be value for S3C_GPIO_END?
The creation of the banks is still done in gpio-samsung.c and the special
handling of bank-a is still sitting in the s3c24xx_gpios[] array there.
In general the gpios are still created completely like before and the
devicetree information is only attached to them so that dt-devices can
reference them.
As you can see in the original patch, after the respective chip got created in
s3c24xx_gpiolib_add_chips, s3c24xx_gpiolib_attach_ofnode searches for the
representation of it in the device tree and attaches this to the chip.
So nothing really changed, except that dt-devices can now reference gpios and
if they do s3c24xx_gpio_xlate will get called to configure them with the
target values from the dt.
Currently I'm also only using it in a mixed mode configuration [1],
supplementing a normal machine file with some devices from the devicetree -
especially as I still need to figure out, what to do about the interrupt
controller.
Heiko
[1] https://github.com/mmind/linux-es600/blob/topic/es600-
devel/arch/arm/boot/dts/s3c24xx-sg06.dts
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-27 8:01 ` Heiko Stübner
2012-08-27 8:57 ` Vasily Khoruzhick
@ 2012-08-28 4:44 ` Thomas Abraham
1 sibling, 0 replies; 9+ messages in thread
From: Thomas Abraham @ 2012-08-28 4:44 UTC (permalink / raw)
To: linux-arm-kernel
On 27 August 2012 13:31, Heiko St?bner <heiko@sntech.de> wrote:
> Hi Thomas,
>
> thanks for your review:
>
> Am Montag, 27. August 2012, 06:20:49 schrieb Thomas Abraham:
>> Hi Heiko,
>>
>> On 26 August 2012 03:23, Heiko St?bner <heiko@sntech.de> wrote:
>> > Until now the Exynos-SoC was the only Samsung-SoC supporting the GPIOs
>> > via the device tree. This patch implements dt-support for the
>> > s3c24xx arches.
>> >
>> > The controllers contain only 3 cells, as the underlying gpio controller
>> > does not support controlling the drive strength on a gpio level.
>> >
>> > Tested with the gpio-keys driver on a s3c2416 based machine.
>> >
>> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>> > ---
>> >
>> > .../devicetree/bindings/gpio/gpio-samsung.txt | 38 ++++++++++++
>> > drivers/gpio/gpio-samsung.c | 63
>> > ++++++++++++++++++++ 2 files changed, 101 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
>> > b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt index
>> > 5375625..ce6a7d4 100644
>> > --- a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
>> > +++ b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
>> >
>> > @@ -39,3 +39,41 @@ Example:
>> > #gpio-cells = <4>;
>> > gpio-controller;
>> >
>> > };
>> >
>> > +
>> > +
>> > +Samsung S3C24XX GPIO Controller
>> > +
>> > +Required properties:
>> > +- compatible: Compatible property value should be
>> > "samsung,s3c24xx-gpio".
>>
>> This is debatable, but I would choose to be specific here and use
>> "samsing,s3c2416-gpio". For instance, SoC's in s3c24xx family have
>> differing value for pull none/down/up.
>
> Hmmm, but the logic to drive the gpio controller is the same for all arches.
> The only difference is the number to use for the different pull up/down
> settings. So I think the "samsung,s3c24xx-gpio" is ok but the documentation
> should simply reflect the different pull settings.
Ok.
>
>
>> > +
>> > +- reg: Physical base address of the controller and length of memory
>> > mapped + region.
>> > +
>> > +- #gpio-cells: Should be 3. The syntax of the gpio specifier used by
>> > client nodes + should be the following with values derived from the SoC
>> > user manual. + <[phandle of the gpio controller node]
>> > + [pin number within the gpio controller]
>> > + [mux function]
>> > + [flags and pull up/down]
>> > +
>> > + Values for gpio specifier:
>> > + - Pin number: depending on the controller a number from 0 up to 15.
>> > + - Flags and Pull Up/Down: 0 - Pull Up/Down Disabled.
>> > + 1 - Pull Down Enabled.
>> > + 3 - Pull Up Enabled.
>>
>> As per s3c2416 user manual, 2 is used for Pull Up and 3 is listed as
>> reserved value. Any particular reason to use 3 here for pull up and
>> not 2 as per the user manual?
>
> Sorry, copy and paste error :-)
Ok.
>
>
>> > + Bit 16 (0x00010000) - Input is active low.
>> > +
>> > +- gpio-controller: Specifies that the node is a gpio controller.
>> > +- #address-cells: should be 1.
>> > +- #size-cells: should be 1.
>>
>> It would be informative to add information about the 'mux function'
>> cell here as well. Specifically, on how to handle the banks that have
>> an extended GPxSEL register that allow additional pin function
>> selection.
>
> Will add a mux function description.
>
> Until now I've never realised the existence of the GPxSEL registers, so thanks
> for the pointer :-).
>
> But is this used in the driver at all? All the setcfg/getcfg functions in
> gpio-samsung.c only handle the GPxCON registers - I haven't found code to
> handle the GPxSEL registers at all. So my guess is that this was never
> implemented - or that I'm blind ;-) .
You are right, there is no code that is handling the GPxCON registers.
Thanks,
Thomas.
>
>
>> > +
>> > +Example:
>> > +
>> > + gpa: gpio-controller at 56000000 {
>> > + #address-cells = <1>;
>> > + #size-cells = <1>;
>> > + compatible = "samsung,s3c24xx-gpio";
>> > + reg = <0x56000000 0x10>;
>> > + #gpio-cells = <3>;
>> > + gpio-controller;
>> > + };
>> > diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
>> > index a150d2e..80a2817 100644
>> > --- a/drivers/gpio/gpio-samsung.c
>> > +++ b/drivers/gpio/gpio-samsung.c
>> > @@ -938,6 +938,67 @@ static void __init samsung_gpiolib_add(struct
>> > samsung_gpio_chip *chip)
>> >
>> > s3c_gpiolib_track(chip);
>> >
>> > }
>> >
>> > +#if defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF)
>> > +static int s3c24xx_gpio_xlate(struct gpio_chip *gc,
>> > + const struct of_phandle_args *gpiospec, u32
>> > *flags) +{
>> > + unsigned int pin;
>> > +
>> > + if (WARN_ON(gc->of_gpio_n_cells < 3))
>> > + return -EINVAL;
>> > +
>> > + if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
>> > + return -EINVAL;
>> > +
>> > + if (gpiospec->args[0] > gc->ngpio)
>> > + return -EINVAL;
>> > +
>> > + pin = gc->base + gpiospec->args[0];
>> > +
>> > + if (s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(gpiospec->args[1])))
>> > + pr_warn("gpio_xlate: failed to set pin function\n");
>> > + if (s3c_gpio_setpull(pin, gpiospec->args[2] & 0xffff))
>> > + pr_warn("gpio_xlate: failed to set pin pull up/down\n");
>> > +
>> > + if (flags)
>> > + *flags = gpiospec->args[2] >> 16;
>> > +
>> > + return gpiospec->args[0];
>> > +}
>> > +
>> > +static const struct of_device_id s3c24xx_gpio_dt_match[] __initdata = {
>> > + { .compatible = "samsung,s3c24xx-gpio", },
>> > + {}
>> > +};
>> > +
>> > +static __init void s3c24xx_gpiolib_attach_ofnode(struct
>> > samsung_gpio_chip *chip, +
>> > u64 base, u64 offset) +{
>> > + struct gpio_chip *gc = &chip->chip;
>> > + u64 address;
>> > +
>> > + if (!of_have_populated_dt())
>> > + return;
>> > +
>> > + address = chip->base ? base + ((u32)chip->base & 0xfff) : base +
>> > offset; + gc->of_node = of_find_matching_node_by_address(NULL,
>> > + s3c24xx_gpio_dt_match, address);
>> > + if (!gc->of_node) {
>> > + pr_info("gpio: device tree node not found for gpio
>> > controller" + " with base address %08llx\n",
>> > address); + return;
>> > + }
>> > + gc->of_gpio_n_cells = 3;
>> > + gc->of_xlate = s3c24xx_gpio_xlate;
>> > +}
>> > +#elif defined(CONFIG_PLAT_S3C24XX)
>> > +static __init void s3c24xx_gpiolib_attach_ofnode(struct
>> > samsung_gpio_chip *chip, +
>> > u64 base, u64 offset) +{
>> > + return;
>> > +}
>> > +#endif /* defined(CONFIG_PLAT_S3C24XX) && defined(CONFIG_OF) */
>> > +
>> >
>> > static void __init s3c24xx_gpiolib_add_chips(struct samsung_gpio_chip
>> > *chip,
>> >
>> > int nr_chips, void __iomem
>> > *base)
>> >
>> > {
>> >
>> > @@ -962,6 +1023,8 @@ static void __init s3c24xx_gpiolib_add_chips(struct
>> > samsung_gpio_chip *chip,
>> >
>> > gc->direction_output =
>> > samsung_gpiolib_2bit_output;
>> >
>> > samsung_gpiolib_add(chip);
>> >
>> > +
>> > + s3c24xx_gpiolib_attach_ofnode(chip, S3C24XX_PA_GPIO, i *
>> > 0x10);
>> >
>> > }
>> >
>> > }
>> >
>> > --
>> > 1.7.2.3
>>
>> Overall, this is patch looks fine.
>>
>> Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>
>
>
> Heiko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] gpio: samsung: add devicetree init for s3c24xx arches
2012-08-25 21:53 [PATCH] gpio: samsung: add devicetree init for s3c24xx arches Heiko Stübner
2012-08-27 4:20 ` Thomas Abraham
@ 2012-08-31 22:52 ` Linus Walleij
1 sibling, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2012-08-31 22:52 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Aug 25, 2012 at 11:53 PM, Heiko St?bner <heiko@sntech.de> wrote:
> Until now the Exynos-SoC was the only Samsung-SoC supporting the GPIOs
> via the device tree. This patch implements dt-support for the
> s3c24xx arches.
>
> The controllers contain only 3 cells, as the underlying gpio controller
> does not support controlling the drive strength on a gpio level.
>
> Tested with the gpio-keys driver on a s3c2416 based machine.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
I cannot figure out if you guys have reached consensus on this
patch or not.
I guess this will go through the Samsung tree in any case, I have no
objections from a GPIO subsystem point of view.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-08-31 22:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-25 21:53 [PATCH] gpio: samsung: add devicetree init for s3c24xx arches Heiko Stübner
2012-08-27 4:20 ` Thomas Abraham
2012-08-27 8:01 ` Heiko Stübner
2012-08-27 8:57 ` Vasily Khoruzhick
2012-08-27 9:44 ` Heiko Stübner
2012-08-27 9:51 ` Vasily Khoruzhick
2012-08-27 10:12 ` Heiko Stübner
2012-08-28 4:44 ` Thomas Abraham
2012-08-31 22:52 ` Linus Walleij
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).