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

Open Source and information security mailing list archives

Message-ID: <9340342a-a908-d4f6-3293-8571a8d3e242@microchip.com>
Date:   Wed, 18 Dec 2019 10:02:36 +0000
From:   <Claudiu.Beznea@...rochip.com>
To:     <lee.jones@...aro.org>
CC:     <alexandre.belloni@...tlin.com>, <bbrezillon@...nel.org>,
        <airlied@...ux.ie>, <dri-devel@...ts.freedesktop.org>,
        <linux-kernel@...r.kernel.org>, <Ludovic.Desroches@...rochip.com>,
        <daniel@...ll.ch>, <sam@...nborg.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 4/6] mfd: atmel-hlcdc: return in case of error

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@...rochip.com>
>> ---
>>  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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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