* [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges
@ 2024-04-24 18:50 Doug Berger
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Doug Berger @ 2024-04-24 18:50 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, bcm-kernel-feedback-list, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel, Doug Berger
The Raspberry Pi 5 includes Broadcom STB GPIO IP as well as
Broadcom 2712 pin controller IP.
The community has expressed interest in linking the two drivers
with the "gpio-ranges" property in device tree. This commit
stack implements the necessary changes.
Doug Berger (3):
dt-bindings: gpio: brcmstb: add gpio-ranges
gpio: of: support gpio-ranges for multiple gpiochip devices
gpio: brcmstb: add support for gpio-ranges
.../bindings/gpio/brcm,brcmstb-gpio.yaml | 3 +++
drivers/gpio/gpio-brcmstb.c | 2 ++
drivers/gpio/gpiolib-of.c | 23 +++++++++++++++++--
3 files changed, 26 insertions(+), 2 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges
2024-04-24 18:50 [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
@ 2024-04-24 18:50 ` Doug Berger
2024-04-24 23:11 ` Florian Fainelli
` (2 more replies)
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
` (2 subsequent siblings)
3 siblings, 3 replies; 14+ messages in thread
From: Doug Berger @ 2024-04-24 18:50 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, bcm-kernel-feedback-list, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel, Doug Berger
Add optional gpio-ranges device-tree property to the Broadcom
Set-Top-Box GPIO controller.
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml
index a1e71c974e79..f096f286da19 100644
--- a/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/brcm,brcmstb-gpio.yaml
@@ -62,6 +62,8 @@ properties:
interrupt-controller: true
+ gpio-ranges: true
+
wakeup-source:
type: boolean
description: >
@@ -88,6 +90,7 @@ examples:
interrupt-parent = <&irq0_intc>;
interrupts = <0x6>;
brcm,gpio-bank-widths = <32 32 32 24>;
+ gpio-ranges = <&pinctrl 0 0 120>;
};
upg_gio_aon: gpio@f04172c0 {
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
2024-04-24 18:50 [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
@ 2024-04-24 18:50 ` Doug Berger
2024-04-24 23:14 ` Florian Fainelli
` (2 more replies)
2024-04-24 18:50 ` [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
2024-04-26 7:33 ` [PATCH 0/3] " Bartosz Golaszewski
3 siblings, 3 replies; 14+ messages in thread
From: Doug Berger @ 2024-04-24 18:50 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, bcm-kernel-feedback-list, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel, Doug Berger
Some drivers (e.g. gpio-mt7621 and gpio-brcmstb) have multiple
gpiochip banks within a single device. Unfortunately, the
gpio-ranges property of the device node was being applied to
every gpiochip of the device with device relative GPIO offset
values rather than gpiochip relative GPIO offset values.
This commit makes use of the gpio_chip offset value which can be
non-zero for such devices to split the device node gpio-ranges
property into GPIO offset ranges that can be applied to each
of the relevant gpiochips of the device.
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
drivers/gpio/gpiolib-of.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index cb0cefaec37e..d75f6ee37028 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1037,7 +1037,7 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
struct of_phandle_args pinspec;
struct pinctrl_dev *pctldev;
struct device_node *np;
- int index = 0, ret;
+ int index = 0, ret, trim;
const char *name;
static const char group_names_propname[] = "gpio-ranges-group-names";
struct property *group_names;
@@ -1059,7 +1059,14 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
if (!pctldev)
return -EPROBE_DEFER;
+ /* Ignore ranges outside of this GPIO chip */
+ if (pinspec.args[0] >= (chip->offset + chip->ngpio))
+ continue;
+ if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
+ continue;
+
if (pinspec.args[2]) {
+ /* npins != 0: linear range */
if (group_names) {
of_property_read_string_index(np,
group_names_propname,
@@ -1070,7 +1077,19 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
break;
}
}
- /* npins != 0: linear range */
+
+ /* Trim the range to fit this GPIO chip */
+ if (chip->offset > pinspec.args[0]) {
+ trim = chip->offset - pinspec.args[0];
+ pinspec.args[2] -= trim;
+ pinspec.args[1] += trim;
+ pinspec.args[0] = 0;
+ } else {
+ pinspec.args[0] -= chip->offset;
+ }
+ if ((pinspec.args[0] + pinspec.args[2]) > chip->ngpio)
+ pinspec.args[2] = chip->ngpio - pinspec.args[0];
+
ret = gpiochip_add_pin_range(chip,
pinctrl_dev_get_devname(pctldev),
pinspec.args[0],
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges
2024-04-24 18:50 [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
@ 2024-04-24 18:50 ` Doug Berger
2024-04-24 23:12 ` Florian Fainelli
2024-04-26 7:33 ` [PATCH 0/3] " Bartosz Golaszewski
3 siblings, 1 reply; 14+ messages in thread
From: Doug Berger @ 2024-04-24 18:50 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, bcm-kernel-feedback-list, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel, Doug Berger
A pin controller device mapped with the gpio-ranges property
will need implementations of the .request and .free members of
the gpiochip.
Signed-off-by: Doug Berger <opendmb@gmail.com>
Tested-by: Phil Elwell <phil@raspberrypi.com>
---
drivers/gpio/gpio-brcmstb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 790cb278b72a..8dce78ea7139 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -694,6 +694,8 @@ static int brcmstb_gpio_probe(struct platform_device *pdev)
/* not all ngpio lines are valid, will use bank width later */
gc->ngpio = MAX_GPIO_PER_BANK;
gc->offset = bank->id * MAX_GPIO_PER_BANK;
+ gc->request = gpiochip_generic_request;
+ gc->free = gpiochip_generic_free;
if (priv->parent_irq > 0)
gc->to_irq = brcmstb_gpio_to_irq;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
@ 2024-04-24 23:11 ` Florian Fainelli
2024-04-25 6:59 ` Krzysztof Kozlowski
2024-05-03 7:57 ` Linus Walleij
2 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2024-04-24 23:11 UTC (permalink / raw)
To: Doug Berger, Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 257 bytes --]
On 4/24/24 11:50, Doug Berger wrote:
> Add optional gpio-ranges device-tree property to the Broadcom
> Set-Top-Box GPIO controller.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges
2024-04-24 18:50 ` [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
@ 2024-04-24 23:12 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2024-04-24 23:12 UTC (permalink / raw)
To: Doug Berger, Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
On 4/24/24 11:50, Doug Berger wrote:
> A pin controller device mapped with the gpio-ranges property
> will need implementations of the .request and .free members of
> the gpiochip.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> Tested-by: Phil Elwell <phil@raspberrypi.com>
Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
@ 2024-04-24 23:14 ` Florian Fainelli
2024-04-26 7:32 ` Bartosz Golaszewski
2024-05-03 8:25 ` Linus Walleij
2 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2024-04-24 23:14 UTC (permalink / raw)
To: Doug Berger, Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
On 4/24/24 11:50, Doug Berger wrote:
> Some drivers (e.g. gpio-mt7621 and gpio-brcmstb) have multiple
> gpiochip banks within a single device. Unfortunately, the
> gpio-ranges property of the device node was being applied to
> every gpiochip of the device with device relative GPIO offset
> values rather than gpiochip relative GPIO offset values.
>
> This commit makes use of the gpio_chip offset value which can be
> non-zero for such devices to split the device node gpio-ranges
> property into GPIO offset ranges that can be applied to each
> of the relevant gpiochips of the device.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
2024-04-24 23:11 ` Florian Fainelli
@ 2024-04-25 6:59 ` Krzysztof Kozlowski
2024-05-03 7:57 ` Linus Walleij
2 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-04-25 6:59 UTC (permalink / raw)
To: Doug Berger, Linus Walleij, Bartosz Golaszewski
Cc: Phil Elwell, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Florian Fainelli, bcm-kernel-feedback-list, linux-gpio,
devicetree, linux-arm-kernel, linux-kernel
On 24/04/2024 20:50, Doug Berger wrote:
> Add optional gpio-ranges device-tree property to the Broadcom
> Set-Top-Box GPIO controller.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
2024-04-24 23:14 ` Florian Fainelli
@ 2024-04-26 7:32 ` Bartosz Golaszewski
2024-05-03 8:25 ` Linus Walleij
2 siblings, 0 replies; 14+ messages in thread
From: Bartosz Golaszewski @ 2024-04-26 7:32 UTC (permalink / raw)
To: Doug Berger
Cc: Linus Walleij, Phil Elwell, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli, bcm-kernel-feedback-list,
linux-gpio, devicetree, linux-arm-kernel, linux-kernel
On Wed, Apr 24, 2024 at 8:51 PM Doug Berger <opendmb@gmail.com> wrote:
>
> Some drivers (e.g. gpio-mt7621 and gpio-brcmstb) have multiple
> gpiochip banks within a single device. Unfortunately, the
> gpio-ranges property of the device node was being applied to
> every gpiochip of the device with device relative GPIO offset
> values rather than gpiochip relative GPIO offset values.
>
> This commit makes use of the gpio_chip offset value which can be
> non-zero for such devices to split the device node gpio-ranges
> property into GPIO offset ranges that can be applied to each
> of the relevant gpiochips of the device.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
This is a good improvement, thanks!
Bart
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges
2024-04-24 18:50 [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
` (2 preceding siblings ...)
2024-04-24 18:50 ` [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
@ 2024-04-26 7:33 ` Bartosz Golaszewski
2024-05-03 8:29 ` Linus Walleij
3 siblings, 1 reply; 14+ messages in thread
From: Bartosz Golaszewski @ 2024-04-26 7:33 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Doug Berger
Cc: Bartosz Golaszewski, Phil Elwell, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Wed, 24 Apr 2024 11:50:36 -0700, Doug Berger wrote:
> The Raspberry Pi 5 includes Broadcom STB GPIO IP as well as
> Broadcom 2712 pin controller IP.
>
> The community has expressed interest in linking the two drivers
> with the "gpio-ranges" property in device tree. This commit
> stack implements the necessary changes.
>
> [...]
Applied, thanks!
[1/3] dt-bindings: gpio: brcmstb: add gpio-ranges
commit: 7c66f8173360556ac0c3c38a91234af5a0a5a4a9
[2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
commit: e818cd3c8a345c046edff00b5ad0be4d39f7e4d4
[3/3] gpio: brcmstb: add support for gpio-ranges
commit: 5539287ca65686f478e058a1e939294cb5682426
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
2024-04-24 23:11 ` Florian Fainelli
2024-04-25 6:59 ` Krzysztof Kozlowski
@ 2024-05-03 7:57 ` Linus Walleij
2 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2024-05-03 7:57 UTC (permalink / raw)
To: Doug Berger
Cc: Bartosz Golaszewski, Phil Elwell, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
On Wed, Apr 24, 2024 at 8:51 PM Doug Berger <opendmb@gmail.com> wrote:
> Add optional gpio-ranges device-tree property to the Broadcom
> Set-Top-Box GPIO controller.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
2024-04-24 23:14 ` Florian Fainelli
2024-04-26 7:32 ` Bartosz Golaszewski
@ 2024-05-03 8:25 ` Linus Walleij
2024-05-03 20:21 ` Doug Berger
2 siblings, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2024-05-03 8:25 UTC (permalink / raw)
To: Doug Berger
Cc: Bartosz Golaszewski, Phil Elwell, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
Hi Dough,
thanks for your patch!
I'm a bit confused here:
On Wed, Apr 24, 2024 at 8:51 PM Doug Berger <opendmb@gmail.com> wrote:
> + /* Ignore ranges outside of this GPIO chip */
> + if (pinspec.args[0] >= (chip->offset + chip->ngpio))
> + continue;
> + if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
> + continue;
Here pinspec.args[0] and [2] comes directly from the device tree.
The documentation in Documentation/devicetree/bindings/gpio/gpio.txt
says:
> 2.2) Ordinary (numerical) GPIO ranges
> -------------------------------------
>
> It is useful to represent which GPIOs correspond to which pins on which pin
> controllers. The gpio-ranges property described below represents this with
> a discrete set of ranges mapping pins from the pin controller local number space
> to pins in the GPIO controller local number space.
>
> The format is: <[pin controller phandle], [GPIO controller offset],
> [pin controller offset], [number of pins]>;
>
> The GPIO controller offset pertains to the GPIO controller node containing the
> range definition.
So I do not understand how pinspec[0] and [2] can ever be compared
to something involving chip->offset which is a Linux-specific offset.
It rather looks like you are trying to accomodate the Linux numberspace
in the ranges, which it was explicitly designed to avoid.
I just don't get it.
So NACK until I understand what is going on here.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges
2024-04-26 7:33 ` [PATCH 0/3] " Bartosz Golaszewski
@ 2024-05-03 8:29 ` Linus Walleij
0 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2024-05-03 8:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Doug Berger, Bartosz Golaszewski, Phil Elwell, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
On Fri, Apr 26, 2024 at 9:33 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> [2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
> commit: e818cd3c8a345c046edff00b5ad0be4d39f7e4d4
I'm not sure this is a good idea. To me it looks like the commit violates
the device tree bindings, which says offsets are on the local GPIO chip
and turn them into offsets in the Linux GPIO numberspace.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices
2024-05-03 8:25 ` Linus Walleij
@ 2024-05-03 20:21 ` Doug Berger
0 siblings, 0 replies; 14+ messages in thread
From: Doug Berger @ 2024-05-03 20:21 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, Phil Elwell, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
bcm-kernel-feedback-list, linux-gpio, devicetree,
linux-arm-kernel, linux-kernel
On 5/3/2024 1:25 AM, Linus Walleij wrote:
> Hi Dough,
>
> thanks for your patch!
Thanks for your review!
>
> I'm a bit confused here:
"Communication is hard" and I may be confused about your confusion, but
hopefully we can work it out.
>
> On Wed, Apr 24, 2024 at 8:51 PM Doug Berger <opendmb@gmail.com> wrote:
>
>
>> + /* Ignore ranges outside of this GPIO chip */
>> + if (pinspec.args[0] >= (chip->offset + chip->ngpio))
>> + continue;
>> + if (pinspec.args[0] + pinspec.args[2] <= chip->offset)
>> + continue;
>
> Here pinspec.args[0] and [2] comes directly from the device tree.
>
> The documentation in Documentation/devicetree/bindings/gpio/gpio.txt
> says:
>
>> 2.2) Ordinary (numerical) GPIO ranges
>> -------------------------------------
>>
>> It is useful to represent which GPIOs correspond to which pins on which pin
>> controllers. The gpio-ranges property described below represents this with
>> a discrete set of ranges mapping pins from the pin controller local number space
>> to pins in the GPIO controller local number space.
>>
>> The format is: <[pin controller phandle], [GPIO controller offset],
>> [pin controller offset], [number of pins]>;
>>
>> The GPIO controller offset pertains to the GPIO controller node containing the
>> range definition.
I think we are in agreement here. For extra clarity, I will add that in
my understanding pinspec.args[0] corresponds to [GPIO controller offset]
and pinspec.args[2] corresponds to [number of pins].
>
> So I do not understand how pinspec[0] and [2] can ever be compared
> to something involving chip->offset which is a Linux-specific offset.
>
> It rather looks like you are trying to accomodate the Linux numberspace
> in the ranges, which it was explicitly designed to avoid.
The struct gpio_chip documentation in include/linux/gpio/driver.h says:
> * @offset: when multiple gpio chips belong to the same device this
> * can be used as offset within the device so friendly names can
> * be properly assigned.
It is my understanding that this value represents the offset of a
gpiochip relative to the GPIO controller device defined by the GPIO
controller node in device tree. This puts it in the same number space as
[GPIO controller offset]. I believe it was introduced for the specific
purpose of translating [GPIO controller offset] values into
Linux-specific offsets, which is why it is being reused for that purpose
in this patch.
For GPIO Controllers that contain a single gpiochip the 'offset' member
is 0 and the device tree node offsets can be applied directly to the
gpiochip. However, when a GPIO Controller contains multiple gpiochips,
the device tree node offsets must be translated to each individual gpiochip.
>
> I just don't get it.
>
> So NACK until I understand what is going on here.
>
> Yours,
> Linus Walleij
I hope it makes sense now, but if not please help me understand what I
may be missing.
Thanks,
Doug
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-05-03 20:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 18:50 [PATCH 0/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
2024-04-24 18:50 ` [PATCH 1/3] dt-bindings: gpio: brcmstb: add gpio-ranges Doug Berger
2024-04-24 23:11 ` Florian Fainelli
2024-04-25 6:59 ` Krzysztof Kozlowski
2024-05-03 7:57 ` Linus Walleij
2024-04-24 18:50 ` [PATCH 2/3] gpio: of: support gpio-ranges for multiple gpiochip devices Doug Berger
2024-04-24 23:14 ` Florian Fainelli
2024-04-26 7:32 ` Bartosz Golaszewski
2024-05-03 8:25 ` Linus Walleij
2024-05-03 20:21 ` Doug Berger
2024-04-24 18:50 ` [PATCH 3/3] gpio: brcmstb: add support for gpio-ranges Doug Berger
2024-04-24 23:12 ` Florian Fainelli
2024-04-26 7:33 ` [PATCH 0/3] " Bartosz Golaszewski
2024-05-03 8:29 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).