On Tue, 19 Mar 2024 10:28:31 -0500 David Lechner <dlechner@xxxxxxxxxxxx> wrote: > On Tue, Mar 19, 2024 at 10:01 AM Andy Shevchenko > <andy.shevchenko@xxxxxxxxx> wrote: > > > > On Tue, Mar 19, 2024 at 4:28 PM David Lechner <dlechner@xxxxxxxxxxxx> wrote: > > > > > > This simplifies the adi,spi-mode property parsing by using > > > device_property_match_property_string() instead of two separate > > > functions. Also, the error return value is now more informative > > > in cases where there was problem parsing the property. > > > > a problem > > I'll fix that up. > > ... > > > > > + ret = device_property_match_property_string(dev, "adi,spi-mode", > > > + ad7944_spi_modes, > > > + ARRAY_SIZE(ad7944_spi_modes)); > > > + if (ret < 0) { > > > + if (ret != -EINVAL) > > > + return dev_err_probe(dev, ret, > > > + "getting adi,spi-mode property failed\n"); > > > > > - adc->spi_mode = ret; > > > - } else { > > > > Actually we may even leave these unchanged > > > > > /* absence of adi,spi-mode property means default mode */ > > > adc->spi_mode = AD7944_SPI_MODE_DEFAULT; > > > + } else { > > > + adc->spi_mode = ret; > > > } > > > > ret = device_property_match_property_string(dev, "adi,spi-mode", > > ad7944_spi_modes, > > > > ARRAY_SIZE(ad7944_spi_modes)); > > if (ret >= 0) { > > adc->spi_mode = ret; > > } else if (ret != -EINVAL) { > > return dev_err_probe(dev, ret, > > "getting adi,spi-mode > > property failed\n"); > > } else { > > /* absence of adi,spi-mode property means default mode */ > > adc->spi_mode = AD7944_SPI_MODE_DEFAULT; > > } > > > > But I can admit this is not an often used approach. > > > > I think Jonathan prefers to have the error path first, so I would like > to wait and see if he has an opinion here. I do prefer error paths first. Thanks. Jonathan