[PATCH V2 0/3] gpio: modepin: Add driver support for modepin GPIO controller

[PATCH V2 0/3] gpio: modepin: Add driver support for modepin GPIO controller

[Piyush Mehta]

This patch adds support for the zynqmp modepin GPIO controller and
documented for the same. GPIO modepin driver set and get the value and
status of the PS_MODE pin, based on device-tree pin configuration.
These four-bits boot-mode pins are dedicated configurable as input/output.
After the stabilization of the system,these mode pins are sampled.

To access GPIO pins, added Xilinx ZynqMP firmware MDIO API support to
set and get PS_MODE pins value and status. These APIs are interface
APIs, between the mode pin controller driver and low-level API.

---
Changes in v2:
- Added Xilinx ZynqMP firmware MMIO API support to set and get pin
  value and status.
- DT Documentation- Addressed review comments: Update commit message
- Modepin driver- Addressed review comments:
  - Update APIs
  - Removed unwanted variables
  - Handle return path for probe function

Review Comments:
https://lore.kernel.org/linux-arm-kernel/20210624205055.GA1961487@robh.at.kernel.org/T/#u
---

Piyush Mehta (3):
  firmware: zynqmp: Add MMIO read and write support for PS_MODE pin
  dt-bindings: gpio: zynqmp: Add binding documentation for modepin
  gpio: modepin: Add driver support for modepin GPIO controller

 .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml    |  41 ++++++
 drivers/firmware/xilinx/zynqmp.c                   |  46 ++++++
 drivers/gpio/Kconfig                               |  12 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-zynqmp-modepin.c                 | 158 +++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h               |  14 ++
 6 files changed, 272 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml
 create mode 100644 drivers/gpio/gpio-zynqmp-modepin.c

-- 
2.7.4

[PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Piyush Mehta]

Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
pins value and status. These APIs create an interface path between
mode pin controller driver and low-level API to access GPIO pins.

Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
---
Changes in v2:
- Added Xilinx ZynqMP firmware MMIO API support to set and get pin
  value and status.
---
 drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 14 +++++++++++
 2 files changed, 60 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 15b13832..0234423 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -28,6 +28,13 @@
 /* Max HashMap Order for PM API feature check (1<<7 = 128) */
 #define PM_API_FEATURE_CHECK_MAX_ORDER  7
 
+/* CRL registers and bitfields */
+#define CRL_APB_BASE			0xFF5E0000U
+/* BOOT_PIN_CTRL- Used to control the mode pins after boot */
+#define CRL_APB_BOOT_PIN_CTRL		(CRL_APB_BASE + (0x250U))
+/* BOOT_PIN_CTRL_MASK- out_val[11:8], out_en[3:0] */
+#define CRL_APB_BOOTPIN_CTRL_MASK	0xF0FU
+
 static bool feature_check_enabled;
 static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
 
