mbox series

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

Message ID 20240415-pinctrl-scmi-v10-0-59c6e7a586ee@nxp.com (mailing list archive)
Headers show
Series firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support | expand

Message

Peng Fan (OSS) April 15, 2024, 8:50 a.m. UTC
This patchset is a rework from Oleksii's RFC v5 patchset
https://lore.kernel.org/all/cover.1698353854.git.oleksii_moisieiev@epam.com/

This patchset introduces some changes based on RFC v5:
- introduce helper get_max_msg_size
- support compatible string
- iterate the id_table
- Support multiple configs in one command
- Added i.MX support
- Patch 5 firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
  is almost same as RFCv5 expect multiple configs support.
- Patch 4 the dt-bindings includes compatible string to support i.MX
- Rebased on 2023-12-15 linux-next/master

If any comments from RFC v5 are missed, I am sorry in advance.

This PINCTRL Protocol is following Version 3.2 SCMI Spec Beta release.

On ARM-based systems, a separate Cortex-M based System Control Processor
(SCP) provides control on pins, as well as with power, clocks, reset
controllers. So implement the driver to support such cases.

The i.MX95 Example as below:

Configuration:
The scmi-pinctrl driver can be configured using DT bindings.
For example:
/ {
	sram0: sram@445b1000 {
		compatible = "mmio-sram";
		reg = <0x0 0x445b1000 0x0 0x400>;

		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0 0x0 0x445b1000 0x400>;

		scmi_buf0: scmi-sram-section@0 {
			compatible = "arm,scmi-shmem";
			reg = <0x0 0x80>;
		};

		scmi_buf1: scmi-sram-section@80 {
			compatible = "arm,scmi-shmem";
			reg = <0x80 0x80>;
		};
	};

	firmware {
		scmi {
			compatible = "arm,scmi";
			mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
			shmem = <&scmi_buf0>, <&scmi_buf1>;
			#address-cells = <1>;
			#size-cells = <0>;

			scmi_iomuxc: protocol@19 {
				compatible = "fsl,imx95-scmi-pinctrl";
				reg = <0x19>;
			};
		};
	};
};

&scmi_iomuxc {
	pinctrl_tpm3: tpm3grp {
		fsl,pins = <
			IMX95_PAD_GPIO_IO12__TPM3_CH2(0x51e)
		>;
	};
};

This patchset has been tested on i.MX95-19x19-EVK board.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v10:
- Convert the SCMI -EOPNOTSUPP to pinctrl expected -ENOTSUPP, see:
  https://lore.kernel.org/all/CAHp75Vch7TzevZQK4CE_WSVOgUdzODkJXiv0=OuqrmDfgevV+A@mail.gmail.com/
- Link to v9: https://lore.kernel.org/r/20240412-pinctrl-scmi-v9-0-b817344d3a9a@nxp.com

Changes in v9:
- Rebased to next-20240410
- Because "pinctrl: pinconf-generic: check error value EOPNOTSUPP" was
  applied, so pinctrl-scmi.c not convert to err return value for
  'pinctrl_ops->settings_get_one()'.
- Add R-b for patch [1,2,4]/4 from Dhruva Gole
- Link to v8: https://lore.kernel.org/r/20240405-pinctrl-scmi-v8-0-5fc8e33871bf@nxp.com

Changes in v8:
- For the devm_x, I still keep as it is to follow current SCMI style. In
future we may follow cleanup.h, but it is not this patchset's goal.
- Apply Cristian's change, and add tag
- _pin -> pin to drop underscore
- Add headers per Andy's comments
- Drop casting for func->groups
- Minor update for coding style
- Link to v7: https://lore.kernel.org/r/20240402-pinctrl-scmi-v7-0-3ea519d12cf7@nxp.com

Changes in v7:
- Hope I not miss any comments. If any missed, please forgive. Since
  i.MX95 SCMI firmware not support all the pinctrl features, I could only
  do limited test.
- Version set to 0x10000
- Drop scmi_msg_func_set
- Use get_all to replace flag[0,1], not support flag 2 as of now.
- Add settings_get_one and settings_get_all ops to support get_all[false, true]
- PINCTRL_SET_PERMISSIONS is not included in this patchset
- Bail out if nr_pins is 0
- Add check nr_functions and nr_groups if they are 0.
- ext_name_flag changed to bool type
- Drop unrelated comment
- Use a central function for pin request and free
- Coding style optimization
- Use pinfunction to replace scmi_pinctrl_funcs
- For the devm_x APIs comments from Andy, I not update in the x/arm_scmi/pinctrl.c,
  because it is correct usage.
- For included headers, I keep not change. I try to follow 80 max chars
  for scmi driver, but with a few lines still exceed.
