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 <lchen@xxxxxxxxxxxxx> >>> 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 <lchen@xxxxxxxxxxxxx> >> >> 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 <lchen@xxxxxxxxxxxxx> >>> + >>> +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