Re: [PATCH v2 4/6] mfd: atmel-hlcdc: return in case of error

[Claudiu.Beznea]

On 16.12.2019 18:24, Lee Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, 13 Dec 2019, Claudiu Beznea wrote:
> 
>> For HLCDC timing engine configurations bit ATMEL_HLCDC_SIP of
>> ATMEL_HLCDC_SR needs to be polled before applying new config. In case of
>> timeout there is no indicator about this, so, return in case of timeout
>> and also print a message about this.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
>> ---
>>  drivers/mfd/atmel-hlcdc.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
>> index 92bfcaa62ace..a1e46c87b956 100644
>> --- a/drivers/mfd/atmel-hlcdc.c
>> +++ b/drivers/mfd/atmel-hlcdc.c
>> @@ -40,10 +40,17 @@ static int regmap_atmel_hlcdc_reg_write(void *context, unsigned int reg,
>>
>>       if (reg <= ATMEL_HLCDC_DIS) {
>>               u32 status;
>> -
>> -             readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
>> -                                       status, !(status & ATMEL_HLCDC_SIP),
>> -                                       1, 100);
>> +             int ret;
>> +
>> +             ret = readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
>> +                                             status,
>> +                                             !(status & ATMEL_HLCDC_SIP),
>> +                                             1, 100);
>> +             if (ret) {
>> +                     dev_err(hregmap->dev,
>> +                             "Timeout waiting for ATMEL_HLCDC_SIP\n");
> 
> Nit: Just in case you have to rework this, placing register names in
> the kernel log isn't usually helpful.  Can you swap it out for a more
> user friendly message?

Sure! I'll do a v3 for it.

Thank you,
Claudiu Beznea

> 
> "Timed out waiting for ..."
> 
> ... X status
> ... Y to update
> ... setting Z configuration
> 
> Etc.
> 
>> +                     return ret;
>> +             }
>>       }
>>
>>       writel(val, hregmap->regs + reg);
> 
> --
> Lee Jones [æçæ]
> Linaro Services Technical Lead
> Linaro.org â Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>