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

Krzysztof Kozlowski
2026-04-29T08:07:52Z

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