| Message ID | 20230301214952.2190757-1-saravanak@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove use of fw_devlink_purge_absent_suppliers() | expand |
Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan: > Yongqin, Martin, Amelie, > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > ("mtd: mtdpart: Don't create platform device that'll never probe"), > fw_devlink is smarter and doesn't depend on compatible property. So, > I > don't think these calls are needed anymore. But I don't have these > devices to test on and be sure and the hardware I use to test changes > doesn't have this issue either. > > Can you please test these changes on the hardware where you hit the > issue to make sure things work as expected? > > Yongqin, If you didn't have the context, this affected hikey960. > > Greg, > > Let's wait for some tests before we land these. > > Thanks, > Saravana hi Sravana, I picked the 12 commits leading up to commit fb42378dcc7f ("mtd: mtdpart: Don't create platform device that'll never probe") ( https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink ) and included the tipd patch below to test it. With that, I get the following errors: [ 0.237931] imx-uart 30890000.serial: Failed to create device link with regulator-gnss [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device link with regulator-lcd-1v8 [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device link with backlight-dsi but they are independent of this final tipd patch below. I'll test a real linux-next tree soon, for completeness, maybe I missed something? Anyways, on that tree, your tipd removal patch breaks type-c still for me, imx8mq-librem5.dtsi just to give a first reply quickly... thanks, martin > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > Saravana Kannan (4): > usb: typec: stusb160x: Remove use of > fw_devlink_purge_absent_suppliers() > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > driver core: Delete fw_devlink_purge_absent_suppliers() > > drivers/base/core.c | 16 ---------------- > drivers/usb/typec/stusb160x.c | 9 --------- > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > drivers/usb/typec/tipd/core.c | 9 --------- > include/linux/fwnode.h | 1 - > 5 files changed, 44 deletions(-) >
Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin Kepplinger: > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan: > > Yongqin, Martin, Amelie, > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > fw_devlink is smarter and doesn't depend on compatible property. > > So, > > I > > don't think these calls are needed anymore. But I don't have these > > devices to test on and be sure and the hardware I use to test > > changes > > doesn't have this issue either. > > > > Can you please test these changes on the hardware where you hit the > > issue to make sure things work as expected? > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > Greg, > > > > Let's wait for some tests before we land these. > > > > Thanks, > > Saravana > > hi Sravana, > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd: > mtdpart: Don't create platform device that'll never probe") ( > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > ) and included the tipd patch below to test it. > > With that, I get the following errors: > > [ 0.237931] imx-uart 30890000.serial: Failed to create device link > with regulator-gnss > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device > link > with regulator-lcd-1v8 > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device > link > with backlight-dsi > > but they are independent of this final tipd patch below. I'll test a > real linux-next tree soon, for completeness, maybe I missed > something? > > Anyways, on that tree, your tipd removal patch breaks type-c still > for > me, imx8mq-librem5.dtsi > > just to give a first reply quickly... thanks, > > martin > just confirming: it's the same as above on next-20230302 + this patch ( https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 ) with the errors already independent from the patch. I should have tested earlier patches -.- martin
Greg, Don't pull in this series please. It needs more testing from the folks I cc'ed and it's already breaking things for Martin. This needs more revisions. -Saravana On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote: > > Yongqin, Martin, Amelie, > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > ("mtd: mtdpart: Don't create platform device that'll never probe"), > fw_devlink is smarter and doesn't depend on compatible property. So, I > don't think these calls are needed anymore. But I don't have these > devices to test on and be sure and the hardware I use to test changes > doesn't have this issue either. > > Can you please test these changes on the hardware where you hit the > issue to make sure things work as expected? > > Yongqin, If you didn't have the context, this affected hikey960. > > Greg, > > Let's wait for some tests before we land these. > > Thanks, > Saravana > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > Saravana Kannan (4): > usb: typec: stusb160x: Remove use of > fw_devlink_purge_absent_suppliers() > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > driver core: Delete fw_devlink_purge_absent_suppliers() > > drivers/base/core.c | 16 ---------------- > drivers/usb/typec/stusb160x.c | 9 --------- > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > drivers/usb/typec/tipd/core.c | 9 --------- > include/linux/fwnode.h | 1 - > 5 files changed, 44 deletions(-) > > -- > 2.39.2.722.g9855ee24e9-goog >
Hi, Saravana Sorry for the lateness, I was just aware of this today. I tested with the ACK android-mainline branch + the 12 commits ending with fb42378dcc7f + the 4 commits of this series + hikey960 AOSP Master userspace. The hikey960 Android build could boot to the home screen, no stuck there, Here is the link of the logat in case you want to check some message here: https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5 Thanks, Yongqin Liu On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote: > > Greg, > > Don't pull in this series please. It needs more testing from the folks > I cc'ed and it's already breaking things for Martin. This needs more > revisions. > > -Saravana > > On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote: > > > > Yongqin, Martin, Amelie, > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > fw_devlink is smarter and doesn't depend on compatible property. So, I > > don't think these calls are needed anymore. But I don't have these > > devices to test on and be sure and the hardware I use to test changes > > doesn't have this issue either. > > > > Can you please test these changes on the hardware where you hit the > > issue to make sure things work as expected? > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > Greg, > > > > Let's wait for some tests before we land these. > > > > Thanks, > > Saravana > > > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > > Cc: Sumit Semwal <sumit.semwal@linaro.org> > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > > > Saravana Kannan (4): > > usb: typec: stusb160x: Remove use of > > fw_devlink_purge_absent_suppliers() > > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > > driver core: Delete fw_devlink_purge_absent_suppliers() > > > > drivers/base/core.c | 16 ---------------- > > drivers/usb/typec/stusb160x.c | 9 --------- > > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > > drivers/usb/typec/tipd/core.c | 9 --------- > > include/linux/fwnode.h | 1 - > > 5 files changed, 44 deletions(-) > > > > -- > > 2.39.2.722.g9855ee24e9-goog > >
On Thu, Mar 9, 2023 at 10:53 AM Yongqin Liu <yongqin.liu@linaro.org> wrote: > > Hi, Saravana > > Sorry for the lateness, I was just aware of this today. No worries. > I tested with the ACK android-mainline branch + the 12 commits ending > with fb42378dcc7f > + the 4 commits of this series + hikey960 AOSP Master userspace. > The hikey960 Android build could boot to the home screen, no stuck there, Thanks for testing! Can you confirm what happens if you drop the "12 commits ending with fb42378dcc7f" ? Does it get stuck at boot or have some limited functionality? It's surprising that for the same type of DT node, in your case fw_devlink is able to handle it correctly, but no so for Martin's case. -Saravana > > Here is the link of the logat in case you want to check some message here: > https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5 > > Thanks, > Yongqin Liu > On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote: > > > > Greg, > > > > Don't pull in this series please. It needs more testing from the folks > > I cc'ed and it's already breaking things for Martin. This needs more > > revisions. > > > > -Saravana > > > > On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote: > > > > > > Yongqin, Martin, Amelie, > > > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > > fw_devlink is smarter and doesn't depend on compatible property. So, I > > > don't think these calls are needed anymore. But I don't have these > > > devices to test on and be sure and the hardware I use to test changes > > > doesn't have this issue either. > > > > > > Can you please test these changes on the hardware where you hit the > > > issue to make sure things work as expected? > > > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > > > Greg, > > > > > > Let's wait for some tests before we land these. > > > > > > Thanks, > > > Saravana > > > > > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > > > Cc: Sumit Semwal <sumit.semwal@linaro.org> > > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > > > > > Saravana Kannan (4): > > > usb: typec: stusb160x: Remove use of > > > fw_devlink_purge_absent_suppliers() > > > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > > > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > > > driver core: Delete fw_devlink_purge_absent_suppliers() > > > > > > drivers/base/core.c | 16 ---------------- > > > drivers/usb/typec/stusb160x.c | 9 --------- > > > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > > > drivers/usb/typec/tipd/core.c | 9 --------- > > > include/linux/fwnode.h | 1 - > > > 5 files changed, 44 deletions(-) > > > > > > -- > > > 2.39.2.722.g9855ee24e9-goog > > > > > > > -- > Best Regards, > Yongqin Liu > --------------------------------------------------------------- > #mailing list > linaro-android@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-android
On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger <martin.kepplinger@puri.sm> wrote: > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin Kepplinger: > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana Kannan: > > > Yongqin, Martin, Amelie, > > > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > > fw_devlink is smarter and doesn't depend on compatible property. > > > So, > > > I > > > don't think these calls are needed anymore. But I don't have these > > > devices to test on and be sure and the hardware I use to test > > > changes > > > doesn't have this issue either. > > > > > > Can you please test these changes on the hardware where you hit the > > > issue to make sure things work as expected? > > > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > > > Greg, > > > > > > Let's wait for some tests before we land these. > > > > > > Thanks, > > > Saravana > > > > hi Sravana, > > > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd: > > mtdpart: Don't create platform device that'll never probe") ( > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > ) and included the tipd patch below to test it. > > > > With that, I get the following errors: > > > > [ 0.237931] imx-uart 30890000.serial: Failed to create device link > > with regulator-gnss > > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > link > > with regulator-lcd-1v8 > > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > link > > with backlight-dsi > > > > but they are independent of this final tipd patch below. I'll test a > > real linux-next tree soon, for completeness, maybe I missed > > something? > > > > Anyways, on that tree, your tipd removal patch breaks type-c still > > for > > me, imx8mq-librem5.dtsi > > > > just to give a first reply quickly... thanks, > > > > martin > > > > just confirming: it's the same as above on next-20230302 + this patch ( > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 > ) with the errors already independent from the patch. I should have > tested earlier patches -.- Thanks a lot for testing Martin! Your email is a little ambiguous to me. With the 12 refactor commits + the 4 patches in this series, things are breaking for you. But if you drop the 4 patches in this series, things work again. Is that right? Let's ignore the "Failed to create device link" errors for now -- it's not related to this usb-c-connector series. It's basically pointing out issues that we ignored silently in the past -- it's basically pointing out holes in fw_devlink's visibility of devices. I'll get to them later. -Saravana
On Thu, Mar 09, 2023 at 10:04:49AM -0800, Saravana Kannan wrote: > Greg, > > Don't pull in this series please. It needs more testing from the folks > I cc'ed and it's already breaking things for Martin. This needs more > revisions. Ah, missed that, sorry. I've dropped all 4 of these from my tree now, please resend a new version when you all have it worked out. thanks, greg k-h
Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana Kannan: > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger > <martin.kepplinger@puri.sm> wrote: > > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin > > Kepplinger: > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana > > > Kannan: > > > > Yongqin, Martin, Amelie, > > > > > > > > We recent refactor of fw_devlink that ends with commit > > > > fb42378dcc7f > > > > ("mtd: mtdpart: Don't create platform device that'll never > > > > probe"), > > > > fw_devlink is smarter and doesn't depend on compatible > > > > property. > > > > So, > > > > I > > > > don't think these calls are needed anymore. But I don't have > > > > these > > > > devices to test on and be sure and the hardware I use to test > > > > changes > > > > doesn't have this issue either. > > > > > > > > Can you please test these changes on the hardware where you hit > > > > the > > > > issue to make sure things work as expected? > > > > > > > > Yongqin, If you didn't have the context, this affected > > > > hikey960. > > > > > > > > Greg, > > > > > > > > Let's wait for some tests before we land these. > > > > > > > > Thanks, > > > > Saravana > > > > > > hi Sravana, > > > > > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd: > > > mtdpart: Don't create platform device that'll never probe") ( > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > > ) and included the tipd patch below to test it. > > > > > > With that, I get the following errors: > > > > > > [ 0.237931] imx-uart 30890000.serial: Failed to create device > > > link > > > with regulator-gnss > > > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > > link > > > with regulator-lcd-1v8 > > > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > > link > > > with backlight-dsi > > > > > > but they are independent of this final tipd patch below. I'll > > > test a > > > real linux-next tree soon, for completeness, maybe I missed > > > something? > > > > > > Anyways, on that tree, your tipd removal patch breaks type-c > > > still > > > for > > > me, imx8mq-librem5.dtsi > > > > > > just to give a first reply quickly... thanks, > > > > > > martin > > > > > > > just confirming: it's the same as above on next-20230302 + this > > patch ( > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 > > ) with the errors already independent from the patch. I should have > > tested earlier patches -.- > > Thanks a lot for testing Martin! > > Your email is a little ambiguous to me. With the 12 refactor commits > + > the 4 patches in this series, things are breaking for you. But if you > drop the 4 patches in this series, things work again. Is that right? no. Sorry if I wasn't clear. I can't justify to block these 4 patches. they *themselves* don't break anything. Something broke *earlier* than these 4 patches in one of the other 12. As to *what* broke: The error messages during boot, and the charger and fuel gauge don't work anymore, I see: # cat /sys/kernel/debug/devices_deferred 3-0036 max17042: failed: power supply register 0-003f 38100000.usb platform: wait for supplier endpoint 32c00000.hdmi platform: supplier 0-003f not ready 3-006a bq25890-charger: registering power supply I haven't had time to track it down and see where the issue is. Could be one of the 12 patches, could be a wrong description for my board: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi or something else (an i2c issue?). (for completeness, the exact tree I ran: https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink ) > > Let's ignore the "Failed to create device link" errors for now -- > it's > not related to this usb-c-connector series. It's basically pointing > out issues that we ignored silently in the past -- it's basically > pointing out holes in fw_devlink's visibility of devices. I'll get to > them later. ok. good to know. please do. And since I'm the only one seeing an issue, please move the patches forward. I'll need to fix this after the fact and hope to have time for it next week. > > -Saravana martin
On 3/1/23 22:49, Saravana Kannan wrote: > Yongqin, Martin, Amelie, > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > ("mtd: mtdpart: Don't create platform device that'll never probe"), > fw_devlink is smarter and doesn't depend on compatible property. So, I > don't think these calls are needed anymore. But I don't have these > devices to test on and be sure and the hardware I use to test changes > doesn't have this issue either. > > Can you please test these changes on the hardware where you hit the > issue to make sure things work as expected? Hi Saravana, Sorry for the late reply, On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top of commit fb42378dcc7f, and also with your series applied. For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi I noticed some error messages on this board, since the 12 patch series, around the I2C PMIC device links: [ 3.585514] i2c 1-0033: Failed to create device link with 1-0033 [ 3.590115] i2c 1-0033: Failed to create device link with 1-0033 [ 3.596278] i2c 1-0033: Failed to create device link with 1-0033 [ 3.602188] i2c 1-0033: Failed to create device link with 1-0033 [ 3.608165] i2c 1-0033: Failed to create device link with 1-0033 [ 3.614278] i2c 1-0033: Failed to create device link with 1-0033 [ 3.620256] i2c 1-0033: Failed to create device link with 1-0033 [ 3.626253] i2c 1-0033: Failed to create device link with 1-0033 [ 3.632252] i2c 1-0033: Failed to create device link with 1-0033 [ 3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10 [ 3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed dependency cycle(s) with /soc/i2c@5c00200 0/stpmic@33/regulators/boost [ 3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed dependency cycle(s) with /soc/i2c@5c00200 0/stpmic@33/regulators/buck2 [ 3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed dependency cycle(s) with /soc/i2c@5c00200 0/stpmic@33/regulators/buck4 [ 3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 [ 3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Failed to create device link with 1-0033 Strangely some of the regulators seems to have "Fixed dependency", but not all. Regarding the typec stusb160x I noticed the message below. It seems correct, right ? [ 15.962771] typec port0: Fixed dependency cycle(s) with /soc/usb-otg@49000000/port/endpoint But sometimes (lets say 1/5 times) during boot, when I have a cable already plugged in, it looks like there's some race condition. The dwc2 driver reports some error logs in a loop, indefinitely, up to the watchdog resets the platform :-(. [ 16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in Host mode [ 16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in Host mode [ 16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently in Host mode It probably just points some already existing race condition here. Maybe it isn't even linked to this patch. But I have no evidence at this stage. I hope I can investigate further on this one, hopefully I can free up some time for that. Best Regards, Fabrice > > Yongqin, If you didn't have the context, this affected hikey960. > > Greg, > > Let's wait for some tests before we land these. > > Thanks, > Saravana > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > Cc: Sumit Semwal <sumit.semwal@linaro.org> > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > Saravana Kannan (4): > usb: typec: stusb160x: Remove use of > fw_devlink_purge_absent_suppliers() > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > driver core: Delete fw_devlink_purge_absent_suppliers() > > drivers/base/core.c | 16 ---------------- > drivers/usb/typec/stusb160x.c | 9 --------- > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > drivers/usb/typec/tipd/core.c | 9 --------- > include/linux/fwnode.h | 1 - > 5 files changed, 44 deletions(-) >
On Fri, Mar 10, 2023 at 9:21 AM Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote: > > On 3/1/23 22:49, Saravana Kannan wrote: > > Yongqin, Martin, Amelie, > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > fw_devlink is smarter and doesn't depend on compatible property. So, I > > don't think these calls are needed anymore. But I don't have these > > devices to test on and be sure and the hardware I use to test changes > > doesn't have this issue either. > > > > Can you please test these changes on the hardware where you hit the > > issue to make sure things work as expected? > > > Hi Saravana, > > Sorry for the late reply, Thanks for testing! > On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top > of commit fb42378dcc7f, and also with your series applied. > For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi > > I noticed some error messages on this board, since the 12 patch series, > around the I2C PMIC device links: > > [ 3.585514] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.590115] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.596278] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.602188] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.608165] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.614278] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.620256] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.626253] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.632252] i2c 1-0033: Failed to create device link with 1-0033 > [ 3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10 > [ 3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed > dependency cycle(s) with /soc/i2c@5c00200 > 0/stpmic@33/regulators/boost > [ 3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed > dependency cycle(s) with /soc/i2c@5c00200 > 0/stpmic@33/regulators/buck2 > [ 3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed > dependency cycle(s) with /soc/i2c@5c00200 > 0/stpmic@33/regulators/buck4 > [ 3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 > [ 3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: > Failed to create device link with 1-0033 You can ignore all the "Failed to create device link" errors. They are just error logs for stuff that was being ignored silently before. So that's no functional regression AFAIK. I'll fix them separately if necessary. And I'm sure you'll see these messages even without my fw_devlink refactor series. > Strangely some of the regulators seems to have "Fixed dependency", but > not all. Yeah, that's fine too -- that's just fw_devlink being verbose about not enforcing probe ordering between devices in that cycle because it can't tell which one of the dependencies is not a probe requirement. Maybe I'll make it a dbg log if it's confusing people. > Regarding the typec stusb160x I noticed the message below. It seems > correct, right ? > > [ 15.962771] typec port0: Fixed dependency cycle(s) with > /soc/usb-otg@49000000/port/endpoint I don't know if there is a cyclic dependency in your DT or not. But this message itself is not an issue. > But sometimes (lets say 1/5 times) during boot, when I have a cable > already plugged in, it looks like there's some race condition. The dwc2 > driver reports some error logs in a loop, indefinitely, up to the > watchdog resets the platform :-(. Can you try this series (the one you are testing) without my fw_devlink refactor that ends with commit fb42378dcc7f? Trying to make sure we can reproduce the issue Amelie was fixing before I claim my refactor series fixes it. > [ 16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently > in Host mode > [ 16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently > in Host mode > [ 16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently > in Host mode > > It probably just points some already existing race condition here. Maybe > it isn't even linked to this patch. But I have no evidence at this > stage. I hope I can investigate further on this one, hopefully I can > free up some time for that. If you never pick up this series, are you not having any of these 1/5 times boot issues? I wouldn't expect my changes to add any races, but I'll wait to see what you find here. Thanks, Saravana > > Best Regards, > Fabrice > > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > Greg, > > > > Let's wait for some tests before we land these. > > > > Thanks, > > Saravana > > > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > > Cc: Sumit Semwal <sumit.semwal@linaro.org> > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > > > Saravana Kannan (4): > > usb: typec: stusb160x: Remove use of > > fw_devlink_purge_absent_suppliers() > > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > > driver core: Delete fw_devlink_purge_absent_suppliers() > > > > drivers/base/core.c | 16 ---------------- > > drivers/usb/typec/stusb160x.c | 9 --------- > > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > > drivers/usb/typec/tipd/core.c | 9 --------- > > include/linux/fwnode.h | 1 - > > 5 files changed, 44 deletions(-) > > > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >
On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger <martin.kepplinger@puri.sm> wrote: > > Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana Kannan: > > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger > > <martin.kepplinger@puri.sm> wrote: > > > > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin > > > Kepplinger: > > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana > > > > Kannan: > > > > > Yongqin, Martin, Amelie, > > > > > > > > > > We recent refactor of fw_devlink that ends with commit > > > > > fb42378dcc7f > > > > > ("mtd: mtdpart: Don't create platform device that'll never > > > > > probe"), > > > > > fw_devlink is smarter and doesn't depend on compatible > > > > > property. > > > > > So, > > > > > I > > > > > don't think these calls are needed anymore. But I don't have > > > > > these > > > > > devices to test on and be sure and the hardware I use to test > > > > > changes > > > > > doesn't have this issue either. > > > > > > > > > > Can you please test these changes on the hardware where you hit > > > > > the > > > > > issue to make sure things work as expected? > > > > > > > > > > Yongqin, If you didn't have the context, this affected > > > > > hikey960. > > > > > > > > > > Greg, > > > > > > > > > > Let's wait for some tests before we land these. > > > > > > > > > > Thanks, > > > > > Saravana > > > > > > > > hi Sravana, > > > > > > > > I picked the 12 commits leading up to commit fb42378dcc7f ("mtd: > > > > mtdpart: Don't create platform device that'll never probe") ( > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > > > ) and included the tipd patch below to test it. > > > > > > > > With that, I get the following errors: > > > > > > > > [ 0.237931] imx-uart 30890000.serial: Failed to create device > > > > link > > > > with regulator-gnss > > > > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > > > link > > > > with regulator-lcd-1v8 > > > > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create device > > > > link > > > > with backlight-dsi > > > > > > > > but they are independent of this final tipd patch below. I'll > > > > test a > > > > real linux-next tree soon, for completeness, maybe I missed > > > > something? > > > > > > > > Anyways, on that tree, your tipd removal patch breaks type-c > > > > still > > > > for > > > > me, imx8mq-librem5.dtsi > > > > > > > > just to give a first reply quickly... thanks, > > > > > > > > martin > > > > > > > > > > just confirming: it's the same as above on next-20230302 + this > > > patch ( > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 > > > ) with the errors already independent from the patch. I should have > > > tested earlier patches -.- > > > > Thanks a lot for testing Martin! > > > > Your email is a little ambiguous to me. With the 12 refactor commits > > + > > the 4 patches in this series, things are breaking for you. But if you > > drop the 4 patches in this series, things work again. Is that right? > > no. Sorry if I wasn't clear. I can't justify to block these 4 patches. > they *themselves* don't break anything. > > Something broke *earlier* than these 4 patches in one of the other 12. If you find out it's one of the other 12 patches in the refactor that broke things for you, can you please reply to the right email in that series[1] and let me know which patch broke things for you and provide the debug details there? I don't want to mix issues with unrelated threads -- I want them to be easy to find in the future. [1] - https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/ For all my questions below, you don't need to reply here. Just reply to the right thread. > As to *what* broke: The error messages during boot, The error messages, is it anything other than the "Failed to create device link"? > and the charger and > fuel gauge don't work anymore, I see: > > # cat /sys/kernel/debug/devices_deferred > 3-0036 max17042: failed: power supply register > 0-003f This seems to be the main culprit. That 0-003f isn't probing. It's surprising that the reason is empty too. IIRC that means it didn't do deferred probing, but rather failed with an error. Wild guess .... Try increasing the deferred_probe_timeout to see if it helps? > 38100000.usb platform: wait for supplier endpoint > 32c00000.hdmi platform: supplier 0-003f not ready > 3-006a bq25890-charger: registering power supply > > > I haven't had time to track it down and see where the issue is. Could > be one of the 12 patches, could be a wrong description for my board: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi > or something else (an i2c issue?). > > (for completeness, the exact tree I ran: > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > ) > > > > > Let's ignore the "Failed to create device link" errors for now -- > > it's > > not related to this usb-c-connector series. It's basically pointing > > out issues that we ignored silently in the past -- it's basically > > pointing out holes in fw_devlink's visibility of devices. I'll get to > > them later. > > ok. good to know. please do. Here's a fix for the Failed to create device link issues. https://lore.kernel.org/lkml/20230310063910.2474472-1-saravanak@google.com/ > And since I'm the only one seeing an issue, please move the patches > forward. I'll need to fix this after the fact and hope to have time for > it next week. I'd rather wait for you than rush these patches in. The patches are just removing what I expect to be cruft. I'd rather confirm that first than break anything. -Saravana
Am Freitag, dem 10.03.2023 um 14:18 -0800 schrieb Saravana Kannan: > On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger > <martin.kepplinger@puri.sm> wrote: > > > > Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana > > Kannan: > > > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger > > > <martin.kepplinger@puri.sm> wrote: > > > > > > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin > > > > Kepplinger: > > > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana > > > > > Kannan: > > > > > > Yongqin, Martin, Amelie, > > > > > > > > > > > > We recent refactor of fw_devlink that ends with commit > > > > > > fb42378dcc7f > > > > > > ("mtd: mtdpart: Don't create platform device that'll never > > > > > > probe"), > > > > > > fw_devlink is smarter and doesn't depend on compatible > > > > > > property. > > > > > > So, > > > > > > I > > > > > > don't think these calls are needed anymore. But I don't > > > > > > have > > > > > > these > > > > > > devices to test on and be sure and the hardware I use to > > > > > > test > > > > > > changes > > > > > > doesn't have this issue either. > > > > > > > > > > > > Can you please test these changes on the hardware where you > > > > > > hit > > > > > > the > > > > > > issue to make sure things work as expected? > > > > > > > > > > > > Yongqin, If you didn't have the context, this affected > > > > > > hikey960. > > > > > > > > > > > > Greg, > > > > > > > > > > > > Let's wait for some tests before we land these. > > > > > > > > > > > > Thanks, > > > > > > Saravana > > > > > > > > > > hi Sravana, > > > > > > > > > > I picked the 12 commits leading up to commit fb42378dcc7f > > > > > ("mtd: > > > > > mtdpart: Don't create platform device that'll never probe") ( > > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > > > > ) and included the tipd patch below to test it. > > > > > > > > > > With that, I get the following errors: > > > > > > > > > > [ 0.237931] imx-uart 30890000.serial: Failed to create > > > > > device > > > > > link > > > > > with regulator-gnss > > > > > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create > > > > > device > > > > > link > > > > > with regulator-lcd-1v8 > > > > > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create > > > > > device > > > > > link > > > > > with backlight-dsi > > > > > > > > > > but they are independent of this final tipd patch below. I'll > > > > > test a > > > > > real linux-next tree soon, for completeness, maybe I missed > > > > > something? > > > > > > > > > > Anyways, on that tree, your tipd removal patch breaks type-c > > > > > still > > > > > for > > > > > me, imx8mq-librem5.dtsi > > > > > > > > > > just to give a first reply quickly... thanks, > > > > > > > > > > martin > > > > > > > > > > > > > just confirming: it's the same as above on next-20230302 + this > > > > patch ( > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 > > > > ) with the errors already independent from the patch. I should > > > > have > > > > tested earlier patches -.- > > > > > > Thanks a lot for testing Martin! > > > > > > Your email is a little ambiguous to me. With the 12 refactor > > > commits > > > + > > > the 4 patches in this series, things are breaking for you. But if > > > you > > > drop the 4 patches in this series, things work again. Is that > > > right? > > > > no. Sorry if I wasn't clear. I can't justify to block these 4 > > patches. > > they *themselves* don't break anything. > > > > Something broke *earlier* than these 4 patches in one of the other > > 12. > > If you find out it's one of the other 12 patches in the refactor that > broke things for you, can you please reply to the right email in that > series[1] and let me know which patch broke things for you and > provide > the debug details there? I don't want to mix issues with unrelated > threads -- I want them to be easy to find in the future. > > [1] - > https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/ > > For all my questions below, you don't need to reply here. Just reply > to the right thread. Thanks. I'll have to reply here though - I'm puzzled how, but I got it wrong - I must have seen the "Failed to create device link" messages without checking broken drivers: The 12 patches you linked above are fine. (In a way that's good as I saw them in a stable kernel already). commit ("usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers()") breaks things for me. That is patch 2 of this series. That's for sure now. > > > As to *what* broke: The error messages during boot, > > The error messages, is it anything other than the "Failed to create > device link"? > > > and the charger and > > fuel gauge don't work anymore, I see: > > > > # cat /sys/kernel/debug/devices_deferred > > 3-0036 max17042: failed: power supply register > > 0-003f > > This seems to be the main culprit. That 0-003f isn't probing. It's > surprising that the reason is empty too. IIRC that means it didn't do > deferred probing, but rather failed with an error. Wild guess .... > Try > increasing the deferred_probe_timeout to see if it helps? > > > 38100000.usb platform: wait for supplier endpoint > > 32c00000.hdmi platform: supplier 0-003f not ready > > 3-006a bq25890-charger: registering power supply > > > > > > I haven't had time to track it down and see where the issue is. > > Could > > be one of the 12 patches, could be a wrong description for my > > board: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi > > or something else (an i2c issue?). > > > > (for completeness, the exact tree I ran: > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > ) > > > > > > > > Let's ignore the "Failed to create device link" errors for now -- > > > it's > > > not related to this usb-c-connector series. It's basically > > > pointing > > > out issues that we ignored silently in the past -- it's basically > > > pointing out holes in fw_devlink's visibility of devices. I'll > > > get to > > > them later. > > > > ok. good to know. please do. > > Here's a fix for the Failed to create device link issues. > https://lore.kernel.org/lkml/20230310063910.2474472-1-saravanak@google.com/ > > > And since I'm the only one seeing an issue, please move the patches > > forward. I'll need to fix this after the fact and hope to have time > > for > > it next week. > > I'd rather wait for you than rush these patches in. The patches are > just removing what I expect to be cruft. I'd rather confirm that > first > than break anything. > > -Saravana
Am Sonntag, dem 12.03.2023 um 15:41 +0100 schrieb Martin Kepplinger: > Am Freitag, dem 10.03.2023 um 14:18 -0800 schrieb Saravana Kannan: > > On Fri, Mar 10, 2023 at 2:07 AM Martin Kepplinger > > <martin.kepplinger@puri.sm> wrote: > > > > > > Am Donnerstag, dem 09.03.2023 um 16:24 -0800 schrieb Saravana > > > Kannan: > > > > On Thu, Mar 2, 2023 at 1:41 AM Martin Kepplinger > > > > <martin.kepplinger@puri.sm> wrote: > > > > > > > > > > Am Donnerstag, dem 02.03.2023 um 10:12 +0100 schrieb Martin > > > > > Kepplinger: > > > > > > Am Mittwoch, dem 01.03.2023 um 13:49 -0800 schrieb Saravana > > > > > > Kannan: > > > > > > > Yongqin, Martin, Amelie, > > > > > > > > > > > > > > We recent refactor of fw_devlink that ends with commit > > > > > > > fb42378dcc7f > > > > > > > ("mtd: mtdpart: Don't create platform device that'll > > > > > > > never > > > > > > > probe"), > > > > > > > fw_devlink is smarter and doesn't depend on compatible > > > > > > > property. > > > > > > > So, > > > > > > > I > > > > > > > don't think these calls are needed anymore. But I don't > > > > > > > have > > > > > > > these > > > > > > > devices to test on and be sure and the hardware I use to > > > > > > > test > > > > > > > changes > > > > > > > doesn't have this issue either. > > > > > > > > > > > > > > Can you please test these changes on the hardware where > > > > > > > you > > > > > > > hit > > > > > > > the > > > > > > > issue to make sure things work as expected? > > > > > > > > > > > > > > Yongqin, If you didn't have the context, this affected > > > > > > > hikey960. > > > > > > > > > > > > > > Greg, > > > > > > > > > > > > > > Let's wait for some tests before we land these. > > > > > > > > > > > > > > Thanks, > > > > > > > Saravana > > > > > > > > > > > > hi Sravana, > > > > > > > > > > > > I picked the 12 commits leading up to commit fb42378dcc7f > > > > > > ("mtd: > > > > > > mtdpart: Don't create platform device that'll never probe") > > > > > > ( > > > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink > > > > > > ) and included the tipd patch below to test it. > > > > > > > > > > > > With that, I get the following errors: > > > > > > > > > > > > [ 0.237931] imx-uart 30890000.serial: Failed to create > > > > > > device > > > > > > link > > > > > > with regulator-gnss > > > > > > [ 0.334054] nwl-dsi 30a00000.mipi-dsi: Failed to create > > > > > > device > > > > > > link > > > > > > with regulator-lcd-1v8 > > > > > > [ 0.346964] nwl-dsi 30a00000.mipi-dsi: Failed to create > > > > > > device > > > > > > link > > > > > > with backlight-dsi > > > > > > > > > > > > but they are independent of this final tipd patch below. > > > > > > I'll > > > > > > test a > > > > > > real linux-next tree soon, for completeness, maybe I missed > > > > > > something? > > > > > > > > > > > > Anyways, on that tree, your tipd removal patch breaks type- > > > > > > c > > > > > > still > > > > > > for > > > > > > me, imx8mq-librem5.dtsi > > > > > > > > > > > > just to give a first reply quickly... thanks, > > > > > > > > > > > > martin > > > > > > > > > > > > > > > > just confirming: it's the same as above on next-20230302 + > > > > > this > > > > > patch ( > > > > > https://source.puri.sm/martin.kepplinger/linux-next/-/commits/test_fw_devlink_next-20230302 > > > > > ) with the errors already independent from the patch. I > > > > > should > > > > > have > > > > > tested earlier patches -.- > > > > > > > > Thanks a lot for testing Martin! > > > > > > > > Your email is a little ambiguous to me. With the 12 refactor > > > > commits > > > > + > > > > the 4 patches in this series, things are breaking for you. But > > > > if > > > > you > > > > drop the 4 patches in this series, things work again. Is that > > > > right? > > > > > > no. Sorry if I wasn't clear. I can't justify to block these 4 > > > patches. > > > they *themselves* don't break anything. > > > > > > Something broke *earlier* than these 4 patches in one of the > > > other > > > 12. > > > > If you find out it's one of the other 12 patches in the refactor > > that > > broke things for you, can you please reply to the right email in > > that > > series[1] and let me know which patch broke things for you and > > provide > > the debug details there? I don't want to mix issues with unrelated > > threads -- I want them to be easy to find in the future. > > > > [1] - > > https://lore.kernel.org/lkml/20230207014207.1678715-1-saravanak@google.com/ > > > > For all my questions below, you don't need to reply here. Just > > reply > > to the right thread. > > Thanks. I'll have to reply here though - I'm puzzled how, but I got > it > wrong - I must have seen the "Failed to create device link" messages > without checking broken drivers: The 12 patches you linked above are > fine. (In a way that's good as I saw them in a stable kernel > already). > > commit ("usb: typec: tipd: Remove use of > fw_devlink_purge_absent_suppliers()") breaks things for me. That is > patch 2 of this series. That's for sure now. this thing is this: I have downstream patches against tipd, among others. And I don't know yet but we might very well do something wrong in our downstream tree that is now not compatible anymore with this removal... I switched to a tree without any downstream stuff: For the 12 earlier patches, I there see the follwing which is NOT your fault and we need to fix upstream (we fix that in our downstream tree): root@pureos:/home/purism# cat /sys/kernel/debug/devices_deferred 3-0036 And then I add patch 2 of this series (removing the call from tipd), it becomes: root@pureos:/home/purism# cat /sys/kernel/debug/devices_deferred 0-003f 38100000.usb platform: wait for supplier endpoint 3-0036
Hi, Saravana On Fri, 10 Mar 2023 at 08:17, Saravana Kannan <saravanak@google.com> wrote: > > On Thu, Mar 9, 2023 at 10:53 AM Yongqin Liu <yongqin.liu@linaro.org> wrote: > > > > Hi, Saravana > > > > Sorry for the lateness, I was just aware of this today. > > No worries. > > > I tested with the ACK android-mainline branch + the 12 commits ending > > with fb42378dcc7f > > + the 4 commits of this series + hikey960 AOSP Master userspace. > > The hikey960 Android build could boot to the home screen, no stuck there, > > Thanks for testing! Can you confirm what happens if you drop the "12 > commits ending with fb42378dcc7f" ? Does it get stuck at boot or have > some limited functionality? I tried to only apply the 4 commits of this series, but that would cause build error as the commit here: https://lore.kernel.org/all/20210205222644.2357303-2-saravanak@google.com/ so I need to apply the first commit of the "12 commits ending with fb42378dcc7f" here as well: https://lore.kernel.org/all/20230207014207.1678715-2-saravanak@google.com/ With the 5 commits applied on the android-mainline branch, the build could boot to the home screen, but the adb connection could not be created. For details please check here https://termbin.com/wf9hj. Thanks, Yongqin Liu > > It's surprising that for the same type of DT node, in your case > fw_devlink is able to handle it > correctly, but no so for Martin's case. > > -Saravana > > > > > Here is the link of the logat in case you want to check some message here: > > https://gist.github.com/liuyq/6525af08c547cd2e494af5d1c8b181b5 > > > > Thanks, > > Yongqin Liu > > On Fri, 10 Mar 2023 at 02:05, Saravana Kannan <saravanak@google.com> wrote: > > > > > > Greg, > > > > > > Don't pull in this series please. It needs more testing from the folks > > > I cc'ed and it's already breaking things for Martin. This needs more > > > revisions. > > > > > > -Saravana > > > > > > On Wed, Mar 1, 2023 at 1:49 PM Saravana Kannan <saravanak@google.com> wrote: > > > > > > > > Yongqin, Martin, Amelie, > > > > > > > > We recent refactor of fw_devlink that ends with commit fb42378dcc7f > > > > ("mtd: mtdpart: Don't create platform device that'll never probe"), > > > > fw_devlink is smarter and doesn't depend on compatible property. So, I > > > > don't think these calls are needed anymore. But I don't have these > > > > devices to test on and be sure and the hardware I use to test changes > > > > doesn't have this issue either. > > > > > > > > Can you please test these changes on the hardware where you hit the > > > > issue to make sure things work as expected? > > > > > > > > Yongqin, If you didn't have the context, this affected hikey960. > > > > > > > > Greg, > > > > > > > > Let's wait for some tests before we land these. > > > > > > > > Thanks, > > > > Saravana > > > > > > > > Cc: Yongqin Liu <yongqin.liu@linaro.org> > > > > Cc: Sumit Semwal <sumit.semwal@linaro.org> > > > > Cc: Martin Kepplinger <martin.kepplinger@puri.sm> > > > > Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> > > > > > > > > Saravana Kannan (4): > > > > usb: typec: stusb160x: Remove use of > > > > fw_devlink_purge_absent_suppliers() > > > > usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() > > > > usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() > > > > driver core: Delete fw_devlink_purge_absent_suppliers() > > > > > > > > drivers/base/core.c | 16 ---------------- > > > > drivers/usb/typec/stusb160x.c | 9 --------- > > > > drivers/usb/typec/tcpm/tcpm.c | 9 --------- > > > > drivers/usb/typec/tipd/core.c | 9 --------- > > > > include/linux/fwnode.h | 1 - > > > > 5 files changed, 44 deletions(-) > > > > > > > > -- > > > > 2.39.2.722.g9855ee24e9-goog > > > > > > > > > > > > -- > > Best Regards, > > Yongqin Liu > > --------------------------------------------------------------- > > #mailing list > > linaro-android@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-android -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
On 3/10/23 18:40, Saravana Kannan wrote: > On Fri, Mar 10, 2023 at 9:21 AM Fabrice Gasnier > <fabrice.gasnier@foss.st.com> wrote: >> >> On 3/1/23 22:49, Saravana Kannan wrote: >>> Yongqin, Martin, Amelie, >>> >>> We recent refactor of fw_devlink that ends with commit fb42378dcc7f >>> ("mtd: mtdpart: Don't create platform device that'll never probe"), >>> fw_devlink is smarter and doesn't depend on compatible property. So, I >>> don't think these calls are needed anymore. But I don't have these >>> devices to test on and be sure and the hardware I use to test changes >>> doesn't have this issue either. >>> >>> Can you please test these changes on the hardware where you hit the >>> issue to make sure things work as expected? >> >> >> Hi Saravana, >> >> Sorry for the late reply, > > Thanks for testing! > >> On behalf of Amelie, I did some testing on STM32MP15 DK2 board, on top >> of commit fb42378dcc7f, and also with your series applied. >> For reference, it's based on: arch/arm/boot/dts/stm32mp15xx-dkx.dtsi >> >> I noticed some error messages on this board, since the 12 patch series, >> around the I2C PMIC device links: >> >> [ 3.585514] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.590115] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.596278] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.602188] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.608165] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.614278] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.620256] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.626253] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.632252] i2c 1-0033: Failed to create device link with 1-0033 >> [ 3.639001] stpmic1 1-0033: PMIC Chip Version: 0x10 >> [ 3.645398] platform 5c002000.i2c:stpmic@33:regulators: Fixed >> dependency cycle(s) with /soc/i2c@5c00200 >> 0/stpmic@33/regulators/boost >> [ 3.655937] platform 5c002000.i2c:stpmic@33:regulators: Fixed >> dependency cycle(s) with /soc/i2c@5c00200 >> 0/stpmic@33/regulators/buck2 >> [ 3.667824] platform 5c002000.i2c:stpmic@33:regulators: Fixed >> dependency cycle(s) with /soc/i2c@5c00200 >> 0/stpmic@33/regulators/buck4 >> [ 3.719751] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.728099] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.737576] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.747216] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.756750] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.766382] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.775914] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 >> [ 3.785545] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: >> Failed to create device link with 1-0033 > > You can ignore all the "Failed to create device link" errors. They are > just error logs for stuff that was being ignored silently before. So > that's no functional regression AFAIK. I'll fix them separately if > necessary. And I'm sure you'll see these messages even without my > fw_devlink refactor series. Hi Saravana, Thanks for the information. I tested without the 12 patch series, just before commit 3a2dbc510c43 "driver core: fw_devlink: Don't purge child fwnode's consumer links". I don't see the messages here. But I can see these on top of fb42378dcc7f. > >> Strangely some of the regulators seems to have "Fixed dependency", but >> not all. > > Yeah, that's fine too -- that's just fw_devlink being verbose about > not enforcing probe ordering between devices in that cycle because it > can't tell which one of the dependencies is not a probe requirement. > Maybe I'll make it a dbg log if it's confusing people. > >> Regarding the typec stusb160x I noticed the message below. It seems >> correct, right ? >> >> [ 15.962771] typec port0: Fixed dependency cycle(s) with >> /soc/usb-otg@49000000/port/endpoint > > I don't know if there is a cyclic dependency in your DT or not. But > this message itself is not an issue. Ack, > >> But sometimes (lets say 1/5 times) during boot, when I have a cable >> already plugged in, it looks like there's some race condition. The dwc2 >> driver reports some error logs in a loop, indefinitely, up to the >> watchdog resets the platform :-(. > > Can you try this series (the one you are testing) without my > fw_devlink refactor that ends with commit fb42378dcc7f? Trying to make > sure we can reproduce the issue Amelie was fixing before I claim my > refactor series fixes it. Strangely, I tested without the series, and removed earlier patch from Amelie. I don't reproduce the issue she used to hit. > >> [ 16.288458] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently >> in Host mode >> [ 16.288490] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently >> in Host mode >> [ 16.310429] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently >> in Host mode >> >> It probably just points some already existing race condition here. Maybe >> it isn't even linked to this patch. But I have no evidence at this >> stage. I hope I can investigate further on this one, hopefully I can >> free up some time for that. > > If you never pick up this series, are you not having any of these 1/5 > times boot issues? I wouldn't expect my changes to add any races, but > I'll wait to see what you find here. Some good news here is, I've identified a recent change [1], that creates the issue pointed above. I just sent a separate patch [2] for this. So, it's not related to this series. (I managed to reproduce without picking it). [1] https://lore.kernel.org/r/20221206-dwc2-gadget-dual-role-v1-2-36515e1092cd@theobroma-systems.com [2] https://lore.kernel.org/lkml/20230315144433.3095859-1-fabrice.gasnier@foss.st.com/ So for stusb160x: e.g. PATCH 1, feel free to add on my: Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com> Best Regards, Fabrice > > Thanks, > Saravana > >> >> Best Regards, >> Fabrice >> >>> >>> Yongqin, If you didn't have the context, this affected hikey960. >>> >>> Greg, >>> >>> Let's wait for some tests before we land these. >>> >>> Thanks, >>> Saravana >>> >>> Cc: Yongqin Liu <yongqin.liu@linaro.org> >>> Cc: Sumit Semwal <sumit.semwal@linaro.org> >>> Cc: Martin Kepplinger <martin.kepplinger@puri.sm> >>> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> >>> >>> Saravana Kannan (4): >>> usb: typec: stusb160x: Remove use of >>> fw_devlink_purge_absent_suppliers() >>> usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() >>> usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() >>> driver core: Delete fw_devlink_purge_absent_suppliers() >>> >>> drivers/base/core.c | 16 ---------------- >>> drivers/usb/typec/stusb160x.c | 9 --------- >>> drivers/usb/typec/tcpm/tcpm.c | 9 --------- >>> drivers/usb/typec/tipd/core.c | 9 --------- >>> include/linux/fwnode.h | 1 - >>> 5 files changed, 44 deletions(-) >>> >> >> -- >> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. >>
Yongqin, Martin, Amelie, We recent refactor of fw_devlink that ends with commit fb42378dcc7f ("mtd: mtdpart: Don't create platform device that'll never probe"), fw_devlink is smarter and doesn't depend on compatible property. So, I don't think these calls are needed anymore. But I don't have these devices to test on and be sure and the hardware I use to test changes doesn't have this issue either. Can you please test these changes on the hardware where you hit the issue to make sure things work as expected? Yongqin, If you didn't have the context, this affected hikey960. Greg, Let's wait for some tests before we land these. Thanks, Saravana Cc: Yongqin Liu <yongqin.liu@linaro.org> Cc: Sumit Semwal <sumit.semwal@linaro.org> Cc: Martin Kepplinger <martin.kepplinger@puri.sm> Cc: Amelie Delaunay <amelie.delaunay@foss.st.com> Saravana Kannan (4): usb: typec: stusb160x: Remove use of fw_devlink_purge_absent_suppliers() usb: typec: tipd: Remove use of fw_devlink_purge_absent_suppliers() usb: typec: tcpm: Remove use of fw_devlink_purge_absent_suppliers() driver core: Delete fw_devlink_purge_absent_suppliers() drivers/base/core.c | 16 ---------------- drivers/usb/typec/stusb160x.c | 9 --------- drivers/usb/typec/tcpm/tcpm.c | 9 --------- drivers/usb/typec/tipd/core.c | 9 --------- include/linux/fwnode.h | 1 - 5 files changed, 44 deletions(-)