| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210325144743.2d740a06@coco.lan>
Date: Thu, 25 Mar 2021 14:47:43 +0100
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Mark Brown <broonie@...nel.org>, Lee Jones <lee.jones@...aro.org>,
Colin Ian King <colin.king@...onical.com>,
Dan Carpenter <dan.carpenter@...cle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Mayulong <mayulong1@...wei.com>, Stephen Boyd <sboyd@...nel.org>,
YueHaibing <yuehaibing@...wei.com>, devel@...verdev.osuosl.org,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 17/21] spmi: hisi-spmi-controller: move driver from
staging
Em Fri, 5 Feb 2021 16:19:47 -0600
Rob Herring <robh@...nel.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+huawei@...nel.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+huawei@...nel.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@...24000 {
> > + 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
Powered by blists - more mailing lists