Hi Andy,
> @@ -350,8 +348,8 @@ struct work_priv_container {
> * @extclockenable: true if the external clock is to be enabled
> * @extclockfreq: frequency of the external clock
> * @extclockgpio: ca8210 output gpio of the external clock
> - * @gpio_reset: gpio number of ca8210 reset line
> - * @gpio_irq: gpio number of ca8210 interrupt line
> + * @reset_gpio: GPIO of ca8210 reset line
What about "CA8210 Reset GPIO line"? Or Just "Reset GPIO line"? Or even
"Reset GPIO descriptor" (whatever).
> + * @irq_gpio: GPIO of ca8210 interrupt line
Same
> * @irq_id: identifier for the ca8210 irq
> *
> */
> @@ -359,8 +357,8 @@ struct ca8210_platform_data {
> bool extclockenable;
> unsigned int extclockfreq;
> unsigned int extclockgpio;
> - int gpio_reset;
> - int gpio_irq;
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *irq_gpio;
> int irq_id;
> };
[...]
> /* Wait until wakeup indication seen */
> @@ -2784,25 +2782,14 @@ static void ca8210_unregister_ext_clock(struct
> spi_device *spi)
> */
> static int ca8210_reset_init(struct spi_device *spi)
> {
> - int ret;
> - struct ca8210_platform_data *pdata = spi->dev.platform_data;
> + struct device *dev = &spi->dev;
> + struct ca8210_platform_data *pdata = dev_get_platdata(dev);
>
Can you either mention the additional cleanup that you do in the commit
log or split it in a separate commit? (splitting is probably not
necessary here given that most of the cleanup anyway is related to the
actual changes.
> - pdata->gpio_reset = of_get_named_gpio(
> - spi->dev.of_node,
> - "reset-gpio",
> - 0
> - );
> + pdata->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(pdata->reset_gpio))
> + dev_crit(dev, "Reset GPIO did not set to output mode\n");
>
> - ret = gpio_direction_output(pdata->gpio_reset, 1);
> - if (ret < 0) {
> - dev_crit(
> - &spi->dev,
> - "Reset GPIO %d did not set to output mode\n",
> - pdata->gpio_reset
> - );
> - }
> -
> - return ret;
> + return PTR_ERR_OR_ZERO(pdata->reset_gpio);
This is not a strong request, but in general I think it is preferred to return
immediately, so this looks easier to understand:
+ pdata->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(pdata->reset_gpio)) {
+ dev_crit(dev, "Reset GPIO did not set to output mode\n");
+ return PTR_ERR(pdata->reset_pgio);
+ }
+
+ return 0;
Otherwise the rest lgtm.
Thanks,
Miquèl
