Hi Uwe,
Thank you for the patch.
On Sat, Mar 18, 2023 at 08:07:46PM +0100, Uwe Kleine-König wrote:
> Replace the generic error message issued by the driver core when the remove
> callback returns non-zero ("remove callback returned a non-zero value. This
> will be ignored.") by a message that tells the actual problem.
>
> Also simplify a bit by checking the return value of wait_event_timeout a
> bit later.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
> .../drm/bridge/cadence/cdns-mhdp8546-core.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index f6822dfa3805..d74c6fa30ccc 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2574,7 +2574,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> {
> struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
> unsigned long timeout = msecs_to_jiffies(100);
> - bool stop_fw = false;
> int ret;
>
> drm_bridge_remove(&mhdp->bridge);
> @@ -2582,18 +2581,19 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> ret = wait_event_timeout(mhdp->fw_load_wq,
> mhdp->hw_state == MHDP_HW_READY,
> timeout);
> - if (ret == 0)
> - dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> - __func__);
> - else
> - stop_fw = true;
> -
> spin_lock(&mhdp->start_lock);
> mhdp->hw_state = MHDP_HW_STOPPED;
> spin_unlock(&mhdp->start_lock);
>
> - if (stop_fw)
> + if (ret == 0) {
> + dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> + __func__);
> + } else {
> ret = cdns_mhdp_set_firmware_active(mhdp, false);
> + if (ret)
> + dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n",
> + ERR_PTR(ret));
Why not simply
dev_err(mhdp->dev, "Failed to stop firmware (%d)\n",
ret);
? Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> + }
>
> phy_exit(mhdp->phy);
>
> @@ -2609,7 +2609,7 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(mhdp->clk);
>
> - return ret;
> + return 0;
> }
>
> static const struct of_device_id mhdp_ids[] = {
--
Regards,
Laurent Pinchart
