Re: [PATCH v3 06/14] drm/msm/hdmi: drop unused GPIO support

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 10/06/2022 23:47, Dmitry Baryshkov wrote:
> On 10/06/2022 23:46, Stephen Boyd wrote:
>> Quoting Dmitry Baryshkov (2022-06-09 05:23:42)
>>> The HDMI driver has code to configure extra GPIOs, which predates
>>> pinctrl support. Nowadays all platforms should use pinctrl instead.
>>> Neither of upstreamed Qualcomm platforms uses these properties, so it's
>>> safe to drop them.
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>
>> One question
>>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>>
>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c 
>>> b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> index 7267167d5ef1..6d79f1b910a5 100644
>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> @@ -233,6 +233,20 @@ static struct hdmi *msm_hdmi_init(struct 
>>> platform_device *pdev)
>>>                  hdmi->pwr_clks[i] = clk;
>>>          }
>>>
>>> +       hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", 
>>> GPIOD_IN);
>>> +       /* This will catch e.g. -EPROBE_DEFER */
>>> +       if (IS_ERR(hdmi->hpd_gpiod)) {
>>> +               ret = PTR_ERR(hdmi->hpd_gpiod);
>>> +               DRM_DEV_ERROR(&pdev->dev, "failed to get hpd gpio: 
>>> (%d)\n", ret);
>>
>> Did you want to print an error with eprobe defer in it?
> 
> True. We should use dev_err_probe here. I'll fix it in the next revision.

As a second thought, I think I'll merge it as is for now and fix all 
error messages while moving the resource allocation to the _probe() 
function. There are few other places where we are printing the 
EPROBE_DEFER error code.

> 
>>
>>> +               goto fail;
>>> +       }
>>> +
>>> +       if (!hdmi->hpd_gpiod)
>>> +               DBG("failed to get HPD gpio");
>>> +
>>> +       if (hdmi->hpd_gpiod)
>>> +               gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
>>> +
>>>          pm_runtime_enable(&pdev->dev);
>>>
>>>          hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
> 
> 


-- 
With best wishes
Dmitry