2013-04-16 17:41:51

by Sylwester Nawrocki

[permalink] [raw]
Subject: [PATCH 1/2] gpio: samsung: Remove OF support for s3c24xx

There is no users of this code and there is already a pinctrl
driver written for s3c24xx which is going to be used on any
s3c24xx DT platforms. Hence this has been effectively a dead
code in mainline.

This reverts commit 172c6a13653ac8cd6a231293b87c93821e90c1d6
gpio: samsung: add devicetree init for s3c24xx arches

Cc: Heiko Stübner <[email protected]>
Signed-off-by: Sylwester Nawrocki <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---

The pinctrl driver patch can be found at:
http://www.spinics.net/lists/linux-samsung-soc/msg17624.html
---
.../devicetree/bindings/gpio/gpio-samsung.txt | 43 -------------
drivers/gpio/gpio-samsung.c | 63 --------------------
2 files changed, 106 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
index f1e5dfe..5375625 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-samsung.txt
@@ -39,46 +39,3 @@ 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.
- - Mux function: Depending on the SoC and the gpio bank the gpio can be set
- as input, output or a special function
- - Flags and Pull Up/Down: the values to use differ for the individual SoCs
- example S3C2416/S3C2450:
- 0 - Pull Up/Down Disabled.
- 1 - Pull Down Enabled.
- 2 - Pull Up Enabled.
- Bit 16 (0x00010000) - Input is active low.
- Consult the user manual for the correct values of Mux and Pull Up/Down.
-
-- 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 c4b51d8..4fecfb8 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -933,67 +933,6 @@ 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;
-}
-#else
-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)
{
@@ -1018,8 +957,6 @@ 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.9.5


2013-04-16 18:13:52

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: samsung: Remove OF support for s3c24xx

Am Dienstag, 16. April 2013, 19:41:34 schrieb Sylwester Nawrocki:
> There is no users of this code and there is already a pinctrl
> driver written for s3c24xx which is going to be used on any
> s3c24xx DT platforms. Hence this has been effectively a dead
> code in mainline.
>
> This reverts commit 172c6a13653ac8cd6a231293b87c93821e90c1d6
> gpio: samsung: add devicetree init for s3c24xx arches
>
> Cc: Heiko Stübner <[email protected]>
> Signed-off-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>

This patch stems from a time where I thought pinctrl on s3c24xx would be ages
away :-D, so

Acked-by: Heiko Stuebner <[email protected]>

2013-04-17 10:48:17

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: samsung: Remove OF support for s3c24xx

On 04/16/2013 08:13 PM, Heiko Stübner wrote:
> Am Dienstag, 16. April 2013, 19:41:34 schrieb Sylwester Nawrocki:
>> There is no users of this code and there is already a pinctrl
>> driver written for s3c24xx which is going to be used on any
>> s3c24xx DT platforms. Hence this has been effectively a dead
>> code in mainline.
>>
>> This reverts commit 172c6a13653ac8cd6a231293b87c93821e90c1d6
>> gpio: samsung: add devicetree init for s3c24xx arches
>>
>> Cc: Heiko Stübner <[email protected]>
>> Signed-off-by: Sylwester Nawrocki <[email protected]>
>> Signed-off-by: Kyungmin Park <[email protected]>
>
> This patch stems from a time where I thought pinctrl on s3c24xx would be ages
> away :-D, so

Heh, I just thought I would clean this up a bit. And now we have
a nice pinctrl driver for s3c24xx.

As a side note, I've tried it on s3c2440, together with your patch
series adding dt support for s3c2416. Need to resolve a couple more
issues and hopefully will have a booting system soon.

> Acked-by: Heiko Stuebner <[email protected]>

Thanks.

I abstained with posting the second patch, which removes Exynos4/5 OF
support from the gpio-samsung driver. Now when those platforms have
proper pinctrl driver it is not possible to support simultaneously
the old (pre-eliminary) DT bindings and the standard GPIO bindings
implemented by the pinctrl driver. Anyway I have it prepared so I'll
post it, at least it might be a starting point for a discussion.


Thanks,
Sylwester

2013-04-17 11:06:51

by Sylwester Nawrocki

[permalink] [raw]
Subject: [PATCH RFC 2/2] gpio: samsung: Remove OF support for Exynos4/5

Now when Exynos4 and Exynos5 SoC DT platforms use the pinctrl API
we can remove the interim OF support from this GPIO driver. There
should be no need any more to check the compatible property and to
find out if initialization of the GPIO driver should be skipped
we just check if OF tree is available.

Cc: Thomas Abraham <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Olof Johansson <[email protected]>
Signed-off-by: Sylwester Nawrocki <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/gpio/gpio-samsung.c | 106 +++----------------------------------------
1 file changed, 6 insertions(+), 100 deletions(-)

diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index 4fecfb8..e1c59c0 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -2658,70 +2658,6 @@ static struct samsung_gpio_chip exynos5_gpios_4[] = {
};
#endif

-
-#if defined(CONFIG_ARCH_EXYNOS) && defined(CONFIG_OF)
-static int exynos_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 < 4))
- 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 (s5p_gpio_set_drvstr(pin, gpiospec->args[3]))
- pr_warn("gpio_xlate: failed to set pin drive strength\n");
-
- if (flags)
- *flags = gpiospec->args[2] >> 16;
-
- return gpiospec->args[0];
-}
-
-static const struct of_device_id exynos_gpio_dt_match[] __initdata = {
- { .compatible = "samsung,exynos4-gpio", },
- {}
-};
-
-static __init void exynos_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,
- exynos_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 = 4;
- gc->of_xlate = exynos_gpio_xlate;
-}
-#elif defined(CONFIG_ARCH_EXYNOS)
-static __init void exynos_gpiolib_attach_ofnode(struct samsung_gpio_chip *chip,
- u64 base, u64 offset)
-{
- return;
-}
-#endif /* defined(CONFIG_ARCH_EXYNOS) && defined(CONFIG_OF) */
-
static __init void exynos4_gpiolib_init(void)
{
#ifdef CONFIG_CPU_EXYNOS4210
@@ -2746,8 +2682,6 @@ static __init void exynos4_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS4_PA_GPIO1, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos4_gpios_1,
nr_chips, gpio_base1);
@@ -2773,8 +2707,6 @@ static __init void exynos4_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS4_PA_GPIO2, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos4_gpios_2,
nr_chips, gpio_base2);
@@ -2794,8 +2726,6 @@ static __init void exynos4_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS4_PA_GPIO3, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos4_gpios_3,
nr_chips, gpio_base3);
@@ -2849,8 +2779,6 @@ static __init void exynos5_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS5_PA_GPIO1, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos5_gpios_1,
nr_chips, gpio_base1);
@@ -2870,8 +2798,6 @@ static __init void exynos5_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS5_PA_GPIO2, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos5_gpios_2,
nr_chips, gpio_base2);
@@ -2898,8 +2824,6 @@ static __init void exynos5_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS5_PA_GPIO3, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos5_gpios_3,
nr_chips, gpio_base3);
@@ -2919,8 +2843,6 @@ static __init void exynos5_gpiolib_init(void)
chip->config = &exynos_gpio_cfg;
chip->group = group++;
}
- exynos_gpiolib_attach_ofnode(chip,
- EXYNOS5_PA_GPIO4, i * 0x20);
}
samsung_gpiolib_add_4bit_chips(exynos5_gpios_4,
nr_chips, gpio_base4);
@@ -2945,29 +2867,13 @@ static __init int samsung_gpiolib_init(void)
int i, nr_chips;
int group = 0;

