[PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

The i.MX GPIO driver has had deterministic numbering for the GPIOs
for more than 12 years.

Reverting this to dynamically numbered will break existing setups in the
worst manner possible: The build will succeed, the kernel will not print
warnings, but users will find their devices essentially toggling GPIOs
at random with the potential of permanent damage. We thus want to keep
the numbering as-is until the SysFS API is removed and script fail
instead of toggling GPIOs dependent on probe order.

Yet, the warning:

  gpio gpiochip0: Static allocation of GPIO base is deprecated,
  use dynamic allocation.

is annoying and prompts people to set the base to -1 from time to time.

Let's workaround this by adding an opt-out for existing drivers and have
i.MX make use of it.

---
Ahmad Fatoum (4):
      gpiolib: add opt-out for existing drivers with static GPIO base
      checkpatch: warn about use of legacy_static_base
      gpio: mxc: remove dead code after switch to DT-only
      gpio: mxc: silence warning about GPIO base being statically allocated

 drivers/gpio/gpio-mxc.c     | 23 +++++++++++++++++++++--
 drivers/gpio/gpiolib.c      |  2 +-
 include/linux/gpio/driver.h |  5 +++++
 scripts/checkpatch.pl       | 11 +++++++++--
 4 files changed, 36 insertions(+), 5 deletions(-)
---
base-commit: 37136bf5c3a6f6b686d74f41837a6406bec6b7bc
change-id: 20250113-b4-imx-gpio-base-warning-4f9ae89887d0

Best regards,
-- 
Ahmad Fatoum <a.fatoum@pengutronix.de>

[PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Ahmad Fatoum]

Some drivers have had deterministic GPIO numbering for most of
their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
("gpio/mxc: specify gpio base for device tree probe"), more than
12 years ago.

Reverting this to dynamically numbered will break existing setups in
the worst manner possible: The build will succeed, the kernel will not
print warnings, but users will find their devices essentially toggling
GPIOs at random with the potential of permanent damage.

As these concerns won't go away until the sysfs interface is removed,
let's add a new struct gpio_chip::legacy_static_base member that can be
used by existing drivers that have been grandfathered in to suppress
the warning currently being printed:

  gpio gpiochip0: Static allocation of GPIO base is deprecated,
  use dynamic allocation.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c      | 2 +-
 include/linux/gpio/driver.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 679ed764cb143c4b3357106de1570e8d38441372..bedeb8f28badfb7287c4929f9ad0825e050a79c9 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1011,7 +1011,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 			 * drop this and assign a poison instead.
 			 */
 			gc->base = base;
-		} else {
+		} else if (!gc->legacy_static_base) {
 			dev_warn(&gdev->dev,
 				 "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
 		}
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 2dd7cb9cc270a68ddedbcdd5d44e0d0f88dfa785..6e820d79d03e61123f89aaf884d35d4a1a5918a7 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -382,6 +382,10 @@ struct gpio_irq_chip {
  *	implies that if the chip supports IRQs, these IRQs need to be threaded
  *	as the chip access may sleep when e.g. reading out the IRQ status
  *	registers.
+ * @legacy_static_base: set for some existing drivers, where @base is non-negative
+ *	and changing that would induce so much breakage that they were
+ *	grandfathered in until GPIO sysfs support is removed altogether.
+ *	Do not set this for any new drivers.
  * @read_reg: reader function for generic GPIO
  * @write_reg: writer function for generic GPIO
  * @be_bits: if the generic GPIO has big endian bit order (bit 31 is representing
@@ -467,6 +471,7 @@ struct gpio_chip {
 	u16			offset;
 	const char		*const *names;
 	bool			can_sleep;
+	bool			legacy_static_base;
 
 #if IS_ENABLED(CONFIG_GPIO_GENERIC)
 	unsigned long (*read_reg)(void __iomem *reg);

-- 
2.39.5

[PATCH 2/4] checkpatch: warn about use of legacy_static_base

[Ahmad Fatoum]

gpio_chip::legacy_state_base was recently added as opt-out for
existing drivers and shouldn't be used for new drivers. It's thus
sensible to add a deprecation warning whenever it's used.

This doesn't fit with the existing deprecated API check, because it
requires a `(' to follow the symbol name, so a new member specific
pattern is introduced instead.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 scripts/checkpatch.pl | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad3317427a6bf9e27b77065aa3915cb13053..6c57a08833a4298573c7967b2a178fd7f46aa7ce 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -848,6 +848,13 @@ foreach my $entry (keys %deprecated_apis) {
 }
 $deprecated_apis_search = "(?:${deprecated_apis_search})";
 
+our %deprecated_members = (
+	"legacy_static_base"			=> "setting .base to -1",
+);
+
+our $deprecated_memb_search = "(?:" . join("|", keys %deprecated_members) . ")";
+%deprecated_apis = (%deprecated_apis, %deprecated_members);
+
 our $mode_perms_world_writable = qr{
 	S_IWUGO		|
 	S_IWOTH		|
@@ -7407,8 +7414,8 @@ sub process {
 		}
 
 # check for deprecated apis
-		if ($line =~ /\b($deprecated_apis_search)\b\s*\(/) {
-			my $deprecated_api = $1;
+		if ($line =~ /\b(?<old>$deprecated_apis_search)\b\s*\(|(?:->|\.)(?<old>$deprecated_memb_search)\b/) {
+			my $deprecated_api = ${^CAPTURE}{old};
 			my $new_api = $deprecated_apis{$deprecated_api};
 			WARN("DEPRECATED_API",
 			     "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr);

-- 
2.39.5

[PATCH 3/4] gpio: mxc: remove dead code after switch to DT-only

[Ahmad Fatoum]

struct platform_device::id was only set by board code, but since i.MX
became a devicetree-only platform, this will always be -1
(PLATFORM_DEVID_NONE).

Note: of_alias_get_id() returns a negative number on error and base
treats all negative errors the same, so we need not add any additional
error handling.

Fixes: 0f2c7af45d7e ("gpio: mxc: Convert the driver to DT-only")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpio-mxc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 4cb455b2bdee71ba4eb20c93567c3b8db100dbb2..619b6fb9d833a4bb94a93b4209f01b49ad1cbdb0 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -490,8 +490,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	port->gc.request = mxc_gpio_request;
 	port->gc.free = mxc_gpio_free;
 	port->gc.to_irq = mxc_gpio_to_irq;
-	port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
-					     pdev->id * 32;
+	port->gc.base = of_alias_get_id(np, "gpio") * 32;
 
 	err = devm_gpiochip_add_data(&pdev->dev, &port->gc, port);
 	if (err)

-- 
2.39.5

[PATCH 4/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

GPIO numbering has been deterministic since commit 7e6086d9e54a
("gpio/mxc: specify gpio base for device tree probe"), a year after
device tree support was first added back in 2011.

Reverting to dynamically allocated GPIO base now would break most
systems making use of the sysfs API. These systems will be eventually
broken by the removal of the sysfs API, but that would result in GPIO
scripts not working instead of essentially toggling at random according
to probe order, which would happen if we change the base to -1.

Yet, the warning is annoying and has resulted in many attempts
to remove it over the years[1][2][3].

Fix this by silencing the warning via the gpio_base_static_alloc
opt-out instead.

[1]: https://lore.kernel.org/all/20230226205319.1013332-1-dario.binacchi@amarulasolutions.com/
[2]: https://lore.kernel.org/all/20230506085928.933737-2-haibo.chen@nxp.com/
[3]: https://lore.kernel.org/all/20241121145515.3087855-1-catalin.popescu@leica-geosystems.com/

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
This triggers the newly added checkpatch.pl warning, but this is
intended.
---
 drivers/gpio/gpio-mxc.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 619b6fb9d833a4bb94a93b4209f01b49ad1cbdb0..c25a50abb02389e75b5ca2924828f4a7fb32fe6c 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -45,6 +45,11 @@ struct mxc_gpio_hwdata {
 	unsigned high_level;
 	unsigned rise_edge;
 	unsigned fall_edge;
+	/*
+	 * Static allocation of GPIO base is deprecated.
+	 * Set this to false for future SoCs.
+	 */
+	bool gpio_base_static_alloc;
 };
 
 struct mxc_gpio_reg_saved {
@@ -88,6 +93,7 @@ static struct mxc_gpio_hwdata imx1_imx21_gpio_hwdata = {
 	.high_level	= 0x02,
 	.rise_edge	= 0x00,
 	.fall_edge	= 0x01,
+	.gpio_base_static_alloc = true,
 };
 
 static struct mxc_gpio_hwdata imx31_gpio_hwdata = {
@@ -103,6 +109,7 @@ static struct mxc_gpio_hwdata imx31_gpio_hwdata = {
 	.high_level	= 0x01,
 	.rise_edge	= 0x02,
 	.fall_edge	= 0x03,
+	.gpio_base_static_alloc = true,
 };
 
 static struct mxc_gpio_hwdata imx35_gpio_hwdata = {
@@ -118,6 +125,7 @@ static struct mxc_gpio_hwdata imx35_gpio_hwdata = {
 	.high_level	= 0x01,
 	.rise_edge	= 0x02,
 	.fall_edge	= 0x03,
+	.gpio_base_static_alloc = true,
 };
 
 #define GPIO_DR			(port->hwdata->dr_reg)
@@ -490,7 +498,19 @@ static int mxc_gpio_probe(struct platform_device *pdev)
 	port->gc.request = mxc_gpio_request;
 	port->gc.free = mxc_gpio_free;
 	port->gc.to_irq = mxc_gpio_to_irq;
-	port->gc.base = of_alias_get_id(np, "gpio") * 32;
+	port->gc.base = -1;
+
+	if (port->hwdata->gpio_base_static_alloc) {
+		/*
+		 * GPIO indices have been fixed for the i.MX GPIO controllers
+		 * for many years and changing that now will induce a lot of
+		 * breakage at runtime. Setting this member buys users some time
+		 * until they are forced to migrate when sysfs GPIO support is
+		 * removed completely.
+		 */
+		port->gc.legacy_static_base = true;
+		port->gc.base = of_alias_get_id(np, "gpio") * 32;
+	}
 
 	err = devm_gpiochip_add_data(&pdev->dev, &port->gc, port);
 	if (err)

-- 
2.39.5

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Andy Shevchenko]

On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> The i.MX GPIO driver has had deterministic numbering for the GPIOs
> for more than 12 years.
>
> Reverting this to dynamically numbered will break existing setups in the
> worst manner possible: The build will succeed, the kernel will not print
> warnings, but users will find their devices essentially toggling GPIOs
> at random with the potential of permanent damage. We thus want to keep
> the numbering as-is until the SysFS API is removed and script fail
> instead of toggling GPIOs dependent on probe order.

While I understand the issue this tends to get never fixed until the
entire support of iMX boards will be dropped. Personally I do not like
this series at all. Rather let's try to go the hard way and understand
what's going on to fix the current issues.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Andy Shevchenko]

On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Some drivers have had deterministic GPIO numbering for most of
> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
> ("gpio/mxc: specify gpio base for device tree probe"), more than
> 12 years ago.
>
> Reverting this to dynamically numbered will break existing setups in
> the worst manner possible: The build will succeed, the kernel will not
> print warnings, but users will find their devices essentially toggling
> GPIOs at random with the potential of permanent damage.
>
> As these concerns won't go away until the sysfs interface is removed,
> let's add a new struct gpio_chip::legacy_static_base member that can be
> used by existing drivers that have been grandfathered in to suppress
> the warning currently being printed:
>
>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
>   use dynamic allocation.

Warning is harmless and still a good reminder for the stuff that needs
more love.
NAK.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] gpio: mxc: remove dead code after switch to DT-only

[Andy Shevchenko]

On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> struct platform_device::id was only set by board code, but since i.MX
> became a devicetree-only platform, this will always be -1
> (PLATFORM_DEVID_NONE).
>
> Note: of_alias_get_id() returns a negative number on error and base
> treats all negative errors the same, so we need not add any additional
> error handling.

This one is good,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

Hi Andy,

On 14.01.25 10:46, Andy Shevchenko wrote:
> On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> The i.MX GPIO driver has had deterministic numbering for the GPIOs
>> for more than 12 years.
>>
>> Reverting this to dynamically numbered will break existing setups in the
>> worst manner possible: The build will succeed, the kernel will not print
>> warnings, but users will find their devices essentially toggling GPIOs
>> at random with the potential of permanent damage. We thus want to keep
>> the numbering as-is until the SysFS API is removed and script fail
>> instead of toggling GPIOs dependent on probe order.
> 
> While I understand the issue this tends to get never fixed until the
> entire support of iMX boards will be dropped.

i.MX is an actively developed and widely used platform. Why should support
be dropped?

> Personally I do not like
> this series at all. Rather let's try to go the hard way and understand
> what's going on to fix the current issues.

/sys/class/gpio is deprecated and when it is finally removed, this series can
be reverted again. The alternatives are either do nothing and live with 6 kernel
warnings cluttering every boot or show users the finger as described in
the cover letter.

Do you see a different path forward?

Cheers,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Ahmad Fatoum]

On 14.01.25 10:49, Andy Shevchenko wrote:
> On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Some drivers have had deterministic GPIO numbering for most of
>> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
>> ("gpio/mxc: specify gpio base for device tree probe"), more than
>> 12 years ago.
>>
>> Reverting this to dynamically numbered will break existing setups in
>> the worst manner possible: The build will succeed, the kernel will not
>> print warnings, but users will find their devices essentially toggling
>> GPIOs at random with the potential of permanent damage.
>>
>> As these concerns won't go away until the sysfs interface is removed,
>> let's add a new struct gpio_chip::legacy_static_base member that can be
>> used by existing drivers that have been grandfathered in to suppress
>> the warning currently being printed:
>>
>>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
>>   use dynamic allocation.
> 
> Warning is harmless and still a good reminder for the stuff that needs
> more love.
> NAK.

A warning is a call-to-action and it's counterproductive to keep tricking
people into removing the static base and breaking other users' scripts.

I don't understand what love you think this will spawn with regards
to the i.MX GPIO driver. Can you explain?

Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/4] checkpatch: warn about use of legacy_static_base

[Linus Walleij]

On Mon, Jan 13, 2025 at 11:19 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> gpio_chip::legacy_state_base was recently added as opt-out for
> existing drivers and shouldn't be used for new drivers. It's thus
> sensible to add a deprecation warning whenever it's used.
>
> This doesn't fit with the existing deprecated API check, because it
> requires a `(' to follow the symbol name, so a new member specific
> pattern is introduced instead.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thanks Ahmad, much appreciated!!
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Andy Shevchenko]

On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> On 14.01.25 10:49, Andy Shevchenko wrote:
> > On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >> Some drivers have had deterministic GPIO numbering for most of
> >> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
> >> ("gpio/mxc: specify gpio base for device tree probe"), more than
> >> 12 years ago.
> >>
> >> Reverting this to dynamically numbered will break existing setups in
> >> the worst manner possible: The build will succeed, the kernel will not
> >> print warnings, but users will find their devices essentially toggling
> >> GPIOs at random with the potential of permanent damage.
> >>
> >> As these concerns won't go away until the sysfs interface is removed,
> >> let's add a new struct gpio_chip::legacy_static_base member that can be
> >> used by existing drivers that have been grandfathered in to suppress
> >> the warning currently being printed:
> >>
> >>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
> >>   use dynamic allocation.
> >
> > Warning is harmless and still a good reminder for the stuff that needs
> > more love.
> > NAK.
>
> A warning is a call-to-action and it's counterproductive to keep tricking
> people into removing the static base and breaking other users' scripts.

Are you prepared to say the same when the entire GPIO SYSFS will be
removed? Because that's exactly what I referred to in the reply to the
cover letter as an impediment to move forward.

> I don't understand what love you think this will spawn with regards
> to the i.MX GPIO driver. Can you explain?

To fix the bugs you found. If it's not the GPIO driver a culprit, we
need to find the real one and fix that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Andy Shevchenko]

On Tue, Jan 14, 2025 at 11:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> On 14.01.25 10:46, Andy Shevchenko wrote:
> > On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >> The i.MX GPIO driver has had deterministic numbering for the GPIOs
> >> for more than 12 years.
> >>
> >> Reverting this to dynamically numbered will break existing setups in the
> >> worst manner possible: The build will succeed, the kernel will not print
> >> warnings, but users will find their devices essentially toggling GPIOs
> >> at random with the potential of permanent damage. We thus want to keep
> >> the numbering as-is until the SysFS API is removed and script fail
> >> instead of toggling GPIOs dependent on probe order.
> >
> > While I understand the issue this tends to get never fixed until the
> > entire support of iMX boards will be dropped.
>
> i.MX is an actively developed and widely used platform. Why should support
> be dropped?

Exactly, Which means "tend to get never fixed".

> > Personally I do not like
> > this series at all. Rather let's try to go the hard way and understand
> > what's going on to fix the current issues.
>
> /sys/class/gpio is deprecated and when it is finally removed, this series can
> be reverted again. The alternatives are either do nothing and live with 6 kernel
> warnings cluttering every boot or show users the finger as described in
> the cover letter.
>
> Do you see a different path forward?

Yes, try to write your scripts based on the libgpiod or the tools
provided by the project. I.o.w. follow the warning that SYSFS will be
removed at some point and prepare yourself for that. If some kernel
work needs to be done, contribute.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

Hello Andy,

On 14.01.25 20:43, Andy Shevchenko wrote:
> On Tue, Jan 14, 2025 at 11:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> On 14.01.25 10:46, Andy Shevchenko wrote:
>>> On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>> The i.MX GPIO driver has had deterministic numbering for the GPIOs
>>>> for more than 12 years.
>>>>
>>>> Reverting this to dynamically numbered will break existing setups in the
>>>> worst manner possible: The build will succeed, the kernel will not print
>>>> warnings, but users will find their devices essentially toggling GPIOs
>>>> at random with the potential of permanent damage. We thus want to keep
>>>> the numbering as-is until the SysFS API is removed and script fail
>>>> instead of toggling GPIOs dependent on probe order.

Please read my cover letter / commit messages. I do nowhere object to deprecation
and removal of the sysfs interface. But I strongly disagree that a necessary step
towards that is having Linux start toggling random GPIOs after an update on
platforms that behaved consistently for >10 years.

Can you explain why we can't remove the hardcoded base at the same time that
sysfs support is removed for good?

>>> While I understand the issue this tends to get never fixed until the
>>> entire support of iMX boards will be dropped.
>>
>> i.MX is an actively developed and widely used platform. Why should support
>> be dropped?
> 
> Exactly, Which means "tend to get never fixed".

Imagine ReiserFS deprecation strategy involved shipping an update that
just corrupted your existing file system and developers insisted on calling
it a fix, as ReiserFS is going to be removed anyway.

>>> Personally I do not like
>>> this series at all. Rather let's try to go the hard way and understand
>>> what's going on to fix the current issues.
>>
>> /sys/class/gpio is deprecated and when it is finally removed, this series can
>> be reverted again. The alternatives are either do nothing and live with 6 kernel
>> warnings cluttering every boot or show users the finger as described in
>> the cover letter.
>>
>> Do you see a different path forward?
> 
> Yes, try to write your scripts based on the libgpiod or the tools
> provided by the project. I.o.w. follow the warning that SYSFS will be
> removed at some point and prepare yourself for that. If some kernel
> work needs to be done, contribute.

I have been using libgpiod for many years, but have in the past used sysfs
or been involved with projects using sysfs. I agree that these projects
need to switch to the GPIO character device and that they will be eventually
broken. Yet, I still get warnings despite doing everything correctly IMO and no,
I don't want to fix a warning by doing negligent stuff like jumble GPIO numbers,
with the reason that it's going to be broken for good in the future anyway.

To reiterate, my issue is with the manner of breakage:

  - broken, because /sys/class/gpio doesn't exist: good
  - broken, because script executes successfully, but toggles arbitrary pins: bad

Thanks,
Ahmad

> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Ahmad Fatoum]

On 14.01.25 20:38, Andy Shevchenko wrote:
> On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> On 14.01.25 10:49, Andy Shevchenko wrote:
>>> On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>> Some drivers have had deterministic GPIO numbering for most of
>>>> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
>>>> ("gpio/mxc: specify gpio base for device tree probe"), more than
>>>> 12 years ago.
>>>>
>>>> Reverting this to dynamically numbered will break existing setups in
>>>> the worst manner possible: The build will succeed, the kernel will not
>>>> print warnings, but users will find their devices essentially toggling
>>>> GPIOs at random with the potential of permanent damage.
>>>>
>>>> As these concerns won't go away until the sysfs interface is removed,
>>>> let's add a new struct gpio_chip::legacy_static_base member that can be
>>>> used by existing drivers that have been grandfathered in to suppress
>>>> the warning currently being printed:
>>>>
>>>>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
>>>>   use dynamic allocation.
>>>
>>> Warning is harmless and still a good reminder for the stuff that needs
>>> more love.
>>> NAK.
>>
>> A warning is a call-to-action and it's counterproductive to keep tricking
>> people into removing the static base and breaking other users' scripts.
> 
> Are you prepared to say the same when the entire GPIO SYSFS will be
> removed? Because that's exactly what I referred to in the reply to the
> cover letter as an impediment to move forward.

No. But this gives me an idea: We could make the warning dependent
on CONFIG_GPIO_SYSFS and add a comment to the i.MX code suggesting
users do that instead. What do you think?

>> I don't understand what love you think this will spawn with regards
>> to the i.MX GPIO driver. Can you explain?
> 
> To fix the bugs you found. If it's not the GPIO driver a culprit, we
> need to find the real one and fix that.

Again: jumbling GPIOs with potential hardware harm as a result is not a fix.

Cheers,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Linus Walleij]

On Mon, Jan 13, 2025 at 11:19 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> As these concerns won't go away until the sysfs interface is removed,
> let's add a new struct gpio_chip::legacy_static_base member that can be
> used by existing drivers that have been grandfathered in to suppress
> the warning currently being printed:

I think entire drivers, pertaining to in worst case several generations
of SoCs is not the way to approach this. It could be a SoC or, more
likely, single systems using a SoC, that has a problem with this.

If you want to safeguard this I would use some code loop in the
gpiolib(-sysfs) that looks at of_machine_is_compatible("foo,bar-machine")
to match the top-level compatible for known problematic machines
so we can be fine-grained of this so when that machines retires
the driver can start using dynamic GPIO number allotment.

Yours,
Linus Walleij

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Kent Gibson]

On Wed, Jan 15, 2025 at 08:07:38AM +0100, Ahmad Fatoum wrote:
> On 14.01.25 20:38, Andy Shevchenko wrote:
> > On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >> On 14.01.25 10:49, Andy Shevchenko wrote:
> >>> On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>
> >>>> Some drivers have had deterministic GPIO numbering for most of
> >>>> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
> >>>> ("gpio/mxc: specify gpio base for device tree probe"), more than
> >>>> 12 years ago.
> >>>>
> >>>> Reverting this to dynamically numbered will break existing setups in
> >>>> the worst manner possible: The build will succeed, the kernel will not
> >>>> print warnings, but users will find their devices essentially toggling
> >>>> GPIOs at random with the potential of permanent damage.
> >>>>
> >>>> As these concerns won't go away until the sysfs interface is removed,
> >>>> let's add a new struct gpio_chip::legacy_static_base member that can be
> >>>> used by existing drivers that have been grandfathered in to suppress
> >>>> the warning currently being printed:
> >>>>
> >>>>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
> >>>>   use dynamic allocation.
> >>>
> >>> Warning is harmless and still a good reminder for the stuff that needs
> >>> more love.
> >>> NAK.
> >>
> >> A warning is a call-to-action and it's counterproductive to keep tricking
> >> people into removing the static base and breaking other users' scripts.
> >
> > Are you prepared to say the same when the entire GPIO SYSFS will be
> > removed? Because that's exactly what I referred to in the reply to the
> > cover letter as an impediment to move forward.
>
> No. But this gives me an idea: We could make the warning dependent
> on CONFIG_GPIO_SYSFS and add a comment to the i.MX code suggesting
> users do that instead. What do you think?
>

