This patch adds support for the mode pin 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 4-bits boot-mode pins are dedicated configurable as input/output.
After the stabilization of the system,these mode pins are sampled.
Piyush Mehta (2):
dt-bindings: gpio: Add binding documentation for modepin
gpio: modepin: Add driver support for modepin GPIO controller
.../bindings/gpio/xlnx,zynqmp-gpio-modepin.yaml | 41 ++++++
drivers/gpio/Kconfig | 12 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-zynqmp-modepin.c | 154 +++++++++++++++++++++
4 files changed, 208 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
This patch adds support for the mode pin GPIO controller. GPIO Modepin
driver set and get the value and status of the PS_MODE pin, based on
device-tree pin configuration. These 4-bits boot-mode pins are dedicated
configurable as input/output. After the stabilization of the system,
these mode pins are sampled.
Signed-off-by: Piyush Mehta <[email protected]>
---
drivers/gpio/Kconfig | 12 +++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-zynqmp-modepin.c | 154 +++++++++++++++++++++++++++++++++++++
3 files changed, 167 insertions(+)
create mode 100644 drivers/gpio/gpio-zynqmp-modepin.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1dd0ec6..30e0dbf 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 d7c81e1..62bfa73 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -182,3 +182,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..27052f0
--- /dev/null
+++ b/drivers/gpio/gpio-zynqmp-modepin.c
@@ -0,0 +1,154 @@
+// 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>
+
+#define MODE_PINS 4
+#define GET_OUTEN_PIN(pin) (1U << (pin))
+
+/*
+ * 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 out_en;
+ u32 regval = 0;
+ int ret;
+
+ out_en = GET_OUTEN_PIN(pin);
+
+ ret = zynqmp_pm_bootmode_read(®val);
+ if (ret) {
+ pr_err("modepin: get value err %d\n", ret);
+ return ret;
+ }
+
+ return (out_en & (regval >> 8U)) ? 1 : 0;
+}
+
+/*
+ * 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
+ *
+ * Return: None.
+ */
+static void modepin_gpio_set_value(struct gpio_chip *chip, unsigned int pin,
+ int state)
+{
+ u32 out_en;
+ u32 bootpin_val = 0;
+ int ret;
+
+ out_en = GET_OUTEN_PIN(pin);
+ state = state != 0 ? out_en : 0;
+ bootpin_val = (state << (8U)) | out_en;
+
+ /* 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)
+ 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 <[email protected]>");
+MODULE_DESCRIPTION("ZynqMP Boot PS_MODE Configuration");
+MODULE_LICENSE("GPL v2");
--
2.7.4
Add DT binding document for modepin GPIO controller.
Signed-off-by: Piyush Mehta <[email protected]>
State: pending
---
.../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 <[email protected]>
+
+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
On 6/15/21 10:05 AM, Piyush Mehta wrote:
> Add DT binding document for modepin GPIO controller.
>
> Signed-off-by: Piyush Mehta <[email protected]>
> State: pending
note: this shouldn't be the part of commit message.
But please wait when dt guys review this file and then send v2.
M
> ---
> .../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 <[email protected]>
> +
> +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>;
> + };
> +
> +...
>
Hi Piyush!
thanks for your patch!
On Tue, Jun 15, 2021 at 10:06 AM Piyush Mehta <[email protected]> wrote:
> This patch adds support for the mode pin GPIO controller. GPIO Modepin
> driver set and get the value and status of the PS_MODE pin, based on
> device-tree pin configuration. These 4-bits boot-mode pins are dedicated
> configurable as input/output. After the stabilization of the system,
> these mode pins are sampled.
>
> Signed-off-by: Piyush Mehta <[email protected]>
OK, sounds interesting!
> +#include <linux/slab.h>
I think I saw somewhere that this is not needed anymore, check if you need it.
> +#define GET_OUTEN_PIN(pin) (1U << (pin))
Delete this macro and just use BIT(pin) inline.
#include <linux/bits.h>
> +static int modepin_gpio_get_value(struct gpio_chip *chip, unsigned int pin)
> +{
> + u32 out_en;
> + u32 regval = 0;
> + int ret;
> +
> + out_en = GET_OUTEN_PIN(pin);
Drop this and out_en
> + ret = zynqmp_pm_bootmode_read(®val);
> + if (ret) {
> + pr_err("modepin: get value err %d\n", ret);
> + return ret;
> + }
> +
> + return (out_en & (regval >> 8U)) ? 1 : 0;
return !!(regval & BIT(pin + 8));
should work and is easier to read IMO. We just check the right
bit immediately.
> +static void modepin_gpio_set_value(struct gpio_chip *chip, unsigned int pin,
> + int state)
> +{
> + u32 out_en;
> + u32 bootpin_val = 0;
> + int ret;
> +
> + out_en = GET_OUTEN_PIN(pin);
Skip this helper variable.
> + state = state != 0 ? out_en : 0;
Uh that is really hard to read and modified a parameter. Skip that too.
> + bootpin_val = (state << (8U)) | out_en;
What you want is mask and set.
bootpin_val = BIT(pin + 8);
> + /* Configure bootpin value */
> + ret = zynqmp_pm_bootmode_write(bootpin_val);
This just looks weird.
Why are you not reading the value first since you are using
read/modify/write?
I *think* you want to do this:
ret = zynqmp_pm_bootmode_read(&val);
if (ret)
/* error handling */
if (state)
val |= BIT(pin + 8);
else
val &= ~BIT(pin + 8);
ret = zynqmp_pm_bootmode_write(val);
if (ret)
/* error handling */
> +/*
> + * 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;
> +}
I think you said this was configurable in the commit message.
Use the define GPIO_LINE_DIRECTION_OUT rather than 0.
> +static int modepin_gpio_dir_out(struct gpio_chip *chip, unsigned int pin,
> + int state)
> +{
> + return 0;
> +}
Configurable?
> + status = devm_gpiochip_add_data(&pdev->dev, chip, chip);
> + if (status)
> + dev_err_probe(&pdev->dev, status,
> + "Failed to add GPIO chip\n");
just return dev_err_probe(...)
Yours,
Linus Walleij
Hello Linus,
Thanks for the review comments.
We will address all the reviews in the next version.
Regards,
Piyush Mehta
-----Original Message-----
From: Linus Walleij <[email protected]>
Sent: Friday, June 18, 2021 3:14 PM
To: Piyush Mehta <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>; Rob Herring <[email protected]>; open list:GPIO SUBSYSTEM <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; linux-kernel <[email protected]>; Linux ARM <[email protected]>; git <[email protected]>; Srinivas Goud <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [PATCH 2/2] gpio: modepin: Add driver support for modepin GPIO controller
Hi Piyush!
thanks for your patch!
On Tue, Jun 15, 2021 at 10:06 AM Piyush Mehta <[email protected]> wrote:
> This patch adds support for the mode pin GPIO controller. GPIO Modepin
> driver set and get the value and status of the PS_MODE pin, based on
> device-tree pin configuration. These 4-bits boot-mode pins are
> dedicated configurable as input/output. After the stabilization of the
> system, these mode pins are sampled.
>
> Signed-off-by: Piyush Mehta <[email protected]>
OK, sounds interesting!
> +#include <linux/slab.h>
I think I saw somewhere that this is not needed anymore, check if you need it.
> +#define GET_OUTEN_PIN(pin) (1U << (pin))
Delete this macro and just use BIT(pin) inline.
#include <linux/bits.h>
> +static int modepin_gpio_get_value(struct gpio_chip *chip, unsigned
> +int pin) {
> + u32 out_en;
> + u32 regval = 0;
> + int ret;
> +
> + out_en = GET_OUTEN_PIN(pin);
Drop this and out_en
> + ret = zynqmp_pm_bootmode_read(®val);
> + if (ret) {
> + pr_err("modepin: get value err %d\n", ret);
> + return ret;
> + }
> +
> + return (out_en & (regval >> 8U)) ? 1 : 0;
return !!(regval & BIT(pin + 8));
should work and is easier to read IMO. We just check the right bit immediately.
> +static void modepin_gpio_set_value(struct gpio_chip *chip, unsigned int pin,
> + int state) {
> + u32 out_en;
> + u32 bootpin_val = 0;
> + int ret;
> +
> + out_en = GET_OUTEN_PIN(pin);
Skip this helper variable.
> + state = state != 0 ? out_en : 0;
Uh that is really hard to read and modified a parameter. Skip that too.
> + bootpin_val = (state << (8U)) | out_en;
What you want is mask and set.
bootpin_val = BIT(pin + 8);
> + /* Configure bootpin value */
> + ret = zynqmp_pm_bootmode_write(bootpin_val);
This just looks weird.
Why are you not reading the value first since you are using read/modify/write?
I *think* you want to do this:
ret = zynqmp_pm_bootmode_read(&val);
if (ret)
/* error handling */
if (state)
val |= BIT(pin + 8);
else
val &= ~BIT(pin + 8);
ret = zynqmp_pm_bootmode_write(val);
if (ret)
/* error handling */
> +/*
> + * 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;
> +}
I think you said this was configurable in the commit message.
Use the define GPIO_LINE_DIRECTION_OUT rather than 0.
> +static int modepin_gpio_dir_out(struct gpio_chip *chip, unsigned int pin,
> + int state) {
> + return 0;
> +}
Configurable?
> + status = devm_gpiochip_add_data(&pdev->dev, chip, chip);
> + if (status)
> + dev_err_probe(&pdev->dev, status,
> + "Failed to add GPIO chip\n");
just return dev_err_probe(...)
Yours,
Linus Walleij
On Tue, Jun 15, 2021 at 01:35:52PM +0530, Piyush Mehta wrote:
> Add DT binding document for modepin GPIO controller.
Please give some indication in the subject this is for Xilinx.
>
> Signed-off-by: Piyush Mehta <[email protected]>
> State: pending
> ---
> .../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.
And connected to other functions like GPIOs?
> +
> +maintainers:
> + - Piyush Mehta <[email protected]>
> +
> +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 {
Drop unused labels.
> + compatible = "xlnx,zynqmp-gpio-modepin";
> + gpio-controller;
> + #gpio-cells = <2>;
How does one access this h/w?
> + };
> +
> +...
> --
> 2.7.4
>
>