| Message ID | 20170104205536.15963-3-d-gerlach@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: > Add a generic power domain implementation, TI SCI PM Domains, that > will hook into the genpd framework and allow the TI SCI protocol to > control device power states. > > Also, provide macros representing each device index as understood > by TI SCI to be used in the device node power-domain references. > These are identifiers for the K2G devices managed by the PMMC. > > Signed-off-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > --- > v2->v3: > Update k2g_pds node docs to show it should be a child of pmmc node. > In early versions a phandle was used to point to pmmc and docs still > incorrectly showed this. > > .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ > MAINTAINERS | 2 + > include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++ > 3 files changed, 151 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > create mode 100644 include/dt-bindings/genpd/k2g.h > > diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > new file mode 100644 > index 000000000000..4c9064e512cb > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > @@ -0,0 +1,59 @@ > +Texas Instruments TI-SCI Generic Power Domain > +--------------------------------------------- > + > +Some TI SoCs contain a system controller (like the PMMC, etc...) that is > +responsible for controlling the state of the IPs that are present. > +Communication between the host processor running an OS and the system > +controller happens through a protocol known as TI-SCI [1]. This pm domain > +implementation plugs into the generic pm domain framework and makes use of > +the TI SCI protocol power on and off each device when needed. > + > +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt > + > +PM Domain Node > +============== > +The PM domain node represents the global PM domain managed by the PMMC, > +which in this case is the single implementation as documented by the generic > +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt. > +Because this relies on the TI SCI protocol to communicate with the PMMC it > +must be a child of the pmmc node. > + > +Required Properties: > +-------------------- > +- compatible: should be "ti,sci-pm-domain" > +- #power-domain-cells: Must be 0. > + > +Example (K2G): > +------------- > + pmmc: pmmc { > + compatible = "ti,k2g-sci"; > + ... > + > + k2g_pds: k2g_pds { > + compatible = "ti,sci-pm-domain"; > + #power-domain-cells = <0>; > + }; > + }; > + > +PM Domain Consumers > +=================== > +Hardware blocks that require SCI control over their state must provide > +a reference to the sci-pm-domain they are part of and a unique device > +specific ID that identifies the device. > + > +Required Properties: > +-------------------- > +- power-domains: phandle pointing to the corresponding PM domain node. > +- ti,sci-id: index representing the device id to be passed oevr SCI to > + be used for device control. As I've already stated before, this goes in power-domain cells. When you have a single thing (i.e. node) that controls multiple things, then you you need to specify the ID for each of them in phandle args. This is how irqs, gpio, clocks, *everything* in DT works. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob, On 01/09/2017 11:50 AM, Rob Herring wrote: > On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >> Add a generic power domain implementation, TI SCI PM Domains, that >> will hook into the genpd framework and allow the TI SCI protocol to >> control device power states. >> >> Also, provide macros representing each device index as understood >> by TI SCI to be used in the device node power-domain references. >> These are identifiers for the K2G devices managed by the PMMC. >> >> Signed-off-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >> --- >> v2->v3: >> Update k2g_pds node docs to show it should be a child of pmmc node. >> In early versions a phandle was used to point to pmmc and docs still >> incorrectly showed this. >> >> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ >> MAINTAINERS | 2 + >> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++ >> 3 files changed, 151 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >> create mode 100644 include/dt-bindings/genpd/k2g.h >> >> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >> new file mode 100644 >> index 000000000000..4c9064e512cb >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >> @@ -0,0 +1,59 @@ >> +Texas Instruments TI-SCI Generic Power Domain >> +--------------------------------------------- >> + >> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is >> +responsible for controlling the state of the IPs that are present. >> +Communication between the host processor running an OS and the system >> +controller happens through a protocol known as TI-SCI [1]. This pm domain >> +implementation plugs into the generic pm domain framework and makes use of >> +the TI SCI protocol power on and off each device when needed. >> + >> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >> + >> +PM Domain Node >> +============== >> +The PM domain node represents the global PM domain managed by the PMMC, >> +which in this case is the single implementation as documented by the generic >> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt. >> +Because this relies on the TI SCI protocol to communicate with the PMMC it >> +must be a child of the pmmc node. >> + >> +Required Properties: >> +-------------------- >> +- compatible: should be "ti,sci-pm-domain" >> +- #power-domain-cells: Must be 0. >> + >> +Example (K2G): >> +------------- >> + pmmc: pmmc { >> + compatible = "ti,k2g-sci"; >> + ... >> + >> + k2g_pds: k2g_pds { >> + compatible = "ti,sci-pm-domain"; >> + #power-domain-cells = <0>; >> + }; >> + }; >> + >> +PM Domain Consumers >> +=================== >> +Hardware blocks that require SCI control over their state must provide >> +a reference to the sci-pm-domain they are part of and a unique device >> +specific ID that identifies the device. >> + >> +Required Properties: >> +-------------------- >> +- power-domains: phandle pointing to the corresponding PM domain node. >> +- ti,sci-id: index representing the device id to be passed oevr SCI to >> + be used for device control. > > As I've already stated before, this goes in power-domain cells. When you > have a single thing (i.e. node) that controls multiple things, then you > you need to specify the ID for each of them in phandle args. This is how > irqs, gpio, clocks, *everything* in DT works. You think the reasoning for doing it this way provided by both Ulf and myself on v2 [1] is not valid then? From Ulf: To me, the TI SCI ID, is similar to a "conid" for any another "device resource" (like clock, pinctrl, regulator etc) which we can describe in DT and assign to a device node. The only difference here, is that we don't have common API to fetch the resource (like clk_get(), regulator_get()), but instead we fetches the device's resource from SoC specific code, via genpd's device ->attach() callback. From me: Yes, you've pretty much hit it on the head. It is not an index into a list of genpds but rather identifies the device *within* a single genpd. It is a property specific to each device that resides in a ti-sci-genpd, not a mapping describing which genpd the device belongs to. The generic power domain binding is concerned with mapping the device to a specific genpd, which is does fine for us, but we have a sub mapping for devices that exist inside a genpd which, we must describe as well, hence the ti,sci-id. So to summarize, the genpd framework does interpret the phandle arg as an index into multiple genpds, just as you've said other frameworks do, but this is not what I am trying to do, we have multiple devices within this *single* genpd, hence the need for the ti,sci-id property. Regards, Dave [1] https://patchwork.kernel.org/patch/9385371/ > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: > Rob, > > On 01/09/2017 11:50 AM, Rob Herring wrote: >> >> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>> >>> Add a generic power domain implementation, TI SCI PM Domains, that >>> will hook into the genpd framework and allow the TI SCI protocol to >>> control device power states. >>> >>> Also, provide macros representing each device index as understood >>> by TI SCI to be used in the device node power-domain references. >>> These are identifiers for the K2G devices managed by the PMMC. >>> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>> --- >>> v2->v3: >>> Update k2g_pds node docs to show it should be a child of pmmc >>> node. >>> In early versions a phandle was used to point to pmmc and docs >>> still >>> incorrectly showed this. >>> >>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ >>> MAINTAINERS | 2 + >>> include/dt-bindings/genpd/k2g.h | 90 >>> ++++++++++++++++++++++ >>> 3 files changed, 151 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>> create mode 100644 include/dt-bindings/genpd/k2g.h >>> >>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>> new file mode 100644 >>> index 000000000000..4c9064e512cb >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>> @@ -0,0 +1,59 @@ >>> +Texas Instruments TI-SCI Generic Power Domain >>> +--------------------------------------------- >>> + >>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is >>> +responsible for controlling the state of the IPs that are present. >>> +Communication between the host processor running an OS and the system >>> +controller happens through a protocol known as TI-SCI [1]. This pm >>> domain >>> +implementation plugs into the generic pm domain framework and makes use >>> of >>> +the TI SCI protocol power on and off each device when needed. >>> + >>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>> + >>> +PM Domain Node >>> +============== >>> +The PM domain node represents the global PM domain managed by the PMMC, >>> +which in this case is the single implementation as documented by the >>> generic >>> +PM domain bindings in >>> Documentation/devicetree/bindings/power/power_domain.txt. >>> +Because this relies on the TI SCI protocol to communicate with the PMMC >>> it >>> +must be a child of the pmmc node. >>> + >>> +Required Properties: >>> +-------------------- >>> +- compatible: should be "ti,sci-pm-domain" >>> +- #power-domain-cells: Must be 0. >>> + >>> +Example (K2G): >>> +------------- >>> + pmmc: pmmc { >>> + compatible = "ti,k2g-sci"; >>> + ... >>> + >>> + k2g_pds: k2g_pds { >>> + compatible = "ti,sci-pm-domain"; >>> + #power-domain-cells = <0>; >>> + }; >>> + }; >>> + >>> +PM Domain Consumers >>> +=================== >>> +Hardware blocks that require SCI control over their state must provide >>> +a reference to the sci-pm-domain they are part of and a unique device >>> +specific ID that identifies the device. >>> + >>> +Required Properties: >>> +-------------------- >>> +- power-domains: phandle pointing to the corresponding PM domain node. >>> +- ti,sci-id: index representing the device id to be passed oevr SCI to >>> + be used for device control. >> >> >> As I've already stated before, this goes in power-domain cells. When you >> have a single thing (i.e. node) that controls multiple things, then you >> you need to specify the ID for each of them in phandle args. This is how >> irqs, gpio, clocks, *everything* in DT works. > > > You think the reasoning for doing it this way provided by both Ulf and > myself on v2 [1] is not valid then? > > From Ulf: > > To me, the TI SCI ID, is similar to a "conid" for any another "device > resource" (like clock, pinctrl, regulator etc) which we can describe > in DT and assign to a device node. The only difference here, is that > we don't have common API to fetch the resource (like clk_get(), > regulator_get()), but instead we fetches the device's resource from > SoC specific code, via genpd's device ->attach() callback. Sorry, but that sounds like a kernel problem to me and has nothing to do with DT bindings. > From me: > > Yes, you've pretty much hit it on the head. It is not an index into a list > of genpds but rather identifies the device *within* a single genpd. It is a > property specific to each device that resides in a ti-sci-genpd, not a > mapping describing which genpd the device belongs to. The generic power > domain binding is concerned with mapping the device to a specific genpd, > which is does fine for us, but we have a sub mapping for devices that exist > inside a genpd which, we must describe as well, hence the ti,sci-id. > > > So to summarize, the genpd framework does interpret the phandle arg as an > index into multiple genpds, just as you've said other frameworks do, but > this is not what I am trying to do, we have multiple devices within this > *single* genpd, hence the need for the ti,sci-id property. Fix the genpd framework rather than work around it in DT. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob, On 01/11/2017 03:34 PM, Rob Herring wrote: > On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >> Rob, >> >> On 01/09/2017 11:50 AM, Rob Herring wrote: >>> >>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>> >>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>> will hook into the genpd framework and allow the TI SCI protocol to >>>> control device power states. >>>> >>>> Also, provide macros representing each device index as understood >>>> by TI SCI to be used in the device node power-domain references. >>>> These are identifiers for the K2G devices managed by the PMMC. >>>> >>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>> --- >>>> v2->v3: >>>> Update k2g_pds node docs to show it should be a child of pmmc >>>> node. >>>> In early versions a phandle was used to point to pmmc and docs >>>> still >>>> incorrectly showed this. >>>> >>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ >>>> MAINTAINERS | 2 + >>>> include/dt-bindings/genpd/k2g.h | 90 >>>> ++++++++++++++++++++++ >>>> 3 files changed, 151 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>> new file mode 100644 >>>> index 000000000000..4c9064e512cb >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>> @@ -0,0 +1,59 @@ >>>> +Texas Instruments TI-SCI Generic Power Domain >>>> +--------------------------------------------- >>>> + >>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is >>>> +responsible for controlling the state of the IPs that are present. >>>> +Communication between the host processor running an OS and the system >>>> +controller happens through a protocol known as TI-SCI [1]. This pm >>>> domain >>>> +implementation plugs into the generic pm domain framework and makes use >>>> of >>>> +the TI SCI protocol power on and off each device when needed. >>>> + >>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>> + >>>> +PM Domain Node >>>> +============== >>>> +The PM domain node represents the global PM domain managed by the PMMC, >>>> +which in this case is the single implementation as documented by the >>>> generic >>>> +PM domain bindings in >>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>> +Because this relies on the TI SCI protocol to communicate with the PMMC >>>> it >>>> +must be a child of the pmmc node. >>>> + >>>> +Required Properties: >>>> +-------------------- >>>> +- compatible: should be "ti,sci-pm-domain" >>>> +- #power-domain-cells: Must be 0. >>>> + >>>> +Example (K2G): >>>> +------------- >>>> + pmmc: pmmc { >>>> + compatible = "ti,k2g-sci"; >>>> + ... >>>> + >>>> + k2g_pds: k2g_pds { >>>> + compatible = "ti,sci-pm-domain"; >>>> + #power-domain-cells = <0>; >>>> + }; >>>> + }; >>>> + >>>> +PM Domain Consumers >>>> +=================== >>>> +Hardware blocks that require SCI control over their state must provide >>>> +a reference to the sci-pm-domain they are part of and a unique device >>>> +specific ID that identifies the device. >>>> + >>>> +Required Properties: >>>> +-------------------- >>>> +- power-domains: phandle pointing to the corresponding PM domain node. >>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to >>>> + be used for device control. >>> >>> >>> As I've already stated before, this goes in power-domain cells. When you >>> have a single thing (i.e. node) that controls multiple things, then you >>> you need to specify the ID for each of them in phandle args. This is how >>> irqs, gpio, clocks, *everything* in DT works. >> >> >> You think the reasoning for doing it this way provided by both Ulf and >> myself on v2 [1] is not valid then? >> >> From Ulf: >> >> To me, the TI SCI ID, is similar to a "conid" for any another "device >> resource" (like clock, pinctrl, regulator etc) which we can describe >> in DT and assign to a device node. The only difference here, is that >> we don't have common API to fetch the resource (like clk_get(), >> regulator_get()), but instead we fetches the device's resource from >> SoC specific code, via genpd's device ->attach() callback. > > Sorry, but that sounds like a kernel problem to me and has nothing to > do with DT bindings. > >> From me: >> >> Yes, you've pretty much hit it on the head. It is not an index into a list >> of genpds but rather identifies the device *within* a single genpd. It is a >> property specific to each device that resides in a ti-sci-genpd, not a >> mapping describing which genpd the device belongs to. The generic power >> domain binding is concerned with mapping the device to a specific genpd, >> which is does fine for us, but we have a sub mapping for devices that exist >> inside a genpd which, we must describe as well, hence the ti,sci-id. >> >> >> So to summarize, the genpd framework does interpret the phandle arg as an >> index into multiple genpds, just as you've said other frameworks do, but >> this is not what I am trying to do, we have multiple devices within this >> *single* genpd, hence the need for the ti,sci-id property. > > Fix the genpd framework rather than work around it in DT. I still disagree that this has nothing to do with DT bindings, as the current DT binding represents something different already. I am trying to extend it to give me additional information needed for our platforms. Are you saying that we should break what the current DT binding already represents to mean something else? Regards, Dave > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: > Rob, > > On 01/11/2017 03:34 PM, Rob Herring wrote: >> >> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>> >>> Rob, >>> >>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>> >>>> >>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>> >>>>> >>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>> will hook into the genpd framework and allow the TI SCI protocol to >>>>> control device power states. >>>>> >>>>> Also, provide macros representing each device index as understood >>>>> by TI SCI to be used in the device node power-domain references. >>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>> >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>> --- >>>>> v2->v3: >>>>> Update k2g_pds node docs to show it should be a child of pmmc >>>>> node. >>>>> In early versions a phandle was used to point to pmmc and docs >>>>> still >>>>> incorrectly showed this. >>>>> >>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ >>>>> MAINTAINERS | 2 + >>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>> ++++++++++++++++++++++ >>>>> 3 files changed, 151 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>> new file mode 100644 >>>>> index 000000000000..4c9064e512cb >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>> @@ -0,0 +1,59 @@ >>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>> +--------------------------------------------- >>>>> + >>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that >>>>> is >>>>> +responsible for controlling the state of the IPs that are present. >>>>> +Communication between the host processor running an OS and the system >>>>> +controller happens through a protocol known as TI-SCI [1]. This pm >>>>> domain >>>>> +implementation plugs into the generic pm domain framework and makes >>>>> use >>>>> of >>>>> +the TI SCI protocol power on and off each device when needed. >>>>> + >>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>> + >>>>> +PM Domain Node >>>>> +============== >>>>> +The PM domain node represents the global PM domain managed by the >>>>> PMMC, >>>>> +which in this case is the single implementation as documented by the >>>>> generic >>>>> +PM domain bindings in >>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>> +Because this relies on the TI SCI protocol to communicate with the >>>>> PMMC >>>>> it >>>>> +must be a child of the pmmc node. >>>>> + >>>>> +Required Properties: >>>>> +-------------------- >>>>> +- compatible: should be "ti,sci-pm-domain" >>>>> +- #power-domain-cells: Must be 0. >>>>> + >>>>> +Example (K2G): >>>>> +------------- >>>>> + pmmc: pmmc { >>>>> + compatible = "ti,k2g-sci"; >>>>> + ... >>>>> + >>>>> + k2g_pds: k2g_pds { >>>>> + compatible = "ti,sci-pm-domain"; >>>>> + #power-domain-cells = <0>; >>>>> + }; >>>>> + }; >>>>> + >>>>> +PM Domain Consumers >>>>> +=================== >>>>> +Hardware blocks that require SCI control over their state must provide >>>>> +a reference to the sci-pm-domain they are part of and a unique device >>>>> +specific ID that identifies the device. >>>>> + >>>>> +Required Properties: >>>>> +-------------------- >>>>> +- power-domains: phandle pointing to the corresponding PM domain node. >>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to >>>>> + be used for device control. >>>> >>>> >>>> >>>> As I've already stated before, this goes in power-domain cells. When you >>>> have a single thing (i.e. node) that controls multiple things, then you >>>> you need to specify the ID for each of them in phandle args. This is how >>>> irqs, gpio, clocks, *everything* in DT works. >>> >>> >>> >>> You think the reasoning for doing it this way provided by both Ulf and >>> myself on v2 [1] is not valid then? >>> >>> From Ulf: >>> >>> To me, the TI SCI ID, is similar to a "conid" for any another "device >>> resource" (like clock, pinctrl, regulator etc) which we can describe >>> in DT and assign to a device node. The only difference here, is that >>> we don't have common API to fetch the resource (like clk_get(), >>> regulator_get()), but instead we fetches the device's resource from >>> SoC specific code, via genpd's device ->attach() callback. >> >> >> Sorry, but that sounds like a kernel problem to me and has nothing to >> do with DT bindings. >> >>> From me: >>> >>> Yes, you've pretty much hit it on the head. It is not an index into a >>> list >>> of genpds but rather identifies the device *within* a single genpd. It is >>> a >>> property specific to each device that resides in a ti-sci-genpd, not a >>> mapping describing which genpd the device belongs to. The generic power >>> domain binding is concerned with mapping the device to a specific genpd, >>> which is does fine for us, but we have a sub mapping for devices that >>> exist >>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>> >>> >>> So to summarize, the genpd framework does interpret the phandle arg as an >>> index into multiple genpds, just as you've said other frameworks do, but >>> this is not what I am trying to do, we have multiple devices within this >>> *single* genpd, hence the need for the ti,sci-id property. >> >> >> Fix the genpd framework rather than work around it in DT. > > > I still disagree that this has nothing to do with DT bindings, as the > current DT binding represents something different already. I am trying to > extend it to give me additional information needed for our platforms. Are > you saying that we should break what the current DT binding already > represents to mean something else? No idea because what's the current binding? From the patch, looks like a new binding to me. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/13/2017 01:25 PM, Rob Herring wrote: > On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >> Rob, >> >> On 01/11/2017 03:34 PM, Rob Herring wrote: >>> >>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>> >>>> Rob, >>>> >>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>> >>>>> >>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>> >>>>>> >>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>> will hook into the genpd framework and allow the TI SCI protocol to >>>>>> control device power states. >>>>>> >>>>>> Also, provide macros representing each device index as understood >>>>>> by TI SCI to be used in the device node power-domain references. >>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>> >>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>> --- >>>>>> v2->v3: >>>>>> Update k2g_pds node docs to show it should be a child of pmmc >>>>>> node. >>>>>> In early versions a phandle was used to point to pmmc and docs >>>>>> still >>>>>> incorrectly showed this. >>>>>> >>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++ >>>>>> MAINTAINERS | 2 + >>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>> ++++++++++++++++++++++ >>>>>> 3 files changed, 151 insertions(+) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> new file mode 100644 >>>>>> index 000000000000..4c9064e512cb >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> @@ -0,0 +1,59 @@ >>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>> +--------------------------------------------- >>>>>> + >>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that >>>>>> is >>>>>> +responsible for controlling the state of the IPs that are present. >>>>>> +Communication between the host processor running an OS and the system >>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm >>>>>> domain >>>>>> +implementation plugs into the generic pm domain framework and makes >>>>>> use >>>>>> of >>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>> + >>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>> + >>>>>> +PM Domain Node >>>>>> +============== >>>>>> +The PM domain node represents the global PM domain managed by the >>>>>> PMMC, >>>>>> +which in this case is the single implementation as documented by the >>>>>> generic >>>>>> +PM domain bindings in >>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>> +Because this relies on the TI SCI protocol to communicate with the >>>>>> PMMC >>>>>> it >>>>>> +must be a child of the pmmc node. >>>>>> + >>>>>> +Required Properties: >>>>>> +-------------------- >>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>> +- #power-domain-cells: Must be 0. >>>>>> + >>>>>> +Example (K2G): >>>>>> +------------- >>>>>> + pmmc: pmmc { >>>>>> + compatible = "ti,k2g-sci"; >>>>>> + ... >>>>>> + >>>>>> + k2g_pds: k2g_pds { >>>>>> + compatible = "ti,sci-pm-domain"; >>>>>> + #power-domain-cells = <0>; >>>>>> + }; >>>>>> + }; >>>>>> + >>>>>> +PM Domain Consumers >>>>>> +=================== >>>>>> +Hardware blocks that require SCI control over their state must provide >>>>>> +a reference to the sci-pm-domain they are part of and a unique device >>>>>> +specific ID that identifies the device. >>>>>> + >>>>>> +Required Properties: >>>>>> +-------------------- >>>>>> +- power-domains: phandle pointing to the corresponding PM domain node. >>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to >>>>>> + be used for device control. >>>>> >>>>> >>>>> >>>>> As I've already stated before, this goes in power-domain cells. When you >>>>> have a single thing (i.e. node) that controls multiple things, then you >>>>> you need to specify the ID for each of them in phandle args. This is how >>>>> irqs, gpio, clocks, *everything* in DT works. >>>> >>>> >>>> >>>> You think the reasoning for doing it this way provided by both Ulf and >>>> myself on v2 [1] is not valid then? >>>> >>>> From Ulf: >>>> >>>> To me, the TI SCI ID, is similar to a "conid" for any another "device >>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>> in DT and assign to a device node. The only difference here, is that >>>> we don't have common API to fetch the resource (like clk_get(), >>>> regulator_get()), but instead we fetches the device's resource from >>>> SoC specific code, via genpd's device ->attach() callback. >>> >>> >>> Sorry, but that sounds like a kernel problem to me and has nothing to >>> do with DT bindings. >>> >>>> From me: >>>> >>>> Yes, you've pretty much hit it on the head. It is not an index into a >>>> list >>>> of genpds but rather identifies the device *within* a single genpd. It is >>>> a >>>> property specific to each device that resides in a ti-sci-genpd, not a >>>> mapping describing which genpd the device belongs to. The generic power >>>> domain binding is concerned with mapping the device to a specific genpd, >>>> which is does fine for us, but we have a sub mapping for devices that >>>> exist >>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>> >>>> >>>> So to summarize, the genpd framework does interpret the phandle arg as an >>>> index into multiple genpds, just as you've said other frameworks do, but >>>> this is not what I am trying to do, we have multiple devices within this >>>> *single* genpd, hence the need for the ti,sci-id property. >>> >>> >>> Fix the genpd framework rather than work around it in DT. >> >> >> I still disagree that this has nothing to do with DT bindings, as the >> current DT binding represents something different already. I am trying to >> extend it to give me additional information needed for our platforms. Are >> you saying that we should break what the current DT binding already >> represents to mean something else? > > No idea because what's the current binding? From the patch, looks like > a new binding to me. Yes, ti,sci-id is a new binding. I am referring to the current meaning of the "power-domains" binding, which is where you are asking this property to be added, in "power-domains" cells. This is documented here [1] in the kernel, although looking at it I must admit it is not very clear. The power-domains cell represents an offset into an array of power domains, if you choose to use it. That's what the genpd framework is hard coded to interpret it as. This is correct, as it is an index into a static list of power domains, used to identify which power domain a device belongs to, which is exactly what the genpd framework itself is concerned with. This is already how it is used in the kernel today. My ti,sci-id is not an index into a list of power domains, so it should not go in the power-domains cells and go against what the power-domains binding says that the cell expects. We have one single power domain, and the new ti,sci-id binding is not something the genpd framework itself is concerned with as it's our property to identify a device inside a power domain, not to identify which power domain it is associated with. Regards, Dave [1] Documentation/devicetree/bindings/power/power_domain.txt > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: > On 01/13/2017 01:25 PM, Rob Herring wrote: >> >> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>> >>> Rob, >>> >>> On 01/11/2017 03:34 PM, Rob Herring wrote: >>>> >>>> >>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>>> >>>>> >>>>> Rob, >>>>> >>>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>>> will hook into the genpd framework and allow the TI SCI protocol to >>>>>>> control device power states. >>>>>>> >>>>>>> Also, provide macros representing each device index as understood >>>>>>> by TI SCI to be used in the device node power-domain references. >>>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>>> >>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>>> --- >>>>>>> v2->v3: >>>>>>> Update k2g_pds node docs to show it should be a child of pmmc >>>>>>> node. >>>>>>> In early versions a phandle was used to point to pmmc and >>>>>>> docs >>>>>>> still >>>>>>> incorrectly showed this. >>>>>>> >>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 >>>>>>> ++++++++++++++ >>>>>>> MAINTAINERS | 2 + >>>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>>> ++++++++++++++++++++++ >>>>>>> 3 files changed, 151 insertions(+) >>>>>>> create mode 100644 >>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>> new file mode 100644 >>>>>>> index 000000000000..4c9064e512cb >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>> @@ -0,0 +1,59 @@ >>>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>>> +--------------------------------------------- >>>>>>> + >>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) >>>>>>> that >>>>>>> is >>>>>>> +responsible for controlling the state of the IPs that are present. >>>>>>> +Communication between the host processor running an OS and the >>>>>>> system >>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm >>>>>>> domain >>>>>>> +implementation plugs into the generic pm domain framework and makes >>>>>>> use >>>>>>> of >>>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>>> + >>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>>> + >>>>>>> +PM Domain Node >>>>>>> +============== >>>>>>> +The PM domain node represents the global PM domain managed by the >>>>>>> PMMC, >>>>>>> +which in this case is the single implementation as documented by the >>>>>>> generic >>>>>>> +PM domain bindings in >>>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>>> +Because this relies on the TI SCI protocol to communicate with the >>>>>>> PMMC >>>>>>> it >>>>>>> +must be a child of the pmmc node. >>>>>>> + >>>>>>> +Required Properties: >>>>>>> +-------------------- >>>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>>> +- #power-domain-cells: Must be 0. >>>>>>> + >>>>>>> +Example (K2G): >>>>>>> +------------- >>>>>>> + pmmc: pmmc { >>>>>>> + compatible = "ti,k2g-sci"; >>>>>>> + ... >>>>>>> + >>>>>>> + k2g_pds: k2g_pds { >>>>>>> + compatible = "ti,sci-pm-domain"; >>>>>>> + #power-domain-cells = <0>; >>>>>>> + }; >>>>>>> + }; >>>>>>> + >>>>>>> +PM Domain Consumers >>>>>>> +=================== >>>>>>> +Hardware blocks that require SCI control over their state must >>>>>>> provide >>>>>>> +a reference to the sci-pm-domain they are part of and a unique >>>>>>> device >>>>>>> +specific ID that identifies the device. >>>>>>> + >>>>>>> +Required Properties: >>>>>>> +-------------------- >>>>>>> +- power-domains: phandle pointing to the corresponding PM domain >>>>>>> node. >>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI >>>>>>> to >>>>>>> + be used for device control. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> As I've already stated before, this goes in power-domain cells. When >>>>>> you >>>>>> have a single thing (i.e. node) that controls multiple things, then >>>>>> you >>>>>> you need to specify the ID for each of them in phandle args. This is >>>>>> how >>>>>> irqs, gpio, clocks, *everything* in DT works. >>>>> >>>>> >>>>> >>>>> >>>>> You think the reasoning for doing it this way provided by both Ulf and >>>>> myself on v2 [1] is not valid then? >>>>> >>>>> From Ulf: >>>>> >>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device >>>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>>> in DT and assign to a device node. The only difference here, is that >>>>> we don't have common API to fetch the resource (like clk_get(), >>>>> regulator_get()), but instead we fetches the device's resource from >>>>> SoC specific code, via genpd's device ->attach() callback. >>>> >>>> >>>> >>>> Sorry, but that sounds like a kernel problem to me and has nothing to >>>> do with DT bindings. >>>> >>>>> From me: >>>>> >>>>> Yes, you've pretty much hit it on the head. It is not an index into a >>>>> list >>>>> of genpds but rather identifies the device *within* a single genpd. It >>>>> is >>>>> a >>>>> property specific to each device that resides in a ti-sci-genpd, not a >>>>> mapping describing which genpd the device belongs to. The generic power >>>>> domain binding is concerned with mapping the device to a specific >>>>> genpd, >>>>> which is does fine for us, but we have a sub mapping for devices that >>>>> exist >>>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>>> >>>>> >>>>> So to summarize, the genpd framework does interpret the phandle arg as >>>>> an >>>>> index into multiple genpds, just as you've said other frameworks do, >>>>> but >>>>> this is not what I am trying to do, we have multiple devices within >>>>> this >>>>> *single* genpd, hence the need for the ti,sci-id property. >>>> >>>> >>>> >>>> Fix the genpd framework rather than work around it in DT. >>> >>> >>> >>> I still disagree that this has nothing to do with DT bindings, as the >>> current DT binding represents something different already. I am trying to >>> extend it to give me additional information needed for our platforms. Are >>> you saying that we should break what the current DT binding already >>> represents to mean something else? >> >> >> No idea because what's the current binding? From the patch, looks like >> a new binding to me. > > > Yes, ti,sci-id is a new binding. I am referring to the current meaning of > the "power-domains" binding, which is where you are asking this property to > be added, in "power-domains" cells. This is documented here [1] in the > kernel, although looking at it I must admit it is not very clear. > > The power-domains cell represents an offset into an array of power domains, > if you choose to use it. That's what the genpd framework is hard coded to > interpret it as. This is correct, as it is an index into a static list of > power domains, used to identify which power domain a device belongs to, > which is exactly what the genpd framework itself is concerned with. This is > already how it is used in the kernel today. Strictly speaking, the cells are purely for the interpretation of the phandle they are associated with. If some controller wants to have 20 cells, then it could assuming a good reason. The reality is we tend to align the meaning of the cells. If genpd is interpreting the cells and not letting the driver for the power domain controller interpret them, then still, genpd needs to be fixed. IIRC, initially it was said genpd required 0 cells, hence my confusion. > My ti,sci-id is not an index into a list of power domains, so it should not > go in the power-domains cells and go against what the power-domains binding > says that the cell expects. We have one single power domain, and the new > ti,sci-id binding is not something the genpd framework itself is concerned > with as it's our property to identify a device inside a power domain, not to > identify which power domain it is associated with. What is the id used for? I can understand why you need to know what power domain a device is in (as power-domains identifies), but not what devices are in a power domain. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/13/2017 08:40 PM, Rob Herring wrote: > On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: >> On 01/13/2017 01:25 PM, Rob Herring wrote: >>> >>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>> >>>> Rob, >>>> >>>> On 01/11/2017 03:34 PM, Rob Herring wrote: >>>>> >>>>> >>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>>>> >>>>>> >>>>>> Rob, >>>>>> >>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to >>>>>>>> control device power states. >>>>>>>> >>>>>>>> Also, provide macros representing each device index as understood >>>>>>>> by TI SCI to be used in the device node power-domain references. >>>>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>>>> >>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>>>> --- >>>>>>>> v2->v3: >>>>>>>> Update k2g_pds node docs to show it should be a child of pmmc >>>>>>>> node. >>>>>>>> In early versions a phandle was used to point to pmmc and >>>>>>>> docs >>>>>>>> still >>>>>>>> incorrectly showed this. >>>>>>>> >>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 >>>>>>>> ++++++++++++++ >>>>>>>> MAINTAINERS | 2 + >>>>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>>>> ++++++++++++++++++++++ >>>>>>>> 3 files changed, 151 insertions(+) >>>>>>>> create mode 100644 >>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>> new file mode 100644 >>>>>>>> index 000000000000..4c9064e512cb >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>> @@ -0,0 +1,59 @@ >>>>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>>>> +--------------------------------------------- >>>>>>>> + >>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) >>>>>>>> that >>>>>>>> is >>>>>>>> +responsible for controlling the state of the IPs that are present. >>>>>>>> +Communication between the host processor running an OS and the >>>>>>>> system >>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm >>>>>>>> domain >>>>>>>> +implementation plugs into the generic pm domain framework and makes >>>>>>>> use >>>>>>>> of >>>>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>>>> + >>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>>>> + >>>>>>>> +PM Domain Node >>>>>>>> +============== >>>>>>>> +The PM domain node represents the global PM domain managed by the >>>>>>>> PMMC, >>>>>>>> +which in this case is the single implementation as documented by the >>>>>>>> generic >>>>>>>> +PM domain bindings in >>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>>>> +Because this relies on the TI SCI protocol to communicate with the >>>>>>>> PMMC >>>>>>>> it >>>>>>>> +must be a child of the pmmc node. >>>>>>>> + >>>>>>>> +Required Properties: >>>>>>>> +-------------------- >>>>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>>>> +- #power-domain-cells: Must be 0. >>>>>>>> + >>>>>>>> +Example (K2G): >>>>>>>> +------------- >>>>>>>> + pmmc: pmmc { >>>>>>>> + compatible = "ti,k2g-sci"; >>>>>>>> + ... >>>>>>>> + >>>>>>>> + k2g_pds: k2g_pds { >>>>>>>> + compatible = "ti,sci-pm-domain"; >>>>>>>> + #power-domain-cells = <0>; >>>>>>>> + }; >>>>>>>> + }; >>>>>>>> + >>>>>>>> +PM Domain Consumers >>>>>>>> +=================== >>>>>>>> +Hardware blocks that require SCI control over their state must >>>>>>>> provide >>>>>>>> +a reference to the sci-pm-domain they are part of and a unique >>>>>>>> device >>>>>>>> +specific ID that identifies the device. >>>>>>>> + >>>>>>>> +Required Properties: >>>>>>>> +-------------------- >>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain >>>>>>>> node. >>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI >>>>>>>> to >>>>>>>> + be used for device control. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> As I've already stated before, this goes in power-domain cells. When >>>>>>> you >>>>>>> have a single thing (i.e. node) that controls multiple things, then >>>>>>> you >>>>>>> you need to specify the ID for each of them in phandle args. This is >>>>>>> how >>>>>>> irqs, gpio, clocks, *everything* in DT works. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> You think the reasoning for doing it this way provided by both Ulf and >>>>>> myself on v2 [1] is not valid then? >>>>>> >>>>>> From Ulf: >>>>>> >>>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device >>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>>>> in DT and assign to a device node. The only difference here, is that >>>>>> we don't have common API to fetch the resource (like clk_get(), >>>>>> regulator_get()), but instead we fetches the device's resource from >>>>>> SoC specific code, via genpd's device ->attach() callback. >>>>> >>>>> >>>>> >>>>> Sorry, but that sounds like a kernel problem to me and has nothing to >>>>> do with DT bindings. >>>>> >>>>>> From me: >>>>>> >>>>>> Yes, you've pretty much hit it on the head. It is not an index into a >>>>>> list >>>>>> of genpds but rather identifies the device *within* a single genpd. It >>>>>> is >>>>>> a >>>>>> property specific to each device that resides in a ti-sci-genpd, not a >>>>>> mapping describing which genpd the device belongs to. The generic power >>>>>> domain binding is concerned with mapping the device to a specific >>>>>> genpd, >>>>>> which is does fine for us, but we have a sub mapping for devices that >>>>>> exist >>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>>>> >>>>>> >>>>>> So to summarize, the genpd framework does interpret the phandle arg as >>>>>> an >>>>>> index into multiple genpds, just as you've said other frameworks do, >>>>>> but >>>>>> this is not what I am trying to do, we have multiple devices within >>>>>> this >>>>>> *single* genpd, hence the need for the ti,sci-id property. >>>>> >>>>> >>>>> >>>>> Fix the genpd framework rather than work around it in DT. >>>> >>>> >>>> >>>> I still disagree that this has nothing to do with DT bindings, as the >>>> current DT binding represents something different already. I am trying to >>>> extend it to give me additional information needed for our platforms. Are >>>> you saying that we should break what the current DT binding already >>>> represents to mean something else? >>> >>> >>> No idea because what's the current binding? From the patch, looks like >>> a new binding to me. >> >> >> Yes, ti,sci-id is a new binding. I am referring to the current meaning of >> the "power-domains" binding, which is where you are asking this property to >> be added, in "power-domains" cells. This is documented here [1] in the >> kernel, although looking at it I must admit it is not very clear. >> >> The power-domains cell represents an offset into an array of power domains, >> if you choose to use it. That's what the genpd framework is hard coded to >> interpret it as. This is correct, as it is an index into a static list of >> power domains, used to identify which power domain a device belongs to, >> which is exactly what the genpd framework itself is concerned with. This is >> already how it is used in the kernel today. > > Strictly speaking, the cells are purely for the interpretation of the > phandle they are associated with. If some controller wants to have 20 > cells, then it could assuming a good reason. The reality is we tend to > align the meaning of the cells. If genpd is interpreting the cells and > not letting the driver for the power domain controller interpret them, > then still, genpd needs to be fixed. Ok, perhaps the genpd folks on the thread can jump in here with any thoughts that they have. > > IIRC, initially it was said genpd required 0 cells, hence my confusion. > >> My ti,sci-id is not an index into a list of power domains, so it should not >> go in the power-domains cells and go against what the power-domains binding >> says that the cell expects. We have one single power domain, and the new >> ti,sci-id binding is not something the genpd framework itself is concerned >> with as it's our property to identify a device inside a power domain, not to >> identify which power domain it is associated with. > > What is the id used for? I can understand why you need to know what > power domain a device is in (as power-domains identifies), but not > what devices are in a power domain. We have a system control processor that provides power management services to the OS and it responsible for handling the power state of each device. This control happens over a communication interface we have called TI SCI (implemented at drivers/firmware/ti-sci.c). The communication protocol uses these ids to identify each device within the power domain so that the control processor can do what is necessary to enable that device. Regards, Dave > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/01/17 00:12, Dave Gerlach wrote: > On 01/13/2017 08:40 PM, Rob Herring wrote: >> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: >>> On 01/13/2017 01:25 PM, Rob Herring wrote: >>>> >>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>>> >>>>> Rob, >>>>> >>>>> On 01/11/2017 03:34 PM, Rob Herring wrote: >>>>>> >>>>>> >>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Rob, >>>>>>> >>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>>>>> will hook into the genpd framework and allow the TI SCI >>>>>>>>> protocol to >>>>>>>>> control device power states. >>>>>>>>> >>>>>>>>> Also, provide macros representing each device index as understood >>>>>>>>> by TI SCI to be used in the device node power-domain references. >>>>>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>>>>> >>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>>>>> --- >>>>>>>>> v2->v3: >>>>>>>>> Update k2g_pds node docs to show it should be a child >>>>>>>>> of pmmc >>>>>>>>> node. >>>>>>>>> In early versions a phandle was used to point to pmmc and >>>>>>>>> docs >>>>>>>>> still >>>>>>>>> incorrectly showed this. >>>>>>>>> >>>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 >>>>>>>>> ++++++++++++++ >>>>>>>>> MAINTAINERS | 2 + >>>>>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>>>>> ++++++++++++++++++++++ >>>>>>>>> 3 files changed, 151 insertions(+) >>>>>>>>> create mode 100644 >>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>>>>> >>>>>>>>> diff --git >>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>> new file mode 100644 >>>>>>>>> index 000000000000..4c9064e512cb >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>> @@ -0,0 +1,59 @@ >>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>>>>> +--------------------------------------------- >>>>>>>>> + >>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) >>>>>>>>> that >>>>>>>>> is >>>>>>>>> +responsible for controlling the state of the IPs that are >>>>>>>>> present. >>>>>>>>> +Communication between the host processor running an OS and the >>>>>>>>> system >>>>>>>>> +controller happens through a protocol known as TI-SCI [1]. >>>>>>>>> This pm >>>>>>>>> domain >>>>>>>>> +implementation plugs into the generic pm domain framework and >>>>>>>>> makes >>>>>>>>> use >>>>>>>>> of >>>>>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>>>>> + >>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>>>>> + >>>>>>>>> +PM Domain Node >>>>>>>>> +============== >>>>>>>>> +The PM domain node represents the global PM domain managed by the >>>>>>>>> PMMC, >>>>>>>>> +which in this case is the single implementation as documented >>>>>>>>> by the >>>>>>>>> generic >>>>>>>>> +PM domain bindings in >>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>>>>> +Because this relies on the TI SCI protocol to communicate with >>>>>>>>> the >>>>>>>>> PMMC >>>>>>>>> it >>>>>>>>> +must be a child of the pmmc node. >>>>>>>>> + >>>>>>>>> +Required Properties: >>>>>>>>> +-------------------- >>>>>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>>>>> +- #power-domain-cells: Must be 0. >>>>>>>>> + >>>>>>>>> +Example (K2G): >>>>>>>>> +------------- >>>>>>>>> + pmmc: pmmc { >>>>>>>>> + compatible = "ti,k2g-sci"; >>>>>>>>> + ... >>>>>>>>> + >>>>>>>>> + k2g_pds: k2g_pds { >>>>>>>>> + compatible = "ti,sci-pm-domain"; >>>>>>>>> + #power-domain-cells = <0>; >>>>>>>>> + }; >>>>>>>>> + }; >>>>>>>>> + >>>>>>>>> +PM Domain Consumers >>>>>>>>> +=================== >>>>>>>>> +Hardware blocks that require SCI control over their state must >>>>>>>>> provide >>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique >>>>>>>>> device >>>>>>>>> +specific ID that identifies the device. >>>>>>>>> + >>>>>>>>> +Required Properties: >>>>>>>>> +-------------------- >>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain >>>>>>>>> node. >>>>>>>>> +- ti,sci-id: index representing the device id to be passed >>>>>>>>> oevr SCI >>>>>>>>> to >>>>>>>>> + be used for device control. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> As I've already stated before, this goes in power-domain cells. >>>>>>>> When >>>>>>>> you >>>>>>>> have a single thing (i.e. node) that controls multiple things, then >>>>>>>> you >>>>>>>> you need to specify the ID for each of them in phandle args. >>>>>>>> This is >>>>>>>> how >>>>>>>> irqs, gpio, clocks, *everything* in DT works. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> You think the reasoning for doing it this way provided by both >>>>>>> Ulf and >>>>>>> myself on v2 [1] is not valid then? >>>>>>> >>>>>>> From Ulf: >>>>>>> >>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another >>>>>>> "device >>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>>>>> in DT and assign to a device node. The only difference here, is that >>>>>>> we don't have common API to fetch the resource (like clk_get(), >>>>>>> regulator_get()), but instead we fetches the device's resource from >>>>>>> SoC specific code, via genpd's device ->attach() callback. >>>>>> >>>>>> >>>>>> >>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to >>>>>> do with DT bindings. >>>>>> >>>>>>> From me: >>>>>>> >>>>>>> Yes, you've pretty much hit it on the head. It is not an index >>>>>>> into a >>>>>>> list >>>>>>> of genpds but rather identifies the device *within* a single >>>>>>> genpd. It >>>>>>> is >>>>>>> a >>>>>>> property specific to each device that resides in a ti-sci-genpd, >>>>>>> not a >>>>>>> mapping describing which genpd the device belongs to. The generic >>>>>>> power >>>>>>> domain binding is concerned with mapping the device to a specific >>>>>>> genpd, >>>>>>> which is does fine for us, but we have a sub mapping for devices >>>>>>> that >>>>>>> exist >>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>>>>> >>>>>>> >>>>>>> So to summarize, the genpd framework does interpret the phandle >>>>>>> arg as >>>>>>> an >>>>>>> index into multiple genpds, just as you've said other frameworks do, >>>>>>> but >>>>>>> this is not what I am trying to do, we have multiple devices within >>>>>>> this >>>>>>> *single* genpd, hence the need for the ti,sci-id property. >>>>>> >>>>>> >>>>>> >>>>>> Fix the genpd framework rather than work around it in DT. >>>>> >>>>> >>>>> >>>>> I still disagree that this has nothing to do with DT bindings, as the >>>>> current DT binding represents something different already. I am >>>>> trying to >>>>> extend it to give me additional information needed for our >>>>> platforms. Are >>>>> you saying that we should break what the current DT binding already >>>>> represents to mean something else? >>>> >>>> >>>> No idea because what's the current binding? From the patch, looks like >>>> a new binding to me. >>> >>> >>> Yes, ti,sci-id is a new binding. I am referring to the current >>> meaning of >>> the "power-domains" binding, which is where you are asking this >>> property to >>> be added, in "power-domains" cells. This is documented here [1] in the >>> kernel, although looking at it I must admit it is not very clear. >>> >>> The power-domains cell represents an offset into an array of power >>> domains, >>> if you choose to use it. That's what the genpd framework is hard >>> coded to >>> interpret it as. This is correct, as it is an index into a static >>> list of >>> power domains, used to identify which power domain a device belongs to, >>> which is exactly what the genpd framework itself is concerned with. >>> This is >>> already how it is used in the kernel today. >> >> Strictly speaking, the cells are purely for the interpretation of the >> phandle they are associated with. If some controller wants to have 20 >> cells, then it could assuming a good reason. The reality is we tend to >> align the meaning of the cells. If genpd is interpreting the cells and >> not letting the driver for the power domain controller interpret them, >> then still, genpd needs to be fixed. > > Ok, perhaps the genpd folks on the thread can jump in here with any > thoughts that they have. > >> >> IIRC, initially it was said genpd required 0 cells, hence my confusion. >> >>> My ti,sci-id is not an index into a list of power domains, so it >>> should not >>> go in the power-domains cells and go against what the power-domains >>> binding >>> says that the cell expects. We have one single power domain, and the new >>> ti,sci-id binding is not something the genpd framework itself is >>> concerned >>> with as it's our property to identify a device inside a power domain, >>> not to >>> identify which power domain it is associated with. >> >> What is the id used for? I can understand why you need to know what >> power domain a device is in (as power-domains identifies), but not >> what devices are in a power domain. > > We have a system control processor that provides power management > services to the OS and it responsible for handling the power state of > each device. This control happens over a communication interface we have > called TI SCI (implemented at drivers/firmware/ti-sci.c). The > communication protocol uses these ids to identify each device within the > power domain so that the control processor can do what is necessary to > enable that device. I think a minor detail here that Rob might be missing right now is, that the ti,sci-id is only controlling the PM runtime handling, and providing the ID per-device for this purpose only. AFAIK, it is not really connected to the power domain anymore as such, as we don't have power-domains / per device anymore as was the case in some earlier revision of this work. One could argue though that the whole usage of power-domains is now moot, as we basically only have implemented one genpd in the whole SoC, which doesn't really reflect the reality. I wonder if better approach would be to have this replaced with proper power domains at some point (if needed), and just have a runtime-pm implementation in place for the devices that require it. So, as an example in DT, we would only have: uart0: serial@02530c00 { compatible = "xyz"; ... ti,sci-id = <K2G_DEV_UART0>; }; This is somewhat analogous to what OMAP family of SoCs have in place now, under "ti,hwmods" property. I also wonder if the "ti,sci-id" should be replaced with something like "ti,sci-dev-id" to make its purpose clearer. -Tero > > Regards, > Dave > >> >> Rob >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 17, 2017 at 1:48 AM, Tero Kristo <t-kristo@ti.com> wrote: > On 17/01/17 00:12, Dave Gerlach wrote: >> >> On 01/13/2017 08:40 PM, Rob Herring wrote: >>> >>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>> >>>> On 01/13/2017 01:25 PM, Rob Herring wrote: >>>>> >>>>> >>>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>>>> >>>>>> >>>>>> Rob, >>>>>> >>>>>> On 01/11/2017 03:34 PM, Rob Herring wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Rob, >>>>>>>> >>>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>>>>>> will hook into the genpd framework and allow the TI SCI >>>>>>>>>> protocol to >>>>>>>>>> control device power states. >>>>>>>>>> >>>>>>>>>> Also, provide macros representing each device index as understood >>>>>>>>>> by TI SCI to be used in the device node power-domain references. >>>>>>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>>>>>> --- >>>>>>>>>> v2->v3: >>>>>>>>>> Update k2g_pds node docs to show it should be a child >>>>>>>>>> of pmmc >>>>>>>>>> node. >>>>>>>>>> In early versions a phandle was used to point to pmmc and >>>>>>>>>> docs >>>>>>>>>> still >>>>>>>>>> incorrectly showed this. >>>>>>>>>> >>>>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 >>>>>>>>>> ++++++++++++++ >>>>>>>>>> MAINTAINERS | 2 + >>>>>>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>>>>>> ++++++++++++++++++++++ >>>>>>>>>> 3 files changed, 151 insertions(+) >>>>>>>>>> create mode 100644 >>>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>>>>>> >>>>>>>>>> diff --git >>>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> new file mode 100644 >>>>>>>>>> index 000000000000..4c9064e512cb >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> @@ -0,0 +1,59 @@ >>>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>>>>>> +--------------------------------------------- >>>>>>>>>> + >>>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) >>>>>>>>>> that >>>>>>>>>> is >>>>>>>>>> +responsible for controlling the state of the IPs that are >>>>>>>>>> present. >>>>>>>>>> +Communication between the host processor running an OS and the >>>>>>>>>> system >>>>>>>>>> +controller happens through a protocol known as TI-SCI [1]. >>>>>>>>>> This pm >>>>>>>>>> domain >>>>>>>>>> +implementation plugs into the generic pm domain framework and >>>>>>>>>> makes >>>>>>>>>> use >>>>>>>>>> of >>>>>>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>>>>>> + >>>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>>>>>> + >>>>>>>>>> +PM Domain Node >>>>>>>>>> +============== >>>>>>>>>> +The PM domain node represents the global PM domain managed by the >>>>>>>>>> PMMC, >>>>>>>>>> +which in this case is the single implementation as documented >>>>>>>>>> by the >>>>>>>>>> generic >>>>>>>>>> +PM domain bindings in >>>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>>>>>> +Because this relies on the TI SCI protocol to communicate with >>>>>>>>>> the >>>>>>>>>> PMMC >>>>>>>>>> it >>>>>>>>>> +must be a child of the pmmc node. >>>>>>>>>> + >>>>>>>>>> +Required Properties: >>>>>>>>>> +-------------------- >>>>>>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>>>>>> +- #power-domain-cells: Must be 0. >>>>>>>>>> + >>>>>>>>>> +Example (K2G): >>>>>>>>>> +------------- >>>>>>>>>> + pmmc: pmmc { >>>>>>>>>> + compatible = "ti,k2g-sci"; >>>>>>>>>> + ... >>>>>>>>>> + >>>>>>>>>> + k2g_pds: k2g_pds { >>>>>>>>>> + compatible = "ti,sci-pm-domain"; >>>>>>>>>> + #power-domain-cells = <0>; >>>>>>>>>> + }; >>>>>>>>>> + }; >>>>>>>>>> + >>>>>>>>>> +PM Domain Consumers >>>>>>>>>> +=================== >>>>>>>>>> +Hardware blocks that require SCI control over their state must >>>>>>>>>> provide >>>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique >>>>>>>>>> device >>>>>>>>>> +specific ID that identifies the device. >>>>>>>>>> + >>>>>>>>>> +Required Properties: >>>>>>>>>> +-------------------- >>>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain >>>>>>>>>> node. >>>>>>>>>> +- ti,sci-id: index representing the device id to be passed >>>>>>>>>> oevr SCI >>>>>>>>>> to >>>>>>>>>> + be used for device control. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> As I've already stated before, this goes in power-domain cells. >>>>>>>>> When >>>>>>>>> you >>>>>>>>> have a single thing (i.e. node) that controls multiple things, then >>>>>>>>> you >>>>>>>>> you need to specify the ID for each of them in phandle args. >>>>>>>>> This is >>>>>>>>> how >>>>>>>>> irqs, gpio, clocks, *everything* in DT works. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> You think the reasoning for doing it this way provided by both >>>>>>>> Ulf and >>>>>>>> myself on v2 [1] is not valid then? >>>>>>>> >>>>>>>> From Ulf: >>>>>>>> >>>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another >>>>>>>> "device >>>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>>>>>> in DT and assign to a device node. The only difference here, is that >>>>>>>> we don't have common API to fetch the resource (like clk_get(), >>>>>>>> regulator_get()), but instead we fetches the device's resource from >>>>>>>> SoC specific code, via genpd's device ->attach() callback. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to >>>>>>> do with DT bindings. >>>>>>> >>>>>>>> From me: >>>>>>>> >>>>>>>> Yes, you've pretty much hit it on the head. It is not an index >>>>>>>> into a >>>>>>>> list >>>>>>>> of genpds but rather identifies the device *within* a single >>>>>>>> genpd. It >>>>>>>> is >>>>>>>> a >>>>>>>> property specific to each device that resides in a ti-sci-genpd, >>>>>>>> not a >>>>>>>> mapping describing which genpd the device belongs to. The generic >>>>>>>> power >>>>>>>> domain binding is concerned with mapping the device to a specific >>>>>>>> genpd, >>>>>>>> which is does fine for us, but we have a sub mapping for devices >>>>>>>> that >>>>>>>> exist >>>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>>>>>> >>>>>>>> >>>>>>>> So to summarize, the genpd framework does interpret the phandle >>>>>>>> arg as >>>>>>>> an >>>>>>>> index into multiple genpds, just as you've said other frameworks do, >>>>>>>> but >>>>>>>> this is not what I am trying to do, we have multiple devices within >>>>>>>> this >>>>>>>> *single* genpd, hence the need for the ti,sci-id property. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Fix the genpd framework rather than work around it in DT. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> I still disagree that this has nothing to do with DT bindings, as the >>>>>> current DT binding represents something different already. I am >>>>>> trying to >>>>>> extend it to give me additional information needed for our >>>>>> platforms. Are >>>>>> you saying that we should break what the current DT binding already >>>>>> represents to mean something else? >>>>> >>>>> >>>>> >>>>> No idea because what's the current binding? From the patch, looks like >>>>> a new binding to me. >>>> >>>> >>>> >>>> Yes, ti,sci-id is a new binding. I am referring to the current >>>> meaning of >>>> the "power-domains" binding, which is where you are asking this >>>> property to >>>> be added, in "power-domains" cells. This is documented here [1] in the >>>> kernel, although looking at it I must admit it is not very clear. >>>> >>>> The power-domains cell represents an offset into an array of power >>>> domains, >>>> if you choose to use it. That's what the genpd framework is hard >>>> coded to >>>> interpret it as. This is correct, as it is an index into a static >>>> list of >>>> power domains, used to identify which power domain a device belongs to, >>>> which is exactly what the genpd framework itself is concerned with. >>>> This is >>>> already how it is used in the kernel today. >>> >>> >>> Strictly speaking, the cells are purely for the interpretation of the >>> phandle they are associated with. If some controller wants to have 20 >>> cells, then it could assuming a good reason. The reality is we tend to >>> align the meaning of the cells. If genpd is interpreting the cells and >>> not letting the driver for the power domain controller interpret them, >>> then still, genpd needs to be fixed. >> >> >> Ok, perhaps the genpd folks on the thread can jump in here with any >> thoughts that they have. >> >>> >>> IIRC, initially it was said genpd required 0 cells, hence my confusion. >>> >>>> My ti,sci-id is not an index into a list of power domains, so it >>>> should not >>>> go in the power-domains cells and go against what the power-domains >>>> binding >>>> says that the cell expects. We have one single power domain, and the new >>>> ti,sci-id binding is not something the genpd framework itself is >>>> concerned >>>> with as it's our property to identify a device inside a power domain, >>>> not to >>>> identify which power domain it is associated with. >>> >>> >>> What is the id used for? I can understand why you need to know what >>> power domain a device is in (as power-domains identifies), but not >>> what devices are in a power domain. >> >> >> We have a system control processor that provides power management >> services to the OS and it responsible for handling the power state of >> each device. This control happens over a communication interface we have >> called TI SCI (implemented at drivers/firmware/ti-sci.c). The >> communication protocol uses these ids to identify each device within the >> power domain so that the control processor can do what is necessary to >> enable that device. > > > I think a minor detail here that Rob might be missing right now is, that the > ti,sci-id is only controlling the PM runtime handling, and providing the ID > per-device for this purpose only. AFAIK, it is not really connected to the > power domain anymore as such, as we don't have power-domains / per device > anymore as was the case in some earlier revision of this work. So you used to have multiple power domains and now you don't? Did the h/w change? > One could argue though that the whole usage of power-domains is now moot, as > we basically only have implemented one genpd in the whole SoC, which doesn't > really reflect the reality. I wonder if better approach would be to have > this replaced with proper power domains at some point (if needed), and just > have a runtime-pm implementation in place for the devices that require it. We're talking about bindings here. Any explanation in terms of runtime-pm and genpd a) goes over my head and b) isn't relevant to describing the hardware. I'm still confused with how many power domains (as defined by the h/w design) there are controlled by the SCI? Is it 1 or multiple? If 1, then why do you need the sci-id? For other things that are not the power domain? Or perhaps the SCI abstracts things such that you don't really know what the relationship between devices and power domains is? IOW, you can't tell from the SCI interface how many power domains there are. > > So, as an example in DT, we would only have: > > uart0: serial@02530c00 { > compatible = "xyz"; > ... > ti,sci-id = <K2G_DEV_UART0>; > }; > > This is somewhat analogous to what OMAP family of SoCs have in place now, > under "ti,hwmods" property. I also wonder if the "ti,sci-id" should be > replaced with something like "ti,sci-dev-id" to make its purpose clearer. Describing in terms of hwmods doesn't help me either. Never understood that either. Sorry. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo <t-kristo@ti.com> writes: > On 17/01/17 00:12, Dave Gerlach wrote: >> On 01/13/2017 08:40 PM, Rob Herring wrote: >>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>> On 01/13/2017 01:25 PM, Rob Herring wrote: >>>>> >>>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >>>>>> >>>>>> Rob, >>>>>> >>>>>> On 01/11/2017 03:34 PM, Rob Herring wrote: >>>>>>> >>>>>>> >>>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> Rob, >>>>>>>> >>>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>>>>>> will hook into the genpd framework and allow the TI SCI >>>>>>>>>> protocol to >>>>>>>>>> control device power states. >>>>>>>>>> >>>>>>>>>> Also, provide macros representing each device index as understood >>>>>>>>>> by TI SCI to be used in the device node power-domain references. >>>>>>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>>>>>>>> --- >>>>>>>>>> v2->v3: >>>>>>>>>> Update k2g_pds node docs to show it should be a child >>>>>>>>>> of pmmc >>>>>>>>>> node. >>>>>>>>>> In early versions a phandle was used to point to pmmc and >>>>>>>>>> docs >>>>>>>>>> still >>>>>>>>>> incorrectly showed this. >>>>>>>>>> >>>>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 >>>>>>>>>> ++++++++++++++ >>>>>>>>>> MAINTAINERS | 2 + >>>>>>>>>> include/dt-bindings/genpd/k2g.h | 90 >>>>>>>>>> ++++++++++++++++++++++ >>>>>>>>>> 3 files changed, 151 insertions(+) >>>>>>>>>> create mode 100644 >>>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>>>>>> >>>>>>>>>> diff --git >>>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> new file mode 100644 >>>>>>>>>> index 000000000000..4c9064e512cb >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>>>>>> @@ -0,0 +1,59 @@ >>>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>>>>>> +--------------------------------------------- >>>>>>>>>> + >>>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) >>>>>>>>>> that >>>>>>>>>> is >>>>>>>>>> +responsible for controlling the state of the IPs that are >>>>>>>>>> present. >>>>>>>>>> +Communication between the host processor running an OS and the >>>>>>>>>> system >>>>>>>>>> +controller happens through a protocol known as TI-SCI [1]. >>>>>>>>>> This pm >>>>>>>>>> domain >>>>>>>>>> +implementation plugs into the generic pm domain framework and >>>>>>>>>> makes >>>>>>>>>> use >>>>>>>>>> of >>>>>>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>>>>>> + >>>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>>>>>> + >>>>>>>>>> +PM Domain Node >>>>>>>>>> +============== >>>>>>>>>> +The PM domain node represents the global PM domain managed by the >>>>>>>>>> PMMC, >>>>>>>>>> +which in this case is the single implementation as documented >>>>>>>>>> by the >>>>>>>>>> generic >>>>>>>>>> +PM domain bindings in >>>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt. >>>>>>>>>> +Because this relies on the TI SCI protocol to communicate with >>>>>>>>>> the >>>>>>>>>> PMMC >>>>>>>>>> it >>>>>>>>>> +must be a child of the pmmc node. >>>>>>>>>> + >>>>>>>>>> +Required Properties: >>>>>>>>>> +-------------------- >>>>>>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>>>>>> +- #power-domain-cells: Must be 0. >>>>>>>>>> + >>>>>>>>>> +Example (K2G): >>>>>>>>>> +------------- >>>>>>>>>> + pmmc: pmmc { >>>>>>>>>> + compatible = "ti,k2g-sci"; >>>>>>>>>> + ... >>>>>>>>>> + >>>>>>>>>> + k2g_pds: k2g_pds { >>>>>>>>>> + compatible = "ti,sci-pm-domain"; >>>>>>>>>> + #power-domain-cells = <0>; >>>>>>>>>> + }; >>>>>>>>>> + }; >>>>>>>>>> + >>>>>>>>>> +PM Domain Consumers >>>>>>>>>> +=================== >>>>>>>>>> +Hardware blocks that require SCI control over their state must >>>>>>>>>> provide >>>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique >>>>>>>>>> device >>>>>>>>>> +specific ID that identifies the device. >>>>>>>>>> + >>>>>>>>>> +Required Properties: >>>>>>>>>> +-------------------- >>>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain >>>>>>>>>> node. >>>>>>>>>> +- ti,sci-id: index representing the device id to be passed >>>>>>>>>> oevr SCI >>>>>>>>>> to >>>>>>>>>> + be used for device control. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> As I've already stated before, this goes in power-domain cells. >>>>>>>>> When >>>>>>>>> you >>>>>>>>> have a single thing (i.e. node) that controls multiple things, then >>>>>>>>> you >>>>>>>>> you need to specify the ID for each of them in phandle args. >>>>>>>>> This is >>>>>>>>> how >>>>>>>>> irqs, gpio, clocks, *everything* in DT works. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> You think the reasoning for doing it this way provided by both >>>>>>>> Ulf and >>>>>>>> myself on v2 [1] is not valid then? >>>>>>>> >>>>>>>> From Ulf: >>>>>>>> >>>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another >>>>>>>> "device >>>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe >>>>>>>> in DT and assign to a device node. The only difference here, is that >>>>>>>> we don't have common API to fetch the resource (like clk_get(), >>>>>>>> regulator_get()), but instead we fetches the device's resource from >>>>>>>> SoC specific code, via genpd's device ->attach() callback. >>>>>>> >>>>>>> >>>>>>> >>>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to >>>>>>> do with DT bindings. >>>>>>> >>>>>>>> From me: >>>>>>>> >>>>>>>> Yes, you've pretty much hit it on the head. It is not an index >>>>>>>> into a >>>>>>>> list >>>>>>>> of genpds but rather identifies the device *within* a single >>>>>>>> genpd. It >>>>>>>> is >>>>>>>> a >>>>>>>> property specific to each device that resides in a ti-sci-genpd, >>>>>>>> not a >>>>>>>> mapping describing which genpd the device belongs to. The generic >>>>>>>> power >>>>>>>> domain binding is concerned with mapping the device to a specific >>>>>>>> genpd, >>>>>>>> which is does fine for us, but we have a sub mapping for devices >>>>>>>> that >>>>>>>> exist >>>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id. >>>>>>>> >>>>>>>> >>>>>>>> So to summarize, the genpd framework does interpret the phandle >>>>>>>> arg as >>>>>>>> an >>>>>>>> index into multiple genpds, just as you've said other frameworks do, >>>>>>>> but >>>>>>>> this is not what I am trying to do, we have multiple devices within >>>>>>>> this >>>>>>>> *single* genpd, hence the need for the ti,sci-id property. >>>>>>> >>>>>>> >>>>>>> >>>>>>> Fix the genpd framework rather than work around it in DT. >>>>>> >>>>>> >>>>>> >>>>>> I still disagree that this has nothing to do with DT bindings, as the >>>>>> current DT binding represents something different already. I am >>>>>> trying to >>>>>> extend it to give me additional information needed for our >>>>>> platforms. Are >>>>>> you saying that we should break what the current DT binding already >>>>>> represents to mean something else? >>>>> >>>>> >>>>> No idea because what's the current binding? From the patch, looks like >>>>> a new binding to me. >>>> >>>> >>>> Yes, ti,sci-id is a new binding. I am referring to the current >>>> meaning of >>>> the "power-domains" binding, which is where you are asking this >>>> property to >>>> be added, in "power-domains" cells. This is documented here [1] in the >>>> kernel, although looking at it I must admit it is not very clear. >>>> >>>> The power-domains cell represents an offset into an array of power >>>> domains, >>>> if you choose to use it. That's what the genpd framework is hard >>>> coded to >>>> interpret it as. This is correct, as it is an index into a static >>>> list of >>>> power domains, used to identify which power domain a device belongs to, >>>> which is exactly what the genpd framework itself is concerned with. >>>> This is >>>> already how it is used in the kernel today. >>> >>> Strictly speaking, the cells are purely for the interpretation of the >>> phandle they are associated with. If some controller wants to have 20 >>> cells, then it could assuming a good reason. The reality is we tend to >>> align the meaning of the cells. If genpd is interpreting the cells and >>> not letting the driver for the power domain controller interpret them, >>> then still, genpd needs to be fixed. >> >> Ok, perhaps the genpd folks on the thread can jump in here with any >> thoughts that they have. >> >>> >>> IIRC, initially it was said genpd required 0 cells, hence my confusion. >>> >>>> My ti,sci-id is not an index into a list of power domains, so it >>>> should not >>>> go in the power-domains cells and go against what the power-domains >>>> binding >>>> says that the cell expects. We have one single power domain, and the new >>>> ti,sci-id binding is not something the genpd framework itself is >>>> concerned >>>> with as it's our property to identify a device inside a power domain, >>>> not to >>>> identify which power domain it is associated with. >>> >>> What is the id used for? I can understand why you need to know what >>> power domain a device is in (as power-domains identifies), but not >>> what devices are in a power domain. >> >> We have a system control processor that provides power management >> services to the OS and it responsible for handling the power state of >> each device. This control happens over a communication interface we have >> called TI SCI (implemented at drivers/firmware/ti-sci.c). The >> communication protocol uses these ids to identify each device within the >> power domain so that the control processor can do what is necessary to >> enable that device. > > I think a minor detail here that Rob might be missing right now is, > that the ti,sci-id is only controlling the PM runtime handling, and > providing the ID per-device for this purpose only. AFAIK, it is not > really connected to the power domain anymore as such, as we don't have > power-domains / per device anymore as was the case in some earlier > revision of this work. I think this gets to the heart of things. IMO The confusion arises because we're throwing around the term "power domain" when there isn't an actual hardware power domain here. Unfortunately, the genpd bindings have used the terminology power-domain when in fact genpd is more generic than that and can be used not just for hardware power domains, but for arbitrary grouping of devices that have common PM properties. That's why genpd actually stands for generic PM domain, not power domain. Unfortunately, the bindings have grown primarily out of the usage for hardware power domains. > One could argue though that the whole usage of power-domains is now > moot, as we basically only have implemented one genpd in the whole > SoC, which doesn't really reflect the reality. I wonder if better > approach would be to have this replaced with proper power domains at > some point (if needed), and just have a runtime-pm implementation in > place for the devices that require it. > > So, as an example in DT, we would only have: > > uart0: serial@02530c00 { > compatible = "xyz"; > ... > ti,sci-id = <K2G_DEV_UART0>; > }; > > This is somewhat analogous to what OMAP family of SoCs have in place > now, under "ti,hwmods" property. I also wonder if the "ti,sci-id" > should be replaced with something like "ti,sci-dev-id" to make its > purpose clearer. Unless I'm missing something, that still begs the question of who reads that property and takes care of the call into TI-SCI though. Kevin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote: > Tero Kristo <t-kristo@ti.com> writes: >> On 17/01/17 00:12, Dave Gerlach wrote: >>> On 01/13/2017 08:40 PM, Rob Herring wrote: >>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: [...] >>>>> My ti,sci-id is not an index into a list of power domains, so it >>>>> should not >>>>> go in the power-domains cells and go against what the power-domains >>>>> binding >>>>> says that the cell expects. We have one single power domain, and the new >>>>> ti,sci-id binding is not something the genpd framework itself is >>>>> concerned >>>>> with as it's our property to identify a device inside a power domain, >>>>> not to >>>>> identify which power domain it is associated with. >>>> >>>> What is the id used for? I can understand why you need to know what >>>> power domain a device is in (as power-domains identifies), but not >>>> what devices are in a power domain. >>> >>> We have a system control processor that provides power management >>> services to the OS and it responsible for handling the power state of >>> each device. This control happens over a communication interface we have >>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The >>> communication protocol uses these ids to identify each device within the >>> power domain so that the control processor can do what is necessary to >>> enable that device. >> >> I think a minor detail here that Rob might be missing right now is, >> that the ti,sci-id is only controlling the PM runtime handling, and >> providing the ID per-device for this purpose only. AFAIK, it is not >> really connected to the power domain anymore as such, as we don't have >> power-domains / per device anymore as was the case in some earlier >> revision of this work. > > I think this gets to the heart of things. IMO The confusion arises > because we're throwing around the term "power domain" when there isn't > an actual hardware power domain here. I thought there was 1. > Unfortunately, the genpd bindings have used the terminology power-domain > when in fact genpd is more generic than that and can be used not just > for hardware power domains, but for arbitrary grouping of devices that > have common PM properties. That's why genpd actually stands for generic > PM domain, not power domain. Unfortunately, the bindings have grown > primarily out of the usage for hardware power domains. Now it makes some sense. So the question is does this PM domain grouping need to be described in DT or not, and if so what does that look like? We could continue to use the power domain binding (maybe we already are and that ship has sailed). I'm not totally against the idea even if there is no power domain, but I'm not sold on it either. If we do go this route, then I still say the id should be a cell in the power-domain phandle. Another option is create something new either common or TI SCI specific. It could be just a table of ids and phandles in the SCI node. I'm much more comfortable with an isolated property in one node than something scattered throughout the DT. >> One could argue though that the whole usage of power-domains is now >> moot, as we basically only have implemented one genpd in the whole >> SoC, which doesn't really reflect the reality. I wonder if better >> approach would be to have this replaced with proper power domains at >> some point (if needed), and just have a runtime-pm implementation in >> place for the devices that require it. >> >> So, as an example in DT, we would only have: >> >> uart0: serial@02530c00 { >> compatible = "xyz"; >> ... >> ti,sci-id = <K2G_DEV_UART0>; >> }; >> >> This is somewhat analogous to what OMAP family of SoCs have in place >> now, under "ti,hwmods" property. I also wonder if the "ti,sci-id" >> should be replaced with something like "ti,sci-dev-id" to make its >> purpose clearer. > > Unless I'm missing something, that still begs the question of who reads > that property and takes care of the call into TI-SCI though. > > Kevin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+ Sudeep On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote: > On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote: >> Tero Kristo <t-kristo@ti.com> writes: >>> On 17/01/17 00:12, Dave Gerlach wrote: >>>> On 01/13/2017 08:40 PM, Rob Herring wrote: >>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: > > [...] > >>>>>> My ti,sci-id is not an index into a list of power domains, so it >>>>>> should not >>>>>> go in the power-domains cells and go against what the power-domains >>>>>> binding >>>>>> says that the cell expects. We have one single power domain, and the new >>>>>> ti,sci-id binding is not something the genpd framework itself is >>>>>> concerned >>>>>> with as it's our property to identify a device inside a power domain, >>>>>> not to >>>>>> identify which power domain it is associated with. >>>>> >>>>> What is the id used for? I can understand why you need to know what >>>>> power domain a device is in (as power-domains identifies), but not >>>>> what devices are in a power domain. >>>> >>>> We have a system control processor that provides power management >>>> services to the OS and it responsible for handling the power state of >>>> each device. This control happens over a communication interface we have >>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The >>>> communication protocol uses these ids to identify each device within the >>>> power domain so that the control processor can do what is necessary to >>>> enable that device. >>> >>> I think a minor detail here that Rob might be missing right now is, >>> that the ti,sci-id is only controlling the PM runtime handling, and >>> providing the ID per-device for this purpose only. AFAIK, it is not >>> really connected to the power domain anymore as such, as we don't have >>> power-domains / per device anymore as was the case in some earlier >>> revision of this work. >> >> I think this gets to the heart of things. IMO The confusion arises >> because we're throwing around the term "power domain" when there isn't >> an actual hardware power domain here. > > I thought there was 1. > >> Unfortunately, the genpd bindings have used the terminology power-domain >> when in fact genpd is more generic than that and can be used not just >> for hardware power domains, but for arbitrary grouping of devices that >> have common PM properties. That's why genpd actually stands for generic >> PM domain, not power domain. Unfortunately, the bindings have grown >> primarily out of the usage for hardware power domains. > > Now it makes some sense. > > So the question is does this PM domain grouping need to be described > in DT or not, and if so what does that look like? Yes, it's needed and already being done. For example, we have clock domains for several Renesas platforms. > > We could continue to use the power domain binding (maybe we already > are and that ship has sailed). I'm not totally against the idea even > if there is no power domain, but I'm not sold on it either. If we do > go this route, then I still say the id should be a cell in the > power-domain phandle. > > Another option is create something new either common or TI SCI > specific. It could be just a table of ids and phandles in the SCI > node. I'm much more comfortable with an isolated property in one node > than something scattered throughout the DT. To me, this seems like the best possible solution. However, perhaps we should also consider the SCPI Generic power domain (drivers/firmware/scpi_pm_domain.c), because I believe it's closely related. To change the power state of a device, this PM domain calls scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which also needs a device id as a parameter. Very similar to our case with the TI SCI domain. Currently these SCPI device ids lacks corresponding DT bindings, so the scpi_pm_domain tries to work around it by assigning ids dynamically at genpd creation time. That makes me wonder, whether we should think of something common/generic? [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/01/17 14:00, Ulf Hansson wrote: > + Sudeep > > On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote: >> >> We could continue to use the power domain binding (maybe we already >> are and that ship has sailed). I'm not totally against the idea even >> if there is no power domain, but I'm not sold on it either. If we do >> go this route, then I still say the id should be a cell in the >> power-domain phandle. >> >> Another option is create something new either common or TI SCI >> specific. It could be just a table of ids and phandles in the SCI >> node. I'm much more comfortable with an isolated property in one node >> than something scattered throughout the DT. > > To me, this seems like the best possible solution. > > However, perhaps we should also consider the SCPI Generic power domain > (drivers/firmware/scpi_pm_domain.c), because I believe it's closely > related. > To change the power state of a device, this PM domain calls > scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which > also needs a device id as a parameter. Very similar to our case with > the TI SCI domain. > > Currently these SCPI device ids lacks corresponding DT bindings, so > the scpi_pm_domain tries to work around it by assigning ids > dynamically at genpd creation time. > IIUC do you mean the binding for the power domain provider to have a list of domain ids ? If so yes, we don't have one. But the idea was to have the range to be continuous and create genpd for the complete range. Though the SCPI specification lacked a command to get the max. no. of domains supported. That's the reason we had to introduce the num-domains(*) which may be optional if we have firmware interface to obtain that information.
On 01/20/2017 08:00 AM, Ulf Hansson wrote: > + Sudeep > > On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote: >> On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote: >>> Tero Kristo <t-kristo@ti.com> writes: >>>> On 17/01/17 00:12, Dave Gerlach wrote: >>>>> On 01/13/2017 08:40 PM, Rob Herring wrote: >>>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote: >> >> [...] >> >>>>>>> My ti,sci-id is not an index into a list of power domains, so it >>>>>>> should not >>>>>>> go in the power-domains cells and go against what the power-domains >>>>>>> binding >>>>>>> says that the cell expects. We have one single power domain, and the new >>>>>>> ti,sci-id binding is not something the genpd framework itself is >>>>>>> concerned >>>>>>> with as it's our property to identify a device inside a power domain, >>>>>>> not to >>>>>>> identify which power domain it is associated with. >>>>>> >>>>>> What is the id used for? I can understand why you need to know what >>>>>> power domain a device is in (as power-domains identifies), but not >>>>>> what devices are in a power domain. >>>>> >>>>> We have a system control processor that provides power management >>>>> services to the OS and it responsible for handling the power state of >>>>> each device. This control happens over a communication interface we have >>>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The >>>>> communication protocol uses these ids to identify each device within the >>>>> power domain so that the control processor can do what is necessary to >>>>> enable that device. >>>> >>>> I think a minor detail here that Rob might be missing right now is, >>>> that the ti,sci-id is only controlling the PM runtime handling, and >>>> providing the ID per-device for this purpose only. AFAIK, it is not >>>> really connected to the power domain anymore as such, as we don't have >>>> power-domains / per device anymore as was the case in some earlier >>>> revision of this work. >>> >>> I think this gets to the heart of things. IMO The confusion arises >>> because we're throwing around the term "power domain" when there isn't >>> an actual hardware power domain here. >> >> I thought there was 1. >> >>> Unfortunately, the genpd bindings have used the terminology power-domain >>> when in fact genpd is more generic than that and can be used not just >>> for hardware power domains, but for arbitrary grouping of devices that >>> have common PM properties. That's why genpd actually stands for generic >>> PM domain, not power domain. Unfortunately, the bindings have grown >>> primarily out of the usage for hardware power domains. >> >> Now it makes some sense. >> >> So the question is does this PM domain grouping need to be described >> in DT or not, and if so what does that look like? > > Yes, it's needed and already being done. For example, we have clock > domains for several Renesas platforms. > >> >> We could continue to use the power domain binding (maybe we already >> are and that ship has sailed). I'm not totally against the idea even >> if there is no power domain, but I'm not sold on it either. If we do >> go this route, then I still say the id should be a cell in the >> power-domain phandle. >> >> Another option is create something new either common or TI SCI >> specific. It could be just a table of ids and phandles in the SCI >> node. I'm much more comfortable with an isolated property in one node >> than something scattered throughout the DT. > > To me, this seems like the best possible solution. > > However, perhaps we should also consider the SCPI Generic power domain > (drivers/firmware/scpi_pm_domain.c), because I believe it's closely > related. > To change the power state of a device, this PM domain calls > scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which > also needs a device id as a parameter. Very similar to our case with > the TI SCI domain. > > Currently these SCPI device ids lacks corresponding DT bindings, so > the scpi_pm_domain tries to work around it by assigning ids > dynamically at genpd creation time. > > That makes me wonder, whether we should think of something common/generic? When you say something common/generic, do you mean a better binding for genpd, or something bigger than that like a new driver? Because I do think a phandle cell left open for the genpd provider to interpret solves both the scpi and ti-sci problem we are facing here in the best way. Using generic PM domains lets us do exactly what we want apart from interpreting the phandle cell with our driver, and I feel like anything else we try at this point is just going to be to work around that. Is bringing back genpd xlate something we can discuss? Regards, Dave > > [...] > > Kind regards > Uffe > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >>> Another option is create something new either common or TI SCI >>> specific. It could be just a table of ids and phandles in the SCI >>> node. I'm much more comfortable with an isolated property in one node >>> than something scattered throughout the DT. >> >> To me, this seems like the best possible solution. >> >> However, perhaps we should also consider the SCPI Generic power domain >> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >> related. >> To change the power state of a device, this PM domain calls >> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >> also needs a device id as a parameter. Very similar to our case with >> the TI SCI domain. >> >> Currently these SCPI device ids lacks corresponding DT bindings, so >> the scpi_pm_domain tries to work around it by assigning ids >> dynamically at genpd creation time. >> >> That makes me wonder, whether we should think of something common/generic? > > When you say something common/generic, do you mean a better binding for genpd, > or something bigger than that like a new driver? Because I do think a phandle > cell left open for the genpd provider to interpret solves both the scpi and > ti-sci problem we are facing here in the best way. Using generic PM domains lets > us do exactly what we want apart from interpreting the phandle cell with our > driver, and I feel like anything else we try at this point is just going to be > to work around that. Is bringing back genpd xlate something we can discuss? Bringing back xlate, how would that help? Wouldn't that just mean that you will get one genpd per device? That's not an option, I think we are all in agreement to that. Or am I missing something here? Regarding something "common/generic", I was merely thinking of some common bindings describing the list of phandle to device ids mapping. However, let's not make this more complicated than it is already. So please just ignore my suggestion so we can make this work for your case first. However, maybe I didn't fully understood Rob's suggestion. Perhaps Rob can clarify once more. Anyway, this is how interpreted Rob's suggestion: TISCI_PM_DOMAIN_DEVLIST: tisci-pm-domain-devlist { devs = <&serial0>, <&mmc0>; devids = <5 10>; } With this, you should be to extract the devid which corresponds to the device that has been attached to the TI SCI PM domain. Don't you think? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/20/2017 10:52 AM, Ulf Hansson wrote: > [...] > >>>> Another option is create something new either common or TI SCI >>>> specific. It could be just a table of ids and phandles in the SCI >>>> node. I'm much more comfortable with an isolated property in one node >>>> than something scattered throughout the DT. >>> >>> To me, this seems like the best possible solution. >>> >>> However, perhaps we should also consider the SCPI Generic power domain >>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >>> related. >>> To change the power state of a device, this PM domain calls >>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >>> also needs a device id as a parameter. Very similar to our case with >>> the TI SCI domain. >>> >>> Currently these SCPI device ids lacks corresponding DT bindings, so >>> the scpi_pm_domain tries to work around it by assigning ids >>> dynamically at genpd creation time. >>> >>> That makes me wonder, whether we should think of something common/generic? >> >> When you say something common/generic, do you mean a better binding for genpd, >> or something bigger than that like a new driver? Because I do think a phandle >> cell left open for the genpd provider to interpret solves both the scpi and >> ti-sci problem we are facing here in the best way. Using generic PM domains lets >> us do exactly what we want apart from interpreting the phandle cell with our >> driver, and I feel like anything else we try at this point is just going to be >> to work around that. Is bringing back genpd xlate something we can discuss? > > Bringing back xlate, how would that help? Wouldn't that just mean that > you will get one genpd per device? That's not an option, I think we > are all in agreement to that. Sure, perhaps the custom xlate wouldn't be the right way to do it, as we wouldn't be able to associate a device directly to a phandle, at least with how it was implemented before, but I think we can skip that entirely. Does opening up the interpretation of the cells of the 'power-domains' phandle not solve all of these issues? Is that out of the question? genpd_xlate_simple currently just makes sure the args_count of the 'power-domains' phandle was zero and bails if it was not. Why couldn't we remove this check and let the driver interpret it while still using of_genpd_add_provider_simple to register the provider? It's still a 'simple' provider from the perspective of the genpd framework and the actual pm domain mapping will not change, but now the driver can parse the cells and do whatever it needs to, such as reading a device id. I think that's a bit more flexible and will avoid breaking anything that is there today. Regards, Dave > > Or am I missing something here? > > Regarding something "common/generic", I was merely thinking of some > common bindings describing the list of phandle to device ids mapping. > However, let's not make this more complicated than it is already. So > please just ignore my suggestion so we can make this work for your > case first. > > However, maybe I didn't fully understood Rob's suggestion. Perhaps Rob > can clarify once more. > > Anyway, this is how interpreted Rob's suggestion: > > TISCI_PM_DOMAIN_DEVLIST: tisci-pm-domain-devlist { > devs = <&serial0>, <&mmc0>; > devids = <5 10>; > } > > With this, you should be to extract the devid which corresponds to the > device that has been attached to the TI SCI PM domain. Don't you > think? > > Kind regards > Uffe > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote: > On 01/20/2017 10:52 AM, Ulf Hansson wrote: >> >> [...] >> >>>>> Another option is create something new either common or TI SCI >>>>> specific. It could be just a table of ids and phandles in the SCI >>>>> node. I'm much more comfortable with an isolated property in one node >>>>> than something scattered throughout the DT. >>>> >>>> >>>> To me, this seems like the best possible solution. >>>> >>>> However, perhaps we should also consider the SCPI Generic power domain >>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >>>> related. >>>> To change the power state of a device, this PM domain calls >>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >>>> also needs a device id as a parameter. Very similar to our case with >>>> the TI SCI domain. >>>> >>>> Currently these SCPI device ids lacks corresponding DT bindings, so >>>> the scpi_pm_domain tries to work around it by assigning ids >>>> dynamically at genpd creation time. >>>> >>>> That makes me wonder, whether we should think of something >>>> common/generic? >>> >>> >>> When you say something common/generic, do you mean a better binding for >>> genpd, >>> or something bigger than that like a new driver? Because I do think a >>> phandle >>> cell left open for the genpd provider to interpret solves both the scpi >>> and >>> ti-sci problem we are facing here in the best way. Using generic PM >>> domains lets >>> us do exactly what we want apart from interpreting the phandle cell with >>> our >>> driver, and I feel like anything else we try at this point is just going >>> to be >>> to work around that. Is bringing back genpd xlate something we can >>> discuss? >> >> >> Bringing back xlate, how would that help? Wouldn't that just mean that >> you will get one genpd per device? That's not an option, I think we >> are all in agreement to that. > > > Sure, perhaps the custom xlate wouldn't be the right way to do it, as we > wouldn't be able to associate a device directly to a phandle, at least with > how it was implemented before, but I think we can skip that entirely. Does > opening up the interpretation of the cells of the 'power-domains' phandle > not solve all of these issues? Is that out of the question? > > genpd_xlate_simple currently just makes sure the args_count of the > 'power-domains' phandle was zero and bails if it was not. Why couldn't we > remove this check and let the driver interpret it while still using > of_genpd_add_provider_simple to register the provider? It's still a 'simple' > provider from the perspective of the genpd framework and the actual pm > domain mapping will not change, but now the driver can parse the cells and > do whatever it needs to, such as reading a device id. > > I think that's a bit more flexible and will avoid breaking anything that is > there today. Would you mind providing an example? Perhaps also some code snippets dealing with the parsing? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25 January 2017 at 17:59, Dave Gerlach <d-gerlach@ti.com> wrote: > On 01/24/2017 04:03 AM, Ulf Hansson wrote: >> >> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote: >>> >>> On 01/20/2017 10:52 AM, Ulf Hansson wrote: >>>> >>>> >>>> [...] >>>> >>>>>>> Another option is create something new either common or TI SCI >>>>>>> specific. It could be just a table of ids and phandles in the SCI >>>>>>> node. I'm much more comfortable with an isolated property in one node >>>>>>> than something scattered throughout the DT. >>>>>> >>>>>> >>>>>> >>>>>> To me, this seems like the best possible solution. >>>>>> >>>>>> However, perhaps we should also consider the SCPI Generic power domain >>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >>>>>> related. >>>>>> To change the power state of a device, this PM domain calls >>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >>>>>> also needs a device id as a parameter. Very similar to our case with >>>>>> the TI SCI domain. >>>>>> >>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so >>>>>> the scpi_pm_domain tries to work around it by assigning ids >>>>>> dynamically at genpd creation time. >>>>>> >>>>>> That makes me wonder, whether we should think of something >>>>>> common/generic? >>>>> >>>>> >>>>> >>>>> When you say something common/generic, do you mean a better binding for >>>>> genpd, >>>>> or something bigger than that like a new driver? Because I do think a >>>>> phandle >>>>> cell left open for the genpd provider to interpret solves both the scpi >>>>> and >>>>> ti-sci problem we are facing here in the best way. Using generic PM >>>>> domains lets >>>>> us do exactly what we want apart from interpreting the phandle cell >>>>> with >>>>> our >>>>> driver, and I feel like anything else we try at this point is just >>>>> going >>>>> to be >>>>> to work around that. Is bringing back genpd xlate something we can >>>>> discuss? >>>> >>>> >>>> >>>> Bringing back xlate, how would that help? Wouldn't that just mean that >>>> you will get one genpd per device? That's not an option, I think we >>>> are all in agreement to that. >>> >>> >>> >>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we >>> wouldn't be able to associate a device directly to a phandle, at least >>> with >>> how it was implemented before, but I think we can skip that entirely. >>> Does >>> opening up the interpretation of the cells of the 'power-domains' phandle >>> not solve all of these issues? Is that out of the question? >>> >>> genpd_xlate_simple currently just makes sure the args_count of the >>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we >>> remove this check and let the driver interpret it while still using >>> of_genpd_add_provider_simple to register the provider? It's still a >>> 'simple' >>> provider from the perspective of the genpd framework and the actual pm >>> domain mapping will not change, but now the driver can parse the cells >>> and >>> do whatever it needs to, such as reading a device id. >>> >>> I think that's a bit more flexible and will avoid breaking anything that >>> is >>> there today. >> >> >> Would you mind providing an example? Perhaps also some code snippets >> dealing with the parsing? > > > So again the goal of this is to move the ti,sci-id value back to > power-domains phandle instead of having a separate property, so that would > be step one in the DT. Then in the power-domains node change > #power-domain-cells to one. And then from there, the only change to the > genpd framework is this: > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index a5e1262b964b..b82e61f0bcfa 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple > struct of_phandle_args *genpdspec, > void *data) > { > - if (genpdspec->args_count != 0) > - return ERR_PTR(-EINVAL); > return data; > } > > > because genpd_xlate_simple only checks that the phandle is zero so that it > can fail if it is not, but there's no functional reason it needs to do this. > The genpd framework works as it did before no matter what the cells are set > to if using of_genpd_add_provider_simple. Then in the attach_dev callback > inside the ti_sci_pm_domains driver instead of doing > > ret = of_property_read_u32(np, "ti,sci-id", &idx); > > to read the ti,sc-id for a device into idx we can now do: > > ret = of_parse_phandle_with_args(np, "power-domains", > "#power-domain-cells", 0, &pd_args); > idx = pd_args.args[0]; > > or even simpler from within our driver > > ret = of_property_read_u32_index(np, "power-domains", 1, &idx); > > To read the value into idx. > > This requires minimal changes to the genpd framework and gives the option > for the driver to interpret the cell manually when using a simple provider. > The genpd framework still uses the phandle just to get the power-domain > device and the cells are left entirely up to the driver to interpret, but if > desired you could still use the genpd onecell driver for a specific use of > the phandle cell, or use it with zero cells. > > I can send out an updated series if there are no major objections, or just > to start discussion there. Ok, this seems to work! I am ready to review! :-) Of course, don't forget to update the existing DT doc for the power domain bindings. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote: > On 01/24/2017 04:03 AM, Ulf Hansson wrote: >> >> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote: >>> >>> On 01/20/2017 10:52 AM, Ulf Hansson wrote: >>>> >>>> >>>> [...] >>>> >>>>>>> Another option is create something new either common or TI SCI >>>>>>> specific. It could be just a table of ids and phandles in the SCI >>>>>>> node. I'm much more comfortable with an isolated property in one node >>>>>>> than something scattered throughout the DT. >>>>>> >>>>>> >>>>>> >>>>>> To me, this seems like the best possible solution. >>>>>> >>>>>> However, perhaps we should also consider the SCPI Generic power domain >>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >>>>>> related. >>>>>> To change the power state of a device, this PM domain calls >>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >>>>>> also needs a device id as a parameter. Very similar to our case with >>>>>> the TI SCI domain. >>>>>> >>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so >>>>>> the scpi_pm_domain tries to work around it by assigning ids >>>>>> dynamically at genpd creation time. >>>>>> >>>>>> That makes me wonder, whether we should think of something >>>>>> common/generic? >>>>> >>>>> >>>>> >>>>> When you say something common/generic, do you mean a better binding for >>>>> genpd, >>>>> or something bigger than that like a new driver? Because I do think a >>>>> phandle >>>>> cell left open for the genpd provider to interpret solves both the scpi >>>>> and >>>>> ti-sci problem we are facing here in the best way. Using generic PM >>>>> domains lets >>>>> us do exactly what we want apart from interpreting the phandle cell >>>>> with >>>>> our >>>>> driver, and I feel like anything else we try at this point is just >>>>> going >>>>> to be >>>>> to work around that. Is bringing back genpd xlate something we can >>>>> discuss? >>>> >>>> >>>> >>>> Bringing back xlate, how would that help? Wouldn't that just mean that >>>> you will get one genpd per device? That's not an option, I think we >>>> are all in agreement to that. >>> >>> >>> >>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we >>> wouldn't be able to associate a device directly to a phandle, at least >>> with >>> how it was implemented before, but I think we can skip that entirely. >>> Does >>> opening up the interpretation of the cells of the 'power-domains' phandle >>> not solve all of these issues? Is that out of the question? >>> >>> genpd_xlate_simple currently just makes sure the args_count of the >>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we >>> remove this check and let the driver interpret it while still using >>> of_genpd_add_provider_simple to register the provider? It's still a >>> 'simple' >>> provider from the perspective of the genpd framework and the actual pm >>> domain mapping will not change, but now the driver can parse the cells >>> and >>> do whatever it needs to, such as reading a device id. >>> >>> I think that's a bit more flexible and will avoid breaking anything that >>> is >>> there today. >> >> >> Would you mind providing an example? Perhaps also some code snippets >> dealing with the parsing? > > > So again the goal of this is to move the ti,sci-id value back to > power-domains phandle instead of having a separate property, so that would > be step one in the DT. Then in the power-domains node change > #power-domain-cells to one. And then from there, the only change to the > genpd framework is this: I'd still like to understand how the ID is used in order to understand if as a power-domain cell is appropriate. I think the test is this: is the ID meaningful to (or defined by) the device or the power domain controller? The former should be a property in the device node. The latter should be phandle args. > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index a5e1262b964b..b82e61f0bcfa 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple > struct of_phandle_args *genpdspec, > void *data) > { > - if (genpdspec->args_count != 0) > - return ERR_PTR(-EINVAL); > return data; > } > > > because genpd_xlate_simple only checks that the phandle is zero so that it > can fail if it is not, but there's no functional reason it needs to do this. > The genpd framework works as it did before no matter what the cells are set > to if using of_genpd_add_provider_simple. Then in the attach_dev callback > inside the ti_sci_pm_domains driver instead of doing > > ret = of_property_read_u32(np, "ti,sci-id", &idx); > > to read the ti,sc-id for a device into idx we can now do: > > ret = of_parse_phandle_with_args(np, "power-domains", > "#power-domain-cells", 0, &pd_args); > idx = pd_args.args[0]; > > or even simpler from within our driver > > ret = of_property_read_u32_index(np, "power-domains", 1, &idx); This you should not be doing. The client driver shouldn't care how many cells or what their values are. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/25/2017 04:32 PM, Rob Herring wrote: > On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote: >> On 01/24/2017 04:03 AM, Ulf Hansson wrote: >>> >>> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote: >>>> >>>> On 01/20/2017 10:52 AM, Ulf Hansson wrote: >>>>> >>>>> >>>>> [...] >>>>> >>>>>>>> Another option is create something new either common or TI SCI >>>>>>>> specific. It could be just a table of ids and phandles in the SCI >>>>>>>> node. I'm much more comfortable with an isolated property in one node >>>>>>>> than something scattered throughout the DT. >>>>>>> >>>>>>> >>>>>>> >>>>>>> To me, this seems like the best possible solution. >>>>>>> >>>>>>> However, perhaps we should also consider the SCPI Generic power domain >>>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely >>>>>>> related. >>>>>>> To change the power state of a device, this PM domain calls >>>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which >>>>>>> also needs a device id as a parameter. Very similar to our case with >>>>>>> the TI SCI domain. >>>>>>> >>>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so >>>>>>> the scpi_pm_domain tries to work around it by assigning ids >>>>>>> dynamically at genpd creation time. >>>>>>> >>>>>>> That makes me wonder, whether we should think of something >>>>>>> common/generic? >>>>>> >>>>>> >>>>>> >>>>>> When you say something common/generic, do you mean a better binding for >>>>>> genpd, >>>>>> or something bigger than that like a new driver? Because I do think a >>>>>> phandle >>>>>> cell left open for the genpd provider to interpret solves both the scpi >>>>>> and >>>>>> ti-sci problem we are facing here in the best way. Using generic PM >>>>>> domains lets >>>>>> us do exactly what we want apart from interpreting the phandle cell >>>>>> with >>>>>> our >>>>>> driver, and I feel like anything else we try at this point is just >>>>>> going >>>>>> to be >>>>>> to work around that. Is bringing back genpd xlate something we can >>>>>> discuss? >>>>> >>>>> >>>>> >>>>> Bringing back xlate, how would that help? Wouldn't that just mean that >>>>> you will get one genpd per device? That's not an option, I think we >>>>> are all in agreement to that. >>>> >>>> >>>> >>>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we >>>> wouldn't be able to associate a device directly to a phandle, at least >>>> with >>>> how it was implemented before, but I think we can skip that entirely. >>>> Does >>>> opening up the interpretation of the cells of the 'power-domains' phandle >>>> not solve all of these issues? Is that out of the question? >>>> >>>> genpd_xlate_simple currently just makes sure the args_count of the >>>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we >>>> remove this check and let the driver interpret it while still using >>>> of_genpd_add_provider_simple to register the provider? It's still a >>>> 'simple' >>>> provider from the perspective of the genpd framework and the actual pm >>>> domain mapping will not change, but now the driver can parse the cells >>>> and >>>> do whatever it needs to, such as reading a device id. >>>> >>>> I think that's a bit more flexible and will avoid breaking anything that >>>> is >>>> there today. >>> >>> >>> Would you mind providing an example? Perhaps also some code snippets >>> dealing with the parsing? >> >> >> So again the goal of this is to move the ti,sci-id value back to >> power-domains phandle instead of having a separate property, so that would >> be step one in the DT. Then in the power-domains node change >> #power-domain-cells to one. And then from there, the only change to the >> genpd framework is this: > > I'd still like to understand how the ID is used in order to understand > if as a power-domain cell is appropriate. I think the test is this: is > the ID meaningful to (or defined by) the device or the power domain > controller? The former should be a property in the device node. The > latter should be phandle args. The device itself doesn't care about the ID or ever need to know what it is. The power domain controller is the only user of the ID. The power domain controller needs the ID for a specific device to be able to turn it on, so during probe of each device the power-domain controller makes this association which is why I included it in each device node, so I think it fits as a phandle arg based on your test. > >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index a5e1262b964b..b82e61f0bcfa 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple >> struct of_phandle_args *genpdspec, >> void *data) >> { >> - if (genpdspec->args_count != 0) >> - return ERR_PTR(-EINVAL); >> return data; >> } >> >> >> because genpd_xlate_simple only checks that the phandle is zero so that it >> can fail if it is not, but there's no functional reason it needs to do this. >> The genpd framework works as it did before no matter what the cells are set >> to if using of_genpd_add_provider_simple. Then in the attach_dev callback >> inside the ti_sci_pm_domains driver instead of doing >> >> ret = of_property_read_u32(np, "ti,sci-id", &idx); >> >> to read the ti,sc-id for a device into idx we can now do: >> >> ret = of_parse_phandle_with_args(np, "power-domains", >> "#power-domain-cells", 0, &pd_args); >> idx = pd_args.args[0]; >> >> or even simpler from within our driver >> >> ret = of_property_read_u32_index(np, "power-domains", 1, &idx); > > This you should not be doing. The client driver shouldn't care how > many cells or what their values are. Client drivers in other places use xlate functions, like in the drivers/reset framework, to interpret the cells. Is doing it this way really that different? Regards, Dave > > Rob > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 26, 2017 at 9:09 AM, Dave Gerlach <d-gerlach@ti.com> wrote: > On 01/25/2017 04:32 PM, Rob Herring wrote: >> >> On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote: [...] >>> because genpd_xlate_simple only checks that the phandle is zero so that >>> it >>> can fail if it is not, but there's no functional reason it needs to do >>> this. >>> The genpd framework works as it did before no matter what the cells are >>> set >>> to if using of_genpd_add_provider_simple. Then in the attach_dev callback >>> inside the ti_sci_pm_domains driver instead of doing >>> >>> ret = of_property_read_u32(np, "ti,sci-id", &idx); >>> >>> to read the ti,sc-id for a device into idx we can now do: >>> >>> ret = of_parse_phandle_with_args(np, "power-domains", >>> "#power-domain-cells", 0, &pd_args); >>> idx = pd_args.args[0]; >>> >>> or even simpler from within our driver >>> >>> ret = of_property_read_u32_index(np, "power-domains", 1, &idx); >> >> >> This you should not be doing. The client driver shouldn't care how >> many cells or what their values are. > > > Client drivers in other places use xlate functions, like in the > drivers/reset framework, to interpret the cells. Is doing it this way really > that different? Oh, I was thinking the client driver, not the power domain controller driver. NM. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt new file mode 100644 index 000000000000..4c9064e512cb --- /dev/null +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt @@ -0,0 +1,59 @@ +Texas Instruments TI-SCI Generic Power Domain +--------------------------------------------- + +Some TI SoCs contain a system controller (like the PMMC, etc...) that is +responsible for controlling the state of the IPs that are present. +Communication between the host processor running an OS and the system +controller happens through a protocol known as TI-SCI [1]. This pm domain +implementation plugs into the generic pm domain framework and makes use of +the TI SCI protocol power on and off each device when needed. + +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt + +PM Domain Node +============== +The PM domain node represents the global PM domain managed by the PMMC, +which in this case is the single implementation as documented by the generic +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt. +Because this relies on the TI SCI protocol to communicate with the PMMC it +must be a child of the pmmc node. + +Required Properties: +-------------------- +- compatible: should be "ti,sci-pm-domain" +- #power-domain-cells: Must be 0. + +Example (K2G): +------------- + pmmc: pmmc { + compatible = "ti,k2g-sci"; + ... + + k2g_pds: k2g_pds { + compatible = "ti,sci-pm-domain"; + #power-domain-cells = <0>; + }; + }; + +PM Domain Consumers +=================== +Hardware blocks that require SCI control over their state must provide +a reference to the sci-pm-domain they are part of and a unique device +specific ID that identifies the device. + +Required Properties: +-------------------- +- power-domains: phandle pointing to the corresponding PM domain node. +- ti,sci-id: index representing the device id to be passed oevr SCI to + be used for device control. + +See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g. + +Example (K2G): +-------------------- + uart0: serial@02530c00 { + compatible = "ns16550a"; + ... + power-domains = <&k2g_pds>; + ti,sci-id = <K2G_DEV_UART0>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index cfff2c9e3d94..7e68af170dae 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12144,6 +12144,8 @@ S: Maintained F: Documentation/devicetree/bindings/arm/keystone/ti,sci.txt F: drivers/firmware/ti_sci* F: include/linux/soc/ti/ti_sci_protocol.h +F: Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt +F: include/dt-bindings/genpd/k2g.h THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER M: Hans Verkuil <hverkuil@xs4all.nl> diff --git a/include/dt-bindings/genpd/k2g.h b/include/dt-bindings/genpd/k2g.h new file mode 100644 index 000000000000..91ad827e0ca1 --- /dev/null +++ b/include/dt-bindings/genpd/k2g.h @@ -0,0 +1,90 @@ +/* + * TI K2G SoC Device definitions + * + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _DT_BINDINGS_GENPD_K2G_H +#define _DT_BINDINGS_GENPD_K2G_H + +/* Documented in http://processors.wiki.ti.com/index.php/TISCI */ + +#define K2G_DEV_PMMC0 0x0000 +#define K2G_DEV_MLB0 0x0001 +#define K2G_DEV_DSS0 0x0002 +#define K2G_DEV_MCBSP0 0x0003 +#define K2G_DEV_MCASP0 0x0004 +#define K2G_DEV_MCASP1 0x0005 +#define K2G_DEV_MCASP2 0x0006 +#define K2G_DEV_DCAN0 0x0008 +#define K2G_DEV_DCAN1 0x0009 +#define K2G_DEV_EMIF0 0x000a +#define K2G_DEV_MMCHS0 0x000b +#define K2G_DEV_MMCHS1 0x000c +#define K2G_DEV_GPMC0 0x000d +#define K2G_DEV_ELM0 0x000e +#define K2G_DEV_SPI0 0x0010 +#define K2G_DEV_SPI1 0x0011 +#define K2G_DEV_SPI2 0x0012 +#define K2G_DEV_SPI3 0x0013 +#define K2G_DEV_ICSS0 0x0014 +#define K2G_DEV_ICSS1 0x0015 +#define K2G_DEV_USB0 0x0016 +#define K2G_DEV_USB1 0x0017 +#define K2G_DEV_NSS0 0x0018 +#define K2G_DEV_PCIE0 0x0019 +#define K2G_DEV_GPIO0 0x001b +#define K2G_DEV_GPIO1 0x001c +#define K2G_DEV_TIMER64_0 0x001d +#define K2G_DEV_TIMER64_1 0x001e +#define K2G_DEV_TIMER64_2 0x001f +#define K2G_DEV_TIMER64_3 0x0020 +#define K2G_DEV_TIMER64_4 0x0021 +#define K2G_DEV_TIMER64_5 0x0022 +#define K2G_DEV_TIMER64_6 0x0023 +#define K2G_DEV_MSGMGR0 0x0025 +#define K2G_DEV_BOOTCFG0 0x0026 +#define K2G_DEV_ARM_BOOTROM0 0x0027 +#define K2G_DEV_DSP_BOOTROM0 0x0029 +#define K2G_DEV_DEBUGSS0 0x002b +#define K2G_DEV_UART0 0x002c +#define K2G_DEV_UART1 0x002d +#define K2G_DEV_UART2 0x002e +#define K2G_DEV_EHRPWM0 0x002f +#define K2G_DEV_EHRPWM1 0x0030 +#define K2G_DEV_EHRPWM2 0x0031 +#define K2G_DEV_EHRPWM3 0x0032 +#define K2G_DEV_EHRPWM4 0x0033 +#define K2G_DEV_EHRPWM5 0x0034 +#define K2G_DEV_EQEP0 0x0035 +#define K2G_DEV_EQEP1 0x0036 +#define K2G_DEV_EQEP2 0x0037 +#define K2G_DEV_ECAP0 0x0038 +#define K2G_DEV_ECAP1 0x0039 +#define K2G_DEV_I2C0 0x003a +#define K2G_DEV_I2C1 0x003b +#define K2G_DEV_I2C2 0x003c +#define K2G_DEV_EDMA0 0x003f +#define K2G_DEV_SEMAPHORE0 0x0040 +#define K2G_DEV_INTC0 0x0041 +#define K2G_DEV_GIC0 0x0042 +#define K2G_DEV_QSPI0 0x0043 +#define K2G_DEV_ARM_64B_COUNTER0 0x0044 +#define K2G_DEV_TETRIS0 0x0045 +#define K2G_DEV_CGEM0 0x0046 +#define K2G_DEV_MSMC0 0x0047 +#define K2G_DEV_CBASS0 0x0049 +#define K2G_DEV_BOARD0 0x004c +#define K2G_DEV_EDMA1 0x004f + +#endif