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

[<Balakrishnan.S@microchip.com>]

Hi Alexandre Belloni,

On 01/07/26 2:27 pm, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello,
> 
> On 01/07/2026 07:26:29+0000, Balakrishnan.S@microchip.com wrote:
>> 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.
> 
> Honestly, my plan was to not apply those patches because once I do
> that, I'll get hundreds of those. There is no benefit to the change and
> I'll cite the dev_err_probe doc:
> 
>   * This helper implements common pattern present in probe functions for error
>   * checking: print debug or error message depending if the error value is
>   * -EPROBE_DEFER and propagate error upwards.
>   * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
>   * checked later by reading devices_deferred debugfs attribute.
>   * It replaces the following code sequence::
>   *
>   *      if (err != -EPROBE_DEFER)
>   *              dev_err(dev, ...);
>   *      else
>   *              dev_dbg(dev, ...);
>   *      return err;
> 
> 
> We are not checking for EPROBE_DEFER in any of the drivers so there is
> no point in doing the change.
Okay thanks for the feedback. Lets drop this.
> 
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com