-#if defined(CONFIG_PINCTRL_EXYNOS) || defined(CONFIG_PINCTRL_EXYNOS5440)
/*
- * This gpio driver includes support for device tree support and there
- * are platforms using it. In order to maintain compatibility with those
- * platforms, and to allow non-dt Exynos4210 platforms to use this
- * gpiolib support, a check is added to find out if there is a active
- * pin-controller driver support available. If it is available, this
- * gpiolib support is ignored and the gpiolib support available in
- * pin-controller driver is used. This is a temporary check and will go
- * away when all of the Exynos4210 platforms have switched to using
- * device tree and the pin-ctrl driver.
- */
- struct device_node *pctrl_np;
- static const struct of_device_id exynos_pinctrl_ids[] = {
- { .compatible = "samsung,exynos4210-pinctrl", },
- { .compatible = "samsung,exynos4x12-pinctrl", },
- { .compatible = "samsung,exynos5440-pinctrl", },
- { }
- };
- for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
- if (pctrl_np && of_device_is_available(pctrl_np))
- return -ENODEV;
-#endif
+ * All Samsung platforms that support device tree will use the pinctrl
+ * driver, hence we skip initialization of this driver if the OF tree
+ * is available.
+ */
+ if (of_have_populated_dt())
+ return -ENODEV;

samsung_gpiolib_set_cfg(samsung_gpio_cfgs, ARRAY_SIZE(samsung_gpio_cfgs));

--
1.7.9.5

2013-04-17 16:33:42

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: samsung: Remove OF support for s3c24xx

On Tue, Apr 16, 2013 at 7:41 PM, Sylwester Nawrocki
<[email protected]> wrote:

> There is no users of this code and there is already a pinctrl
> driver written for s3c24xx which is going to be used on any
> s3c24xx DT platforms. Hence this has been effectively a dead
> code in mainline.
>
> This reverts commit 172c6a13653ac8cd6a231293b87c93821e90c1d6
> gpio: samsung: add devicetree init for s3c24xx arches
>
> Cc: Heiko St?bner <[email protected]>
> Signed-off-by: Sylwester Nawrocki <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>

Acked-by: Linus Walleij <[email protected]>

I guess this goes to go into the Samsung tree too.

Yours,
Linus Walleij

2013-04-17 16:35:37

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH RFC 2/2] gpio: samsung: Remove OF support for Exynos4/5

On Wed, Apr 17, 2013 at 1:06 PM, Sylwester Nawrocki
<[email protected]> wrote:

> -#endif
> + * All Samsung platforms that support device tree will use the pinctrl
> + * driver, hence we skip initialization of this driver if the OF tree
> + * is available.
> + */
> + if (of_have_populated_dt())
> + return -ENODEV;
>
> samsung_gpiolib_set_cfg(samsung_gpio_cfgs, ARRAY_SIZE(samsung_gpio_cfgs));

Oh is that how you do it? It looks fragile.

Can't you check if you have the pinctrl node atleast?

Yours,
Linus Walleij

2013-04-18 19:57:31

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH RFC 2/2] gpio: samsung: Remove OF support for Exynos4/5

On 04/17/2013 06:35 PM, Linus Walleij wrote:
> On Wed, Apr 17, 2013 at 1:06 PM, Sylwester Nawrocki
> <[email protected]> wrote:
>
>> -#endif
>> + * All Samsung platforms that support device tree will use the pinctrl
>> + * driver, hence we skip initialization of this driver if the OF tree
>> + * is available.
>> + */
>> + if (of_have_populated_dt())
>> + return -ENODEV;
>>
>> samsung_gpiolib_set_cfg(samsung_gpio_cfgs, ARRAY_SIZE(samsung_gpio_cfgs));
>
> Oh is that how you do it? It looks fragile.
>
> Can't you check if you have the pinctrl node atleast?

But this driver won't help if there is no pinctrl node found anyway, since
any OF support is removed from it.

This way there is no need for any additional DT parsing, just to skip
probing
of this legacy GPIO driver.

There might be something that could have gone wrong and I am missing though.

Thanks,
Sylwester