Re: [PATCH v2 3/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LPCHC)

From: Andrew Jeffery
Date: Mon Nov 21 2016 - 22:25:58 EST


On Fri, 2016-11-18 at 18:45 +0000, Lee Jones wrote:
> [Sending Arnd this time!]
>
> > Arnd,
> >
> > Do you have a preference?
> >
> > > The Aspeed LPC Host Controller is presented as a syscon device to
> > > arbitrate access by LPC and pinmux drivers. LPC pinmux configuration on
> > > fifth generation SoCs depends on bits in both the System Control Unit
> > > and the LPC Host Controller.
> > >
> > > > > > Signed-off-by: Andrew Jeffery <andrew@xxxxxxxx>
> > > ---
> > > ÂDocumentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++
> >
> > Create a new directory in bindings/mfd called 'syscon'.
> >
> > Or perhaps 'bindings/syscon'.
> >

Sounds good to me. I'll wait for Arnd's feedback.

Note that this patch conflicts with some of the ideas I outlined in

https://www.spinics.net/lists/arm-kernel/msg543233.html

I sent it hoping to get some feedback on the approach to take for these
LPC-related bits. Did you have any suggestions? The problems with
hardware complexity are amplified by the fact that the datasheet is
only available under NDA, but I will do what I can to clarify.

> > > Â1 file changed, 17 insertions(+)
> > > Âcreate mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > > new file mode 100644
> > > index 000000000000..792651488c3d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt
> > > @@ -0,0 +1,17 @@
> > > +* Device tree bindings for the Aspeed LPC Host Controller (LPCHC)
> > > +
> > > +The LPCHC registers configure LPC behaviour between the BMC and the host
> > > +system. The LPCHC also participates in pinmux requests on g5 SoCs and is
> > > +therefore considered a syscon device.
> > > +
> > > +Required properties:
> > > > > > +- compatible: "aspeed,ast2500-lpchc", "syscon"
> > > > > > +- reg: contains offset/length value of the LPCHC memory
> > > + region.
> >
> > Why not just use a single tab, then you don't have to linewrap?

I'll clean that up.

Cheers,

Andrew

> >
> > > +Example:
> > > +
> > > > > > +lpchc: lpchc@1e7890a0 {
> > > > > > + compatible = "aspeed,ast2500-lpchc", "syscon";
> > > > > > + reg = <0x1e7890a0 0xc4>;
> > > +};
>
>

Attachment: signature.asc
Description: This is a digitally signed message part