2024-03-18 21:57:20

by David Lechner

Subject: [PATCH] iio: adc: ad7944: simplify adi,spi-mode property parsing

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.

Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: David Lechner <[email protected]>
---
drivers/iio/adc/ad7944.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c
index d5ec6b5a41c7..356aea02920f 100644
--- a/drivers/iio/adc/ad7944.c
+++ b/drivers/iio/adc/ad7944.c
@@ -366,7 +366,6 @@ static int ad7944_probe(struct spi_device *spi)
struct ad7944_adc *adc;
bool have_refin = false;
struct regulator *ref;
- const char *str_val;
int ret;

indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
@@ -382,16 +381,18 @@ static int ad7944_probe(struct spi_device *spi)

adc->timing_spec = chip_info->timing_spec;

- if (device_property_read_string(dev, "adi,spi-mode", &str_val) == 0) {
- ret = sysfs_match_string(ad7944_spi_modes, str_val);
- if (ret < 0)
- return dev_err_probe(dev, -EINVAL,
- "unsupported adi,spi-mode\n");
-
- adc->spi_mode = ret;
- } else {
+ ret = device_property_match_property_string(dev, "adi,spi-mode",
+ ad7944_spi_modes,
+ ARRAY_SIZE(ad7944_spi_modes));
+ if (ret < 0) {
/* absence of adi,spi-mode property means default mode */
- adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
+ if (ret == -EINVAL)
+ adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
+ else
+ return dev_err_probe(dev, ret,
+ "getting adi,spi-mode property failed\n");
+ } else {
+ adc->spi_mode = ret;
}

if (adc->spi_mode == AD7944_SPI_MODE_CHAIN)

---
base-commit: 1446d8ef48196409f811c25071b2cc510a49fc60
change-id: 20240318-ad7944-cleanups-9f95a7c598b6

2024-03-18 22:44:10

by Andy Shevchenko

Subject: Re: [PATCH] iio: adc: ad7944: simplify adi,spi-mode property parsing

On Mon, Mar 18, 2024 at 11:57 PM David Lechner <[email protected]> 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

..

> + ret = device_property_match_property_string(dev, "adi,spi-mode",
> + ad7944_spi_modes,
> + ARRAY_SIZE(ad7944_spi_modes));
> + if (ret < 0) {
> /* absence of adi,spi-mode property means default mode */
> - adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
> + if (ret == -EINVAL)
> + adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
> + else
> + return dev_err_probe(dev, ret,
> + "getting adi,spi-mode property failed\n");

No need to have 'else'

if (ret != -EINVAL)
return dev_err_probe(dev, ret, "getting
adi,spi-mode property failed\n");

/* absence of adi,spi-mode property means default mode */
adc->spi_mode = AD7944_SPI_MODE_DEFAULT;

> + } else {
> + adc->spi_mode = ret;
> }

--
With Best Regards,
Andy Shevchenko

2024-03-18 22:50:01

by David Lechner

Subject: Re: [PATCH] iio: adc: ad7944: simplify adi,spi-mode property parsing

On 3/18/24 5:43 PM, Andy Shevchenko wrote:
> On Mon, Mar 18, 2024 at 11:57 PM David Lechner <[email protected]> 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
>
> ...
>
>> + ret = device_property_match_property_string(dev, "adi,spi-mode",
>> + ad7944_spi_modes,
>> + ARRAY_SIZE(ad7944_spi_modes));
>> + if (ret < 0) {
>> /* absence of adi,spi-mode property means default mode */
>> - adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>> + if (ret == -EINVAL)
>> + adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>> + else
>> + return dev_err_probe(dev, ret,
>> + "getting adi,spi-mode property failed\n");
>
> No need to have 'else'
>
> if (ret != -EINVAL)
> return dev_err_probe(dev, ret, "getting
> adi,spi-mode property failed\n");
>
> /* absence of adi,spi-mode property means default mode */
> adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>
>> + } else {
>> + adc->spi_mode = ret;
>> }
>

I agree it is better that way. Will send a v2.