[PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver

[Dmitry Osipenko]

On 9/24/18 2:59 PM, Thierry Reding wrote:
> On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote:
>> Some of definitions in the code changed the meaning, unfortunately one
>> place missed the change.
>>
>> Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support")
>> Cc: <stable at vger.kernel.org> # v4.8+
>> Signed-off-by: Dmitry Osipenko <digetx at gmail.com>
>> ---
>>
>> I don't have HW to test DPAUX driver, apparently it has been broken for
>> 2+ years now. There is also a known issue on with the DPAUX driver that
>> prevents it from probing, that was discussed on the #tegra IRC. Thierry,
>> please take a closer look at this driver and test it thoroughly, it has
>> some obvious problems.
>>
>>   drivers/gpu/drm/tegra/dpaux.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> It's odd that you claim that the driver is always failing probe and at
> the same time you say that you don't have hardware to test the driver.
> =)
> 
> I know for a fact that this driver does not usually fail because it is
> required on all recent chips (Tegra210 and later) to drive HDMI, which
> we support on all boards, so it is indeed thoroughly tested.
> 
>>
>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
>> index d84e81ff36ad..ba5681fab73b 100644
>> --- a/drivers/gpu/drm/tegra/dpaux.c
>> +++ b/drivers/gpu/drm/tegra/dpaux.c
>> @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>>   	 * is no possibility to perform the I2C mode configuration in the
>>   	 * HDMI path.
>>   	 */
>> -	err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C);
>> +	err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C);
>>   	if (err < 0)
>>   		return err;
>>   
> 
> If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and
> DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which
> is a good explanation for why the driver performs flawlessly.
> 
> That said, your change is obviously correct. I've applied it, but since
> it doesn't actually fix anything, and doesn't change anything from a
> binary point of view, I've removed the Fixes: and Cc: stable tags.

That's the great news! Somehow it even didn't strike me to check that the actual values 
are the same. In a longer term it should be better to use enum's to avoid such confusions 
in the future, compiler will complain about the wrong types.