Em Fri, 5 Feb 2021 16:19:47 -0600 Rob Herring <robh@xxxxxxxxxx> 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+huawei@xxxxxxxxxx> > > --- > > .../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+huawei@xxxxxxxxxx> > > + > > +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
