Skip to contentSkip to conversation

gpio: exynos4: Add device tree support

PATCH New Thomas Abraham 4 participants 11 messages Oct 11, 2011 – Oct 13, 2011

Subsystems (3)
Device tree bindings GPIO Arm Samsung Exynos
lore
Share & tools
lore

Thomas Abraham

author
#0 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
https://github.com/kgene/linux-samsung.git branch: for-next

 .../devicetree/bindings/gpio/gpio-samsung.txt      |   30 +++++++++++
 drivers/gpio/gpio-samsung.c                        |   53 ++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +30 -0
new file mode 100644
index 0000000..883faeb
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+  "samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+  compatible property value should be "samsung,exynos4-gpio-gpa0".
+
+- reg: Physical base address of the controller and length of memory mapped region.
+
+- #gpio-cells: Should be 4. 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] [pull up/down] [drive strength]>
+
+- gpio-controller: Specifies that the node is a gpio controller.
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 1. 
+
+Example:
+
+	gpa0: gpio-controller@11400000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "samsung,exynos4-gpio-gpa0";
+		reg = <0x11400000 0x20>;
+		#gpio-cells = <4>;
+		gpio-controller;
+	};
drivers/gpio/gpio-samsung.c +53 -0

83 lines not on this page · raw message

Signature · 1 line
1.6.6.rc2
#1 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
https://github.com/kgene/linux-samsung.git branch: for-next

 .../devicetree/bindings/gpio/gpio-samsung.txt      |   30 +++++++++++
 drivers/gpio/gpio-samsung.c                        |   53 ++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+  "samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+  compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 13

Quoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +7 -0
+
+- reg: Physical base address of the controller and length of memory mapped region.
+
+- #gpio-cells: Should be 4. 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] [pull up/down] [drive strength]>

It would be better to list out the values here.

Quoted 22 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +17 -0
+
+- gpio-controller: Specifies that the node is a gpio controller.
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 1. 
+
+Example:
+
+	gpa0: gpio-controller at 11400000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "samsung,exynos4-gpio-gpa0";
+		reg = <0x11400000 0x20>;
+		#gpio-cells = <4>;
+		gpio-controller;
+	};
drivers/gpio/gpio-samsung.c +0 -0
index b6be77a..037d3bb 100644

drivers/gpio/gpio-samsung.c line 30

Quoted 7 lines
drivers/gpio/gpio-samsung.c +4 -0
@@ -24,6 +24,10 @@
 #include <linux/interrupt.h>
 #include <linux/sysdev.h>
 #include <linux/ioport.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/slab.h>
+#endif

Don't need the ifdef here.

drivers/gpio/gpio-samsung.c static struct samsung_gpio_chip exynos4_gpios_3[] = { · line 2361

Quoted 9 lines
drivers/gpio/gpio-samsung.c +2 -0
 #include <asm/irq.h>
 

@@ -2353,6 +2357,52 @@ static struct samsung_gpio_chip exynos4_gpios_3[] = {
 #endif
 };
 
+#if defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF)
+int exynos4_gpio_xlate(struct gpio_chip *gc, struct device_node *np,

static

Quoted 64 lines
drivers/gpio/gpio-samsung.c +46 -0
+			 const void *gpio_spec, u32 *flags)
+{
+	const __be32 *gpio = gpio_spec;
+	const u32 n = be32_to_cpup(gpio);
+	unsigned int pin = gc->base + be32_to_cpu(gpio[0]);
+
+	if (gc->of_gpio_n_cells < 4) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	if (n > gc->ngpio)
+		return -EINVAL;
+
+	s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(be32_to_cpu(gpio[1])));
+	s3c_gpio_setpull(pin, be32_to_cpu(gpio[2]));
+	s5p_gpio_set_drvstr(pin, be32_to_cpu(gpio[3]));
+	return n;
+}
+
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *gc)
+{
+	const char exynos4_gpio_compat_base[] = "samsung,exynos4-gpio-";
+	char *exynos4_gpio_compat;
+
+	exynos4_gpio_compat = kzalloc(strlen(exynos4_gpio_compat_base) +
+				strlen(gc->label), GFP_KERNEL);
+	if (!exynos4_gpio_compat)
+		return;
+
+	strcpy(exynos4_gpio_compat, exynos4_gpio_compat_base);
+	strcat(exynos4_gpio_compat, gc->label);
+	gc->of_node = of_find_compatible_node(NULL, NULL, exynos4_gpio_compat);
+	gc->of_gpio_n_cells = 4;
+	gc->of_xlate = exynos4_gpio_xlate;
+	kfree(exynos4_gpio_compat);
+}
+#else
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *chip)
+{
+	return;
+}
+#endif /* defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF) */
+
 /* TODO: cleanup soc_is_* */
 static __init int samsung_gpiolib_init(void)
 {

@@ -2434,6 +2484,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_1, nr_chips, S5P_VA_GPIO1);
 

