From: Prashant Malani <[email protected]>
When using OF graph, the fw_devlink code will create links between the
individual port driver (cros-ec-typec here) and the parent device for
a Type-C switch (like mode-switch). Since the mode-switch will in turn
have the usb-c-connector (i.e the child of the port driver) as a
supplier, fw_devlink will not be able to resolve the cyclic dependency
correctly.
As a result, the mode-switch driver probe() never runs, so mode-switches
are never registered. Because of that, the port driver probe constantly
fails with -EPROBE_DEFER, because the Type-C connector class requires all
switch devices to be registered prior to port registration.
To break this deadlock and allow the mode-switch registration to occur,
purge all the usb-c-connector nodes' absent suppliers. This eliminates
the connector as a supplier for a switch and allows it to be probed.
Signed-off-by: Prashant Malani <[email protected]>
Signed-off-by: Pin-yen Lin <[email protected]>
---
Changes in v6:
- New in v6
drivers/platform/chrome/cros_ec_typec.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 2a7ff14dc37e..f74e01d18ef3 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -382,6 +382,15 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
return -EINVAL;
}
+ /*
+ * OF graph may have set up some device links with switches, since connectors have their
+ * own compatible. Purge these to avoid a deadlock in switch probe (the switch mistakenly
+ * assumes the connector is a supplier).
+ */
+ if (dev->of_node)
+ device_for_each_child_node(dev, fwnode)
+ fw_devlink_purge_absent_suppliers(fwnode);
+
/* DT uses "reg" to specify port number. */
port_prop = dev->of_node ? "reg" : "port-number";
device_for_each_child_node(dev, fwnode) {
--
2.38.1.584.g0f3c55d4c2-goog
On Thu, Nov 24, 2022 at 06:20:51PM +0800, Pin-yen Lin wrote:
> From: Prashant Malani <[email protected]>
>
> When using OF graph, the fw_devlink code will create links between the
> individual port driver (cros-ec-typec here) and the parent device for
> a Type-C switch (like mode-switch). Since the mode-switch will in turn
> have the usb-c-connector (i.e the child of the port driver) as a
> supplier, fw_devlink will not be able to resolve the cyclic dependency
> correctly.
>
> As a result, the mode-switch driver probe() never runs, so mode-switches
> are never registered. Because of that, the port driver probe constantly
> fails with -EPROBE_DEFER, because the Type-C connector class requires all
> switch devices to be registered prior to port registration.
>
> To break this deadlock and allow the mode-switch registration to occur,
> purge all the usb-c-connector nodes' absent suppliers. This eliminates
> the connector as a supplier for a switch and allows it to be probed.
...
> + /*
> + * OF graph may have set up some device links with switches, since connectors have their
> + * own compatible. Purge these to avoid a deadlock in switch probe (the switch mistakenly
> + * assumes the connector is a supplier).
> + */
A bit too long lines...
> + if (dev->of_node)
Why do you need this check?
> + device_for_each_child_node(dev, fwnode)
> + fw_devlink_purge_absent_suppliers(fwnode);
--
With Best Regards,
Andy Shevchenko
Hi Andy,
Thanks for taking a look at this patch.
Pin-Yen beat me to the punch with comment responses, but I'll add mine anyway.
On Thu, Nov 24, 2022 at 4:25 AM Andy Shevchenko
<[email protected]> wrote:
> ...
>
> > + /*
> > + * OF graph may have set up some device links with switches, since connectors have their
> > + * own compatible. Purge these to avoid a deadlock in switch probe (the switch mistakenly
> > + * assumes the connector is a supplier).
> > + */
>
> A bit too long lines...
They are within the 100 character limit [1] which is followed
elsewhere in the driver; has something
changed recently to make that invalid?
>
> > + if (dev->of_node)
>
> Why do you need this check?
This issue only arises when using DT for this device. So the rationale
is we don't need to
perform this step on systems that don't use DT.
Best regards,
-Prashant
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144
Hi Andy,
On Thu, Nov 24, 2022 at 8:25 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, Nov 24, 2022 at 06:20:51PM +0800, Pin-yen Lin wrote:
> > From: Prashant Malani <[email protected]>
> >
> > When using OF graph, the fw_devlink code will create links between the
> > individual port driver (cros-ec-typec here) and the parent device for
> > a Type-C switch (like mode-switch). Since the mode-switch will in turn
> > have the usb-c-connector (i.e the child of the port driver) as a
> > supplier, fw_devlink will not be able to resolve the cyclic dependency
> > correctly.
> >
> > As a result, the mode-switch driver probe() never runs, so mode-switches
> > are never registered. Because of that, the port driver probe constantly
> > fails with -EPROBE_DEFER, because the Type-C connector class requires all
> > switch devices to be registered prior to port registration.
> >
> > To break this deadlock and allow the mode-switch registration to occur,
> > purge all the usb-c-connector nodes' absent suppliers. This eliminates
> > the connector as a supplier for a switch and allows it to be probed.
>
> ...
>
> > + /*
> > + * OF graph may have set up some device links with switches, since connectors have their
> > + * own compatible. Purge these to avoid a deadlock in switch probe (the switch mistakenly
> > + * assumes the connector is a supplier).
> > + */
>
> A bit too long lines...
I'll fix this in v7.
>
> > + if (dev->of_node)
>
> Why do you need this check?
We use this check to make sure only platforms using OF have their
links purged. I'm not sure if this should also be done on x86
platforms.
Best regards,
Pin-yen
>
> > + device_for_each_child_node(dev, fwnode)
> > + fw_devlink_purge_absent_suppliers(fwnode);
>
> --
> With Best Regards,
> Andy Shevchenko
>
>