[PATCH 0/4] rtc: convert several drivers to dev_err_probe()

[PATCH 0/4] rtc: convert several drivers to dev_err_probe()

[Balakrishnan Sambath]

Use dev_err_probe() in place of dev_err() and return across four rtc
drivers, which communicates the error type and helps debugging
hardware issues.

Build-tested with x86_64 allmodconfig. No functional change.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
Balakrishnan Sambath (4):
      rtc: palmas: convert to dev_err_probe()
      rtc: moxart: convert to dev_err_probe()
      rtc: sc27xx: convert to dev_err_probe()
      rtc: s35390a: convert to dev_err_probe()

 drivers/rtc/rtc-moxart.c  | 25 +++++++++----------------
 drivers/rtc/rtc-palmas.c  | 21 +++++++--------------
 drivers/rtc/rtc-s35390a.c | 18 ++++++------------
 drivers/rtc/rtc-sc27xx.c  | 24 ++++++++----------------
 4 files changed, 30 insertions(+), 58 deletions(-)
---
base-commit: b72386864481cf7fb6153842d22561ac3032302f
change-id: 20260528-cleanup-dev-err-probe-rtc-bc44f6ccbc01

Best regards,
-- 
Balakrishnan Sambath <balakrishnan.s@microchip.com>

[PATCH 1/4] rtc: palmas: convert to dev_err_probe()

[Balakrishnan Sambath]

Use dev_err_probe() in place of dev_err() and return, which
communicates the error type and helps debugging hardware issues.

No functional change.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/rtc/rtc-palmas.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
index aecada6bcf8..25fe7a8b73a 100644
--- a/drivers/rtc/rtc-palmas.c
+++ b/drivers/rtc/rtc-palmas.c
@@ -242,10 +242,8 @@ static int palmas_rtc_probe(struct platform_device *pdev)
 
 	/* Clear pending interrupts */
 	ret = palmas_clear_interrupts(&pdev->dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "clear RTC int failed, err = %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "clear RTC int failed\n");
 
 	palmas_rtc->dev = &pdev->dev;
 	platform_set_drvdata(pdev, palmas_rtc);
@@ -280,10 +278,8 @@ static int palmas_rtc_probe(struct platform_device *pdev)
 	ret = palmas_update_bits(palmas, PALMAS_RTC_BASE, PALMAS_RTC_CTRL_REG,
 			PALMAS_RTC_CTRL_REG_STOP_RTC,
 			PALMAS_RTC_CTRL_REG_STOP_RTC);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "RTC_CTRL write failed, err = %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "RTC_CTRL write failed\n");
 
 	palmas_rtc->irq = platform_get_irq(pdev, 0);
 
@@ -292,18 +288,15 @@ static int palmas_rtc_probe(struct platform_device *pdev)
 				&palmas_rtc_ops, THIS_MODULE);
 	if (IS_ERR(palmas_rtc->rtc)) {
 		ret = PTR_ERR(palmas_rtc->rtc);
-		dev_err(&pdev->dev, "RTC register failed, err = %d\n", ret);
-		return ret;
+		return dev_err_probe(&pdev->dev, ret, "RTC register failed\n");
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, palmas_rtc->irq, NULL,
 			palmas_rtc_interrupt,
 			IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 			dev_name(&pdev->dev), palmas_rtc);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "IRQ request failed, err = %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "IRQ request failed\n");
 
 	return 0;
 }

-- 
2.34.1

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

[Alexandre Mergnat]

On Thu, 28 May 2026 09:16:44 +0530, Balakrishnan Sambath <balakrishnan.s@microchip.com> wrote:
> diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
> index aecada6bcf8b..25fe7a8b73a0 100644
> --- a/drivers/rtc/rtc-palmas.c
> +++ b/drivers/rtc/rtc-palmas.c
> @@ -280,10 +278,8 @@ static int palmas_rtc_probe(struct platform_device *pdev)
>  	ret = palmas_update_bits(palmas, PALMAS_RTC_BASE, PALMAS_RTC_CTRL_REG,
>  			PALMAS_RTC_CTRL_REG_STOP_RTC,
>  			PALMAS_RTC_CTRL_REG_STOP_RTC);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "RTC_CTRL write failed, err = %d\n", ret);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "RTC_CTRL write failed\n");

The enable_bb_charging block earlier in this function (the two
palmas_update_bits() / "BACKUP_BATTERY_CTRL update failed" error paths)
follows the same dev_err()+return ret pattern but is left unconverted.
Converting those too keeps the whole probe consistent.
It's purely about readability, not correctness, so I give my RB.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

[PATCH 2/4] rtc: moxart: convert to dev_err_probe()

[Balakrishnan Sambath]

Use dev_err_probe() in place of dev_err() and return, which
communicates the error type and helps debugging hardware issues.

