diff mbox

[1/3] dt: Add DT bindings documentation for SUNXI Security System

Message ID 1400771396-9686-2-git-send-email-clabbe.montjoie@gmail.com
State Superseded, archived
Headers show

Commit Message

Corentin Labbe May 22, 2014, 3:09 p.m. UTC
Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt

Comments

Marek Vasut May 24, 2014, 11:21 a.m. UTC | #1
On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:

Missing commit message. Please fix this and send a V2.

> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> 100644
> index 0000000..356563b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> @@ -0,0 +1,9 @@
> +* Allwinner Security System found on A20 SoC
> +
> +Required properties:
> +- compatible : Should be "allwinner,sun7i-a20-crypto".

Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can that not 
be described by DT props ?

> +- reg: Should contain the SS register location and length.

SS? What is that? Fix this text to be actually descriptive please.

> +- interrupts: Should contain the IRQ line for the SS.
> +- clocks : A phandle to the functional clock node of the SS module
> +- clock-names : Name of the functional clock, should be "ahb" and "mod".
> +

Best regards,
Marek Vasut
--
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
Tomasz Figa May 24, 2014, 7:20 p.m. UTC | #2
Hi Marek,

On 24.05.2014 13:21, Marek Vasut wrote:
> On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> 
> Missing commit message. Please fix this and send a V2.
> 
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
>> 100644
>> index 0000000..356563b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>> @@ -0,0 +1,9 @@
>> +* Allwinner Security System found on A20 SoC
>> +
>> +Required properties:
>> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> 
> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can that not 
> be described by DT props ?

A widely used convention is to define compatible strings after first
SoCs on which particular IP blocks appear. It is quite common among IP
blocks for which there is no well defined versioning scheme.

Best regards,
Tomasz
--
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
Marek Vasut May 24, 2014, 7:43 p.m. UTC | #3
On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
> Hi Marek,
> 
> On 24.05.2014 13:21, Marek Vasut wrote:
> > On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> > 
> > Missing commit message. Please fix this and send a V2.
> > 
> >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> >> ---
> >> 
> >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> >> 100644
> >> index 0000000..356563b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> @@ -0,0 +1,9 @@
> >> +* Allwinner Security System found on A20 SoC
> >> +
> >> +Required properties:
> >> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> > 
> > Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > that not be described by DT props ?
> 
> A widely used convention is to define compatible strings after first
> SoCs on which particular IP blocks appear. It is quite common among IP
> blocks for which there is no well defined versioning scheme.

Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
manufactured by AW. I find that information redundant, the "allwinner,a20-
crypto" would suffice. But I wonder if that IP block might have appeared even 
earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
be a better string ?

Best regards,
Marek Vasut
--
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
Tomasz Figa May 24, 2014, 7:51 p.m. UTC | #4
On 24.05.2014 21:43, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
>> Hi Marek,
>>
>> On 24.05.2014 13:21, Marek Vasut wrote:
>>> On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
>>>
>>> Missing commit message. Please fix this and send a V2.
>>>
>>>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>>>> ---
>>>>
>>>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>  create mode 100644
>>>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
>>>> 100644
>>>> index 0000000..356563b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>> @@ -0,0 +1,9 @@
>>>> +* Allwinner Security System found on A20 SoC
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be "allwinner,sun7i-a20-crypto".
>>>
>>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
>>> that not be described by DT props ?
>>
>> A widely used convention is to define compatible strings after first
>> SoCs on which particular IP blocks appear. It is quite common among IP
>> blocks for which there is no well defined versioning scheme.
> 
> Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
> manufactured by AW. I find that information redundant, the "allwinner,a20-
> crypto" would suffice. But I wonder if that IP block might have appeared even 
> earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
> be a better string ?

I'm not aware of Allwinner naming schemes too much, so please correct me
if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
would be better indeed.

Whether it was really the first SoC is another thing. Obviously this
needs to be checked, although it isn't really that important. For this
particular naming scheme you need to specify all the SoCs for which
given compatible string can be used for this IP anyway, because there is
usually no other source of information about this available (except
directly comparing two datasheets...).