@@ -926,6 +933,45 @@ int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
 EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
 
 /**
+ * zynqmp_pm_bootmode_read() - PM Config API for read bootpin status
+ * @ps_mode: Returned output value of ps_mode
+ *
+ * This API function is to be used for notify the power management controller
+ * to read bootpin status.
+ *
+ * Return: status, either success or error+reason
+ */
+unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode)
+{
+	unsigned int ret;
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+
+	ret = zynqmp_pm_invoke_fn(PM_MMIO_READ, CRL_APB_BOOT_PIN_CTRL, 0,
+				  0, 0, ret_payload);
+
+	*ps_mode = ret_payload[1];
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_read);
+
+/**
+ * zynqmp_pm_bootmode_write() - PM Config API for Configure bootpin
+ * @ps_mode: Value to be written to the bootpin ctrl register
+ *
+ * This API function is to be used for notify the power management controller
+ * to configure bootpin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_bootmode_write(u32 ps_mode)
+{
+	return zynqmp_pm_invoke_fn(PM_MMIO_WRITE, CRL_APB_BOOT_PIN_CTRL,
+				   CRL_APB_BOOTPIN_CTRL_MASK, ps_mode, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_write);
+
+/**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *			       master has initialized its own power management
  *
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 9d1a5c1..dc6f39f 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -68,6 +68,8 @@ enum pm_api_id {
 	PM_SET_REQUIREMENT = 15,
 	PM_RESET_ASSERT = 17,
 	PM_RESET_GET_STATUS = 18,
+	PM_MMIO_WRITE = 19,
+	PM_MMIO_READ = 20,
 	PM_PM_INIT_FINALIZE = 21,
 	PM_FPGA_LOAD = 22,
 	PM_FPGA_GET_STATUS = 23,
@@ -386,6 +388,8 @@ int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type);
 int zynqmp_pm_reset_assert(const enum zynqmp_pm_reset reset,
 			   const enum zynqmp_pm_reset_action assert_flag);
 int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, u32 *status);
+unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode);
+int zynqmp_pm_bootmode_write(u32 ps_mode);
 int zynqmp_pm_init_finalize(void);
 int zynqmp_pm_set_suspend_mode(u32 mode);
 int zynqmp_pm_request_node(const u32 node, const u32 capabilities,
@@ -515,6 +519,16 @@ static inline int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
 	return -ENODEV;
 }
 
+static inline unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_bootmode_write(u32 ps_mode)
+{
+	return -ENODEV;
+}
+
 static inline int zynqmp_pm_init_finalize(void)
 {
 	return -ENODEV;
-- 
2.7.4

Re: [PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Michal Simek]

On 8/5/21 7:42 PM, Piyush Mehta wrote:
> Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
> pins value and status. These APIs create an interface path between
> mode pin controller driver and low-level API to access GPIO pins.
> 
> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> ---
> Changes in v2:
> - Added Xilinx ZynqMP firmware MMIO API support to set and get pin
>   value and status.
> ---
>  drivers/firmware/xilinx/zynqmp.c     | 46 ++++++++++++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h | 14 +++++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 15b13832..0234423 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -28,6 +28,13 @@
>  /* Max HashMap Order for PM API feature check (1<<7 = 128) */
>  #define PM_API_FEATURE_CHECK_MAX_ORDER  7
>  
> +/* CRL registers and bitfields */
> +#define CRL_APB_BASE			0xFF5E0000U
> +/* BOOT_PIN_CTRL- Used to control the mode pins after boot */
> +#define CRL_APB_BOOT_PIN_CTRL		(CRL_APB_BASE + (0x250U))
> +/* BOOT_PIN_CTRL_MASK- out_val[11:8], out_en[3:0] */
> +#define CRL_APB_BOOTPIN_CTRL_MASK	0xF0FU
> +
>  static bool feature_check_enabled;
>  static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
>  
> @@ -926,6 +933,45 @@ int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
>  EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
>  
>  /**
> + * zynqmp_pm_bootmode_read() - PM Config API for read bootpin status
> + * @ps_mode: Returned output value of ps_mode
> + *
> + * This API function is to be used for notify the power management controller
> + * to read bootpin status.
> + *
> + * Return: status, either success or error+reason
> + */
> +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode)
> +{
> +	unsigned int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +
> +	ret = zynqmp_pm_invoke_fn(PM_MMIO_READ, CRL_APB_BOOT_PIN_CTRL, 0,
> +				  0, 0, ret_payload);
> +
> +	*ps_mode = ret_payload[1];
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_read);
> +
> +/**
> + * zynqmp_pm_bootmode_write() - PM Config API for Configure bootpin
> + * @ps_mode: Value to be written to the bootpin ctrl register
> + *
> + * This API function is to be used for notify the power management controller
> + * to configure bootpin.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_bootmode_write(u32 ps_mode)
> +{
> +	return zynqmp_pm_invoke_fn(PM_MMIO_WRITE, CRL_APB_BOOT_PIN_CTRL,
> +				   CRL_APB_BOOTPIN_CTRL_MASK, ps_mode, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_write);
> +
> +/**
>   * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
>   *			       master has initialized its own power management
>   *
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 9d1a5c1..dc6f39f 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -68,6 +68,8 @@ enum pm_api_id {
>  	PM_SET_REQUIREMENT = 15,
>  	PM_RESET_ASSERT = 17,
>  	PM_RESET_GET_STATUS = 18,
> +	PM_MMIO_WRITE = 19,
> +	PM_MMIO_READ = 20,
>  	PM_PM_INIT_FINALIZE = 21,
>  	PM_FPGA_LOAD = 22,
>  	PM_FPGA_GET_STATUS = 23,
> @@ -386,6 +388,8 @@ int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type);
>  int zynqmp_pm_reset_assert(const enum zynqmp_pm_reset reset,
>  			   const enum zynqmp_pm_reset_action assert_flag);
>  int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, u32 *status);
> +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode);
> +int zynqmp_pm_bootmode_write(u32 ps_mode);
>  int zynqmp_pm_init_finalize(void);
>  int zynqmp_pm_set_suspend_mode(u32 mode);
>  int zynqmp_pm_request_node(const u32 node, const u32 capabilities,
> @@ -515,6 +519,16 @@ static inline int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset,
>  	return -ENODEV;
>  }
>  
> +static inline unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int zynqmp_pm_bootmode_write(u32 ps_mode)
> +{
> +	return -ENODEV;
> +}
> +
>  static inline int zynqmp_pm_init_finalize(void)
>  {
>  	return -ENODEV;
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Linus Walleij]

On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:

> Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
> pins value and status. These APIs create an interface path between
> mode pin controller driver and low-level API to access GPIO pins.
>
> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> ---
> Changes in v2:
> - Added Xilinx ZynqMP firmware MMIO API support to set and get pin
>   value and status.

I doubt this is "GPIO".
General Purpose? I think not. It seems to be about boot mode.

If you need a userspace ABI, then add sysfs files to this firmware
driver instead of bridging it to the GPIO subsystem.

However I can be argued down from usecases etc that it is used as
GPIO but I need to push back on this.

Yours,
Linus Walleij

Re: [PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Arnd Bergmann]

On Wed, Aug 11, 2021 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:
>
> > Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
> > pins value and status. These APIs create an interface path between
> > mode pin controller driver and low-level API to access GPIO pins.
> >
> > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> > ---
> > Changes in v2:
> > - Added Xilinx ZynqMP firmware MMIO API support to set and get pin
> >   value and status.
>
> I doubt this is "GPIO".
> General Purpose? I think not. It seems to be about boot mode.

Agreed.

> If you need a userspace ABI, then add sysfs files to this firmware
> driver instead of bridging it to the GPIO subsystem.

I don't really want custom user interfaces in firmware drivers either.

What is the high-level description of the 'PS_MODE' here? Is
this perhaps something we already have a user interface for?

       Arnd

Re: [PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Michal Simek]

Hi Arnd,

On 8/11/21 3:46 PM, Arnd Bergmann wrote:
> On Wed, Aug 11, 2021 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>> On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:
>>
>>> Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
>>> pins value and status. These APIs create an interface path between
>>> mode pin controller driver and low-level API to access GPIO pins.
>>>
>>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
>>> ---
>>> Changes in v2:
>>> - Added Xilinx ZynqMP firmware MMIO API support to set and get pin
>>>   value and status.
>>
>> I doubt this is "GPIO".
>> General Purpose? I think not. It seems to be about boot mode.
> 
> Agreed.

here is register description.

https://www.xilinx.com/html_docs/registers/ug1087/crl_apb___boot_pin_ctrl.html#

> 
>> If you need a userspace ABI, then add sysfs files to this firmware
>> driver instead of bridging it to the GPIO subsystem.
> 
> I don't really want custom user interfaces in firmware drivers either.
> 
> What is the high-level description of the 'PS_MODE' here? Is
> this perhaps something we already have a user interface for?

The reason why this can't be mapped as memory mapped device is that it
is in IP which has be secure. That's why routing is done via firmware
driver.

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

It means ROM just capture boot mode at start that's why they have
special meaning and after it is free to use for whatever purpose you
want which seems to pretty much as generic purpose I/O.

I wrote more comments in reply to Linus already.

Thanks,
Michal

Re: [PATCH V2 1/3] firmware: zynqmp: Add MMIO read and write support for PS_MODE pin

[Linus Walleij]

On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:

> Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE
> pins value and status. These APIs create an interface path between
> mode pin controller driver and low-level API to access GPIO pins.
>
> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com>
> ---
> Changes in v2:

After Michals description of how this is controlling USB
PHY and misc resets I'm OK with the concept.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH V2 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin

[Piyush Mehta]

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>
---
Changes in v2:
- Addressed review comments: Update commit message

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
---
 .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml    | 41 ++++++++++++++++++++++
 1 file changed, 41 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..39d78f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml
@@ -0,0 +1,41 @@
+# 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.
+
+maintainers:
+  - Piyush Mehta <piyush.mehta@xilinx.com>
+
+properties:
+  compatible:
+    const: xlnx,zynqmp-gpio-modepin
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    modepin_gpio: gpio {
+        compatible = "xlnx,zynqmp-gpio-modepin";
+        gpio-controller;
+        #gpio-cells = <2>;
+    };
+
+...
-- 
2.7.4

Re: [PATCH V2 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin

[Rob Herring]

On Thu, Aug 05, 2021 at 11:12:18PM +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>
> ---
> Changes in v2:
> - Addressed review comments: Update commit message
> 
> 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
> ---
>  .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml    | 41 ++++++++++++++++++++++
>  1 file changed, 41 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..39d78f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml
> @@ -0,0 +1,41 @@
> +# 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.
> +
> +maintainers:
> +  - Piyush Mehta <piyush.mehta@xilinx.com>
> +
> +properties:
> +  compatible:
> +    const: xlnx,zynqmp-gpio-modepin
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - gpio-controller
> +  - "#gpio-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    modepin_gpio: gpio {
> +        compatible = "xlnx,zynqmp-gpio-modepin";
> +        gpio-controller;
> +        #gpio-cells = <2>;

No way to interact with this h/w?

As it is part of the firmware interface, it must be a child node in the 
firmware node schema.

> +    };
> +
> +...
> -- 
> 2.7.4
> 
> 

Re: [PATCH V2 2/3] dt-bindings: gpio: zynqmp: Add binding documentation for modepin

[Michal Simek]

On 8/13/21 9:01 PM, Rob Herring wrote:
> On Thu, Aug 05, 2021 at 11:12:18PM +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>
>> ---
>> Changes in v2:
>> - Addressed review comments: Update commit message
>>
>> 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
>> ---
>>  .../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml    | 41 ++++++++++++++++++++++
>>  1 file changed, 41 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..39d78f8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml
>> @@ -0,0 +1,41 @@
>> +# 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.
>> +
>> +maintainers:
>> +  - Piyush Mehta <piyush.mehta@xilinx.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: xlnx,zynqmp-gpio-modepin
>> +
>> +  gpio-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +
>> +required:
>> +  - compatible
>> +  - gpio-controller
>> +  - "#gpio-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    modepin_gpio: gpio {
>> +        compatible = "xlnx,zynqmp-gpio-modepin";
>> +        gpio-controller;
>> +        #gpio-cells = <2>;
> 
> No way to interact with this h/w?
> 
> As it is part of the firmware interface, it must be a child node in the 
> firmware node schema.

Actually it is there. Only example is not showing this properly.
https://github.com/Xilinx/u-boot-xlnx/blob/master/arch/arm/dts/zynqmp.dtsi#L250

Piyush: Can you please do it as we have done with pinctrl.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml?h=v5.14-rc6#n301

like this

zynqmp_firmware: zynqmp-firmware {
    modepin_gpio: gpio {
        compatible = "xlnx,zynqmp-gpio-modepin";
        gpio-controller;
        #gpio-cells = <2>;
    };
};

That dt labels can be also removed because they are not needed in example.

Thanks,
Michal

[PATCH V2 3/3] gpio: modepin: Add driver support for modepin GPIO controller

[Piyush Mehta]

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>
---
Changes in v2:
- Modepin driver- Addressed review comments:
  - Update APIs
  - Removed unwanted variables
  - Handle return path for probe function

https://lore.kernel.org/linux-arm-kernel/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#m276c8a5c52f8dc1ed1cd91a2d660f78d498e4ae5
---
 drivers/gpio/Kconfig               |  12 +++
 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpio-zynqmp-modepin.c | 158 +++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+)
 create mode 100644 drivers/gpio/gpio-zynqmp-modepin.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index fab5710..90a3a3d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -755,6 +755,18 @@ config GPIO_ZYNQ
 	help
 	  Say yes here to support Xilinx Zynq GPIO controller.
 
+config GPIO_ZYNQMP_MODEPIN
+	tristate "ZynqMP ps-mode pin gpio configuration driver"
+	depends on ZYNQMP_FIRMWARE
+	default ZYNQMP_FIRMWARE
+	help
+	  Say yes here to support the ZynqMP ps-mode pin gpio configuration
+	  driver.
+
+	  This ps-mode pin gpio driver is based on GPIO framework, PS_MODE
+	  is 4-bits boot mode pins. It sets and gets the status of
+	  the ps-mode pin. Every pin can be configured as input/output.
+
 config GPIO_LOONGSON1
 	tristate "Loongson1 GPIO support"
 	depends on MACH_LOONGSON32
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 32a3265..978dc4595 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -183,3 +183,4 @@ obj-$(CONFIG_GPIO_XRA1403)		+= gpio-xra1403.o
 obj-$(CONFIG_GPIO_XTENSA)		+= gpio-xtensa.o
 obj-$(CONFIG_GPIO_ZEVIO)		+= gpio-zevio.o
 obj-$(CONFIG_GPIO_ZYNQ)			+= gpio-zynq.o
+obj-$(CONFIG_GPIO_ZYNQMP_MODEPIN)	+= gpio-zynqmp-modepin.o
diff --git a/drivers/gpio/gpio-zynqmp-modepin.c b/drivers/gpio/gpio-zynqmp-modepin.c
new file mode 100644
index 0000000..99c69df
--- /dev/null
+++ b/drivers/gpio/gpio-zynqmp-modepin.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the ps-mode pin configuration.
+ *
+ * Copyright (c) 2021 Xilinx, Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+/* 4-bit boot mode pins */
+#define MODE_PINS			4
+
+/**
+ * modepin_gpio_get_value - Get the state of the specified pin of GPIO device
+ * @chip:	gpio_chip instance to be worked on
+ * @pin:	gpio pin number within the device
+ *
+ * This function reads the state of the specified pin of the GPIO device.
+ *
+ * Return: 0 if the pin is low, 1 if pin is high, -EINVAL wrong pin configured
+ *         or error value.
+ */
+static int modepin_gpio_get_value(struct gpio_chip *chip, unsigned int pin)
+{
+	u32 regval = 0;
+	int ret;
+
+	ret = zynqmp_pm_bootmode_read(&regval);
+	if (ret) {
+		pr_err("modepin: get value err %d\n", ret);
+		return ret;
+	}
+
+	return !!(regval & BIT(pin + 8));
+}
+
+/**
+ * modepin_gpio_set_value - Modify the state of the pin with specified value
+ * @chip:	gpio_chip instance to be worked on
+ * @pin:	gpio pin number within the device
+ * @state:	value used to modify the state of the specified pin
+ *
+ * This function reads the state of the specified pin of the GPIO device, mask
+ * with the capture state of GPIO pin, and update pin of GPIO device.
+ *
+ * Return:	None.
+ */
+static void modepin_gpio_set_value(struct gpio_chip *chip, unsigned int pin,
+				   int state)
+{
+	u32 bootpin_val = 0;
+	int ret;
+
+	ret = zynqmp_pm_bootmode_read(&bootpin_val);
+	if (ret)
+		pr_err("modepin: get value err %d\n", ret);
+
+	if (state)
+		bootpin_val |= BIT(pin + 8);
+	else
+		bootpin_val &= ~BIT(pin + 8);
+
+	/* Configure bootpin value */
+	ret = zynqmp_pm_bootmode_write(bootpin_val);
+	if (ret)
+		pr_err("modepin: %s failed\n", __func__);
+}
+
+/**
+ * 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;
+}
+
+/**
+ * modepin_gpio_dir_out - Set the direction of the specified GPIO pin as output
+ * @chip:	gpio_chip instance to be worked on
+ * @pin:	gpio pin number within the device
+ * @state:	value to be written to specified pin
+ *
+ * Return: 0 always
+ */
+static int modepin_gpio_dir_out(struct gpio_chip *chip, unsigned int pin,
+				int state)
+{
+	return 0;
+}
+
+/**
+ * modepin_gpio_probe - Initialization method for modepin_gpio
+ * @pdev:		platform device instance
+ *
+ * Return: 0 on success, negative error otherwise.
+ */
+static int modepin_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_chip *chip;
+	int status;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, chip);
+
+	/* configure the gpio chip */
+	chip->base = -1;
+	chip->ngpio = MODE_PINS;
+	chip->owner = THIS_MODULE;
+	chip->parent = &pdev->dev;
+	chip->get = modepin_gpio_get_value;
+	chip->set = modepin_gpio_set_value;
+	chip->direction_input = modepin_gpio_dir_in;
+	chip->direction_output = modepin_gpio_dir_out;
+	chip->label = dev_name(&pdev->dev);
+
+	/* modepin gpio registration */
+	status = devm_gpiochip_add_data(&pdev->dev, chip, chip);
+	if (status)
+		return dev_err_probe(&pdev->dev, status,
+			      "Failed to add GPIO chip\n");
+
+	return status;
+}
+
+static const struct of_device_id modepin_platform_id[] = {
+	{ .compatible = "xlnx,zynqmp-gpio-modepin", },
+	{ }
+};
+
+static struct platform_driver modepin_platform_driver = {
+	.driver = {
+		.name = "modepin-gpio",
+		.of_match_table = modepin_platform_id,
+	},
+	.probe = modepin_gpio_probe,
+};
+
+module_platform_driver(modepin_platform_driver);
+
+MODULE_AUTHOR("Piyush Mehta <piyush.mehta@xilinx.com>");
+MODULE_DESCRIPTION("ZynqMP Boot PS_MODE Configuration");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH V2 3/3] gpio: modepin: Add driver support for modepin GPIO controller

