Re: [PATCH] drm/meson: fix resource cleanup in meson_drv_bind_master() on error

[neil.armstrong@linaro.org]

On 19/04/2025 23:32, Martin Blumenstingl wrote:
> Hi Martijn, Hi Neil,
> 
> On Thu, Apr 10, 2025 at 8:46 PM <linux@martijnvandeventer.nl> wrote:
>>
>> Hi Martin,
>>
>> Thank you for the patch.
>>
>> I encountered this issue some time ago as well and had a possible fix in my tree (see
>> below).
>> My apologies for not upstreaming it earlier.
> No worries, we're all busy with both, offline and online life ;-)
> 
>> While my fix is not as symmetric as yours—I like symmetry—it is somewhat simpler. It
>> did make the assumption that only  calling component_unbind_all() was at fault and the the rest of the
>> code was correct. Therefore, calling one of the following functions:
>> meson_encoder_dsi_remove()
>> meson_encoder_hdmi_remove()
>> meson_encoder_cvbs_remove()
>> in case their counterpart was not called, should not result in any issues.
>>
>> I just verified, and, as far as I understand, all of these functions do a check to confirm
>> whether the encoder was initialized before proceeding with cleanup.
> Yep, that seems to be the case right now.
> Neil, would you like Martijn's more simple approach with a Fixes tag
> and backport that?
> Then I'd send my patch as a small cleanup which doesn't have to be
> backported. Otherwise I'd spin a v2 with a fix for the issue that
> Martijn found (and including Anand's Reviewed/Tested-by)?

Please send a follow-up, I'll apply this one today.

Thanks,
Neil

> 
> [...]
>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c
>>> b/drivers/gpu/drm/meson/meson_drv.c
>>> index 81d2ee37e773..031686fd4104 100644
>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>> @@ -314,35 +314,35 @@ static int meson_drv_bind_master(struct device
>>> *dev, bool has_components)
>>>                        dev_err(drm->dev, "Couldn't bind all
>>> components\n");
>>>                        /* Do not try to unbind */
>>>                        has_components = false;
>>
>> Above two lines are no longer used, so can be removed.
> Well spotted, thank you!
> 
> 
> Best regards,
> Martin