| Message ID | 20210818081018.2620544-1-piyush.mehta@xilinx.com |
|---|---|
| Headers | show |
| Series | gpio: modepin: Add driver support for modepin GPIO controller | expand |
On 18.08.21 10:10, Piyush Mehta wrote: > This patch adds driver support for the zynqmp modepin GPIO controller. > GPIO modepin driver set and get the value and status of the PS_MODE pin, > based on device-tree pin configuration. These four mode pins are > configurable as input/output. The mode pin has a control register, which > have lower four-bits [0:3] are configurable as input/output, next four-bits > can be used for reading the data as input[4:7], and next setting the > output pin state output[8:11]. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > +/** > + * modepin_gpio_dir_in - Set the direction of the specified GPIO pin as input > + * @chip: gpio_chip instance to be worked on > + * @pin: gpio pin number within the device > + * > + * Return: 0 always > + */ > +static int modepin_gpio_dir_in(struct gpio_chip *chip, unsigned int pin) > +{ > + return 0; > +} You say the gpio controller can configure pins as inputs or outputs. Yet, .direction_input is doing nothing. So it's not clear to me, how this sequence could work: - set gpio output high (writes bootmode) - set gpio to input (no-op, pin will remain high, not high impedance) I didn't check the previous discussions, but if this indeed works as intended, the how should be written here into the driver. That is a more useful comment than kernel doc for a stub function.
On 18.08.21 11:38, Michal Simek wrote: > Hi Ahmad, > > On 8/18/21 11:00 AM, Ahmad Fatoum wrote: >> On 18.08.21 10:10, Piyush Mehta wrote: >>> This patch adds DT binding document for zynqmp modepin GPIO controller. >>> Modepin GPIO controller has four GPIO pins which can be configurable >>> as input or output. >>> >>> Modepin driver is a bridge between the peripheral driver and GPIO pins. >>> It has set and get APIs for accessing GPIO pins, based on the device-tree >>> entry of reset-gpio property in the peripheral driver, every pin can be >>> configured as input/output and trigger GPIO pin. >>> >>> For more information please refer zynqMp TRM link: >>> Link: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf >>> Chapter 2: Signals, Interfaces, and Pins >>> Table 2-2: Clock, Reset, and Configuration Pins - PS_MODE >>> >>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> >>> Acked-by: Michal Simek <michal.simek@xilinx.com> >>> --- >>> Changes in v2: >>> - Addressed review comments: Update commit message >>> >>> Review Comments: >>> https://lore.kernel.org/linux-arm-kernel/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#mbd1fbda813e33b19397b350bde75747c92a0d7e1 >>> https://lore.kernel.org/linux-arm-kernel/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#me82b1444ab3776162cdb0077dfc9256365c7e736 >>> >>> Changes in v3: >>> - Addressed Rob and Michal review comments: >>> - Update DT example. >>> >>> Review Comments: >>> https://lore.kernel.org/linux-arm-kernel/YRbBnRS0VosXcZWz@robh.at.kernel.org/ >>> https://lore.kernel.org/linux-arm-kernel/d71ad7f9-6972-8cc0-6dfb-b5306c9900d0@xilinx.com/ >>> --- >>> .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml | 41 ++++++++++++++++++++++ >>> .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml | 43 ++++++++++++++++++++++ >>> 1 file changed, 43 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml b/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml >>> new file mode 100644 >>> index 0000000..1442815 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml >>> @@ -0,0 +1,43 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: "http://devicetree.org/schemas/gpio/xlnx,zynqmp-gpio-modepin.yaml#" >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>> + >>> +title: ZynqMP Mode Pin GPIO controller >>> + >>> +description: >>> + PS_MODE is 4-bits boot mode pins sampled on POR deassertion. Mode Pin >>> + GPIO controller with configurable from numbers of pins (from 0 to 3 per >>> + PS_MODE). Every pin can be configured as input/output. >> So, at Linux runtime, someone decides to boot the system into e.g. a USB >> recovery mode and then toggles the appropriate GPIOs and does a system >> reset? >> >> If so, are you aware of the reboot mode[1] infrastructure? >> >> A reboot-mode-gpio driver on top of this GPIO controller would allow you >> to describe the supported reboot modes in the device tree and instead of >> exporting GPIOs to userspace, users can then just do >> >> systemctl restart recovery >> >> to toggle the appropriate bits. >> >> Also to be sure: PS_MODE are actual GPIO pins that you could toggle >> board level components with, right? i.e. it's not just a register that >> overrides the values read from the boot mode pins? (In the latter case >> a syscon-reboot-mode without GPIO controller would be the correct >> abstraction). >> >> [1]: drivers/power/reset/reboot-mode.c > > Thanks for these links. I wasn't aware about it. > But this device/IP is not working like this. Changing gpios to certain > state won't ensure that on reboot/reset (done in whatever way) won't > stay on values you chose. Ah, the "PS_MODE is 4-bits boot mode pins sampled on POR deassertion" part misled me. These pins are sampled on startup, but can afterwards be reused via talking to firmware. Thanks for clearing this up. > modepin gpio driver is at BOOT_PIN_CTRL 0xFF5E0250 > > (To be fair if you add additional external chip it could work like this > but I have never seen it). Ye, that would've been strange, that's why I asked. :) > But when you bring this up. Xilinx ZynqMP is providing a way how to > setup alternative boot mode which is done via > BOOT_MODE_USER 0xFF5E0200 > Bit 8 and 15-12. > Then you can setup any bootmode. > > ZynqMP supports couple of modes listed here > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-zynqmp/include/mach/hardware.h#L73 > > but again routing to this register needs to be done via firmware > interface but it should be done via separate driver. Yes. > Is there an option to setup whatever modes you like? > > I mean to simply cover all modes like this? > > mode-jtag = <0>; > mode-sd = <3>; > mode-sd1 = <5>; Yes, you can define the supported modes in the SoC dtsi and boards inherit that and can extend it as necessary. > And then users/customers can say what normal/recovery/test modes are. Yes, that would be nice. But after your clarification, I see that it's unrelated to this patch series. Binding is fine. Question on driver is still applicable. Cheers, Ahmad > > Thanks, > Michal >
Hi Ahmad, -----Original Message----- From: Ahmad Fatoum <a.fatoum@pengutronix.de> Sent: Wednesday, August 18, 2021 2:22 PM To: Piyush Mehta <piyushm@xilinx.com>; arnd@arndb.de; zou_wei@huawei.com; gregkh@linuxfoundation.org; linus.walleij@linaro.org; Michal Simek <michals@xilinx.com>; Jiaying Liang <jliang@xilinx.com>; iwamatsu@nigauri.org; bgolaszewski@baylibre.com; robh+dt@kernel.org; Rajan Vaja <RAJANV@xilinx.com> Cc: linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; git <git@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team <kernel@pengutronix.de> Subject: Re: [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller On 18.08.21 10:10, Piyush Mehta wrote: > This patch adds driver support for the zynqmp modepin GPIO controller. > GPIO modepin driver set and get the value and status of the PS_MODE > pin, based on device-tree pin configuration. These four mode pins are > configurable as input/output. The mode pin has a control register, > which have lower four-bits [0:3] are configurable as input/output, > next four-bits can be used for reading the data as input[4:7], and > next setting the output pin state output[8:11]. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > +/** > + * modepin_gpio_dir_in - Set the direction of the specified GPIO pin as input > + * @chip: gpio_chip instance to be worked on > + * @pin: gpio pin number within the device > + * > + * Return: 0 always > + */ > +static int modepin_gpio_dir_in(struct gpio_chip *chip, unsigned int > +pin) { > + return 0; > +} You say the gpio controller can configure pins as inputs or outputs. These pins are controller via firmware driver. We are updating BOOT_PIN_CTRL 0xFF5E0250 register. [0:3] = When 0, the pins will be inputs from the board to the PS. When 1, the PS will drive these pins [4:7] = Value captured from the mode pins [8:11] = Value driven onto the mode pins, when control register bit set out = 1 The lower four-bits [0:3] we can set either input and output, based on configuration we read pin as for input [4:7] and write on pin [8:11]. Example: If we want to configure pin 1 as output, then we will configure as [0:3]=[0100], for access pin will trigger upper bit [8:11]=[0100]. Based on https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf page 46 PS_MODE Input/Output Dedicated 4-bit boot mode pins sampled on POR deassertion Xilinx is using this pin for usb phy resets. Yet, .direction_input is doing nothing. So, it's not clear to me, how this sequence could work: - set gpio output high (writes bootmode) - set gpio to input (no-op, pin will remain high, not high impedance) I didn't check the previous discussions, but if this indeed works as intended, the how should be written here into the driver. That is a more useful comment than kernel doc for a stub function. -- 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 | Regards, Piyush Mehta
On Wed, Aug 18, 2021 at 10:11 AM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > > This patch adds driver support for the zynqmp modepin GPIO controller. > GPIO modepin driver set and get the value and status of the PS_MODE pin, > based on device-tree pin configuration. These four mode pins are > configurable as input/output. The mode pin has a control register, which > have lower four-bits [0:3] are configurable as input/output, next four-bits > can be used for reading the data as input[4:7], and next setting the > output pin state output[8:11]. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- Which tree should this go through? Bart
Hi Bart, On 8/23/21 10:02 AM, Bartosz Golaszewski wrote: > On Wed, Aug 18, 2021 at 10:11 AM Piyush Mehta <piyush.mehta@xilinx.com> wrote: >> >> This patch adds driver support for the zynqmp modepin GPIO controller. >> GPIO modepin driver set and get the value and status of the PS_MODE pin, >> based on device-tree pin configuration. These four mode pins are >> configurable as input/output. The mode pin has a control register, which >> have lower four-bits [0:3] are configurable as input/output, next four-bits >> can be used for reading the data as input[4:7], and next setting the >> output pin state output[8:11]. >> >> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> >> Acked-by: Michal Simek <michal.simek@xilinx.com> >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >> --- > > Which tree should this go through? I would prefer to go this via gpio tree. Thanks, Michal
On Wed, 18 Aug 2021 13:40:17 +0530, Piyush Mehta wrote: > This patch adds DT binding document for zynqmp modepin GPIO controller. > Modepin GPIO controller has four GPIO pins which can be configurable > as input or output. > > Modepin driver is a bridge between the peripheral driver and GPIO pins. > It has set and get APIs for accessing GPIO pins, based on the device-tree > entry of reset-gpio property in the peripheral driver, every pin can be > configured as input/output and trigger GPIO pin. > > For more information please refer zynqMp TRM link: > Link: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf > Chapter 2: Signals, Interfaces, and Pins > Table 2-2: Clock, Reset, and Configuration Pins - PS_MODE > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> > --- > Changes in v2: > - Addressed review comments: Update commit message > > Review Comments: > https://lore.kernel.org/linux-arm-kernel/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#mbd1fbda813e33b19397b350bde75747c92a0d7e1 > https://lore.kernel.org/linux-arm-kernel/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#me82b1444ab3776162cdb0077dfc9256365c7e736 > > Changes in v3: > - Addressed Rob and Michal review comments: > - Update DT example. > > Review Comments: > https://lore.kernel.org/linux-arm-kernel/YRbBnRS0VosXcZWz@robh.at.kernel.org/ > https://lore.kernel.org/linux-arm-kernel/d71ad7f9-6972-8cc0-6dfb-b5306c9900d0@xilinx.com/ > --- > .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml | 41 ++++++++++++++++++++++ > .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml | 43 ++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Mon, Aug 23, 2021 at 10:14 AM Michal Simek <michal.simek@xilinx.com> wrote: > > Hi Bart, > > On 8/23/21 10:02 AM, Bartosz Golaszewski wrote: > > On Wed, Aug 18, 2021 at 10:11 AM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > >> > >> This patch adds driver support for the zynqmp modepin GPIO controller. > >> GPIO modepin driver set and get the value and status of the PS_MODE pin, > >> based on device-tree pin configuration. These four mode pins are > >> configurable as input/output. The mode pin has a control register, which > >> have lower four-bits [0:3] are configurable as input/output, next four-bits > >> can be used for reading the data as input[4:7], and next setting the > >> output pin state output[8:11]. > >> > >> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > >> Acked-by: Michal Simek <michal.simek@xilinx.com> > >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >> --- > > > > Which tree should this go through? > > I would prefer to go this via gpio tree. > > Thanks, > Michal Sure, just make sure to get an Ack from Rob Herring on the DT bindings. Bart
On Wed, Sep 22, 2021 at 12:18 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > On Mon, Aug 23, 2021 at 10:14 AM Michal Simek <michal.simek@xilinx.com> wrote: > > > > Hi Bart, > > > > On 8/23/21 10:02 AM, Bartosz Golaszewski wrote: > > > On Wed, Aug 18, 2021 at 10:11 AM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > > >> > > >> This patch adds driver support for the zynqmp modepin GPIO controller. > > >> GPIO modepin driver set and get the value and status of the PS_MODE pin, > > >> based on device-tree pin configuration. These four mode pins are > > >> configurable as input/output. The mode pin has a control register, which > > >> have lower four-bits [0:3] are configurable as input/output, next four-bits > > >> can be used for reading the data as input[4:7], and next setting the > > >> output pin state output[8:11]. > > >> > > >> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > > >> Acked-by: Michal Simek <michal.simek@xilinx.com> > > >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > >> --- > > > > > > Which tree should this go through? > > > > I would prefer to go this via gpio tree. > > > > Thanks, > > Michal > > Sure, just make sure to get an Ack from Rob Herring on the DT bindings. > > Bart Nevermind - it's already there. Bart
On 9/22/21 12:21 PM, Bartosz Golaszewski wrote: > On Wed, Sep 22, 2021 at 12:18 PM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: >> >> On Mon, Aug 23, 2021 at 10:14 AM Michal Simek <michal.simek@xilinx.com> wrote: >>> >>> Hi Bart, >>> >>> On 8/23/21 10:02 AM, Bartosz Golaszewski wrote: >>>> On Wed, Aug 18, 2021 at 10:11 AM Piyush Mehta <piyush.mehta@xilinx.com> wrote: >>>>> >>>>> This patch adds driver support for the zynqmp modepin GPIO controller. >>>>> GPIO modepin driver set and get the value and status of the PS_MODE pin, >>>>> based on device-tree pin configuration. These four mode pins are >>>>> configurable as input/output. The mode pin has a control register, which >>>>> have lower four-bits [0:3] are configurable as input/output, next four-bits >>>>> can be used for reading the data as input[4:7], and next setting the >>>>> output pin state output[8:11]. >>>>> >>>>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> >>>>> Acked-by: Michal Simek <michal.simek@xilinx.com> >>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>> --- >>>> >>>> Which tree should this go through? >>> >>> I would prefer to go this via gpio tree. >>> >>> Thanks, >>> Michal >> >> Sure, just make sure to get an Ack from Rob Herring on the DT bindings. >> >> Bart > > Nevermind - it's already there. yes. that's what I thought. :-) Cheers, Michal