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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.


>  		goto err_free;
>  	}
>  
> -	ret = gpio_request(p54spi_gpio_irq, "p54spi irq");
> -	if (ret < 0) {
> -		dev_err(&priv->spi->dev, "irq GPIO request failed: %d", ret);
> -		goto err_free_gpio_power;
> -	}
> -
> -	gpio_direction_output(p54spi_gpio_power, 0);
> -	gpio_direction_input(p54spi_gpio_irq);
> -
> -	ret = request_irq(gpio_to_irq(p54spi_gpio_irq),
> -			  p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi",
> -			  priv->spi);
> +	ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi);
>  	if (ret < 0) {
>  		dev_err(&priv->spi->dev, "request_irq() failed");
> -		goto err_free_gpio_irq;
> +		goto err_free_gpio_power;
>  	}
>  
> -	irq_set_irq_type(gpio_to_irq(p54spi_gpio_irq), IRQ_TYPE_EDGE_RISING);
>  
>  	INIT_WORK(&priv->work, p54spi_work);
>  	init_completion(&priv->fw_comp);
> @@ -659,11 +636,9 @@ static int p54spi_probe(struct spi_device *spi)
>  
>  err_free_common:
>  	release_firmware(priv->firmware);
> -	free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
> -err_free_gpio_irq:
> -	gpio_free(p54spi_gpio_irq);
> +	free_irq(priv->irq, spi);
>  err_free_gpio_power:
> -	gpio_free(p54spi_gpio_power);
> +	gpiod_put(priv->gpio_powerdown);
>  err_free:
>  	p54_free_common(priv->hw);
>  	return ret;
> @@ -675,10 +650,8 @@ static void p54spi_remove(struct spi_device *spi)
>  
>  	p54_unregister_common(priv->hw);
>  
> -	free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
> -
> -	gpio_free(p54spi_gpio_power);
> -	gpio_free(p54spi_gpio_irq);
> +	free_irq(priv->irq, spi);
> +	gpiod_put(priv->gpio_powerdown);
>  	release_firmware(priv->firmware);
>  
>  	mutex_destroy(&priv->mutex);
> @@ -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

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

> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, p54spi_of_ids);
>  
>  static struct spi_driver p54spi_driver = {
>  	.driver = {
>  		.name		= "p54spi",
> +		.of_match_table = p54spi_of_ids,
>  	},


Best regards,
Krzysztof