Zephyr: addition to winbond,w25q16.yaml required for SPI CS to be controlled by driver

[andyaoe]

if SPI_FLASH_W25QXXDV_GPIO_SPI_CS is selected in Kconfig.w25qxxdv, then the following 2 device tree variables are required in winbond,w25q16.yaml in order to compile the code

cs-gpio-pin:
  type: int
  category: optional  
  description: Pin Number used for the CS Pin
  generation: define

cs-gpio-controller:
  type: string
  category: optional
  description: Gpio Controller Used for the CS Pin
  generation: define

[ulfalizer]

Should now be this, to avoid warnings (see this section in the devicetree manual):

cs-gpio-pin:
    type: int
    required: false
    description: Pin number used for the CS pin

cs-gpio-controller:
    type: string
    required: false
    description: GPIO controller used for the CS pin

generation: define has been removed. All properties now generate #defines.

[pavlohamov]

@andyaoe Why not just use generic "jedec,spi-nor"? seems like the " winbond,w25q" driver is obsolete (sincerely it's the corner case for spi-nor flash)

[andyaoe]

OK cool, I understand by what you mean by corner case. I will try our jedec,spi-nor!

[andyaoe]

jedec,spi-nor.yaml is also missing the same fields

[pabigot]

I think this is related to #12226 and #12698 and is mostly about how chip-select is selected based on the SPI configuration. In short: we should not need to add those fields, because the SPI flash should be a child of a SPI node (identifying the controller), and should have a reg property that identifies the corresponding CS which is defined in the SPI node.

It is possible that the driver does need to be updated for some way of specifying CS that isn't normal.

So @andyaoe what is your target platform?

[andyaoe]

Hi @pabigot , my target is the nordic nrf9160

[pabigot]

@andyaoe What does your devicetree entry for the flash chip look like? For example, for nrf52840_pca10056 it's:

&spi2 {
        compatible = "nordic,nrf-spi";
        status = "okay";
        sck-pin = <19>;
        mosi-pin = <20>;
        miso-pin = <21>;
        cs-gpios = <&gpio0 17 0>, <&gpio1 5 0>;
        mx25r64: mx25r6435f@0 {
                compatible = "jedec,spi-nor";
                reg = <0>;
                spi-max-frequency = <80000000>;
                label = "MX25R64";
                jedec-id = [c2 28 17];
                size = <67108864>;
                has-be32k;
                wp-gpios = <&gpio0 22 0>;
                hold-gpios = <&gpio0 23 0>;
        };
};

which provides two chip selects, one of which is used for the SPI flash.

[andyaoe]

&spi3 {
    spi-flash@0 {
        #address-cells = <1>;
        #size-cells = <1>;
        compatible = "jedec,spi-nor";
        reg = <0x0>;
        myvar = <0x10>;
        spi-max-frequency = <1000000>;
        label = "spi_flash";
        erase-block-size = <4096>;
        write-block-size = <4096>;
        size = <1000000>;
        jedec-id = <0xc2 0x28 0x17>;
    };
};

[pabigot]

Based on the property values it looks like you copied that from a very outdated devicetree file. Assuming the JEDEC ID is right you're using an MX25R64 and should just copy the contents of the nrf52840_pca10056 one I showed. You can skip wp-gpios and hold-gpios properties as they're not used yet.

It seems likely the problem is you're simply extending the base spi3 node, which probably doesn't have a cs-gpios property. You'll need to add that so the pin can be found. Something like this builds with nrf9160_pca10090.

&spi3 {
    cs-gpios = <&gpio0 17 0>;
        mx25r64: mx25r6435f@0 {
                compatible = "jedec,spi-nor";
                reg = <0>;
                spi-max-frequency = <80000000>;
                label = "MX25R64";
                jedec-id = [c2 28 17];
                size = <67108864>;
                has-be32k;
        };
};

[andyaoe]

Awesome, it worked! Thank you so much!

[pabigot]

You're welcome.