AIUI, the purpose of the warning is to remind driver authors, not end users,
to update their drivers, as the old behaviour is deprecated. That is
independent of GPIO SYSFS - that just happens to be something that makes the
change visible to userspace.

Rather than making the warning conditional, how about making the fix for the
warning in your driver, so switching to dynamic allocation, conditional on
CONFIG_GPIO_SYSFS not being set?
That would provide a path forward for users that want to dispense with
the warning - as long as they dispense with GPIO SYSFS.

Cheers,
Kent.

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Andy Shevchenko]

On Wed, Jan 15, 2025 at 9:03 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> On 14.01.25 20:43, Andy Shevchenko wrote:
> > On Tue, Jan 14, 2025 at 11:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >> On 14.01.25 10:46, Andy Shevchenko wrote:
> >>> On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>
> >>>> The i.MX GPIO driver has had deterministic numbering for the GPIOs
> >>>> for more than 12 years.
> >>>>
> >>>> Reverting this to dynamically numbered will break existing setups in the
> >>>> worst manner possible: The build will succeed, the kernel will not print
> >>>> warnings, but users will find their devices essentially toggling GPIOs
> >>>> at random with the potential of permanent damage. We thus want to keep
> >>>> the numbering as-is until the SysFS API is removed and script fail
> >>>> instead of toggling GPIOs dependent on probe order.
>
> Please read my cover letter / commit messages. I do nowhere object to deprecation
> and removal of the sysfs interface. But I strongly disagree that a necessary step
> towards that is having Linux start toggling random GPIOs after an update on
> platforms that behaved consistently for >10 years.
>
> Can you explain why we can't remove the hardcoded base at the same time that
> sysfs support is removed for good?

