Re: [PATCH 4/4] rtc: s35390a: convert to dev_err_probe()

[<Balakrishnan.S@microchip.com>]

Hi Alexandre,

Thanks for the review/feedback.

On 30/06/26 10:40 pm, Alexandre Mergnat wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, 28 May 2026 09:16:47 +0530, Balakrishnan Sambath <balakrishnan.s@microchip.com> wrote:
>> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
>> index a4678d7c6cf6..342fd2b568a3 100644
>> --- a/drivers/rtc/rtc-s35390a.c
>> +++ b/drivers/rtc/rtc-s35390a.c
>> @@ -479,10 +479,8 @@ static int s35390a_probe(struct i2c_client *client)
>>                return PTR_ERR(rtc);
>>
>>        err_read = s35390a_read_status(s35390a, &status1);
>> -     if (err_read < 0) {
>> -             dev_err(dev, "error resetting chip\n");
>> -             return err_read;
>> -     }
>> +     if (err_read < 0)
>> +             return dev_err_probe(dev, err_read, "error resetting chip\n");
> 
> The devm_i2c_new_dummy_device() loop above this hunk still uses
> dev_err()+return PTR_ERR("Address %02x unavailable"). dev_err_probe() takes
> format args, so it converts cleanly:
> 
>      return dev_err_probe(dev, PTR_ERR(s35390a->client[i]),
>                           "Address %02x unavailable\n", client->addr + i);
> 
> Worth converting for consistency with the rest of the probe.
Sure, I'll fix this too in next revision.
> 
>> @@ -493,16 +491,12 @@ static int s35390a_probe(struct i2c_client *client)
>>                /* disable alarm (and maybe test mode) */
>>                buf = 0;
>>                err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
>> -             if (err < 0) {
>> -                     dev_err(dev, "error disabling alarm");
>> -                     return err;
>> -             }
>> +             if (err < 0)
>> +                     return dev_err_probe(dev, err, "error disabling alarm");
> 
> This message is missing its trailing newline (pre-existing). dev_err_probe()
> formats as "error %pe: %pV" and does not append "\n" itself, so the line
> runs into the next log message. Since you are touching this line, adding
> "\n" is a cheap fix even if the issue was here before your patch.
> I recommand to fix it ;)
Okay noted. Will fix it too.
> 
> --
> Alexandre Mergnat <amergnat@baylibre.com>