[PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties

Bartosz Golaszewski posted 6 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
module, let's add support for getting the relevant values from generic
device properties. "label" is a semi-standardized property in some GPIO
drivers so let's go with it. There's no standard "base" property, so
let's use the name "gpio-mmio,base" to tie it to this driver
specifically. The number of GPIOs will be retrieved using
gpiochip_get_ngpios() so there's no need to look it up in the software
node.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-mmio.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index ffe6b6f6cc9b1e9341e1c42cf8fee917e0147bf3..8108aa8e6b86ae3d0b520a0f22a09689ab1d9bc5 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -737,6 +737,9 @@ MODULE_DEVICE_TABLE(of, bgpio_of_match);
 static struct bgpio_pdata *bgpio_parse_fw(struct device *dev, unsigned long *flags)
 {
 	struct bgpio_pdata *pdata;
+	const char *label;
+	unsigned int base;
+	int ret;
 
 	if (!dev_fwnode(dev))
 		return NULL;
@@ -753,6 +756,14 @@ static struct bgpio_pdata *bgpio_parse_fw(struct device *dev, unsigned long *fla
 	if (device_property_read_bool(dev, "no-output"))
 		*flags |= BGPIOF_NO_OUTPUT;
 
+	ret = device_property_read_string(dev, "label", &label);
+	if (!ret)
+		pdata->label = label;
+
+	ret = device_property_read_u32(dev, "gpio-mmio,base", &base);
+	if (!ret && base <= INT_MAX)
+		pdata->base = base;
+
 	return pdata;
 }
 

-- 
2.48.1
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Linus Walleij 3 months, 2 weeks ago
On Tue, Jun 24, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
> module, let's add support for getting the relevant values from generic
> device properties. "label" is a semi-standardized property in some GPIO
> drivers so let's go with it. There's no standard "base" property, so
> let's use the name "gpio-mmio,base" to tie it to this driver
> specifically. The number of GPIOs will be retrieved using
> gpiochip_get_ngpios() so there's no need to look it up in the software
> node.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

This works for me.
I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
property all of a sudden just because it now exists as a device
property though... I kind of wish we had a way to opt out of exposing
this to all the sub-property paths. But it seems tiresome, so:

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Tue, 24 Jun 2025 at 21:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jun 24, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
> > module, let's add support for getting the relevant values from generic
> > device properties. "label" is a semi-standardized property in some GPIO
> > drivers so let's go with it. There's no standard "base" property, so
> > let's use the name "gpio-mmio,base" to tie it to this driver
> > specifically. The number of GPIOs will be retrieved using
> > gpiochip_get_ngpios() so there's no need to look it up in the software
> > node.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> This works for me.
> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
> property all of a sudden just because it now exists as a device
> property though... I kind of wish we had a way to opt out of exposing
> this to all the sub-property paths. But it seems tiresome, so:
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Yours,
> Linus Walleij

That's not a problem - this property is not in any DT bindings and as
such is not an allowed property in DT sources. For out-of-tree DTs? We
don't care about those.

Bartosz
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 25/06/2025 09:35, Bartosz Golaszewski wrote:
> On Tue, 24 Jun 2025 at 21:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>> On Tue, Jun 24, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
>>> module, let's add support for getting the relevant values from generic
>>> device properties. "label" is a semi-standardized property in some GPIO
>>> drivers so let's go with it. There's no standard "base" property, so
>>> let's use the name "gpio-mmio,base" to tie it to this driver
>>> specifically. The number of GPIOs will be retrieved using
>>> gpiochip_get_ngpios() so there's no need to look it up in the software
>>> node.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> This works for me.
>> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
>> property all of a sudden just because it now exists as a device
>> property though... I kind of wish we had a way to opt out of exposing
>> this to all the sub-property paths. But it seems tiresome, so:
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Yours,
>> Linus Walleij
> 
> That's not a problem - this property is not in any DT bindings and as
> such is not an allowed property in DT sources. For out-of-tree DTs? We
> don't care about those.
That's not true, we do care about implied ABI. Try changing/breaking
this later, when users complain their out of tree DTS is affected, and
explaining this all to Greg.

Rob was/is working on tools checking this for such implied ABI, I think.
For of_xxx() calls it is obvious, for device_xxx() or fwnode_xxx() it is
not, therefore please add a comment that this is not allowed to be used
by Devicetree platforms (or that this is only for
ACPI/platform_data/whatever). I don't have any other guideline/solution
other than a comment.

Best regards,
Krzysztof
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Wed, Jun 25, 2025 at 10:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 25/06/2025 09:35, Bartosz Golaszewski wrote:
> > On Tue, 24 Jun 2025 at 21:44, Linus Walleij <linus.walleij@linaro.org> wrote:
> >>
> >> On Tue, Jun 24, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >>
> >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>>
> >>> Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
> >>> module, let's add support for getting the relevant values from generic
> >>> device properties. "label" is a semi-standardized property in some GPIO
> >>> drivers so let's go with it. There's no standard "base" property, so
> >>> let's use the name "gpio-mmio,base" to tie it to this driver
> >>> specifically. The number of GPIOs will be retrieved using
> >>> gpiochip_get_ngpios() so there's no need to look it up in the software
> >>> node.
> >>>
> >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> This works for me.
> >> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
> >> property all of a sudden just because it now exists as a device
> >> property though... I kind of wish we had a way to opt out of exposing
> >> this to all the sub-property paths. But it seems tiresome, so:
> >>
> >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >>
> >> Yours,
> >> Linus Walleij
> >
> > That's not a problem - this property is not in any DT bindings and as
> > such is not an allowed property in DT sources. For out-of-tree DTs? We
> > don't care about those.
> That's not true, we do care about implied ABI. Try changing/breaking
> this later, when users complain their out of tree DTS is affected, and
> explaining this all to Greg.
>

Wait, seriously? I thought that the upstream bindings are the source
of truth for device-tree sources...

> Rob was/is working on tools checking this for such implied ABI, I think.
> For of_xxx() calls it is obvious, for device_xxx() or fwnode_xxx() it is
> not, therefore please add a comment that this is not allowed to be used
> by Devicetree platforms (or that this is only for
> ACPI/platform_data/whatever). I don't have any other guideline/solution
> other than a comment.
>

Ok.

Bart
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 25/06/2025 12:23, Bartosz Golaszewski wrote:
> On Wed, Jun 25, 2025 at 10:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 25/06/2025 09:35, Bartosz Golaszewski wrote:
>>> On Tue, 24 Jun 2025 at 21:44, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>
>>>> On Tue, Jun 24, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>>
>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>
>>>>> Ahead of removing struct bgpio_pdata support from the gpio-mmio generic
>>>>> module, let's add support for getting the relevant values from generic
>>>>> device properties. "label" is a semi-standardized property in some GPIO
>>>>> drivers so let's go with it. There's no standard "base" property, so
>>>>> let's use the name "gpio-mmio,base" to tie it to this driver
>>>>> specifically. The number of GPIOs will be retrieved using
>>>>> gpiochip_get_ngpios() so there's no need to look it up in the software
>>>>> node.
>>>>>
>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> This works for me.
>>>> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
>>>> property all of a sudden just because it now exists as a device
>>>> property though... I kind of wish we had a way to opt out of exposing
>>>> this to all the sub-property paths. But it seems tiresome, so:
>>>>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>
>>>> Yours,
>>>> Linus Walleij
>>>
>>> That's not a problem - this property is not in any DT bindings and as
>>> such is not an allowed property in DT sources. For out-of-tree DTs? We
>>> don't care about those.
>> That's not true, we do care about implied ABI. Try changing/breaking
>> this later, when users complain their out of tree DTS is affected, and
>> explaining this all to Greg.
>>
> 
> Wait, seriously? I thought that the upstream bindings are the source
> of truth for device-tree sources...


They are, until they are not... ok, we don't really care that much about
out of tree DTS, but in-tree DTS still could use these and don't care
about bindings check, right?

Best regards,
Krzysztof
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Bartosz Golaszewski 3 months, 2 weeks ago
On Wed, Jun 25, 2025 at 12:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> >>>> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
> >>>> property all of a sudden just because it now exists as a device
> >>>> property though... I kind of wish we had a way to opt out of exposing
> >>>> this to all the sub-property paths. But it seems tiresome, so:
> >>>>
> >>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >>>>
> >>>> Yours,
> >>>> Linus Walleij
> >>>
> >>> That's not a problem - this property is not in any DT bindings and as
> >>> such is not an allowed property in DT sources. For out-of-tree DTs? We
> >>> don't care about those.
> >> That's not true, we do care about implied ABI. Try changing/breaking
> >> this later, when users complain their out of tree DTS is affected, and
> >> explaining this all to Greg.
> >>
> >
> > Wait, seriously? I thought that the upstream bindings are the source
> > of truth for device-tree sources...
>
>
> They are, until they are not... ok, we don't really care that much about
> out of tree DTS, but in-tree DTS still could use these and don't care
> about bindings check, right?
>

Could they though? I can imagine this happening by accident but in
general: you'd expect new sources to follow the bindings and be
verifiable against them? Otherwise, what's the point of the schema?

Bart
Re: [PATCH RFT 2/6] gpio: mmio: get chip label and GPIO base from device properties
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 25/06/2025 12:28, Bartosz Golaszewski wrote:
> On Wed, Jun 25, 2025 at 12:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>>>>>> I wouldn't be stoked to see device trees abusing the "gpio-mmio,base"
>>>>>> property all of a sudden just because it now exists as a device
>>>>>> property though... I kind of wish we had a way to opt out of exposing
>>>>>> this to all the sub-property paths. But it seems tiresome, so:
>>>>>>
>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>>
>>>>>> Yours,
>>>>>> Linus Walleij
>>>>>
>>>>> That's not a problem - this property is not in any DT bindings and as
>>>>> such is not an allowed property in DT sources. For out-of-tree DTs? We
>>>>> don't care about those.
>>>> That's not true, we do care about implied ABI. Try changing/breaking
>>>> this later, when users complain their out of tree DTS is affected, and
>>>> explaining this all to Greg.
>>>>
>>>
>>> Wait, seriously? I thought that the upstream bindings are the source
>>> of truth for device-tree sources...
>>
>>
>> They are, until they are not... ok, we don't really care that much about
>> out of tree DTS, but in-tree DTS still could use these and don't care
>> about bindings check, right?
>>
> 
> Could they though? I can imagine this happening by accident but in
> general: you'd expect new sources to follow the bindings and be

Yes, that is what I would expect.

> verifiable against them? Otherwise, what's the point of the schema?

Of course, but do you really think most SoC maintainers even run
dtbs_check on their existing or new code?

I think some maintainers pay attention, but RISC-V and my tree are the
only ones actually actively checking this for existing and new code.
Except me, no other maintainer even referenced maintainer-soc-clean-dts
profile.

I can easily imagine DTS with warnings and undocumented ABI gets merged
and then released in some kernel. And when you release such kernel in
v6.17 with DTS and drivers, isn't this already an implied ABI?

Best regards,
Krzysztof