| Message ID | 20170123204851.12808-2-stephen.boyd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Stephen, Sorry I did not get to v1 and v2 in a timely manner. On 01/23/17 12:48, Stephen Boyd wrote: > Platforms like 96boards have a standardized connector/expansion > slot that exposes signals like GPIOs to expansion boards in an > SoC agnostic way. We'd like the DT overlays for the expansion > boards to be written once without knowledge of the SoC on the > other side of the connector. This avoids the unscalable > combinatorial explosion of a different DT overlay for each > expansion board and SoC pair. > > We need a way to describe the GPIOs routed through the connector > in an SoC agnostic way. Let's introduce nexus property parsing > into the OF core to do this. This is largely based on the > interrupt nexus support we already have. This allows us to remap > a phandle list in a consumer node (e.g. reset-gpios) through a > connector in a generic way (e.g. via gpio-map). Do this in a > generic routine so that we can remap any sort of variable length > phandle list. > > Taking GPIOs as an example, the connector would be a GPIO nexus, > supporting the remapping of a GPIO specifier space to multiple > GPIO providers on the SoC. DT would look as shown below, where > 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an > expansion port where boards can be plugged in, and > 'expansion_device' is a device on the expansion board. > > soc { > soc_gpio1: gpio-controller1 { > #gpio-cells = <2>; > }; > > soc_gpio2: gpio-controller2 { > #gpio-cells = <2>; > }; > }; > > connector: connector { > #gpio-cells = <2>; > gpio-map = <0 0 &soc_gpio1 1 0>, > <1 0 &soc_gpio2 4 0>, > <2 0 &soc_gpio1 3 0>, > <3 0 &soc_gpio2 2 0>; > gpio-map-mask = <0xf 0x0>; > gpio-map-pass-thru = <0x0 0x1> > }; > > expansion_device { > reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; > }; The how to architect connectors and plugs threads fell asleep before coming to a resolution. We need to revive that discussion. One of the concepts of the plug and connector architecture is that a main board may contain multiple connectors of the same type (or different types, but the same type is sufficient for this discussion). The node describing the card that plugs into one of the connectors does not know the phandle of the connector it is going to be connected to. Some other mechanism is provided to allow a card to be plugged into any of the available connectors. If there are two identical cards plugged into two connectors, then both cards have the same exact device tree node. But some mechanism will exist to resolve (or "link") the two card nodes to the different connector nodes. As a result of this, in the above example the reset-gpios property in the node 'expansion_device' can not contain '&connector'. The concept of &connector belongs to the entire expansion_device node, not to individual properties within the node. I'm not sure where this puts us. I'm thinking.... -Frank > > The GPIO core would use of_parse_phandle_with_args_map() instead > of of_parse_phandle_with_args() and arrive at the same type of > result, a phandle and argument list. The difference is that the > phandle and arguments will be remapped through the nexus node to > the underlying SoC GPIO controller node. In the example above, > we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW> > to <&soc_gpio1 3 GPIO_ACTIVE_LOW>. > > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Mark Brown <broonie@kernel.org> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org> > --- > drivers/of/base.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/of.h | 12 ++++ > 2 files changed, 196 insertions(+) < snip >
On Tue, Jan 24, 2017 at 12:36 AM, Frank Rowand <frowand.list@gmail.com> wrote: > Hi Stephen, > > Sorry I did not get to v1 and v2 in a timely manner. > > > On 01/23/17 12:48, Stephen Boyd wrote: >> Platforms like 96boards have a standardized connector/expansion >> slot that exposes signals like GPIOs to expansion boards in an >> SoC agnostic way. We'd like the DT overlays for the expansion >> boards to be written once without knowledge of the SoC on the >> other side of the connector. This avoids the unscalable >> combinatorial explosion of a different DT overlay for each >> expansion board and SoC pair. >> >> We need a way to describe the GPIOs routed through the connector >> in an SoC agnostic way. Let's introduce nexus property parsing >> into the OF core to do this. This is largely based on the >> interrupt nexus support we already have. This allows us to remap >> a phandle list in a consumer node (e.g. reset-gpios) through a >> connector in a generic way (e.g. via gpio-map). Do this in a >> generic routine so that we can remap any sort of variable length >> phandle list. >> >> Taking GPIOs as an example, the connector would be a GPIO nexus, >> supporting the remapping of a GPIO specifier space to multiple >> GPIO providers on the SoC. DT would look as shown below, where >> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an >> expansion port where boards can be plugged in, and >> 'expansion_device' is a device on the expansion board. >> >> soc { >> soc_gpio1: gpio-controller1 { >> #gpio-cells = <2>; >> }; >> >> soc_gpio2: gpio-controller2 { >> #gpio-cells = <2>; >> }; >> }; >> >> connector: connector { >> #gpio-cells = <2>; >> gpio-map = <0 0 &soc_gpio1 1 0>, >> <1 0 &soc_gpio2 4 0>, >> <2 0 &soc_gpio1 3 0>, >> <3 0 &soc_gpio2 2 0>; >> gpio-map-mask = <0xf 0x0>; >> gpio-map-pass-thru = <0x0 0x1> >> }; >> >> expansion_device { >> reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; >> }; > > The how to architect connectors and plugs threads fell asleep before > coming to a resolution. We need to revive that discussion. > > One of the concepts of the plug and connector architecture is that > a main board may contain multiple connectors of the same type (or > different types, but the same type is sufficient for this discussion). > > The node describing the card that plugs into one of the connectors > does not know the phandle of the connector it is going to be > connected to. Some other mechanism is provided to allow a card > to be plugged into any of the available connectors. If there are > two identical cards plugged into two connectors, then both cards > have the same exact device tree node. But some mechanism will > exist to resolve (or "link") the two card nodes to the different > connector nodes. > > As a result of this, in the above example the reset-gpios property > in the node 'expansion_device' can not contain '&connector'. The > concept of &connector belongs to the entire expansion_device node, > not to individual properties within the node. I think this is easily solved with a connector having 2 halves and that we need to search parents for *-map properties. Inheriting from parents is a common pattern in DT though perhaps not walking the parents of a phandle. So we'd have something like this: base-connector-1 { gpio-map = ... connector { child { some-gpios = <&connector 1>; }; }; }; base-connector-2 { gpio-map = ... connector { child { some-gpios = <&connector 1>; }; }; }; Now, how we resolve that /connector from an overlay targets /base-connector-1 and /base-connector-2 is an orthogonal issue and one that's going to be connector specific (at least for probe-able connectors). Rob
On Tue, Jan 24, 2017 at 12:05 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Tue, Jan 24, 2017 at 12:36 AM, Frank Rowand <frowand.list@gmail.com> wrote: >> Hi Stephen, >> >> Sorry I did not get to v1 and v2 in a timely manner. >> >> >> On 01/23/17 12:48, Stephen Boyd wrote: >>> Platforms like 96boards have a standardized connector/expansion >>> slot that exposes signals like GPIOs to expansion boards in an >>> SoC agnostic way. We'd like the DT overlays for the expansion >>> boards to be written once without knowledge of the SoC on the >>> other side of the connector. This avoids the unscalable >>> combinatorial explosion of a different DT overlay for each >>> expansion board and SoC pair. >>> >>> We need a way to describe the GPIOs routed through the connector >>> in an SoC agnostic way. Let's introduce nexus property parsing >>> into the OF core to do this. This is largely based on the >>> interrupt nexus support we already have. This allows us to remap >>> a phandle list in a consumer node (e.g. reset-gpios) through a >>> connector in a generic way (e.g. via gpio-map). Do this in a >>> generic routine so that we can remap any sort of variable length >>> phandle list. >>> >>> Taking GPIOs as an example, the connector would be a GPIO nexus, >>> supporting the remapping of a GPIO specifier space to multiple >>> GPIO providers on the SoC. DT would look as shown below, where >>> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an >>> expansion port where boards can be plugged in, and >>> 'expansion_device' is a device on the expansion board. >>> >>> soc { >>> soc_gpio1: gpio-controller1 { >>> #gpio-cells = <2>; >>> }; >>> >>> soc_gpio2: gpio-controller2 { >>> #gpio-cells = <2>; >>> }; >>> }; >>> >>> connector: connector { >>> #gpio-cells = <2>; >>> gpio-map = <0 0 &soc_gpio1 1 0>, >>> <1 0 &soc_gpio2 4 0>, >>> <2 0 &soc_gpio1 3 0>, >>> <3 0 &soc_gpio2 2 0>; >>> gpio-map-mask = <0xf 0x0>; >>> gpio-map-pass-thru = <0x0 0x1> >>> }; >>> >>> expansion_device { >>> reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; >>> }; >> >> The how to architect connectors and plugs threads fell asleep before >> coming to a resolution. We need to revive that discussion. >> >> One of the concepts of the plug and connector architecture is that >> a main board may contain multiple connectors of the same type (or >> different types, but the same type is sufficient for this discussion). >> >> The node describing the card that plugs into one of the connectors >> does not know the phandle of the connector it is going to be >> connected to. Some other mechanism is provided to allow a card >> to be plugged into any of the available connectors. If there are >> two identical cards plugged into two connectors, then both cards >> have the same exact device tree node. But some mechanism will >> exist to resolve (or "link") the two card nodes to the different >> connector nodes. >> >> As a result of this, in the above example the reset-gpios property >> in the node 'expansion_device' can not contain '&connector'. The >> concept of &connector belongs to the entire expansion_device node, >> not to individual properties within the node. > > I think this is easily solved with a connector having 2 halves and > that we need to search parents for *-map properties. Inheriting from > parents is a common pattern in DT though perhaps not walking the > parents of a phandle. So we'd have something like this: > > base-connector-1 { > gpio-map = ... > connector { > child { > some-gpios = <&connector 1>; > }; > }; > }; > > base-connector-2 { > gpio-map = ... > connector { > child { > some-gpios = <&connector 1>; > }; > }; > }; > > Now, how we resolve that /connector from an overlay targets > /base-connector-1 and /base-connector-2 is an orthogonal issue and one > that's going to be connector specific (at least for probe-able > connectors). Frank, any more comments on this? If not, I plan to apply this series. Rob
On Mon, Jan 23, 2017 at 12:48:49PM -0800, Stephen Boyd wrote: > connector: connector { > #gpio-cells = <2>; > gpio-map = <0 0 &soc_gpio1 1 0>, > <1 0 &soc_gpio2 4 0>, > <2 0 &soc_gpio1 3 0>, > <3 0 &soc_gpio2 2 0>; > gpio-map-mask = <0xf 0x0>; > gpio-map-pass-thru = <0x0 0x1> This -map-mask and -map-pass-thru mechanism needs documentation in Documentation/devicetree. There's a bunch of users of -map-mask (eg, interrupt-map-mask) but there's nothing that actually describes how this works. The "-map-pass-thru" thing also seems to be a new, undocumented idea. So, NAK until this stuff goes through the same level of review WRT DT documentation as other bindings are required to. I'm not saying that it's a bad idea, but it's important for consistency that this stuff is well documented, and people don't end up having to interpret the code when writing their DT descriptions.
On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote:
> Frank, any more comments on this? If not, I plan to apply this series.
Well, I find that a little annoying, because DT has the requirement
that new bindings are properly documented in Documentation, and it
appears that this comes with no documentation what so ever, despite
introducing new properties (like the -map-mask-passthru thing).
So I'm NAKing it until there's some documentation of how this
mechanism is supposed to work.
Merely providing an example in a commit log is (IMHO) insufficient.
An example doesn't explain how it was created, or how to create an
implementation.
Remember, we expect people to do exactly that, so we need to give
them this information so that they can make use of it.
I did try to work out from the code how the -map-mask thing worked,
but eventually gave up.
On Thu, Feb 9, 2017 at 9:35 AM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote: >> Frank, any more comments on this? If not, I plan to apply this series. > > Well, I find that a little annoying, because DT has the requirement > that new bindings are properly documented in Documentation, and it > appears that this comes with no documentation what so ever, despite > introducing new properties (like the -map-mask-passthru thing). > > So I'm NAKing it until there's some documentation of how this > mechanism is supposed to work. > > Merely providing an example in a commit log is (IMHO) insufficient. > An example doesn't explain how it was created, or how to create an > implementation. Yes, you are right. However, I'd like to see this documented in the DT spec, rather than kernel Documentation/ as this is a core binding. Though that would also imply first moving the GPIO bindings there. Perhaps just how this works generically could be in the spec, but the GPIO specifics can live with the rest of the GPIO bindings for now. This is intended to extend to other bindings. > Remember, we expect people to do exactly that, so we need to give > them this information so that they can make use of it. > > I did try to work out from the code how the -map-mask thing worked, > but eventually gave up. The code is definitely hard to follow and I've not come up with any ways to make it easier to read. It's largely copied from the interrupt-map version, but different enough that sharing isn't really possible either. For interrupts, it's documented in the DT spec. The best (only?) explanation for how interrupt-map works is here[1] (used to be at devicetree.org). Rob [1] http://elinux.org/Device_Tree_Usage#Advanced_Interrupt_Mapping
On 02/09/17 07:17, Rob Herring wrote: > On Tue, Jan 24, 2017 at 12:05 PM, Rob Herring <robh+dt@kernel.org> wrote: >> On Tue, Jan 24, 2017 at 12:36 AM, Frank Rowand <frowand.list@gmail.com> wrote: >>> Hi Stephen, >>> >>> Sorry I did not get to v1 and v2 in a timely manner. >>> >>> >>> On 01/23/17 12:48, Stephen Boyd wrote: >>>> Platforms like 96boards have a standardized connector/expansion >>>> slot that exposes signals like GPIOs to expansion boards in an >>>> SoC agnostic way. We'd like the DT overlays for the expansion >>>> boards to be written once without knowledge of the SoC on the >>>> other side of the connector. This avoids the unscalable >>>> combinatorial explosion of a different DT overlay for each >>>> expansion board and SoC pair. >>>> >>>> We need a way to describe the GPIOs routed through the connector >>>> in an SoC agnostic way. Let's introduce nexus property parsing >>>> into the OF core to do this. This is largely based on the >>>> interrupt nexus support we already have. This allows us to remap >>>> a phandle list in a consumer node (e.g. reset-gpios) through a >>>> connector in a generic way (e.g. via gpio-map). Do this in a >>>> generic routine so that we can remap any sort of variable length >>>> phandle list. >>>> >>>> Taking GPIOs as an example, the connector would be a GPIO nexus, >>>> supporting the remapping of a GPIO specifier space to multiple >>>> GPIO providers on the SoC. DT would look as shown below, where >>>> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an >>>> expansion port where boards can be plugged in, and >>>> 'expansion_device' is a device on the expansion board. >>>> >>>> soc { >>>> soc_gpio1: gpio-controller1 { >>>> #gpio-cells = <2>; >>>> }; >>>> >>>> soc_gpio2: gpio-controller2 { >>>> #gpio-cells = <2>; >>>> }; >>>> }; >>>> >>>> connector: connector { >>>> #gpio-cells = <2>; >>>> gpio-map = <0 0 &soc_gpio1 1 0>, >>>> <1 0 &soc_gpio2 4 0>, >>>> <2 0 &soc_gpio1 3 0>, >>>> <3 0 &soc_gpio2 2 0>; >>>> gpio-map-mask = <0xf 0x0>; >>>> gpio-map-pass-thru = <0x0 0x1> >>>> }; >>>> >>>> expansion_device { >>>> reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; >>>> }; >>> >>> The how to architect connectors and plugs threads fell asleep before >>> coming to a resolution. We need to revive that discussion. >>> >>> One of the concepts of the plug and connector architecture is that >>> a main board may contain multiple connectors of the same type (or >>> different types, but the same type is sufficient for this discussion). >>> >>> The node describing the card that plugs into one of the connectors >>> does not know the phandle of the connector it is going to be >>> connected to. Some other mechanism is provided to allow a card >>> to be plugged into any of the available connectors. If there are >>> two identical cards plugged into two connectors, then both cards >>> have the same exact device tree node. But some mechanism will >>> exist to resolve (or "link") the two card nodes to the different >>> connector nodes. >>> >>> As a result of this, in the above example the reset-gpios property >>> in the node 'expansion_device' can not contain '&connector'. The >>> concept of &connector belongs to the entire expansion_device node, >>> not to individual properties within the node. >> >> I think this is easily solved with a connector having 2 halves and >> that we need to search parents for *-map properties. Inheriting from >> parents is a common pattern in DT though perhaps not walking the >> parents of a phandle. So we'd have something like this: >> >> base-connector-1 { >> gpio-map = ... >> connector { >> child { >> some-gpios = <&connector 1>; >> }; >> }; >> }; >> >> base-connector-2 { >> gpio-map = ... >> connector { >> child { >> some-gpios = <&connector 1>; >> }; >> }; >> }; >> >> Now, how we resolve that /connector from an overlay targets >> /base-connector-1 and /base-connector-2 is an orthogonal issue and one >> that's going to be connector specific (at least for probe-able >> connectors). > > Frank, any more comments on this? If not, I plan to apply this series. > > Rob Yes, how we resolve which connector a plug goes into is orthogonal. My objection is that the original example has a property in the plug node (that is, on the expansion board), directly referencing the connector node, instead of referencing a resource inside the connector node. In the original example, it would make more sense for the first item in the reset-gpios property to be &gpio-map or "gpio-map" instead of &connector. -Frank
Quoting Rob Herring (2017-02-09 08:00:05) > On Thu, Feb 9, 2017 at 9:35 AM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > On Thu, Feb 09, 2017 at 09:17:58AM -0600, Rob Herring wrote: > >> Frank, any more comments on this? If not, I plan to apply this series. > > > > Well, I find that a little annoying, because DT has the requirement > > that new bindings are properly documented in Documentation, and it > > appears that this comes with no documentation what so ever, despite > > introducing new properties (like the -map-mask-passthru thing). > > > > So I'm NAKing it until there's some documentation of how this > > mechanism is supposed to work. > > > > Merely providing an example in a commit log is (IMHO) insufficient. > > An example doesn't explain how it was created, or how to create an > > implementation. > > Yes, you are right. > > However, I'd like to see this documented in the DT spec, rather than > kernel Documentation/ as this is a core binding. Though that would > also imply first moving the GPIO bindings there. Perhaps just how this > works generically could be in the spec, but the GPIO specifics can > live with the rest of the GPIO bindings for now. This is intended to > extend to other bindings. I will make a patch against the devicetree spec and send it as a reply to this series. > > > Remember, we expect people to do exactly that, so we need to give > > them this information so that they can make use of it. > > > > I did try to work out from the code how the -map-mask thing worked, > > but eventually gave up. > > The code is definitely hard to follow and I've not come up with any > ways to make it easier to read. It's largely copied from the > interrupt-map version, but different enough that sharing isn't really > possible either. > > For interrupts, it's documented in the DT spec. The best (only?) > explanation for how interrupt-map works is here[1] (used to be at > devicetree.org). > interrupt-map is also documented in the latest DT spec[2]. I can add another section for "Generic Nexus Properties" and describe how this code works. [2] http://www.devicetree.org/specifications-pdf
On 02/09/17 10:52, Frank Rowand wrote: > On 02/09/17 07:17, Rob Herring wrote: >> On Tue, Jan 24, 2017 at 12:05 PM, Rob Herring <robh+dt@kernel.org> wrote: >>> On Tue, Jan 24, 2017 at 12:36 AM, Frank Rowand <frowand.list@gmail.com> wrote: >>>> Hi Stephen, >>>> >>>> Sorry I did not get to v1 and v2 in a timely manner. >>>> >>>> >>>> On 01/23/17 12:48, Stephen Boyd wrote: >>>>> Platforms like 96boards have a standardized connector/expansion >>>>> slot that exposes signals like GPIOs to expansion boards in an >>>>> SoC agnostic way. We'd like the DT overlays for the expansion >>>>> boards to be written once without knowledge of the SoC on the >>>>> other side of the connector. This avoids the unscalable >>>>> combinatorial explosion of a different DT overlay for each >>>>> expansion board and SoC pair. >>>>> >>>>> We need a way to describe the GPIOs routed through the connector >>>>> in an SoC agnostic way. Let's introduce nexus property parsing >>>>> into the OF core to do this. This is largely based on the >>>>> interrupt nexus support we already have. This allows us to remap >>>>> a phandle list in a consumer node (e.g. reset-gpios) through a >>>>> connector in a generic way (e.g. via gpio-map). Do this in a >>>>> generic routine so that we can remap any sort of variable length >>>>> phandle list. >>>>> >>>>> Taking GPIOs as an example, the connector would be a GPIO nexus, >>>>> supporting the remapping of a GPIO specifier space to multiple >>>>> GPIO providers on the SoC. DT would look as shown below, where >>>>> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an >>>>> expansion port where boards can be plugged in, and >>>>> 'expansion_device' is a device on the expansion board. >>>>> >>>>> soc { >>>>> soc_gpio1: gpio-controller1 { >>>>> #gpio-cells = <2>; >>>>> }; >>>>> >>>>> soc_gpio2: gpio-controller2 { >>>>> #gpio-cells = <2>; >>>>> }; >>>>> }; >>>>> >>>>> connector: connector { >>>>> #gpio-cells = <2>; >>>>> gpio-map = <0 0 &soc_gpio1 1 0>, >>>>> <1 0 &soc_gpio2 4 0>, >>>>> <2 0 &soc_gpio1 3 0>, >>>>> <3 0 &soc_gpio2 2 0>; >>>>> gpio-map-mask = <0xf 0x0>; >>>>> gpio-map-pass-thru = <0x0 0x1> >>>>> }; >>>>> >>>>> expansion_device { >>>>> reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; >>>>> }; >>>> >>>> The how to architect connectors and plugs threads fell asleep before >>>> coming to a resolution. We need to revive that discussion. >>>> >>>> One of the concepts of the plug and connector architecture is that >>>> a main board may contain multiple connectors of the same type (or >>>> different types, but the same type is sufficient for this discussion). >>>> >>>> The node describing the card that plugs into one of the connectors >>>> does not know the phandle of the connector it is going to be >>>> connected to. Some other mechanism is provided to allow a card >>>> to be plugged into any of the available connectors. If there are >>>> two identical cards plugged into two connectors, then both cards >>>> have the same exact device tree node. But some mechanism will >>>> exist to resolve (or "link") the two card nodes to the different >>>> connector nodes. >>>> >>>> As a result of this, in the above example the reset-gpios property >>>> in the node 'expansion_device' can not contain '&connector'. The >>>> concept of &connector belongs to the entire expansion_device node, >>>> not to individual properties within the node. >>> >>> I think this is easily solved with a connector having 2 halves and >>> that we need to search parents for *-map properties. Inheriting from >>> parents is a common pattern in DT though perhaps not walking the >>> parents of a phandle. So we'd have something like this: >>> >>> base-connector-1 { >>> gpio-map = ... >>> connector { >>> child { >>> some-gpios = <&connector 1>; >>> }; >>> }; >>> }; >>> >>> base-connector-2 { >>> gpio-map = ... >>> connector { >>> child { >>> some-gpios = <&connector 1>; >>> }; >>> }; >>> }; >>> >>> Now, how we resolve that /connector from an overlay targets >>> /base-connector-1 and /base-connector-2 is an orthogonal issue and one >>> that's going to be connector specific (at least for probe-able >>> connectors). >> >> Frank, any more comments on this? If not, I plan to apply this series. >> >> Rob > > Yes, how we resolve which connector a plug goes into is orthogonal. > > My objection is that the original example has a property in the > plug node (that is, on the expansion board), directly referencing > the connector node, instead of referencing a resource inside the > connector node. > > In the original example, it would make more sense for the first > item in the reset-gpios property to be &gpio-map or "gpio-map" > instead of &connector. My suggestion of &gpio-map or "gpio-map" was just shooting from the hip. After going back to the DT connector thread, I have a different answer of &gpio1 instead of &connector: reset-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>; The full answer in more than changing &connector to &gpio1, see below for the fuller dts. I took David Gibson's initial socket / plug proposal[1], and added in the gpios from Stephen's proposal. [1] https://lkml.org/lkml/2016/7/18/332 ----- Socket: /dts-v1/; / { compatible = "foo,oldboard"; ranges; soc@... { ranges; mmio: mmio-bus@... { #address-cells = <2>; #size-cells = <2>; ranges; }; i2c: i2c@... { }; intc: intc@... { #interrupt-cells = <2>; }; }; connectors { widget1 { compatible = "foo,widget-socket"; w1_irqs: irqs { interrupt-controller; #address-cells = <0>; #interrupt-cells = <1>; interrupt-map-mask = <0xffffffff>; interrupt-map = < 0 &intc 7 0 1 &intc 8 0 >; }; w1_gpio1: gpio1 { #gpio-cells = <1>; gpio-map = <0 &soc_gpio1 1 0>, <1 &soc_gpio2 4 0>, <2 &soc_gpio1 3 0>, <3 &soc_gpio2 2 0>; gpio-map-mask = <0xf>; }; aliases = { i2c = &i2c; intc = &w1_irqs; mmio = &mmio; gpio1 = &w1_gpios1; }; }; }; }; ----- Expansion board: /dts-v1/; /plugin/ foo,widget-socket { compatible = "foo,whirligig-widget"; }; expansion_device { reset-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>; }; &i2c { whirligig-controller@... { ... interrupt-parent = <&widget-irqs>; interrupts = <0>; }; };
diff --git a/drivers/of/base.c b/drivers/of/base.c index d4bea3c797d6..fb6bb855714e 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1775,6 +1775,190 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na EXPORT_SYMBOL(of_parse_phandle_with_args); /** + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it + * @np: pointer to a device tree node containing a list + * @list_name: property name that contains a list + * @stem_name: stem of property names that specify phandles' arguments count + * @index: index of a phandle to parse out + * @out_args: optional pointer to output arguments structure (will be filled) + * + * This function is useful to parse lists of phandles and their arguments. + * Returns 0 on success and fills out_args, on error returns appropriate errno + * value. The difference between this function and of_parse_phandle_with_args() + * is that this API remaps a phandle if the node the phandle points to has + * a <@stem_name>-map property. + * + * Caller is responsible to call of_node_put() on the returned out_args->np + * pointer. + * + * Example: + * + * phandle1: node1 { + * #list-cells = <2>; + * } + * + * phandle2: node2 { + * #list-cells = <1>; + * } + * + * phandle3: node3 { + * #list-cells = <1>; + * list-map = <0 &phandle2 3>, + * <1 &phandle2 2>, + * <2 &phandle1 5 1>; + * list-map-mask = <0x3>; + * }; + * + * node4 { + * list = <&phandle1 1 2 &phandle3 0>; + * } + * + * To get a device_node of the `node2' node you may call this: + * of_parse_phandle_with_args(node4, "list", "list", 1, &args); + */ +int of_parse_phandle_with_args_map(const struct device_node *np, + const char *list_name, + const char *stem_name, + int index, struct of_phandle_args *out_args) +{ + char *cells_name, *map_name = NULL, *mask_name = NULL; + char *pass_name = NULL; + struct device_node *cur, *new = NULL; + const __be32 *map, *mask, *pass; + static const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 }; + static const __be32 dummy_pass[] = { [0 ... MAX_PHANDLE_ARGS] = 0 }; + __be32 initial_match_array[MAX_PHANDLE_ARGS]; + const __be32 *match_array = initial_match_array; + int i, ret, map_len, match; + u32 list_size, new_size; + + if (index < 0) + return -EINVAL; + + cells_name = kasprintf(GFP_KERNEL, "#%s-cells", stem_name); + if (!cells_name) + return -ENOMEM; + + ret = -ENOMEM; + map_name = kasprintf(GFP_KERNEL, "%s-map", stem_name); + if (!map_name) + goto free; + + mask_name = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name); + if (!mask_name) + goto free; + + pass_name = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name); + if (!pass_name) + goto free; + + ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index, + out_args); + if (ret) + goto free; + + /* Get the #<list>-cells property */ + cur = out_args->np; + ret = of_property_read_u32(cur, cells_name, &list_size); + if (ret < 0) + goto put; + + /* Precalculate the match array - this simplifies match loop */ + for (i = 0; i < list_size; i++) + initial_match_array[i] = cpu_to_be32(out_args->args[i]); + + ret = -EINVAL; + while (cur) { + /* Get the <list>-map property */ + map = of_get_property(cur, map_name, &map_len); + if (!map) { + ret = 0; + goto free; + } + map_len /= sizeof(u32); + + /* Get the <list>-map-mask property (optional) */ + mask = of_get_property(cur, mask_name, NULL); + if (!mask) + mask = dummy_mask; + /* Iterate through <list>-map property */ + match = 0; + while (map_len > (list_size + 1) && !match) { + /* Compare specifiers */ + match = 1; + for (i = 0; i < list_size; i++, map_len--) + match &= !((match_array[i] ^ *map++) & mask[i]); + + of_node_put(new); + new = of_find_node_by_phandle(be32_to_cpup(map)); + map++; + map_len--; + + /* Check if not found */ + if (!new) + goto put; + + if (!of_device_is_available(new)) + match = 0; + + ret = of_property_read_u32(new, cells_name, &new_size); + if (ret) + goto put; + + /* Check for malformed properties */ + if (WARN_ON(new_size > MAX_PHANDLE_ARGS)) + goto put; + if (map_len < new_size) + goto put; + + /* Move forward by new node's #<list>-cells amount */ + map += new_size; + map_len -= new_size; + } + if (!match) + goto put; + + /* Get the <list>-map-pass-thru property (optional) */ + pass = of_get_property(cur, pass_name, NULL); + if (!pass) + pass = dummy_pass; + + /* + * Successfully parsed a <list>-map translation; copy new + * specifier into the out_args structure, keeping the + * bits specified in <list>-map-pass-thru. + */ + match_array = map - new_size; + for (i = 0; i < new_size; i++) { + __be32 val = *(map - new_size + i); + + if (i < list_size) { + val &= ~pass[i]; + val |= cpu_to_be32(out_args->args[i]) & pass[i]; + } + + out_args->args[i] = be32_to_cpu(val); + } + out_args->args_count = list_size = new_size; + /* Iterate again with new provider */ + out_args->np = new; + of_node_put(cur); + cur = new; + } +put: + of_node_put(cur); + of_node_put(new); +free: + kfree(mask_name); + kfree(map_name); + kfree(cells_name); + kfree(pass_name); + + return ret; +} +EXPORT_SYMBOL(of_parse_phandle_with_args_map); + +/** * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list * @np: pointer to a device tree node containing a list * @list_name: property name that contains a list diff --git a/include/linux/of.h b/include/linux/of.h index 011c4984cdf5..f22d4a83ca07 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np, extern int of_parse_phandle_with_args(const struct device_node *np, const char *list_name, const char *cells_name, int index, struct of_phandle_args *out_args); +extern int of_parse_phandle_with_args_map(const struct device_node *np, + const char *list_name, const char *stem_name, int index, + struct of_phandle_args *out_args); extern int of_parse_phandle_with_fixed_args(const struct device_node *np, const char *list_name, int cells_count, int index, struct of_phandle_args *out_args); @@ -738,6 +741,15 @@ static inline int of_parse_phandle_with_args(const struct device_node *np, return -ENOSYS; } +static inline int of_parse_phandle_with_args_map(const struct device_node *np, + const char *list_name, + const char *stem_name, + int index, + struct of_phandle_args *out_args) +{ + return -ENOSYS; +} + static inline int of_parse_phandle_with_fixed_args(const struct device_node *np, const char *list_name, int cells_count, int index, struct of_phandle_args *out_args)
Platforms like 96boards have a standardized connector/expansion slot that exposes signals like GPIOs to expansion boards in an SoC agnostic way. We'd like the DT overlays for the expansion boards to be written once without knowledge of the SoC on the other side of the connector. This avoids the unscalable combinatorial explosion of a different DT overlay for each expansion board and SoC pair. We need a way to describe the GPIOs routed through the connector in an SoC agnostic way. Let's introduce nexus property parsing into the OF core to do this. This is largely based on the interrupt nexus support we already have. This allows us to remap a phandle list in a consumer node (e.g. reset-gpios) through a connector in a generic way (e.g. via gpio-map). Do this in a generic routine so that we can remap any sort of variable length phandle list. Taking GPIOs as an example, the connector would be a GPIO nexus, supporting the remapping of a GPIO specifier space to multiple GPIO providers on the SoC. DT would look as shown below, where 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an expansion port where boards can be plugged in, and 'expansion_device' is a device on the expansion board. soc { soc_gpio1: gpio-controller1 { #gpio-cells = <2>; }; soc_gpio2: gpio-controller2 { #gpio-cells = <2>; }; }; connector: connector { #gpio-cells = <2>; gpio-map = <0 0 &soc_gpio1 1 0>, <1 0 &soc_gpio2 4 0>, <2 0 &soc_gpio1 3 0>, <3 0 &soc_gpio2 2 0>; gpio-map-mask = <0xf 0x0>; gpio-map-pass-thru = <0x0 0x1> }; expansion_device { reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>; }; The GPIO core would use of_parse_phandle_with_args_map() instead of of_parse_phandle_with_args() and arrive at the same type of result, a phandle and argument list. The difference is that the phandle and arguments will be remapped through the nexus node to the underlying SoC GPIO controller node. In the example above, we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>. Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Mark Brown <broonie@kernel.org> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org> --- drivers/of/base.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of.h | 12 ++++ 2 files changed, 196 insertions(+)