| Message ID | 20210714061807.5737-1-martin.kepplinger@puri.sm |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] usb: typec: tipd: Don't block probing of consumer of "connector" nodes | expand |
Wed, Jul 14, 2021 at 08:18:07AM +0200, Martin Kepplinger kirjoitti: > Similar as with tcpm this patch lets fw_devlink know not to wait on the > fwnode to be populated as a struct device. > > Without this patch, USB functionality can be broken on some previously > supported boards. > > Fixes: 28ec344bb891 ("usb: typec: tcpm: Don't block probing of consumers of "connector" nodes") > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > > revision history > ---------------- > v2: (thank you Saravana) > * add a code-comment why the call is needed. > > v1: > https://lore.kernel.org/linux-usb/20210713073946.102501-1-martin.kepplinger@puri.sm/ > > > > drivers/usb/typec/tipd/core.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index 938219bc1b4b..21b3ae25c76d 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -629,6 +629,15 @@ static int tps6598x_probe(struct i2c_client *client) > if (!fwnode) > return -ENODEV; > > + /* > + * This fwnode has a "compatible" property, but is never populated as a > + * struct device. Instead we simply parse it to read the properties. > + * This breaks fw_devlink=on. To maintain backward compatibility > + * with existing DT files, we work around this by deleting any > + * fwnode_links to/from this fwnode. > + */ > + fw_devlink_purge_absent_suppliers(fwnode); > + > tps->role_sw = fwnode_usb_role_switch_get(fwnode); > if (IS_ERR(tps->role_sw)) { > ret = PTR_ERR(tps->role_sw); > -- > 2.30.2 -- heikki
On Tue, Jul 13, 2021 at 11:18 PM Martin Kepplinger <martin.kepplinger@puri.sm> wrote: > > Similar as with tcpm this patch lets fw_devlink know not to wait on the > fwnode to be populated as a struct device. > > Without this patch, USB functionality can be broken on some previously > supported boards. > > Fixes: 28ec344bb891 ("usb: typec: tcpm: Don't block probing of consumers of "connector" nodes") > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > --- > > revision history > ---------------- > v2: (thank you Saravana) > * add a code-comment why the call is needed. > > v1: > https://lore.kernel.org/linux-usb/20210713073946.102501-1-martin.kepplinger@puri.sm/ > > > > drivers/usb/typec/tipd/core.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index 938219bc1b4b..21b3ae25c76d 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -629,6 +629,15 @@ static int tps6598x_probe(struct i2c_client *client) > if (!fwnode) > return -ENODEV; > > + /* > + * This fwnode has a "compatible" property, but is never populated as a > + * struct device. Instead we simply parse it to read the properties. > + * This breaks fw_devlink=on. To maintain backward compatibility > + * with existing DT files, we work around this by deleting any > + * fwnode_links to/from this fwnode. > + */ > + fw_devlink_purge_absent_suppliers(fwnode); > + Hey Martin, As part of a series I'm working on, I'm looking into deleting fw_devlink_purge_absent_suppliers() and having the driver core figure this out automatically. So I was making sure all the current uses of fw_devlink_purge_absent_suppliers() would automatically be handled by driver core. But when I looked at this usage of fw_devlink_purge_absent_suppliers(), I didn't see any "compatible" property in the "connector" child node. So, I'm confused how you had a problem without this patch. fw_devlink would have created fwnode links between typec_pd and usb_dwc3_0, figured out it's a cycle and have stopped enforcing the "remote-endpoint" dependencies. Can you give me more details please on why you were having a problem without this patch? I'm looking at arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi as an example because you listed it in your v1 patch. -Saravana > tps->role_sw = fwnode_usb_role_switch_get(fwnode); > if (IS_ERR(tps->role_sw)) { > ret = PTR_ERR(tps->role_sw); > -- > 2.30.2 >
Am Donnerstag, dem 21.07.2022 um 16:06 -0700 schrieb Saravana Kannan: > On Tue, Jul 13, 2021 at 11:18 PM Martin Kepplinger > <martin.kepplinger@puri.sm> wrote: > > > > Similar as with tcpm this patch lets fw_devlink know not to wait on > > the > > fwnode to be populated as a struct device. > > > > Without this patch, USB functionality can be broken on some > > previously > > supported boards. > > > > Fixes: 28ec344bb891 ("usb: typec: tcpm: Don't block probing of > > consumers of "connector" nodes") > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > --- > > > > revision history > > ---------------- > > v2: (thank you Saravana) > > * add a code-comment why the call is needed. > > > > v1: > > https://lore.kernel.org/linux-usb/20210713073946.102501-1-martin.kepplinger@puri.sm/ > > > > > > > > drivers/usb/typec/tipd/core.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/usb/typec/tipd/core.c > > b/drivers/usb/typec/tipd/core.c > > index 938219bc1b4b..21b3ae25c76d 100644 > > --- a/drivers/usb/typec/tipd/core.c > > +++ b/drivers/usb/typec/tipd/core.c > > @@ -629,6 +629,15 @@ static int tps6598x_probe(struct i2c_client > > *client) > > if (!fwnode) > > return -ENODEV; > > > > + /* > > + * This fwnode has a "compatible" property, but is never > > populated as a > > + * struct device. Instead we simply parse it to read the > > properties. > > + * This breaks fw_devlink=on. To maintain backward > > compatibility > > + * with existing DT files, we work around this by deleting > > any > > + * fwnode_links to/from this fwnode. > > + */ > > + fw_devlink_purge_absent_suppliers(fwnode); > > + > > Hey Martin, > > As part of a series I'm working on, I'm looking into deleting > fw_devlink_purge_absent_suppliers() and having the driver core figure > this out automatically. > > So I was making sure all the current uses of > fw_devlink_purge_absent_suppliers() would automatically be handled by > driver core. But when I looked at this usage of > fw_devlink_purge_absent_suppliers(), I didn't see any "compatible" > property in the "connector" child node. So, I'm confused how you had > a > problem without this patch. > > fw_devlink would have created fwnode links > between typec_pd and usb_dwc3_0, figured out it's a cycle and have > stopped enforcing the "remote-endpoint" dependencies. > > Can you give me more details please on why you were having a problem > without this patch? > > I'm looking at arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi as > an > example because you listed it in your v1 patch. > > -Saravana It's downstream. I'm very sorry about that. The patch that adds the compatible string ( https://source.puri.sm/martin.kepplinger/linux-next/-/commit/3d887ea76158851ae50f192094d7865cc3f4da9d ) sits in our tree ( https://source.puri.sm/martin.kepplinger/linux-next/-/commits/5.19-rc7/librem5__integration ) and I'll make sure it'll be sent out after I'm back from vacations in a few weeks. Sorry for the confusion! martin > > > tps->role_sw = fwnode_usb_role_switch_get(fwnode); > > if (IS_ERR(tps->role_sw)) { > > ret = PTR_ERR(tps->role_sw); > > -- > > 2.30.2 > >
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index 938219bc1b4b..21b3ae25c76d 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -629,6 +629,15 @@ static int tps6598x_probe(struct i2c_client *client) if (!fwnode) return -ENODEV; + /* + * This fwnode has a "compatible" property, but is never populated as a + * struct device. Instead we simply parse it to read the properties. + * This breaks fw_devlink=on. To maintain backward compatibility + * with existing DT files, we work around this by deleting any + * fwnode_links to/from this fwnode. + */ + fw_devlink_purge_absent_suppliers(fwnode); + tps->role_sw = fwnode_usb_role_switch_get(fwnode); if (IS_ERR(tps->role_sw)) { ret = PTR_ERR(tps->role_sw);
Similar as with tcpm this patch lets fw_devlink know not to wait on the fwnode to be populated as a struct device. Without this patch, USB functionality can be broken on some previously supported boards. Fixes: 28ec344bb891 ("usb: typec: tcpm: Don't block probing of consumers of "connector" nodes") Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> --- revision history ---------------- v2: (thank you Saravana) * add a code-comment why the call is needed. v1: https://lore.kernel.org/linux-usb/20210713073946.102501-1-martin.kepplinger@puri.sm/ drivers/usb/typec/tipd/core.c | 9 +++++++++ 1 file changed, 9 insertions(+)