| Message ID | 20240829124813.3264437-1-yujiaoliang@vivo.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] soc: qcom: pbs: Simplify with dev_err_probe() | expand |
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
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 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 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 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
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);
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(-)