| Message ID | 20160919161314.25858-1-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: > The RPi firmware exposes all of the board's GPIO lines through > property calls. Linux chooses to control most lines directly through > the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we > need to access them through the firmware. > > Signed-off-by: Eric Anholt <eric@anholt.net> Aha > +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > @@ -0,0 +1,22 @@ > +Raspberry Pi power domain driver Really? :) > +Required properties: > + > +- compatible: Should be "raspberrypi,firmware-gpio" Usually this is vendor,compat, is the vendors name "raspberrypi"? > +- gpio-controller: Marks the device node as a gpio controller > +- #gpio-cells: Should be <2> for GPIO number and flags > +- ngpios: Number of GPIO lines to control. See gpio.txt Is this ever anything else than 8? Else omit it and hardcode 8 in the driver instead. > +- firmware: Reference to the RPi firmware device node Reference the DT binding for this. > +- raspberrypi,firmware-gpio-offset: > + Number the firmware uses for the first GPIO line > + controlled by this driver Does this differ between different instances of this hardware or can it just be open coded in the driver instead? Yours, Linus Walleij
Linus Walleij <linus.walleij@linaro.org> writes: > On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: > >> The RPi firmware exposes all of the board's GPIO lines through >> property calls. Linux chooses to control most lines directly through >> the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we >> need to access them through the firmware. >> >> Signed-off-by: Eric Anholt <eric@anholt.net> > > Aha > >> +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >> @@ -0,0 +1,22 @@ >> +Raspberry Pi power domain driver > > Really? :) Thanks. >> +Required properties: >> + >> +- compatible: Should be "raspberrypi,firmware-gpio" > > Usually this is vendor,compat, is the vendors name "raspberrypi"? Yes, this driver is for part of the Raspberry Pi Foundation's firmware code (you can find the same pattern in the firmware and firmware power domain drivers). >> +- gpio-controller: Marks the device node as a gpio controller >> +- #gpio-cells: Should be <2> for GPIO number and flags >> +- ngpios: Number of GPIO lines to control. See gpio.txt > > Is this ever anything else than 8? Else omit it and hardcode > 8 in the driver instead. (see below) >> +- firmware: Reference to the RPi firmware device node > > Reference the DT binding for this. > >> +- raspberrypi,firmware-gpio-offset: >> + Number the firmware uses for the first GPIO line >> + controlled by this driver > > Does this differ between different instances of this hardware or > can it just be open coded in the driver instead? This is which range (128-135) of the firmware's GPIOs we're controlling. If another GPIO expander appears later (quite believable, I think they're down to 1 spare line on this expander), then we would just make another node with a new offset and ngpios for that expander. Sort of related: I also worry that we have races with the firmware for the platform GPIO bits, since both ARM and firmware are doing RMWs (or, even worse, maybe just Ws?) of the registers controlled by the pinctrl driver. Hopefully I can get the firmware to pass control of devices like this over to Linux, with firmware making requests to us, but I don't know if that will happen and we may need to access other GPIOs using this interface :(
On Fri, Sep 23, 2016 at 3:08 PM, Eric Anholt <eric@anholt.net> wrote: > Sort of related: I also worry that we have races with the firmware for > the platform GPIO bits, since both ARM and firmware are doing RMWs (or, > even worse, maybe just Ws?) of the registers controlled by the pinctrl > driver. Hopefully I can get the firmware to pass control of devices > like this over to Linux, with firmware making requests to us, but I > don't know if that will happen and we may need to access other GPIOs > using this interface :( For the race with firmware I have no good solutions, it's just one of those things I guess :( When two kernel drivers compete about registers, say for example one driver needs to RMW bits 0-5 and another driver needs to RMW bits 7-11, the right solution is usually to use syscon and then regmap-mmio will deal with the concurrency as part of regmap_update_bits() etc. Yours, Linus Walleij
Hi Eric, > Eric Anholt <eric@anholt.net> hat am 19. September 2016 um 18:13 geschrieben: > > > The RPi firmware exposes all of the board's GPIO lines through > property calls. Linux chooses to control most lines directly through > the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we > need to access them through the firmware. > > Signed-off-by: Eric Anholt <eric@anholt.net> > --- > .../bindings/gpio/gpio-raspberrypi-firmware.txt | 22 > ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > > diff --git > a/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > new file mode 100644 > index 000000000000..2b635c23a6f8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > @@ -0,0 +1,22 @@ > +Raspberry Pi power domain driver > + > +Required properties: > + > +- compatible: Should be "raspberrypi,firmware-gpio" i think the compatible should be more specific like raspberrypi,rpi3-firmware-gpio and all information which aren't requestable from the firmware should be stored in a info structure. This makes the driver easier to extend in the future by adding new compatibles and their info structures. > +- gpio-controller: Marks the device node as a gpio controller > +- #gpio-cells: Should be <2> for GPIO number and flags > +- ngpios: Number of GPIO lines to control. See gpio.txt > +- firmware: Reference to the RPi firmware device node > +- raspberrypi,firmware-gpio-offset: > + Number the firmware uses for the first GPIO line > + controlled by this driver We should avoid such properties because they don't really describe the hardware. Stefan
On 09/23/2016 12:39 PM, Stefan Wahren wrote: > Hi Eric, > >> Eric Anholt <eric@anholt.net> hat am 19. September 2016 um 18:13 geschrieben: >> >> >> The RPi firmware exposes all of the board's GPIO lines through >> property calls. Linux chooses to control most lines directly through >> the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we >> need to access them through the firmware. >> >> Signed-off-by: Eric Anholt <eric@anholt.net> >> --- >> .../bindings/gpio/gpio-raspberrypi-firmware.txt | 22 >> ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >> >> diff --git >> a/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >> b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >> new file mode 100644 >> index 000000000000..2b635c23a6f8 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >> @@ -0,0 +1,22 @@ >> +Raspberry Pi power domain driver >> + >> +Required properties: >> + >> +- compatible: Should be "raspberrypi,firmware-gpio" > > i think the compatible should be more specific like > > raspberrypi,rpi3-firmware-gpio > > and all information which aren't requestable from the firmware should be stored > in a info structure. This makes the driver easier to extend in the future by > adding new compatibles and their info structures. Is this actually specific to the Pi3 at all? Isn't the FW the same across all Pis; the part that's specific to the Pi3 is whether it's useful to use that API? As such, I'd suggest just raspberrypi,firmware-gpio as the compatible value.
On 09/23/2016 07:08 AM, Eric Anholt wrote: > Linus Walleij <linus.walleij@linaro.org> writes: > >> On Mon, Sep 19, 2016 at 6:13 PM, Eric Anholt <eric@anholt.net> wrote: >> >>> The RPi firmware exposes all of the board's GPIO lines through >>> property calls. Linux chooses to control most lines directly through >>> the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we >>> need to access them through the firmware. >>> >>> Signed-off-by: Eric Anholt <eric@anholt.net> >> >> Aha >> >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >>> @@ -0,0 +1,22 @@ >>> +Raspberry Pi power domain driver >> >> Really? :) > > Thanks. > >>> +Required properties: >>> + >>> +- compatible: Should be "raspberrypi,firmware-gpio" >> >> Usually this is vendor,compat, is the vendors name "raspberrypi"? > > Yes, this driver is for part of the Raspberry Pi Foundation's firmware > code (you can find the same pattern in the firmware and firmware power > domain drivers). > >>> +- gpio-controller: Marks the device node as a gpio controller >>> +- #gpio-cells: Should be <2> for GPIO number and flags >>> +- ngpios: Number of GPIO lines to control. See gpio.txt >> >> Is this ever anything else than 8? Else omit it and hardcode >> 8 in the driver instead. > > (see below) > >>> +- firmware: Reference to the RPi firmware device node >> >> Reference the DT binding for this. >> >>> +- raspberrypi,firmware-gpio-offset: >>> + Number the firmware uses for the first GPIO line >>> + controlled by this driver >> >> Does this differ between different instances of this hardware or >> can it just be open coded in the driver instead? > > This is which range (128-135) of the firmware's GPIOs we're controlling. > If another GPIO expander appears later (quite believable, I think > they're down to 1 spare line on this expander), then we would just make > another node with a new offset and ngpios for that expander. Why would we make another node for that? Wouldn't we always have a single node to represent the FW's control over GPIOs, and have that node expose all GPIOs that the FW supports. Which GPIO IDs clients actually use will simply be determined by the HW schematic, and kernel-side SW would just act as a conduit to pass those IDs between clients and the FW. > Sort of related: I also worry that we have races with the firmware for > the platform GPIO bits, since both ARM and firmware are doing RMWs (or, > even worse, maybe just Ws?) of the registers controlled by the pinctrl > driver. Hopefully I can get the firmware to pass control of devices > like this over to Linux, with firmware making requests to us, but I > don't know if that will happen and we may need to access other GPIOs > using this interface :( Aren't there write-to-set/write-to-clear registers? If not, then either FW owns everything in a particular register or Linux does; the HW won't allow sharing.
> Stephen Warren <swarren@wwwdotorg.org> hat am 26. September 2016 um 18:38 > geschrieben: > > > On 09/23/2016 12:39 PM, Stefan Wahren wrote: > > Hi Eric, > > > >> Eric Anholt <eric@anholt.net> hat am 19. September 2016 um 18:13 > >> geschrieben: > >> > >> > >> The RPi firmware exposes all of the board's GPIO lines through > >> property calls. Linux chooses to control most lines directly through > >> the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we > >> need to access them through the firmware. > >> > >> Signed-off-by: Eric Anholt <eric@anholt.net> > >> --- > >> .../bindings/gpio/gpio-raspberrypi-firmware.txt | 22 > >> ++++++++++++++++++++++ > >> 1 file changed, 22 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > >> > >> diff --git > >> a/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > >> b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > >> new file mode 100644 > >> index 000000000000..2b635c23a6f8 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt > >> @@ -0,0 +1,22 @@ > >> +Raspberry Pi power domain driver > >> + > >> +Required properties: > >> + > >> +- compatible: Should be "raspberrypi,firmware-gpio" > > > > i think the compatible should be more specific like > > > > raspberrypi,rpi3-firmware-gpio > > > > and all information which aren't requestable from the firmware should be > > stored > > in a info structure. This makes the driver easier to extend in the future by > > adding new compatibles and their info structures. > > Is this actually specific to the Pi3 at all? AFAIK only the Raspberry Pi 3 has a GPIO expander which is accessible via the common FW. My suggestion tries to follow the basic guideline "A precise compatible string is better than a vague one" from Device Tree for Dummies [1]. So in case the next Raspberry Pi would have a different GPIO expander with different parameters we could add a new compatible. But you are right the word order in "rpi3-firmware-gpio" suggests that there are different FW which is wrong. At the end it's only a compatible string. So no strong opinion about the naming. [1] - https://events.linuxfoundation.org/sites/events/files/slides/petazzoni-device-tree-dummies.pdf > Isn't the FW the same > across all Pis; the part that's specific to the Pi3 is whether it's > useful to use that API? > > As such, I'd suggest just raspberrypi,firmware-gpio as the compatible value. >
On 09/26/2016 12:42 PM, Stefan Wahren wrote: > >> Stephen Warren <swarren@wwwdotorg.org> hat am 26. September 2016 um 18:38 >> geschrieben: >> >> >> On 09/23/2016 12:39 PM, Stefan Wahren wrote: >>> Hi Eric, >>> >>>> Eric Anholt <eric@anholt.net> hat am 19. September 2016 um 18:13 >>>> geschrieben: >>>> >>>> >>>> The RPi firmware exposes all of the board's GPIO lines through >>>> property calls. Linux chooses to control most lines directly through >>>> the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we >>>> need to access them through the firmware. >>>> >>>> Signed-off-by: Eric Anholt <eric@anholt.net> >>>> --- >>>> .../bindings/gpio/gpio-raspberrypi-firmware.txt | 22 >>>> ++++++++++++++++++++++ >>>> 1 file changed, 22 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >>>> b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >>>> new file mode 100644 >>>> index 000000000000..2b635c23a6f8 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt >>>> @@ -0,0 +1,22 @@ >>>> +Raspberry Pi power domain driver >>>> + >>>> +Required properties: >>>> + >>>> +- compatible: Should be "raspberrypi,firmware-gpio" >>> >>> i think the compatible should be more specific like >>> >>> raspberrypi,rpi3-firmware-gpio >>> >>> and all information which aren't requestable from the firmware should be >>> stored >>> in a info structure. This makes the driver easier to extend in the future by >>> adding new compatibles and their info structures. >> >> Is this actually specific to the Pi3 at all? > > AFAIK only the Raspberry Pi 3 has a GPIO expander which is accessible via the > common FW. My suggestion tries to follow the basic guideline "A precise > compatible string is better than a vague one" from Device Tree for Dummies [1]. > So in case the next Raspberry Pi would have a different GPIO expander with > different parameters we could add a new compatible. > > But you are right the word order in "rpi3-firmware-gpio" suggests that there are > different FW which is wrong. At the end it's only a compatible string. So no > strong opinion about the naming. > > [1] - > https://events.linuxfoundation.org/sites/events/files/slides/petazzoni-device-tree-dummies.pdf To deal with that requirement, what you'd want is the following: RPi 1: "raspberrypi,firmware-gpio" RPi 2: "raspberrypi,rpi2-firmware-gpio", "raspberrypi,firmware-gpio" RPi 3: "raspberrypi,rpi3-firmware-gpio", "raspberrypi,firmware-gpio" Compatible values should always list both the most specific value and the baseline value that it's 100% backwards compatible with. However, I don't think this argument applies in this case. It isn't the case that there's a separate Pi1 FW, Pi2 FW, Pi3 FW. Rather, there is a single FW image that runs on all (currently existing) Pis. As such, I believe that using solely "raspberrypi,firmware-gpio" is appropriate everywhere, since the thing being described is always the exact same thing with no variance. This contrasts with HW modules in the SoC, since the different Pis really do have a different SoC, and hence potentially different HW modules, so e.g. compatible = "brcm,bcm2837-i2c", "brcm,bcm2835-i2c" could actually be useful. >> Isn't the FW the same >> across all Pis; the part that's specific to the Pi3 is whether it's >> useful to use that API? >> >> As such, I'd suggest just raspberrypi,firmware-gpio as the compatible value.
diff --git a/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt new file mode 100644 index 000000000000..2b635c23a6f8 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt @@ -0,0 +1,22 @@ +Raspberry Pi power domain driver + +Required properties: + +- compatible: Should be "raspberrypi,firmware-gpio" +- gpio-controller: Marks the device node as a gpio controller +- #gpio-cells: Should be <2> for GPIO number and flags +- ngpios: Number of GPIO lines to control. See gpio.txt +- firmware: Reference to the RPi firmware device node +- raspberrypi,firmware-gpio-offset: + Number the firmware uses for the first GPIO line + controlled by this driver + +Example: +fxl6408: firmware-gpio-128 { + compatible = "raspberrypi,firmware-gpio"; + gpio-controller; + #gpio-cells = <2>; + firmware = <&firmware>; + ngpios = <8>; + raspberrypi,firmware-gpio-offset = <128>; +};
The RPi firmware exposes all of the board's GPIO lines through property calls. Linux chooses to control most lines directly through the pinctrl driver, but for the FXL6408 GPIO expander on the Pi3, we need to access them through the firmware. Signed-off-by: Eric Anholt <eric@anholt.net> --- .../bindings/gpio/gpio-raspberrypi-firmware.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-raspberrypi-firmware.txt