Error handling in probe() can be a bit simpler with dev_err_probe().

Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
---
 drivers/soc/qcom/qcom-pbs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
index 77a70d3d0d0b..ab9de12ec901 100644
--- a/drivers/soc/qcom/qcom-pbs.c
+++ b/drivers/soc/qcom/qcom-pbs.c
@@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
 	}
 
 	ret = device_property_read_u32(pbs->dev, "reg", &val);
-	if (ret < 0) {
-		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
+
 	pbs->base = val;
 	mutex_init(&pbs->lock);
 
-- 
2.34.1

On 29/08/2024 14:48, Yu Jiaoliang wrote:
> Error handling in probe() can be a bit simpler with dev_err_probe().
> 
> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
> ---
>  drivers/soc/qcom/qcom-pbs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
> index 77a70d3d0d0b..ab9de12ec901 100644
> --- a/drivers/soc/qcom/qcom-pbs.c
> +++ b/drivers/soc/qcom/qcom-pbs.c
> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
>  	}
>  
>  	ret = device_property_read_u32(pbs->dev, "reg", &val);
> -	if (ret < 0) {
> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");

This cannot defer, so not much benefits. And you ignore other place in
the probe()... That's like a weird pattern with all your patches change
something irrelevant, but leave other places unchanged.

That's pointless and churn.

Best regards,
Krzysztof

On 2024/8/30 16:08, Krzysztof Kozlowski wrote:
> On 29/08/2024 14:48, Yu Jiaoliang wrote:
>> Error handling in probe() can be a bit simpler with dev_err_probe().
>>
>> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
>> ---
>>   drivers/soc/qcom/qcom-pbs.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
>> index 77a70d3d0d0b..ab9de12ec901 100644
>> --- a/drivers/soc/qcom/qcom-pbs.c
>> +++ b/drivers/soc/qcom/qcom-pbs.c
>> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	ret = device_property_read_u32(pbs->dev, "reg", &val);
>> -	if (ret < 0) {
>> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
>> -		return ret;
>> -	}
>> +	if (ret < 0)
>> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
Thank you for the feedback. I apologize for the oversight.
> This cannot defer, so not much benefits.
As noted in the dev_err_probe documentation, using this helper offers 
benefits like standardized error code formatting and more compact error 
paths.

"Using this helper in your probe function is totally fine even if @err  
known to nerver be -EPROBE_DEFER. The benefit compared to a normal 
dev_err() is the standardized format of the error code, it being emitted 
syumbolically (i.e. you get "EAGAIN" instead of "-35") and the fact that 
the error code is returned which allows more compact error paths."
> And you ignore other place in
> the probe()... That's like a weird pattern with all your patches change
> something irrelevant, but leave other places unchanged.
If you think it's OK, I will update the patch to include both 
modifications in the probe() function. I plan to submit patch v2 with 
these changes and hope it will be more acceptable.
> That's pointless and churn.
>
> Best regards,
> Krzysztof
>
>
Best Regards,

Yu

On 30/08/2024 12:52, 于佼良 wrote:
> On 2024/8/30 16:08, Krzysztof Kozlowski wrote:
>> On 29/08/2024 14:48, Yu Jiaoliang wrote:
>>> Error handling in probe() can be a bit simpler with dev_err_probe().
>>>
>>> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
>>> ---
>>>   drivers/soc/qcom/qcom-pbs.c | 7 +++----
>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
>>> index 77a70d3d0d0b..ab9de12ec901 100644
>>> --- a/drivers/soc/qcom/qcom-pbs.c
>>> +++ b/drivers/soc/qcom/qcom-pbs.c
>>> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
>>>   	}
>>>   
>>>   	ret = device_property_read_u32(pbs->dev, "reg", &val);
>>> -	if (ret < 0) {
>>> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
>>> -		return ret;
>>> -	}
>>> +	if (ret < 0)
>>> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
> Thank you for the feedback. I apologize for the oversight.
>> This cannot defer, so not much benefits.
> As noted in the dev_err_probe documentation, using this helper offers 
> benefits like standardized error code formatting and more compact error 
> paths.
> 
> "Using this helper in your probe function is totally fine even if @err  
> known to nerver be -EPROBE_DEFER. The benefit compared to a normal 
> dev_err() is the standardized format of the error code, it being emitted 
> syumbolically (i.e. you get "EAGAIN" instead of "-35") and the fact that 
> the error code is returned which allows more compact error paths."

I know, I wrote a bit of patches for the kernel myself...

It's still for me little benefit.


>> And you ignore other place in
>> the probe()... That's like a weird pattern with all your patches change
>> something irrelevant, but leave other places unchanged.
> If you think it's OK, I will update the patch to include both 
> modifications in the probe() function. I plan to submit patch v2 with 
> these changes and hope it will be more acceptable.

Doing this one-by-one is churn. For me that's not correct.

Changing this everywhere in the driver is questionable/subjective: some
find it ok, some not (considering this cannot defer).

But, after looking at your other patches like this (see serial), I have
doubts you know what you are doing in general. And that's the real
problem. You send innocent patch which requires a serious review,
because you do not understand the code.

Please get a mentor which will guide you through this. Or do some more
impactful changes like fixing warnings.