[Linus Walleij]

Hi Piyush,

thanks for your patch!

Can you explain one thing to me: since this is now a GPIO driver
that means "General Purpos Input/Output", then these bits are
accessed like this:

On Thu, Aug 5, 2021 at 7:43 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:

> +       ret = zynqmp_pm_bootmode_read(&bootpin_val);

This does not look very general purpose. These seem to be all about
boot mode, right?

So can you explain why this should be a GPIO driver at all?

I understand it is sometimes convenient to describe stuff as GPIO even
if it is not (for example to get a convenient userspace interface) but
as maintainers
we really need to make sure that the subsystem is not being abused
for things not GPIO.

Yours,
Linus Walleij

Re: [PATCH V2 3/3] gpio: modepin: Add driver support for modepin GPIO controller

[Michal Simek]

Hi Linus,

On 8/11/21 2:59 PM, Linus Walleij wrote:
> Hi Piyush,
> 
> thanks for your patch!
> 
> Can you explain one thing to me: since this is now a GPIO driver
> that means "General Purpos Input/Output", then these bits are
> accessed like this:
> 
> On Thu, Aug 5, 2021 at 7:43 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote:
> 
>> +       ret = zynqmp_pm_bootmode_read(&bootpin_val);
> 
> This does not look very general purpose. These seem to be all about
> boot mode, right?
> 
> So can you explain why this should be a GPIO driver at all?
> 
> I understand it is sometimes convenient to describe stuff as GPIO even
> if it is not (for example to get a convenient userspace interface) but
> as maintainers
> we really need to make sure that the subsystem is not being abused
> for things not GPIO.

