RE: [PATCH v10 0/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support

[Peng Fan <peng.fan@xxxxxxx>]

Hi Dan,

> Subject: Re: [PATCH v10 0/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol
> protocol basic support
>
> I'm trying to re-base AKASHI Takahiro's gpio driver on top of your scmi pinctrl
> driver.
> https://lore.ke/
> rnel.org%2Fall%2F20231005025843.508689-1-
> takahiro.akashi%40linaro.org%2F&data=05%7C02%7Cpeng.fan%40nxp.com%
> 7C342dd6eb0463456d0d6608dc5e41de1c%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C638488884186606528%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C0%7C%7C%7C&sdata=DMJZ2uwuJigkEnEcY7JdBw6DMPjHxcUvvh7
> 2fsaep50%3D&reserved=0
> I need to do something like this below to save the gpio information.
>
> So now, great, I have the information but I'm not sure how to export it from
> the scmi pinctrl driver to the gpio driver...  (This is a probably a stupid
> question but I am real newbie with regards to gpio).
>
> The other thing is that the SCMI spec says:
>
>     4.11.2.7
>     PINCTRL_SETTINGS_GET
>
>     This command can be used by an agent to get the pin or group
>     configuration, and the function selected to be enabled. It can also
>     be used to read the value of a pin when it is set to GPIO mode.
>
> What does that mean?  Is that right, or is it something left over from a
> previous revision of the spec.
>
> regards,
> dan carpenter
>
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c
> b/drivers/firmware/arm_scmi/pinctrl.c

Just a short question, you will make this a standalone
patch part of your gpio pinctrl patchset, right?

Or you wanna include this change in my v11 patch?

I hope v11 + imx oem patches could land in 6.10,
so I would not expect big changes to v11.

Thanks,
Peng.

> index a2a7f880d6a3..f803be8a223f 100644
> --- a/drivers/firmware/arm_scmi/pinctrl.c
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -26,6 +26,7 @@
>  #define GET_PINS_NR(x)               le32_get_bits((x), GENMASK(15, 0))
>  #define GET_FUNCTIONS_NR(x)  le32_get_bits((x), GENMASK(15, 0))
>
> +#define IS_GPIO_FUNC(x)              le32_get_bits((x), BIT(17))
>  #define EXT_NAME_FLAG(x)     le32_get_bits((x), BIT(31))
>  #define NUM_ELEMS(x)         le32_get_bits((x), GENMASK(15, 0))
>
> @@ -107,6 +108,7 @@ struct scmi_group_info {  struct scmi_function_info {
>       char name[SCMI_MAX_STR_SIZE];
>       bool present;
> +     bool gpio;
>       u32 *groups;
>       u32 nr_groups;
>  };
> @@ -189,7 +191,7 @@ static int scmi_pinctrl_validate_id(const struct
> scmi_protocol_handle *ph,
>
>  static int scmi_pinctrl_attributes(const struct scmi_protocol_handle *ph,
>                                  enum scmi_pinctrl_selector_type type,
> -                                u32 selector, char *name,
> +                                u32 selector, char *name, bool *gpio,
>                                  u32 *n_elems)
>  {
>       int ret;
> @@ -216,17 +218,20 @@ static int scmi_pinctrl_attributes(const struct
> scmi_protocol_handle *ph,
>       tx->flags = cpu_to_le32(type);
>
>       ret = ph->xops->do_xfer(ph, t);
> -     if (!ret) {
> -             if (n_elems)
> -                     *n_elems = NUM_ELEMS(rx->attributes);
> +     if (ret)
> +             goto xfer_put;
>
> -             strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
> +     if (n_elems)
> +             *n_elems = NUM_ELEMS(rx->attributes);
>
> -             ext_name_flag = !!EXT_NAME_FLAG(rx->attributes);
> -     }
> +     if (type == FUNCTION_TYPE && gpio)
> +             *gpio = !!IS_GPIO_FUNC(rx->attributes);
>
> -     ph->xops->xfer_put(ph, t);
> +     strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
> +     ext_name_flag = !!EXT_NAME_FLAG(rx->attributes);
>
> +xfer_put:
> +     ph->xops->xfer_put(ph, t);
>       if (ret)
>               return ret;
>       /*
> @@ -602,7 +607,7 @@ static int scmi_pinctrl_get_group_info(const struct
> scmi_protocol_handle *ph,
>       int ret;
>
>       ret = scmi_pinctrl_attributes(ph, GROUP_TYPE, selector, group-
> >name,
> -                                   &group->nr_pins);
> +                                   NULL, &group->nr_pins);
>       if (ret)
>               return ret;
>
> @@ -687,7 +692,7 @@ static int scmi_pinctrl_get_function_info(const struct
> scmi_protocol_handle *ph,
>       int ret;
>
>       ret = scmi_pinctrl_attributes(ph, FUNCTION_TYPE, selector, func-
> >name,
> -                                   &func->nr_groups);
> +                                   &func->gpio, &func->nr_groups);
>       if (ret)
>               return ret;
>
> @@ -778,7 +783,8 @@ static int scmi_pinctrl_get_pin_info(const struct
> scmi_protocol_handle *ph,
>       if (!pin)
>               return -EINVAL;
>
> -     ret = scmi_pinctrl_attributes(ph, PIN_TYPE, selector, pin->name,
> NULL);
> +     ret = scmi_pinctrl_attributes(ph, PIN_TYPE, selector, pin->name,
> NULL,
> +                                   NULL);
>       if (ret)
>               return ret;
>