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