- Link to v6: https://lore.kernel.org/r/20240323-pinctrl-scmi-v6-0-a895243257c0@nxp.com

Changes in v6:
- Update pinctrl driver following ARM SCMI 3.2 public release
- Addressed Dan's comments, and followed Dan's suggestions, thanks.
- Dropped R-b/T-b in patch 3/4 and patch 4/4,
- Link to v5: https://lore.kernel.org/r/20240314-pinctrl-scmi-v5-0-b19576e557f2@nxp.com

Changes in v5:
- Rebased to linux-next next-20240313
- Link to v4: https://lore.kernel.org/r/20240223-pinctrl-scmi-v4-0-10eb5a379274@nxp.com

Changes in v4:
- Rebased to next-20240222
- Drop pinctrl-scmi-imx and compatible patches in V3
- Add T-b and R-b collected from v3
- Link to v3: https://lore.kernel.org/r/20240121-pinctrl-scmi-v3-0-8d94ba79dca8@nxp.com

Changes in v3:
- Add R-b for dt-binding patch
- Use 80 chars per line to align with other scmi drivers
- Add pinctrl_scmi_alloc_configs pinctrl_scmi_free_configs to replace
  driver global config_value and config_type array to avoid in parrell
  access issue. When num_configs is larger than 4, use alloc, else use
  stack.
- Drop the separate MAITAINERS entry for firmware scmi pinctrl
- Use enum type, not u8 when referring the scmi or generic pin conf type
- Drop scmi_pinctrl_config_get_all which is not used at all for now.
- Update copyright year to 2024
- Move the enum scmi_pinctrl_conf_type above pinctrl_proto_ops for consistency
- Link to v2: https://lore.kernel.org/r/20240104-pinctrl-scmi-v2-0-a9bd86ab5a84@nxp.com

Changes in v2:
 Added comments, and added R-b for Patch 1
 Moved the compatile string and i.MX patch to the end, marked NOT APPLY
 Patchset based on lore.kernel.org/all/20231221151129.325749-1-cristian.marussi@arm.com/
 Addressed the binding doc issue, dropped i.MX content.
 For the firmware pinctrl scmi driver, addressed the comments from Cristian
 For the pinctrl scmi driver, addressed comments from Cristian

 For the i.MX95 OEM stuff, I not have good idea, expect using compatbile
 string. Maybe the firmware public an protocol attribute to indicate it is
 VENDOR stuff or NXP use a new protocol id, not 0x19. But I think
 current pinctrl-scmi.c not able to support OEM config, should we extend
 it with some method? Anyway if patch 1-4 is good enough, they could
 be picked up first.

 Since I am only able to test the patch on i.MX95 which not support
 geneirc pinconf, only OEM configs are tested in my side.

---
Peng Fan (4):
      firmware: arm_scmi: introduce helper get_max_msg_size
      dt-bindings: firmware: arm,scmi: support pinctrl protocol
      firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
      pinctrl: Implementation of the generic scmi-pinctrl driver

 .../devicetree/bindings/firmware/arm,scmi.yaml     |  50 ++
 MAINTAINERS                                        |   1 +
 drivers/firmware/arm_scmi/Makefile                 |   3 +-
 drivers/firmware/arm_scmi/driver.c                 |  17 +
 drivers/firmware/arm_scmi/pinctrl.c                | 916 +++++++++++++++++++++
 drivers/firmware/arm_scmi/protocols.h              |   3 +
 drivers/pinctrl/Kconfig                            |  11 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-scmi.c                     | 572 +++++++++++++
 include/linux/scmi_protocol.h                      |  84 ++
 10 files changed, 1657 insertions(+), 1 deletion(-)
---
base-commit: ec3243f13db334437b5ce253be2fb23bcf35b490
change-id: 20231215-pinctrl-scmi-4c5b0374f4c6

Best regards,

Comments

Dan Carpenter April 16, 2024, 6:20 p.m. UTC | #1
I'm trying to re-base AKASHI Takahiro's gpio driver on top of your scmi
pinctrl driver.
https://lore.kernel.org/all/20231005025843.508689-1-takahiro.akashi@linaro.org/
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
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;
Cristian Marussi April 17, 2024, 6:38 a.m. UTC | #2
On Tue, Apr 16, 2024 at 09:20:11PM +0300, Dan Carpenter wrote:
> I'm trying to re-base AKASHI Takahiro's gpio driver on top of your scmi
> pinctrl driver.
> https://lore.kernel.org/all/20231005025843.508689-1-takahiro.akashi@linaro.org/
> 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).
> 

Hi Dan,

I dont think it is a stupid question, I'll try to answer your questions
as much as possible, regarding the SCMI side, since I am definitely not so
much familiar with the GPIO/Pinctrl subsystem either.