They are bootmode pins because that pins are designed and used by ROM to
get information which boot device should be used.
But after this is it is really behaving as generic purpose I/O pins.
Xilinx is using them for years for usb phy resets. I have also seen them
to be used for other reset functionality.
And that's exactly what we are trying to do here by this driver. Because
usb hubs/phys have reset normally connected via gpio pin which is
toggled. And we have boards where these resets are connected via these
pins or via hard gpio IP or via i2c-gpio expanders that's why IMHO this
option should be designed in the same way to have gpio reset
functionality added dwc3 and based on DT/board bootmode gpio, ps gpio,
pl gpio, i2c gpio or gpio over power regulators, etc will be used.

Please let me know if you want to get more information about it.

Thanks,
Michal

Re: [PATCH V2 3/3] gpio: modepin: Add driver support for modepin GPIO controller

[Linus Walleij]

On Wed, Aug 11, 2021 at 3:30 PM Michal Simek <michal.simek@xilinx.com> wrote:

> They are bootmode pins because that pins are designed and used by ROM to
> get information which boot device should be used.
> But after this is it is really behaving as generic purpose I/O pins.
> Xilinx is using them for years for usb phy resets. I have also seen them
> to be used for other reset functionality.

OK if they are used for general purpose tasks then a GPIO driver
is fine, I was worried that it was just a way to read these pins from
userspace.

Go ahead with this patch series!

Yours,
Linus Walleij

Re: [PATCH V2 3/3] gpio: modepin: Add driver support for modepin GPIO controller

[Linus Walleij]

On Thu, Aug 5, 2021 at 7:43 PM 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>
> ---
> Changes in v2:

After discussion with Michal:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij