| Message ID | 20240314-pinctrl-scmi-v5-0-b19576e557f2@nxp.com |
|---|---|
| Headers | show |
| Series | firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support | expand |
On Fri, Mar 15, 2024 at 12:31:51AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH v5 0/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol > > protocol basic support > > > > On Thu, Mar 14, 2024 at 09:35:17PM +0800, Peng Fan (OSS) wrote: > > > Since SCMI 3.2 Spec is released, and this patchset has got R-b/T-b, is > > > it ok to land this patchset? > > > > > > > I'll have a look at this last version and a spin on my test setup. > > > > ...but has this V5 change at all since the Reviewed-by tags due to the latest > > spec changes ? > > The tags are same as V4. I only did a rebase, no more changes. > > Ok. > > ...IOW does this V5 include the latest small bits spec-changes or those latest > > gpio-related spec-changes are just not needed at the level of the Linux pinctrl > > support as of now and can be added later on when a Linux gpio driver will be > > built on top of this ? > > In my current test, I no need the gpio related changes, so I would add that later > if you are ok. > I COULD have agreed with this, since AFAIK there is currently an effort to add support for GPIO on top of SCMI Pinctrl BUT not in Linux, so no reason to block this series for gpio-related missing features, that should only be additions not breaking backward compatibility... ....BUT, I've just wrapped my head again around the latest public release of v3.2 spec (which has gone through so many changes and additions that I had lost track O_o) AND beside the above mentioned GPIO changes there are indeed also BREAKING changes around the commands PINCTRL_SETTINGS_GET and PINCTRL_SETTINGS_CONFIGURE (which were the old PINCTRL_CONFIG_GET/SET), that now also get/set the selected function: so that, at the end the payload itself of those commands/replies has also changed IN SIZE, so the driver needs definitely to be updated (and whatever you use to test on the backend server too, if you want to test this...) I think these changes (which I forgot being there) were in since last month, so already V4 was broken in these regards (which I have not looked at) I'll leave some comments along the series and test all of this again next week... ...since too many things has changed and I want to re-verify all on my side. Thanks, Cristian
On Fri, Mar 15, 2024 at 04:53:10PM +0000, Cristian Marussi wrote: > On Fri, Mar 15, 2024 at 12:31:51AM +0000, Peng Fan wrote: > (and whatever you use to test on the backend server too, if you want > to test this...) > What are people using to test this, btw? regards, dan carpenter
On Fri, Mar 15, 2024 at 09:20:02PM +0300, Dan Carpenter wrote: > On Fri, Mar 15, 2024 at 04:53:10PM +0000, Cristian Marussi wrote: > > On Fri, Mar 15, 2024 at 12:31:51AM +0000, Peng Fan wrote: > > (and whatever you use to test on the backend server too, if you want > > to test this...) > > > > What are people using to test this, btw? Hi Dan, I think NXP has their own SCMI server embedded in TF-A (Peng posted a link at the public repo a while ago I think...) supporting Pinctrl; additionally Oleksii/EPAM (the original author of this series) have their own distinct proprietary SCMI server implementation with Pinctrl (not sure where it run from in this case but it is a Xen based setup if I remember right..). Beside these, there are at least a couple more Vendor proprietary incarnations of SCMI servers that I am aware of (that most probably did not support Pinctrl anyway as of now...) On top of this, on the other side there is, of course, the official SCP/SCMI server reference implementation, that can live in a number of different places thanks to the the virtualized environment built by Linaro, but this latter SCP/SCMI server does not have any official Pinctrl support either as of now. Thanks, Cristian
Since SCMI 3.2 Spec is released, and this patchset has got R-b/T-b, is it ok to land this patchset? 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 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. --- Oleksii Moisieiev (1): firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support Peng Fan (3): 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 .../devicetree/bindings/firmware/arm,scmi.yaml | 50 ++ MAINTAINERS | 1 + drivers/firmware/arm_scmi/Makefile | 1 + drivers/firmware/arm_scmi/driver.c | 17 + 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.c | 593 ++++++++++++++ include/linux/scmi_protocol.h | 75 ++ 10 files changed, 1660 insertions(+) --- base-commit: 6bb954de844bc99bf9e90f304a41d9477efe468b change-id: 20231215-pinctrl-scmi-4c5b0374f4c6 Best regards,