On 23/01/2023 08:32, Li Chen wrote:
> Create a vendor directory for Ambarella, and add
> cpuid, rct, scratchpad documents.
>
> Signed-off-by: Li Chen <[email protected]>
> Change-Id: I2c29e45c08666489b0d9b588ac37d713f5b723d1
Please run scripts/checkpatch.pl and fix reported warnings.
Applies to all your patches. Also test them... I have doubts that you
tested if you actually ignored checkpatch :/
> ---
> .../arm/ambarella/ambarella,cpuid.yaml | 24 +++++++++++++++++++
> .../bindings/arm/ambarella/ambarella,rct.yaml | 24 +++++++++++++++++++
> .../arm/ambarella/ambarella,scratchpad.yaml | 24 +++++++++++++++++++
> .../bindings/arm/ambarella/ambarella.yaml | 22 +++++++++++++++++
> MAINTAINERS | 4 ++++
> 5 files changed, 98 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
> new file mode 100644
> index 000000000000..1f4d9cec8f92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
This goes to soc
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ambarella,cpuid.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ambarella SoC ID
> +
> +maintainers:
> + - Li Chen <[email protected]>
Missing description.
> +
> +properties:
> + compatible:
> + const: "ambarella,cpuid", "syscon"
Drop quotes (applies to all your patches)
Missing SoC specific compatible.
> +
> + reg:
> + maxItems: 1
Missing additionalProperties. sorry, start from scratch from some
existing recent bindings or better example-schema.
> +
> +examples:
> + - |
> + cpuid_syscon: cpuid@e0000000 {
> + compatible = "ambarella,cpuid", "syscon";
> + reg = <0xe0000000 0x1000>;
> + };
> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> new file mode 100644
> index 000000000000..7279bab17d9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ambarella,rct.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ambarella RCT module
> +
> +maintainers:
> + - Li Chen <[email protected]>
> +
> +properties:
> + compatible:
> + const: "ambarella,rct", "syscon"
All the same problems.
> +
> + reg:
> + maxItems: 1
> +
> +examples:
> + - |
> + rct_syscon: rct_syscon@ed080000 {
Really? Just take a look and you will see wrong indentation. Also drop
underscores in node names and "rct". Node names should be generic.
> + compatible = "ambarella,rct", "syscon";
> + reg = <0xed080000 0x1000>;
> + };
> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> new file mode 100644
> index 000000000000..5d2bd243b5c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ambarella,scratchpad.yaml#
That's not a clock controller!
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ambarella Scratchpad
> +
> +maintainers:
> + - Li Chen <[email protected]>
> +
> +properties:
> + compatible:
> + const: "ambarella,scratchpad", "syscon"
> +
> + reg:
> + maxItems: 1
> +
> +examples:
> + - |
> + scratchpad_syscon: scratchpad_syscon@e0022000 {
All the same problems.
> + compatible = "ambarella,scratchpad", "syscon";
> + reg = <0xe0022000 0x100>;
> + };
> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
> new file mode 100644
> index 000000000000..5991bd745c05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/ambarella.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ambarella SoC Device Tree Bindings
> +
> +maintainers:
> + - Li Chen <[email protected]>
> +
> +properties:
> + $nodename:
> + const: "/"
> + compatible:
> + oneOf:
> + - description: Ambarella SoC based platforms
> + items:
> + - enum:
> + - ambarella,s6lm
What is this? How do you expect it to apply? Can you try by yourself?
Best regards,
Krzysztof
On Mon, 23 Jan 2023 16:07:32 +0800,
Krzysztof Kozlowski wrote:
>
> On 23/01/2023 08:32, Li Chen wrote:
> > Create a vendor directory for Ambarella, and add
> > cpuid, rct, scratchpad documents.
> >
> > Signed-off-by: Li Chen <[email protected]>
> > Change-Id: I2c29e45c08666489b0d9b588ac37d713f5b723d1
>
> Please run scripts/checkpatch.pl and fix reported warnings.
>
> Applies to all your patches. Also test them... I have doubts that you
> tested if you actually ignored checkpatch :/
Yeah, I checkpatch all patches, and have planned to fix Change-Id finally(manually),
but forget it before sending mails, my bad, sorry. I will remove it in v2.
> > ---
> > .../arm/ambarella/ambarella,cpuid.yaml | 24 +++++++++++++++++++
> > .../bindings/arm/ambarella/ambarella,rct.yaml | 24 +++++++++++++++++++
> > .../arm/ambarella/ambarella,scratchpad.yaml | 24 +++++++++++++++++++
> > .../bindings/arm/ambarella/ambarella.yaml | 22 +++++++++++++++++
> > MAINTAINERS | 4 ++++
> > 5 files changed, 98 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
> > create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> > create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> > create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
> > new file mode 100644
> > index 000000000000..1f4d9cec8f92
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
>
> This goes to soc
Thanks, I wasn't aware that there is a document dir named soc. I will move cpuid yaml
to bindings/soc/ambarella/, and leave other yaml still here.
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/ambarella,cpuid.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ambarella SoC ID
> > +
> > +maintainers:
> > + - Li Chen <[email protected]>
>
> Missing description.
Sorry, description will be added in v2. BTW, does other YAMLs in this patch
also need descriptions?
> > +
> > +properties:
> > + compatible:
> > + const: "ambarella,cpuid", "syscon"
>
> Drop quotes (applies to all your patches)
OK, thanks!
> Missing SoC specific compatible.
>
> > +
> > + reg:
> > + maxItems: 1
>
> Missing additionalProperties. sorry, start from scratch from some
> existing recent bindings or better example-schema.
Good to know that there is example-schema, thanks!
> > +
> > +examples:
> > + - |
> > + cpuid_syscon: cpuid@e0000000 {
> > + compatible = "ambarella,cpuid", "syscon";
> > + reg = <0xe0000000 0x1000>;
> > + };
> > diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> > new file mode 100644
> > index 000000000000..7279bab17d9e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/ambarella,rct.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ambarella RCT module
> > +
> > +maintainers:
> > + - Li Chen <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: "ambarella,rct", "syscon"
>
> All the same problems.
Well noted.
> > +
> > + reg:
> > + maxItems: 1
> > +
> > +examples:
> > + - |
> > + rct_syscon: rct_syscon@ed080000 {
>
> Really? Just take a look and you will see wrong indentation. Also drop
> underscores in node names and "rct". Node names should be generic.
Sorry for the wrong indentation, will fix it in v2.
Is it ok to contain underscores in lable? if so, I will change it into
rct_syscon: syscon@ed080000 {
in v2.
>
> > + compatible = "ambarella,rct", "syscon";
> > + reg = <0xed080000 0x1000>;
> > + };
> > diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> > new file mode 100644
> > index 000000000000..5d2bd243b5c9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/ambarella,scratchpad.yaml#
>
> That's not a clock controller!
Sorry, will fix it in v2.
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ambarella Scratchpad
> > +
> > +maintainers:
> > + - Li Chen <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: "ambarella,scratchpad", "syscon"
> > +
> > + reg:
> > + maxItems: 1
> > +
> > +examples:
> > + - |
> > + scratchpad_syscon: scratchpad_syscon@e0022000 {
>
> All the same problems.
Well noted.
> > + compatible = "ambarella,scratchpad", "syscon";
> > + reg = <0xe0022000 0x100>;
> > + };
> > diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
> > new file mode 100644
> > index 000000000000..5991bd745c05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
> > @@ -0,0 +1,22 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/ambarella.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ambarella SoC Device Tree Bindings
> > +
> > +maintainers:
> > + - Li Chen <[email protected]>
> > +
> > +properties:
> > + $nodename:
> > + const: "/"
> > + compatible:
> > + oneOf:
> > + - description: Ambarella SoC based platforms
> > + items:
> > + - enum:
> > + - ambarella,s6lm
>
> What is this? How do you expect it to apply? Can you try by yourself?
Sorry, I didn't find this file is duplicited with outside ambarella.yaml.
I will remove it in v2.
Thanks for your review!
Regards,
Li
On 23/01/2023 16:09, Li Chen wrote:
> On Mon, 23 Jan 2023 16:07:32 +0800,
> Krzysztof Kozlowski wrote:
>>
>> On 23/01/2023 08:32, Li Chen wrote:
>>> Create a vendor directory for Ambarella, and add
>>> cpuid, rct, scratchpad documents.
>>>
>>> Signed-off-by: Li Chen <[email protected]>
>>> Change-Id: I2c29e45c08666489b0d9b588ac37d713f5b723d1
>>
>> Please run scripts/checkpatch.pl and fix reported warnings.
>>
>> Applies to all your patches. Also test them... I have doubts that you
>> tested if you actually ignored checkpatch :/
>
> Yeah, I checkpatch all patches, and have planned to fix Change-Id finally(manually),
> but forget it before sending mails, my bad, sorry. I will remove it in v2.
>
>>> ---
>>> .../arm/ambarella/ambarella,cpuid.yaml | 24 +++++++++++++++++++
>>> .../bindings/arm/ambarella/ambarella,rct.yaml | 24 +++++++++++++++++++
>>> .../arm/ambarella/ambarella,scratchpad.yaml | 24 +++++++++++++++++++
>>> .../bindings/arm/ambarella/ambarella.yaml | 22 +++++++++++++++++
>>> MAINTAINERS | 4 ++++
>>> 5 files changed, 98 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
>>> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
>>> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella,scratchpad.yaml
>>> create mode 100644 Documentation/devicetree/bindings/arm/ambarella/ambarella.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
>>> new file mode 100644
>>> index 000000000000..1f4d9cec8f92
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,cpuid.yaml
>>
>> This goes to soc
>
> Thanks, I wasn't aware that there is a document dir named soc. I will move cpuid yaml
> to bindings/soc/ambarella/, and leave other yaml still here.
However if device has chip identification features (chipid), then the
location is "hwinfo".
>
>>> @@ -0,0 +1,24 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/ambarella,cpuid.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Ambarella SoC ID
>>> +
>>> +maintainers:
>>> + - Li Chen <[email protected]>
>>
>> Missing description.
>
> Sorry, description will be added in v2. BTW, does other YAMLs in this patch
> also need descriptions?
In general yes - we want descriptions which will bring additional
information. Description should not repeat title, but add more data. For
trivial cases - maybe actually this one SoC ID - you can skip it.
>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: "ambarella,cpuid", "syscon"
>>
>> Drop quotes (applies to all your patches)
>
> OK, thanks!
>
>> Missing SoC specific compatible.
>>
>>> +
>>> + reg:
>>> + maxItems: 1
>>
>> Missing additionalProperties. sorry, start from scratch from some
>> existing recent bindings or better example-schema.
>
> Good to know that there is example-schema, thanks!
>
>>> +
>>> +examples:
>>> + - |
>>> + cpuid_syscon: cpuid@e0000000 {
>>> + compatible = "ambarella,cpuid", "syscon";
>>> + reg = <0xe0000000 0x1000>;
>>> + };
>>> diff --git a/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
>>> new file mode 100644
>>> index 000000000000..7279bab17d9e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/ambarella/ambarella,rct.yaml
>>> @@ -0,0 +1,24 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/ambarella,rct.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Ambarella RCT module
>>> +
>>> +maintainers:
>>> + - Li Chen <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + const: "ambarella,rct", "syscon"
>>
>> All the same problems.
>
> Well noted.
>
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> +examples:
>>> + - |
>>> + rct_syscon: rct_syscon@ed080000 {
>>
>> Really? Just take a look and you will see wrong indentation. Also drop
>> underscores in node names and "rct". Node names should be generic.
>
> Sorry for the wrong indentation, will fix it in v2.
>
> Is it ok to contain underscores in lable? if so, I will change it into
>
> rct_syscon: syscon@ed080000 {
Yes, label can have it.
Best regards,
Krzysztof