@@ -2446,6 +2497,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_2, nr_chips, S5P_VA_GPIO2);

drivers/gpio/gpio-samsung.c static __init int samsung_gpiolib_init(void) · line 2516

Quoted 7 lines
drivers/gpio/gpio-samsung.c +1 -0
@@ -2458,6 +2510,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_3, nr_chips, S5P_VA_GPIO3);

This code is really ugly, but I guess you inherited it. Converting to a platform driver and using id table would be much cleaner.

Rob

author
#2 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Hi Rob,

Quoted 19 lines · Rob Herring

Thomas,

Quoted 16 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
? https://github.com/kgene/linux-samsung.git ?branch: for-next

?.../devicetree/bindings/gpio/gpio-samsung.txt ? ? ?| ? 30 +++++++++++ ?drivers/gpio/gpio-samsung.c ? ? ? ? ? ? ? ? ? ? ? ?| ? 53 ++++++++++++++++++++ ?2 files changed, 83 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Quoted 14 lines

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 12 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
new file mode 100644
index 0000000..883faeb
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+ ?"samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+ ?compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

Quoted 10 lines

Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Quoted 8 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +7 -0
+
+- reg: Physical base address of the controller and length of memory mapped region.
+
+- #gpio-cells: Should be 4. 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] [pull up/down] [drive strength]>

It would be better to list out the values here.

Ok. I will do that in the next version of the patch.

Quoted 24 lines
Quoted 24 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +17 -0
+
+- gpio-controller: Specifies that the node is a gpio controller.
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 1.
+
+Example:
+
+ ? ? gpa0: gpio-controller at 11400000 {
+ ? ? ? ? ? ? #address-cells = <1>;
+ ? ? ? ? ? ? #size-cells = <1>;
+ ? ? ? ? ? ? compatible = "samsung,exynos4-gpio-gpa0";
+ ? ? ? ? ? ? reg = <0x11400000 0x20>;
+ ? ? ? ? ? ? #gpio-cells = <4>;
+ ? ? ? ? ? ? gpio-controller;
+ ? ? };
drivers/gpio/gpio-samsung.c +0 -0
index b6be77a..037d3bb 100644
@@ -24,6 +24,10 @@

?#include <linux/interrupt.h>

Quoted 8 lines

drivers/gpio/gpio-samsung.c line 30

Quoted 6 lines

?#include <linux/sysdev.h>
?#include <linux/ioport.h>

drivers/gpio/gpio-samsung.c +4 -0
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/slab.h>
+#endif

Don't need the ifdef here.

Ok. I will remove it.

Quoted 4 lines
Quoted 4 lines

?#include <asm/irq.h>

Quoted 8 lines