Because (if follow your logic!) it won't ever happen until all the
platforms that are using the non-dynamic bases are being removed as
well.

Otherwise this situation isn't anyhow different to the broken platform
as you described.

> >>> While I understand the issue this tends to get never fixed until the
> >>> entire support of iMX boards will be dropped.
> >>
> >> i.MX is an actively developed and widely used platform. Why should support
> >> be dropped?
> >
> > Exactly, Which means "tend to get never fixed".
>
> Imagine ReiserFS deprecation strategy involved shipping an update that
> just corrupted your existing file system and developers insisted on calling
> it a fix, as ReiserFS is going to be removed anyway.

It's not the same. If you still want to compare, then it means that
what I suggest is to move from Reiser to say XFS.

> >>> Personally I do not like
> >>> this series at all. Rather let's try to go the hard way and understand
> >>> what's going on to fix the current issues.
> >>
> >> /sys/class/gpio is deprecated and when it is finally removed, this series can
> >> be reverted again. The alternatives are either do nothing and live with 6 kernel
> >> warnings cluttering every boot or show users the finger as described in
> >> the cover letter.
> >>
> >> Do you see a different path forward?
> >
> > Yes, try to write your scripts based on the libgpiod or the tools
> > provided by the project. I.o.w. follow the warning that SYSFS will be
> > removed at some point and prepare yourself for that. If some kernel
> > work needs to be done, contribute.
>
> I have been using libgpiod for many years, but have in the past used sysfs
> or been involved with projects using sysfs. I agree that these projects
> need to switch to the GPIO character device and that they will be eventually
> broken. Yet, I still get warnings despite doing everything correctly IMO and no,
> I don't want to fix a warning by doing negligent stuff like jumble GPIO numbers,
> with the reason that it's going to be broken for good in the future anyway.
>
> To reiterate, my issue is with the manner of breakage:
>
>   - broken, because /sys/class/gpio doesn't exist: good
>   - broken, because script executes successfully, but toggles arbitrary pins: bad

