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

["Arnd Bergmann" <arnd@arndb.de>]

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