drivers/gpio/gpio-samsung.c static struct samsung_gpio_chip exynos4_gpios_3[] = { · line 2361

Quoted 6 lines
drivers/gpio/gpio-samsung.c +0 -0
@@ -2353,6 +2357,52 @@ static struct samsung_gpio_chip exynos4_gpios_3[] = {

?#endif
?};

drivers/gpio/gpio-samsung.c +2 -0
+#if defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF)
+int exynos4_gpio_xlate(struct gpio_chip *gc, struct device_node *np,

static

Ok. I will add it.

Quoted 64 lines
Quoted 64 lines

+ ? ? ? ? ? ? ? ? ? ? ?const void *gpio_spec, u32 *flags)

drivers/gpio/gpio-samsung.c +43 -0
+{
+ ? ? const __be32 *gpio = gpio_spec;
+ ? ? const u32 n = be32_to_cpup(gpio);
+ ? ? unsigned int pin = gc->base + be32_to_cpu(gpio[0]);
+
+ ? ? if (gc->of_gpio_n_cells < 4) {
+ ? ? ? ? ? ? WARN_ON(1);
+ ? ? ? ? ? ? return -EINVAL;
+ ? ? }
+
+ ? ? if (n > gc->ngpio)
+ ? ? ? ? ? ? return -EINVAL;
+
+ ? ? s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(be32_to_cpu(gpio[1])));
+ ? ? s3c_gpio_setpull(pin, be32_to_cpu(gpio[2]));
+ ? ? s5p_gpio_set_drvstr(pin, be32_to_cpu(gpio[3]));
+ ? ? return n;
+}
+
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *gc)
+{
+ ? ? const char exynos4_gpio_compat_base[] = "samsung,exynos4-gpio-";
+ ? ? char *exynos4_gpio_compat;
+
+ ? ? exynos4_gpio_compat = kzalloc(strlen(exynos4_gpio_compat_base) +
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? strlen(gc->label), GFP_KERNEL);
+ ? ? if (!exynos4_gpio_compat)
+ ? ? ? ? ? ? return;
+
+ ? ? strcpy(exynos4_gpio_compat, exynos4_gpio_compat_base);
+ ? ? strcat(exynos4_gpio_compat, gc->label);
+ ? ? gc->of_node = of_find_compatible_node(NULL, NULL, exynos4_gpio_compat);
+ ? ? gc->of_gpio_n_cells = 4;
+ ? ? gc->of_xlate = exynos4_gpio_xlate;
+ ? ? kfree(exynos4_gpio_compat);
+}
+#else
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *chip)
+{
+ ? ? return;
+}
+#endif /* defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF) */
+

?/* TODO: cleanup soc_is_* */
?static __init int samsung_gpiolib_init(void)
?{

drivers/gpio/gpio-samsung.c +2 -0
@@ -2434,6 +2484,7 @@ static __init int samsung_gpiolib_init(void)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->config = &exynos4_gpio_cfg;
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->group = group++;
 ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? exynos4_gpiolib_attach_ofnode(&chip->chip);
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? samsung_gpiolib_add_4bit_chips(exynos4_gpios_1, nr_chips, S5P_VA_GPIO1);
 

@@ -2446,6 +2497,7 @@ static __init int samsung_gpiolib_init(void)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->config = &exynos4_gpio_cfg;
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->group = group++;
 ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? exynos4_gpiolib_attach_ofnode(&chip->chip);
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? samsung_gpiolib_add_4bit_chips(exynos4_gpios_2, nr_chips, S5P_VA_GPIO2);
Quoted 11 lines

drivers/gpio/gpio-samsung.c static __init int samsung_gpiolib_init(void) · line 2516

Quoted 8 lines
drivers/gpio/gpio-samsung.c +1 -0
@@ -2458,6 +2510,7 @@ static __init int samsung_gpiolib_init(void)
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->config = &exynos4_gpio_cfg;
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chip->group = group++;
 ? ? ? ? ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? ? ? ? exynos4_gpiolib_attach_ofnode(&chip->chip);
 ? ? ? ? ? ? ? }
 ? ? ? ? ? ? ? samsung_gpiolib_add_4bit_chips(exynos4_gpios_3, nr_chips, S5P_VA_GPIO3);

This code is really ugly, but I guess you inherited it. Converting to a platform driver and using id table would be much cleaner.

Ok. I wanted to get the initial dt support for gpio controller on Exynos started because dt support for other controllers depend on this. I will revisit this as per your suggestion during the 3.2 cycle.

Thanks for reviewing this patch.

Regards,
Thomas

Quoted 3 lines

Rob

#3 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 19 lines · Rob Herring

Thomas,

Quoted 16 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
https://github.com/kgene/linux-samsung.git branch: for-next

 .../devicetree/bindings/gpio/gpio-samsung.txt      |   30 +++++++++++
 drivers/gpio/gpio-samsung.c                        |   53 ++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt
Quoted 14 lines

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 12 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
new file mode 100644
index 0000000..883faeb
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+  "samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+  compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

Rob

author
#4 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 38 lines · Rob Herring
Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 33 lines · Rob Herring

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
? https://github.com/kgene/linux-samsung.git ?branch: for-next

?.../devicetree/bindings/gpio/gpio-samsung.txt ? ? ?| ? 30 +++++++++++ ?drivers/gpio/gpio-samsung.c ? ? ? ? ? ? ? ? ? ? ? ?| ? 53 ++++++++++++++++++++ ?2 files changed, 83 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+ ?"samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+ ?compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 8 lines
Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

No, that does not affect the programming of the controller. The reason for the instance name extension in compatible string is to match the gpio_chip with a gpio controller node. When matched, the of_node pointer of the gpio_chip is set to point to that controller node.

This might not be the best possible implementation but the device tree support code in this patch is dictated by the structure of the existing gpio driver code. As you suggested previously, I will look at reworking the gpio driver a little later but for now, there was a need for working gpio dt solution to make progress on dt support for other controllers.

Thanks,
Thomas.

Quoted 4 lines

Rob

#5 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 54 lines · Thomas Abraham
Quoted 38 lines · Rob Herring
Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 33 lines · Rob Herring

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
https://github.com/kgene/linux-samsung.git branch: for-next

 .../devicetree/bindings/gpio/gpio-samsung.txt      |   30 +++++++++++
 drivers/gpio/gpio-samsung.c                        |   53 ++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+  "samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+  compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 8 lines
Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

No, that does not affect the programming of the controller. The reason for the instance name extension in compatible string is to match the gpio_chip with a gpio controller node. When matched, the of_node pointer of the gpio_chip is set to point to that controller node.

Quoted 6 lines

This might not be the best possible implementation but the device tree support code in this patch is dictated by the structure of the existing gpio driver code. As you suggested previously, I will look at reworking the gpio driver a little later but for now, there was a need for working gpio dt solution to make progress on dt support for other controllers.

Linux should provide clues about what's needed in a binding, but the binding should not be defined based on current Linux code. Doing the binding one way and changing it later is not a good plan.

I think you need to convert all users of gpio over as well so you can move to dynamic gpio_chip creation and gpio numbering. Or maybe you can match based on base address? This is going to be a common problem as gpio is converted over to DT. Perhaps Grant or others have suggestions on the approach to use.

Rob

author
#6 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 62 lines · Rob Herring
Quoted 54 lines · Thomas Abraham
Quoted 38 lines · Rob Herring
Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 33 lines · Rob Herring

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
? https://github.com/kgene/linux-samsung.git ?branch: for-next

?.../devicetree/bindings/gpio/gpio-samsung.txt ? ? ?| ? 30 +++++++++++ ?drivers/gpio/gpio-samsung.c ? ? ? ? ? ? ? ? ? ? ? ?| ? 53 ++++++++++++++++++++ ?2 files changed, 83 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+ ?"samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+ ?compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 8 lines
Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

No, that does not affect the programming of the controller. The reason for the instance name extension in compatible string is to match the gpio_chip with a gpio controller node. When matched, the of_node pointer of the gpio_chip is set to point to that controller node.

Quoted 6 lines

This might not be the best possible implementation but the device tree support code in this patch is dictated by the structure of the existing gpio driver code. As you suggested previously, I will look at reworking the gpio driver a little later but for now, there was a need for working gpio dt solution to make progress on dt support for other controllers.

Quoted 3 lines

Linux should provide clues about what's needed in a binding, but the binding should not be defined based on current Linux code. Doing the binding one way and changing it later is not a good plan.

Ok. When starting on this, two compatible values where used for the gpio controllers, one was "samsung,exynos4-gpio" and another was "samsung,exynos4-gpio-<ctrl_name>". And when the gpio dt support would mature, the second compatible value could be dropped. Non-linux platforms could always use the generic "samsung,exynos4-gpio" compatible value to match. I moved to using only "samsung,exynos4-gpio-<ctrl_name>" just before sending this patch because I was not sure of the right approach.

Quoted 6 lines

I think you need to convert all users of gpio over as well so you can move to dynamic gpio_chip creation and gpio numbering. Or maybe you can match based on base address? This is going to be a common problem as gpio is converted over to DT. Perhaps Grant or others have suggestions on the approach to use.

Yes, I agree with you about the dynamic gpio_chip creation and gpio numbering. Probably, I should have focussed more on this before moving to dt support for other controllers.

But matching based on base address is still an option if that is better than matching with compatible values as defined currently. The 'struct samsung_gpio_chip' which encapsulates 'struct gpio_chip' has information about the base address of the gpio controller. The match of gpio device node with 'struct gpio_chip' can be quickly moved over to base address matching. Would this be better than the current approach?

Thank you for your comments Rob.

Regards,
Thomas.

Quoted 3 lines

Rob

#7 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 87 lines · Thomas Abraham
Quoted 62 lines · Rob Herring
Quoted 54 lines · Thomas Abraham
Quoted 38 lines · Rob Herring
Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 33 lines · Rob Herring

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
? https://github.com/kgene/linux-samsung.git ?branch: for-next

?.../devicetree/bindings/gpio/gpio-samsung.txt ? ? ?| ? 30 +++++++++++ ?drivers/gpio/gpio-samsung.c ? ? ? ? ? ? ? ? ? ? ? ?| ? 53 ++++++++++++++++++++ ?2 files changed, 83 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+ ?"samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+ ?compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 8 lines
Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

No, that does not affect the programming of the controller. The reason for the instance name extension in compatible string is to match the gpio_chip with a gpio controller node. When matched, the of_node pointer of the gpio_chip is set to point to that controller node.

Quoted 6 lines

This might not be the best possible implementation but the device tree support code in this patch is dictated by the structure of the existing gpio driver code. As you suggested previously, I will look at reworking the gpio driver a little later but for now, there was a need for working gpio dt solution to make progress on dt support for other controllers.

Quoted 3 lines

Linux should provide clues about what's needed in a binding, but the binding should not be defined based on current Linux code. Doing the binding one way and changing it later is not a good plan.

Ok. When starting on this, two compatible values where used for the gpio controllers, one was "samsung,exynos4-gpio" and another was "samsung,exynos4-gpio-<ctrl_name>". And when the gpio dt support would mature, the second compatible value could be dropped. Non-linux platforms could always use the generic "samsung,exynos4-gpio" compatible value to match. I moved to using only "samsung,exynos4-gpio-<ctrl_name>" just before sending this patch because I was not sure of the right approach.

Quoted 6 lines

I think you need to convert all users of gpio over as well so you can move to dynamic gpio_chip creation and gpio numbering. Or maybe you can match based on base address? This is going to be a common problem as gpio is converted over to DT. Perhaps Grant or others have suggestions on the approach to use.

Yes, I agree with you about the dynamic gpio_chip creation and gpio numbering. Probably, I should have focussed more on this before moving to dt support for other controllers.

Quoted 7 lines

But matching based on base address is still an option if that is better than matching with compatible values as defined currently. The 'struct samsung_gpio_chip' which encapsulates 'struct gpio_chip' has information about the base address of the gpio controller. The match of gpio device node with 'struct gpio_chip' can be quickly moved over to base address matching. Would this be better than the current approach?

That's what I would do.

g.

author
#8 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 96 lines · Grant Likely
Quoted 87 lines · Thomas Abraham
Quoted 62 lines · Rob Herring
Quoted 54 lines · Thomas Abraham
Quoted 38 lines · Rob Herring
Quoted 37 lines · Thomas Abraham

Hi Rob,

Quoted 33 lines · Rob Herring

Thomas,

Quoted 22 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver available in the following tree:
? https://github.com/kgene/linux-samsung.git ?branch: for-next

?.../devicetree/bindings/gpio/gpio-samsung.txt ? ? ?| ? 30 +++++++++++ ?drivers/gpio/gpio-samsung.c ? ? ? ? ? ? ? ? ? ? ? ?| ? 53 ++++++++++++++++++++ ?2 files changed, 83 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +0 -0
new file mode 100644
index 0000000..883faeb

Documentation/devicetree/bindings/gpio/gpio-samsung.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +6 -0
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+ ?"samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller, the
+ ?compatible property value should be "samsung,exynos4-gpio-gpa0".

