| Message ID | 20190830121816.30034-2-t-kristo@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [PATCHv3,01/10] dt-bindings: omap: add new binding for PRM instances | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: > Add new binding for OMAP PRM (Power and Reset Manager) instances. Each > of these will act as a power domain controller and potentially as a reset > provider. > Converting this to schema would be nice. > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ bindings/reset/ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt > > diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > new file mode 100644 > index 000000000000..7c7527c37734 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > @@ -0,0 +1,31 @@ > +OMAP PRM instance bindings > + > +Power and Reset Manager is an IP block on OMAP family of devices which > +handle the power domains and their current state, and provide reset > +handling for the domains and/or separate IP blocks under the power domain > +hierarchy. > + > +Required properties: > +- compatible: Must be one of: > + "ti,am3-prm-inst" > + "ti,am4-prm-inst" > + "ti,omap4-prm-inst" > + "ti,omap5-prm-inst" > + "ti,dra7-prm-inst" '-inst' seems a bit redundant. > +- reg: Contains PRM instance register address range > + (base address and length) > + > +Optional properties: > +- #reset-cells: Should be 1 if the PRM instance in question supports resets. > +- clocks: Associated clocks for the reset signals if any. Certain reset > + signals can't be toggled properly without functional clock > + being active for them. > + > +Example: > + > +prm_dsp2: prm@1b00 { reset-controller@... > + compatible = "ti,dra7-prm-inst"; > + reg = <0x1b00 0x40>; > + #reset-cells = <1>; > + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>; > +}; > -- > 2.17.1 > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 02/09/2019 16:39, Rob Herring wrote: > On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: >> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each >> of these will act as a power domain controller and potentially as a reset >> provider. >> > > Converting this to schema would be nice. Do you have documentation about schema somewhere? Basically what I need to do to fix this. > >> Signed-off-by: Tero Kristo <t-kristo@ti.com> >> --- >> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ > > bindings/reset/ I did not put this under reset, because this is basically a multi-purpose function. Reset just happens to be the first functionality it is going to provide. It will be followed by power domain support later on. Any thoughts? > >> 1 file changed, 31 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt >> >> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >> new file mode 100644 >> index 000000000000..7c7527c37734 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >> @@ -0,0 +1,31 @@ >> +OMAP PRM instance bindings >> + >> +Power and Reset Manager is an IP block on OMAP family of devices which >> +handle the power domains and their current state, and provide reset >> +handling for the domains and/or separate IP blocks under the power domain >> +hierarchy. >> + >> +Required properties: >> +- compatible: Must be one of: >> + "ti,am3-prm-inst" >> + "ti,am4-prm-inst" >> + "ti,omap4-prm-inst" >> + "ti,omap5-prm-inst" >> + "ti,dra7-prm-inst" > > '-inst' seems a bit redundant. ti,xyz-prm is already reserved by the parent node of all these. The hierarchy is basically like this (omap4 as example): prm: prm@4a306000 { compatible = "ti,omap4-prm"; ... prm_dsp: prm@400 { compatible = "ti,omap4-prm-inst"; ... }; prm_device: prm@1b00 { compatible = "ti,omap4-prm-inst"; ... }; ... }; > >> +- reg: Contains PRM instance register address range >> + (base address and length) >> + >> +Optional properties: >> +- #reset-cells: Should be 1 if the PRM instance in question supports resets. >> +- clocks: Associated clocks for the reset signals if any. Certain reset >> + signals can't be toggled properly without functional clock >> + being active for them. >> + >> +Example: >> + >> +prm_dsp2: prm@1b00 { > > reset-controller@... Well, as said, the same node is going to be also power domain provider later on... > >> + compatible = "ti,dra7-prm-inst"; >> + reg = <0x1b00 0x40>; >> + #reset-cells = <1>; >> + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>; >> +}; >> -- >> 2.17.1 >> >> -- > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On Tue, Sep 3, 2019 at 8:26 AM Tero Kristo <t-kristo@ti.com> wrote: > > On 02/09/2019 16:39, Rob Herring wrote: > > On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: > >> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each > >> of these will act as a power domain controller and potentially as a reset > >> provider. > >> > > > > Converting this to schema would be nice. > > Do you have documentation about schema somewhere? Basically what I need > to do to fix this. Documentation/devicetree/writing-schema.md (.rst in -next) Documentation/devicetree/bindings/example-schema.yaml > >> Signed-off-by: Tero Kristo <t-kristo@ti.com> > >> --- > >> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ > > > > bindings/reset/ > > I did not put this under reset, because this is basically a > multi-purpose function. Reset just happens to be the first functionality > it is going to provide. It will be followed by power domain support > later on. > > Any thoughts? I prefer that bindings be complete as possible even if driver support is not there yet. Adding power domain support may only mean adding '#power-domain-cells'. The location is fine then. > >> 1 file changed, 31 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> > >> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> new file mode 100644 > >> index 000000000000..7c7527c37734 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> @@ -0,0 +1,31 @@ > >> +OMAP PRM instance bindings > >> + > >> +Power and Reset Manager is an IP block on OMAP family of devices which > >> +handle the power domains and their current state, and provide reset > >> +handling for the domains and/or separate IP blocks under the power domain > >> +hierarchy. > >> + > >> +Required properties: > >> +- compatible: Must be one of: > >> + "ti,am3-prm-inst" > >> + "ti,am4-prm-inst" > >> + "ti,omap4-prm-inst" > >> + "ti,omap5-prm-inst" > >> + "ti,dra7-prm-inst" > > > > '-inst' seems a bit redundant. > > ti,xyz-prm is already reserved by the parent node of all these. > > The hierarchy is basically like this (omap4 as example): > > prm: prm@4a306000 { > compatible = "ti,omap4-prm"; > ... > > prm_dsp: prm@400 { > compatible = "ti,omap4-prm-inst"; > ... > }; > > prm_device: prm@1b00 { > compatible = "ti,omap4-prm-inst"; > ... > }; > > ... > }; Okay. Then you need to state this binding must be a child of PRM. The schema would need to take this into account too, so probably best to not convert this yet. Rob
On 03/09/2019 11:10, Rob Herring wrote: > On Tue, Sep 3, 2019 at 8:26 AM Tero Kristo <t-kristo@ti.com> wrote: >> >> On 02/09/2019 16:39, Rob Herring wrote: >>> On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: >>>> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each >>>> of these will act as a power domain controller and potentially as a reset >>>> provider. >>>> >>> >>> Converting this to schema would be nice. >> >> Do you have documentation about schema somewhere? Basically what I need >> to do to fix this. > > Documentation/devicetree/writing-schema.md (.rst in -next) > Documentation/devicetree/bindings/example-schema.yaml > >>>> Signed-off-by: Tero Kristo <t-kristo@ti.com> >>>> --- >>>> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ >>> >>> bindings/reset/ >> >> I did not put this under reset, because this is basically a >> multi-purpose function. Reset just happens to be the first functionality >> it is going to provide. It will be followed by power domain support >> later on. >> >> Any thoughts? > > I prefer that bindings be complete as possible even if driver support > is not there yet. Adding power domain support may only mean adding > '#power-domain-cells'. > > The location is fine then. Yeah, I assume just adding power-domain-cells should be enough. I am not too sure before I start trying this out though so did not want to add it yet. > >>>> 1 file changed, 31 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> new file mode 100644 >>>> index 000000000000..7c7527c37734 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> @@ -0,0 +1,31 @@ >>>> +OMAP PRM instance bindings >>>> + >>>> +Power and Reset Manager is an IP block on OMAP family of devices which >>>> +handle the power domains and their current state, and provide reset >>>> +handling for the domains and/or separate IP blocks under the power domain >>>> +hierarchy. >>>> + >>>> +Required properties: >>>> +- compatible: Must be one of: >>>> + "ti,am3-prm-inst" >>>> + "ti,am4-prm-inst" >>>> + "ti,omap4-prm-inst" >>>> + "ti,omap5-prm-inst" >>>> + "ti,dra7-prm-inst" >>> >>> '-inst' seems a bit redundant. >> >> ti,xyz-prm is already reserved by the parent node of all these. >> >> The hierarchy is basically like this (omap4 as example): >> >> prm: prm@4a306000 { >> compatible = "ti,omap4-prm"; >> ... >> >> prm_dsp: prm@400 { >> compatible = "ti,omap4-prm-inst"; >> ... >> }; >> >> prm_device: prm@1b00 { >> compatible = "ti,omap4-prm-inst"; >> ... >> }; >> >> ... >> }; > > Okay. Then you need to state this binding must be a child of PRM. The > schema would need to take this into account too, so probably best to > not convert this yet. > Ok thanks, I'll make the necessary updates and post v4. -Tero -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
* Tero Kristo <t-kristo@ti.com> [190903 08:15]: > On 03/09/2019 11:10, Rob Herring wrote: > > I prefer that bindings be complete as possible even if driver support > > is not there yet. Adding power domain support may only mean adding > > '#power-domain-cells'. > > > > The location is fine then. > > Yeah, I assume just adding power-domain-cells should be enough. I am not too > sure before I start trying this out though so did not want to add it yet. Should we call the device tree node name power-controller instead of reset controller though? Most of the PRM instances do not have a separate rstctrl reset control functionality. Anybody got better any better naming in mind? Regards, Tony
On Tue, Sep 3, 2019 at 2:26 AM Tero Kristo <t-kristo@ti.com> wrote: > > On 02/09/2019 16:39, Rob Herring wrote: > > On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: > >> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each > >> of these will act as a power domain controller and potentially as a reset > >> provider. > >> > > > > Converting this to schema would be nice. > > Do you have documentation about schema somewhere? Basically what I need > to do to fix this. > > > > >> Signed-off-by: Tero Kristo <t-kristo@ti.com> > >> --- > >> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ > > > > bindings/reset/ > > I did not put this under reset, because this is basically a > multi-purpose function. Reset just happens to be the first functionality > it is going to provide. It will be followed by power domain support > later on. > > Any thoughts? > > > > >> 1 file changed, 31 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> > >> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> new file mode 100644 > >> index 000000000000..7c7527c37734 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt > >> @@ -0,0 +1,31 @@ > >> +OMAP PRM instance bindings > >> + > >> +Power and Reset Manager is an IP block on OMAP family of devices which > >> +handle the power domains and their current state, and provide reset > >> +handling for the domains and/or separate IP blocks under the power domain > >> +hierarchy. > >> + > >> +Required properties: > >> +- compatible: Must be one of: > >> + "ti,am3-prm-inst" Would it make sense to call it am33 instead of am3? The AM35xx is different than AM33. > >> + "ti,am4-prm-inst" > >> + "ti,omap4-prm-inst" > >> + "ti,omap5-prm-inst" > >> + "ti,dra7-prm-inst" > > > > '-inst' seems a bit redundant. > > ti,xyz-prm is already reserved by the parent node of all these. > > The hierarchy is basically like this (omap4 as example): > > prm: prm@4a306000 { > compatible = "ti,omap4-prm"; > ... > > prm_dsp: prm@400 { > compatible = "ti,omap4-prm-inst"; > ... > }; > > prm_device: prm@1b00 { > compatible = "ti,omap4-prm-inst"; > ... > }; > > ... > }; > > > > > > >> +- reg: Contains PRM instance register address range > >> + (base address and length) > >> + > >> +Optional properties: > >> +- #reset-cells: Should be 1 if the PRM instance in question supports resets. > >> +- clocks: Associated clocks for the reset signals if any. Certain reset > >> + signals can't be toggled properly without functional clock > >> + being active for them. > >> + > >> +Example: > >> + > >> +prm_dsp2: prm@1b00 { > > > > reset-controller@... > > Well, as said, the same node is going to be also power domain provider > later on... > > > > >> + compatible = "ti,dra7-prm-inst"; > >> + reg = <0x1b00 0x40>; > >> + #reset-cells = <1>; > >> + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>; > >> +}; > >> -- > >> 2.17.1 > >> > >> -- > > > adam > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi Tony, On Tue, 2019-09-03 at 06:16 -0700, Tony Lindgren wrote: > * Tero Kristo <t-kristo@ti.com> [190903 08:15]: > > On 03/09/2019 11:10, Rob Herring wrote: > > > I prefer that bindings be complete as possible even if driver support > > > is not there yet. Adding power domain support may only mean adding > > > '#power-domain-cells'. > > > > > > The location is fine then. > > > > Yeah, I assume just adding power-domain-cells should be enough. I am not too > > sure before I start trying this out though so did not want to add it yet. > > Should we call the device tree node name power-controller instead of > reset controller though? Most of the PRM instances do not have a > separate rstctrl reset control functionality. > > Anybody got better any better naming in mind? power-controller seems fine to me, that is their primary function. regards Philipp
On 03/09/2019 16:19, Adam Ford wrote: > On Tue, Sep 3, 2019 at 2:26 AM Tero Kristo <t-kristo@ti.com> wrote: >> >> On 02/09/2019 16:39, Rob Herring wrote: >>> On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote: >>>> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each >>>> of these will act as a power domain controller and potentially as a reset >>>> provider. >>>> >>> >>> Converting this to schema would be nice. >> >> Do you have documentation about schema somewhere? Basically what I need >> to do to fix this. >> >>> >>>> Signed-off-by: Tero Kristo <t-kristo@ti.com> >>>> --- >>>> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ >>> >>> bindings/reset/ >> >> I did not put this under reset, because this is basically a >> multi-purpose function. Reset just happens to be the first functionality >> it is going to provide. It will be followed by power domain support >> later on. >> >> Any thoughts? >> >>> >>>> 1 file changed, 31 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> new file mode 100644 >>>> index 000000000000..7c7527c37734 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt >>>> @@ -0,0 +1,31 @@ >>>> +OMAP PRM instance bindings >>>> + >>>> +Power and Reset Manager is an IP block on OMAP family of devices which >>>> +handle the power domains and their current state, and provide reset >>>> +handling for the domains and/or separate IP blocks under the power domain >>>> +hierarchy. >>>> + >>>> +Required properties: >>>> +- compatible: Must be one of: >>>> + "ti,am3-prm-inst" > > Would it make sense to call it am33 instead of am3? The AM35xx is > different than AM33. Well, am35xx is effectively just a variant of omap3, they just named it funnily. Same for dra7 vs. am57xx. Also, bindings of type "ti,am3-*" exist for other am33xx functionality already. -Tero > >>>> + "ti,am4-prm-inst" >>>> + "ti,omap4-prm-inst" >>>> + "ti,omap5-prm-inst" >>>> + "ti,dra7-prm-inst" >>> >>> '-inst' seems a bit redundant. >> >> ti,xyz-prm is already reserved by the parent node of all these. >> >> The hierarchy is basically like this (omap4 as example): >> >> prm: prm@4a306000 { >> compatible = "ti,omap4-prm"; >> ... >> >> prm_dsp: prm@400 { >> compatible = "ti,omap4-prm-inst"; >> ... >> }; >> >> prm_device: prm@1b00 { >> compatible = "ti,omap4-prm-inst"; >> ... >> }; >> >> ... >> }; >> >> >> >>> >>>> +- reg: Contains PRM instance register address range >>>> + (base address and length) >>>> + >>>> +Optional properties: >>>> +- #reset-cells: Should be 1 if the PRM instance in question supports resets. >>>> +- clocks: Associated clocks for the reset signals if any. Certain reset >>>> + signals can't be toggled properly without functional clock >>>> + being active for them. >>>> + >>>> +Example: >>>> + >>>> +prm_dsp2: prm@1b00 { >>> >>> reset-controller@... >> >> Well, as said, the same node is going to be also power domain provider >> later on... >> >>> >>>> + compatible = "ti,dra7-prm-inst"; >>>> + reg = <0x1b00 0x40>; >>>> + #reset-cells = <1>; >>>> + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>; >>>> +}; >>>> -- >>>> 2.17.1 >>>> >>>> -- >>> >> > > adam >> -- -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt new file mode 100644 index 000000000000..7c7527c37734 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt @@ -0,0 +1,31 @@ +OMAP PRM instance bindings + +Power and Reset Manager is an IP block on OMAP family of devices which +handle the power domains and their current state, and provide reset +handling for the domains and/or separate IP blocks under the power domain +hierarchy. + +Required properties: +- compatible: Must be one of: + "ti,am3-prm-inst" + "ti,am4-prm-inst" + "ti,omap4-prm-inst" + "ti,omap5-prm-inst" + "ti,dra7-prm-inst" +- reg: Contains PRM instance register address range + (base address and length) + +Optional properties: +- #reset-cells: Should be 1 if the PRM instance in question supports resets. +- clocks: Associated clocks for the reset signals if any. Certain reset + signals can't be toggled properly without functional clock + being active for them. + +Example: + +prm_dsp2: prm@1b00 { + compatible = "ti,dra7-prm-inst"; + reg = <0x1b00 0x40>; + #reset-cells = <1>; + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>; +};
Add new binding for OMAP PRM (Power and Reset Manager) instances. Each of these will act as a power domain controller and potentially as a reset provider. Signed-off-by: Tero Kristo <t-kristo@ti.com> --- .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt