Re: [PATCH v2 2/5] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 10/6/25 7:26 PM, Laurent Pinchart wrote:
> On Mon, Oct 06, 2025 at 05:37:23PM +0300, Cristian Ciocaltea wrote:
>> On 10/6/25 3:02 PM, Dmitry Baryshkov wrote:
>>> On Mon, Oct 06, 2025 at 02:55:38AM +0300, Laurent Pinchart wrote:
>>>> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>>
>>>> The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
>>>> i.e. in some cases the error code is not included in the message, while
>>>> in some other cases there is no check for -EPROBE_DEFER.
>>>>
>>>> Since this is part of the probe path, address the aforementioned issues
>>>> by switching to dev_err_probe(), which also reduces the code a bit.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> ---
>>>>  .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c    | 62 +++++++------------
>>>>  1 file changed, 24 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> index 7d531b6f4c09..4e7794aa2dde 100644
>>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>>>> @@ -457,10 +457,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>>>  		return -ENODEV;
>>>>  
>>>>  	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
>>>> -	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
>>>> -		dev_err(dev, "Missing platform ctrl ops\n");
>>>> -		return -ENODEV;
>>>> -	}
>>>> +	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
>>>> +		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
>>>
>>> This only makes sense for the purpose of unification.
>>
>> Right, as mentioned in the commit description, the intention was to ensure
>> consistent error handling across the probe path rather than limiting the scope
>> to -EPROBE_DEFER exclusively.
> 
> Should I revert this change in v3 or keep it ? I see value in
> unification, but I don't mind either way. Dmitry, what's your preference
> ?

I missed to point out this patch has been also sent a while ago as part of
another series [1] which should be ready for merging.  It'd be great if there's
no need to revert any changes, otherwise we need to keep those in sync.

Regardless, I'll let you know if that gets applied first, allowing us to drop
this one after rebasing.

[1] https://lore.kernel.org/all/20250903-rk3588-hdmi-cec-v4-3-fa25163c4b08@collabora.com/

Regards,
Cristian