Em Fri, 5 Feb 2021 16:19:47 -0600
Rob Herring <r...@kernel.org> escreveu:

> On Tue, Jan 19, 2021 at 05:10:43PM +0100, Mauro Carvalho Chehab wrote:
> > The Hisilicon 6421v600 SPMI driver is ready for mainstream.
> > 
> > So, move it from staging.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
> > ---
> >  .../spmi/hisilicon,hisi-spmi-controller.yaml  |  75 ++++
> >  MAINTAINERS                                   |   7 +
> >  drivers/spmi/Kconfig                          |   9 +
> >  drivers/spmi/Makefile                         |   1 +
> >  drivers/spmi/hisi-spmi-controller.c           | 358 ++++++++++++++++++
> >  drivers/staging/hikey9xx/Kconfig              |  11 -
> >  drivers/staging/hikey9xx/Makefile             |   1 -
> >  .../staging/hikey9xx/hisi-spmi-controller.c   | 358 ------------------
> >  .../hisilicon,hisi-spmi-controller.yaml       |  75 ----
> >  9 files changed, 450 insertions(+), 445 deletions(-)
> >  create mode 100644 
> > Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> >  create mode 100644 drivers/spmi/hisi-spmi-controller.c
> >  delete mode 100644 drivers/staging/hikey9xx/hisi-spmi-controller.c
> >  delete mode 100644 
> > drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> >  
> > b/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> > new file mode 100644
> > index 000000000000..21f68a9c2df1
> > --- /dev/null
> > +++ 
> > b/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/spmi/hisilicon,hisi-spmi-controller.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HiSilicon SPMI controller
> > +
> > +maintainers:
> > +  - Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
> > +
> > +description: |
> > +  The HiSilicon SPMI BUS controller is found on some Kirin-based designs.
> > +  It is a MIPI System Power Management (SPMI) controller.
> > +
> > +  The PMIC part is provided by
> > +  drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "spmi@[0-9a-f]"
> > +
> > +  compatible:
> > +    const: hisilicon,kirin970-spmi-controller  
> 
> '-controller' is kind of redundant.

Ok. Will drop it.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +  
> 
> > +  "#address-cells":
> > +    const: 2
> > +
> > +  "#size-cells":
> > +    const: 0  
> 
> These 2 are covered by spmi.yaml

Ok.

> 
> > +
> > +  spmi-channel:
> > +    description: |
> > +      number of the Kirin 970 SPMI channel where the SPMI devices are 
> > connected.  
> 
> Common to SPMI? If not, needs a vendor prefix.

That's an interesting question. My understanding is that this is not
vendor-specific, but maybe Stephen can give us more details.

The spmi.h header calls it "nr", and documents it at include/linux/spmi.h
as:

        /**
         * struct spmi_controller - interface to the SPMI master controller
         * @dev:        Driver model representation of the device.
         * @nr:         board-specific number identifier for this controller/bus
         * @cmd:        sends a non-data command sequence on the SPMI bus.
         * @read_cmd:   sends a register read command sequence on the SPMI bus.
         * @write_cmd:  sends a register write command sequence on the SPMI bus.
         */

There, it says that this is "board-specific number identifier".

Yet, as the SPMI is a serial bus with up to 4 masters (controller), I 
suspect that the idea is to associate it with the master ID.

This is used on boards with multiple SoCs. See, for instance, slide 5 of:

        
https://www.mipi.org/sites/default/files/Bangalore-Qualcomm-SPMI-1.0-Multi-master-Verification.pdf

However, it is hard to know for sure, as no drivers use it, except by
Hikey 970 controller:

        $ grep "\b\->nr\b" $(git grep -l spmi.h)
        drivers/spmi/spmi.c:    ida_simple_remove(&ctrl_ida, ctrl->nr);
        drivers/spmi/spmi.c:    dev_set_name(&sdev->dev, "%d-%02x", ctrl->nr, 
sdev->usid);
        drivers/spmi/spmi.c:    ctrl->nr = id;
        drivers/spmi/spmi.c:            ctrl->nr, &ctrl->dev);
        drivers/staging/hikey9xx/hisi-spmi-controller.c:        ctrl->nr = 
spmi_controller->channel;

> 
> Type? Range of values?

The SPMI core defines it as "unsigned int". So, I would use:

            $ref: /schemas/types.yaml#/definitions/uint32

as a type. 

At the driver, this is used to calculate the channel offset with:

        static int spmi_write_cmd(...) {
                u32 chnl_ofst = SPMI_CHANNEL_OFFSET * spmi_controller->channel;
...
                writel((u32 __force)cpu_to_be32(data),
                       spmi_controller->base + chnl_ofst +
                       SPMI_APB_SPMI_WDATA0_BASE_ADDR +
                       SPMI_PER_DATAREG_BYTE * i);
...
        }

As on both spmi.h and the Hikey 970 SPMI controller defines it as uint32, 
it doesn't seem to be a good idea to put a range of values, specially 
since we don't have the datasheets for this SoC.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spmi-channel  
> 
> > +  - "#address-cells"
> > +  - "#size-cells"  
> 
> Covered by spmi.yaml.
> 
> > +
> > +patternProperties:
> > +  "^pmic@[0-9a-f]$":  
> 
> Presumably you could have something besides a PMIC.

Hmm... SPMI means MIPI System Power Management Interface.
The MIPI says that [1]:

        "The MIPI System Power Management Interface is a two-wire serial
         interface that uses CMOS I/Os for the physical layer. The interface
         connects the integrated power controller of a system-on-chip (SoC)
         processor system with one or more power management IC voltage
         regulation systems."

[1] https://www.mipi.org/specifications/system-power-management-interface

OK, as this is a serial bus, I guess one could abuse the interface
and add non-PMIC devices on it. Also, some future version of SPMI
might extend it to non-PMIC devices, but, IMO, if we ever add a 
non-PMIC device, another patternProperties would be needed in order
to describe the other device types that could be connected to the PM bus.

> 
> > +    description: |
> > +      PMIC properties, which are specific to the used SPMI PMIC device(s).
> > +      When used in combination with HiSilicon 6421v600, the properties
> > +      are documented at
> > +      drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml.
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    bus {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      spmi: spmi@fff24000 {
> > +        compatible = "hisilicon,kirin970-spmi-controller";
> > +        #address-cells = <2>;
> > +        #size-cells = <0>;
> > +        status = "ok";  
> 
> Drop status.

Ok.

> 
> > +        reg = <0x0 0xfff24000 0x0 0x1000>;
> > +        spmi-channel = <2>;
> > +
> > +        pmic@0 {
> > +          reg = <0 0>;
> > +          /* pmic properties */
> > +        };
> > +      };
> > +    };


Thanks,
Mauro
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to