On Thu, Oct 31, 2024 at 12:45:24AM +0800, Sui Jingfeng wrote: > Hi, > > On 2024/10/18 23:43, Dmitry Baryshkov wrote: > > On Fri, Oct 18, 2024 at 03:49:34PM +0300, Abel Vesa wrote: > >> The assignment of the of_node to the aux bridge needs to mark the > >> of_node as reused as well, otherwise resource providers like pinctrl will > >> report a gpio as already requested by a different device when both pinconf > >> and gpios property are present. > >> Fix that by using the device_set_of_node_from_dev() helper instead. > >> > >> Fixes: 6914968a0b52 ("drm/bridge: properly refcount DT nodes in aux bridge drivers") > >> Cc: stable@xxxxxxxxxxxxxxx # 6.8 > >> Cc: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > >> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx> > >> --- > >> Changes in v2: > >> - Re-worded commit to be more explicit of what it fixes, as Johan suggested > >> - Used device_set_of_node_from_dev() helper, as per Johan's suggestion > >> - Added Fixes tag and cc'ed stable > >> - Link to v1: https://lore.kernel.org/r/20241017-drm-aux-bridge-mark-of-node-reused-v1-1-7cd5702bb4f2@xxxxxxxxxx > >> --- > >> drivers/gpu/drm/bridge/aux-bridge.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > Technically speaking, your driver just move the burden to its caller. > Because this driver requires its user call drm_aux_bridge_register() > to create an AUX child device manually, you need it call ida_alloc() > to generate a unique id. There's a relevant discussion for a ti-sn65dsi86 patch, see https://lore.kernel.org/r/20241030102846.GB14276@xxxxxxxxxxxxxxxxxxxxxxxxxx I agree it shouldn't be the responsibility of each caller to generate unique IDs. > Functions symbols still have to leak to other subsystems, which is > not really preserve coding sharing. > > What's worse, the action that allocating unique device id traditionally > is the duty of driver core. Why breaks (so called) perfect device driver > model by moving that out of core. Especially in the DT world that the > core knows very well how to populate device instance and manage the > reference counter. > > HPD handling is traditionally belongs to connector, create standalone > driver like this one *abuse* to both Maxime's simple bridge driver and > Laurent's display-connector bridge driver or drm_bridge_connector or > whatever. Why those work can't satisfy you? At least, their drivers > are able to passing the mode setting states to the next bridge. > > Basically those AUX drivers implementation abusing the definition of > bridge, abusing the definition of connector and abusing the DT. > Its just manually populate instances across drivers. -- Regards, Laurent Pinchart