Re: [PATCH 1/5] dt-bindings: nvmem: add google,gs101-otp

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2937 bytes --]

On Fri, Oct 31, 2025 at 12:45:09PM +0000, Tudor Ambarus wrote:
> Add binding for the OTP controller found on Google GS101.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  .../bindings/nvmem/google,gs101-otp.yaml           | 68 ++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/google,gs101-otp.yaml b/Documentation/devicetree/bindings/nvmem/google,gs101-otp.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..2144911297beb89337b0389b30fe6609db4156ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/google,gs101-otp.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/google,gs101-otp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google GS101 OTP Controller
> +
> +maintainers:
> +  - Tudor Ambarus <tudor.ambarus@linaro.org>
> +
> +description: |
> +  OTP controller drives a NVMEM memory where system or user specific data
> +  can be stored. The OTP controller register space if of interest as well
> +  because it contains dedicated registers where it stores the Product ID
> +  and the Chip ID (apart other things like TMU or ASV info).
> +
> +allOf:
> +  - $ref: nvmem.yaml#
> +  - $ref: nvmem-deprecated-cells.yaml

Why are the deprecated cells needed here?
|  Before introducing NVMEM layouts all NVMEM (fixed) cells were defined
|  as direct device subnodes. That syntax was replaced by "fixed-layout"
|  and is deprecated now. No new bindings should use it.

> +
> +properties:
> +  compatible:
> +    items:
> +      - const: google,gs101-otp
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: pclk

Why bother with clock-names when you only have one clock? Are you
anticipating a variant with more?

> +
> +  reg:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - clock-names
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/google,gs101.h>
> +
> +    otp: efuse@10000000 {
> +        compatible = "google,gs101-otp";
> +        reg = <0x10000000 0xf084>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        clocks = <&cmu_misc CLK_GOUT_MISC_OTP_CON_TOP_PCLK>;
> +        clock-names = "pclk";
> +
> +        product_id: product_id@0 {

Why does this node name have an underscore?

Additionally, all nodes here should lose their labels.

pw-bot: changes-requested

> +            reg = <0x0 0x4>;
> +        };
> +
> +        chip_id: chip-id@4 {
> +            reg = <0x4 0x10>;
> +        };
> +    };
> 
> -- 
> 2.51.1.930.gacf6e81ea2-goog
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]