Best regards,
Tomasz
--
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
Marek Vasut May 24, 2014, 7:59 p.m. UTC | #5
On Saturday, May 24, 2014 at 09:51:59 PM, Tomasz Figa wrote:
[...]
> >>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> >>> that not be described by DT props ?
> >> 
> >> A widely used convention is to define compatible strings after first
> >> SoCs on which particular IP blocks appear. It is quite common among IP
> >> blocks for which there is no well defined versioning scheme.
> > 
> > Well yeah, that's fine. But in this case, "sun7i" is the entire group of
> > CPUs manufactured by AW. I find that information redundant, the
> > "allwinner,a20- crypto" would suffice. But I wonder if that IP block
> > might have appeared even earlier ? Or if it is CPU family specific, thus
> > "allwinner,sun7i-crypto" would be a better string ?
> 
> I'm not aware of Allwinner naming schemes too much, so please correct me
> if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
> would be better indeed.

True.

> Whether it was really the first SoC is another thing. Obviously this
> needs to be checked, although it isn't really that important. For this
> particular naming scheme you need to specify all the SoCs for which
> given compatible string can be used for this IP anyway, because there is
> usually no other source of information about this available (except
> directly comparing two datasheets...).

Better get the DT stuff correctly right from the start. That's why I'm asking 
what chips contains the IP block, so we can guess the right name.

Best regards,
Marek Vasut
--
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
Maxime Ripard May 25, 2014, 1:07 p.m. UTC | #6
On Sat, May 24, 2014 at 09:43:42PM +0200, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
> > Hi Marek,
> > 
> > On 24.05.2014 13:21, Marek Vasut wrote:
> > > On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> > > 
> > > Missing commit message. Please fix this and send a V2.
> > > 
> > >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > >> ---
> > >> 
> > >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
> > >>  1 file changed, 9 insertions(+)
> > >>  create mode 100644
> > >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> 
> > >> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> > >> 100644
> > >> index 0000000..356563b
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> @@ -0,0 +1,9 @@
> > >> +* Allwinner Security System found on A20 SoC
> > >> +
> > >> +Required properties:
> > >> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> > > 
> > > Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > > that not be described by DT props ?
> > 
> > A widely used convention is to define compatible strings after first
> > SoCs on which particular IP blocks appear. It is quite common among IP
> > blocks for which there is no well defined versioning scheme.
> 
> Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
> manufactured by AW. I find that information redundant, the "allwinner,a20-
> crypto" would suffice. But I wonder if that IP block might have appeared even 
> earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
> be a better string ?

No. sun7i-a20-crypto is perfectly fine, and the pattern is used for
all the IPs. sun7i is the SoC family, A20 the actual SoC. In the A20
case, they're equivalent, it's not the case for other Allwinner
SoCs. And I definitely prefer consistency over plain mess. You might
see it differently, but that's not going to change.

Maxime
Maxime Ripard May 25, 2014, 1:09 p.m. UTC | #7
On Sat, May 24, 2014 at 09:59:30PM +0200, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:51:59 PM, Tomasz Figa wrote:
> [...]
> > >>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > >>> that not be described by DT props ?
> > >> 
> > >> A widely used convention is to define compatible strings after first
> > >> SoCs on which particular IP blocks appear. It is quite common among IP
> > >> blocks for which there is no well defined versioning scheme.
> > > 
> > > Well yeah, that's fine. But in this case, "sun7i" is the entire group of
> > > CPUs manufactured by AW. I find that information redundant, the
> > > "allwinner,a20- crypto" would suffice. But I wonder if that IP block
> > > might have appeared even earlier ? Or if it is CPU family specific, thus
> > > "allwinner,sun7i-crypto" would be a better string ?
> > 
> > I'm not aware of Allwinner naming schemes too much, so please correct me
> > if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
> > would be better indeed.
> 
> True.
> 
> > Whether it was really the first SoC is another thing. Obviously this
> > needs to be checked, although it isn't really that important. For this
> > particular naming scheme you need to specify all the SoCs for which
> > given compatible string can be used for this IP anyway, because there is
> > usually no other source of information about this available (except
> > directly comparing two datasheets...).
> 
> Better get the DT stuff correctly right from the start. That's why I'm asking 
> what chips contains the IP block, so we can guess the right name.

The name is fine, please stop this bikeshedding.

Maxime
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
new file mode 100644
index 0000000..356563b
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
@@ -0,0 +1,9 @@ 
+* Allwinner Security System found on A20 SoC
+
+Required properties:
+- compatible : Should be "allwinner,sun7i-a20-crypto".
+- reg: Should contain the SS register location and length.
+- interrupts: Should contain the IRQ line for the SS.
+- clocks : A phandle to the functional clock node of the SS module
+- clock-names : Name of the functional clock, should be "ahb" and "mod".
+