Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy <christophe.leroy@c-s.fr>]

Le 27/11/2019 à 14:00, Linus Walleij a écrit :
> On Wed, Nov 27, 2019 at 1:05 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>> Le 27/11/2019 à 11:55, Linus Walleij a écrit :
> 
>>> That's weird, because when ->use_gpio_descriptors is set
>>> (as for this driver) the core only attempts to look up
>>> spi->cs_gpiods and not spi->cs_gpios, and consequently
>>> can only assign spi->cd_gpiod and not spi->cs_gpio:
>>
>> That's it. ->use_gpio_descriptors isn't set for the FSL driver:
> 
> Oh, my coding mistake. :(
> 
> And an especially stupid one too. OK I make a separate patch
> in the series to fix that too.
> 
>> I have now added it, together with the DTS cs-gpios name change (without
>> your counting patch with crashes), and I get something which is almost
>> working: I get temperature back into sensors, but temperature is 0°C !!!
> 
> OK we almost fixed it I think. It is probably better to test with
> all three patches (will send out soon) that rely on the gpiolib
> to do appropriate counting of the gpiod's and so on.
> 
>> Looking into dmesg, I see:
>>
>> [    3.153521] lm74@1 GPIO handle specifies active low - ignored
>> [    3.178093] lm74@5 GPIO handle specifies active low - ignored
>>
>> Any link with the problem ?
> 
> This is because your GPIO handles look like this:
> 
>          gpios = <&CPM1_PIO_C 4 1        /* SICOFI 1 */
>                   &CPM1_PIO_B 23 1       /* TEMP MCR */
>                   &CPM1_PIO_C 8 1        /* SICOFI 2 */
>                   &CPM1_PIO_C 12 1       /* EEPROM MIAE */
>                   &CPM1_PIO_D 6 1        /* SICOFI 3 */
>                   &CPM1_PIO_B 14 1       /* TEMP MPC885 */
>                   &CPM1_PIO_B 21 1       /* EEPROM CMPC885 */
>                   &FAV_CS_SPI 0 1        /* FAV SPI */
>                   &FAV_CS_SPI 2 1>;      /* FAV POSTE FPGA */
> 
> That "1" at the end of each GPIO phandle means "active low"
> as can be seen in <dt-bindings/gpio/gpio.h>
> 
> /* Bit 0 express polarity */
> #define GPIO_ACTIVE_HIGH 0
> #define GPIO_ACTIVE_LOW 1
> 
> But your child nodes look like this:
> 
>         sicofi@0 {
>                  compatible = "infineon,sicofi";
>                  spi-max-frequency = <1000000>;
>                  reg = <0>;
>                  spi-cs-high;
>                  spi-cpha;
>          };
> 
>          lm74@1 {
>                  compatible = "ns,lm74";
>                  spi-max-frequency = <1000000>;
>                  reg = <1>;
>                  spi-cs-high;
>          };
> 
> And the spi-cs-high in the child node takes precedence.
> 
> That's a bit ambigous so that is what the warning is about.
> 
> Try to remove the "spi-cs-high" bool flag from your nodes,
> because it seems like the old code was ignoring them.
> 
> Does that solve the problem?

Yes it does. Many thanks. I let you manage the packaging of fixes.

Christophe

> 
> Yours,
> Linus Walleij
>