On Mon, May 29, 2017 at 6:39 PM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
> Hi Joel,
>
> On Fri, 2017-05-26 at 13:32 +1000, Joel Stanley wrote:
>> This adds the bindings documentation for a basic single-register reset
>> controller.
>>
>> The bindings describe a single 32-bit register that contains up to 32
>> reset lines, each deasserted by clearing the appropriate bit in the
>> register.
>>
>> Signed-off-by: Joel Stanley <joel@xxxxxxxxx>
>> ---
>> .../devicetree/bindings/reset/reset-basic.txt | 31 ++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/reset/reset-basic.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/reset-basic.txt b/Documentation/devicetree/bindings/reset/reset-basic.txt
>> new file mode 100644
>> index 000000000000..7341e04e7904
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/reset-basic.txt
>> @@ -0,0 +1,31 @@
>> +Basic single-register reset controller
>> +======================================
>> +
>> +This describes a generic reset controller where the reset lines are controlled
>> +by single bits within a 32-bit memory location. The memory location is assumed
>> +to be part of a syscon regmap.
>
> There are a few more assumptions. First, that the reset line is asserted
> by setting the corresponding bits, and that it is deasserted by clearing
> them. Further, that the bits are not auto-clearing. And then, that the
> same bit can be read back to provide the current reset line status.
I'll add a property to indicate set-to-enable in v2. How about:
- reset-set-assert: Specifies that the bit should be set to assert
the reset. The default is to set the bit to deassert the reset.
I'm sure we can find a better way to write that.
I'll add a note that the bits are not auto-clearing, and therefore
they can be read back. This is in the spirit of trying to describe a
basic reset controller.
>
>> +Reset controller required properties:
>> + - compatible: should be "reset-basic"
>> + - #reset-cells: must be set to 1
>> + - reg: reset register location within regmap
>> +
>> +Device node required properties:
>> + - resets phandle
>> + - bit number, counting from zero, for the desired reset line. Max is 31.
>> +
>> +Example:
>> +
>> +syscon {
>> + compatible = "syscon";
>> +
>> + uart_rest: rest@0c {
>
> The device node name should be "reset-controller", and leading zeroes
> should be dropped from the address part:
>
> uart_rest: reset-controller@c {
Ok.
>
>> + compatible = "reset-basic";
>
> Maybe this is not necessary for the example, but the compatible should
> contain a vendor/hardware specific string first.
You're suggesting I add a hardware-specific string in addition to the
generic reset-basic? eg:
compatible = "aspeed,uart-rest", "reset-basic";
I don't think that helps anyone. We don't do that for eg.
timeriomem-rng or gpio-fan. Or perhaps I've misunderstood your
suggestion?
>
>> + #reset-cells = <1>;
>> + reg = <0x0c>;
>> + };
>> +}
>> +
>> +&uart {
>> + resets = <&uart_rest 0x04>;
>
> I'd use decimal here.
Ok.
Cheers,
Joel
--
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
