[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1452742757.31558.8.camel@kernel.crashing.org>
Date: Thu, 14 Jan 2016 14:39:17 +1100
From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
To: Rongrong Zou <zourongrong@...il.com>, arnd@...db.de,
catalin.marinas@....com, will.deacon@....com
Cc: lijianhua@...wei.com, lixiancai@...wei.com, linuxarm@...wei.com,
linux-kernel@...r.kernel.org, minyard@....org,
gregkh@...uxfoundation.org
Subject: Re: [PATCH v1 3/3] ARM64 LPC: update binding doc
On Thu, 2016-01-14 at 10:03 +0800, Rongrong Zou wrote:
> On 2016/1/14 7:29, Benjamin Herrenschmidt wrote:
> > On Tue, 2015-12-29 at 21:33 +0800, Rongrong Zou wrote:
> > > Signed-off-by: Rongrong Zou <zourongrong@...il.com>
> > > ---
> > > .../devicetree/bindings/arm64/low-pin-count.txt | 20
> > > ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/arm64/low-
> > > pin-count.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm64/low-pin-
> > > count.txt b/Documentation/devicetree/bindings/arm64/low-pin-
> > > count.txt
> > > new file mode 100644
> > > index 0000000..215f2c4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm64/low-pin-count.txt
> > > @@ -0,0 +1,20 @@
> > > +Low Pin Count bus driver
> > > +
> > > +Usually LPC controller is part of PCI host bridge, so the legacy
> > > ISA
> > > +port locate on LPC bus can be accessed directly. But some SoC
> > > have
> > > +independent LPC controller, and we can access the legacy port by
> > > specifying
> > > +LPC address cycle. Thus, LPC driver is introduced.
> > > +
> > > +Required properties:
> > > +- compatible: "low-pin-count"
> >
> > I'm not sure about the above. I'd rather just make it "isa" or
> > maybe
> > isa-lpc. The LPC bus is fundamentally an ISA bus with the 3 cycle
> > types of ISA etc... I would also allow the node to be named "isa".
>
> I had modified its name to "isa@...*", otherwise, the kernel do not
> understand its children devices are on ISA bus.
Right, and the "compatible" property should be something like the
specific implementation of the LPC bridge. For example, ibm,power8-lpc
in my case. Not something generic.
Maybe we could allow for a generic one if the LPC is directly MMIO
mapped via the ranges property.
> > > +- reg: specifies low pin count address range
> > > +
> > > +
> > > +Example:
> > > +
> > > + lpc_0: lpc@...b0000 {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> >
> > As discussed earlier, address-cells should be 2 with the first cell
> > indicating the address space type (0 = mem, 1 = IO, possibly 2 =
> > firmware but that remains somewhat TBD).
> >
> > > + compatible = "low-pin-count";
> > > + reg = <0x0 0xa01b0000 0x0 0x10000>;
> >
> > And also as discussed, this is the business of the "ranges"
> > property so
> > that children devices can be properly expressed.
> >
>
> As discussed before,
> > A missing ranges property means that there is no translation,
> while an
> > empty ranges means a 1:1 translation to the parent bus.
> > We really want the former here, as I/O port addresses are not
> mapped into
> > the MMIO space of the parent bus.
Right ok but then it's not a generic binding for "low-pin-count". It's
a specific binding for that specific vendor LPC controller. In that
case yes, reg contains the registers for it but your compatible
property should be more precise.
For a generic binding of LPC, you'd want a ranges property though.
> > > + };
> >
> > Also, this being a bus binding, it should describe the format for
> > children (for example, PNP related properties).
> >
> > That leads to the obvious question: Why not just reference the
> > existing
> > Open Firmware ISA binding ?
>
> Unfortunately, I found all these bindings are based on memory-mapped
> I/O.
You should still refer to it for the definition of the properties of
children of the LPC.
Basically, a generic LPC bus is an ISA bus and honors the ISA binding.
A specific LPC controller provides a method of generating the ISA bus
cycles. If it's memory mapped, it can just stay generic and use a
ranges property. If not, it's more specific, thus has no range and has
a reg and a *precise* compatible property.
But that shouldn't affect the definition of the children nodes.
> Such as below binding I found in
> arch/x86/platform/ce4100/falconfalls.dts
> pci@3fc {
> #address-cells = <3>;
> #size-cells = <2>;
> compatible = "intel,ce4100-pci", "pci";
> device_type = "pci";
> bus-range = <0 0>;
> ranges = <0x2000000 0 0xbffff000 0xbffff000
> 0 0x1000
> 0x2000000 0 0xdffe0000 0xdffe0000
> 0 0x1000
> 0x0000000 0
> 0x0 0x0 0 0x100>;
>
> isa@1f,0 {
> #address-cells = <2>;
> #size-cells = <1>;
> compatible = "isa";
> reg = <0xf800 0x0 0x0 0x0 0x0>;
> ranges = <1 0 0 0 0 0x100>;
>
> rtc@70 {
> compatible = "intel,ce4100-
> rtc", "motorola,mc146818";
> interrupts = <8 3>;
> interrupt-parent =
> <&ioapic1>;
> ctrl-reg = <2>;
> freq-reg = <0x26>;
> reg = <1 0x70 2>;
> };
> };
> };
Yes that's a generic one.
Cheers,
Ben.
Powered by blists - more mailing lists