I understand that, but what the series is trying to do is to put on
hold _any_ sysfs removal activity along with reducing test coverage
and motivation to fix the certain platform to work with dynamic base.

So, prepare your scripts not to toggle arbitrary numbers then and use libgpiod.

P.S. I think this discussion goes nowhere. Talk to the GPIO
maintainers for the matter, I'm not preventing you to put on hold GPIO
development for _this_ platform, but I'm strongly against that because
of your platform others should also be on hold, hence my NAK for that
gpiolib patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Bartosz Golaszewski]

On Tue, Jan 14, 2025 at 10:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hi Andy,
>
> On 14.01.25 10:46, Andy Shevchenko wrote:
> > On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >> The i.MX GPIO driver has had deterministic numbering for the GPIOs
> >> for more than 12 years.
> >>
> >> Reverting this to dynamically numbered will break existing setups in the
> >> worst manner possible: The build will succeed, the kernel will not print
> >> warnings, but users will find their devices essentially toggling GPIOs
> >> at random with the potential of permanent damage. We thus want to keep
> >> the numbering as-is until the SysFS API is removed and script fail
> >> instead of toggling GPIOs dependent on probe order.
> >
> > While I understand the issue this tends to get never fixed until the
> > entire support of iMX boards will be dropped.
>
> i.MX is an actively developed and widely used platform. Why should support
> be dropped?
>
> > Personally I do not like
> > this series at all. Rather let's try to go the hard way and understand
> > what's going on to fix the current issues.
>
> /sys/class/gpio is deprecated and when it is finally removed, this series can
> be reverted again. The alternatives are either do nothing and live with 6 kernel
> warnings cluttering every boot or show users the finger as described in
> the cover letter.
>
> Do you see a different path forward?
>

