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

[PATCH V3 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

Changes in v3:
- Update example in dt-bindings documentation
- Update probe function return value
- Remove unnecessary print and header file

Review Comments:
https://lore.kernel.org/linux-arm-kernel/20210805174219.3000667-1-piyush.mehta@xilinx.com/#t
---

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    |  43 ++++++
 drivers/firmware/xilinx/zynqmp.c                   |  46 +++++++
 drivers/gpio/Kconfig                               |  12 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-zynqmp-modepin.c                 | 153 +++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h               |  14 ++
 6 files changed, 269 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 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>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 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>
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.
+
+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:
+  - |
+    zynqmp-firmware {
+        gpio {
+            compatible = "xlnx,zynqmp-gpio-modepin";
+            gpio-controller;
+            #gpio-cells = <2>;
+        };
+    };
+
+...
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

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

Cheers,
Ahmad

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michal Simek]

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.

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


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

And then users/customers can say what normal/recovery/test modes are.

Thanks,
Michal

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

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
> 


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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michal Simek]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

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>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 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>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v2:
- 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/20210615080553.2021061-2-piyush.mehta@xilinx.com/T/#m276c8a5c52f8dc1ed1cd91a2d660f78d498e4ae5

Changes in v3:
- Addressed Linus and Arnd review comments:
  - Update probe function return value
  - Remove unnecessary print and header file
  - Update error message for set value method

Review Comments:
https://lore.kernel.org/linux-arm-kernel/20210805174219.3000667-4-piyush.mehta@xilinx.com/T/#m70acd39653033e32458633e21a2e6d21afdd16e6
---
 drivers/gpio/Kconfig               |  12 +++
 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpio-zynqmp-modepin.c | 153 +++++++++++++++++++++++++++++++++++++
 3 files changed, 166 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..d52e391
--- /dev/null
+++ b/drivers/gpio/gpio-zynqmp-modepin.c
@@ -0,0 +1,153 @@
+// 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/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)
+		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;
+
+	zynqmp_pm_bootmode_read(&bootpin_val);
+
+	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: set value error %d for pin %d\n", ret, pin);
+}
+
+/**
+ * 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

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.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Piyush Mehta]

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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ahmad Fatoum]

Hello Piyush,

On 18.08.21 12:09, Piyush Mehta wrote:
> 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

Ok. So if you want to configure the pin as input, you should call zynqmp_pm_bootmode_write
to write a zero into that bit.

But there's only one zynqmp_pm_bootmode_write in the GPIO driver and it's in modepin_gpio_set_value,
which does output, not input. If I understand you right, there should be a modepin_gpio_set_value in
modepin_gpio_dir_in as well?
 
> 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)

This is a valid sequence for a GPIO consumer and I don't see how this GPIO driver could
honor it. Could you clarify?

Cheers,
Ahmad

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Bartosz Golaszewski]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michal Simek]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Bartosz Golaszewski]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Bartosz Golaszewski]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michal Simek]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel