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@xxxxxx> >>> [s-anna@xxxxxx: revise the binding format] >>> Signed-off-by: Suman Anna <s-anna@xxxxxx> >>> --- >>> .../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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
