linux-kernel - Re: [PATCH v7 10/15] drm/rockchip: analogix_dp: Add support to get panel from the DP AUX bus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5dc47134-fabb-4f9c-acc3-8bf37d2cc733@rock-chips.com>
Date: Mon, 10 Mar 2025 18:09:20 +0800
From: Damon Ding <damon.ding@...k-chips.com>
To: Doug Anderson <dianders@...omium.org>
Cc: heiko@...ech.de, andy.yan@...k-chips.com, hjc@...k-chips.com,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org,
 Laurent.pinchart@...asonboard.com, jonas@...boo.se,
 jernej.skrabec@...il.com, dmitry.baryshkov@...aro.org,
 sebastian.reichel@...labora.com, cristian.ciocaltea@...labora.com,
 boris.brezillon@...labora.com, l.stach@...gutronix.de,
 dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 10/15] drm/rockchip: analogix_dp: Add support to get
 panel from the DP AUX bus

Hi Doug,

On 2025/2/25 9:42, Doug Anderson wrote:
> Hi,
> 
> On Mon, Feb 24, 2025 at 12:14 AM Damon Ding <damon.ding@...k-chips.com> wrote:
>>
>> @@ -392,11 +393,27 @@ static const struct component_ops rockchip_dp_component_ops = {
>>          .unbind = rockchip_dp_unbind,
>>   };
>>
>> +static int rockchip_dp_link_panel(struct drm_dp_aux *aux)
>> +{
>> +       struct analogix_dp_plat_data *plat_data = analogix_dp_aux_to_plat_data(aux);
>> +       struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
>> +       int ret;
>> +
>> +       ret = drm_of_find_panel_or_bridge(dp->dev->of_node, 1, 0, &plat_data->panel, NULL);
>> +       if (ret && ret != -ENODEV)
>> +               return ret;
> 
> Can you explain why you treat -ENODEV as a non-error case here? Maybe
> this is for the non-eDP case (AKA the DP case) where there's no
> further panels or bridges? Maybe a comment would be helpful to remind
> us?
> 

I think the commit 86caee745e45 ("drm/rockchip: analogix_dp: allow to 
work without panel") can help the Analogix DP driver work when the 
bridge is driver-free or when the user uses the eDP IP as a DP.

And I will add some comments in the next version.

> 
>> +       ret = component_add(dp->dev, &rockchip_dp_component_ops);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return ret;
> 
> nit: the above could just be:
> 
> return component_add(dp->dev, &rockchip_dp_component_ops);
> 

Yeah, it is a good idea.

> 
>> @@ -448,9 +460,16 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>>          if (IS_ERR(dp->adp))
>>                  return PTR_ERR(dp->adp);
>>
>> -       ret = component_add(dev, &rockchip_dp_component_ops);
>> -       if (ret)
>> -               return ret;
>> +       ret = devm_of_dp_aux_populate_bus(analogix_dp_get_aux(dp->adp), rockchip_dp_link_panel);
>> +       if (ret) {
>> +               if (ret != -ENODEV)
>> +                       return dev_err_probe(dp->dev, ret,
>> +                                            "failed to populate aux bus : %d\n", ret);
> 
> IIRC this -ENODEV case is for old legacy panels that aren't listed
> under the aux bus in the device tree. Maybe a comment would be helpful
> to remind us?

I will add a comment here if devm_of_dp_aux_populate_bus() returns -ENODEV.

> 
> nit: don't need the %d in your error message. dev_err_probe() already
> prints the error code.
> 

I will remove it in the next version.

> 
>> +               ret = rockchip_dp_link_panel(analogix_dp_get_aux(dp->adp));
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>          return 0;
> 
> You can get rid of a few of your return cases by just returning "ret" here.
> 

Yeah, it is better.

> 
> -Doug
> 
> 

Best regards
Damon