Re: [PATCH v4 11/13] ASoC: arizona-jack: Cleanup logging

2021-01-30 Thread Charles Keepax
On Sat, Jan 23, 2021 at 01:17:18PM +0100, Hans de Goede wrote:
> Cleanup the use of dev_foo functions used for logging:
> 
> 1. Many of these are unnecessarily split over multiple lines
> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFER
>return value
> 
> Suggested-by: Andy Shevchenko 
> Signed-off-by: Hans de Goede 
> ---

Acked-by: Charles Keepax 
Tested-by: Charles Keepax 

Thanks,
Charles


Re: [PATCH v4 11/13] ASoC: arizona-jack: Cleanup logging

2021-01-24 Thread Hans de Goede
Hi,

On 1/24/21 8:53 PM, Andy Shevchenko wrote:
> On Sat, Jan 23, 2021 at 2:17 PM Hans de Goede  wrote:
>>
>> Cleanup the use of dev_foo functions used for logging:
>>
>> 1. Many of these are unnecessarily split over multiple lines
>> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFER
>>return value
> 
> ...
> 
>> if (IS_ERR(info->micd_pol_gpio)) {
>> ret = PTR_ERR(info->micd_pol_gpio);
>> -   dev_err(arizona->dev,
>> -   "Failed to get microphone polarity GPIO: 
>> %d\n",
>> -   ret);
>> +   dev_err_probe(arizona->dev, ret, "getting microphone 
>> polarity GPIO\n");
>> return ret;
>> }
> 
> I still think that using dev_err_probe() naturally, i.e. as a part of
> the return statement is better.

Just because it can be used that way does not mean that it must be used that 
way.

More importantly I don't think that this small tihng is worth doing a v5 of
this large series for. But if a v5 is necessary for other reasons,
then I'll change this into:

 return dev_err_probe(arizona->dev, ret, "getting microphone polarity GPIO\n");

Regards,

Hans



Re: [PATCH v4 11/13] ASoC: arizona-jack: Cleanup logging

2021-01-24 Thread Andy Shevchenko
On Sat, Jan 23, 2021 at 2:17 PM Hans de Goede  wrote:
>
> Cleanup the use of dev_foo functions used for logging:
>
> 1. Many of these are unnecessarily split over multiple lines
> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFER
>return value

...

> if (IS_ERR(info->micd_pol_gpio)) {
> ret = PTR_ERR(info->micd_pol_gpio);
> -   dev_err(arizona->dev,
> -   "Failed to get microphone polarity GPIO: 
> %d\n",
> -   ret);
> +   dev_err_probe(arizona->dev, ret, "getting microphone 
> polarity GPIO\n");
> return ret;
> }

I still think that using dev_err_probe() naturally, i.e. as a part of
the return statement is better.

-- 
With Best Regards,
Andy Shevchenko