diff mbox series

[RFC,4/6] gpio: add scmi driver based on pinctrl

Message ID 20230906024011.17488-5-takahiro.akashi@linaro.org
State New
Headers show
Series firmware: scmi: add SCMI pinctrl protocol support | expand

Commit Message

AKASHI Takahiro Sept. 6, 2023, 2:40 a.m. UTC
This DM-compliant driver deals with SCMI pinctrl protocol and presents
gpio devices exposed by SCMI firmware (server).

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
 1 file changed, 539 insertions(+), 5 deletions(-)

Comments

Michal Simek Sept. 6, 2023, 2:56 p.m. UTC | #1
On 9/6/23 04:40, AKASHI Takahiro wrote:
> This DM-compliant driver deals with SCMI pinctrl protocol and presents
> gpio devices exposed by SCMI firmware (server).
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
>   1 file changed, 539 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> index 3ebdad57b86c..73d385bdbfcc 100644
> --- a/drivers/pinctrl/pinctrl-scmi.c
> +++ b/drivers/pinctrl/pinctrl-scmi.c
> @@ -11,21 +11,20 @@
>   #include <malloc.h>
>   #include <scmi_agent.h>
>   #include <scmi_protocols.h>
> +#include <asm-generic/gpio.h>
>   #include <dm/device_compat.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
>   #include <dm/pinctrl.h>
>   
>   /**
>    * struct scmi_pin - attributes for a pin
>    * @name:	Name of pin
> - * @value:	Value of pin
> - * @flags:	A set of flags
>    * @function:	Function selected
>    * @status:	An array of status of configuration types
>    */
>   struct scmi_pin {
>   	char		*name;
> -	u32		value;
> -	u32		flags;


You have added this in 3/6 then there is no reason to remove it in this version.

M
AKASHI Takahiro Sept. 6, 2023, 11:37 p.m. UTC | #2
On Wed, Sep 06, 2023 at 04:56:56PM +0200, Michal Simek wrote:
> 
> 
> On 9/6/23 04:40, AKASHI Takahiro wrote:
> > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > gpio devices exposed by SCMI firmware (server).
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
> >   1 file changed, 539 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> > index 3ebdad57b86c..73d385bdbfcc 100644
> > --- a/drivers/pinctrl/pinctrl-scmi.c
> > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > @@ -11,21 +11,20 @@
> >   #include <malloc.h>
> >   #include <scmi_agent.h>
> >   #include <scmi_protocols.h>
> > +#include <asm-generic/gpio.h>
> >   #include <dm/device_compat.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/lists.h>
> >   #include <dm/pinctrl.h>
> >   /**
> >    * struct scmi_pin - attributes for a pin
> >    * @name:	Name of pin
> > - * @value:	Value of pin
> > - * @flags:	A set of flags
> >    * @function:	Function selected
> >    * @status:	An array of status of configuration types
> >    */
> >   struct scmi_pin {
> >   	char		*name;
> > -	u32		value;
> > -	u32		flags;
> 
> 
> You have added this in 3/6 then there is no reason to remove it in this version.

Right.
It was my mistake in a last-minute cleanup.
The hunk should be merged in 3/6.

BTW, this part of code, holding status of every pin's pinconf properties
locally, is a bit clumsy. I'd remove the whole code if possible.

-Takahiro Akashi


> M
Simon Glass Sept. 7, 2023, 12:23 p.m. UTC | #3
Hi AKASHI,

On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> This DM-compliant driver deals with SCMI pinctrl protocol and presents
> gpio devices exposed by SCMI firmware (server).
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
>  1 file changed, 539 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> index 3ebdad57b86c..73d385bdbfcc 100644
> --- a/drivers/pinctrl/pinctrl-scmi.c
> +++ b/drivers/pinctrl/pinctrl-scmi.c
> @@ -11,21 +11,20 @@
>  #include <malloc.h>
>  #include <scmi_agent.h>
>  #include <scmi_protocols.h>
> +#include <asm-generic/gpio.h>
>  #include <dm/device_compat.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
>  #include <dm/pinctrl.h>
>
[..]

> +
> +U_BOOT_DRIVER(scmi_gpio) = {
> +       .name = "scmi_gpio",
> +       .id = UCLASS_GPIO,
> +       .of_match = scmi_gpio_ids,
> +       .of_to_plat = scmi_gpio_probe,
> +       .ops = &scmi_gpio_ops,
> +       .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> +};
> +
> +/**
> + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> + * @parent:    SCMI pinctrl device
> + *
> + * Create a pinctrl-controlled gpio device
> + *
> + * Return:     0 on success, error code on failure
> + */
> +static int scmi_gpiochip_register(struct udevice *parent)
> +{
> +       ofnode node;
> +       struct driver *drv;
> +       struct udevice *gpio_dev;
> +       int ret;
> +
> +       /* TODO: recovery if failed */
> +       dev_for_each_subnode(node, parent) {
> +               if (!ofnode_is_enabled(node))
> +                       continue;

Can we not rely on the normal driver model binding to bind these
devices? Why does it need to be done manually here?
> +
> +               if (!ofnode_read_prop(node, "gpio-controller", NULL))
> +                       /* not a GPIO node */
> +                       continue;
> +
> +               drv = DM_DRIVER_GET(scmi_gpio);
> +               if (!drv) {
> +                       dev_err(parent, "No gpio driver?\n");
> +                       return -ENODEV;
> +               }
> +
> +               ret = device_bind(parent, drv, ofnode_get_name(node), NULL,
> +                                 node, &gpio_dev);
> +               if (ret) {
> +                       dev_err(parent, "failed to bind %s to gpio (%d)\n",
> +                               ofnode_get_name(node), ret);
> +                       return -ENODEV;
> +               }
> +
> +               return 0;
> +       }
> +
> +       return -ENODEV;
> +}
> +

Regards,
Simon
AKASHI Takahiro Sept. 8, 2023, 4:32 a.m. UTC | #4
Hi Simon,

On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> Hi AKASHI,
> 
> On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > gpio devices exposed by SCMI firmware (server).
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 539 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> > index 3ebdad57b86c..73d385bdbfcc 100644
> > --- a/drivers/pinctrl/pinctrl-scmi.c
> > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > @@ -11,21 +11,20 @@
> >  #include <malloc.h>
> >  #include <scmi_agent.h>
> >  #include <scmi_protocols.h>
> > +#include <asm-generic/gpio.h>
> >  #include <dm/device_compat.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/lists.h>
> >  #include <dm/pinctrl.h>
> >
> [..]
> 
> > +
> > +U_BOOT_DRIVER(scmi_gpio) = {
> > +       .name = "scmi_gpio",
> > +       .id = UCLASS_GPIO,
> > +       .of_match = scmi_gpio_ids,
> > +       .of_to_plat = scmi_gpio_probe,
> > +       .ops = &scmi_gpio_ops,
> > +       .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > +};
> > +
> > +/**
> > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > + * @parent:    SCMI pinctrl device
> > + *
> > + * Create a pinctrl-controlled gpio device
> > + *
> > + * Return:     0 on success, error code on failure
> > + */
> > +static int scmi_gpiochip_register(struct udevice *parent)
> > +{
> > +       ofnode node;
> > +       struct driver *drv;
> > +       struct udevice *gpio_dev;
> > +       int ret;
> > +
> > +       /* TODO: recovery if failed */
> > +       dev_for_each_subnode(node, parent) {
> > +               if (!ofnode_is_enabled(node))
> > +                       continue;
> 
> Can we not rely on the normal driver model binding to bind these
> devices? Why does it need to be done manually here?

First, please take a look at the cover letter.
In this RFC, I basically assume two patterns of DT bindings,
(A) and (B) in the cover letter (or sandbox's test.dts in patch#5).

In (B), a DT node as a gpio device, which is essentially a child
of pinctrl device, is located *under* a pinctrl device. It need
to be probed manually as there is no implicit method to enumerate
it as a DM device automatically.

On the other hand, in (A), the same node can be put anywhere in a DT
as it contains a "compatible" property to identify itself.
So if we want to only support (A), scmi_gpiochip_register() and
scmi_pinctrl_bind() are not necessary.

Since there is no discussion about bindings for GPIO managed by SCMI
pinctrl device yet on the kernel side, I have left two solutions
in this RFC.

Thanks,
-Takakahiro Akashi


> > +
> > +               if (!ofnode_read_prop(node, "gpio-controller", NULL))
> > +                       /* not a GPIO node */
> > +                       continue;
> > +
> > +               drv = DM_DRIVER_GET(scmi_gpio);
> > +               if (!drv) {
> > +                       dev_err(parent, "No gpio driver?\n");
> > +                       return -ENODEV;
> > +               }
> > +
> > +               ret = device_bind(parent, drv, ofnode_get_name(node), NULL,
> > +                                 node, &gpio_dev);
> > +               if (ret) {
> > +                       dev_err(parent, "failed to bind %s to gpio (%d)\n",
> > +                               ofnode_get_name(node), ret);
> > +                       return -ENODEV;
> > +               }
> > +
> > +               return 0;
> > +       }
> > +
> > +       return -ENODEV;
> > +}
> > +
> 
> Regards,
> Simon
Simon Glass Sept. 10, 2023, 7:13 p.m. UTC | #5
Hi,

On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> > Hi AKASHI,
> >
> > On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > > gpio devices exposed by SCMI firmware (server).
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >  drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
> > >  1 file changed, 539 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> > > index 3ebdad57b86c..73d385bdbfcc 100644
> > > --- a/drivers/pinctrl/pinctrl-scmi.c
> > > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > > @@ -11,21 +11,20 @@
> > >  #include <malloc.h>
> > >  #include <scmi_agent.h>
> > >  #include <scmi_protocols.h>
> > > +#include <asm-generic/gpio.h>
> > >  #include <dm/device_compat.h>
> > > +#include <dm/device-internal.h>
> > > +#include <dm/lists.h>
> > >  #include <dm/pinctrl.h>
> > >
> > [..]
> >
> > > +
> > > +U_BOOT_DRIVER(scmi_gpio) = {
> > > +       .name = "scmi_gpio",
> > > +       .id = UCLASS_GPIO,
> > > +       .of_match = scmi_gpio_ids,
> > > +       .of_to_plat = scmi_gpio_probe,
> > > +       .ops = &scmi_gpio_ops,
> > > +       .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > > +};
> > > +
> > > +/**
> > > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > > + * @parent:    SCMI pinctrl device
> > > + *
> > > + * Create a pinctrl-controlled gpio device
> > > + *
> > > + * Return:     0 on success, error code on failure
> > > + */
> > > +static int scmi_gpiochip_register(struct udevice *parent)
> > > +{
> > > +       ofnode node;
> > > +       struct driver *drv;
> > > +       struct udevice *gpio_dev;
> > > +       int ret;
> > > +
> > > +       /* TODO: recovery if failed */
> > > +       dev_for_each_subnode(node, parent) {
> > > +               if (!ofnode_is_enabled(node))
> > > +                       continue;
> >
> > Can we not rely on the normal driver model binding to bind these
> > devices? Why does it need to be done manually here?
>
> First, please take a look at the cover letter.
> In this RFC, I basically assume two patterns of DT bindings,
> (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
>
> In (B), a DT node as a gpio device, which is essentially a child
> of pinctrl device, is located *under* a pinctrl device. It need
> to be probed manually as there is no implicit method to enumerate
> it as a DM device automatically.

You could add a post_bind() method to the pinctrl uclass to call
dm_scan_fdt_dev() as we do with TPM.

>
> On the other hand, in (A), the same node can be put anywhere in a DT
> as it contains a "compatible" property to identify itself.
> So if we want to only support (A), scmi_gpiochip_register() and
> scmi_pinctrl_bind() are not necessary.
>
> Since there is no discussion about bindings for GPIO managed by SCMI
> pinctrl device yet on the kernel side, I have left two solutions
> in this RFC.


OK.

[..]

Regards,
Simon
AKASHI Takahiro Sept. 11, 2023, 5:38 a.m. UTC | #6
Hi Simon,

On Sun, Sep 10, 2023 at 01:13:24PM -0600, Simon Glass wrote:
> Hi,
> 
> On Thu, 7 Sept 2023 at 22:32, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Sep 07, 2023 at 06:23:05AM -0600, Simon Glass wrote:
> > > Hi AKASHI,
> > >
> > > On Tue, 5 Sept 2023 at 20:41, AKASHI Takahiro
> > > <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > This DM-compliant driver deals with SCMI pinctrl protocol and presents
> > > > gpio devices exposed by SCMI firmware (server).
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >  drivers/pinctrl/pinctrl-scmi.c | 544 ++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 539 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> > > > index 3ebdad57b86c..73d385bdbfcc 100644
> > > > --- a/drivers/pinctrl/pinctrl-scmi.c
> > > > +++ b/drivers/pinctrl/pinctrl-scmi.c
> > > > @@ -11,21 +11,20 @@
> > > >  #include <malloc.h>
> > > >  #include <scmi_agent.h>
> > > >  #include <scmi_protocols.h>
> > > > +#include <asm-generic/gpio.h>
> > > >  #include <dm/device_compat.h>
> > > > +#include <dm/device-internal.h>
> > > > +#include <dm/lists.h>
> > > >  #include <dm/pinctrl.h>
> > > >
> > > [..]
> > >
> > > > +
> > > > +U_BOOT_DRIVER(scmi_gpio) = {
> > > > +       .name = "scmi_gpio",
> > > > +       .id = UCLASS_GPIO,
> > > > +       .of_match = scmi_gpio_ids,
> > > > +       .of_to_plat = scmi_gpio_probe,
> > > > +       .ops = &scmi_gpio_ops,
> > > > +       .plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
> > > > +};
> > > > +
> > > > +/**
> > > > + * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
> > > > + * @parent:    SCMI pinctrl device
> > > > + *
> > > > + * Create a pinctrl-controlled gpio device
> > > > + *
> > > > + * Return:     0 on success, error code on failure
> > > > + */
> > > > +static int scmi_gpiochip_register(struct udevice *parent)
> > > > +{
> > > > +       ofnode node;
> > > > +       struct driver *drv;
> > > > +       struct udevice *gpio_dev;
> > > > +       int ret;
> > > > +
> > > > +       /* TODO: recovery if failed */
> > > > +       dev_for_each_subnode(node, parent) {
> > > > +               if (!ofnode_is_enabled(node))
> > > > +                       continue;
> > >
> > > Can we not rely on the normal driver model binding to bind these
> > > devices? Why does it need to be done manually here?
> >
> > First, please take a look at the cover letter.
> > In this RFC, I basically assume two patterns of DT bindings,
> > (A) and (B) in the cover letter (or sandbox's test.dts in patch#5).
> >
> > In (B), a DT node as a gpio device, which is essentially a child
> > of pinctrl device, is located *under* a pinctrl device. It need
> > to be probed manually as there is no implicit method to enumerate
> > it as a DM device automatically.
> 
> You could add a post_bind() method to the pinctrl uclass to call
> dm_scan_fdt_dev() as we do with TPM.

Okay, but in that case, defining "compatible" property will become
mandatory, while I invented (B) so that we don't need extra bindings.

Please note that no discussion started about the binding of gpio devices.

-Takahiro Akashi

> >
> > On the other hand, in (A), the same node can be put anywhere in a DT
> > as it contains a "compatible" property to identify itself.
> > So if we want to only support (A), scmi_gpiochip_register() and
> > scmi_pinctrl_bind() are not necessary.
> >
> > Since there is no discussion about bindings for GPIO managed by SCMI
> > pinctrl device yet on the kernel side, I have left two solutions
> > in this RFC.
> 
> 
> OK.
> 
> [..]
> 
> Regards,
> Simon
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 3ebdad57b86c..73d385bdbfcc 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -11,21 +11,20 @@ 
 #include <malloc.h>
 #include <scmi_agent.h>
 #include <scmi_protocols.h>
+#include <asm-generic/gpio.h>
 #include <dm/device_compat.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
 #include <dm/pinctrl.h>
 
 /**
  * struct scmi_pin - attributes for a pin
  * @name:	Name of pin
- * @value:	Value of pin
- * @flags:	A set of flags
  * @function:	Function selected
  * @status:	An array of status of configuration types
  */
 struct scmi_pin {
 	char		*name;
-	u32		value;
-	u32		flags;
 	unsigned int	function;
 	u32		status[SCMI_PINCTRL_CONFIG_RESERVED];
 };
@@ -308,7 +307,6 @@  static int scmi_pinmux_group_set(struct udevice *dev,
 	return 0;
 }
 
-/* TODO: may be driver-specific */
 /**
  * pinmux_property_set - Enable a pinmux group
  * @dev:	SCMI pinctrl device to use
@@ -424,6 +422,539 @@  const struct pinctrl_ops scmi_pinctrl_ops = {
 	.set_state = pinctrl_generic_set_state,
 };
 
+#if CONFIG_IS_ENABLED(DM_GPIO)
+/*
+ * GPIO part
+ */
+
+/**
+ * struct scmi_pinctrl_gpio_plat - platform data
+ * @pinctrl_dev:	Associated SCMI pinctrl device
+ * @gpio_map:		A list of gpio mapping information
+ * @name:		Name of ?
+ * @ngpios:		A number of gpio pins
+ */
+struct scmi_pinctrl_gpio_plat {
+	struct list_head gpio_map;
+	char *name;
+	u32 ngpios;
+};
+
+/**
+ * struct scmi_pinctrl_gpio_map - gpio mapping information
+ * @gpio_pin:		Start gpio pin selector
+ * @pinctrl_pin:	Mapped start pinctrl pin selector, used for linear mapping
+ * @pinctrl_group:	Name of an associated group, used for group namee mapping
+ * @num_pins:		A number of pins
+ * @node:		Node for a mapping list
+ */
+struct scmi_pinctrl_gpio_map {
+	struct udevice *pinctrl_dev;
+	u32 gpio_pin;
+	u32 pinctrl_pin;
+	u16 *pinctrl_group;
+	u32 num_pins;
+	struct list_head node;
+};
+
+/**
+ * map_to_id - Map a gpio pin offset to a pinctrl pin selector
+ * @dev:	SCMI pinctrl device
+ * @offset:	Pin offset
+ * @id:		Pinctrl pin selector
+ *
+ * Map a gpio pin offset, @offset, to an associated pinctrl pin selector
+ *
+ * Return:	0 on success, -1 on failure
+ */
+static int map_to_id(struct udevice *dev, unsigned int offset,
+		     struct udevice **pinctrl_dev, u32 *id)
+{
+	struct scmi_pinctrl_gpio_plat *plat = dev_get_plat(dev);
+	struct scmi_pinctrl_gpio_map *map;
+
+	/* if no mapping is defined, return 1:1 pin */
+	if (list_empty(&plat->gpio_map)) {
+		uclass_first_device(UCLASS_PINCTRL, pinctrl_dev);
+		if (!*pinctrl_dev)
+			return -1;
+
+		*id = offset;
+		return 0;
+	}
+
+	list_for_each_entry(map, &plat->gpio_map, node) {
+		if (offset >= map->gpio_pin &&
+		    offset < (map->gpio_pin + map->num_pins)) {
+			*pinctrl_dev = map->pinctrl_dev;
+			if (!pinctrl_dev)
+				return -1;
+
+			if (map->pinctrl_group)
+				*id = map->pinctrl_group[offset
+							  - map->gpio_pin];
+			else
+				*id = map->pinctrl_pin
+					+ (offset - map->gpio_pin);
+
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+/**
+ * scmi_gpio_get_value - Get a value of a gpio pin
+ * @dev:	SCMI gpio device
+ * @offset:	Pin offset
+ *
+ * Get a value of a gpio pin in @offset
+ *
+ * Return:	Value of a pin on success, error code on failure
+ */
+static int scmi_gpio_get_value(struct udevice *dev, unsigned int offset)
+{
+	struct scmi_pinctrl_priv *priv;
+	u32 id;
+	struct udevice *pinctrl_dev;
+	struct scmi_pin_entry *config;
+	int config_type, ret;
+
+	if (map_to_id(dev, offset, &pinctrl_dev, &id)) {
+		dev_err(dev, "Invalid pin: %u\n", offset);
+		return -EINVAL;
+	}
+
+	priv = dev_get_priv(pinctrl_dev);
+	if (priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE]) {
+		config_type = SCMI_PINCTRL_CONFIG_INPUT_VALUE;
+	} else if (priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE]) {
+		config_type = SCMI_PINCTRL_CONFIG_OUTPUT_VALUE;
+	} else {
+		dev_err(dev, "Invalid pin mode: %u\n", offset);
+		return -EINVAL;
+	}
+
+	ret = scmi_pinctrl_config_get(pinctrl_dev, id, false,
+				      SCMI_PINCTRL_TYPE_PIN, config_type,
+				      &config);
+	if (ret) {
+		dev_err(dev, "config_get failed (%d)\n", ret);
+		return ret;
+	}
+
+	return config->value;
+}
+
+/**
+ * config_pin_set - Configure a pinctrl pin
+ * @dev:		SCMI pinctrl device
+ * @id:			Pin selector
+ * @config_type:	Configuration type
+ * @value:		Value
+ *
+ * Set a configuration of @config_type of a pinctrl pin, @id, to @value.
+ *
+ * Return:	0 on success, error code on failure
+ */
+static int config_pin_set(struct udevice *dev, u32 id, u32 config_type,
+			  u32 value)
+{
+	struct scmi_pinctrl_priv *priv = dev_get_priv(dev);
+	struct scmi_pin_entry config;
+	int ret;
+
+	config.type = config_type;
+	config.value = value;
+	ret = scmi_pinctrl_config_set(dev, id, SCMI_PINCTRL_TYPE_PIN,
+				      1, &config);
+	if (ret) {
+		dev_err(dev, "config_set failed (%d)\n", ret);
+		return ret;
+	}
+
+	priv->pins[id].status[config_type] = value;
+
+	return 0;
+}
+
+/**
+ * scmi_gpio_set_value - Set a value of a gpio pin
+ * @dev:	SCMI gpio device
+ * @offset:	Pin offset
+ * @value:	Value
+ *
+ * Set a value of a gpio pin in @offset to @value.
+ *
+ * Return:	0 on success, error code on failure
+ */
+static int scmi_gpio_set_value(struct udevice *dev, unsigned int offset,
+			       int value)
+{
+	struct udevice *pinctrl_dev;
+	u32 id;
+
+	if (map_to_id(dev, offset, &pinctrl_dev, &id)) {
+		dev_err(dev, "Invalid pin: %u\n", offset);
+		return -EINVAL;
+	}
+
+	return config_pin_set(pinctrl_dev, id,
+			      SCMI_PINCTRL_CONFIG_OUTPUT_VALUE,
+			      (u32)value);
+}
+
+/**
+ * scmi_gpio_get_function - Get a gpio function of a gpio pin
+ * @dev:	SCMI gpio device
+ * @offset:	Pin offset
+ *
+ * Get a gpio function of a gpio pin in @offset.
+ *
+ * Return:	GPIOF_OUTPUT or GPIO_INPUT on success, otherwise GPIOF_UNKNOWN
+ */
+static int scmi_gpio_get_function(struct udevice *dev, unsigned int offset)
+{
+	struct udevice *pinctrl_dev;
+	u32 id;
+	struct scmi_pin_entry *config;
+	int ret;
+
+	if (map_to_id(dev, offset, &pinctrl_dev, &id)) {
+		dev_err(dev, "Invalid pin: %u\n", offset);
+		return -EINVAL;
+	}
+
+	ret = scmi_pinctrl_config_get(pinctrl_dev, id, false,
+				      SCMI_PINCTRL_TYPE_PIN,
+				      SCMI_PINCTRL_CONFIG_INPUT_MODE, &config);
+	if (!ret && config->value == 1)
+		return GPIOF_INPUT;
+
+	ret = scmi_pinctrl_config_get(pinctrl_dev, id, false,
+				      SCMI_PINCTRL_TYPE_PIN,
+				      SCMI_PINCTRL_CONFIG_OUTPUT_MODE,
+				      &config);
+	if (!ret && config->value == 1)
+		return GPIOF_OUTPUT;
+
+	return GPIOF_UNKNOWN;
+}
+
+/**
+ * set_flags - Adjust GPIO flags
+ * @dev:        SCMI gpio device
+ * @offset:     Pin offset
+ * @flags:      New flags value
+ *
+ * This function should set up the GPIO configuration according to information
+ * provided by @flags (GPIOD_xxx).
+ *
+ * @return 0 if OK, otherwise -ve on error
+ */
+static int scmi_gpio_set_flags(struct udevice *dev, unsigned int offset,
+			       ulong flags)
+{
+	struct udevice *pinctrl_dev;
+	struct scmi_pin_entry config[5]; /* 5 is enough for now */
+	u32 id;
+	struct scmi_pinctrl_priv *priv;
+	int i = 0, ret;
+
+	if (map_to_id(dev, offset, &pinctrl_dev, &id)) {
+		dev_err(dev, "Invalid pin: %u\n", offset);
+		return -EINVAL;
+	}
+
+	if (flags & GPIOD_IS_OUT) {
+		config[i].type = SCMI_PINCTRL_CONFIG_INPUT_MODE;
+		config[i++].value = 0;
+
+		config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_MODE;
+		config[i++].value = 1;
+		config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_VALUE;
+		config[i++].value = (u32)!!(flags & GPIOD_IS_OUT_ACTIVE);
+	} else if (flags & GPIOD_IS_IN) {
+		config[i].type = SCMI_PINCTRL_CONFIG_OUTPUT_MODE;
+		config[i++].value = 0;
+
+		config[i].type = SCMI_PINCTRL_CONFIG_INPUT_MODE;
+		config[i++].value = 1;
+	}
+
+	if (flags & GPIOD_OPEN_DRAIN) {
+		config[i].type = SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN;
+		config[i++].value = 1;
+	} else if (flags & GPIOD_OPEN_SOURCE) {
+		config[i].type = SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE;
+		config[i++].value = 1;
+	}
+
+	if (flags & GPIOD_PULL_UP) {
+		config[i].type = SCMI_PINCTRL_CONFIG_BIAS_PULL_UP;
+		config[i++].value = 1;
+	} else if (flags & GPIOD_PULL_DOWN) {
+		config[i].type = SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN;
+		config[i++].value = 1;
+	}
+
+	ret = scmi_pinctrl_config_set(pinctrl_dev, id, SCMI_PINCTRL_TYPE_PIN,
+				      i, config);
+	if (ret) {
+		dev_err(dev, "config_set failed (%d)\n", ret);
+		return ret;
+	}
+
+	/* update local status */
+	priv = dev_get_priv(pinctrl_dev);
+
+	if (flags & GPIOD_IS_OUT) {
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE] = 1;
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_VALUE] = 1;
+
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE] = 0;
+	} else if (flags & GPIOD_IS_IN) {
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_INPUT_MODE] = 1;
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_OUTPUT_MODE] = 0;
+	}
+
+	if (flags & GPIOD_OPEN_DRAIN)
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_DRIVE_OPEN_DRAIN] = 1;
+	else if (flags & GPIOD_OPEN_SOURCE)
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_DRIVE_OPEN_SOURCE] =
+				1;
+
+	if (flags & GPIOD_PULL_UP)
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_BIAS_PULL_UP] = 1;
+	else if (flags & GPIOD_PULL_DOWN)
+		priv->pins[id].status[SCMI_PINCTRL_CONFIG_BIAS_PULL_DOWN] = 1;
+
+	return 0;
+}
+
+/**
+ * lookup_pin_group - find out a list of pins for group name
+ * @dev:	SCMI pinctrl device
+ * @name:	Name of a group
+ * @num_pins:	Pointer to a number of pins
+ * @pins:	Pointer to array of pin selectors
+ *
+ * Look up a group named @name and return a list of pin selectors associated with
+ * the group in @pins.
+ *
+ * Return:	0 on success, -1 on failure
+ */
+static int lookup_pin_group(struct udevice *dev, const char *name,
+			    u32 *num_pins, u16 **pins)
+{
+	struct scmi_pinctrl_priv *priv = dev_get_priv(dev);
+	int i;
+
+	for (i = 0; i < priv->num_groups; i++)
+		if (!strcmp(name, priv->groups[i].name)) {
+			*num_pins = priv->groups[i].num_pins;
+			*pins = priv->groups[i].pins;
+			return 0;
+		}
+
+	return -1;
+}
+
+/**
+ * get_pinctrl_dev - Get a pin control device at node
+ * @node:	Reference to the node
+ *
+ * Get a pin control device at @node.
+ *
+ * Return:	0 on success, otherwise NULL
+ */
+static struct udevice *get_pinctrl_dev(ofnode *node)
+{
+	struct udevice *dev;
+	struct uclass *uc;
+
+	if (uclass_get(UCLASS_PINCTRL, &uc))
+		return NULL;
+
+	uclass_foreach_dev(dev, uc)
+		if (dev->node_.np == node->np)
+			return dev;
+
+	return NULL;
+}
+
+/**
+ * scmi_gpio_probe - Probe a SCMI gpio device
+ * @dev:	SCMI gpio device
+ *
+ * Probe and initialize a SCMI gpio device.
+ *
+ * 0 on success, error code on failure
+ */
+static int scmi_gpio_probe(struct udevice *dev)
+{
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	struct scmi_pinctrl_gpio_plat *plat = dev_get_plat(dev);
+
+	struct scmi_pinctrl_gpio_map *range;
+	struct ofnode_phandle_args args;
+	const char *name;
+	int num_pins = 0, index, ret;
+
+	uc_priv->bank_name = dev_read_string(dev, "gpio-bank-name");
+	if (!uc_priv->bank_name)
+		uc_priv->bank_name = dev->name;
+
+	INIT_LIST_HEAD(&plat->gpio_map);
+	for (index = 0; ; index++) {
+		ret = dev_read_phandle_with_args(dev, "gpio-ranges",
+						 NULL, 3, index, &args);
+		if (ret)
+			break;
+
+		range = malloc(sizeof(*range));
+		if (!range) {
+			dev_err(dev, "Memory not available\n");
+			return -ENOMEM;
+		}
+
+		range->pinctrl_dev = get_pinctrl_dev(&args.node);
+		if (!range->pinctrl_dev) {
+			dev_err(dev, "Pin control device not found\n");
+			return -ENODEV;
+		}
+
+		/* make sure pinctrl is activated */
+		ret = device_probe(range->pinctrl_dev);
+		if (ret)
+			return ret;
+
+		if (args.args[2]) {
+			range->gpio_pin = args.args[0];
+			range->pinctrl_pin = args.args[1];
+			range->num_pins = args.args[2];
+			range->pinctrl_group = NULL;
+		} else {
+			ret = of_property_read_string_index(dev_np(dev),
+						"gpio-ranges-group-names",
+						index, &name);
+			if (ret) {
+				dev_err(dev,
+					"gpio-ranges-group-names required\n");
+				return ret;
+			}
+
+			ret = lookup_pin_group(range->pinctrl_dev, name,
+					       &range->num_pins,
+					       &range->pinctrl_group);
+			if (ret) {
+				dev_err(dev, "Group not found: %s\n", name);
+				return -EINVAL;
+			}
+
+			range->gpio_pin = args.args[0];
+		}
+		num_pins += range->num_pins;
+		list_add_tail(&range->node, &plat->gpio_map);
+	}
+
+	uc_priv->gpio_count = num_pins;
+
+	return 0;
+}
+
+static const struct dm_gpio_ops scmi_gpio_ops = {
+	.get_function = scmi_gpio_get_function,
+	.get_value = scmi_gpio_get_value,
+	.set_value = scmi_gpio_set_value,
+	.set_flags = scmi_gpio_set_flags,
+};
+
+static const struct udevice_id scmi_gpio_ids[] = {
+	{ .compatible = "arm,scmi-gpio-generic" },
+	{ }
+};
+
+U_BOOT_DRIVER(scmi_gpio) = {
+	.name = "scmi_gpio",
+	.id = UCLASS_GPIO,
+	.of_match = scmi_gpio_ids,
+	.of_to_plat = scmi_gpio_probe,
+	.ops = &scmi_gpio_ops,
+	.plat_auto = sizeof(struct scmi_pinctrl_gpio_plat),
+};
+
+/**
+ * scmi_gpiochip_register - Create a pinctrl-controlled gpio device
+ * @parent:	SCMI pinctrl device
+ *
+ * Create a pinctrl-controlled gpio device
+ *
+ * Return:	0 on success, error code on failure
+ */
+static int scmi_gpiochip_register(struct udevice *parent)
+{
+	ofnode node;
+	struct driver *drv;
+	struct udevice *gpio_dev;
+	int ret;
+
+	/* TODO: recovery if failed */
+	dev_for_each_subnode(node, parent) {
+		if (!ofnode_is_enabled(node))
+			continue;
+
+		if (!ofnode_read_prop(node, "gpio-controller", NULL))
+			/* not a GPIO node */
+			continue;
+
+		drv = DM_DRIVER_GET(scmi_gpio);
+		if (!drv) {
+			dev_err(parent, "No gpio driver?\n");
+			return -ENODEV;
+		}
+
+		ret = device_bind(parent, drv, ofnode_get_name(node), NULL,
+				  node, &gpio_dev);
+		if (ret) {
+			dev_err(parent, "failed to bind %s to gpio (%d)\n",
+				ofnode_get_name(node), ret);
+			return -ENODEV;
+		}
+
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
+/**
+ * scmi_pinctrl_bind - Bind a pinctrl-controlled gpio device
+ * @dev:	SCMI pinctrl device
+ *
+ * Bind a pinctrl-controlled gpio device
+ *
+ * Return:	0 on success, error code on failure
+ */
+static int scmi_pinctrl_bind(struct udevice *dev)
+{
+	int ret;
+
+	/* gpiochip register */
+	if (CONFIG_IS_ENABLED(DM_GPIO)) {
+		ret = scmi_gpiochip_register(dev);
+		if (ret && ret != -ENODEV) {
+			dev_err(dev, "failed to register gpio driver (%d)\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+#endif /* CONFIG_DM_GPIO */
+
 /**
  * scmi_pinctrl_probe - probe a device
  * @dev:	SCMI pinctrl device
@@ -532,6 +1063,9 @@  U_BOOT_DRIVER(scmi_pinctrl) = {
 	.name = "scmi_pinctrl",
 	.id = UCLASS_PINCTRL,
 	.ops = &scmi_pinctrl_ops,
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	.bind = scmi_pinctrl_bind,
+#endif
 	.probe = scmi_pinctrl_probe,
 	.priv_auto	= sizeof(struct scmi_pinctrl_priv),
 };