Isn't gpa0 an instance of the h/w, not a version?

Quoted 8 lines
Quoted 4 lines

GPA0 is a instance of the gpio controller. There are several such instances and there could be differences in those instances such as the number of GPIO lines managed by that gpio controller instance.

That doesn't seem like a reason to have different compatible strings. Does that affect the programming model of the controller? Unused lines whether at the board level or SOC level shouldn't really matter.

No, that does not affect the programming of the controller. The reason for the instance name extension in compatible string is to match the gpio_chip with a gpio controller node. When matched, the of_node pointer of the gpio_chip is set to point to that controller node.

Quoted 6 lines

This might not be the best possible implementation but the device tree support code in this patch is dictated by the structure of the existing gpio driver code. As you suggested previously, I will look at reworking the gpio driver a little later but for now, there was a need for working gpio dt solution to make progress on dt support for other controllers.

Quoted 3 lines

Linux should provide clues about what's needed in a binding, but the binding should not be defined based on current Linux code. Doing the binding one way and changing it later is not a good plan.

Ok. When starting on this, two compatible values where used for the gpio controllers, one was "samsung,exynos4-gpio" and another was "samsung,exynos4-gpio-<ctrl_name>". And when the gpio dt support would mature, the second compatible value could be dropped. Non-linux platforms could always use the generic "samsung,exynos4-gpio" compatible value to match. I moved to using only "samsung,exynos4-gpio-<ctrl_name>" just before sending this patch because I was not sure of the right approach.

Quoted 6 lines

I think you need to convert all users of gpio over as well so you can move to dynamic gpio_chip creation and gpio numbering. Or maybe you can match based on base address? This is going to be a common problem as gpio is converted over to DT. Perhaps Grant or others have suggestions on the approach to use.

Yes, I agree with you about the dynamic gpio_chip creation and gpio numbering. Probably, I should have focussed more on this before moving to dt support for other controllers.

Quoted 7 lines

But matching based on base address is still an option if that is better than matching with compatible values as defined currently. The 'struct samsung_gpio_chip' which encapsulates 'struct gpio_chip' has information about the base address of the gpio controller. The match of gpio device node with 'struct gpio_chip' can be quickly moved over to base address matching. Would this be better than the current approach?

Quoted 1 line

That's what I would do.

Thanks Grant. I will modify the patch to match against the base address of the controller.

Regards,
Thomas.

Quoted 4 lines

g.

#9 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

In reply to Thomas Abraham
Quoted 9 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>

---
This patch is based on the latest consolidated Samsung GPIO driver

available

Quoted 8 lines

in the following tree:
https://github.com/kgene/linux-samsung.git branch: for-next

 .../devicetree/bindings/gpio/gpio-samsung.txt      |   30 +++++++++++
 drivers/gpio/gpio-samsung.c                        |   53
++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644

Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Quoted 14 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +5 -0
new file mode 100644
index 0000000..883faeb
@@ -0,0 +1,30 @@
+Samsung Exynos4 GPIO Controller
+
+Required properties:
+- compatible: Format of compatible property value should be
+  "samsung,exynos4-gpio-<controller_name>". Example: For GPA0 controller,

the

Documentation/devicetree/bindings/gpio/gpio-samsung.txt

Quoted 3 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +3 -0
+  compatible property value should be "samsung,exynos4-gpio-gpa0".
+
+- reg: Physical base address of the controller and length of memory

mapped

Quoted 3 lines

region.
+
+- #gpio-cells: Should be 4. The syntax of the gpio specifier used by

client nodes

Quoted 119 lines
Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +2 -0
+  should be the following with values derived from the SoC user manual.
+     <[phandle of the gpio controller node] <pin number within the gpio

controller]

Documentation/devicetree/bindings/gpio/gpio-samsung.txt new file +18 -0
+      [mux function] [pull up/down] [drive strength]>
+
+- gpio-controller: Specifies that the node is a gpio controller.
+
+- #address-cells: should be 1.
+
+- #size-cells: should be 1.
+
+Example:
+
+	gpa0: gpio-controller at 11400000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "samsung,exynos4-gpio-gpa0";
+		reg = <0x11400000 0x20>;
+		#gpio-cells = <4>;
+		gpio-controller;
+	};
drivers/gpio/gpio-samsung.c +50 -0
index b6be77a..037d3bb 100644
@@ -24,6 +24,10 @@
 #include <linux/interrupt.h>
 #include <linux/sysdev.h>
 #include <linux/ioport.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/slab.h>
+#endif
 
 #include <asm/irq.h>
 

@@ -2353,6 +2357,52 @@ static struct samsung_gpio_chip exynos4_gpios_3[] =
 {
 #endif
 };
 
+#if defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF)
+int exynos4_gpio_xlate(struct gpio_chip *gc, struct device_node *np,
+			 const void *gpio_spec, u32 *flags)
+{
+	const __be32 *gpio = gpio_spec;
+	const u32 n = be32_to_cpup(gpio);
+	unsigned int pin = gc->base + be32_to_cpu(gpio[0]);
+
+	if (gc->of_gpio_n_cells < 4) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	if (n > gc->ngpio)
+		return -EINVAL;
+
+	s3c_gpio_cfgpin(pin, S3C_GPIO_SFN(be32_to_cpu(gpio[1])));
+	s3c_gpio_setpull(pin, be32_to_cpu(gpio[2]));
+	s5p_gpio_set_drvstr(pin, be32_to_cpu(gpio[3]));
+	return n;
+}
+
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *gc)
+{
+	const char exynos4_gpio_compat_base[] = "samsung,exynos4-gpio-";
+	char *exynos4_gpio_compat;
+
+	exynos4_gpio_compat = kzalloc(strlen(exynos4_gpio_compat_base) +
+				strlen(gc->label), GFP_KERNEL);
+	if (!exynos4_gpio_compat)
+		return;
+
+	strcpy(exynos4_gpio_compat, exynos4_gpio_compat_base);
+	strcat(exynos4_gpio_compat, gc->label);
+	gc->of_node = of_find_compatible_node(NULL, NULL,
 exynos4_gpio_compat);
+	gc->of_gpio_n_cells = 4;
+	gc->of_xlate = exynos4_gpio_xlate;
+	kfree(exynos4_gpio_compat);
+}
+#else
+static __init void exynos4_gpiolib_attach_ofnode(struct gpio_chip *chip)
+{
+	return;
+}
+#endif /* defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF) */
+
 /* TODO: cleanup soc_is_* */

static __init int samsung_gpiolib_init(void)
{

drivers/gpio/gpio-samsung.c +3 -1
@@ -2434,6 +2484,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_1, nr_chips,
 S5P_VA_GPIO1);

@@ -2446,6 +2497,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_2, nr_chips,
 S5P_VA_GPIO2);

@@ -2458,6 +2510,7 @@ static __init int samsung_gpiolib_init(void)
 				chip->config = &exynos4_gpio_cfg;
 				chip->group = group++;
 			}
+			exynos4_gpiolib_attach_ofnode(&chip->chip);
 		}
 		samsung_gpiolib_add_4bit_chips(exynos4_gpios_3, nr_chips,
 S5P_VA_GPIO3);
 
--
Quoted 1 line

1.6.6.rc2

Hi Thomas,

You got some comments from Rob, so could you please address from him? Others, looks ok to me.

And I need to get the ack from Grant to upstream via Samsung tree for upcoming merge window.
Grant, is this ok for you?

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#10 lore raw

PATCH 1/1 gpio: exynos4: Add device tree support

Quoted 9 lines · Kukjin Kim
Quoted 7 lines · Thomas Abraham

As gpio chips get registered, a device tree node which represents the gpio chip is searched and attached to it. A translate function is also provided to convert the gpio specifier into actual platform settings for pin function selection, pull up/down and driver strength settings.

Signed-off-by Thomas Abraham <[email protected]>
Quoted 8 lines

Hi Thomas,

You got some comments from Rob, so could you please address from him? Others, looks ok to me.

And I need to get the ack from Grant to upstream via Samsung tree for upcoming merge window.
Grant, is this ok for you?

Yes, go ahead and merge it with my:

Acked-by Grant Likely <[email protected]>

g.