| Message ID | 1478097481-14895-4-git-send-email-andrew@aj.id.au |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew@aj.id.au> wrote: > 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@aj.id.au> > --- > Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++ > 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) I had to check the data sheet for that acronym. They call the registers LHC. I somewhat prefer that name, but if you're happy with it as-is then that's fine. I assume this is not an issue on the g4/ast2400? > + > +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. > + > +Example: > + > +lpchc: lpchc@1e7890a0 { > + compatible = "aspeed,ast2500-lpchc", "syscon"; > + reg = <0x1e7890a0 0xc4>; Where's the 0xc4 come from? I can see 9 registers, which would mean the length should be 0x24? Cheers, Joel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-11-04 at 09:36 +1030, Joel Stanley wrote: > On Thu, Nov 3, 2016 at 1:07 AM, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > 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@aj.id.au> > > --- > > Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++ > > 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) > I had to check the data sheet for that acronym. They call the > registers LHC. I somewhat prefer that name, but if you're happy with > it as-is then that's fine. I had an internal debate about this. I figured LPCHC might give a bit more context to the acronym. I'm not unhappy with it but I wouldn't claim I'm happy either. I will change it to LHC since you somewhat prefer it, and it better aligns with the datasheet. > > I assume this is not an issue on the g4/ast2400? Correct, we don't have the issue of pinmux needing to reach into the LPC IO space on the AST2400. I don't think we've had anything else to drive us to looking at the host controller space there, so I wasn't going to add it to the bindings yet. > > > > > + > > +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. > > + > > +Example: > > + > > +lpchc: lpchc@1e7890a0 { > > + compatible = "aspeed,ast2500-lpchc", "syscon"; > > + reg = <0x1e7890a0 0xc4>; > Where's the 0xc4 come from? I can see 9 registers, which would mean > the length should be 0x24? Yes, it should be 0x24. I can't even claim that 'c' is near '2'. Thanks for catching that. Andrew > > Cheers, > > Joel
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@aj.id.au> > --- > Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++ Create a new directory in bindings/mfd called 'syscon'. Or perhaps 'bindings/syscon'. > 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? > +Example: > + > +lpchc: lpchc@1e7890a0 { > + compatible = "aspeed,ast2500-lpchc", "syscon"; > + reg = <0x1e7890a0 0xc4>; > +};
[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@aj.id.au> > > --- > > Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++ > > Create a new directory in bindings/mfd called 'syscon'. > > Or perhaps 'bindings/syscon'. > > > 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? > > > +Example: > > + > > +lpchc: lpchc@1e7890a0 { > > + compatible = "aspeed,ast2500-lpchc", "syscon"; > > + reg = <0x1e7890a0 0xc4>; > > +}; >
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@aj.id.au> > > > --- > > > 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>; > > > +}; > >
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. + +Example: + +lpchc: lpchc@1e7890a0 { + compatible = "aspeed,ast2500-lpchc", "syscon"; + reg = <0x1e7890a0 0xc4>; +};
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@aj.id.au> --- Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpchc.txt