Em Tue, 18 Aug 2020 11:07:55 -0600
Rob Herring <robh@xxxxxxxxxx> escreveu:
> > > > + spmi-channel:
> > > > + description: number of the SPMI channel where the PMIC is connected
> > >
> > > This looks like a common (to SPMI), but it's not something defined in
> > > spmi.txt
> >
> > This one is not part of the SPMI core. It is stored inside a private
> > structure inside at the HiSilicon spmi controller driver. It is stored
> > there as ctrl_dev->channel, and it is used to calculate the register offset
> > for readl():
> >
> > offset = SPMI_APB_SPMI_STATUS_BASE_ADDR;
> > offset += SPMI_CHANNEL_OFFSET * ctrl_dev->channel + SPMI_SLAVE_OFFSET * sid;
> > do {
> > status = readl(base + offset);
> > ...
> >
> > The SPMI bus is somewhat similar to I2C: it is a 2-wire serial bus
> > with up to 16 devices connected to it.
> >
> > Now, most modern I2C chipsets provide multiple independent I2C
> > channels, on different pins. Also, on some chipsets, certain
> > GPIO pins can be used either as GPIO or as I2C.
> >
> > I strongly suspect that this is the case here: according with
> > the Hikey 970 schematics:
> >
> > https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf
> >
> > The pins used by SPMI clock/data can also be used as GPIO.
> >
> > While I don't have access to the datasheets for Kirin 970 (or any other
> > chipsets on this board), for me, it sounds that different GPIO pins
> > are allowed to use SPMI. The "spmi-channel" property specifies
> > what pins will be used for SPMI, among the ones that are
> > compatible with MIPI SPMI specs.
>
> Based on this, I think it should be called 'hisilicon,spmi-channel' as
> it is vendor specific.
I'm fine with "hisilicon,spmi-channel".
> > > > +
> > > > + vsel-reg:
> > > > + description: Voltage selector register.
> > >
> > > 'reg' can have multiple entries if you want.
> >
> > Yes, I know. I was in doubt if I should either place vsel-reg on
> > a separate property or together with reg. I ended keeping it
> > in separate on the submitted patch series.
> >
> > What makes more sense?
>
> Really, not putting it in DT. Like other things, it's fixed for the
> chip.
I agree, but, as I said before, without the datasheet, we can only
hardcode a small subset of the LDO settings.
Due to that, I prefer keeping it at DT - either grouped together at "reg" or
as two separated properties (reg and vsel-reg).
> > > > +description: |
> > > > + The HiSilicon SPMI controller is found on some Kirin-based designs.
> > > > + It is a MIPI System Power Management (SPMI) controller.
> > > > +
> > > > + The PMIC part is provided by
> > > > + Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml.
> > > > +
> > > > +properties:
> > > > + $nodename:
> > > > + pattern: "spmi@[0-9a-f]"
> > > > +
> > > > + compatible:
> > > > + const: hisilicon,spmi-controller
> > >
> > > Needs an SoC specific compatible.
> >
> > What about:
> > hisilicon,kirin970-spmi-controller
>
> Is 'kirin970' really the SoC name? The older ones are all 'hi[0-9]+'.
This SoC is named Kirin 970. Yet, you can see places where 3670 is
used, like:
https://en.wikichip.org/wiki/hisilicon/kirin/970
There, it says that Hi3670 is the part number.
Thanks,
Mauro