I recently wrote a user-space compatibility layer for sysfs[1]. While
right now it doesn't support static base numbers, I'm very open to
adding it except that I wasn't sure how to approach it.

Is this of any use to you and could it help you switch to libgpiod
without changing your user-space set-up (given the support for static
GPIO numbering)? If so, how would you like to see this implemented? A
config file at /etc that would list chip labels and their desired GPIO
base?

Bartosz

[1] https://github.com/brgl/gpiod-sysfs-proxy

Re: [PATCH 3/4] gpio: mxc: remove dead code after switch to DT-only

[Bartosz Golaszewski]

On Mon, Jan 13, 2025 at 11:19 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> struct platform_device::id was only set by board code, but since i.MX
> became a devicetree-only platform, this will always be -1
> (PLATFORM_DEVID_NONE).
>
> Note: of_alias_get_id() returns a negative number on error and base
> treats all negative errors the same, so we need not add any additional
> error handling.
>
> Fixes: 0f2c7af45d7e ("gpio: mxc: Convert the driver to DT-only")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/gpio/gpio-mxc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 4cb455b2bdee71ba4eb20c93567c3b8db100dbb2..619b6fb9d833a4bb94a93b4209f01b49ad1cbdb0 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -490,8 +490,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>         port->gc.request = mxc_gpio_request;
>         port->gc.free = mxc_gpio_free;
>         port->gc.to_irq = mxc_gpio_to_irq;
> -       port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
> -                                            pdev->id * 32;
> +       port->gc.base = of_alias_get_id(np, "gpio") * 32;
>
>         err = devm_gpiochip_add_data(&pdev->dev, &port->gc, port);
>         if (err)
>
> --
> 2.39.5
>

This looks like I can pick it up separately from the rest? Is that right?

Bart

Re: [PATCH 3/4] gpio: mxc: remove dead code after switch to DT-only

[Ahmad Fatoum]

On 15.01.25 17:55, Bartosz Golaszewski wrote:
> On Mon, Jan 13, 2025 at 11:19 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> struct platform_device::id was only set by board code, but since i.MX
>> became a devicetree-only platform, this will always be -1
>> (PLATFORM_DEVID_NONE).
>>
>> Note: of_alias_get_id() returns a negative number on error and base
>> treats all negative errors the same, so we need not add any additional
>> error handling.
>>
>> Fixes: 0f2c7af45d7e ("gpio: mxc: Convert the driver to DT-only")
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/gpio/gpio-mxc.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 4cb455b2bdee71ba4eb20c93567c3b8db100dbb2..619b6fb9d833a4bb94a93b4209f01b49ad1cbdb0 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -490,8 +490,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>         port->gc.request = mxc_gpio_request;
>>         port->gc.free = mxc_gpio_free;
>>         port->gc.to_irq = mxc_gpio_to_irq;
>> -       port->gc.base = (pdev->id < 0) ? of_alias_get_id(np, "gpio") * 32 :
>> -                                            pdev->id * 32;
>> +       port->gc.base = of_alias_get_id(np, "gpio") * 32;
>>
>>         err = devm_gpiochip_add_data(&pdev->dev, &port->gc, port);
>>         if (err)
>>
>> --
>> 2.39.5
>>
> 
> This looks like I can pick it up separately from the rest? Is that right?

Yes, please do.

Thanks,
Ahmad

> 
> Bart
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Ahmad Fatoum]

Hi Kent,

On 15.01.25 14:04, Kent Gibson wrote:
> On Wed, Jan 15, 2025 at 08:07:38AM +0100, Ahmad Fatoum wrote:
>> On 14.01.25 20:38, Andy Shevchenko wrote:
>>> On Tue, Jan 14, 2025 at 12:06 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>> On 14.01.25 10:49, Andy Shevchenko wrote:
>>>>> On Tue, Jan 14, 2025 at 12:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>
>>>>>> Some drivers have had deterministic GPIO numbering for most of
>>>>>> their existence, e.g. the i.MX GPIO since commit 7e6086d9e54a
>>>>>> ("gpio/mxc: specify gpio base for device tree probe"), more than
>>>>>> 12 years ago.
>>>>>>
>>>>>> Reverting this to dynamically numbered will break existing setups in
>>>>>> the worst manner possible: The build will succeed, the kernel will not
>>>>>> print warnings, but users will find their devices essentially toggling
>>>>>> GPIOs at random with the potential of permanent damage.
>>>>>>
>>>>>> As these concerns won't go away until the sysfs interface is removed,
>>>>>> let's add a new struct gpio_chip::legacy_static_base member that can be
>>>>>> used by existing drivers that have been grandfathered in to suppress
>>>>>> the warning currently being printed:
>>>>>>
>>>>>>   gpio gpiochip0: Static allocation of GPIO base is deprecated,
>>>>>>   use dynamic allocation.
>>>>>
>>>>> Warning is harmless and still a good reminder for the stuff that needs
>>>>> more love.
>>>>> NAK.
>>>>
>>>> A warning is a call-to-action and it's counterproductive to keep tricking
>>>> people into removing the static base and breaking other users' scripts.
>>>
>>> Are you prepared to say the same when the entire GPIO SYSFS will be
>>> removed? Because that's exactly what I referred to in the reply to the
>>> cover letter as an impediment to move forward.
>>
>> No. But this gives me an idea: We could make the warning dependent
>> on CONFIG_GPIO_SYSFS and add a comment to the i.MX code suggesting
>> users do that instead. What do you think?
>>
> 
> AIUI, the purpose of the warning is to remind driver authors, not end users,
> to update their drivers, as the old behaviour is deprecated. That is
> independent of GPIO SYSFS - that just happens to be something that makes the
> change visible to userspace.
> 
> Rather than making the warning conditional, how about making the fix for the
> warning in your driver, so switching to dynamic allocation, conditional on
> CONFIG_GPIO_SYSFS not being set?
> That would provide a path forward for users that want to dispense with
> the warning - as long as they dispense with GPIO SYSFS.

That could work for gpio-mxc, provided that SysFS is the only user for which
the static base matters. I assume that's the case, but I am not sure.

An argument for suppressing the warning selectively in the GPIO core is that
this doesn't only affect gpio-mxc, but also e.g. gpio-zynq or gpio-mxs.

Cheers,
Ahmad



> 
> Cheers,
> Kent.
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

Hello Bartosz,

On 15.01.25 17:52, Bartosz Golaszewski wrote:
> On Tue, Jan 14, 2025 at 10:55 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hi Andy,
>>
>> On 14.01.25 10:46, Andy Shevchenko wrote:
>>> On Tue, Jan 14, 2025 at 12:19 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>> The i.MX GPIO driver has had deterministic numbering for the GPIOs
>>>> for more than 12 years.
>>>>
>>>> Reverting this to dynamically numbered will break existing setups in the
>>>> worst manner possible: The build will succeed, the kernel will not print
>>>> warnings, but users will find their devices essentially toggling GPIOs
>>>> at random with the potential of permanent damage. We thus want to keep
>>>> the numbering as-is until the SysFS API is removed and script fail
>>>> instead of toggling GPIOs dependent on probe order.
>>>
>>> While I understand the issue this tends to get never fixed until the
>>> entire support of iMX boards will be dropped.
>>
>> i.MX is an actively developed and widely used platform. Why should support
>> be dropped?
>>
>>> Personally I do not like
>>> this series at all. Rather let's try to go the hard way and understand
>>> what's going on to fix the current issues.
>>
>> /sys/class/gpio is deprecated and when it is finally removed, this series can
>> be reverted again. The alternatives are either do nothing and live with 6 kernel
>> warnings cluttering every boot or show users the finger as described in
>> the cover letter.
>>
>> Do you see a different path forward?
>>
> 
> I recently wrote a user-space compatibility layer for sysfs[1]. While
> right now it doesn't support static base numbers, I'm very open to
> adding it except that I wasn't sure how to approach it.
> 
> Is this of any use to you and could it help you switch to libgpiod
> without changing your user-space set-up (given the support for static
> GPIO numbering)?

If the goal is to have a drop-in replacement for sysfs outside
of the kernel, I think it needs to maintain the same numbering.

I am not sure I see myself using it, because the new projects are using
libgpiod from the get-go. My issue is not with removal of sysfs, but with
user hostile deprecation approaches.

> If so, how would you like to see this implemented? A
> config file at /etc that would list chip labels and their desired GPIO
> base?

Many GPIOs tend to not have labels. I think the mapping config file
should rather map GPIO controllers to a base address. The same thing the
kernel is currently doing. This assumes the numbering of the built-in
GPIO controllers is deterministic, e.g. by consulting /aliases. I haven't
checked, but I hope this is already the case.

Cheers,
Ahmad

> 
> Bartosz
> 
> [1] https://github.com/brgl/gpiod-sysfs-proxy
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Ahmad Fatoum]

Hi Andy,

On 15.01.25 16:16, Andy Shevchenko wrote:
> On Wed, Jan 15, 2025 at 9:03 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> Please read my cover letter / commit messages. I do nowhere object to deprecation
>> and removal of the sysfs interface. But I strongly disagree that a necessary step
>> towards that is having Linux start toggling random GPIOs after an update on
>> platforms that behaved consistently for >10 years.
>>
>> Can you explain why we can't remove the hardcoded base at the same time that
>> sysfs support is removed for good?
> 
> Because (if follow your logic!) it won't ever happen until all the
> platforms that are using the non-dynamic bases are being removed as
> well.
>
> Otherwise this situation isn't anyhow different to the broken platform
> as you described.

Sorry, it's not clear to me why non-dynamic-bases can't be removed
at the same time that SysFS itself is removed. Can you explain?

>>>> i.MX is an actively developed and widely used platform. Why should support
>>>> be dropped?
>>>
>>> Exactly, Which means "tend to get never fixed".
>>
>> Imagine ReiserFS deprecation strategy involved shipping an update that
>> just corrupted your existing file system and developers insisted on calling
>> it a fix, as ReiserFS is going to be removed anyway.
> 
> It's not the same. If you still want to compare, then it means that
> what I suggest is to move from Reiser to say XFS.

I made a chart.

Starting position is that both ReiserFS and GPIO SysFS are going to be removed.

                +------------------------------------------------------------------------+
                | File System                       | GPIO                               |
+---------------+-----------------------------------+------------------------------------+
| Sensible      | Use XFS. ReiserFS will be         | Use libgpiod. /sys/class/gpio will |
|               | removed in future.                | be removed in future               |
+---------------+-----------------------------------+------------------------------------+
| User-hostile  | Mounting will jumble your inodes  | Booting will jumble your GPIOs     |
|               | and possibly corrupt your FS.     | and possibly brick your board.     |
+---------------+-----------------------------------+------------------------------------+

I believe the second row is bad and I don't want it for i.MX
users (or any users for that matter).

>> To reiterate, my issue is with the manner of breakage:
>>
>>   - broken, because /sys/class/gpio doesn't exist: good
>>   - broken, because script executes successfully, but toggles arbitrary pins: bad
> 
> I understand that, but what the series is trying to do is to put on
> hold _any_ sysfs removal activity along with reducing test coverage
> and motivation to fix the certain platform to work with dynamic base.

Why can't consumers of the static base be removed and then when none
are left, the base goes away too. Why does it have to be the other
way round?

> So, prepare your scripts not to toggle arbitrary numbers then and use libgpiod.

The SoC's own GPIO controllers have had deterministic numbering
for a long time. What would make them arbitrary is setting the base
to -1.

> P.S. I think this discussion goes nowhere. Talk to the GPIO
> maintainers for the matter, 

I believe that's what I am doing now?

> I'm not preventing you to put on hold GPIO
> development for _this_ platform, but I'm strongly against that because
> of your platform others should also be on hold, hence my NAK for that
> gpiolib patch.

Can you explain or point me at resources to understand why a static base
is blocking GPIO development subsystem-wide?

Thanks,
Ahmad


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/4] gpiolib: add opt-out for existing drivers with static GPIO base

[Ahmad Fatoum]

Hi Linus,

On 15.01.25 13:00, Linus Walleij wrote:
> On Mon, Jan 13, 2025 at 11:19 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
>> As these concerns won't go away until the sysfs interface is removed,
>> let's add a new struct gpio_chip::legacy_static_base member that can be
>> used by existing drivers that have been grandfathered in to suppress
>> the warning currently being printed:
> 
> I think entire drivers, pertaining to in worst case several generations
> of SoCs is not the way to approach this. It could be a SoC or, more
> likely, single systems using a SoC, that has a problem with this.
> 
> If you want to safeguard this I would use some code loop in the
> gpiolib(-sysfs) that looks at of_machine_is_compatible("foo,bar-machine")
> to match the top-level compatible for known problematic machines
> so we can be fine-grained of this so when that machines retires
> the driver can start using dynamic GPIO number allotment.

It's meant to apply to all existing i.MX SoCs, but not for new ones using
the same driver.

Filtering by board is not practical and doesn't address the problem of
a kernel update leading to toggling of arbitrary GPIOs.

I am wondering, what remaining _users_ of the GPIO base do we have.
Is it just SysFS and legacy board code?

Cheers,
Ahmad

> 
> Yours,
> Linus Walleij
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: (subset) [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 13 Jan 2025 23:19:08 +0100, Ahmad Fatoum wrote:
> The i.MX GPIO driver has had deterministic numbering for the GPIOs
> for more than 12 years.
> 
> Reverting this to dynamically numbered will break existing setups in the
> worst manner possible: The build will succeed, the kernel will not print
> warnings, but users will find their devices essentially toggling GPIOs
> at random with the potential of permanent damage. We thus want to keep
> the numbering as-is until the SysFS API is removed and script fail
> instead of toggling GPIOs dependent on probe order.
> 
> [...]

Applied, thanks!

[3/4] gpio: mxc: remove dead code after switch to DT-only
      commit: b049e7abe9001a780d58e78e3833dcceee22f396

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH 0/4] gpio: mxc: silence warning about GPIO base being statically allocated

[Bartosz Golaszewski]

On Tue, Jan 21, 2025 at 11:37 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Bartosz,
>
> On 15.01.25 17:52, Bartosz Golaszewski wrote:
> >
> > I recently wrote a user-space compatibility layer for sysfs[1]. While
> > right now it doesn't support static base numbers, I'm very open to
> > adding it except that I wasn't sure how to approach it.
> >
> > Is this of any use to you and could it help you switch to libgpiod
> > without changing your user-space set-up (given the support for static
> > GPIO numbering)?
>
> If the goal is to have a drop-in replacement for sysfs outside
> of the kernel, I think it needs to maintain the same numbering.
>
> I am not sure I see myself using it, because the new projects are using
> libgpiod from the get-go. My issue is not with removal of sysfs, but with
> user hostile deprecation approaches.
>
> > If so, how would you like to see this implemented? A
> > config file at /etc that would list chip labels and their desired GPIO
> > base?
>
> Many GPIOs tend to not have labels. I think the mapping config file
> should rather map GPIO controllers to a base address. The same thing the
> kernel is currently doing. This assumes the numbering of the built-in
> GPIO controllers is deterministic, e.g. by consulting /aliases. I haven't
> checked, but I hope this is already the case.

Well, they will have labels, it's just that the label will be
something like "6e80000.gpio" which can very well be mapped onto a
predefined GPIO range.

The file could look like:

/etc/gpio-static-base.conf

```
6e80000.gpio 12
foobar 340
```

Where the first column is the label and the second the static base
that must be less than 512 - ngpio.

Bart