Best regards,
Krzysztof

On 30.08.2024 10:08 AM, Krzysztof Kozlowski wrote:
> On 29/08/2024 14:48, Yu Jiaoliang wrote:
>> Error handling in probe() can be a bit simpler with dev_err_probe().
>>
>> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
>> ---
>>  drivers/soc/qcom/qcom-pbs.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
>> index 77a70d3d0d0b..ab9de12ec901 100644
>> --- a/drivers/soc/qcom/qcom-pbs.c
>> +++ b/drivers/soc/qcom/qcom-pbs.c
>> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	ret = device_property_read_u32(pbs->dev, "reg", &val);
>> -	if (ret < 0) {
>> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
>> -		return ret;
>> -	}
>> +	if (ret < 0)
>> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
> 
> This cannot defer, so not much benefits. And you ignore other place in
> the probe()... That's like a weird pattern with all your patches change
> something irrelevant, but leave other places unchanged.
> 
> That's pointless and churn.

Hm, that's a good point.. Maybe the static checker folks could come up
with a way that would find functions that call something that can defer
at one point or another and suggest (not) using dev_err_probe with W=1/2..
(although that is probably not going to be very high prio given all the
other static checker issues we're still yet to resolve)

Unless we have something like that already? +CC Dan

Konrad

On Fri, Aug 30, 2024 at 12:03:20PM +0200, Konrad Dybcio wrote:
> On 30.08.2024 10:08 AM, Krzysztof Kozlowski wrote:
> > On 29/08/2024 14:48, Yu Jiaoliang wrote:
> >> Error handling in probe() can be a bit simpler with dev_err_probe().
> >>
> >> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
> >> ---
> >>  drivers/soc/qcom/qcom-pbs.c | 7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
> >> index 77a70d3d0d0b..ab9de12ec901 100644
> >> --- a/drivers/soc/qcom/qcom-pbs.c
> >> +++ b/drivers/soc/qcom/qcom-pbs.c
> >> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
> >>  	}
> >>  
> >>  	ret = device_property_read_u32(pbs->dev, "reg", &val);
> >> -	if (ret < 0) {
> >> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
> >> -		return ret;
> >> -	}
> >> +	if (ret < 0)
> >> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
> > 
> > This cannot defer, so not much benefits. And you ignore other place in
> > the probe()... That's like a weird pattern with all your patches change
> > something irrelevant, but leave other places unchanged.
> > 
> > That's pointless and churn.
> 
> Hm, that's a good point.. Maybe the static checker folks could come up
> with a way that would find functions that call something that can defer
> at one point or another and suggest (not) using dev_err_probe with W=1/2..
> (although that is probably not going to be very high prio given all the
> other static checker issues we're still yet to resolve)
> 
> Unless we have something like that already? +CC Dan

I believe these patches are from people writing their own Coccinelle scripts to
do the conversions.  There aren't any published scripts which care one way or
the other.

device_property_read_u32() can't return -EPROBE_DEFER.  It's documented in a
comment.  So this is just a question of preferred style.  There isn't a kernel
wide preferred style on this.  Some maintainers prefer to not use dev_err_probe()
where it "doesn't make sense because ret isn't -EPROBE_DEFER".  Other maintainers
use it all the time even for error code literals like:
	return dev_err_probe(pbs->dev, -EINVAL, "invalid input.\n");
Because "it's cleaner, more uniform and only takes one line".  I think Julia
said she didn't want to get involved with this debate and I definitely don't.

There are a few things which we could do:

1) Returning -EPROBE_DEFER to an ioctl or something besides a probe()
   This is a bug right?  -EPROBE_DEFER is basically kernel internal for probe()
   functions.  It tried to write this but it was complicated so I gave up.

2) Printing an error message for -EPROBE_DEFER warnings
   I've written this check and I can test it tonight.

3) Not propagating the -EPROBE_DEFER returns
   This shouldn't be too hard to write.

Let me add a KTODO in case anyone wants to do this before I get around to it.

KTODO: write Smatch EPROBE_DEFER warnings

regards,
dan carpenter

