Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <066919b1-c43d-f8ed-0191-cce8c575ee37@linaro.org>
Date:   Sun, 6 Nov 2022 10:38:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH 3/8] dt-bindings: spi: Add Nuvoton WPCM450 Flash Interface
 Unit (FIU)
Content-Language: en-US
To:     =?UTF-8?Q?Jonathan_Neusch=c3=a4fer?= <j.neuschaefer@gmx.net>,
        linux-spi@vger.kernel.org, openbmc@lists.ozlabs.org
Cc:     Lee Jones <lee@kernel.org>, Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Mark Brown <broonie@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-gpio@vger.kernel.org
References: <20221105185911.1547847-1-j.neuschaefer@gmx.net>
 <20221105185911.1547847-4-j.neuschaefer@gmx.net>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221105185911.1547847-4-j.neuschaefer@gmx.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 05/11/2022 19:59, Jonathan Neuschäfer wrote:
> The Flash Interface Unit (FIU) is the SPI flash controller in the
> Nuvoton WPCM450 BMC SoC. It supports four chip selects, and direct
> (memory-mapped) access to 16 MiB per chip. Larger flash chips can be
> accessed by software-defined SPI transfers.
> 
> The FIU in newer NPCM7xx SoCs is not compatible with the WPCM450 FIU.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
>  .../bindings/spi/nuvoton,wpcm450-fiu.yaml     | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.yaml b/Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.yaml
> new file mode 100644
> index 0000000000000..dc0ea2184f8d0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/nuvoton,wpcm450-fiu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton WPCM450 Flash Interface Unit (FIU)
> +
> +maintainers:
> +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> +
> +allOf:
> +  - $ref: "/schemas/spi/spi-controller.yaml#"

Drop the quotes.

> +
> +properties:
> +  compatible:
> +    const: nuvoton,wpcm450-fiu
> +
> +  reg:
> +    items:
> +      - description: FIU registers
> +      - description: Memory-mapped flash contents
> +
> +  reg-names:
> +    items:
> +      - const: control
> +      - const: memory
> +    minItems: 1

This does not match your 'reg'. Two items are required there.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  nuvoton,shm:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: a phandle to the SHM block (see ../arm/nuvoton,shm.yaml)
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> +    spi@c8000000 {
> +      compatible = "nuvoton,wpcm450-fiu";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      reg = <0xc8000000 0x1000>, <0xc0000000 0x4000000>;

reg is the second property.

> +      reg-names = "control", "memory";
> +      clocks = <&clk WPCM450_CLK_FIU>;
> +      nuvoton,shm = <&shm>;
> +
> +      flash@0 {
> +        compatible = "jedec,spi-nor";
> +      };
> +    };
> +
> +    shm: syscon@c8001000 {
> +      compatible = "nuvoton,wpcm450-shm", "syscon";
> +      reg = <0xc8001000 0x1000>;
> +    };
> +
> +  - |
> +    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> +    spi@c8000000 {
> +      compatible = "nuvoton,wpcm450-fiu";
> +      // the "memory" resource may be omitted

This is rather obvious, so what you should comment is WHY or WHEN second
resource can be omitted.

Not every instance on the hardware has it?

Just to remind - this is the description of hardware, not Linux behavior.

> +      reg = <0xc8000000 0x1000>;
> +      reg-names = "control";
> +    };
> --
> 2.35.1
> 

Best regards,
Krzysztof