Re: [RFC PATCH v1 01/12] Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors"

[Heikki Krogerus]

Hi Dmitry,

On Mon, Sep 04, 2023 at 12:41:39AM +0300, Dmitry Baryshkov wrote:
> The kdev->fwnode pointer is never set in drm_sysfs_connector_add(), so
> dev_fwnode() checks never succeed, making the respective commit NOP.

That's not true. The dev->fwnode is assigned when the device is
created on ACPI platforms automatically. If the drm_connector fwnode
member is assigned before the device is registered, then that fwnode
is assigned also to the device - see drm_connector_acpi_find_companion().

But please note that even if drm_connector does not have anything in
its fwnode member, the device may still be assigned fwnode, just based
on some other logic (maybe in drivers/acpi/acpi_video.c?).

> And if drm_sysfs_connector_add() is modified to set kdev->fwnode, it
> breaks drivers already using components (as it was pointed at [1]),
> resulting in a deadlock. Lockdep trace is provided below.
> 
> Granted these two issues, it seems impractical to fix this commit in any
> sane way. Revert it instead.

I think there is already user space stuff that relies on these links,
so I'm not sure you can just remove them like that. If the component
framework is not the correct tool here, then I think you need to
suggest some other way of creating them.

Side note. The problem you are describing here is a limitation in the
component framework - right now it's made with the idea that a device
can represent a single component, but it really should allow a device
to represent multiple components. I'm not saying that you should try
to fix the component framework, but I just wanted to make a note about
this (and this is not the only problem with the component framework).

I like the component framework as a concept, but I think it needs a
lot of improvements - possibly rewrite.

thanks,

-- 
heikki