No functional change.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/rtc/rtc-moxart.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/rtc/rtc-moxart.c b/drivers/rtc/rtc-moxart.c
index 2247dd39ee4..e1766f03d73 100644
--- a/drivers/rtc/rtc-moxart.c
+++ b/drivers/rtc/rtc-moxart.c
@@ -253,26 +253,20 @@ static int moxart_rtc_probe(struct platform_device *pdev)
 	moxart_rtc->gpio_data = devm_gpiod_get(&pdev->dev, "rtc-data",
 					       GPIOD_IN);
 	ret = PTR_ERR_OR_ZERO(moxart_rtc->gpio_data);
-	if (ret) {
-		dev_err(&pdev->dev, "can't get rtc data gpio: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get rtc data gpio\n");
 
 	moxart_rtc->gpio_sclk = devm_gpiod_get(&pdev->dev, "rtc-sclk",
 					       GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(moxart_rtc->gpio_sclk);
-	if (ret) {
-		dev_err(&pdev->dev, "can't get rtc sclk gpio: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get rtc sclk gpio\n");
 
 	moxart_rtc->gpio_reset = devm_gpiod_get(&pdev->dev, "rtc-reset",
 						GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(moxart_rtc->gpio_reset);
-	if (ret) {
-		dev_err(&pdev->dev, "can't get rtc reset gpio: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "can't get rtc reset gpio\n");
 
 	spin_lock_init(&moxart_rtc->rtc_lock);
 	platform_set_drvdata(pdev, moxart_rtc);
@@ -280,10 +274,9 @@ static int moxart_rtc_probe(struct platform_device *pdev)
 	moxart_rtc->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
 						   &moxart_rtc_ops,
 						   THIS_MODULE);
-	if (IS_ERR(moxart_rtc->rtc)) {
-		dev_err(&pdev->dev, "devm_rtc_device_register failed\n");
-		return PTR_ERR(moxart_rtc->rtc);
-	}
+	if (IS_ERR(moxart_rtc->rtc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(moxart_rtc->rtc),
+				     "devm_rtc_device_register failed\n");
 
 	return 0;
 }

-- 
2.34.1

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

[Alexandre Mergnat]

On Thu, 28 May 2026 09:16:45 +0530, Balakrishnan Sambath <balakrishnan.s@microchip.com> wrote:
> Use dev_err_probe() in place of dev_err() and return, which
> communicates the error type and helps debugging hardware issues.
> 
> No functional change.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

[PATCH 3/4] rtc: sc27xx: convert to dev_err_probe()

[Balakrishnan Sambath]

Use dev_err_probe() in place of dev_err() and return, which
communicates the error type and helps debugging hardware issues.

No functional change.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/rtc/rtc-sc27xx.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/rtc/rtc-sc27xx.c b/drivers/rtc/rtc-sc27xx.c
index 2b83561d4d2..2c6d4565389 100644
--- a/drivers/rtc/rtc-sc27xx.c
+++ b/drivers/rtc/rtc-sc27xx.c
@@ -574,10 +574,8 @@ static int sprd_rtc_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	ret = of_property_read_u32(node, "reg", &rtc->base);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to get RTC base address\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to get RTC base address\n");
 
 	rtc->irq = platform_get_irq(pdev, 0);
 	if (rtc->irq < 0)
@@ -592,26 +590,20 @@ static int sprd_rtc_probe(struct platform_device *pdev)
 
 	/* check if we need set the alarm interrupt */
 	ret = sprd_rtc_check_alarm_int(rtc);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to check RTC alarm interrupt\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to check RTC alarm interrupt\n");
 
 	/* check if RTC time values are valid */
 	ret = sprd_rtc_check_power_down(rtc);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to check RTC time values\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to check RTC time values\n");
 
 	ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
 					sprd_rtc_handler,
 					IRQF_ONESHOT | IRQF_EARLY_RESUME,
 					pdev->name, rtc);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to request RTC irq\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "failed to request RTC irq\n");
 
 	device_init_wakeup(&pdev->dev, true);
 

-- 
2.34.1

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

[Alexandre Mergnat]

On Thu, 28 May 2026 09:16:46 +0530, Balakrishnan Sambath <balakrishnan.s@microchip.com> wrote:
> Use dev_err_probe() in place of dev_err() and return, which
> communicates the error type and helps debugging hardware issues.
> 
> No functional change.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

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

[Balakrishnan Sambath]

Use dev_err_probe() in place of dev_err() and return, which
communicates the error type and helps debugging hardware issues.

No functional change.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/rtc/rtc-s35390a.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index a4678d7c6cf..342fd2b568a 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");
 
 	if (status1 & S35390A_FLAG_24H)
 		s35390a->twentyfourhour = 1;
@@ -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");
 	} else {
 		err = s35390a_disable_test_mode(s35390a);
-		if (err < 0) {
-			dev_err(dev, "error disabling test mode\n");
-			return err;
-		}
+		if (err < 0)
+			return dev_err_probe(dev, err, "error disabling test mode\n");
 	}
 
 	device_set_wakeup_capable(dev, 1);

-- 
2.34.1

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

[Alexandre Mergnat]

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.

> @@ -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 ;)

-- 
Alexandre Mergnat <amergnat@baylibre.com>

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

[Balakrishnan.S]

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>

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

[Alexandre Belloni]

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.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Balakrishnan.S]

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