On 30.08.2024 5:19 PM, Dan Carpenter wrote:
> On Fri, Aug 30, 2024 at 12:03:20PM +0200, Konrad Dybcio wrote:
>> On 30.08.2024 10:08 AM, Krzysztof Kozlowski wrote:
>>> On 29/08/2024 14:48, Yu Jiaoliang wrote:
>>>> Error handling in probe() can be a bit simpler with dev_err_probe().
>>>>
>>>> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
>>>> ---
>>>>  drivers/soc/qcom/qcom-pbs.c | 7 +++----
>>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
>>>> index 77a70d3d0d0b..ab9de12ec901 100644
>>>> --- a/drivers/soc/qcom/qcom-pbs.c
>>>> +++ b/drivers/soc/qcom/qcom-pbs.c
>>>> @@ -201,10 +201,9 @@ static int qcom_pbs_probe(struct platform_device *pdev)
>>>>  	}
>>>>  
>>>>  	ret = device_property_read_u32(pbs->dev, "reg", &val);
>>>> -	if (ret < 0) {
>>>> -		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
>>>> -		return ret;
>>>> -	}
>>>> +	if (ret < 0)
>>>> +		return dev_err_probe(pbs->dev, ret, "Couldn't find reg\n");
>>>
>>> This cannot defer, so not much benefits. And you ignore other place in
>>> the probe()... That's like a weird pattern with all your patches change
>>> something irrelevant, but leave other places unchanged.
>>>
>>> That's pointless and churn.
>>
>> Hm, that's a good point.. Maybe the static checker folks could come up
>> with a way that would find functions that call something that can defer
>> at one point or another and suggest (not) using dev_err_probe with W=1/2..
>> (although that is probably not going to be very high prio given all the
>> other static checker issues we're still yet to resolve)
>>
>> Unless we have something like that already? +CC Dan
> 
> I believe these patches are from people writing their own Coccinelle scripts to
> do the conversions.  There aren't any published scripts which care one way or
> the other.
> 
> device_property_read_u32() can't return -EPROBE_DEFER.  It's documented in a
> comment.  So this is just a question of preferred style.  There isn't a kernel
> wide preferred style on this.  Some maintainers prefer to not use dev_err_probe()
> where it "doesn't make sense because ret isn't -EPROBE_DEFER".  Other maintainers
> use it all the time even for error code literals like:
> 	return dev_err_probe(pbs->dev, -EINVAL, "invalid input.\n");
> Because "it's cleaner, more uniform and only takes one line".  I think Julia
> said she didn't want to get involved with this debate and I definitely don't.

Personally, I don't mind either.. so longer as it's consistent within
the file

> 
> There are a few things which we could do:
> 
> 1) Returning -EPROBE_DEFER to an ioctl or something besides a probe()
>    This is a bug right?  -EPROBE_DEFER is basically kernel internal for probe()
>    functions.  It tried to write this but it was complicated so I gave up.

Maybe call_tree.pl can somehow be used with an if name[-5:] == "probe"
or something along those lines..

> 
> 2) Printing an error message for -EPROBE_DEFER warnings
>    I've written this check and I can test it tonight.
> 
> 3) Not propagating the -EPROBE_DEFER returns
>    This shouldn't be too hard to write.
> 
> Let me add a KTODO in case anyone wants to do this before I get around to it.
> 
> KTODO: write Smatch EPROBE_DEFER warnings

Thanks!

Konrad

On Fri, Aug 30, 2024 at 05:31:14PM +0200, Konrad Dybcio wrote:
> > 
> > There are a few things which we could do:
> > 
> > 1) Returning -EPROBE_DEFER to an ioctl or something besides a probe()
> >    This is a bug right?  -EPROBE_DEFER is basically kernel internal for probe()
> >    functions.  It tried to write this but it was complicated so I gave up.
> 
> Maybe call_tree.pl can somehow be used with an if name[-5:] == "probe"
> or something along those lines..
> 

I wrote the call_tree.pl script before I had the database.  These days I tend to
use the database instead.

I've implemented this check but it only looks at ioctls.  I'll test it tonight.

> > 
> > 2) Printing an error message for -EPROBE_DEFER warnings
> >    I've written this check and I can test it tonight.
> > 

I've done this.  See the attached check and the dont_print.list file attached.
The line numbers are based on linux next.  The false positives from here are
pretty harmless because calling dev_err_probe() is fine.

> > 3) Not propagating the -EPROBE_DEFER returns
> >    This shouldn't be too hard to write.
> > 

I've done this too.  The false positives from this could be bad, because we only
want to propagate -EPROBE_DEFER back from probe() functions.

See propagate.list.

