| Message ID | 20160719191442.15439-2-swarren@wwwdotorg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > The BPMP implements some services which must be represented by separate > nodes. For example, it can provide access to certain I2C controllers, and > the I2C bindings represent each I2C controller as a device tree node. > Update the binding to describe how the BPMP supports this. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > index 9a3864f56955..142d363406f6 100644 > --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > @@ -38,6 +38,24 @@ implemented by this node: > - .../reset/reset.txt > - <dt-bindings/reset/tegra186-reset.h> > > +The BPMP implements some services which must be represented by separate nodes. > +For example, it can provide access to certain I2C controllers, and the I2C > +bindings represent each I2C controller as a device tree node. Such nodes should > +be nested directly inside the main BPMP node. > + > +Software can determine whether a child node of the BPMP node represents a device > +by checking for a compatible property. Any node with a compatible property > +represents a device that can be instantiated. Nodes without a compatible > +property may be used to provide configuration information regarding the BPMP > +itself, although no such configuration nodes are currently defined by this > +binding. > + > +The BPMP firmware defines no single global name-/numbering-space for such > +services. Put another way, the numbering scheme for I2C buses is distinct from > +the numbering scheme for any other service the BPMP may provide (e.g. a future > +hypothetical SPI bus service). As such, child device nodes will have no reg > +property, and the BPMP node will have no #address-cells or #size-cells property. > + > The shared memory bindings for BPMP > ----------------------------------- > > @@ -78,4 +96,9 @@ bpmp { > #clock-cells = <1>; > #power-domain-cells = <1>; > #reset-cells = <1>; > + > + bpmp-i2c { Just 'i2c' here. With that: Acked-by: Rob Herring <robh@kernel.org> > + compatible = "..."; > + ... > + }; > }; > -- > 2.9.2 >
On 07/20/2016 07:16 AM, Rob Herring wrote: > On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> The BPMP implements some services which must be represented by separate >> nodes. For example, it can provide access to certain I2C controllers, and >> the I2C bindings represent each I2C controller as a device tree node. >> Update the binding to describe how the BPMP supports this. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> index 9a3864f56955..142d363406f6 100644 >> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> @@ -38,6 +38,24 @@ implemented by this node: >> - .../reset/reset.txt >> - <dt-bindings/reset/tegra186-reset.h> >> >> +The BPMP implements some services which must be represented by separate nodes. >> +For example, it can provide access to certain I2C controllers, and the I2C >> +bindings represent each I2C controller as a device tree node. Such nodes should >> +be nested directly inside the main BPMP node. >> + >> +Software can determine whether a child node of the BPMP node represents a device >> +by checking for a compatible property. Any node with a compatible property >> +represents a device that can be instantiated. Nodes without a compatible >> +property may be used to provide configuration information regarding the BPMP >> +itself, although no such configuration nodes are currently defined by this >> +binding. >> + >> +The BPMP firmware defines no single global name-/numbering-space for such >> +services. Put another way, the numbering scheme for I2C buses is distinct from >> +the numbering scheme for any other service the BPMP may provide (e.g. a future >> +hypothetical SPI bus service). As such, child device nodes will have no reg >> +property, and the BPMP node will have no #address-cells or #size-cells property. >> + >> The shared memory bindings for BPMP >> ----------------------------------- >> >> @@ -78,4 +96,9 @@ bpmp { >> #clock-cells = <1>; >> #power-domain-cells = <1>; >> #reset-cells = <1>; >> + >> + bpmp-i2c { > > Just 'i2c' here. With that: > > Acked-by: Rob Herring <robh@kernel.org> What if we have multiple BPMP I2C buses? The node names need to be unique, and there's no concept of register number so we can't use a unit address to do that. I guess we could go for plain "i2c" for now and defer the more general naming discussion until some later point if/when we actually instantiate multiple buses, but I would like to confirm we can solve the problem if/when that time comes. (As an aside, "pwr-i2c" might be a better name for the current bus, when the time comes, since any/all I2C nodes inside the BPMP would be BPMP I2C buses).
On 19/07/16 20:14, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > The BPMP implements some services which must be represented by separate > nodes. For example, it can provide access to certain I2C controllers, and > the I2C bindings represent each I2C controller as a device tree node. > Update the binding to describe how the BPMP supports this. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) Acked-by: Jon Hunter <jonathanh@nvidia.com> Jon
On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > The BPMP implements some services which must be represented by separate > nodes. For example, it can provide access to certain I2C controllers, and > the I2C bindings represent each I2C controller as a device tree node. > Update the binding to describe how the BPMP supports this. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > index 9a3864f56955..142d363406f6 100644 > --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > @@ -38,6 +38,24 @@ implemented by this node: > - .../reset/reset.txt > - <dt-bindings/reset/tegra186-reset.h> > > +The BPMP implements some services which must be represented by separate nodes. > +For example, it can provide access to certain I2C controllers, and the I2C > +bindings represent each I2C controller as a device tree node. Such nodes should > +be nested directly inside the main BPMP node. > + > +Software can determine whether a child node of the BPMP node represents a device > +by checking for a compatible property. Any node with a compatible property > +represents a device that can be instantiated. Nodes without a compatible > +property may be used to provide configuration information regarding the BPMP > +itself, although no such configuration nodes are currently defined by this > +binding. > + > +The BPMP firmware defines no single global name-/numbering-space for such > +services. Put another way, the numbering scheme for I2C buses is distinct from > +the numbering scheme for any other service the BPMP may provide (e.g. a future > +hypothetical SPI bus service). As such, child device nodes will have no reg > +property, and the BPMP node will have no #address-cells or #size-cells property. My understanding is that the I2C bus number is passed as part of the request to the BPMP firmware. Does that not count as addressing? Could we not represent that generically using a device tree hierarchy? I'm thinking something along these lines: bpmp { ... services { i2c { i2c@0 { reg = <0>; ... }; i2c@1 { reg = <1>; ... }; ... }; spi { ... }; ... }; ... }; Note that we could probably do without the "services" node, but this is explicit in categorizing these as services exposed by the BPMP. We might need the extra node to work around the issue that some child nodes of the BPMP node don't have a reg property (configuration nodes), whereas any of the services would have those. Although you could make the top- level "i2c" and "spi" nodes take #address-cells and #size-cells properties. Thierry
On 07/26/2016 04:03 AM, Thierry Reding wrote: > On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> The BPMP implements some services which must be represented by separate >> nodes. For example, it can provide access to certain I2C controllers, and >> the I2C bindings represent each I2C controller as a device tree node. >> Update the binding to describe how the BPMP supports this. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> index 9a3864f56955..142d363406f6 100644 >> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >> @@ -38,6 +38,24 @@ implemented by this node: >> - .../reset/reset.txt >> - <dt-bindings/reset/tegra186-reset.h> >> >> +The BPMP implements some services which must be represented by separate nodes. >> +For example, it can provide access to certain I2C controllers, and the I2C >> +bindings represent each I2C controller as a device tree node. Such nodes should >> +be nested directly inside the main BPMP node. >> + >> +Software can determine whether a child node of the BPMP node represents a device >> +by checking for a compatible property. Any node with a compatible property >> +represents a device that can be instantiated. Nodes without a compatible >> +property may be used to provide configuration information regarding the BPMP >> +itself, although no such configuration nodes are currently defined by this >> +binding. >> + >> +The BPMP firmware defines no single global name-/numbering-space for such >> +services. Put another way, the numbering scheme for I2C buses is distinct from >> +the numbering scheme for any other service the BPMP may provide (e.g. a future >> +hypothetical SPI bus service). As such, child device nodes will have no reg >> +property, and the BPMP node will have no #address-cells or #size-cells property. > > My understanding is that the I2C bus number is passed as part of the > request to the BPMP firmware. Does that not count as addressing? Could > we not represent that generically using a device tree hierarchy? I'm > thinking something along these lines: > > bpmp { > ... > > services { > i2c { > i2c@0 { > reg = <0>; Technically, that is possible. However, Rob Herring rejected the idea of multiple levels of sub-nodes. Can we please just go with what we have? DT bindings are frankly an obstacle to getting anything done:-(
On 07/26/2016 10:21 AM, Stephen Warren wrote: > On 07/26/2016 04:03 AM, Thierry Reding wrote: >> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> The BPMP implements some services which must be represented by separate >>> nodes. For example, it can provide access to certain I2C controllers, >>> and >>> the I2C bindings represent each I2C controller as a device tree node. >>> Update the binding to describe how the BPMP supports this. >>> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> --- >>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>> ++++++++++++++++++++++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> index 9a3864f56955..142d363406f6 100644 >>> --- >>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> +++ >>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> @@ -38,6 +38,24 @@ implemented by this node: >>> - .../reset/reset.txt >>> - <dt-bindings/reset/tegra186-reset.h> >>> >>> +The BPMP implements some services which must be represented by >>> separate nodes. >>> +For example, it can provide access to certain I2C controllers, and >>> the I2C >>> +bindings represent each I2C controller as a device tree node. Such >>> nodes should >>> +be nested directly inside the main BPMP node. >>> + >>> +Software can determine whether a child node of the BPMP node >>> represents a device >>> +by checking for a compatible property. Any node with a compatible >>> property >>> +represents a device that can be instantiated. Nodes without a >>> compatible >>> +property may be used to provide configuration information regarding >>> the BPMP >>> +itself, although no such configuration nodes are currently defined >>> by this >>> +binding. >>> + >>> +The BPMP firmware defines no single global name-/numbering-space for >>> such >>> +services. Put another way, the numbering scheme for I2C buses is >>> distinct from >>> +the numbering scheme for any other service the BPMP may provide >>> (e.g. a future >>> +hypothetical SPI bus service). As such, child device nodes will have >>> no reg >>> +property, and the BPMP node will have no #address-cells or >>> #size-cells property. >> >> My understanding is that the I2C bus number is passed as part of the >> request to the BPMP firmware. Does that not count as addressing? Could >> we not represent that generically using a device tree hierarchy? I'm >> thinking something along these lines: >> >> bpmp { >> ... >> >> services { >> i2c { >> i2c@0 { >> reg = <0>; > > Technically, that is possible. However, Rob Herring rejected the idea of > multiple levels of sub-nodes. > > Can we please just go with what we have? DT bindings are frankly an > obstacle to getting anything done:-( Thierry, any further thoughts here? I really need to get these DT bindings finalized so that I can use them in U-Boot, which is very urgent. Thanks.
On Wed, Jul 27, 2016 at 03:50:52PM -0600, Stephen Warren wrote: > On 07/26/2016 10:21 AM, Stephen Warren wrote: > > On 07/26/2016 04:03 AM, Thierry Reding wrote: > > > On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: > > > > From: Stephen Warren <swarren@nvidia.com> > > > > > > > > The BPMP implements some services which must be represented by separate > > > > nodes. For example, it can provide access to certain I2C controllers, > > > > and > > > > the I2C bindings represent each I2C controller as a device tree node. > > > > Update the binding to describe how the BPMP supports this. > > > > > > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > > > --- > > > > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 > > > > ++++++++++++++++++++++ > > > > 1 file changed, 23 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > index 9a3864f56955..142d363406f6 100644 > > > > --- > > > > a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > +++ > > > > b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > @@ -38,6 +38,24 @@ implemented by this node: > > > > - .../reset/reset.txt > > > > - <dt-bindings/reset/tegra186-reset.h> > > > > > > > > +The BPMP implements some services which must be represented by > > > > separate nodes. > > > > +For example, it can provide access to certain I2C controllers, and > > > > the I2C > > > > +bindings represent each I2C controller as a device tree node. Such > > > > nodes should > > > > +be nested directly inside the main BPMP node. > > > > + > > > > +Software can determine whether a child node of the BPMP node > > > > represents a device > > > > +by checking for a compatible property. Any node with a compatible > > > > property > > > > +represents a device that can be instantiated. Nodes without a > > > > compatible > > > > +property may be used to provide configuration information regarding > > > > the BPMP > > > > +itself, although no such configuration nodes are currently defined > > > > by this > > > > +binding. > > > > + > > > > +The BPMP firmware defines no single global name-/numbering-space for > > > > such > > > > +services. Put another way, the numbering scheme for I2C buses is > > > > distinct from > > > > +the numbering scheme for any other service the BPMP may provide > > > > (e.g. a future > > > > +hypothetical SPI bus service). As such, child device nodes will have > > > > no reg > > > > +property, and the BPMP node will have no #address-cells or > > > > #size-cells property. > > > > > > My understanding is that the I2C bus number is passed as part of the > > > request to the BPMP firmware. Does that not count as addressing? Could > > > we not represent that generically using a device tree hierarchy? I'm > > > thinking something along these lines: > > > > > > bpmp { > > > ... > > > > > > services { > > > i2c { > > > i2c@0 { > > > reg = <0>; > > > > Technically, that is possible. However, Rob Herring rejected the idea of > > multiple levels of sub-nodes. > > > > Can we please just go with what we have? DT bindings are frankly an > > obstacle to getting anything done:-( > > Thierry, any further thoughts here? I really need to get these DT bindings > finalized so that I can use them in U-Boot, which is very urgent. Thanks. If everyone else is fine with these bindings, let's go ahead with them. Thierry
On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 07/26/2016 04:03 AM, Thierry Reding wrote: >> >> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>> >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> The BPMP implements some services which must be represented by separate >>> nodes. For example, it can provide access to certain I2C controllers, and >>> the I2C bindings represent each I2C controller as a device tree node. >>> Update the binding to describe how the BPMP supports this. >>> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> --- >>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>> ++++++++++++++++++++++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> index 9a3864f56955..142d363406f6 100644 >>> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>> @@ -38,6 +38,24 @@ implemented by this node: >>> - .../reset/reset.txt >>> - <dt-bindings/reset/tegra186-reset.h> >>> >>> +The BPMP implements some services which must be represented by separate >>> nodes. >>> +For example, it can provide access to certain I2C controllers, and the >>> I2C >>> +bindings represent each I2C controller as a device tree node. Such nodes >>> should >>> +be nested directly inside the main BPMP node. >>> + >>> +Software can determine whether a child node of the BPMP node represents >>> a device >>> +by checking for a compatible property. Any node with a compatible >>> property >>> +represents a device that can be instantiated. Nodes without a compatible >>> +property may be used to provide configuration information regarding the >>> BPMP >>> +itself, although no such configuration nodes are currently defined by >>> this >>> +binding. >>> + >>> +The BPMP firmware defines no single global name-/numbering-space for >>> such >>> +services. Put another way, the numbering scheme for I2C buses is >>> distinct from >>> +the numbering scheme for any other service the BPMP may provide (e.g. a >>> future >>> +hypothetical SPI bus service). As such, child device nodes will have no >>> reg >>> +property, and the BPMP node will have no #address-cells or #size-cells >>> property. >> >> >> My understanding is that the I2C bus number is passed as part of the >> request to the BPMP firmware. Does that not count as addressing? Could >> we not represent that generically using a device tree hierarchy? I'm >> thinking something along these lines: >> >> bpmp { >> ... >> >> services { >> i2c { >> i2c@0 { >> reg = <0>; > > > Technically, that is possible. However, Rob Herring rejected the idea of > multiple levels of sub-nodes. I think I questioned the need, not rejected. What about the above, but remove serivces level: bpmp { ... i2c { i2c@0 { reg = <0>; Rob
On 07/28/2016 01:07 PM, Rob Herring wrote: > On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 07/26/2016 04:03 AM, Thierry Reding wrote: >>> >>> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>>> >>>> From: Stephen Warren <swarren@nvidia.com> >>>> >>>> The BPMP implements some services which must be represented by separate >>>> nodes. For example, it can provide access to certain I2C controllers, and >>>> the I2C bindings represent each I2C controller as a device tree node. >>>> Update the binding to describe how the BPMP supports this. >>>> >>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>>> --- >>>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>>> ++++++++++++++++++++++ >>>> 1 file changed, 23 insertions(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>> index 9a3864f56955..142d363406f6 100644 >>>> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>> @@ -38,6 +38,24 @@ implemented by this node: >>>> - .../reset/reset.txt >>>> - <dt-bindings/reset/tegra186-reset.h> >>>> >>>> +The BPMP implements some services which must be represented by separate >>>> nodes. >>>> +For example, it can provide access to certain I2C controllers, and the >>>> I2C >>>> +bindings represent each I2C controller as a device tree node. Such nodes >>>> should >>>> +be nested directly inside the main BPMP node. >>>> + >>>> +Software can determine whether a child node of the BPMP node represents >>>> a device >>>> +by checking for a compatible property. Any node with a compatible >>>> property >>>> +represents a device that can be instantiated. Nodes without a compatible >>>> +property may be used to provide configuration information regarding the >>>> BPMP >>>> +itself, although no such configuration nodes are currently defined by >>>> this >>>> +binding. >>>> + >>>> +The BPMP firmware defines no single global name-/numbering-space for >>>> such >>>> +services. Put another way, the numbering scheme for I2C buses is >>>> distinct from >>>> +the numbering scheme for any other service the BPMP may provide (e.g. a >>>> future >>>> +hypothetical SPI bus service). As such, child device nodes will have no >>>> reg >>>> +property, and the BPMP node will have no #address-cells or #size-cells >>>> property. >>> >>> >>> My understanding is that the I2C bus number is passed as part of the >>> request to the BPMP firmware. Does that not count as addressing? Could >>> we not represent that generically using a device tree hierarchy? I'm >>> thinking something along these lines: >>> >>> bpmp { >>> ... >>> >>> services { >>> i2c { >>> i2c@0 { >>> reg = <0>; >> >> >> Technically, that is possible. However, Rob Herring rejected the idea of >> multiple levels of sub-nodes. > > I think I questioned the need, not rejected. What about the above, but > remove serivces level: > > bpmp { > ... > > i2c { > i2c@0 { > reg = <0>; Sigh. Can you please talk to Thierry and work out what the binding would be (perhaps on IRC to expedite things?) and I'll just implement whatever you two agree upon. I don't really care much what the binding looks like any more; I just need something that will pass review. Thanks.
On Thu, Jul 28, 2016 at 03:24:22PM -0600, Stephen Warren wrote: > On 07/28/2016 01:07 PM, Rob Herring wrote: > > On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > > > On 07/26/2016 04:03 AM, Thierry Reding wrote: > > > > > > > > On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: > > > > > > > > > > From: Stephen Warren <swarren@nvidia.com> > > > > > > > > > > The BPMP implements some services which must be represented by separate > > > > > nodes. For example, it can provide access to certain I2C controllers, and > > > > > the I2C bindings represent each I2C controller as a device tree node. > > > > > Update the binding to describe how the BPMP supports this. > > > > > > > > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > > > > --- > > > > > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 > > > > > ++++++++++++++++++++++ > > > > > 1 file changed, 23 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > > b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > > index 9a3864f56955..142d363406f6 100644 > > > > > --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > > +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt > > > > > @@ -38,6 +38,24 @@ implemented by this node: > > > > > - .../reset/reset.txt > > > > > - <dt-bindings/reset/tegra186-reset.h> > > > > > > > > > > +The BPMP implements some services which must be represented by separate > > > > > nodes. > > > > > +For example, it can provide access to certain I2C controllers, and the > > > > > I2C > > > > > +bindings represent each I2C controller as a device tree node. Such nodes > > > > > should > > > > > +be nested directly inside the main BPMP node. > > > > > + > > > > > +Software can determine whether a child node of the BPMP node represents > > > > > a device > > > > > +by checking for a compatible property. Any node with a compatible > > > > > property > > > > > +represents a device that can be instantiated. Nodes without a compatible > > > > > +property may be used to provide configuration information regarding the > > > > > BPMP > > > > > +itself, although no such configuration nodes are currently defined by > > > > > this > > > > > +binding. > > > > > + > > > > > +The BPMP firmware defines no single global name-/numbering-space for > > > > > such > > > > > +services. Put another way, the numbering scheme for I2C buses is > > > > > distinct from > > > > > +the numbering scheme for any other service the BPMP may provide (e.g. a > > > > > future > > > > > +hypothetical SPI bus service). As such, child device nodes will have no > > > > > reg > > > > > +property, and the BPMP node will have no #address-cells or #size-cells > > > > > property. > > > > > > > > > > > > My understanding is that the I2C bus number is passed as part of the > > > > request to the BPMP firmware. Does that not count as addressing? Could > > > > we not represent that generically using a device tree hierarchy? I'm > > > > thinking something along these lines: > > > > > > > > bpmp { > > > > ... > > > > > > > > services { > > > > i2c { > > > > i2c@0 { > > > > reg = <0>; > > > > > > > > > Technically, that is possible. However, Rob Herring rejected the idea of > > > multiple levels of sub-nodes. > > > > I think I questioned the need, not rejected. What about the above, but > > remove serivces level: > > > > bpmp { > > ... > > > > i2c { > > i2c@0 { > > reg = <0>; > > Sigh. Can you please talk to Thierry and work out what the binding would be > (perhaps on IRC to expedite things?) and I'll just implement whatever you > two agree upon. I don't really care much what the binding looks like any > more; I just need something that will pass review. Thanks. Like I said, I'm fine going with what you proposed. I'm sure by now the BPMP has long been frozen and if the proposed binding works fine that's good. No need to over-engineer. If we ever need more than one I2C (or expose other busses) I'm sure we can find a way to switch to something like the above later on. Like I said, that's very unlikely to happen on Tegra186, so we'll have a new compatible string (and hence an easy way to differentiate between these bindings) anyway. Thierry
On 07/29/2016 07:48 AM, Thierry Reding wrote: > On Thu, Jul 28, 2016 at 03:24:22PM -0600, Stephen Warren wrote: >> On 07/28/2016 01:07 PM, Rob Herring wrote: >>> On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> On 07/26/2016 04:03 AM, Thierry Reding wrote: >>>>> >>>>> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>>>>> >>>>>> From: Stephen Warren <swarren@nvidia.com> >>>>>> >>>>>> The BPMP implements some services which must be represented by separate >>>>>> nodes. For example, it can provide access to certain I2C controllers, and >>>>>> the I2C bindings represent each I2C controller as a device tree node. >>>>>> Update the binding to describe how the BPMP supports this. >>>>>> >>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>>>>> --- >>>>>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>>>>> ++++++++++++++++++++++ >>>>>> 1 file changed, 23 insertions(+) >>>>>> >>>>>> diff --git >>>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>> index 9a3864f56955..142d363406f6 100644 >>>>>> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>> @@ -38,6 +38,24 @@ implemented by this node: >>>>>> - .../reset/reset.txt >>>>>> - <dt-bindings/reset/tegra186-reset.h> >>>>>> >>>>>> +The BPMP implements some services which must be represented by separate >>>>>> nodes. >>>>>> +For example, it can provide access to certain I2C controllers, and the >>>>>> I2C >>>>>> +bindings represent each I2C controller as a device tree node. Such nodes >>>>>> should >>>>>> +be nested directly inside the main BPMP node. >>>>>> + >>>>>> +Software can determine whether a child node of the BPMP node represents >>>>>> a device >>>>>> +by checking for a compatible property. Any node with a compatible >>>>>> property >>>>>> +represents a device that can be instantiated. Nodes without a compatible >>>>>> +property may be used to provide configuration information regarding the >>>>>> BPMP >>>>>> +itself, although no such configuration nodes are currently defined by >>>>>> this >>>>>> +binding. >>>>>> + >>>>>> +The BPMP firmware defines no single global name-/numbering-space for >>>>>> such >>>>>> +services. Put another way, the numbering scheme for I2C buses is >>>>>> distinct from >>>>>> +the numbering scheme for any other service the BPMP may provide (e.g. a >>>>>> future >>>>>> +hypothetical SPI bus service). As such, child device nodes will have no >>>>>> reg >>>>>> +property, and the BPMP node will have no #address-cells or #size-cells >>>>>> property. >>>>> >>>>> >>>>> My understanding is that the I2C bus number is passed as part of the >>>>> request to the BPMP firmware. Does that not count as addressing? Could >>>>> we not represent that generically using a device tree hierarchy? I'm >>>>> thinking something along these lines: >>>>> >>>>> bpmp { >>>>> ... >>>>> >>>>> services { >>>>> i2c { >>>>> i2c@0 { >>>>> reg = <0>; >>>> >>>> >>>> Technically, that is possible. However, Rob Herring rejected the idea of >>>> multiple levels of sub-nodes. >>> >>> I think I questioned the need, not rejected. What about the above, but >>> remove serivces level: >>> >>> bpmp { >>> ... >>> >>> i2c { >>> i2c@0 { >>> reg = <0>; >> >> Sigh. Can you please talk to Thierry and work out what the binding would be >> (perhaps on IRC to expedite things?) and I'll just implement whatever you >> two agree upon. I don't really care much what the binding looks like any >> more; I just need something that will pass review. Thanks. > > Like I said, I'm fine going with what you proposed. OK, great. For the record, I'm going to send U-Boot patches today that apply this binding patch (the original one I sent, unmodified) to the U-Boot tree, and implement it in the U-Boot driver.
On Fri, Jul 29, 2016 at 11:06 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 07/29/2016 07:48 AM, Thierry Reding wrote: >> >> On Thu, Jul 28, 2016 at 03:24:22PM -0600, Stephen Warren wrote: >>> >>> On 07/28/2016 01:07 PM, Rob Herring wrote: >>>> >>>> On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> >>>> wrote: >>>>> >>>>> On 07/26/2016 04:03 AM, Thierry Reding wrote: >>>>>> >>>>>> >>>>>> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>>>>>> >>>>>>> >>>>>>> From: Stephen Warren <swarren@nvidia.com> >>>>>>> >>>>>>> The BPMP implements some services which must be represented by >>>>>>> separate >>>>>>> nodes. For example, it can provide access to certain I2C controllers, >>>>>>> and >>>>>>> the I2C bindings represent each I2C controller as a device tree node. >>>>>>> Update the binding to describe how the BPMP supports this. >>>>>>> >>>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>>>>>> --- >>>>>>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>>>>>> ++++++++++++++++++++++ >>>>>>> 1 file changed, 23 insertions(+) >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>> index 9a3864f56955..142d363406f6 100644 >>>>>>> --- >>>>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>> +++ >>>>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>> @@ -38,6 +38,24 @@ implemented by this node: >>>>>>> - .../reset/reset.txt >>>>>>> - <dt-bindings/reset/tegra186-reset.h> >>>>>>> >>>>>>> +The BPMP implements some services which must be represented by >>>>>>> separate >>>>>>> nodes. >>>>>>> +For example, it can provide access to certain I2C controllers, and >>>>>>> the >>>>>>> I2C >>>>>>> +bindings represent each I2C controller as a device tree node. Such >>>>>>> nodes >>>>>>> should >>>>>>> +be nested directly inside the main BPMP node. >>>>>>> + >>>>>>> +Software can determine whether a child node of the BPMP node >>>>>>> represents >>>>>>> a device >>>>>>> +by checking for a compatible property. Any node with a compatible >>>>>>> property >>>>>>> +represents a device that can be instantiated. Nodes without a >>>>>>> compatible >>>>>>> +property may be used to provide configuration information regarding >>>>>>> the >>>>>>> BPMP >>>>>>> +itself, although no such configuration nodes are currently defined >>>>>>> by >>>>>>> this >>>>>>> +binding. >>>>>>> + >>>>>>> +The BPMP firmware defines no single global name-/numbering-space for >>>>>>> such >>>>>>> +services. Put another way, the numbering scheme for I2C buses is >>>>>>> distinct from >>>>>>> +the numbering scheme for any other service the BPMP may provide >>>>>>> (e.g. a >>>>>>> future >>>>>>> +hypothetical SPI bus service). As such, child device nodes will have >>>>>>> no >>>>>>> reg >>>>>>> +property, and the BPMP node will have no #address-cells or >>>>>>> #size-cells >>>>>>> property. >>>>>> >>>>>> >>>>>> >>>>>> My understanding is that the I2C bus number is passed as part of the >>>>>> request to the BPMP firmware. Does that not count as addressing? Could >>>>>> we not represent that generically using a device tree hierarchy? I'm >>>>>> thinking something along these lines: >>>>>> >>>>>> bpmp { >>>>>> ... >>>>>> >>>>>> services { >>>>>> i2c { >>>>>> i2c@0 { >>>>>> reg = <0>; >>>>> >>>>> >>>>> >>>>> Technically, that is possible. However, Rob Herring rejected the idea >>>>> of >>>>> multiple levels of sub-nodes. >>>> >>>> >>>> I think I questioned the need, not rejected. What about the above, but >>>> remove serivces level: >>>> >>>> bpmp { >>>> ... >>>> >>>> i2c { >>>> i2c@0 { >>>> reg = <0>; >>> >>> >>> Sigh. Can you please talk to Thierry and work out what the binding would >>> be >>> (perhaps on IRC to expedite things?) and I'll just implement whatever you >>> two agree upon. I don't really care much what the binding looks like any >>> more; I just need something that will pass review. Thanks. >> >> >> Like I said, I'm fine going with what you proposed. > > > OK, great. For the record, I'm going to send U-Boot patches today that apply > this binding patch (the original one I sent, unmodified) to the U-Boot tree, > and implement it in the U-Boot driver. That's fine, I did ack that. I really don't want to ask, but why are you documenting in u-boot? Is it some how different for u-boot? Rob
On 07/29/2016 02:28 PM, Rob Herring wrote: > On Fri, Jul 29, 2016 at 11:06 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 07/29/2016 07:48 AM, Thierry Reding wrote: >>> >>> On Thu, Jul 28, 2016 at 03:24:22PM -0600, Stephen Warren wrote: >>>> >>>> On 07/28/2016 01:07 PM, Rob Herring wrote: >>>>> >>>>> On Tue, Jul 26, 2016 at 11:21 AM, Stephen Warren <swarren@wwwdotorg.org> >>>>> wrote: >>>>>> >>>>>> On 07/26/2016 04:03 AM, Thierry Reding wrote: >>>>>>> >>>>>>> >>>>>>> On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: >>>>>>>> >>>>>>>> >>>>>>>> From: Stephen Warren <swarren@nvidia.com> >>>>>>>> >>>>>>>> The BPMP implements some services which must be represented by >>>>>>>> separate >>>>>>>> nodes. For example, it can provide access to certain I2C controllers, >>>>>>>> and >>>>>>>> the I2C bindings represent each I2C controller as a device tree node. >>>>>>>> Update the binding to describe how the BPMP supports this. >>>>>>>> >>>>>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>>>>>>> --- >>>>>>>> .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 >>>>>>>> ++++++++++++++++++++++ >>>>>>>> 1 file changed, 23 insertions(+) >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>>> index 9a3864f56955..142d363406f6 100644 >>>>>>>> --- >>>>>>>> a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>>> +++ >>>>>>>> b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt >>>>>>>> @@ -38,6 +38,24 @@ implemented by this node: >>>>>>>> - .../reset/reset.txt >>>>>>>> - <dt-bindings/reset/tegra186-reset.h> >>>>>>>> >>>>>>>> +The BPMP implements some services which must be represented by >>>>>>>> separate >>>>>>>> nodes. >>>>>>>> +For example, it can provide access to certain I2C controllers, and >>>>>>>> the >>>>>>>> I2C >>>>>>>> +bindings represent each I2C controller as a device tree node. Such >>>>>>>> nodes >>>>>>>> should >>>>>>>> +be nested directly inside the main BPMP node. >>>>>>>> + >>>>>>>> +Software can determine whether a child node of the BPMP node >>>>>>>> represents >>>>>>>> a device >>>>>>>> +by checking for a compatible property. Any node with a compatible >>>>>>>> property >>>>>>>> +represents a device that can be instantiated. Nodes without a >>>>>>>> compatible >>>>>>>> +property may be used to provide configuration information regarding >>>>>>>> the >>>>>>>> BPMP >>>>>>>> +itself, although no such configuration nodes are currently defined >>>>>>>> by >>>>>>>> this >>>>>>>> +binding. >>>>>>>> + >>>>>>>> +The BPMP firmware defines no single global name-/numbering-space for >>>>>>>> such >>>>>>>> +services. Put another way, the numbering scheme for I2C buses is >>>>>>>> distinct from >>>>>>>> +the numbering scheme for any other service the BPMP may provide >>>>>>>> (e.g. a >>>>>>>> future >>>>>>>> +hypothetical SPI bus service). As such, child device nodes will have >>>>>>>> no >>>>>>>> reg >>>>>>>> +property, and the BPMP node will have no #address-cells or >>>>>>>> #size-cells >>>>>>>> property. >>>>>>> >>>>>>> >>>>>>> >>>>>>> My understanding is that the I2C bus number is passed as part of the >>>>>>> request to the BPMP firmware. Does that not count as addressing? Could >>>>>>> we not represent that generically using a device tree hierarchy? I'm >>>>>>> thinking something along these lines: >>>>>>> >>>>>>> bpmp { >>>>>>> ... >>>>>>> >>>>>>> services { >>>>>>> i2c { >>>>>>> i2c@0 { >>>>>>> reg = <0>; >>>>>> >>>>>> >>>>>> >>>>>> Technically, that is possible. However, Rob Herring rejected the idea >>>>>> of >>>>>> multiple levels of sub-nodes. >>>>> >>>>> >>>>> I think I questioned the need, not rejected. What about the above, but >>>>> remove serivces level: >>>>> >>>>> bpmp { >>>>> ... >>>>> >>>>> i2c { >>>>> i2c@0 { >>>>> reg = <0>; >>>> >>>> >>>> Sigh. Can you please talk to Thierry and work out what the binding would >>>> be >>>> (perhaps on IRC to expedite things?) and I'll just implement whatever you >>>> two agree upon. I don't really care much what the binding looks like any >>>> more; I just need something that will pass review. Thanks. >>> >>> >>> Like I said, I'm fine going with what you proposed. >> >> >> OK, great. For the record, I'm going to send U-Boot patches today that apply >> this binding patch (the original one I sent, unmodified) to the U-Boot tree, >> and implement it in the U-Boot driver. > > That's fine, I did ack that. > > I really don't want to ask, but why are you documenting in u-boot? Is > it some how different for u-boot? There's a copy of the DT binding documents in U-Boot as well. At least for the bindings I deal with, they should be the same, although the U-Boot copies might get stale as hopefully-backwards-compatible enhancements are made to the kernel versions. I'm implementing a U-Boot driver for this new feature, so am copying these new binding docs into the U-Boot DT binding docs directory too. I'd be happy personally not to have another copy of the bindings, and just tell people to refer to the docs in the kernel, or any other canonical source, but that's not my call.
On Tue, Jul 19, 2016 at 01:14:41PM -0600, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > The BPMP implements some services which must be represented by separate > nodes. For example, it can provide access to certain I2C controllers, and > the I2C bindings represent each I2C controller as a device tree node. > Update the binding to describe how the BPMP supports this. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > .../bindings/firmware/nvidia,tegra186-bpmp.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) Applied, with bpmp-i2c changed to i2c as per Rob's comment. Thanks, Thierry
diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt index 9a3864f56955..142d363406f6 100644 --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.txt @@ -38,6 +38,24 @@ implemented by this node: - .../reset/reset.txt - <dt-bindings/reset/tegra186-reset.h> +The BPMP implements some services which must be represented by separate nodes. +For example, it can provide access to certain I2C controllers, and the I2C +bindings represent each I2C controller as a device tree node. Such nodes should +be nested directly inside the main BPMP node. + +Software can determine whether a child node of the BPMP node represents a device +by checking for a compatible property. Any node with a compatible property +represents a device that can be instantiated. Nodes without a compatible +property may be used to provide configuration information regarding the BPMP +itself, although no such configuration nodes are currently defined by this +binding. + +The BPMP firmware defines no single global name-/numbering-space for such +services. Put another way, the numbering scheme for I2C buses is distinct from +the numbering scheme for any other service the BPMP may provide (e.g. a future +hypothetical SPI bus service). As such, child device nodes will have no reg +property, and the BPMP node will have no #address-cells or #size-cells property. + The shared memory bindings for BPMP ----------------------------------- @@ -78,4 +96,9 @@ bpmp { #clock-cells = <1>; #power-domain-cells = <1>; #reset-cells = <1>; + + bpmp-i2c { + compatible = "..."; + ... + }; };