From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>,
Colin Ian King <colin.king@canonical.com>,
Dan Carpenter <dan.carpenter@oracle.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Mayulong <mayulong1@huawei.com>, Stephen Boyd <sboyd@kernel.org>,
YueHaibing <yuehaibing@huawei.com>,
devel@driverdev.osuosl.org, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 17/21] spmi: hisi-spmi-controller: move driver from staging
Date: Thu, 25 Mar 2021 14:47:43 +0100 [thread overview]
Message-ID: <20210325144743.2d740a06@coco.lan> (raw)
In-Reply-To: <20210205221947.GA3848249@robh.at.kernel.org>
Em Fri, 5 Feb 2021 16:19:47 -0600
Rob Herring <robh@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+huawei@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+huawei@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
next prev parent reply other threads:[~2021-03-25 13:48 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-19 16:10 [PATCH v4 00/21] Move Hisilicon 6421v600 SPMI driver set out of staging Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 01/21] staging: hikey9xx: hisilicon,hisi-spmi-controller.yaml fix bindings Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 02/21] staging: hikey9xx: hisilicon,hi6421-spmi-pmic.yaml: simplify props Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 03/21] staging: hikey9xx: hisi-spmi-controller: clean sparse warnings Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 04/21] staging: hikey9xx: hi6421v600-regulator: do some cleanups Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 05/21] staging: hikey9xx: hi6421v600-regulator: move LDO config from DT Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 06/21] staging: hikey9xx: hi6421v600-regulator: cleanup debug msgs Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 07/21] staging: hikey9xx: hi6421v600-regulator: get rid of an static data Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 08/21] staging: hikey9xx: hi6421v600-regulator: do some cleanups Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 09/21] staging: hikey9xx: hi6421v600-regulator: update copyright Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 10/21] staging: hikey9xx: hi6421v600-regulator: fix delay logic Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 11/21] staging: hikey9xx: hi6421v600-regulator: cleanup comments Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 12/21] staging: hikey9xx: hi6421v600-regulator: fix get_optimum_mode Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 13/21] staging: hikey9xx: hisilicon,hi6421-spmi-pmic.yaml: cleanup a warning Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 14/21] staging: hikey9xx: spmi driver: convert to regmap Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 15/21] staging: hikey9xx: hi6421-spmi-pmic: update copyright Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 16/21] staging: hikey9xx: simplify includes Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 17/21] spmi: hisi-spmi-controller: move driver from staging Mauro Carvalho Chehab
2021-02-05 22:19 ` Rob Herring
2021-03-25 13:47 ` Mauro Carvalho Chehab [this message]
2021-01-19 16:10 ` [PATCH v4 18/21] mfd: hi6421-spmi-pmic: " Mauro Carvalho Chehab
2021-01-27 11:05 ` Lee Jones
2021-02-05 22:26 ` Rob Herring
2021-02-05 22:26 ` Rob Herring
2021-01-19 16:10 ` [PATCH v4 19/21] regulator: hi6421v600-regulator: move it " Mauro Carvalho Chehab
2021-01-19 16:19 ` Mark Brown
2021-01-19 23:02 ` Mauro Carvalho Chehab
2021-01-20 17:07 ` Mark Brown
2021-01-21 7:29 ` Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 20/21] dts: hisilicon: add support for USB3 on Hikey 970 Mauro Carvalho Chehab
2021-01-19 16:10 ` [PATCH v4 21/21] dts: hisilicon: add support for the PMIC found " Mauro Carvalho Chehab
2021-01-19 23:01 ` [PATCH v4 22/21] regulator: hi6421v600-regulator: use some regmap helpers Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210325144743.2d740a06@coco.lan \
--to=mchehab+huawei@kernel.org \
--cc=broonie@kernel.org \
--cc=colin.king@canonical.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=lee.jones@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mayulong1@huawei.com \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=yuehaibing@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).