regards,
dan carpenter
drivers/rpmsg/rpmsg_core.c:569 rpmsg_dev_probe() EPROBE_DEFER path should not print warnings
drivers/platform/x86/x86-android-tablets/core.c:107 x86_acpi_irq_helper_get() EPROBE_DEFER path should not print warnings
drivers/platform/surface/surface_hotplug.c:237 surface_hotplug_probe() EPROBE_DEFER path should not print warnings
drivers/input/joystick/as5011.c:274 as5011_probe() EPROBE_DEFER path should not print warnings
drivers/input/keyboard/gpio_keys.c:551 gpio_keys_setup_key() EPROBE_DEFER path should not print warnings
drivers/input/keyboard/imx_sc_key.c:157 imx_sc_key_probe() EPROBE_DEFER path should not print warnings
drivers/input/keyboard/matrix_keypad.c:283 matrix_keypad_init_gpio() EPROBE_DEFER path should not print warnings
drivers/input/keyboard/matrix_keypad.c:296 matrix_keypad_init_gpio() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c:174 sun8i_dw_hdmi_bind() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/tegra/output.c:158 tegra_output_probe() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:171 acp_genpd_add_device() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/omapdrm/omap_drv.c:723 omapdrm_init() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/i915/i915_driver.c:426 i915_pcode_init() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/i915/intel_pcode.c:237 intel_pcode_init() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/vc4/vc4_hdmi.c:2300 vc4_hdmi_audio_init() EPROBE_DEFER path should not print warnings
drivers/gpu/drm/bridge/sil-sii8620.c:2332 sii8620_probe() EPROBE_DEFER path should not print warnings
drivers/gpu/host1x/context.c:72 host1x_memory_context_list_init() EPROBE_DEFER path should not print warnings
drivers/gpu/host1x/dev.c:612 host1x_probe() EPROBE_DEFER path should not print warnings
drivers/tty/serial/serial_mctrl_gpio.c:243 mctrl_gpio_init() EPROBE_DEFER path should not print warnings
drivers/memory/samsung/exynos5422-dmc.c:1503 exynos5_dmc_probe() EPROBE_DEFER path should not print warnings
drivers/usb/common/usb-conn-gpio.c:229 usb_conn_probe() EPROBE_DEFER path should not print warnings
drivers/usb/common/usb-conn-gpio.c:246 usb_conn_probe() EPROBE_DEFER path should not print warnings
drivers/usb/misc/brcmstb-usb-pinmap.c:311 brcmstb_usb_pinmap_probe() EPROBE_DEFER path should not print warnings
drivers/usb/typec/tcpm/fusb302.c:1640 init_gpio() EPROBE_DEFER path should not print warnings
drivers/usb/typec/anx7411.c:1340 anx7411_get_gpio_irq() EPROBE_DEFER path should not print warnings
drivers/vfio/fsl-mc/vfio_fsl_mc.c:474 vfio_fsl_mc_init_device() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-usb-gpio.c:154 usb_extcon_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-usb-gpio.c:172 usb_extcon_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-max3355.c:93 max3355_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-ptn5150.c:273 ptn5150_i2c_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-palmas.c:287 palmas_usb_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-palmas.c:338 palmas_usb_probe() EPROBE_DEFER path should not print warnings
drivers/extcon/extcon-intel-int3496.c:147 int3496_probe() EPROBE_DEFER path should not print warnings
drivers/phy/broadcom/phy-bcm-ns2-usbdrd.c:347 ns2_drd_phy_probe() EPROBE_DEFER path should not print warnings
drivers/phy/broadcom/phy-bcm-ns2-usbdrd.c:353 ns2_drd_phy_probe() EPROBE_DEFER path should not print warnings
drivers/phy/motorola/phy-mapphone-mdm6600.c:486 phy_mdm6600_deferred_power_on() EPROBE_DEFER path should not print warnings
drivers/soc/rockchip/io-domain.c:677 rockchip_iodomain_probe() EPROBE_DEFER path should not print warnings
drivers/hte/hte-tegra194-test.c:140 tegra_hte_test_probe() EPROBE_DEFER path should not print warnings
drivers/devfreq/devfreq.c:1318 devfreq_add_governor() EPROBE_DEFER path should not print warnings
drivers/devfreq/devfreq.c:1482 governor_store() EPROBE_DEFER path should not print warnings
drivers/devfreq/devfreq.c:1489 governor_store() EPROBE_DEFER path should not print warnings
drivers/devfreq/devfreq.c:1910 timer_store() EPROBE_DEFER path should not print warnings
drivers/devfreq/mtk-cci-devfreq.c:54 mtk_ccifreq_set_voltage() EPROBE_DEFER path should not print warnings
drivers/devfreq/mtk-cci-devfreq.c:60 mtk_ccifreq_set_voltage() EPROBE_DEFER path should not print warnings
drivers/devfreq/mtk-cci-devfreq.c:158 mtk_ccifreq_target() EPROBE_DEFER path should not print warnings
drivers/firmware/imx/imx-scu.c:333 imx_scu_probe() EPROBE_DEFER path should not print warnings
drivers/firmware/imx/imx-scu.c:337 imx_scu_probe() EPROBE_DEFER path should not print warnings
drivers/firmware/arm_scmi/driver.c:2708 scmi_txrx_setup() EPROBE_DEFER path should not print warnings
drivers/uio/uio_dfl.c:42 uio_dfl_probe() EPROBE_DEFER path should not print warnings
drivers/uio/uio_pdrv_genirq.c:250 uio_pdrv_genirq_probe() EPROBE_DEFER path should not print warnings
drivers/uio/uio_hv_generic.c:340 hv_uio_probe() EPROBE_DEFER path should not print warnings
drivers/nfc/nfcmrvl/main.c:121 nfcmrvl_nci_register_dev() EPROBE_DEFER path should not print warnings
drivers/net/wireless/intersil/p54/p54spi.c:613 p54spi_probe() EPROBE_DEFER path should not print warnings
drivers/net/wireless/intersil/p54/p54spi.c:619 p54spi_probe() EPROBE_DEFER path should not print warnings
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c:542 stm32_dwmac_probe() EPROBE_DEFER path should not print warnings
drivers/net/ethernet/mellanox/mlx4/main.c:4013 mlx4_devlink_reload_up() EPROBE_DEFER path should not print warnings
drivers/net/ethernet/mellanox/mlx4/main.c:4263 mlx4_restart_one_up() EPROBE_DEFER path should not print warnings
drivers/net/ethernet/mellanox/mlx4/main.c:4400 mlx4_pci_resume() EPROBE_DEFER path should not print warnings
drivers/net/can/m_can/tcan4x5x-core.c:452 tcan4x5x_can_probe() EPROBE_DEFER path should not print warnings
drivers/cpufreq/scmi-cpufreq.c:388 scmi_cpufreq_probe() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq-hw.c:329 mtk_cpufreq_hw_driver_probe() EPROBE_DEFER path should not print warnings
drivers/cpufreq/amd-pstate.c:1948 amd_pstate_init() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:89 mtk_cpufreq_voltage_tracking() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:96 mtk_cpufreq_voltage_tracking() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:224 mtk_cpufreq_set_target() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:257 mtk_cpufreq_set_target() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:299 mtk_cpufreq_set_target() EPROBE_DEFER path should not print warnings
drivers/cpufreq/mediatek-cpufreq.c:336 mtk_cpufreq_opp_notifier() EPROBE_DEFER path should not print warnings
drivers/cpufreq/qcom-cpufreq-hw.c:725 qcom_cpufreq_hw_driver_probe() EPROBE_DEFER path should not print warnings
drivers/cpufreq/cpufreq-dt.c:327 dt_cpufreq_probe() EPROBE_DEFER path should not print warnings
drivers/cpufreq/scpi-cpufreq.c:204 scpi_cpufreq_probe() EPROBE_DEFER path should not print warnings
drivers/media/platform/amphion/vpu_imx8q.c:206 vpu_imx8q_get_fuse() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp/mtk_mdp_core.c:200 mtk_mdp_probe() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp/mtk_mdp_vpu.c:69 mtk_mdp_vpu_register() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp/mtk_mdp_m2m.c:1102 mtk_mdp_m2m_open() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c:132 mdp_vpu_register() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c:114 mdp_vpu_get_locked() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c:169 mdp_m2m_start_streaming() EPROBE_DEFER path should not print warnings
drivers/media/platform/mediatek/vpu/mtk_vpu.c:875 mtk_vpu_probe() EPROBE_DEFER path should not print warnings
drivers/media/usb/em28xx/em28xx-dvb.c:742 em28xx_pctv_290e_set_lna() EPROBE_DEFER path should not print warnings
drivers/media/usb/em28xx/em28xx-dvb.c:1713 em28xx_dvb_init() EPROBE_DEFER path should not print warnings
drivers/media/dvb-frontends/tc90522.c:583 tc90522_sleep() EPROBE_DEFER path should not print warnings
drivers/media/dvb-frontends/tc90522.c:616 tc90522_init() EPROBE_DEFER path should not print warnings
drivers/media/pci/sta2x11/sta2x11_vip.c:902 vip_gpio_reserve() EPROBE_DEFER path should not print warnings
drivers/media/i2c/hi556.c:1336 hi556_probe() EPROBE_DEFER path should not print warnings
drivers/media/i2c/ov13b10.c:1505 ov13b10_probe() EPROBE_DEFER path should not print warnings
drivers/media/cec/platform/seco/seco-cec.c:549 secocec_acpi_probe() EPROBE_DEFER path should not print warnings
drivers/cxl/pci.c:840 cxl_pci_probe() EPROBE_DEFER path should not print warnings
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c:847 gmin_v1p8_ctrl() EPROBE_DEFER path should not print warnings
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c:928 gmin_v2p8_ctrl() EPROBE_DEFER path should not print warnings
drivers/bluetooth/hci_intel.c:1171 intel_probe() EPROBE_DEFER path should not print warnings
drivers/pci/controller/dwc/pcie-tegra194.c:2050 tegra_pcie_config_ep() EPROBE_DEFER path should not print warnings
drivers/regulator/vctrl-regulator.c:195 vctrl_set_voltage_sel() EPROBE_DEFER path should not print warnings
drivers/regulator/vctrl-regulator.c:502 vctrl_probe() EPROBE_DEFER path should not print warnings
drivers/regulator/max5970-regulator.c:634 max597x_regulator_probe() EPROBE_DEFER path should not print warnings
drivers/power/supply/olpc_battery.c:540 olpc_bat_eeprom_read() EPROBE_DEFER path should not print warnings
drivers/power/supply/lt3651-charger.c:159 lt3651_charger_probe() EPROBE_DEFER path should not print warnings
drivers/power/supply/lt3651-charger.c:169 lt3651_charger_probe() EPROBE_DEFER path should not print warnings
drivers/power/supply/lt3651-charger.c:179 lt3651_charger_probe() EPROBE_DEFER path should not print warnings
drivers/power/supply/sc27xx_fuel_gauge.c:1239 sc27xx_fgu_probe() EPROBE_DEFER path should not print warnings
drivers/mfd/arizona-irq.c:367 arizona_irq_init() EPROBE_DEFER path should not print warnings
drivers/mfd/aat2870-core.c:373 aat2870_i2c_probe() EPROBE_DEFER path should not print warnings
drivers/mfd/si476x-i2c.c:725 si476x_core_probe() EPROBE_DEFER path should not print warnings
drivers/mfd/wm8994-irq.c:211 wm8994_irq_init() EPROBE_DEFER path should not print warnings
drivers/hwtracing/intel_th/sth.c:230 intel_th_sth_probe() EPROBE_DEFER path should not print warnings
drivers/hwtracing/stm/ftrace.c:72 stm_ftrace_init() EPROBE_DEFER path should not print warnings
drivers/gpio/gpiolib-acpi.c:463 acpi_gpiochip_alloc_event() EPROBE_DEFER path should not print warnings
drivers/gpio/gpiolib.c:1386 gpiochip_set_hierarchical_irqchip() EPROBE_DEFER path should not print warnings
drivers/gpio/gpiolib.c:1466 gpiochip_hierarchy_irq_domain_alloc() EPROBE_DEFER path should not print warnings
drivers/iio/adc/stm32-adc-core.c:544 stm32_adc_core_hw_start() EPROBE_DEFER path should not print warnings
drivers/iio/adc/stm32-adc-core.c:652 stm32_adc_core_switches_probe() EPROBE_DEFER path should not print warnings
drivers/iio/adc/stm32-adc-core.c:773 stm32_adc_probe() EPROBE_DEFER path should not print warnings
drivers/iio/adc/meson_saradc.c:692 meson_sar_adc_iio_info_read_raw() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ltc1660.c:62 ltc1660_read_raw() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ad7293.c:845 ad7293_init() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ad7293.c:854 ad7293_init() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ad8801.c:139 ad8801_probe() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ad8801.c:162 ad8801_probe() EPROBE_DEFER path should not print warnings
drivers/iio/dac/stm32-dac-core.c:135 stm32_dac_probe() EPROBE_DEFER path should not print warnings
drivers/iio/dac/ad5761.c:324 ad5761_get_vref() EPROBE_DEFER path should not print warnings
drivers/iio/dac/max5821.c:346 max5821_probe() EPROBE_DEFER path should not print warnings
drivers/iio/proximity/srf04.c:304 srf04_probe() EPROBE_DEFER path should not print warnings
drivers/iio/proximity/ping.c:121 ping_read() EPROBE_DEFER path should not print warnings
drivers/iio/light/gp2ap002.c:539 gp2ap002_probe() EPROBE_DEFER path should not print warnings
drivers/iio/humidity/dht11.c:311 dht11_probe() EPROBE_DEFER path should not print warnings
sound/soc/rockchip/rockchip_i2s_tdm.c:1369 rockchip_i2s_tdm_probe() EPROBE_DEFER path should not print warnings
sound/soc/rockchip/rockchip_pdm.c:642 rockchip_pdm_probe() EPROBE_DEFER path should not print warnings
sound/soc/rockchip/rockchip_i2s.c:835 rockchip_i2s_probe() EPROBE_DEFER path should not print warnings
sound/soc/rockchip/rockchip_spdif.c:354 rk_spdif_probe() EPROBE_DEFER path should not print warnings
sound/soc/amd/acp/acp-legacy-mach.c:168 acp_asoc_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/atmel-i2s.c:715 atmel_i2s_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/mchp-spdifrx.c:1161 mchp_spdifrx_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/mchp-pdmc.c:1098 mchp_pdmc_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/mchp-spdiftx.c:857 mchp_spdiftx_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/mchp-i2s-mcc.c:1078 mchp_i2s_mcc_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/atmel-pdmic.c:659 atmel_pdmic_probe() EPROBE_DEFER path should not print warnings
sound/soc/atmel/atmel-classd.c:586 atmel_classd_probe() EPROBE_DEFER path should not print warnings
sound/soc/bcm/bcm2835-i2s.c:904 bcm2835_i2s_probe() EPROBE_DEFER path should not print warnings
sound/soc/fsl/lpc3xxx-pcm.c:65 lpc3xxx_pcm_register() EPROBE_DEFER path should not print warnings
sound/soc/fsl/fsl_xcvr.c:1402 fsl_xcvr_probe() EPROBE_DEFER path should not print warnings
sound/soc/fsl/fsl_aud2htx.c:242 fsl_aud2htx_probe() EPROBE_DEFER path should not print warnings
sound/soc/fsl/fsl_micfil.c:1246 fsl_micfil_probe() EPROBE_DEFER path should not print warnings
sound/soc/samsung/speyside.c:156 speyside_wm8996_init() EPROBE_DEFER path should not print warnings
sound/soc/samsung/aries_wm8994.c:376 aries_late_probe() EPROBE_DEFER path should not print warnings
sound/soc/sunxi/sun4i-codec.c:1795 sun4i_codec_probe() EPROBE_DEFER path should not print warnings
sound/soc/sunxi/sun4i-i2s.c:1610 sun4i_i2s_probe() EPROBE_DEFER path should not print warnings
sound/soc/starfive/jh7110_tdm.c:618 jh7110_tdm_probe() EPROBE_DEFER path should not print warnings
sound/soc/sof/imx/imx8.c:235 imx8_probe() EPROBE_DEFER path should not print warnings
sound/soc/intel/avs/boards/hdaudio.c:151 avs_probing_link_init() EPROBE_DEFER path should not print warnings
sound/soc/intel/keembay/kmb_platform.c:892 kmb_plat_dai_probe() EPROBE_DEFER path should not print warnings
sound/soc/dwc/dwc-i2s.c:1027 dw_i2s_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tlv320aic32x4.c:1386 aic32x4_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/twl4030.c:266 twl4030_init_chip() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tas2764.c:726 tas2764_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tlv320dac33.c:1507 dac33_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tas2770.c:680 tas2770_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/cs42l52.c:1148 cs42l52_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/arizona-jack.c:1331 arizona_jack_codec_dev_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/arizona-jack.c:1363 arizona_jack_codec_dev_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tas2780.c:615 tas2780_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/tpa6130a2.c:261 tpa6130a2_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/cs42l73.c:1321 cs42l73_i2c_probe() EPROBE_DEFER path should not print warnings
sound/soc/codecs/cs42l56.c:1206 cs42l56_i2c_probe() EPROBE_DEFER path should not print warnings
drivers/platform/cznic/turris-omnia-mcu-trng.c:102 omnia_mcu_register_trng() warn: why not propogate EPROBE_DEFER from 'irq'
drivers/input/misc/soc_button_array.c:149 soc_button_lookup_gpio() warn: why not propogate EPROBE_DEFER from '*irq_ret'
drivers/input/keyboard/mpr121_touchkey.c:252 mpr_touchkey_probe() warn: why not propogate EPROBE_DEFER from 'vdd_uv'
drivers/gpu/drm/panel/panel-novatek-nt35950.c:354 nt35950_sharp_init_vregs() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/gpu/drm/omapdrm/dss/dsi.c:4328 omap_dsi_register_te_irq() warn: why not propogate EPROBE_DEFER from 'te_irq'
drivers/gpu/drm/bridge/samsung-dsim.c:1697 samsung_dsim_register_te_irq() warn: why not propogate EPROBE_DEFER from 'te_gpio_irq'
drivers/gpu/drm/bridge/display-connector.c:381 display_connector_probe() warn: why not propogate EPROBE_DEFER from 'conn->hpd_irq'
drivers/gpu/drm/bridge/ti-tpd12s015.c:179 tpd12s015_probe() warn: why not propogate EPROBE_DEFER from 'tpd->hpd_irq'
drivers/gpu/drm/nouveau/nvkm/subdev/volt/gk20a.c:104 gk20a_volt_vid_get() warn: why not propogate EPROBE_DEFER from 'uv'
drivers/gpu/drm/nouveau/nvkm/subdev/volt/gk20a.c:171 gk20a_volt_ctor() warn: why not propogate EPROBE_DEFER from 'uv'
drivers/clk/clk-si5341.c:1381 si5341_dt_parse_dt() warn: why not propogate EPROBE_DEFER from 'vdd'
drivers/usb/gadget/udc/pch_udc.c:1401 pch_vbus_gpio_init() warn: why not propogate EPROBE_DEFER from 'irq_num'
drivers/mmc/core/regulator.c:157 mmc_regulator_set_voltage_if_supported() warn: why not propogate EPROBE_DEFER from 'current_uV'
drivers/mmc/core/regulator.c:274 mmc_regulator_get_supply() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/phy/allwinner/phy-sun4i-usb.c:901 sun4i_usb_phy_probe() warn: why not propogate EPROBE_DEFER from 'data->id_det_irq'
drivers/base/dd.c:1134 device_driver_attach() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/uio/uio_aec.c:120 probe() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/nfc/trf7970a.c:2135 trf7970a_probe() warn: why not propogate EPROBE_DEFER from 'uvolts'
drivers/net/phy/broadcom.c:1170 bcm54xx_phy_probe() warn: why not propogate EPROBE_DEFER from 'priv->wake_irq'
drivers/net/mdio/acpi_mdio.c:59 __acpi_mdiobus_register() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/net/ethernet/broadcom/cnic.c:1340 cnic_alloc_bnx2x_resc() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c:171 mdp_m2m_start_streaming() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/media/cec/platform/seco/seco-cec.c:550 secocec_acpi_probe() warn: why not propogate EPROBE_DEFER from 'irq'
drivers/media/cec/platform/cec-gpio/cec-gpio.c:255 cec_gpio_probe() warn: why not propogate EPROBE_DEFER from 'cec->cec_irq'
drivers/hwmon/sht15.c:1008 sht15_probe() warn: why not propogate EPROBE_DEFER from 'data->supply_uv'
drivers/hwmon/gpio-fan.c:91 fan_alarm_init() warn: why not propogate EPROBE_DEFER from 'alarm_irq'
drivers/hwmon/ads7828.c:134 ads7828_probe() warn: why not propogate EPROBE_DEFER from '__x'
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c:932 gmin_v2p8_ctrl() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/staging/greybus/arche-platform.c:545 arche_platform_probe() warn: why not propogate EPROBE_DEFER from 'arche_pdata->wake_detect_irq'
drivers/bluetooth/hci_bcm.c:1173 bcm_get_resources() warn: why not propogate EPROBE_DEFER from 'dev->irq'
drivers/bluetooth/hci_intel.c:1191 intel_probe() warn: why not propogate EPROBE_DEFER from 'idev->irq'
drivers/bluetooth/hci_nokia.c:751 nokia_bluetooth_serdev_probe() warn: why not propogate EPROBE_DEFER from 'btdev->wake_irq'
drivers/pci/controller/dwc/pcie-qcom-ep.c:754 qcom_pcie_ep_enable_irq_resources() warn: why not propogate EPROBE_DEFER from 'pcie_ep->perst_irq'
drivers/regulator/vctrl-regulator.c:142 vctrl_set_voltage() warn: why not propogate EPROBE_DEFER from 'orig_ctrl_uV'
drivers/regulator/core.c:3565 _regulator_call_set_voltage() warn: why not propogate EPROBE_DEFER from 'data.old_uV'
drivers/regulator/core.c:3589 _regulator_call_set_voltage_sel() warn: why not propogate EPROBE_DEFER from 'data.old_uV'
drivers/power/supply/gpio-charger.c:350 gpio_charger_probe() warn: why not propogate EPROBE_DEFER from 'charge_status_irq'
drivers/power/supply/lt3651-charger.c:184 lt3651_charger_probe() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/power/supply/sbs-battery.c:1222 sbs_probe() warn: why not propogate EPROBE_DEFER from 'irq'
drivers/mfd/stmpe.c:1450 stmpe_probe() warn: why not propogate EPROBE_DEFER from 'stmpe->irq'
drivers/gpio/gpiolib-cdev.c:1062 debounce_setup() warn: why not propogate EPROBE_DEFER from 'irq'
drivers/gpio/gpiolib-cdev.c:1177 edge_detector_setup() warn: why not propogate EPROBE_DEFER from 'irq'
drivers/gpio/gpiolib-sysfs.c:181 gpio_sysfs_request_irq() warn: why not propogate EPROBE_DEFER from 'data->irq'
drivers/iio/adc/vf610_adc.c:907 vf610_adc_probe() warn: why not propogate EPROBE_DEFER from 'info->vref_uv'
drivers/iio/adc/rcar-gyroadc.c:230 rcar_gyroadc_read_raw() warn: why not propogate EPROBE_DEFER from 'vref'
drivers/iio/adc/imx7d_adc.c:324 imx7d_adc_read_raw() warn: why not propogate EPROBE_DEFER from 'info->vref_uv'
drivers/iio/adc/max11100.c:91 max11100_read_raw() warn: why not propogate EPROBE_DEFER from 'vref_uv'
drivers/iio/adc/ad4130.c:1707 ad4310_parse_fw() warn: why not propogate EPROBE_DEFER from 'avdd_uv'
drivers/iio/adc/ti-ads1100.c:148 ads1100_set_scale() warn: why not propogate EPROBE_DEFER from 'microvolts'
drivers/iio/adc/ad7173.c:852 ad7173_read_raw() warn: why not propogate EPROBE_DEFER from '*val'
drivers/iio/adc/npcm_adc.c:181 npcm_adc_read_raw() warn: why not propogate EPROBE_DEFER from 'vref_uv'
drivers/iio/adc/ad7298.c:258 ad7298_read_raw() warn: why not propogate EPROBE_DEFER from '*val'
drivers/iio/dac/max5522.c:91 max5522_read_raw() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/iio/dac/ad5592r-base.c:449 ad5592r_read_raw() warn: why not propogate EPROBE_DEFER from 'ret'
drivers/iio/addac/ad74413r.c:666 ad74413r_get_output_current_scale() warn: why not propogate EPROBE_DEFER from '*val'
drivers/iio/humidity/dht11.c:312 dht11_probe() warn: why not propogate EPROBE_DEFER from 'dht11->irq'
sound/soc/samsung/aries_wm8994.c:377 aries_late_probe() warn: why not propogate EPROBE_DEFER from 'irq'

