Message ID | 20200819031723.1398378-2-nico@fluxnic.net |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | [v2,1/2] dt-bindings: i3c: MIPI I3C Host Controller Interface | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | success |
On Tue, Aug 18, 2020 at 11:17:22PM -0400, Nicolas Pitre wrote: > From: Nicolas Pitre <npitre@baylibre.com> > > The MIPI I3C HCI (Host Controller Interface) specification defines > a common software driver interface to support compliant MIPI I3C > host controller hardware implementations from multiple vendors. > > Signed-off-by: Nicolas Pitre <npitre@baylibre.com> > --- > .../devicetree/bindings/i3c/mipi-i3c-hci.yaml | 41 +++++++++++++++++++ > 1 file changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > diff --git a/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > new file mode 100644 > index 0000000000..8fc18ea922 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > @@ -0,0 +1,41 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/i3c/mipi-i3c-hci.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: MIPI I3C HCI Device Tree Bindings > + > +maintainers: > + - Nicolas Pitre <npitre@baylibre.com> > + > +description: | > + MIPI I3C Host Controller Interface > + > + The MIPI I3C HCI (Host Controller Interface) specification defines > + a common software driver interface to support compliant MIPI I3C > + host controller hardware implementations from multiple vendors. > + > + For details, please see: > + https://www.mipi.org/specifications/i3c-hci > + > +properties: > + compatible: > + const: mipi-i3c-hci What about my comments on v1? Pasted again: A register interface (or protocol) spec is never complete enough to capture all the details about a specific h/w implementation. One just has to go look at AHCI, EHCI, OHCI, XHCI, UFS, 8250, etc. bindings. Let's not start with pretending that here. Fine for this to be a fallback, but it must have a compatible for a specific implementation. Also, which version of the spec does this compatible correspond to? Or are there not HCI differences in the spec versions you mention in the cover letter? > + reg: > + maxItems: 1 > + interrupts: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + > +examples: > + - | > + mipi_i3c_hci@a0000000 { i3c@a0000000 > + compatible = "mipi-i3c-hci"; > + reg = <0xa0000000 0x2000>; > + interrupts = <89>; > + }; > -- > 2.26.2 >
On Tue, 25 Aug 2020, Rob Herring wrote: > On Tue, Aug 18, 2020 at 11:17:22PM -0400, Nicolas Pitre wrote: > > From: Nicolas Pitre <npitre@baylibre.com> > > > > The MIPI I3C HCI (Host Controller Interface) specification defines > > a common software driver interface to support compliant MIPI I3C > > host controller hardware implementations from multiple vendors. > > > > Signed-off-by: Nicolas Pitre <npitre@baylibre.com> > > --- > > .../devicetree/bindings/i3c/mipi-i3c-hci.yaml | 41 +++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > > > diff --git a/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > new file mode 100644 > > index 0000000000..8fc18ea922 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > @@ -0,0 +1,41 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/i3c/mipi-i3c-hci.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: MIPI I3C HCI Device Tree Bindings > > + > > +maintainers: > > + - Nicolas Pitre <npitre@baylibre.com> > > + > > +description: | > > + MIPI I3C Host Controller Interface > > + > > + The MIPI I3C HCI (Host Controller Interface) specification defines > > + a common software driver interface to support compliant MIPI I3C > > + host controller hardware implementations from multiple vendors. > > + > > + For details, please see: > > + https://www.mipi.org/specifications/i3c-hci > > + > > +properties: > > + compatible: > > + const: mipi-i3c-hci > > What about my comments on v1? Pasted again: Oops, sorry, I missed them. > A register interface (or protocol) spec is never complete enough to > capture all the details about a specific h/w implementation. One just > has to go look at AHCI, EHCI, OHCI, XHCI, UFS, 8250, etc. bindings. > Let's not start with pretending that here. Fine for this to be a > fallback, but it must have a compatible for a specific implementation. You might have to indulge me a bit as I don't understand what you're asking. Currently there are very few implementations. One of them lives in an FPGA and the example below is actually the DT entry I use for it. I'm guessing specific vendor implementations will have their own tweaks eventually, such as clock sources and whatnot. But that is outside of the spec (actually the spec defines a register area for eventual vendor specific usage). But I have no visibility into that and of course the code has no provision for that yet either. So I imagine there will be something like this in dts files eventually: compatibvle = "intel,foobar_soc_i3c_hci", "mipi-i3c-hci"; Is that what you mean? > Also, which version of the spec does this compatible correspond to? All of them. > Or are there not HCI differences in the spec versions you mention in > the cover letter? The hardware is self advertising per the spec. So there is no need to carry such distinction in the DT compatible. Even vendor extensions are tagged with MIPI vendor IDs in the hardware directly. > > + reg: > > + maxItems: 1 > > + interrupts: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +examples: > > + - | > > + mipi_i3c_hci@a0000000 { > > i3c@a0000000 OK. Nicolas
On Tue, Aug 25, 2020 at 4:02 PM Nicolas Pitre <nico@fluxnic.net> wrote: > > On Tue, 25 Aug 2020, Rob Herring wrote: > > > On Tue, Aug 18, 2020 at 11:17:22PM -0400, Nicolas Pitre wrote: > > > From: Nicolas Pitre <npitre@baylibre.com> > > > > > > The MIPI I3C HCI (Host Controller Interface) specification defines > > > a common software driver interface to support compliant MIPI I3C > > > host controller hardware implementations from multiple vendors. > > > > > > Signed-off-by: Nicolas Pitre <npitre@baylibre.com> > > > --- > > > .../devicetree/bindings/i3c/mipi-i3c-hci.yaml | 41 +++++++++++++++++++ > > > 1 file changed, 41 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > > new file mode 100644 > > > index 0000000000..8fc18ea922 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml > > > @@ -0,0 +1,41 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/i3c/mipi-i3c-hci.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: MIPI I3C HCI Device Tree Bindings > > > + > > > +maintainers: > > > + - Nicolas Pitre <npitre@baylibre.com> > > > + > > > +description: | > > > + MIPI I3C Host Controller Interface > > > + > > > + The MIPI I3C HCI (Host Controller Interface) specification defines > > > + a common software driver interface to support compliant MIPI I3C > > > + host controller hardware implementations from multiple vendors. > > > + > > > + For details, please see: > > > + https://www.mipi.org/specifications/i3c-hci > > > + > > > +properties: > > > + compatible: > > > + const: mipi-i3c-hci > > > > What about my comments on v1? Pasted again: > > Oops, sorry, I missed them. > > > A register interface (or protocol) spec is never complete enough to > > capture all the details about a specific h/w implementation. One just > > has to go look at AHCI, EHCI, OHCI, XHCI, UFS, 8250, etc. bindings. > > Let's not start with pretending that here. Fine for this to be a > > fallback, but it must have a compatible for a specific implementation. > > You might have to indulge me a bit as I don't > understand what you're asking. > > Currently there are very few implementations. One of them lives in an > FPGA and the example below is actually the DT entry I use for it. I'm > guessing specific vendor implementations will have their own tweaks > eventually, such as clock sources and whatnot. Yes, exactly. And bugs too. > But that is outside of > the spec (actually the spec defines a register area for eventual vendor > specific usage). But I have no visibility into that and of course the > code has no provision for that yet either. > > So I imagine there will be something like this in dts files eventually: > > compatibvle = "intel,foobar_soc_i3c_hci", "mipi-i3c-hci"; > > Is that what you mean? Yes. Even your FPGA is tied to some implementation... > > Also, which version of the spec does this compatible correspond to? > > All of them. > > > Or are there not HCI differences in the spec versions you mention in > > the cover letter? > > The hardware is self advertising per the spec. So there is no need to > carry such distinction in the DT compatible. Even vendor extensions are > tagged with MIPI vendor IDs in the hardware directly. Oh good, folks are learning. :) Is the vendor ID (and revision) discoverable even if no vendor extensions? If so, then I'm more comfortable with "mipi-i3c-hci" on it's own. The exception will be if there's setup needed to discover the h/w which seems likely. In that case, we should probably do compatible strings based on VID/PID like PCI, USB, etc. No need to define that now I guess, but please add some sort of summary of the above about the discoverability of the HCI implementer and features. Rob
On Tue, 25 Aug 2020, Rob Herring wrote: > On Tue, Aug 25, 2020 at 4:02 PM Nicolas Pitre <nico@fluxnic.net> wrote: > > > Currently there are very few implementations. One of them lives in an > > FPGA and the example below is actually the DT entry I use for it. I'm > > guessing specific vendor implementations will have their own tweaks > > eventually, such as clock sources and whatnot. > > Yes, exactly. And bugs too. Obviously. ;-) > > But that is outside of > > the spec (actually the spec defines a register area for eventual vendor > > specific usage). But I have no visibility into that and of course the > > code has no provision for that yet either. > > > > So I imagine there will be something like this in dts files eventually: > > > > compatibvle = "intel,foobar_soc_i3c_hci", "mipi-i3c-hci"; > > > > Is that what you mean? > > Yes. Even your FPGA is tied to some implementation... It is, but so far it's self-discoverable. > > > Also, which version of the spec does this compatible correspond to? > > > > All of them. > > > > > Or are there not HCI differences in the spec versions you mention in > > > the cover letter? > > > > The hardware is self advertising per the spec. So there is no need to > > carry such distinction in the DT compatible. Even vendor extensions are > > tagged with MIPI vendor IDs in the hardware directly. > > Oh good, folks are learning. :) > > Is the vendor ID (and revision) discoverable even if no vendor > extensions? Yes. It's even in the very first 32-bit word from the register space. Here's the relevant code: #define HCI_VERSION 0x00 /* HCI Version (in BCD) */ [...] /* Validate HCI hardware version */ regval = reg_read(HCI_VERSION); hci->version_major = (regval >> 8) & 0xf; hci->version_minor = (regval >> 4) & 0xf; hci->revision = regval & 0xf; NOTE("HCI v%u.%u r%02u", hci->version_major, hci->version_minor, hci->revision); /* known versions */ switch (regval & ~0xf) { case 0x100: /* version 1.0 */ case 0x110: /* version 1.1 */ case 0x200: /* version 2.0 */ break; default: ERR("unsupported HCI version"); return -EPROTONOSUPPORT; } Then there is a register that provides the relative offset to another register area where "extended attributes" are to be found. Those attributes are also spec defined. One of the standard attributes contains the MIPI vendor ID, the vendor version ID and vendor product ID. And then there is a range of attributes which are vendor defined. That's where specific stuff like fancy clock controls would be located and vendor specific tweaks to be applied. But so far everything can be predicated on hardware-provided data. > If so, then I'm more comfortable with "mipi-i3c-hci" on it's own. The > exception will be if there's setup needed to discover the h/w which > seems likely. In that case, we should probably do compatible strings > based on VID/PID like PCI, USB, etc. No need to define that now I > guess, but please add some sort of summary of the above about the > discoverability of the HCI implementer and features. OK. Nicolas
diff --git a/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml new file mode 100644 index 0000000000..8fc18ea922 --- /dev/null +++ b/Documentation/devicetree/bindings/i3c/mipi-i3c-hci.yaml @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/i3c/mipi-i3c-hci.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: MIPI I3C HCI Device Tree Bindings + +maintainers: + - Nicolas Pitre <npitre@baylibre.com> + +description: | + MIPI I3C Host Controller Interface + + The MIPI I3C HCI (Host Controller Interface) specification defines + a common software driver interface to support compliant MIPI I3C + host controller hardware implementations from multiple vendors. + + For details, please see: + https://www.mipi.org/specifications/i3c-hci + +properties: + compatible: + const: mipi-i3c-hci + reg: + maxItems: 1 + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + +examples: + - | + mipi_i3c_hci@a0000000 { + compatible = "mipi-i3c-hci"; + reg = <0xa0000000 0x2000>; + interrupts = <89>; + };