Re:Re: [PATCH] drm/rockchip: Use dev_err_probe() to simplify code

From: "Andy Yan" <andyshrk@163.com>
To: "Krzysztof Kozlowski" <krzk@kernel.org>
Cc: "Heiko Stübner" <heiko@sntech.de>,
	long.yunjian@zte.com.cn, hjc@rock-chips.com,
	andy.yan@rock-chips.com, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com,
	simona@ffwll.ch, dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	fang.yumeng@zte.com.cn, mou.yi@zte.com.cn, xu.lifeng1@zte.com.cn,
	ouyang.maochun@zte.com.cn
Subject: Re:Re: [PATCH] drm/rockchip: Use dev_err_probe() to simplify code
Date: Fri, 23 May 2025 17:36:03 +0800 (CST)	[thread overview]
Message-ID: <5d6ab5c0.93d3.196fc7fe8c9.Coremail.andyshrk@163.com> (raw)
In-Reply-To: <ba9a49d5-132f-4be8-9f64-4904c91e26be@kernel.org>

Hi,

在 2025-05-22 16:45:08，"Krzysztof Kozlowski" <krzk@kernel.org> 写道：
>On 16/05/2025 11:58, Heiko Stübner wrote:
>> Am Donnerstag, 15. Mai 2025, 17:54:20 Mitteleuropäische Sommerzeit schrieb Krzysztof Kozlowski:
>>> On 15/05/2025 14:35, long.yunjian@zte.com.cn wrote:
>>>> From: Yumeng Fang <fang.yumeng@zte.com.cn>
>>>>
>>>> In the probe path, dev_err() can be replaced with dev_err_probe()
>>>
>>> That's not probe path. I am not sure if you really understand this code.
>> 
>> I think that is somewhat debateable.
>> 
>> dw_hdmi_rockchip_bind() is part of the rockchip-drm component device,
>> so part of its probe-path. Also I think just the presence of EPROBE_DEFER
>> which causes the device to re-try probing later is a nice indicator that the
>
>No, that's not true. You can call every API like regulator_get from any
>context and you will get EPROBE_DEFER. This will not be a probe path.
>There are multiple cases of such drivers, I saw such patch even day ago.
>
>> code in question is _a_ probe path. (and usage of EPROBE_DEFER is an
>> established pattern to make that component device re-try probing later)
>> 
>> And the parse_dt function itself is part of that path too.
>
>I quickly glanced and this was not obvious. The commit msg is poor here
>and does not explain that component_bind is ALWAYS probe path (unless it
>is clear for DRM folks).

Yes, the component_bind is called in the probe path, here is one dump stack:

[    1.113096] platform fdd90000.vop: Adding to iommu group 8
[    1.120336] rockchip-drm display-subsystem: bound fdd90000.vop (ops vop2_component_ops)
[    1.121543] rockchip-drm display-subsystem: bound fde50000.dp (ops dw_dp_rockchip_component_ops)
[    1.122834] dwhdmiqp-rockchip fde80000.hdmi: registered DesignWare HDMI QP I2C bus driver
[    1.123554] CPU: 5 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.15.0-rc5+ #667 PREEMPT 
[    1.123561] Hardware name: RK3588S CoolPi 4 Model B (DT)
[    1.123565] Call trace:
[    1.123567]  show_stack+0x18/0x24 (C)
[    1.123577]  dump_stack_lvl+0x74/0x8c
[    1.123584]  dump_stack+0x18/0x24
[    1.123589]  dw_hdmi_qp_rockchip_bind+0x2f8/0x468
[    1.123601]  component_bind_all+0x120/0x278
[    1.123611]  rockchip_drm_bind+0x9c/0x1ac
[    1.123616]  try_to_bring_up_aggregate_device+0x214/0x2d8
[    1.123625]  component_master_add_with_match+0xc4/0x104
[    1.123635]  rockchip_drm_platform_probe+0x214/0x33c
[    1.123640]  platform_probe+0x68/0xc8
[    1.123646]  really_probe+0xc0/0x390
[    1.123655]  __driver_probe_device+0x7c/0x15c
[    1.123663]  driver_probe_device+0x3c/0x110
[    1.123671]  __driver_attach+0xf0/0x1f8
[    1.123679]  bus_for_each_dev+0x7c/0xe0
[    1.123686]  driver_attach+0x24/0x30
>
>
>Best regards,
>Krzysztof
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip