Re: [PATCH v3 3/3] p54spi: convert to devicetree

Arnd Bergmann
2026-04-29T21:36:45Z

On Wed, Apr 29, 2026, at 10:07, Krzysztof Kozlowski wrote:
> On 27/04/2026 16:23, Arnd Bergmann wrote:
>>
>> - ret = gpio_request(p54spi_gpio_power, "p54spi power");
>> - if (ret < 0) {
>> - dev_err(&priv->spi->dev, "power GPIO request failed: %d", ret);
>> + priv->gpio_powerdown = gpiod_get(&spi->dev, "powerdown", GPIOD_OUT_HIGH);
>> + if (IS_ERR(priv->gpio_powerdown)) {
>> + ret = PTR_ERR(priv->gpio_powerdown);
>> + dev_err(&priv->spi->dev, "powerdown GPIO request failed: %d", ret);
>
> Binding said it is optional, so this cannot be a failure.
>
> Also, please use ret = dev_err_probe syntax.

Ok, fixed both.

>> @@ -686,10 +659,19 @@ static void p54spi_remove(struct spi_device *spi)
>> p54_free_common(priv->hw);
>> }
>>
>> +struct of_device_id p54spi_of_ids[] = {
>
> static const

I would have expected that to trigger a compile-time warning for a
missing declaration, not sure what happened here. Fixed now.

>> + { .compatible = "cnxt,3110x", },
>> + { .compatible = "isil,p54spi", },
>> + { .compatible = "st,stlc4550", },
>> + { .compatible = "st,stlc4560", },
>
> At least last two devices are then compatible, so this should be
> expressed in the binding with fallback and drop stlc4560 here. Maybe all
> of them are compatible.

The driver doesn't know the difference, so I assume they are
either all compatible, or the other ones don't actually work.
I've dropped everything except "st,stlc4550" now, as that is the
one I used in the dts file. I kept the other identifiers
in the binding as:

compatible:
oneOf:
- const: st,stlc4560
- items:
- enum:
- cnxt,3110x
- st,stlc4550
- isil,p54spi
- const: st,stlc4560

Not sure if that's the best way to express this.

Arnd