| Message ID | 1457646365-1723-2-git-send-email-afd@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, Mar 10, 2016 at 03:46:04PM -0600, Andrew F. Davis wrote: > Add syscon reset controller binding. This will hook to the reset > framework and use syscon/regmap to set reset bits. This allows > reset control of individual SoC subsytems and devices with > memory-mapped reset registers in a common register memory > space. > > Signed-off-by: Andrew F. Davis <afd@ti.com> > [s-anna@ti.com: revise the binding format] > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > .../devicetree/bindings/reset/syscon-reset.txt | 114 +++++++++++++++++++++ > include/dt-bindings/reset/syscon.h | 23 +++++ > 2 files changed, 137 insertions(+) > create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt > create mode 100644 include/dt-bindings/reset/syscon.h > > diff --git a/Documentation/devicetree/bindings/reset/syscon-reset.txt b/Documentation/devicetree/bindings/reset/syscon-reset.txt > new file mode 100644 > index 0000000..02bc9e3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/reset/syscon-reset.txt > @@ -0,0 +1,114 @@ > +SysCon Reset Controller > +======================= > + > +Almost all SoCs have hardware modules that require reset control in addition > +to clock and power control for their functionality. The reset control is > +typically provided by means of memory-mapped I/O registers. These registers are > +sometimes a part of a larger register space region implementing various > +functionalities. This register range is best represented as a syscon node to > +allow multiple entities to access their relevant registers in the common > +register space. > + > +A SysCon Reset Controller node defines a device that uses a syscon node > +and provides reset management functionality for various hardware modules > +present on the SoC. > + > +SysCon Reset Controller Node > +============================ > +Each of the reset provider/controller nodes should be a child of a syscon > +node and have the following properties. > + > +Required properties: > +-------------------- > + - compatible : Should be "syscon-reset" > + - #reset-cells : Should be 1. Please see the reset consumer node below > + for usage details > + - #address-cells : Should be 1 > + - #size-cells : Should be 0 > + > +SysCon Reset Child Node > +============================ > +Each reset provider/controller node should have a child node for each reset > +it would like to expose to consumers. A node per register bit... Now I'm only more convinced this is too low level. > + > +Required properties: > +-------------------- > + - reg : This reset's number, this will be used to reference > + this reset by consumers of this reset Now you have a made up index unrelated to anything in the h/w. Sometimes that's unavoidable, but your prior version did. > + - reset-control : Contains the reset control register information > + Should contain 3 cells defined as: > + Cell #1 : register offset of the reset > + control/status register from the syscon > + register base > + Cell #2 : bit shift value for the reset in the > + respective reset control/status register > + Cell #3 : polarity of the reset bit. Should be 1 or > + RESET_ASSERT_SET for resets that are > + asserted when the bit is set, 0 or > + RESET_ASSERT_CLEAR for resets that are > + asserted when the bit is cleared > + > +Optional properties: > +-------------------- > + - reset-status : Contains the reset status register information. The > + contents of this property are the equivalent to > + reset-control as defined above. If this property is > + not present and the toggle flag is not set, the > + reset register is assumed to be the same as the > + control register > + - toggle : Mark this reset as a toggle only reset, this is used > + when no status register is available. -- 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
Hi Rob, On 03/18/2016 11:39 AM, Rob Herring wrote: > On Thu, Mar 10, 2016 at 03:46:04PM -0600, Andrew F. Davis wrote: >> Add syscon reset controller binding. This will hook to the reset >> framework and use syscon/regmap to set reset bits. This allows >> reset control of individual SoC subsytems and devices with >> memory-mapped reset registers in a common register memory >> space. >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> [s-anna@ti.com: revise the binding format] >> Signed-off-by: Suman Anna <s-anna@ti.com> >> --- >> .../devicetree/bindings/reset/syscon-reset.txt | 114 +++++++++++++++++++++ >> include/dt-bindings/reset/syscon.h | 23 +++++ >> 2 files changed, 137 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt >> create mode 100644 include/dt-bindings/reset/syscon.h >> >> diff --git a/Documentation/devicetree/bindings/reset/syscon-reset.txt b/Documentation/devicetree/bindings/reset/syscon-reset.txt >> new file mode 100644 >> index 0000000..02bc9e3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/reset/syscon-reset.txt >> @@ -0,0 +1,114 @@ >> +SysCon Reset Controller >> +======================= >> + >> +Almost all SoCs have hardware modules that require reset control in addition >> +to clock and power control for their functionality. The reset control is >> +typically provided by means of memory-mapped I/O registers. These registers are >> +sometimes a part of a larger register space region implementing various >> +functionalities. This register range is best represented as a syscon node to >> +allow multiple entities to access their relevant registers in the common >> +register space. >> + >> +A SysCon Reset Controller node defines a device that uses a syscon node >> +and provides reset management functionality for various hardware modules >> +present on the SoC. >> + >> +SysCon Reset Controller Node >> +============================ >> +Each of the reset provider/controller nodes should be a child of a syscon >> +node and have the following properties. >> + >> +Required properties: >> +-------------------- >> + - compatible : Should be "syscon-reset" >> + - #reset-cells : Should be 1. Please see the reset consumer node below >> + for usage details >> + - #address-cells : Should be 1 >> + - #size-cells : Should be 0 >> + >> +SysCon Reset Child Node >> +============================ >> +Each reset provider/controller node should have a child node for each reset >> +it would like to expose to consumers. > > A node per register bit... Now I'm only more convinced this is too low > level. Thanks for the review/comments. So, what's your recommendation here - add the reset info to driver match data and use SoC-specific compatibles? That will also work, we just didn't go that route as it appeared to be adding hardware data to a driver. >> + >> +Required properties: >> +-------------------- >> + - reg : This reset's number, this will be used to reference >> + this reset by consumers of this reset > > Now you have a made up index unrelated to anything in the h/w. Sometimes > that's unavoidable, but your prior version did. We have made this change to avoid adding the reset data in the consumer nodes as was done in v1, and provide it in the controller node itself. We still need a way for consumers to match a specific reset. This is unavoidable if the controller were to encode all the reset data (even if we go with coding up the resets in the driver using SoC-specific compatibles, and use a dt-include header file to mark the indices). regards Suman > >> + - reset-control : Contains the reset control register information >> + Should contain 3 cells defined as: >> + Cell #1 : register offset of the reset >> + control/status register from the syscon >> + register base >> + Cell #2 : bit shift value for the reset in the >> + respective reset control/status register >> + Cell #3 : polarity of the reset bit. Should be 1 or >> + RESET_ASSERT_SET for resets that are >> + asserted when the bit is set, 0 or >> + RESET_ASSERT_CLEAR for resets that are >> + asserted when the bit is cleared >> + >> +Optional properties: >> +-------------------- >> + - reset-status : Contains the reset status register information. The >> + contents of this property are the equivalent to >> + reset-control as defined above. If this property is >> + not present and the toggle flag is not set, the >> + reset register is assumed to be the same as the >> + control register >> + - toggle : Mark this reset as a toggle only reset, this is used >> + when no status register is available. > -- 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 03/18/2016 12:02 PM, Suman Anna wrote: > Hi Rob, > > On 03/18/2016 11:39 AM, Rob Herring wrote: >> On Thu, Mar 10, 2016 at 03:46:04PM -0600, Andrew F. Davis wrote: >>> Add syscon reset controller binding. This will hook to the reset >>> framework and use syscon/regmap to set reset bits. This allows >>> reset control of individual SoC subsytems and devices with >>> memory-mapped reset registers in a common register memory >>> space. >>> >>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>> [s-anna@ti.com: revise the binding format] >>> Signed-off-by: Suman Anna <s-anna@ti.com> >>> --- >>> .../devicetree/bindings/reset/syscon-reset.txt | 114 +++++++++++++++++++++ >>> include/dt-bindings/reset/syscon.h | 23 +++++ >>> 2 files changed, 137 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/reset/syscon-reset.txt >>> create mode 100644 include/dt-bindings/reset/syscon.h >>> >>> diff --git a/Documentation/devicetree/bindings/reset/syscon-reset.txt b/Documentation/devicetree/bindings/reset/syscon-reset.txt >>> new file mode 100644 >>> index 0000000..02bc9e3 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/reset/syscon-reset.txt >>> @@ -0,0 +1,114 @@ >>> +SysCon Reset Controller >>> +======================= >>> + >>> +Almost all SoCs have hardware modules that require reset control in addition >>> +to clock and power control for their functionality. The reset control is >>> +typically provided by means of memory-mapped I/O registers. These registers are >>> +sometimes a part of a larger register space region implementing various >>> +functionalities. This register range is best represented as a syscon node to >>> +allow multiple entities to access their relevant registers in the common >>> +register space. >>> + >>> +A SysCon Reset Controller node defines a device that uses a syscon node >>> +and provides reset management functionality for various hardware modules >>> +present on the SoC. >>> + >>> +SysCon Reset Controller Node >>> +============================ >>> +Each of the reset provider/controller nodes should be a child of a syscon >>> +node and have the following properties. >>> + >>> +Required properties: >>> +-------------------- >>> + - compatible : Should be "syscon-reset" >>> + - #reset-cells : Should be 1. Please see the reset consumer node below >>> + for usage details >>> + - #address-cells : Should be 1 >>> + - #size-cells : Should be 0 >>> + >>> +SysCon Reset Child Node >>> +============================ >>> +Each reset provider/controller node should have a child node for each reset >>> +it would like to expose to consumers. >> >> A node per register bit... Now I'm only more convinced this is too low >> level. > > Thanks for the review/comments. So, what's your recommendation here - > add the reset info to driver match data and use SoC-specific > compatibles? That will also work, we just didn't go that route as it > appeared to be adding hardware data to a driver. > >>> + >>> +Required properties: >>> +-------------------- >>> + - reg : This reset's number, this will be used to reference >>> + this reset by consumers of this reset >> >> Now you have a made up index unrelated to anything in the h/w. Sometimes >> that's unavoidable, but your prior version did. > > We have made this change to avoid adding the reset data in the consumer > nodes as was done in v1, and provide it in the controller node itself. > We still need a way for consumers to match a specific reset. This is > unavoidable if the controller were to encode all the reset data (even if > we go with coding up the resets in the driver using SoC-specific > compatibles, and use a dt-include header file to mark the indices). Hi Rob, Philipp, Any more comments on this series? If you don't have any, we will repost with some minor cleanups/updates against 4.6-rc1 for the next merge window. Or do you feel that the generic driver is still not the way to go and restrict this to a keystone specific driver? regards Suman > > regards > Suman > >> >>> + - reset-control : Contains the reset control register information >>> + Should contain 3 cells defined as: >>> + Cell #1 : register offset of the reset >>> + control/status register from the syscon >>> + register base >>> + Cell #2 : bit shift value for the reset in the >>> + respective reset control/status register >>> + Cell #3 : polarity of the reset bit. Should be 1 or >>> + RESET_ASSERT_SET for resets that are >>> + asserted when the bit is set, 0 or >>> + RESET_ASSERT_CLEAR for resets that are >>> + asserted when the bit is cleared >>> + >>> +Optional properties: >>> +-------------------- >>> + - reset-status : Contains the reset status register information. The >>> + contents of this property are the equivalent to >>> + reset-control as defined above. If this property is >>> + not present and the toggle flag is not set, the >>> + reset register is assumed to be the same as the >>> + control register >>> + - toggle : Mark this reset as a toggle only reset, this is used >>> + when no status register is available. >> > -- 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/reset/syscon-reset.txt b/Documentation/devicetree/bindings/reset/syscon-reset.txt new file mode 100644 index 0000000..02bc9e3 --- /dev/null +++ b/Documentation/devicetree/bindings/reset/syscon-reset.txt @@ -0,0 +1,114 @@ +SysCon Reset Controller +======================= + +Almost all SoCs have hardware modules that require reset control in addition +to clock and power control for their functionality. The reset control is +typically provided by means of memory-mapped I/O registers. These registers are +sometimes a part of a larger register space region implementing various +functionalities. This register range is best represented as a syscon node to +allow multiple entities to access their relevant registers in the common +register space. + +A SysCon Reset Controller node defines a device that uses a syscon node +and provides reset management functionality for various hardware modules +present on the SoC. + +SysCon Reset Controller Node +============================ +Each of the reset provider/controller nodes should be a child of a syscon +node and have the following properties. + +Required properties: +-------------------- + - compatible : Should be "syscon-reset" + - #reset-cells : Should be 1. Please see the reset consumer node below + for usage details + - #address-cells : Should be 1 + - #size-cells : Should be 0 + +SysCon Reset Child Node +============================ +Each reset provider/controller node should have a child node for each reset +it would like to expose to consumers. + +Required properties: +-------------------- + - reg : This reset's number, this will be used to reference + this reset by consumers of this reset + - reset-control : Contains the reset control register information + Should contain 3 cells defined as: + Cell #1 : register offset of the reset + control/status register from the syscon + register base + Cell #2 : bit shift value for the reset in the + respective reset control/status register + Cell #3 : polarity of the reset bit. Should be 1 or + RESET_ASSERT_SET for resets that are + asserted when the bit is set, 0 or + RESET_ASSERT_CLEAR for resets that are + asserted when the bit is cleared + +Optional properties: +-------------------- + - reset-status : Contains the reset status register information. The + contents of this property are the equivalent to + reset-control as defined above. If this property is + not present and the toggle flag is not set, the + reset register is assumed to be the same as the + control register + - toggle : Mark this reset as a toggle only reset, this is used + when no status register is available. + +SysCon Reset Consumer Nodes +=========================== +Each of the reset consumer nodes should have the following properties, +in addition to their own properties. + +Required properties: +-------------------- + - resets : A phandle and a reset specifier, the reset specifier should + be a numerical address matching the desired reset as set + by the reg property defined above. + +Please also refer to Documentation/devicetree/bindings/reset/reset.txt for +common reset controller usage by consumers. + +Example: +-------- +The following example demonstrates a syscon node, the reset controller node +using the syscon node, and a consumer (a DSP device) on the TI Keystone 2 +Hawking SoC. + +/ { + soc { + psc: power-sleep-controller@02350000 { + compatible = "syscon", "simple-mfd"; + reg = <0x02350000 0x1000>; + + pscrst: psc-reset { + compatible = "syscon-reset"; + #reset-cells = <1>; + #address-cells = <1>; + #size-cells = <0>; + + dsp@0 { + reg = <0>; + reset-control = <0xa3c 8 RESET_ASSERT_CLEAR>; + reset-status = <0x83c 8 RESET_ASSERT_CLEAR>; + }; + + imu@1 { + reg = <1>; + reset-control = <0xa40 5 RESET_ASSERT_SET>; + toggle; + }; + }; + }; + + dsp0: dsp0 { + ... + resets = <&pscrst 0>; + ... + }; + }; +}; diff --git a/include/dt-bindings/reset/syscon.h b/include/dt-bindings/reset/syscon.h new file mode 100644 index 0000000..9927779 --- /dev/null +++ b/include/dt-bindings/reset/syscon.h @@ -0,0 +1,23 @@ +/* + * Syscon Reset 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_RESET_SYSCON_H__ +#define __DT_BINDINGS_RESET_SYSCON_H__ + +#define RESET_ASSERT_CLEAR 0x0 +#define RESET_ASSERT_SET 0x1 + +#endif