Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] gpio: omap: Honor "aliases" node
@ 2021-03-02  1:18 Alexander Sverdlin
  2021-03-02  9:27 ` Grygorii Strashko
  2021-03-02 16:21 ` Linus Walleij
  0 siblings, 2 replies; 5+ messages in thread
From: Alexander Sverdlin @ 2021-03-02  1:18 UTC (permalink / raw)
  To: linux-omap
  Cc: Alexander Sverdlin, Grygorii Strashko, Santosh Shilimkar,
	Kevin Hilman, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	linux-gpio, devicetree, linux-kernel

Currently the naming of the GPIO chips depends on their order in the DT,
but also on the kernel version (I've noticed the change from v5.10.x to
v5.11). Honor the persistent enumeration in the "aliases" node like other
GPIO drivers do.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Yes, I noticed checkpatch "WARNING: DT binding docs and includes should be
a separate patch."
However, the parts below are tiny and barely make sense separately.

 Documentation/devicetree/bindings/gpio/gpio-omap.txt | 6 ++++++
 drivers/gpio/gpio-omap.c                             | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
index e57b2cb28f6c..6050db3fd84e 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-omap.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
@@ -30,9 +30,15 @@ OMAP specific properties:
 - ti,gpio-always-on: 	Indicates if a GPIO bank is always powered and
 			so will never lose its logic state.
 
+Note: GPIO ports can have an alias correctly numbered in "aliases" node for
+persistent enumeration.
 
 Example:
 
