On 3/10/23 18:40, Saravana Kannan wrote: > On Fri, Mar 10, 2023 at 9:21 AM Fabrice Gasnier > <fabrice.gasnier@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx [2] https://lore.kernel.org/lkml/20230315144433.3095859-1-fabrice.gasnier@xxxxxxxxxxx/ So for stusb160x: e.g. PATCH 1, feel free to add on my: Tested-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> 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@xxxxxxxxxx> >>> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx> >>> Cc: Martin Kepplinger <martin.kepplinger@xxxxxxx> >>> Cc: Amelie Delaunay <amelie.delaunay@xxxxxxxxxxx> >>> >>> 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@xxxxxxxxxxx. >>