Re: [RFC 0/6] firmware: scmi: add SCMI pinctrl protocol support

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Hi Peng,

On Wed, Sep 06, 2023 at 12:09:45AM -0300, Fabio Estevam wrote:
> Adding Peng Fan, who is working on scmi/pinctrl support for i.MX9:
> 
> https://lore.kernel.org/all/ZO9GLG5tQynYyAvR@pluto/T/

I made a comment there.

BTW, do you already have your own implementation of SCMI pin control
protocol at SCMI firmware(server)?
Is it public available?

-Takahiro Akashi


> On Tue, Sep 5, 2023 at 11:41 PM AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > This is an RFC and meant to get feedback from other developers as
> > - the specification (pinctrl part) is still in a draft
> > - the upstream patch for linux, including dt bindings, is still WIP
> > - I'm not confident the drivers are generic enough to cover most HWs
> > - The tests ("ut") doesn't cover all the features yet
> >
> >
> > This patch series allows users to access SCMI pin control protocol provided
> > by SCMI server (platform). See SCMI specification document v3.2 beta 2[1]
> > for more details about SCMI pin control protocol.
> >
> > The implementation consists of two layers:
> > - basic helper functions for SCMI pin control protocol
> >   in drivers/firmware/scmi/pinctrl.c (patch#2)
> > - DM-compliant pinctrl/gpio drivers, which utilizes the helper functions,
> >   in drivers/pinctrl/pinctrl-scmi.c (patch#3,#4)
> >
> > [1] https://developer.arm.com/documentation/den0056/e/?lang=en
> >
> > DT bindings
> > ===========
> > Upstream pinctrl patch for linux defines the bindings in [2] though
> > it doesn't say much.
> > I expect that my implementation basically complies with U-Boot's generic
> > bindings described in [3], but not all the features are verified.
> >
> > As for gpio, unless you hard-code pin assignments directly in a device
> > driver, my implementation allows the following alternatives in DT.
> > Either way, we may need an additional binding description for gpio.
> >
> > (A)
> >     scmi {
> >         ... // other protocols
> >         scmi_pinctrl: protocol@19 { // Pin control protocol
> >             ...
> >             {pinmux definitions}... // if any, including GPIO?
> >         }
> >     }
> >     scmi_gpio: scmi_gpio {
> >         compatible = "arm,scmi-gpio-generic";
> >         gpio-controller;
> >         #gpio-cells = <2>;
> >         gpio-ranges = <&scmi_pinctrl 0 5 4>,
> >                       <&scmi_pinctrl 4 0 0>;
> >         gpio-ranges-group-names = "",
> >                                   "ANOTHER_GPIO_GROUP";
> >     }
> >     some_device {
> >         ...
> >         reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>;
> >     }
> > (B)
> >     scmi {
> >         ... // other protocols
> >         scmi_pinctrl: protocol@19 { // Pin control protocol
> >             ...
> >             {pinmux definitions}... // if any, including GPIO?
> >
> >             scmi_gpio: scmi_gpio {
> >                 // no need for "compatible"
> >                 gpio-controller;
> >                 #gpio-cells = <2>;
> >                 gpio-ranges = <&scmi_pinctrl 0 5 4>,
> >                               <&scmi_pinctrl 4 0 0>;
> >                 gpio-ranges-group-names = "",
> >                                           "ANOTHER_GPIO_GROUP";
> >             }
> >         }
> >     }
> >     some_device {
> >         ...
> >         reset-gpios = <&scmi_gpio 0 GPIO_ACTIVE_HIGH>;
> >     }
> > (C)
> >     if "gpio-ranges" is missing in gpio definition, assume 1:1 mapping,
> >     i.e. use a native pinctrl pin number (5).
> >     some_device {
> >         ...
> >         reset-gpios = <&scmi_gpio 5 GPIO_ACTIVE_HIGH>;
> >     }
> >
> >
> > [2] https://lkml.iu.edu/hypermail/linux/kernel/2308.1/01084.html
> > [3] <u-boot>/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt
> >     <u-boot>/doc/device-tree-bindings/gpio/gpio.txt
> >
> > Test
> > ====
> > The patch series was tested on the following platforms:
> > * sandbox ("ut dm pinmux" and manually using gpio command)
> >
> >
> > Prerequisite:
> > =============
> > * This patch series is based on my WIP "Base protocol support" patches
> >   on v2023.10-rc3. You can fetch the whole code from [4].
> >
> > [4] https://git.linaro.org/people/takahiro.akashi/u-boot.git
> >     branch:scmi/pinctrl
> >
> >
> > Patches:
> > ========
> > Patch#1: Add SCMI base protocol driver
> > Patch#2-#4: Add drivers
> > Patch#5-#6: Test related
> >
> >
> > Change history:
> > ===============
> > RFC (Sep 6, 2023)
> > * initial release as RFC
> >
> > AKASHI Takahiro (6):
> >   firmware: scmi: fix protocol enumeration logic
> >   firmware: scmi: add pinctrl protocol support
> >   pinctrl: add scmi driver
> >   gpio: add scmi driver based on pinctrl
> >   firmware: scmi: add pseudo pinctrl protocol support on sandbox
> >   test: dm: add SCMI pinctrl test
> >
> >  arch/sandbox/dts/test.dts                  |  115 +++
> >  cmd/scmi.c                                 |    1 +
> >  drivers/firmware/scmi/Kconfig              |    3 +
> >  drivers/firmware/scmi/Makefile             |    1 +
> >  drivers/firmware/scmi/pinctrl.c            |  412 ++++++++
> >  drivers/firmware/scmi/sandbox-scmi_agent.c |  722 +++++++++++++
> >  drivers/firmware/scmi/scmi_agent-uclass.c  |   18 +-
> >  drivers/pinctrl/Kconfig                    |   11 +
> >  drivers/pinctrl/Makefile                   |    1 +
> >  drivers/pinctrl/pinctrl-scmi.c             | 1071 ++++++++++++++++++++
> >  include/scmi_agent-uclass.h                |    2 +
> >  include/scmi_protocols.h                   |  435 ++++++++
> >  test/dm/scmi.c                             |   62 ++
> >  13 files changed, 2852 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/firmware/scmi/pinctrl.c
> >  create mode 100644 drivers/pinctrl/pinctrl-scmi.c
> >
> > --
> > 2.34.1
> >