[0/8] Nuvoton WPCM450 FIU SPI flash controller

This patchset adds DT bindings and a driver for the Flash Interface Unit
(FIU), 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 existing NPCM7xx FIU driver is sufficitently incompatible with the
WPCM450 FIU that I decided to write a new driver.

This patchset depends on the WPCM450 clock support patchset,

	[PATCH v5 0/6] Nuvoton WPCM450 clock and reset driver


Jonathan


Jonathan Neuschäfer (8):
  pinctrl: nuvoton: wpcm450: Refactor MFSEL setting code
  pinctrl: nuvoton: wpcm450: Fix handling of inverted MFSEL bits
  dt-bindings: spi: Add Nuvoton WPCM450 Flash Interface Unit (FIU)
  dt-bindings: mfd: syscon: Add nuvoton,wpcm450-shm
  ARM: dts: wpcm450: Add FIU SPI controller node
  ARM: dts: wpcm450-supermicro-x9sci-ln4f: Add SPI flash
  spi: wpcm-fiu: Add driver for Nuvoton WPCM450 Flash Interface Unit
    (FIU)
  spi: wpcm-fiu: Add direct map support

 .../devicetree/bindings/mfd/syscon.yaml       |   1 +
 .../bindings/spi/nuvoton,wpcm450-fiu.yaml     |  76 +++
 .../nuvoton-wpcm450-supermicro-x9sci-ln4f.dts |   9 +
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi        |  16 +
 drivers/pinctrl/nuvoton/pinctrl-wpcm450.c     |  32 +-
 drivers/spi/Kconfig                           |  11 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-wpcm-fiu.c                    | 507 ++++++++++++++++++
 8 files changed, 644 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.yaml
 create mode 100644 drivers/spi/spi-wpcm-fiu.c

--
2.35.1

On 05/11/2022 19:59, Jonathan Neuschäfer wrote:
> The Shared Memory interface (SHM) is a piece of hardware in Nuvoton BMCs
> that allows a host processor (connected via LPC) to access flash and RAM
> that belong to the BMC. The SHM includes a register block accessible from
> the BMC side.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

On Sat, 05 Nov 2022 19:59:06 +0100, 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
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.example.dts:18:18: fatal error: dt-bindings/clock/nuvoton,wpcm450-clk.h: No such file or directory
   18 |         #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/spi/nuvoton,wpcm450-fiu.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

On Sun, Nov 06, 2022 at 10:38:45AM +0100, Krzysztof Kozlowski wrote:
> 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>
> > ---
[...]
> > +allOf:
> > +  - $ref: "/schemas/spi/spi-controller.yaml#"
> 
> Drop the quotes.

Will do.


> > +
> > +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.

My intention was rather to make the second reg item actually optional,
i.e. add minItems: 1 for reg as well.  (But, further discussion below.)


> > +    spi@c8000000 {
> > +      compatible = "nuvoton,wpcm450-fiu";
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      reg = <0xc8000000 0x1000>, <0xc0000000 0x4000000>;
> 
> reg is the second property.

Ok, I'll move it up.

> 
> > +      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.

Ok, I'll add more reasoning, which is basically: The "memory" mapping is
only an optimization for faster access, knowledge of it is not necessary
for full operation of the device.

> Not every instance on the hardware has it?

AFAIK every instance has it, and there's unlikely to be any variation on
this fact anymore, because newer Nuvoton SoCs replaced the FIU with a
redesigned and incompatible version.

I admit that the value of making the "memory" mapping optional is rather
theoretical, and I'm open to making this reg item mandatory to simplify
the binding.

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

Indeed.


Thanks for your review,
Jonathan