On 4.09.2024 8:55 PM, Dan Carpenter wrote:
> On Fri, Aug 30, 2024 at 05:31:14PM +0200, Konrad Dybcio wrote:
>>>
>>> There are a few things which we could do:
>>>
>>> 1) Returning -EPROBE_DEFER to an ioctl or something besides a probe()
>>>    This is a bug right?  -EPROBE_DEFER is basically kernel internal for probe()
>>>    functions.  It tried to write this but it was complicated so I gave up.
>>
>> Maybe call_tree.pl can somehow be used with an if name[-5:] == "probe"
>> or something along those lines..
>>
> 
> I wrote the call_tree.pl script before I had the database.  These days I tend to
> use the database instead.
> 
> I've implemented this check but it only looks at ioctls.  I'll test it tonight.
> 
>>>
>>> 2) Printing an error message for -EPROBE_DEFER warnings
>>>    I've written this check and I can test it tonight.
>>>
> 
> I've done this.  See the attached check and the dont_print.list file attached.
> The line numbers are based on linux next.  The false positives from here are
> pretty harmless because calling dev_err_probe() is fine.
> 
>>> 3) Not propagating the -EPROBE_DEFER returns
>>>    This shouldn't be too hard to write.
>>>
> 
> I've done this too.  The false positives from this could be bad, because we only
> want to propagate -EPROBE_DEFER back from probe() functions.
> 
> See propagate.list.

This is great work, thank you Dan!

Konrad

On 29.08.2024 2:48 PM, Yu Jiaoliang wrote:
> Error handling in probe() can be a bit simpler with dev_err_probe().
> 
> Signed-off-by: Yu Jiaoliang <yujiaoliang@vivo.com>
> ---


Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>

Konrad