First of all, to put things in perspective, drivers/firmware/arm_scmi/pinctrl.c
is just the SCMI protocol layer, which as part of the core SCMI driver is in
charge of implementing the specific protocol (i.e. building and sending
appropriate messages via the SCMI core) and which, in turn, exposes a set of
protocol-specific operations in scmi_protocol.h.

On top of this there are the SCMI drivers (like drivers/pinctrl/pinctrl-scmi.c)
that, on one side, plug into the SCMI stack, and as such can use the specific
protocol_ops, and on the other side register into some custom existing Linux
susbsystem like Pinctrl, so that it can relay generic Pinctrl related requests,
via the above SCMI pinctrl_ops, to the platform SCMI fw (finally translated into
SCMI messages at the protocol layer...)

In all of this, note that the various protocol_ops in scmi_protocol.h are NOT
exported symbols (would have been dozens): that is the reason why a driver
willing to use an SCMI protocol via its specific ops, has to be, first, an SCMI
driver so that can grab the related protocol_ops and an handle to the SCMI
instance, during its probe phase.

NOW, as far as I can remember (and have understood) AKASHI gpio-pinctrl driver
was INSTEAD meant to be a generic GPIO driver on top of Pinctrl subsystem,
so something that could work on top of any pinctrl controller, NOT necessarily
an SCMI one, so, as a consequence it is NOT an SCMI driver and it cannot access
directly any of the pinctrl_ops (existing or future), BUT it will have, instead,
to be based on the Pinctrl subsystem API to achieve its functionalities in a generic
manner.

So at the end, AFAICU:

- you collect any additional gpio info you need (and can get from the spec) at
  the SCMI Pinctrl protocol layer in drivers/firmware/arm_scmi/pinctrl.c (as you
  are doing)

- you expose such info via pinctrl_ops: in these regards many OTHER
  protocols usually exposes some .get_info() ops to get a generic info
  descriptor including all info about a specific resource, BUT this is not
  the case for pinctrl_ops, which just exposes a few custom ops to get
  only the bits that are strictly needed (like resource names via
  .name_get()). Here is up to you which kind of interface to expose really,
  depending on the SCMI Pinctrl driver usage pattern. (is_gpio() ?
  .get_gpios() ? just a new out-param in an existing ops ?)

- in the SCMI pinctrl-scmi driver you can finally make use of your new
  protocol_ops to provide to the Pinctrl subsystem the funcs needed by
  the Pinctrl API calls as issued by the gpio-pinctrl driver....and in these
  regards I really dont know what are the missing bits...I suppose something
  that has to work as the SCMI backend for the pinctrl_gpio_* calls inside
  gpio-pinctrl.

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

My guess is that, this is a (certainly obscure) way for the spec to
express the fact that using this message you can get the pin/group
selected funcs AND pin/group configs, configs, that, include the settings
for any OEM Config type as specified in SCMI spec Table 24, which, in
turn, contains Input-mode/Output-mode/Output-value types that I suppose
pertain to the GPIO world.

As a consequence, I guess you neeed somehow to connect the above
pinctrl_gpio_set/get_config and pinctrl_gpio_get_direction into the
pinctrl_ops .setting_get_one() and .settings_conf() by using the proper
GPIO-related OEM types, not sure of the details (as said I am ignorant)
BUT it could be that this is already handled somehow by the current
pinctrl-scmi driver if the GPIO ranges are handled correctly throughout
all the chain of susbsystem involved... (looking at the internals of
 https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/core.c#L912)
...but I really not familiar on how GPIO ranges are supposed to work so
it is better that now I shut up :D

...apologies for the long email, especially if I said something already obvious.

Thanks,
Cristian
Peng Fan April 17, 2024, 12:15 p.m. UTC | #3
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;
>
Dan Carpenter April 17, 2024, 12:42 p.m. UTC | #4
On Wed, Apr 17, 2024 at 12:15:57PM +0000, Peng Fan wrote:
> 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.

Yeah.  Let's get your patches merged.  I don't think there is anything
in mine which conflict with yours.  It's just addon.

regards,
dan carpenter
Sudeep Holla April 17, 2024, 1:07 p.m. UTC | #5
On Wed, Apr 17, 2024 at 12:15:57PM +0000, Peng Fan wrote:
> Hi Dan,
>
> 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,

I haven't looked at i.MX OEM patches even once so far. IIRC it is all
in the pinctrl driver and you may not need my review/ack. But let us get
this series in first.

This series looks good overall. Since it has pinctrl driver, I need Linus
to ack/agree to pick the whole series up or I can ack them so that Linus
can take the whole series. Either way it is fine for me.

Thanks for all your efforts in pursuing this.

--
Regards,
Sudeep