LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Michal Simek <michal.simek@xilinx.com>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Michal Simek <michal.simek@xilinx.com>,
	Piyush Mehta <piyush.mehta@xilinx.com>, <arnd@arndb.de>,
	<zou_wei@huawei.com>, <gregkh@linuxfoundation.org>,
	<linus.walleij@linaro.org>, <wendy.liang@xilinx.com>,
	<iwamatsu@nigauri.org>, <bgolaszewski@baylibre.com>,
	<robh+dt@kernel.org>, <rajan.vaja@xilinx.com>
Cc: <linux-gpio@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<git@xilinx.com>, <sgoud@xilinx.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>
Subject: Re: [PATCH V3 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin
Date: Wed, 18 Aug 2021 12:01:19 +0200	[thread overview]
Message-ID: <c53d7b06-e619-45c5-8b93-d1688792f270@xilinx.com> (raw)
In-Reply-To: <4a724736-8bb9-a21d-974c-d9598c3d7b37@pengutronix.de>



On 8/18/21 11:55 AM, Ahmad Fatoum wrote:
> 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.

yes

>> 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. :)

No issue at all.

> 
>> 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.

ok.

> 
>> 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.

I remember any discussion about it between Piyush and Linus and I will
let Piyush to handle it.

Thanks,
Michal

  reply	other threads:[~2021-08-18 10:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18  8:10 [PATCH V3 0/3] gpio: modepin: Add driver support for modepin GPIO controller Piyush Mehta
2021-08-18  8:10 ` [PATCH V3 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin Piyush Mehta
2021-08-18  8:10 ` [PATCH V3 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin Piyush Mehta
2021-08-18  9:00   ` Ahmad Fatoum
2021-08-18  9:38     ` Michal Simek
2021-08-18  9:55       ` Ahmad Fatoum
2021-08-18 10:01         ` Michal Simek [this message]
2021-08-23 18:09   ` Rob Herring
2021-08-18  8:10 ` [PATCH V3 3/3] gpio: modepin: Add driver support for modepin GPIO controller Piyush Mehta
2021-08-18  8:52   ` Ahmad Fatoum
2021-08-18 10:09     ` Piyush Mehta
2021-08-18 13:05       ` Ahmad Fatoum
2021-08-23  8:02   ` Bartosz Golaszewski
2021-08-23  8:14     ` Michal Simek
2021-09-22 10:18       ` Bartosz Golaszewski
2021-09-22 10:21         ` Bartosz Golaszewski
2021-09-22 10:23           ` Michal Simek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c53d7b06-e619-45c5-8b93-d1688792f270@xilinx.com \
    --to=michal.simek@xilinx.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=arnd@arndb.de \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=git@xilinx.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=iwamatsu@nigauri.org \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=piyush.mehta@xilinx.com \
    --cc=rajan.vaja@xilinx.com \
    --cc=robh+dt@kernel.org \
    --cc=sgoud@xilinx.com \
    --cc=wendy.liang@xilinx.com \
    --cc=zou_wei@huawei.com \
    --subject='Re: [PATCH V3 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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 NNTP newsgroup(s).