Re: [PATCH v2 1/4] crypto: jz4780-rng: Add JZ4780 PRNG devicetree binding documentation

[Rob Herring]

On Fri, Aug 25, 2017 at 10:20 PM, PrasannaKumar Muralidharan
 wrote:
> Hi Rob,
>
> On 26 August 2017 at 03:27, Rob Herring  wrote:
>> On Wed, Aug 23, 2017 at 08:27:04AM +0530, PrasannaKumar Muralidharan wrote:
>>> Add devicetree bindings for hardware pseudo random number generator
>>> present in Ingenic JZ4780 SoC.
>>>
>>> Signed-off-by: PrasannaKumar Muralidharan 
>>> ---
>>> Changes in v2:
>>> * Add "syscon" in CGU node's compatible section
>>> * Make RNG child node of CGU.
>>>
>>>  .../bindings/crypto/ingenic,jz4780-rng.txt   | 20 
>>> 
>>
>> bindings/rng/ for RNG h/w.
>
> There are two subsystem for dealing with RNG hw. Hw_random subsystem
> for true RNG (driver/char/hw_random) and crypto framework for pseudo
> RNG (crypto/ and drviers/crypto). This HW is pseudo RNG so I have
> placed the dt bindings in bindings/crypto as the driver itself is in
> drivers/crypto folder. I am wondering if there is any relation between
> driver folder and bindings folder. Can you please explain the folder
> relation? Should this be put in bindings/rng or bindings/crypto?

There's not a 1-1 relationship though obviously there's a lot of
overlap. I'd still say this should go in bindings/rng.

>>>  1 file changed, 20 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt 
>>> b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>>> new file mode 100644
>>> index 000..a0c18e5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>>> @@ -0,0 +1,20 @@
>>> +Ingenic jz4780 RNG driver
>>> +
>>> +Required properties:
>>> +- compatible : Should be "ingenic,jz4780-rng"
>>> +
>>> +Example:
>>> +
>>> +cgu: jz4780-cgu@1000 {
>>> + compatible = "ingenic,jz4780-cgu", "syscon";
>>> + reg = <0x1000 0x100>;
>>> +
>>> + clocks = <>, <>;
>>> + clock-names = "ext", "rtc";
>>> +
>>> + #clock-cells = <1>;
>>> +
>>> + rng: rng@d8 {
>>
>> unit-address requires reg property.
>
> The driver uses regmap to access the registers. In this case reg
> property is not useful. Is reg property still needed? If not, how
> should the node be declared?

What the driver (currently) does is irrelevant to the binding. Your
choice is either add the reg property or name the node just "rng".
Either is fine, but better to have more information than less IMO.

Rob

Re: [PATCH v2 1/4] crypto: jz4780-rng: Add JZ4780 PRNG devicetree binding documentation

[PrasannaKumar Muralidharan]

Hi Rob,

On 26 August 2017 at 03:27, Rob Herring  wrote:
> On Wed, Aug 23, 2017 at 08:27:04AM +0530, PrasannaKumar Muralidharan wrote:
>> Add devicetree bindings for hardware pseudo random number generator
>> present in Ingenic JZ4780 SoC.
>>
>> Signed-off-by: PrasannaKumar Muralidharan 
>> ---
>> Changes in v2:
>> * Add "syscon" in CGU node's compatible section
>> * Make RNG child node of CGU.
>>
>>  .../bindings/crypto/ingenic,jz4780-rng.txt   | 20 
>> 
>
> bindings/rng/ for RNG h/w.

There are two subsystem for dealing with RNG hw. Hw_random subsystem
for true RNG (driver/char/hw_random) and crypto framework for pseudo
RNG (crypto/ and drviers/crypto). This HW is pseudo RNG so I have
placed the dt bindings in bindings/crypto as the driver itself is in
drivers/crypto folder. I am wondering if there is any relation between
driver folder and bindings folder. Can you please explain the folder
relation? Should this be put in bindings/rng or bindings/crypto?

>
>>  1 file changed, 20 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt 
>> b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>> new file mode 100644
>> index 000..a0c18e5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
>> @@ -0,0 +1,20 @@
>> +Ingenic jz4780 RNG driver
>> +
>> +Required properties:
>> +- compatible : Should be "ingenic,jz4780-rng"
>> +
>> +Example:
>> +
>> +cgu: jz4780-cgu@1000 {
>> + compatible = "ingenic,jz4780-cgu", "syscon";
>> + reg = <0x1000 0x100>;
>> +
>> + clocks = <>, <>;
>> + clock-names = "ext", "rtc";
>> +
>> + #clock-cells = <1>;
>> +
>> + rng: rng@d8 {
>
> unit-address requires reg property.

The driver uses regmap to access the registers. In this case reg
property is not useful. Is reg property still needed? If not, how
should the node be declared?

>
>> + compatible = "ingenic,jz480-rng";
>> + };
>> +};
>> --
>> 2.10.0
>>

Thanks,
PrasannaKumar

Re: [PATCH v2 1/4] crypto: jz4780-rng: Add JZ4780 PRNG devicetree binding documentation

[Rob Herring]

On Wed, Aug 23, 2017 at 08:27:04AM +0530, PrasannaKumar Muralidharan wrote:
> Add devicetree bindings for hardware pseudo random number generator
> present in Ingenic JZ4780 SoC.
> 
> Signed-off-by: PrasannaKumar Muralidharan 
> ---
> Changes in v2:
> * Add "syscon" in CGU node's compatible section
> * Make RNG child node of CGU.
> 
>  .../bindings/crypto/ingenic,jz4780-rng.txt   | 20 
> 

bindings/rng/ for RNG h/w.

>  1 file changed, 20 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt 
> b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
> new file mode 100644
> index 000..a0c18e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
> @@ -0,0 +1,20 @@
> +Ingenic jz4780 RNG driver
> +
> +Required properties:
> +- compatible : Should be "ingenic,jz4780-rng"
> +
> +Example:
> +
> +cgu: jz4780-cgu@1000 {
> + compatible = "ingenic,jz4780-cgu", "syscon";
> + reg = <0x1000 0x100>;
> +
> + clocks = <>, <>;
> + clock-names = "ext", "rtc";
> +
> + #clock-cells = <1>;
> +
> + rng: rng@d8 {

unit-address requires reg property.

> + compatible = "ingenic,jz480-rng";
> + };
> +};
> -- 
> 2.10.0
> 

[PATCH v2 1/4] crypto: jz4780-rng: Add JZ4780 PRNG devicetree binding documentation

[PrasannaKumar Muralidharan]

Add devicetree bindings for hardware pseudo random number generator
present in Ingenic JZ4780 SoC.

Signed-off-by: PrasannaKumar Muralidharan 
---
Changes in v2:
* Add "syscon" in CGU node's compatible section
* Make RNG child node of CGU.

 .../bindings/crypto/ingenic,jz4780-rng.txt   | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt

diff --git a/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt 
b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
new file mode 100644
index 000..a0c18e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/ingenic,jz4780-rng.txt
@@ -0,0 +1,20 @@
+Ingenic jz4780 RNG driver
+
+Required properties:
+- compatible : Should be "ingenic,jz4780-rng"
+
+Example:
+
+cgu: jz4780-cgu@1000 {
+   compatible = "ingenic,jz4780-cgu", "syscon";
+   reg = <0x1000 0x100>;
+
+   clocks = <>, <>;
+   clock-names = "ext", "rtc";
+
+   #clock-cells = <1>;
+
+   rng: rng@d8 {
+   compatible = "ingenic,jz480-rng";
+   };
+};
-- 
2.10.0