+aliases {
+	gpio0 = &gpio0;
+};
+
 gpio0: gpio@44e07000 {
     compatible = "ti,omap4-gpio";
     reg = <0x44e07000 0x1000>;
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 41952bb818ad..dd2a8f6d920f 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1014,6 +1014,11 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
 			bank->chip.parent = &omap_mpuio_device.dev;
 		bank->chip.base = OMAP_MPUIO(0);
 	} else {
+#ifdef CONFIG_OF_GPIO
+		ret = of_alias_get_id(bank->chip.of_node, "gpio");
+		if (ret >= 0)
+			gpio = ret * bank->width;
+#endif
 		label = devm_kasprintf(bank->chip.parent, GFP_KERNEL, "gpio-%d-%d",
 				       gpio, gpio + bank->width - 1);
 		if (!label)
-- 
2.30.1


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

* Re: [PATCH] gpio: omap: Honor "aliases" node
  2021-03-02  1:18 [PATCH] gpio: omap: Honor "aliases" node Alexander Sverdlin
@ 2021-03-02  9:27 ` Grygorii Strashko
  2021-03-02 16:21 ` Linus Walleij
  1 sibling, 0 replies; 5+ messages in thread
From: Grygorii Strashko @ 2021-03-02  9:27 UTC (permalink / raw)
  To: Alexander Sverdlin, linux-omap
  Cc: Santosh Shilimkar, Kevin Hilman, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, linux-gpio, devicetree,
	linux-kernel



On 02/03/2021 03:18, Alexander Sverdlin wrote:
> Currently the naming of the GPIO chips depends on their order in the DT,
> but also on the kernel version (I've noticed the change from v5.10.x to
> v5.11). Honor the persistent enumeration in the "aliases" node like other
> GPIO drivers do.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
> Yes, I noticed checkpatch "WARNING: DT binding docs and includes should be
> a separate patch."
> However, the parts below are tiny and barely make sense separately.
> 
>   Documentation/devicetree/bindings/gpio/gpio-omap.txt | 6 ++++++
>   drivers/gpio/gpio-omap.c                             | 5 +++++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> index e57b2cb28f6c..6050db3fd84e 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> @@ -30,9 +30,15 @@ OMAP specific properties:
>   - ti,gpio-always-on: 	Indicates if a GPIO bank is always powered and
>   			so will never lose its logic state.
>   
> +Note: GPIO ports can have an alias correctly numbered in "aliases" node for
> +persistent enumeration.
>   
>   Example:
>   
> +aliases {
> +	gpio0 = &gpio0;
> +};
> +
>   gpio0: gpio@44e07000 {
>       compatible = "ti,omap4-gpio";
>       reg = <0x44e07000 0x1000>;
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 41952bb818ad..dd2a8f6d920f 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1014,6 +1014,11 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
>   			bank->chip.parent = &omap_mpuio_device.dev;
>   		bank->chip.base = OMAP_MPUIO(0);
>   	} else {
> +#ifdef CONFIG_OF_GPIO
> +		ret = of_alias_get_id(bank->chip.of_node, "gpio");
> +		if (ret >= 0)
> +			gpio = ret * bank->width;
> +#endif
>   		label = devm_kasprintf(bank->chip.parent, GFP_KERNEL, "gpio-%d-%d",
>   				       gpio, gpio + bank->width - 1);
>   		if (!label)
> 

You're not the first one, this was not accepted. See [1]
[1] https://patchwork.kernel.org/project/linux-omap/patch/1465898604-16294-1-git-send-email-u.kleine-koenig@pengutronix.de/


-- 
Best regards,
grygorii

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

* Re: [PATCH] gpio: omap: Honor "aliases" node
  2021-03-02  1:18 [PATCH] gpio: omap: Honor "aliases" node Alexander Sverdlin
  2021-03-02  9:27 ` Grygorii Strashko
@ 2021-03-02 16:21 ` Linus Walleij
  2021-03-08 18:37   ` Rob Herring
  1 sibling, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2021-03-02 16:21 UTC (permalink / raw)
  To: Alexander Sverdlin
  Cc: Linux-OMAP, Grygorii Strashko, Santosh Shilimkar, Kevin Hilman,
	Bartosz Golaszewski, Rob Herring, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel

On Tue, Mar 2, 2021 at 2:18 AM Alexander Sverdlin
<alexander.sverdlin@gmail.com> wrote:

> Currently the naming of the GPIO chips depends on their order in the DT,
> but also on the kernel version (I've noticed the change from v5.10.x to
> v5.11). Honor the persistent enumeration in the "aliases" node like other
> GPIO drivers do.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
> Yes, I noticed checkpatch "WARNING: DT binding docs and includes should be
> a separate patch."
> However, the parts below are tiny and barely make sense separately.

I've shut it down in the past because the instance ordering is a
linuxism and the needs are in the Linux userspace somehow.
It is different from a UART for example, which always need to
be at the same place on any operating system, hence it has an
alias.

For kernelspace the instance order should not matter, since
all resources are obtained from the device tree anyway
by phandle.

For userspace:
The way to determine topology in Linux userspace is to use sysfs,
and combined with the GPIO character device this provides a
unique ID for each GPIO chip and line on the system.

/sys/bus/gpio/devices/gpiochip0/
/sys/bus/gpio/devices/gpiochip1/

etc can change, but these appear as PCI, I2C, SPI, platform
etc nodes as well. On my PC:

/sys/devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.5/1-1.5:1.0/gpiochip0

It's pretty clear where that gpiochip sits.

Yours,
Linus Walleij

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

* Re: [PATCH] gpio: omap: Honor "aliases" node
  2021-03-02 16:21 ` Linus Walleij
@ 2021-03-08 18:37   ` Rob Herring
  2021-03-09 13:40     ` Linus Walleij
  0 siblings, 1 reply; 5+ messages in thread
From: Rob Herring @ 2021-03-08 18:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexander Sverdlin, Linux-OMAP, Grygorii Strashko,
	Santosh Shilimkar, Kevin Hilman, Bartosz Golaszewski,
	open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel

On Tue, Mar 02, 2021 at 05:21:23PM +0100, Linus Walleij wrote:
> On Tue, Mar 2, 2021 at 2:18 AM Alexander Sverdlin
> <alexander.sverdlin@gmail.com> wrote:
> 
> > Currently the naming of the GPIO chips depends on their order in the DT,
> > but also on the kernel version (I've noticed the change from v5.10.x to
> > v5.11). Honor the persistent enumeration in the "aliases" node like other
> > GPIO drivers do.
> >
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > ---
> > Yes, I noticed checkpatch "WARNING: DT binding docs and includes should be
> > a separate patch."
> > However, the parts below are tiny and barely make sense separately.
> 
> I've shut it down in the past because the instance ordering is a
> linuxism and the needs are in the Linux userspace somehow.
> It is different from a UART for example, which always need to
> be at the same place on any operating system, hence it has an
> alias.
> 
> For kernelspace the instance order should not matter, since
> all resources are obtained from the device tree anyway
> by phandle.

Thank you!

Can we remove the ones we have already for GPIO? 

BTW, It's been on my todo list for a while to start requiring 
documentation of alias names so we can reject new ones and get rid of 
some of the unused existing ones. Some platforms have numbered 
everything...

Rob

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

* Re: [PATCH] gpio: omap: Honor "aliases" node
  2021-03-08 18:37   ` Rob Herring
@ 2021-03-09 13:40     ` Linus Walleij
  0 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2021-03-09 13:40 UTC (permalink / raw)
  To: Rob Herring, open list:GPIO SUBSYSTEM
  Cc: Alexander Sverdlin, Linux-OMAP, Grygorii Strashko,
	Santosh Shilimkar, Kevin Hilman, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel

On Mon, Mar 8, 2021 at 7:37 PM Rob Herring <robh@kernel.org> wrote:

> Can we remove the ones we have already for GPIO?

I think we would get pretty hard pushback if we attempt that.
We have all these drivers that utilize it:

gpio-clps711x.c:        id = of_alias_get_id(np, "gpio");
gpio-mvebu.c:   id = of_alias_get_id(pdev->dev.of_node, "gpio");
gpio-mxc.c:     port->gc.base = (pdev->id < 0) ? of_alias_get_id(np,
"gpio") * 32 :
gpio-mxs.c:     port->id = of_alias_get_id(np, "gpio");
gpio-vf610.c:   gc->base = of_alias_get_id(np, "gpio") * VF610_GPIO_PER_PORT;
gpio-zynq.c:    chip->base = of_alias_get_id(pdev->dev.of_node, "gpio");
pinctrl-at91.c: int alias_idx = of_alias_get_id(np, "gpio");
pinctrl-st.c:   int bank_num = of_alias_get_id(np, "gpio");
samsung/pinctrl-samsung.c:      id = of_alias_get_id(node, "pinctrl");

Predictably it is so many bad examples that new driver authors will claim
something along the line of
"why can't I have a lollipop when all other kids got one".

Several of those have this by a claim one way or another that
the DT boot need to look like the boardfile boot. Some of these
have been migrated from board files so could possible drop
this id/base coding.

I don't know what the maintainers would say, should we send
attack patches? :D At least some kind of motivation would come
out of it.

Yours,
Linus Walleij

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02  1:18 [PATCH] gpio: omap: Honor "aliases" node Alexander Sverdlin
2021-03-02  9:27 ` Grygorii Strashko
2021-03-02 16:21 ` Linus Walleij
2021-03-08 18:37   ` Rob Herring
2021-03-09 13:40     ` Linus Walleij

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git