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

Peng Fan (OSS) posted 6 patches 1 year, 11 months ago
There is a newer version of this series
.../devicetree/bindings/firmware/arm,scmi.yaml     |  50 ++
MAINTAINERS                                        |   1 +
drivers/firmware/arm_scmi/Makefile                 |   1 +
drivers/firmware/arm_scmi/bus.c                    |  39 +-
drivers/firmware/arm_scmi/common.h                 |   2 +-
drivers/firmware/arm_scmi/driver.c                 |  32 +-
drivers/firmware/arm_scmi/pinctrl.c                | 908 +++++++++++++++++++++
drivers/firmware/arm_scmi/protocols.h              |   3 +
drivers/pinctrl/Kconfig                            |  11 +
drivers/pinctrl/Makefile                           |   1 +
drivers/pinctrl/pinctrl-scmi-imx.c                 | 117 +++
drivers/pinctrl/pinctrl-scmi.c                     | 609 ++++++++++++++
drivers/pinctrl/pinctrl-scmi.h                     |  12 +
include/linux/scmi_protocol.h                      |  77 ++
14 files changed, 1849 insertions(+), 14 deletions(-)
[PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
Posted by Peng Fan (OSS) 1 year, 11 months ago
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 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.

---
Oleksii Moisieiev (1):
      firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support

Peng Fan (5):
      firmware: arm_scmi: introduce helper get_max_msg_size
      dt-bindings: firmware: arm,scmi: support pinctrl protocol
      pinctrl: Implementation of the generic scmi-pinctrl driver
      [NOT APPLY]firmware: scmi: support compatible string
      [NOT APPLY] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map

 .../devicetree/bindings/firmware/arm,scmi.yaml     |  50 ++
 MAINTAINERS                                        |   1 +
 drivers/firmware/arm_scmi/Makefile                 |   1 +
 drivers/firmware/arm_scmi/bus.c                    |  39 +-
 drivers/firmware/arm_scmi/common.h                 |   2 +-
 drivers/firmware/arm_scmi/driver.c                 |  32 +-
 drivers/firmware/arm_scmi/pinctrl.c                | 908 +++++++++++++++++++++
 drivers/firmware/arm_scmi/protocols.h              |   3 +
 drivers/pinctrl/Kconfig                            |  11 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-scmi-imx.c                 | 117 +++
 drivers/pinctrl/pinctrl-scmi.c                     | 609 ++++++++++++++
 drivers/pinctrl/pinctrl-scmi.h                     |  12 +
 include/linux/scmi_protocol.h                      |  77 ++
 14 files changed, 1849 insertions(+), 14 deletions(-)
---
base-commit: 5389a88b06eb19c3fb08200cc1519406e299b7b0
change-id: 20231215-pinctrl-scmi-4c5b0374f4c6

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>
Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
Posted by Peng Fan 1 year, 11 months ago
Hi Sudeep, Cristian

Would you pick up patch 1-4?
And for i.MX95 OEM extenstion, do you have any suggestions?
I have two points:
1. use vendor compatible. This would also benefit when supporting vendor 
protocol.
2. Introduce a property saying supporting-generic-pinconf

How do you think?

Thanks,
Peng.

在 1/21/2024 6:21 PM, Peng Fan (OSS) 写道:
> 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 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.
> 
> ---
> Oleksii Moisieiev (1):
>        firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
> 
> Peng Fan (5):
>        firmware: arm_scmi: introduce helper get_max_msg_size
>        dt-bindings: firmware: arm,scmi: support pinctrl protocol
>        pinctrl: Implementation of the generic scmi-pinctrl driver
>        [NOT APPLY]firmware: scmi: support compatible string
>        [NOT APPLY] pinctrl: scmi: implement pinctrl_scmi_imx_dt_node_to_map
> 
>   .../devicetree/bindings/firmware/arm,scmi.yaml     |  50 ++
>   MAINTAINERS                                        |   1 +
>   drivers/firmware/arm_scmi/Makefile                 |   1 +
>   drivers/firmware/arm_scmi/bus.c                    |  39 +-
>   drivers/firmware/arm_scmi/common.h                 |   2 +-
>   drivers/firmware/arm_scmi/driver.c                 |  32 +-
>   drivers/firmware/arm_scmi/pinctrl.c                | 908 +++++++++++++++++++++
>   drivers/firmware/arm_scmi/protocols.h              |   3 +
>   drivers/pinctrl/Kconfig                            |  11 +
>   drivers/pinctrl/Makefile                           |   1 +
>   drivers/pinctrl/pinctrl-scmi-imx.c                 | 117 +++
>   drivers/pinctrl/pinctrl-scmi.c                     | 609 ++++++++++++++
>   drivers/pinctrl/pinctrl-scmi.h                     |  12 +
>   include/linux/scmi_protocol.h                      |  77 ++
>   14 files changed, 1849 insertions(+), 14 deletions(-)
> ---
> base-commit: 5389a88b06eb19c3fb08200cc1519406e299b7b0
> change-id: 20231215-pinctrl-scmi-4c5b0374f4c6
> 
> Best regards,
Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
Posted by Sudeep Holla 1 year, 11 months ago
On Mon, Jan 29, 2024 at 08:36:50PM +0800, Peng Fan wrote:
> Hi Sudeep, Cristian
> 
> Would you pick up patch 1-4?

I will for v6.9 sometime.

> And for i.MX95 OEM extenstion, do you have any suggestions?
> I have two points:
> 1. use vendor compatible. This would also benefit when supporting vendor
> protocol.

May be, but that was never on plate for standard protocols. So I don't
like that approach either.

> 2. Introduce a property saying supporting-generic-pinconf
>

I am not sure what you mean by that. But that doesn't sound right especial
in context of SCMI. So I would say no.

> How do you think?
>

I don't have any other suggestions than fix your driver to use the pinmux
properly with features in the upstream pinmux subsystem.

-- 
Regards,
Sudeep
Re: [PATCH v3 0/6] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
Posted by Linus Walleij 1 year, 11 months ago
On Mon, Jan 29, 2024 at 1:37 PM Peng Fan <peng.fan@oss.nxp.com> wrote:

> And for i.MX95 OEM extenstion, do you have any suggestions?
> I have two points:
> 1. use vendor compatible. This would also benefit when supporting vendor
> protocol.
> 2. Introduce a property saying supporting-generic-pinconf
>
> How do you think?

While I don't know how OEM extensions to SCMI were designed,
the pin control subsystem has the philosophy that extensions are
for minor fringe stuff, such as a pin config option that no other
silicon is using and thus have no use for anyone else. Well that
is actually all the custom extensions we have.
(This notion is even carried over to SCMI pinctrl.)

The i.MX95 OEM extension is really odd to me, it looks like a
reimplementation of the core aspects of SCMI pin control, and
looks much more like the old i.MX drivers than like the SCMI driver.

But I sure cannot speak of what is allowed in SCMI OEM
extensions or not.

Yours,
Linus Walleij