Re: [PATCH v3 14/16] media: i2c: Use dev_err_probe() in ov8865

[Daniel Scally <djrscally@gmail.com>]

Hi Hans

On 28/10/2021 15:01, Hans de Goede wrote:
> Hi Dan,
>
> On 10/21/21 23:43, Daniel Scally wrote:
>> There is a chance that regulator_get() returns -EPROBE_DEFER, in which
>> case printing an error message is undesirable. To avoid spurious messages
>> in dmesg in the event that -EPROBE_DEFER is returned, use dev_err_probe()
>> on error paths for regulator_get().
>>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>
>
>> ---
>> Changes in v3:
>>
>> 	- None
>>
>>  drivers/media/i2c/ov8865.c | 46 +++++++++++++++++---------------------
>>  1 file changed, 20 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
>> index 572b9818e950..685539744041 100644
>> --- a/drivers/media/i2c/ov8865.c
>> +++ b/drivers/media/i2c/ov8865.c
>> @@ -2955,6 +2955,26 @@ static int ov8865_probe(struct i2c_client *client)
>>  	sensor->dev = dev;
>>  	sensor->i2c_client = client;
>>  
>> +	/* Regulators */
>> +
>> +	/* DVDD: digital core */
>> +	sensor->dvdd = devm_regulator_get(dev, "dvdd");
>> +	if (IS_ERR(sensor->dvdd))
>> +		return dev_err_probe(dev, PTR_ERR(sensor->dvdd),
>> +				     "cannot get DVDD regulator\n");
>> +
>> +	/* DOVDD: digital I/O */
>> +	sensor->dovdd = devm_regulator_get(dev, "dovdd");
>> +	if (IS_ERR(sensor->dovdd))
>> +		return dev_err_probe(dev, PTR_ERR(sensor->dovdd),
>> +				     "cannot get DOVDD regulator\n");
>> +
>> +	/* AVDD: analog */
>> +	sensor->avdd = devm_regulator_get(dev, "avdd");
>> +	if (IS_ERR(sensor->avdd))
>> +		return dev_err_probe(dev, PTR_ERR(sensor->avdd),
>> +				     "cannot get AVDD (analog) regulator\n");
>> +
>>  	/* Graph Endpoint */
>>  
>>  	handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
>> @@ -2985,32 +3005,6 @@ static int ov8865_probe(struct i2c_client *client)
>>  		goto error_endpoint;
>>  	}
>>  
>> -	/* Regulators */
>> -
>> -	/* DVDD: digital core */
>> -	sensor->dvdd = devm_regulator_get(dev, "dvdd");
>> -	if (IS_ERR(sensor->dvdd)) {
>> -		dev_err(dev, "cannot get DVDD (digital core) regulator\n");
>> -		ret = PTR_ERR(sensor->dvdd);
>> -		goto error_endpoint;
>> -	}
>> -
>> -	/* DOVDD: digital I/O */
>> -	sensor->dovdd = devm_regulator_get(dev, "dovdd");
>> -	if (IS_ERR(sensor->dovdd)) {
>> -		dev_err(dev, "cannot get DOVDD (digital I/O) regulator\n");
>> -		ret = PTR_ERR(sensor->dovdd);
>> -		goto error_endpoint;
>> -	}
>> -
>> -	/* AVDD: analog */
>> -	sensor->avdd = devm_regulator_get(dev, "avdd");
>> -	if (IS_ERR(sensor->avdd)) {
>> -		dev_err(dev, "cannot get AVDD (analog) regulator\n");
>> -		ret = PTR_ERR(sensor->avdd);
>> -		goto error_endpoint;
>> -	}
>> -
>>  	/* External Clock */
>>  
> This line:
>
>>  	sensor->extclk = devm_clk_get(dev, "tps68470-clk");
> Does not exist in the upstream repos, instead it is:
>
> 	sensor->extclk = devm_clk_get(dev, NULL);
>
> I guess you still had your hack to deal with the clk issues we've
> been working on in place in your tree on which you based this series.
>
> Unfortunately this means that this patch (and thus this series)
> will not apply cleanly.
>
> Can you send a v4 fixing this?
>

Oops! My bad, sorry about that. I'll post a v4 later to clean that up