| Message ID | BANLkTi=hkScxYpt449CQCky+bLU3UvkC_A@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Mitch Bradley wrote at Monday, May 30, 2011 2:53 PM: > ... > GPIOs share the need to "point across the tree to different nodes", but > it is unclear that there is a need for a entirely different hierarchy. > > That suggests the possibility of using the device tree addressing > mechanism as much as possible. Normal device tree addresses could be > used to specify GPIO numbers. > > Suppose we have: > > gpio-controller1: gpio-controller { > #address-cells = <2>; > #mode-cells = <2>; > gpio1: gpio@12,0 { > reg = <12 0>; > mode = <55 66>; > usage = "Audio Codec chip select"; /* Optional */ > } > }; > gpio-controller2: gpio-controller { > #address-cells = <1>; > #mode-cells = <1>; > gpio2: gpio@4 { > reg = <4>; > #mode-cells = <1>; > } > }; A quick note on the way that Tegra's devicetree files are broken up in Grant's devicetree/test branch: * There's "tegra250.dtsi" which defines everything on the Tegra SoC itself. * There's a per-board e.g. harmony.dts which includes tegra250.dtsi, And additionally: ** Defines all devices on the board ** Hence, defines the usage of e.g. all the Tegra GPIOs for the board. I like this model, because it shares the complete definition of the Tegra SoC in a single file, rather than duplicating it with cut/paste into every board file. As such, the code quoted above would be in tegra250.dtsi. This has two relevant implications here: 1) The names "gpio1" and "gpio2" would be driven by the Tegra SoC's naming of those GPIO pins, and not any board-specific naming, e.g. "tegra_gpio_px1", "tegra_gpio_pb5". Equally, the usage comment would be at the client/reference site, not the GPIO definition site. 2) The GPIO mode should be defined by the client/user of the GPIO, not the SoC's definition of that GPIO. Without those two conditions, we couldn't share anything through tegra250.dtsi. Re-iteration of these implications below. > [...] > chipsel-gpio = <&gpio1>, > <&gpio-controller1 13 0 55 77>, > <0>, /* holes are permitted, means no GPIO 2 */ > <&gpio2 88>, > <&gpio-controller2 5 1>; > A GPIO spec consist of: > > 1) A reference to either a gpio-controller or a gpio device node. > > 2) 0 or more address cells, according to the value of #address-cells in > the referenced node. If the node has no #address-cells property, the > value is assumed to be 0. I'd expect address cells only if referencing a gpio-controller; if referencing a gpio device node, the address would be implicit. > 3) 0 or more mode cells, according to the value of #mode-cells in the > referenced node. Yes, I agree. Although, I think your (3) is inconsistent with the gpio controller definitions you wrote above, which include the mode definitions in the controller instead of the user. > In the example, the chipsel-gpio specs are interpreted as: > > <&gpio1> - refers to a pre-bound gpio device node, in which the > address (12 0) and mode (55 66) are specified within that node. Just re-iterating: I'd prefer mode to be solely in the reference, and not in the gpio controller. Does this SoC/board segregation make sense to everyone? Obviously it does to me:-)
On 5/31/2011 7:55 AM, Stephen Warren wrote: > Mitch Bradley wrote at Monday, May 30, 2011 2:53 PM: >> ... >> GPIOs share the need to "point across the tree to different nodes", but >> it is unclear that there is a need for a entirely different hierarchy. >> >> That suggests the possibility of using the device tree addressing >> mechanism as much as possible. Normal device tree addresses could be >> used to specify GPIO numbers. >> >> Suppose we have: >> >> gpio-controller1: gpio-controller { >> #address-cells =<2>; >> #mode-cells =<2>; >> gpio1: gpio@12,0 { >> reg =<12 0>; >> mode =<55 66>; >> usage = "Audio Codec chip select"; /* Optional */ >> } >> }; >> gpio-controller2: gpio-controller { >> #address-cells =<1>; >> #mode-cells =<1>; >> gpio2: gpio@4 { >> reg =<4>; >> #mode-cells =<1>; >> } >> }; > > A quick note on the way that Tegra's devicetree files are broken up in > Grant's devicetree/test branch: > > * There's "tegra250.dtsi" which defines everything on the Tegra SoC > itself. > * There's a per-board e.g. harmony.dts which includes tegra250.dtsi, > And additionally: > ** Defines all devices on the board > ** Hence, defines the usage of e.g. all the Tegra GPIOs for the > board. > > I like this model, because it shares the complete definition of the > Tegra SoC in a single file, rather than duplicating it with cut/paste > into every board file. > > As such, the code quoted above would be in tegra250.dtsi. > > This has two relevant implications here: > > 1) The names "gpio1" and "gpio2" would be driven by the Tegra SoC's > naming of those GPIO pins, and not any board-specific naming, e.g. > "tegra_gpio_px1", "tegra_gpio_pb5". Equally, the usage comment would > be at the client/reference site, not the GPIO definition site. > > 2) The GPIO mode should be defined by the client/user of the GPIO, not > the SoC's definition of that GPIO. > > Without those two conditions, we couldn't share anything through > tegra250.dtsi. > > Re-iteration of these implications below. > >> [...] >> chipsel-gpio =<&gpio1>, >> <&gpio-controller1 13 0 55 77>, >> <0>, /* holes are permitted, means no GPIO 2 */ >> <&gpio2 88>, >> <&gpio-controller2 5 1>; > >> A GPIO spec consist of: >> >> 1) A reference to either a gpio-controller or a gpio device node. >> >> 2) 0 or more address cells, according to the value of #address-cells in >> the referenced node. If the node has no #address-cells property, the >> value is assumed to be 0. > > I'd expect address cells only if referencing a gpio-controller; if > referencing a gpio device node, the address would be implicit. Yes. But I think it's better if there is a single rule for interpreting the GPIO spec, namely look at the #address-cells property, rather than deciding implicitly based on the type of the referent node. > >> 3) 0 or more mode cells, according to the value of #mode-cells in the >> referenced node. > > Yes, I agree. Although, I think your (3) is inconsistent with the gpio > controller definitions you wrote above, which include the mode > definitions in the controller instead of the user. Hmmm. I think I got the example right. Both of the gpio controller definitions have explicit "#address-cells" and "#mode-cells" properties, and neither has "mode". Both references to controller nodes have mode values in the user spec. gpio1 has "mode" but not "#mode-cells", and the reference to it has no mode value. gpio2 has "#mode-cells=1" but not "mode", and the reference to it has one mode value. Am I missing something? > >> In the example, the chipsel-gpio specs are interpreted as: >> >> <&gpio1> - refers to a pre-bound gpio device node, in which the >> address (12 0) and mode (55 66) are specified within that node. > > Just re-iterating: I'd prefer mode to be solely in the reference, and > not in the gpio controller. I agree that it doesn't make much sense for a controller node to specify the mode. My example doesn't do that. The only mode value is in an individual gpio node, not a controller node. From the standpoint of a SoC manufacturer, specifying modes in the reference is probably a good idea. But it's not necessarily best in all cases. If the focus of attention is a board design with numerous variants and revisions, "binding" the modes of specific gpio pins according to the board wiring may be the right thing. The way I specified it lets you choose. > > Does this SoC/board segregation make sense to everyone? Obviously it > does to me:-) It makes perfect sense from one viewpoint, but I think that board vendors may have a different focus. The flexibility to specify a mode in either place costs little, as the parsing rule is definite and straightforward. SoC vendors are free to defer mode decisions until later, by omitting "mode" and supplying "#mode-cells" in their device tree templates. Board vendors could choose either to use the SoC vendor's template verbatim, or to amend it with additional board-specific information.
Mitch Bradley wrote at Tuesday, May 31, 2011 12:43 PM: > On 5/31/2011 7:55 AM, Stephen Warren wrote: > > Mitch Bradley wrote at Monday, May 30, 2011 2:53 PM: > >> ... > >> GPIOs share the need to "point across the tree to different nodes", but > >> it is unclear that there is a need for a entirely different hierarchy. > >> > >> That suggests the possibility of using the device tree addressing > >> mechanism as much as possible. Normal device tree addresses could be > >> used to specify GPIO numbers. > >> > >> Suppose we have: > >> > >> gpio-controller1: gpio-controller { > >> #address-cells =<2>; > >> #mode-cells =<2>; > >> gpio1: gpio@12,0 { > >> reg =<12 0>; > >> mode =<55 66>; > >> usage = "Audio Codec chip select"; /* Optional */ > >> } > >> }; > >> gpio-controller2: gpio-controller { > >> #address-cells =<1>; > >> #mode-cells =<1>; > >> gpio2: gpio@4 { > >> reg =<4>; > >> #mode-cells =<1>; > >> } > >> }; > > > > A quick note on the way that Tegra's devicetree files are broken up in > > Grant's devicetree/test branch: > > > > * There's "tegra250.dtsi" which defines everything on the Tegra SoC > > itself. > > * There's a per-board e.g. harmony.dts which includes tegra250.dtsi, > > And additionally: > > ** Defines all devices on the board > > ** Hence, defines the usage of e.g. all the Tegra GPIOs for the > > board. > > > > I like this model, because it shares the complete definition of the > > Tegra SoC in a single file, rather than duplicating it with cut/paste > > into every board file. > > > > As such, the code quoted above would be in tegra250.dtsi. > > > > This has two relevant implications here: > > > > 1) The names "gpio1" and "gpio2" would be driven by the Tegra SoC's > > naming of those GPIO pins, and not any board-specific naming, e.g. > > "tegra_gpio_px1", "tegra_gpio_pb5". Equally, the usage comment would > > be at the client/reference site, not the GPIO definition site. > > > > 2) The GPIO mode should be defined by the client/user of the GPIO, not > > the SoC's definition of that GPIO. > > > > Without those two conditions, we couldn't share anything through > > tegra250.dtsi. > > > > Re-iteration of these implications below. > > > >> [...] > >> chipsel-gpio =<&gpio1>, > >> <&gpio-controller1 13 0 55 77>, > >> <0>, /* holes are permitted, means no GPIO 2 */ > >> <&gpio2 88>, > >> <&gpio-controller2 5 1>; > > > >> A GPIO spec consist of: > >> > >> 1) A reference to either a gpio-controller or a gpio device node. > >> > >> 2) 0 or more address cells, according to the value of #address-cells in > >> the referenced node. If the node has no #address-cells property, the > >> value is assumed to be 0. > > > > I'd expect address cells only if referencing a gpio-controller; if > > referencing a gpio device node, the address would be implicit. > > Yes. But I think it's better if there is a single rule for interpreting > the GPIO spec, namely look at the #address-cells property, rather than > deciding implicitly based on the type of the referent node. > > >> 3) 0 or more mode cells, according to the value of #mode-cells in the > >> referenced node. > > > > Yes, I agree. Although, I think your (3) is inconsistent with the gpio > > controller definitions you wrote above, which include the mode > > definitions in the controller instead of the user. > > Hmmm. I think I got the example right. You're right. The examples are consistent. I think what threw me was that the example code itself contained "<&gpio2 88>" whereas the description later referred to just "<gpio2>". Also, I hadn't noticed that the gpio2 definition repeated "#mode-cells = <1>;" whereas the gpio1 definition didn't. I have to say, I don't like that aspect; i.e. having to repeat #mode-cells in every gpio definition that's inside/underneath the controller definition; in my mind, "passing on" the requirement to define the mode would be the default state, so forcing the namer of GPIOs (i.e. whoever writes the "gpio1: gpio@12,0 {" definitions) to do this seems almost like busy work. Is there a way in *.dts to mark the #mode-cells field as inherited by children unless overridden? > >> In the example, the chipsel-gpio specs are interpreted as: > >> > >> <&gpio1> - refers to a pre-bound gpio device node, in which the > >> address (12 0) and mode (55 66) are specified within that node. > > > > Just re-iterating: I'd prefer mode to be solely in the reference, and > > not in the gpio controller. > > I agree that it doesn't make much sense for a controller node to specify > the mode. My example doesn't do that. The only mode value is in an > individual gpio node, not a controller node. Yes, I suppose that's true. But, in my mind at least, the controller definition would be part of the SoC definition, and provided by the SoC vendor in a separate and basically immutable file. As such, any gpio node inside the controller node really is part of the controller's/SoC's definition. > From the standpoint of a SoC manufacturer, specifying modes in the > reference is probably a good idea. But it's not necessarily best in all > cases. If the focus of attention is a board design with numerous > variants and revisions, "binding" the modes of specific gpio pins > according to the board wiring may be the right thing. > > The way I specified it lets you choose. Granted. However, I'm still not convinced that's a great idea; just because a board vendor might want to cut/paste the entire SoC definition into their DTS file and hack it, rather than just including it, doesn't mean it's a good idea. If we agreed on that (which I'll grant we probably don't) it seems like we shouldn't aim to add features that are probably only needed to make the life of people who do that easier. My perspective is that cut/pasting the entire SoC definition into a board definition is the devicetree equivalent of having more than one driver per chip, not sharing gpio/pinmux/... code, etc. in the kernel; the very stuff that caused Linus to complain about the state of ARM Linux. Equally, a separation of SoC vs. board should make incorporating bugfixes to the SoC definition into board definitions easier; simply replace the file and recompile. And, it'll make it more obvious to board vendors which changes need to be upstreamed to the SoC vendor, i.e. if a board vendor finds a bug in the SoC definition file. I suppose the one area this flexibility might make sense is if a board vendor has N similar boards, they can setup a set of include files: board-a.dts: include board-common.dtsi Do board-a customizations board-b-dts: include board-common.dtsi Do board-b customizations board-common.dtsi: include soc.dtsi Could add the gpio definitions to the controller definition from soc.dtsi soc.dtsi: base SoC definition; gpio controller, etc. But I still don't entirely see the advantage of board-common.dtsi defining GPIOs and having board-*.dts use those GPIOs as parameters to HW modules, e.g. as a GPIO list for an SDHCI controller, rather than simply having board-common.dtsi simply specify the SDHCI controller directly, thus avoiding the new syntax. > > Does this SoC/board segregation make sense to everyone? Obviously it > > does to me:-) > > It makes perfect sense from one viewpoint, but I think that board > vendors may have a different focus. The flexibility to specify a mode > in either place costs little, as the parsing rule is definite and > straightforward. SoC vendors are free to defer mode decisions until > later, by omitting "mode" and supplying "#mode-cells" in their device > tree templates. Board vendors could choose either to use the SoC > vendor's template verbatim, or to amend it with additional > board-specific information.
On Wed, Jun 01, 2011 at 08:59:55AM -0700, Stephen Warren wrote: > My perspective is that cut/pasting the entire SoC definition into a board > definition is the devicetree equivalent of having more than one driver > per chip, not sharing gpio/pinmux/... code, etc. in the kernel; the very > stuff that caused Linus to complain about the state of ARM Linux. Very strongly agreed, we've had to do that in the past because of limitations in the device tree infrastructure but if we end up having to cut'n'paste that's not going to be great for maintainability.
On 6/1/2011 5:59 AM, Stephen Warren wrote: > Mitch Bradley wrote at Tuesday, May 31, 2011 12:43 PM: >> On 5/31/2011 7:55 AM, Stephen Warren wrote: >>> Mitch Bradley wrote at Monday, May 30, 2011 2:53 PM: >>>> ... >>>> GPIOs share the need to "point across the tree to different nodes", but >>>> it is unclear that there is a need for a entirely different hierarchy. >>>> >>>> That suggests the possibility of using the device tree addressing >>>> mechanism as much as possible. Normal device tree addresses could be >>>> used to specify GPIO numbers. >>>> >>>> Suppose we have: >>>> >>>> gpio-controller1: gpio-controller { >>>> #address-cells =<2>; >>>> #mode-cells =<2>; >>>> gpio1: gpio@12,0 { >>>> reg =<12 0>; >>>> mode =<55 66>; >>>> usage = "Audio Codec chip select"; /* Optional */ >>>> } >>>> }; >>>> gpio-controller2: gpio-controller { >>>> #address-cells =<1>; >>>> #mode-cells =<1>; >>>> gpio2: gpio@4 { >>>> reg =<4>; >>>> #mode-cells =<1>; >>>> } >>>> }; >>> >>> A quick note on the way that Tegra's devicetree files are broken up in >>> Grant's devicetree/test branch: >>> >>> * There's "tegra250.dtsi" which defines everything on the Tegra SoC >>> itself. >>> * There's a per-board e.g. harmony.dts which includes tegra250.dtsi, >>> And additionally: >>> ** Defines all devices on the board >>> ** Hence, defines the usage of e.g. all the Tegra GPIOs for the >>> board. >>> >>> I like this model, because it shares the complete definition of the >>> Tegra SoC in a single file, rather than duplicating it with cut/paste >>> into every board file. >>> >>> As such, the code quoted above would be in tegra250.dtsi. >>> >>> This has two relevant implications here: >>> >>> 1) The names "gpio1" and "gpio2" would be driven by the Tegra SoC's >>> naming of those GPIO pins, and not any board-specific naming, e.g. >>> "tegra_gpio_px1", "tegra_gpio_pb5". Equally, the usage comment would >>> be at the client/reference site, not the GPIO definition site. >>> >>> 2) The GPIO mode should be defined by the client/user of the GPIO, not >>> the SoC's definition of that GPIO. >>> >>> Without those two conditions, we couldn't share anything through >>> tegra250.dtsi. >>> >>> Re-iteration of these implications below. >>> >>>> [...] >>>> chipsel-gpio =<&gpio1>, >>>> <&gpio-controller1 13 0 55 77>, >>>> <0>, /* holes are permitted, means no GPIO 2 */ >>>> <&gpio2 88>, >>>> <&gpio-controller2 5 1>; >>> >>>> A GPIO spec consist of: >>>> >>>> 1) A reference to either a gpio-controller or a gpio device node. >>>> >>>> 2) 0 or more address cells, according to the value of #address-cells in >>>> the referenced node. If the node has no #address-cells property, the >>>> value is assumed to be 0. >>> >>> I'd expect address cells only if referencing a gpio-controller; if >>> referencing a gpio device node, the address would be implicit. >> >> Yes. But I think it's better if there is a single rule for interpreting >> the GPIO spec, namely look at the #address-cells property, rather than >> deciding implicitly based on the type of the referent node. >> >>>> 3) 0 or more mode cells, according to the value of #mode-cells in the >>>> referenced node. >>> >>> Yes, I agree. Although, I think your (3) is inconsistent with the gpio >>> controller definitions you wrote above, which include the mode >>> definitions in the controller instead of the user. >> >> Hmmm. I think I got the example right. > > You're right. The examples are consistent. I think what threw me was that > the example code itself contained "<&gpio2 88>" whereas the description > later referred to just "<gpio2>". > > Also, I hadn't noticed that the gpio2 definition repeated > "#mode-cells =<1>;" whereas the gpio1 definition didn't. > > I have to say, I don't like that aspect; i.e. having to repeat > #mode-cells in every gpio definition that's inside/underneath the > controller definition; in my mind, "passing on" the requirement to > define the mode would be the default state, so forcing the namer of > GPIOs (i.e. whoever writes the "gpio1: gpio@12,0 {" definitions) to > do this seems almost like busy work. Is there a way in *.dts to mark > the #mode-cells field as inherited by children unless overridden? That is a very good point. Addressing it led me to a revised scheme that I like much better. (Notice that in the notes below I address your reasonable desire for an immutable SoC core definition.) The example: gpio-controller1: gpio-controller { #address-cells = <2>; #mode-cells = <2>; unbound_gpio1: gpio { /* No reg property */ /* No mode property */ } fully_bound_gpio1: audio-chipsel@12,0 { reg = <12 0>; mode = <55 66>; usage = "Audio Codec chip select"; /* Optional */ } address_bound_gpio1: gpio@13,0 { reg = <13 0>; /* No mode property */ } mode_bound_gpio1: open-drain-gpio { /* No reg property */ mode = <77 88>; } }; gpio-controller2: gpio-controller { #address-cells = <1>; #mode-cells = <1>; unbound_gpio2: gpio { /* No reg property */ /* No mode property */ } address_bound_gpio2: gpio@4 { reg = <4>; /* No mode property */ } }; [...] chipsel-gpio = <&fully_bound_gpio1>, <&unbound_gpio1 13 0 55 77>, <&mode_bound_gpio1 14 0>, <&address_bound_gpio2 88>, <&unbound_gpio2 5 1>; Notes: a) A reference to a GPIO always points to the child of a gpio-controller, i.e. to an individual gpio node. b) If that gpio node has a "reg" property, the number of address cells in the reference is 0, otherwise it is #address-cells from the parent (gpio-controller) node. c) If that gpio node has a "mode" property, the number of mode cells in the reference is 0, otherwise it is #mode-cells from the parent (gpio-controller) node. d) It's unnecessary for all children of a gpio controller to be named just "gpio". The important thing is that the semantics of the node - the set of properties (and, for Open Firmware systems, the set of methods) - meet the usage needs of the node's "client". e) gpios that are mode-bound but not address-bound must have distinct names from each other and from the unbound node name ("gpio"), because of the device tree rule that the combination of name+address must be unique among the children of a given node. It is okay to have both "gpio" and "gpio@12", but you cannot have two nodes both named just "gpio". f) SoC device tree blobs would probably use only the unbound form. A given platform might choose to specialize the tree by adding additional bound nodes to the tree established by the unmodified SoC core blob. g) reg-less nodes have been part of the Open Firmware spec forever; they are called "wildcard nodes". Their intended use is to refer to a class of similar objects, potentially so numerous that full enumeration is undesireable. > >>>> In the example, the chipsel-gpio specs are interpreted as: >>>> >>>> <&gpio1> - refers to a pre-bound gpio device node, in which the >>>> address (12 0) and mode (55 66) are specified within that node. >>> >>> Just re-iterating: I'd prefer mode to be solely in the reference, and >>> not in the gpio controller. >> >> I agree that it doesn't make much sense for a controller node to specify >> the mode. My example doesn't do that. The only mode value is in an >> individual gpio node, not a controller node. > > Yes, I suppose that's true. > > But, in my mind at least, the controller definition would be part of the > SoC definition, and provided by the SoC vendor in a separate and > basically immutable file. As such, any gpio node inside the controller > node really is part of the controller's/SoC's definition. > >> From the standpoint of a SoC manufacturer, specifying modes in the >> reference is probably a good idea. But it's not necessarily best in all >> cases. If the focus of attention is a board design with numerous >> variants and revisions, "binding" the modes of specific gpio pins >> according to the board wiring may be the right thing. >> >> The way I specified it lets you choose. > > Granted. > > However, I'm still not convinced that's a great idea; just because a > board vendor might want to cut/paste the entire SoC definition into their > DTS file and hack it, rather than just including it, doesn't mean it's > a good idea. If we agreed on that (which I'll grant we probably don't) > it seems like we shouldn't aim to add features that are probably only > needed to make the life of people who do that easier. > > My perspective is that cut/pasting the entire SoC definition into a board > definition is the devicetree equivalent of having more than one driver > per chip, not sharing gpio/pinmux/... code, etc. in the kernel; the very > stuff that caused Linus to complain about the state of ARM Linux. > > Equally, a separation of SoC vs. board should make incorporating bugfixes > to the SoC definition into board definitions easier; simply replace the > file and recompile. And, it'll make it more obvious to board vendors which > changes need to be upstreamed to the SoC vendor, i.e. if a board vendor > finds a bug in the SoC definition file. > > I suppose the one area this flexibility might make sense is if a board > vendor has N similar boards, they can setup a set of include files: > > board-a.dts: > include board-common.dtsi > Do board-a customizations > > board-b-dts: > include board-common.dtsi > Do board-b customizations > > board-common.dtsi: include soc.dtsi > Could add the gpio definitions to the controller definition from > soc.dtsi > > soc.dtsi: > base SoC definition; gpio controller, etc. > > But I still don't entirely see the advantage of board-common.dtsi > defining GPIOs and having board-*.dts use those GPIOs as parameters to > HW modules, e.g. as a GPIO list for an SDHCI controller, rather than > simply having board-common.dtsi simply specify the SDHCI controller > directly, thus avoiding the new syntax. > >>> Does this SoC/board segregation make sense to everyone? Obviously it >>> does to me:-) >> >> It makes perfect sense from one viewpoint, but I think that board >> vendors may have a different focus. The flexibility to specify a mode >> in either place costs little, as the parsing rule is definite and >> straightforward. SoC vendors are free to defer mode decisions until >> later, by omitting "mode" and supplying "#mode-cells" in their device >> tree templates. Board vendors could choose either to use the SoC >> vendor's template verbatim, or to amend it with additional >> board-specific information. > >
On Tue, May 31, 2011 at 11:55 AM, Stephen Warren <swarren@nvidia.com> wrote: > Mitch Bradley wrote at Monday, May 30, 2011 2:53 PM: >> ... >> GPIOs share the need to "point across the tree to different nodes", but >> it is unclear that there is a need for a entirely different hierarchy. >> >> That suggests the possibility of using the device tree addressing >> mechanism as much as possible. Normal device tree addresses could be >> used to specify GPIO numbers. >> >> Suppose we have: >> >> gpio-controller1: gpio-controller { >> #address-cells = <2>; >> #mode-cells = <2>; >> gpio1: gpio@12,0 { >> reg = <12 0>; >> mode = <55 66>; >> usage = "Audio Codec chip select"; /* Optional */ >> } >> }; >> gpio-controller2: gpio-controller { >> #address-cells = <1>; >> #mode-cells = <1>; >> gpio2: gpio@4 { >> reg = <4>; >> #mode-cells = <1>; >> } >> }; > > A quick note on the way that Tegra's devicetree files are broken up in > Grant's devicetree/test branch: > > * There's "tegra250.dtsi" which defines everything on the Tegra SoC > itself. > * There's a per-board e.g. harmony.dts which includes tegra250.dtsi, > And additionally: > ** Defines all devices on the board > ** Hence, defines the usage of e.g. all the Tegra GPIOs for the > board. > > I like this model, because it shares the complete definition of the > Tegra SoC in a single file, rather than duplicating it with cut/paste > into every board file. > > As such, the code quoted above would be in tegra250.dtsi. > > This has two relevant implications here: > > 1) The names "gpio1" and "gpio2" would be driven by the Tegra SoC's > naming of those GPIO pins, and not any board-specific naming, e.g. > "tegra_gpio_px1", "tegra_gpio_pb5". Equally, the usage comment would > be at the client/reference site, not the GPIO definition site. > > 2) The GPIO mode should be defined by the client/user of the GPIO, not > the SoC's definition of that GPIO. > > Without those two conditions, we couldn't share anything through > tegra250.dtsi. It's worth noting that the board file can override anything in tegra250.dtsi, even in the SoC nodes, so if needed properties can be added or modified in the SoC's description of the gpio controller. > > Just re-iterating: I'd prefer mode to be solely in the reference, and > not in the gpio controller. > > Does this SoC/board segregation make sense to everyone? Obviously it > does to me:-) It certainly does to me. :-) g.
On Mon, May 30, 2011 at 2:53 PM, Mitch Bradley <wmb@firmworks.com> wrote: > Perhaps the interrupt-mapping binding is not the best model. Interrupt > hardware in general is hierarchical but is not isomorphic to the physical > addressing hierarchy of the device tree. > > GPIOs share the need to "point across the tree to different nodes", but it > is unclear that there is a need for a entirely different hierarchy. > > That suggests the possibility of using the device tree addressing mechanism > as much as possible. Normal device tree addresses could be used to specify > GPIO numbers. > > Suppose we have: > > gpio-controller1: gpio-controller { > #address-cells = <2>; > #mode-cells = <2>; > gpio1: gpio@12,0 { > reg = <12 0>; > mode = <55 66>; > usage = "Audio Codec chip select"; /* Optional */ > } > }; > gpio-controller2: gpio-controller { > #address-cells = <1>; > #mode-cells = <1>; > gpio2: gpio@4 { > reg = <4>; > #mode-cells = <1>; > } > }; > [...] > chipsel-gpio = <&gpio1>, > <&gpio-controller1 13 0 55 77>, > <0>, /* holes are permitted, means no GPIO 2 */ > <&gpio2 88>, > <&gpio-controller2 5 1>; > > > A GPIO spec consist of: > > 1) A reference to either a gpio-controller or a gpio device node. > > 2) 0 or more address cells, according to the value of #address-cells in the > referenced node. If the node has no #address-cells property, the value is > assumed to be 0. > > 3) 0 or more mode cells, according to the value of #mode-cells in the > referenced node. I can see having nodes for individual gpios being useful in circumstances, but I really don't like having multiple methods of specifying a gpio (handle to the gpio-controller, or a handle to the gpio node, and a different specifier depending on the contents of the target node). I think it will be less confusing for users if the reference is always to the gpio controller. A specific gpio controller can still use child nodes to capture extra information about specific gpios, but doing so doesn't need to be exposed to a gpio consumer; it can all be internal to the gpio controller and its hardware specific binding (since any mode details are going to be hardware specific anyway most likely). [Amending to the above which was written before you latest post: even with the refined proposal to link to only a child node, the gpio specifier still changes depending on the contents of the child node] If a gpio controller does use child nodes, then I do like the approach of using #{address,size}-cells to line up with gpio numbering. However, we've already got a definition of #gpio-cells in use which specifies the total number of cells for a '*-gpio' property binding, so I do want to take care not to conflict with the existing pattern. I suspect the solution would simply be to state that #gpio-cells >= #address-cells must be true. > In the example, the chipsel-gpio specs are interpreted as: > > <&gpio1> - refers to a pre-bound gpio device node, in which the address > (12 0) and mode (55 66) are specified within that node. > > <&gpio-controller1 13 0 55 77> - refers to a GPIO controller node, > specifing the address (13 0) and the mode (55 77) in the client's GPIO spec. > > <gpio2> - another reference to a gpio node on a different controller. In > this case the address (4) is bound in the gpio node, but the mode (88) is > passed in from the client's GPIO spec. > > <&gpio-controller2 5 1> - another reference to a controller node, with a > one-cell address (5) and a one-cell mode (1), according to the values of > #address-cells and #mode-cells in that controller node. > > I expect that the "pre-bound gpio node" case would get a lot of use in > practice, as it lets you isolate the client from the details of the > interrupt controller addressing and modes. In my experience, GPIOs often > get reassigned between revisions of the same board, especially early in the > development cycle. I'm not convinced that the pre-bound gpio node will be any better or worse from a usability standpoint that direct references. I've certainly not had problems with keeping up with gpio moves (and everything else moving) on the FPGA projects that I've worked with. g.
On Wed, Jun 1, 2011 at 10:18 AM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Wed, Jun 01, 2011 at 08:59:55AM -0700, Stephen Warren wrote: > >> My perspective is that cut/pasting the entire SoC definition into a board >> definition is the devicetree equivalent of having more than one driver >> per chip, not sharing gpio/pinmux/... code, etc. in the kernel; the very >> stuff that caused Linus to complain about the state of ARM Linux. > > Very strongly agreed, we've had to do that in the past because of > limitations in the device tree infrastructure but if we end up having to > cut'n'paste that's not going to be great for maintainability. +2 cut'n'paste is not an option. period. :-) g. >
Mitch Bradley wrote at Wednesday, June 01, 2011 3:33 PM: > On 6/1/2011 5:59 AM, Stephen Warren wrote: > > ... > > I have to say, I don't like that aspect; i.e. having to repeat > > #mode-cells in every gpio definition that's inside/underneath the > > controller definition; in my mind, "passing on" the requirement to > > define the mode would be the default state, so forcing the namer of > > GPIOs (i.e. whoever writes the "gpio1: gpio@12,0 {" definitions) to > > do this seems almost like busy work. Is there a way in *.dts to mark > > the #mode-cells field as inherited by children unless overridden? > > That is a very good point. Addressing it led me to a revised scheme > that I like much better. (Notice that in the notes below I address your > reasonable desire for an immutable SoC core definition.) > > The example: > > gpio-controller1: gpio-controller { > #address-cells = <2>; > #mode-cells = <2>; > unbound_gpio1: gpio { > /* No reg property */ > /* No mode property */ > } > fully_bound_gpio1: audio-chipsel@12,0 { > reg = <12 0>; > mode = <55 66>; > usage = "Audio Codec chip select"; /* Optional */ > } > address_bound_gpio1: gpio@13,0 { > reg = <13 0>; > /* No mode property */ > } > mode_bound_gpio1: open-drain-gpio { > /* No reg property */ > mode = <77 88>; > } > }; > gpio-controller2: gpio-controller { > #address-cells = <1>; > #mode-cells = <1>; > unbound_gpio2: gpio { > /* No reg property */ > /* No mode property */ > } > address_bound_gpio2: gpio@4 { > reg = <4>; > /* No mode property */ > } > }; > [...] > chipsel-gpio = > <&fully_bound_gpio1>, > <&unbound_gpio1 13 0 55 77>, > <&mode_bound_gpio1 14 0>, > <&address_bound_gpio2 88>, > <&unbound_gpio2 5 1>; Sorry for the slow response. Some brief comments: a) I like this better than the previous proposal; the handling of #address-cells and #mode-cells is more consistent here. b) I like that the GPIO controller (or some overlay file on top of it) can provide names for GPIOs and names for modes. c) I suspect that something like the following won't work: chipsel-gpio = <&address_bound_gpio2 &mode_bound_gpio2>; It's an attempt to symbolically reference both a GPIO name/address and mode. I feel this kind of reference would be the most common case; a device wanting to use symbolic names for a particular GPIO location and mode. Only being able to specify name/address *or* mode symbolically, but not both, seems like a rather serious hole, and makes the scheme a lot less interesting to me. d) Doing this all with DT node references seems complex and overkill. I'd personally far rather see the device-tree compiler grow something like the C pre-processor, so you could just do: gpioc: gpio-controller { #address-cells = <2>; #mode-cells = <2>; }; #define TEGRA_GPIO_PA1 0 0 #define TEGRA_GPIO_PX2 23 1 #define TERGA_GPIO_PULLUP 0 #define TERGA_GPIO_PULLDOWN 1 #define TEGRA_GPIO_FAST 0 #define TEGRA_GPIO_SLOW 1 And then reference them like: chipsel-gpio = <&gpioc TEGRA_GPIO_PX2 TEGRA_GPIO_PULLUP TEGRA_GPIO_FAST>; Related to this, I'm having difficulty understanding why editing a GPIO reference in the style immediately above is any harder than editing a GPIO node within the GPIO controller of the style you most recently proposed.
On 6/3/2011 11:24 AM, Stephen Warren wrote: > Mitch Bradley wrote at Wednesday, June 01, 2011 3:33 PM: >> On 6/1/2011 5:59 AM, Stephen Warren wrote: >>> ... >>> I have to say, I don't like that aspect; i.e. having to repeat >>> #mode-cells in every gpio definition that's inside/underneath the >>> controller definition; in my mind, "passing on" the requirement to >>> define the mode would be the default state, so forcing the namer of >>> GPIOs (i.e. whoever writes the "gpio1: gpio@12,0 {" definitions) to >>> do this seems almost like busy work. Is there a way in *.dts to mark >>> the #mode-cells field as inherited by children unless overridden? >> >> That is a very good point. Addressing it led me to a revised scheme >> that I like much better. (Notice that in the notes below I address your >> reasonable desire for an immutable SoC core definition.) >> >> The example: >> >> gpio-controller1: gpio-controller { >> #address-cells =<2>; >> #mode-cells =<2>; >> unbound_gpio1: gpio { >> /* No reg property */ >> /* No mode property */ >> } >> fully_bound_gpio1: audio-chipsel@12,0 { >> reg =<12 0>; >> mode =<55 66>; >> usage = "Audio Codec chip select"; /* Optional */ >> } >> address_bound_gpio1: gpio@13,0 { >> reg =<13 0>; >> /* No mode property */ >> } >> mode_bound_gpio1: open-drain-gpio { >> /* No reg property */ >> mode =<77 88>; >> } >> }; >> gpio-controller2: gpio-controller { >> #address-cells =<1>; >> #mode-cells =<1>; >> unbound_gpio2: gpio { >> /* No reg property */ >> /* No mode property */ >> } >> address_bound_gpio2: gpio@4 { >> reg =<4>; >> /* No mode property */ >> } >> }; >> [...] >> chipsel-gpio = >> <&fully_bound_gpio1>, >> <&unbound_gpio1 13 0 55 77>, >> <&mode_bound_gpio1 14 0>, >> <&address_bound_gpio2 88>, >> <&unbound_gpio2 5 1>; > > Sorry for the slow response. Some brief comments: > > a) I like this better than the previous proposal; the handling of > #address-cells and #mode-cells is more consistent here. > > b) I like that the GPIO controller (or some overlay file on top of it) > can provide names for GPIOs and names for modes. > > c) I suspect that something like the following won't work: > > chipsel-gpio = > <&address_bound_gpio2&mode_bound_gpio2>; > > It's an attempt to symbolically reference both a GPIO name/address > and mode. I feel this kind of reference would be the most common > case; a device wanting to use symbolic names for a particular GPIO > location and mode. Only being able to specify name/address *or* mode > symbolically, but not both, seems like a rather serious hole, and > makes the scheme a lot less interesting to me. > > d) Doing this all with DT node references seems complex and overkill. I'd > personally far rather see the device-tree compiler grow something like > the C pre-processor, so you could just do: > > gpioc: gpio-controller { > #address-cells =<2>; > #mode-cells =<2>; > }; > #define TEGRA_GPIO_PA1 0 0 > #define TEGRA_GPIO_PX2 23 1 > #define TERGA_GPIO_PULLUP 0 > #define TERGA_GPIO_PULLDOWN 1 > #define TEGRA_GPIO_FAST 0 > #define TEGRA_GPIO_SLOW 1 > > And then reference them like: > > chipsel-gpio = > <&gpioc TEGRA_GPIO_PX2 TEGRA_GPIO_PULLUP TEGRA_GPIO_FAST>; > > Related to this, I'm having difficulty understanding why editing a > GPIO reference in the style immediately above is any harder than > editing a GPIO node within the GPIO controller of the style you most > recently proposed. > It seems that your focus is on the syntax of the device tree compiler. That's fine, but my focus is rather different - I'm concerned about the semantics of the "object model" represented by the tree layout. I don't use the device tree compiler. I do full-on Open Firmware systems, in which device node are active objects with methods. My intention in suggesting this scheme was primarily to promote the idea that gpios "underneath" a gpio-controller could be addressed using the device tree's normal physical addressing rules. It seems to me that a GPIO identification number is an "address" according to the device tree rules, thus the portion of the "gpio cells" list that identifies the specific pin should be thought of as a bona-file address, instead of something new and different. That thought led me to consider what individual GPIO nodes would look like, and that led me to the thought that it might be useful to "bind" some of them, so clients could refer to the "bound package" without having to know the details. This seemed good from an information-hiding perspective. I liked the idea that you could change the board and fix the reassignment in one place, instead of having to track down all the usage points. I sympathize with your desire for a human-readable DTC syntax that is not error-prone. I think that's somewhat orthogonal to the semantic issues of the device tree structure. Both are important.
On Mon, May 30, 2011 at 1:20 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Mon, May 30, 2011 at 12:54 PM, Segher Boessenkool > <segher@kernel.crashing.org> wrote: >>>> I'm currently dealing with an SoC that has over a hundred GPIOs. >>>> Whatever we choose, I think it should be able to handle an insane >>>> number of GPIOs without getting any more cumbersome that is >>>> necessary. >>> >>> This is *consumer* side GPIOs, not bindings for the device providing the >>> GPIOs. If a single device needs to use hundreds of GPIOs I'd expect >>> many of them will be block functions so you'd have a binding with an >>> array for things like "databus" and "addrbus". >> >> But please name them like "databus-gpio", so that it is obvious what it >> is. Also have to think about how this will work with multiple GPIO >> controllers: do you require the GPIO controller node to be part of every >> GPIO description, or do you do some "gpio-parent" scheme as well, how >> does that interact with not having a single array of GPIOs? >> >> Better write this down as a binding, before committing to it :-) > > Here's my thinking: Exactly the same binding for "gpios" as is now, > except can use different property names, like "chipsel-gpios" or > "wp-gpio". Here is a draft patch to the binding: I know there is still some debate on this, but I'm going to go ahead and commit this extension since it draws naturally from what we're already using, and it does not preclude doing a node binding at some point in the future. g. > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt > b/Documentation/devicetree/bindings/gpio/gpio.txt > index edaa84d..21dd4f9 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > @@ -4,17 +4,45 @@ Specifying GPIO information for devices > 1) gpios property > ----------------- > > -Nodes that makes use of GPIOs should define them using `gpios' property, > -format of which is: <&gpio-controller1-phandle gpio1-specifier > - &gpio-controller2-phandle gpio2-specifier > - 0 /* holes are permitted, means no GPIO 3 */ > - &gpio-controller4-phandle gpio4-specifier > - ...>; > +Nodes that makes use of GPIOs should specify them using one or more > +properties, each containing a 'gpio-list': > > -Note that gpio-specifier length is controller dependent. > + gpio-list ::= <single-gpio> [gpio-list] > + single-gpio ::= <gpio-phandle> <gpio-specifier> > + gpio-phandle : phandle to gpio controller node > + gpio-specifier : Array of #gpio-cells specifying specific gpio > + (controller specific) > + > +GPIO properties should be named either "gpios" or "*-gpio". Exact > +meaning of each gpio property must be documented in the device tree > +binding for each device. > + > +For example, the following could be used to describe gpios pins to use > +as chip select lines; with chip selects 0, 1 and 3 populated, and chip > +select 2 left empty: > + > + gpio1: gpio1 { > + gpio-controller > + #gpio-cells = <2>; > + }; > + gpio2: gpio2 { > + gpio-controller > + #gpio-cells = <1>; > + }; > + [...] > + chipsel-gpio = <&gpio1 12 0>, > + <&gpio1 13 0>, > + <0>, /* holes are permitted, means no GPIO 2 */ > + <&gpio2 2>; > + > +Note that gpio-specifier length is controller dependent. In the > +above example, &gpio1 uses 2 cells to specify a gpio, while &gpio2 > +only uses one. > > gpio-specifier may encode: bank, pin position inside the bank, > whether pin is open-drain and whether pin is logically inverted. > +Exact meaning of each specifier cell is controller specific, and must > +be documented in the device tree binding for the device. > > Example of the node using GPIOs: > > @@ -28,8 +56,8 @@ and empty GPIO flags as accepted by the "qe_pio_e" > gpio-controller. > 2) gpio-controller nodes > ------------------------ > > -Every GPIO controller node must have #gpio-cells property defined, > -this information will be used to translate gpio-specifiers. > +Every GPIO controller node must both an empty "gpio-controller" > +property, and have #gpio-cells contain the size of the gpio-specifier. > > Example of two SOC GPIO banks defined as gpio-controller nodes: >
diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt index edaa84d..21dd4f9 100644 --- a/Documentation/devicetree/bindings/gpio/gpio.txt +++ b/Documentation/devicetree/bindings/gpio/gpio.txt @@ -4,17 +4,45 @@ Specifying GPIO information for devices 1) gpios property ----------------- -Nodes that makes use of GPIOs should define them using `gpios' property, -format of which is: <&gpio-controller1-phandle gpio1-specifier - &gpio-controller2-phandle gpio2-specifier - 0 /* holes are permitted, means no GPIO 3 */ - &gpio-controller4-phandle gpio4-specifier - ...>; +Nodes that makes use of GPIOs should specify them using one or more +properties, each containing a 'gpio-list': -Note that gpio-specifier length is controller dependent. + gpio-list ::= <single-gpio> [gpio-list] + single-gpio ::= <gpio-phandle> <gpio-specifier> + gpio-phandle : phandle to gpio controller node + gpio-specifier : Array of #gpio-cells specifying specific gpio + (controller specific) + +GPIO properties should be named either "gpios" or "*-gpio". Exact +meaning of each gpio property must be documented in the device tree +binding for each device. + +For example, the following could be used to describe gpios pins to use +as chip select lines; with chip selects 0, 1 and 3 populated, and chip +select 2 left empty: + + gpio1: gpio1 { + gpio-controller + #gpio-cells = <2>; + }; + gpio2: gpio2 { + gpio-controller + #gpio-cells = <1>; + }; + [...] + chipsel-gpio = <&gpio1 12 0>, + <&gpio1 13 0>, + <0>, /* holes are permitted, means no GPIO 2 */ + <&gpio2 2>; + +Note that gpio-specifier length is controller dependent. In the +above example, &gpio1 uses 2 cells to specify a gpio, while &gpio2 +only uses one. gpio-specifier may encode: bank, pin position inside the bank, whether pin is open-drain and whether pin is logically inverted. +Exact meaning of each specifier cell is controller specific, and must +be documented in the device tree binding for the device. Example of the node using GPIOs: @@ -28,8 +56,8 @@ and empty GPIO flags as accepted by the "qe_pio_e" gpio-controller. 2) gpio-controller nodes ------------------------ -Every GPIO controller node must have #gpio-cells property defined, -this information will be used to translate gpio-specifiers. +Every GPIO controller node must both an empty "gpio-controller" +property, and have #gpio-cells contain the size of the gpio-specifier. Example of two SOC GPIO banks defined as gpio-controller nodes: