Thread (9 messages) 9 messages, 3 authors, 2021-10-06

Re: [PATCH] Revert "of: property: fw_devlink: Add support for remote-endpoint"

From: Saravana Kannan <hidden>
Date: 2021-09-27 21:58:55
Also in: lkml

On Mon, Sep 27, 2021 at 1:48 PM Dmitry Baryshkov
[off-list ref] wrote:
Since the commit f7514a663016 ("of: property: fw_devlink: Add support
for remote-endpoint") Linux kernel started parsing and adding devlinks
for the remote-endpoint properties. However this brings more harm than
good.

For all the remote-endpoints in the graph two links are created. Thus
each and every remote-endpoint ends up in the cyclic graph (instead of
the original intent of catching a cycle of graph + non-graph link):
Yes, I'm well aware of this. I even called this out in the commit
text. This creating of cycles and then catching and relaxing it is
intentional.
https://lore.kernel.org/all/20210330185056.1022008-1-saravanak@google.com/ (local)
[    0.381057] OF: remote-endpoint linking /soc@0/geniqup@9c0000/i2c@994000/hdmi-bridge@2b to /soc@0/mdss@ae00000/dsi@ae94000/ports/port@1/endpoint
[    0.394421] OF: remote-endpoint linking /soc@0/geniqup@9c0000/i2c@994000/hdmi-bridge@2b to /hdmi-out/port/endpoint
[    0.407007] OF: remote-endpoint linking /soc@0/phy@88e9000 to /soc@0/spmi@c440000/pmic@2/pmic-tcpm/connector/ports/port@0/endpoint@0
[    0.419648] OF: remote-endpoint linking /soc@0/usb@a6f8800/usb@a600000 to /soc@0/spmi@c440000/pmic@2/pmic-tcpm/ports/port@2/endpoint@0
[    0.432578] OF: remote-endpoint linking /soc@0/cci@ac4f000/i2c-bus@1/cam1@c0 to /soc@0/camss@ac6a000/ports/port@1/endpoint
[    0.444450] OF: remote-endpoint linking /soc@0/camss@ac6a000 to /soc@0/cci@ac4f000/i2c-bus@1/cam1@c0/port/endpoint
[    0.455292] OF: remote-endpoint linking /soc@0/mdss@ae00000/mdp@ae01000 to /soc@0/mdss@ae00000/dsi@ae94000/ports/port@0/endpoint
[    0.467210] OF: remote-endpoint linking /soc@0/mdss@ae00000/mdp@ae01000 to /soc@0/mdss@ae00000/dsi@ae96000/ports/port@0/endpoint
[    0.479239] OF: remote-endpoint linking /soc@0/mdss@ae00000/dsi@ae94000 to /soc@0/mdss@ae00000/mdp@ae01000/ports/port@0/endpoint
[    0.491147] OF: remote-endpoint linking /soc@0/mdss@ae00000/dsi@ae94000 to /soc@0/geniqup@9c0000/i2c@994000/hdmi-bridge@2b/ports/port@0/endpoint
[    0.504979] OF: remote-endpoint linking /soc@0/spmi@c440000/pmic@2/typec@1500 to /soc@0/spmi@c440000/pmic@2/pmic-tcpm/ports/port@0/endpoint
[    0.517958] OF: remote-endpoint linking /soc@0/spmi@c440000/pmic@2/pdphy@1700 to /soc@0/spmi@c440000/pmic@2/pmic-tcpm/ports/port@1/endpoint
[    0.565326] OF: remote-endpoint linking /hdmi-out to /soc@0/geniqup@9c0000/i2c@994000/hdmi-bridge@2b/ports/port@2/endpoint

Under some conditions the device can become it's own supplier,
preventing this device to be probed at all:
I'm not sure this analysis is correct -- this shouldn't be happening.
If you go to the device link folder and cat "sync_state_only", I
expect it to be "1" in this case. Can you confirm that?

Which means it won't block probing. Yes, the link itself is useless
and it'll get auto deleted once mdss probes and it's easy to not
create it in the first place. But this is definitely not your issue.
$ ls -l /sys/bus/platform/devices/ae00000.mdss/
lrwxrwxrwx    1 root     root             0 Aug  4 15:13 consumer:platform:ae00000.mdss -> ../../../virtual/devlink/platform:ae00000.mdss--platform:ae00000.mdss

I think that until of_link can be tought to handle bi-directional links
on its own, we should not parse remote-endpoint properties. Thus the
aforementioned commit should be reverted.
Nak. remote-endpoint parsing is working as intended. I don't think the
analysis is correct.

Can you please enable the logs in all these functions and attach the
log so we can see why it's not probing mdss?
device_link_add
device_links_check_suppliers
func fw_devlink_relax_link
fw_devlink_create_devlink

-Saravana
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help