linux-kernel - Re: [PATCH 1/2] iio: adc: ad7291: convert to device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ee1dfe4-3289-4428-5787-6dddd63111b2@metafoo.de>
Date:   Tue, 17 Mar 2020 15:16:12 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Michael Auchter <michael.auchter@...com>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Stefan Popa <stefan.popa@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org
Subject: Re: [PATCH 1/2] iio: adc: ad7291: convert to device tree

On 3/17/20 2:56 PM, Michael Auchter wrote:
> There are no in-tree users of the platform data for this driver, so
> remove it and convert the driver to use device tree instead.
> 
> Signed-off-by: Michael Auchter <michael.auchter@...com>

Hi,

Thanks for the patch, looks good for the most part. One comment inline.

> 
> diff --git a/drivers/iio/adc/ad7291.c b/drivers/iio/adc/ad7291.c
> index b2b137fed246..536e31862309 100644
> --- a/drivers/iio/adc/ad7291.c
> +++ b/drivers/iio/adc/ad7291.c
> @@ -495,8 +482,19 @@ static int ad7291_probe(struct i2c_client *client,
>   			AD7291_T_SENSE_MASK | /* Tsense always enabled */
>   			AD7291_ALERT_POLARITY; /* set irq polarity low level */
>   
> -	if (pdata && pdata->use_external_ref)
> +	chip->reg = devm_regulator_get_optional(&client->dev, "vref");
> +	if (IS_ERR(chip->reg)) {
> +		if (PTR_ERR(chip->reg) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;

We should only continue if the error is ENODEV, which means that no 
regulator was specified. Any other error means it was specified but 
there was an issue requesting it. See for example ad7266.c

> +
> +		chip->reg = NULL;
> +	} else {
> +		ret = regulator_enable(chip->reg);
> +		if (ret)
> +			return ret;
> +
>   		chip->command |= AD7291_EXT_REF;
> +	}