* Re: Endless "supply vcc not found, using dummy regulator" [not found] <20160515173900.GC21194@sesse.net> @ 2016-05-21 14:43 ` Steinar H. Gunderson 2016-05-23 13:47 ` Steinar H. Gunderson 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-21 14:43 UTC (permalink / raw) To: linux-samsung-soc On Sun, May 15, 2016 at 07:39:00PM +0200, Steinar H. Gunderson wrote: > [ 47.161428] exynos-dwc3 usb@12000000: no suspend clk specified > [ 47.162811] usb_phy_generic.49646.auto supply vcc not found, using dummy regulator > [ 47.163532] usb_phy_generic.49647.auto supply vcc not found, using dummy regulator > > They appear to go on forever, blocking boot -- until I set the log level to 0 > to suppress them (using magic SysRq over the serial console), nothing else seems > to happen on the machine. (loglevel=0 on the kernel command line also works > fine.) Hi, I still have this problem. I've noticed that somehow, there's a huge amount of USB PHYs being autodetected; probably related to this issue. sesse@soldroid:~$ ls -ld /sys/devices/platform/usb_phy* | wc -l 4288 /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-21 14:43 ` Endless "supply vcc not found, using dummy regulator" Steinar H. Gunderson @ 2016-05-23 13:47 ` Steinar H. Gunderson 2016-05-23 14:40 ` Steinar H. Gunderson 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-23 13:47 UTC (permalink / raw) To: linux-samsung-soc; +Cc: 823552, broonie On Sat, May 21, 2016 at 04:43:08PM +0200, Steinar H. Gunderson wrote: > I still have this problem. > > I've noticed that somehow, there's a huge amount of USB PHYs being autodetected; > probably related to this issue. > > sesse@soldroid:~$ ls -ld /sys/devices/platform/usb_phy* | wc -l > 4288 I've tracked this down partially. It has to do with probe deferral; for whatever reason, the vdd33 regulator isn't available when the dwc3 driver comes up, and this causes the probe to defer. For each and every such deferral, these messages are printed by the regulator subsystem as part of things attempted probed before vdd33. I don't understand entirely why it tries 2000+ times before it succeeds, but then I don't understand probe deferral very well to begin with. I also don't know if there's a way to avoid these messages for each time, but it seems this is a common problem: https://lkml.org/lkml/2016/3/16/269 In this case, it's not just an annoyance, though; they're so many that they keep the system from booting unless loglevel is turned down. Cc-ing Mark in case he has any insights (I hope I have the right email address). The reason for the huge amount of PHYs is that the driver doesn't clean them up on failure (including probe deferral), so they leak. This is easy to fix; I'm attaching a small fix below. >From 6df2ebcbaae74577d49dbbc41e28d52084a124b0 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" <sesse@google.com> Date: Mon, 23 May 2016 15:44:37 +0200 Subject: [PATCH 1/1] dwc3-exynos: Clean up phys on probe failure. Otherwise, they would leak, which is especially bad given probe deferral (one could end up with thousands of dead phys after a normal boot) Signed-off-by: Steinar H. Gunderson <sesse@google.com> --- drivers/usb/dwc3/dwc3-exynos.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index dd5cb55..4104ef5 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -202,6 +202,8 @@ err4: err3: regulator_disable(exynos->vdd33); err2: + platform_device_unregister(exynos->usb2_phy); + platform_device_unregister(exynos->usb3_phy); clk_disable_unprepare(exynos->axius_clk); clk_disable_unprepare(exynos->susp_clk); clk_disable_unprepare(exynos->clk); /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 13:47 ` Steinar H. Gunderson @ 2016-05-23 14:40 ` Steinar H. Gunderson 2016-05-23 16:24 ` Mark Brown 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-23 14:40 UTC (permalink / raw) To: linux-samsung-soc; +Cc: 823552, broonie On Mon, May 23, 2016 at 03:47:37PM +0200, Steinar H. Gunderson wrote: > I don't understand entirely why it tries 2000+ times before it succeeds Now I do; the initramfs doesn't include i2c-exynos5, and before that is loaded, s2mps11 (the regulator) can't come up either. So fixing initramfs-tools to include the driver will seemingly fix (or maybe more work around) the huge amounts of spam, but this is still a larger issue that needs resolving. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 14:40 ` Steinar H. Gunderson @ 2016-05-23 16:24 ` Mark Brown 2016-05-23 17:06 ` Steinar H. Gunderson 0 siblings, 1 reply; 31+ messages in thread From: Mark Brown @ 2016-05-23 16:24 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: linux-samsung-soc, 823552 [-- Attachment #1: Type: text/plain, Size: 1987 bytes --] On Mon, May 23, 2016 at 04:40:47PM +0200, Steinar H. Gunderson wrote: > On Mon, May 23, 2016 at 03:47:37PM +0200, Steinar H. Gunderson wrote: > > In this case, it's not just an annoyance, though; they're so many that they > > keep the system from booting unless loglevel is turned down. Cc-ing Mark in > > case he has any insights (I hope I have the right email address). But nobody who works on probe deferral or made any of the suggestions I mentioned in the message you linked to, nor anyone who works on the driver you've identified a bug in... :( > > I don't understand entirely why it tries 2000+ times before it succeeds > Now I do; the initramfs doesn't include i2c-exynos5, and before that is > loaded, s2mps11 (the regulator) can't come up either. > So fixing initramfs-tools to include the driver will seemingly fix (or maybe > more work around) the huge amounts of spam, but this is still a larger issue > that needs resolving. Not really, the issue you're seeing is just a plain resource leak in the driver that happens to blow up worse than normal in your particular configuration. This isn't even something related to probe deferral at the regulator level, the core is complaining that your system description is buggy as it has omitted some of the supplies for the device (notice how it says "using dummy regulator"...). This is happening a lot as the DWC3 driver is leaking, it is happening at all because when the Exynos DWC3 integration creates it PHYs it doesn't map the supplies through to them (it should be registering a supply alias to do this). The patch you linked to was for a completely different error message which is at least related to probe deferral, though fundamentally unless we just stop printing diagnostics (which is getting more and more tempting to be honest) I'm not sure anything is going to fully resolve leaks like this, the best chance you've got is something that explicitly looks at the dependencies like Raphael was proposing. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 16:24 ` Mark Brown @ 2016-05-23 17:06 ` Steinar H. Gunderson 2016-05-23 17:46 ` Steinar H. Gunderson ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-23 17:06 UTC (permalink / raw) To: Mark Brown; +Cc: linux-samsung-soc, 823552 On Mon, May 23, 2016 at 05:24:55PM +0100, Mark Brown wrote: >>> Cc-ing Mark in case he has any insights (I hope I have the right email >>> address). > But nobody who works on probe deferral or made any of the suggestions I > mentioned in the message you linked to, nor anyone who works on the > driver you've identified a bug in... :( As for the former, I have no idea who they would be, sorry. For the latter, isn't linux-samsung-soc@ the right place? MAINTAINERS said it was. >> So fixing initramfs-tools to include the driver will seemingly fix (or maybe >> more work around) the huge amounts of spam, but this is still a larger issue >> that needs resolving. > Not really, the issue you're seeing is just a plain resource leak in the > driver that happens to blow up worse than normal in your particular > configuration. This isn't even something related to probe deferral at > the regulator level, the core is complaining that your system > description is buggy as it has omitted some of the supplies for the > device (notice how it says "using dummy regulator"...). This is > happening a lot as the DWC3 driver is leaking, it is happening at all > because when the Exynos DWC3 integration creates it PHYs it doesn't map > the supplies through to them (it should be registering a supply alias to > do this). So there are multiple issues in play here. First of all, the leak is bad, but it doesn't affect the issue with the system not booting. If I set loglevel=0, it boots with or without the leak patch; however, it still sends a gazillion error lines to dmesg (with or without the deferral). Second, there's the issue that the driver takes so long to load in the first place. This is because the regulator isn't up and doesn't come up before after initramfs is done. This is a bug in Debian's initramfs-tools, but hopefully easily remedied. Then, there's the issue of why the messages come for each deferred probe attempt. It seems from your message this is about something in the declaration of the device tree; I don't understand the nuances here, but I suppose it's pretty easy? Fourth, there's the question of why there are thousands of probe attempts; it shouldn't be even if the regulator takes a long time to come up. I guess this is what your original message was about, and fixing that would also reduce the amount of logging a ton (plus presumably speed up boot by wasting less CPU on repeated probing). But as I understand you, it's not strictly necessary to actually fix this issue? Fifth and finally, there's the question of whether we can suppress diagnostics on a probe that ends up being deferred. It would also solve the problem here, although perhaps less elegantly than fixing issues #3 or #4 (or to a lesser degree, #2), either of which will make my system boot. :-) > The patch you linked to was for a completely different error message > which is at least related to probe deferral Yes, it's a different error message, but points to the same issue as #4 above, no? > though fundamentally unless > we just stop printing diagnostics (which is getting more and more > tempting to be honest) I'm not sure anything is going to fully resolve > leaks like this, the best chance you've got is something that explicitly > looks at the dependencies like Raphael was proposing. What do you mean by “leaks” here? /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 17:06 ` Steinar H. Gunderson @ 2016-05-23 17:46 ` Steinar H. Gunderson 2016-05-23 18:56 ` Mark Brown 2016-05-23 18:44 ` Mark Brown 2016-05-24 15:06 ` Krzysztof Kozlowski 2 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-23 17:46 UTC (permalink / raw) To: Mark Brown; +Cc: linux-samsung-soc, 823552 On Mon, May 23, 2016 at 07:06:17PM +0200, Steinar H. Gunderson wrote: > Then, there's the issue of why the messages come for each deferred probe > attempt. It seems from your message this is about something in the > declaration of the device tree; I don't understand the nuances here, but I > suppose it's pretty easy? FWIW, from reading drivers/usb/phy/phy-generic.c, it looks like vcc-supply on the USB phy is supposed to be optional. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 17:46 ` Steinar H. Gunderson @ 2016-05-23 18:56 ` Mark Brown 0 siblings, 0 replies; 31+ messages in thread From: Mark Brown @ 2016-05-23 18:56 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: linux-samsung-soc, 823552 [-- Attachment #1: Type: text/plain, Size: 1092 bytes --] On Mon, May 23, 2016 at 07:46:43PM +0200, Steinar H. Gunderson wrote: > On Mon, May 23, 2016 at 07:06:17PM +0200, Steinar H. Gunderson wrote: > > Then, there's the issue of why the messages come for each deferred probe > > attempt. It seems from your message this is about something in the > > declaration of the device tree; I don't understand the nuances here, but I > > suppose it's pretty easy? > FWIW, from reading drivers/usb/phy/phy-generic.c, it looks like vcc-supply on > the USB phy is supposed to be optional. No, not unless the device can operate without power which seems improbable. Optional supplies are for supplies which may be physically absent, not to shut up warnings from partially specified system descriptions. If there is an optional supply with no configuration of the device to operate in a different mode without that supply it is most likely that the API is being abused. The API is there to support users with things like optional external reference voltages that may be missing in some system designs, it's not there to support broken system integrations. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 17:06 ` Steinar H. Gunderson 2016-05-23 17:46 ` Steinar H. Gunderson @ 2016-05-23 18:44 ` Mark Brown 2016-05-24 15:06 ` Krzysztof Kozlowski 2 siblings, 0 replies; 31+ messages in thread From: Mark Brown @ 2016-05-23 18:44 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: linux-samsung-soc, 823552 [-- Attachment #1: Type: text/plain, Size: 5316 bytes --] On Mon, May 23, 2016 at 07:06:17PM +0200, Steinar H. Gunderson wrote: > On Mon, May 23, 2016 at 05:24:55PM +0100, Mark Brown wrote: > >>> Cc-ing Mark in case he has any insights (I hope I have the right email > >>> address). > > But nobody who works on probe deferral or made any of the suggestions I > > mentioned in the message you linked to, nor anyone who works on the > > driver you've identified a bug in... :( > As for the former, I have no idea who they would be, sorry. For the latter, There's the people I mentioned in the e-mail you linked to for exmaple... you could also look at git annotate for the probe deferral code and see who worked on it, or do whatever it is lead to you finding me. > isn't linux-samsung-soc@ the right place? MAINTAINERS said it was. That's just the list, not people. You really want to see who's working on the driver and talk to them, you can't guarantee that everyone reads the lists sufficiently well that they will notice some random post that doesn't even mention the driver in the subject line (it's probably a good idea to submit that patch BTW). Note also that if MAINTAINERS has multiple hits you want to pay attention to them. > Then, there's the issue of why the messages come for each deferred probe > attempt. It seems from your message this is about something in the > declaration of the device tree; I don't understand the nuances here, but I > suppose it's pretty easy? No. This is nothing to do with the device tree. The dwc3-exynos driver is continually creating new, partially specified devices. Since each of these new devices has the same problem they all get the same warning reported for them. The regulator API has no idea that this is anything to do with deferred probe, it's printing a warning message because something asked for a supply that not mapped at all which is a potential concern if we explode later due to this being an error in the code. All the regulator API can realistically do is remove all diagnostics in case someone triggers them with deferred probe but that's probably not enormously helpful to people if they run into an error directly triggered at the regulator level who might like some diagnostics to help them figure out what went wrong. > Fourth, there's the question of why there are thousands of probe attempts; > it shouldn't be even if the regulator takes a long time to come up. I guess > this is what your original message was about, and fixing that would also > reduce the amount of logging a ton (plus presumably speed up boot by wasting > less CPU on repeated probing). But as I understand you, it's not strictly > necessary to actually fix this issue? No, this is the way probe deferral is supposed to work. It is a bit wasteful of CPU but realistically it's not that much and it's really simple to implement robustly unlike anything that involves actually looking at dependencies. Some breakage in your system is triggering vastly more retries than are normal, even a hundred rounds would be *extremely* high for the initial boot. We're doing repeated probes because the way deferred probe works is that every time a new device binds we start a new retry of all deferred devices to see if that new device unblocked anything (if multiple devices progress in one run it should only schedule one new run). The reason you're seeing the spectacular volume is that every time we retry we add more devices (notice that you're not complaining about the log message generated by the underlying Designware controller failing to get the supply it requests which will print once per probe but rather by the PHY devices it spams in). The fact that the Exynos driver is instantiating subdevices before it's even acquired its own resources is probably not helping here of course, it's more than likely that at least some of those resources need to be passed on to the child devices and of course if the children did actually instantiate that'd trigger continual probe deferral runs. Looking at the code I've got a horrible feeling that might be the root cause here, it's a single regulator that's being requested so the diagnostics should be being printed in the caller but that just silently passes back failures to get supplies (which is why it wasn't immediately obvious that that was failing). > > The patch you linked to was for a completely different error message > > which is at least related to probe deferral > Yes, it's a different error message, but points to the same issue as #4 > above, no? Not from the point of the view of the regulator API and really as far as I can tell this is just a driver specific issue. The regulator API has no idea this is anything to do with deferred probe. > > though fundamentally unless > > we just stop printing diagnostics (which is getting more and more > > tempting to be honest) I'm not sure anything is going to fully resolve > > leaks like this, the best chance you've got is something that explicitly > > looks at the dependencies like Raphael was proposing. > What do you mean by “leaks” here? Resources that are not cleaned up are said to be leaks, as you identifed the dwc3-exynos code isn't cleaning up the child devices it creates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-23 17:06 ` Steinar H. Gunderson 2016-05-23 17:46 ` Steinar H. Gunderson 2016-05-23 18:44 ` Mark Brown @ 2016-05-24 15:06 ` Krzysztof Kozlowski 2016-05-24 15:26 ` Steinar H. Gunderson 2016-05-24 15:39 ` Mark Brown 2 siblings, 2 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2016-05-24 15:06 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: Mark Brown, linux-samsung-soc, 823552 On Mon, May 23, 2016 at 7:06 PM, Steinar H. Gunderson <sgunderson@bigfoot.com> wrote: > On Mon, May 23, 2016 at 05:24:55PM +0100, Mark Brown wrote: >>>> Cc-ing Mark in case he has any insights (I hope I have the right email >>>> address). >> But nobody who works on probe deferral or made any of the suggestions I >> mentioned in the message you linked to, nor anyone who works on the >> driver you've identified a bug in... :( > > As for the former, I have no idea who they would be, sorry. For the latter, > isn't linux-samsung-soc@ the right place? MAINTAINERS said it was. I looked quickly through the thread and I am not sure what is exactly your problem. I understood that the Exynos dwc3 driver is leaking the PHY. That is a good catch but your patch is not sufficient. You should clean up starting from devm_clk_get() error. Unless you are fixing this for different kernel version. Please send an appropriate separate patch for fixing this. Your email did not reach people, I think. Just make a patch, test it, scripts/checkpatch and scripts/get_maintainers. It is a fix so also a candidate for this release cycle. if you do not plan to send a patch, I can prepare on my own with your "Reported-by" tag but actually this is your finding so credits (and effort) should go to you. :) What other problems exactly do you have? Late binding of S2MPS11 regulator driver? That does not look like a problem. If it is built as a module then it should be loaded, probably from initramfs because these are regulators. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 15:06 ` Krzysztof Kozlowski @ 2016-05-24 15:26 ` Steinar H. Gunderson 2016-05-24 15:45 ` Mark Brown 2016-05-24 15:53 ` Krzysztof Kozlowski 2016-05-24 15:39 ` Mark Brown 1 sibling, 2 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-24 15:26 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: Mark Brown, linux-samsung-soc, 823552 On Tue, May 24, 2016 at 05:06:43PM +0200, Krzysztof Kozlowski wrote: > I looked quickly through the thread and I am not sure what is exactly > your problem. My immediate problem is that the repeated (deferred) probing is causing so much logging that the system doesn't actually boot. The root causes are a bit more complex and muddy (see my previous email for a breakdown). > I understood that the Exynos dwc3 driver is leaking the > PHY. That is a good catch but your patch is not sufficient. You should > clean up starting from devm_clk_get() error. Unless you are fixing > this for different kernel version. I have zero idea how all of this is supposed to be connected, much less how DWC3 works or what the driver is supposed to do. I'm just an end user trying to figure out why my system didn't boot. :-) Which devm_clk_get() error are you talking about? The one with susp_clk? That's an interesting case because a) nothing actually uses susp_clk (it's dead code, presumably waiting for further patches), and b) there was a commit not too long ago (42f69a02) upgrading the message from dev_dbg to dev_info for reasons I don't understand, making the problem worse. (The commit message had an argument that I could accept for changing _to_ dbg, but not the other way round.) > Please send an appropriate separate patch for fixing this. Your email > did not reach people, I think. I only sent one patch so far, namely the leak fix. > What other problems exactly do you have? Late binding of S2MPS11 > regulator driver? That does not look like a problem. If it is built as > a module then it should be loaded, probably from initramfs because > these are regulators. In this specific case, Debian's initramfs has neglected to include the i2c driver in the initramfs, so the regulator doesn't come up until the boot is out of the initramfs. That's probably a bug in its own right, and fixing it reduces the amount of messages by a _lot_, but even so, it sounds to me like the kernel should be able to boot without that fix. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 15:26 ` Steinar H. Gunderson @ 2016-05-24 15:45 ` Mark Brown 2016-05-24 15:53 ` Krzysztof Kozlowski 1 sibling, 0 replies; 31+ messages in thread From: Mark Brown @ 2016-05-24 15:45 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: Krzysztof Kozlowski, linux-samsung-soc, 823552 [-- Attachment #1: Type: text/plain, Size: 618 bytes --] On Tue, May 24, 2016 at 05:26:38PM +0200, Steinar H. Gunderson wrote: > On Tue, May 24, 2016 at 05:06:43PM +0200, Krzysztof Kozlowski wrote: > > Please send an appropriate separate patch for fixing this. Your email > > did not reach people, I think. > I only sent one patch so far, namely the leak fix. You buried it in the middle of another mail and didn't CC any of the maintainers - he's asking you to submit it using the process in SubmittingPatches, the USB maintainers won't have seen this discussion and may not notice a patch buried in the middle of a message in the middle of a thread even if they see it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 15:26 ` Steinar H. Gunderson 2016-05-24 15:45 ` Mark Brown @ 2016-05-24 15:53 ` Krzysztof Kozlowski 2016-05-24 16:44 ` Steinar H. Gunderson [not found] ` <574478FE.4070800-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 1 sibling, 2 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2016-05-24 15:53 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: Mark Brown, linux-samsung-soc, 823552 On 05/24/2016 05:26 PM, Steinar H. Gunderson wrote: > On Tue, May 24, 2016 at 05:06:43PM +0200, Krzysztof Kozlowski wrote: >> I looked quickly through the thread and I am not sure what is exactly >> your problem. > > My immediate problem is that the repeated (deferred) probing is causing so > much logging that the system doesn't actually boot. The root causes are a bit > more complex and muddy (see my previous email for a breakdown). Ah, I got it. > >> I understood that the Exynos dwc3 driver is leaking the >> PHY. That is a good catch but your patch is not sufficient. You should >> clean up starting from devm_clk_get() error. Unless you are fixing >> this for different kernel version. > > I have zero idea how all of this is supposed to be connected, much less how > DWC3 works or what the driver is supposed to do. I'm just an end user trying > to figure out why my system didn't boot. :-) > > Which devm_clk_get() error are you talking about? The one with susp_clk? Now I saw your original report on Debian bugzilla. Let's stick to v4.5. The platform_device_alloc() is done in dwc3_exynos_register_phys(). So on error paths you have clean up starting from next error: ret = dwc3_exynos_register_phys(exynos); if (ret) { dev_err(dev, "couldn't register PHYs\n"); return ret; } exynos->dev = dev; exynos->clk = devm_clk_get(dev, "usbdrd30"); if (IS_ERR(exynos->clk)) { + // On each error path since here we need to + // revert work done by dwc3_exynos_register_phys() dev_err(dev, "couldn't get clock\n"); return -EINVAL; } clk_prepare_enable(exynos->clk); > That's an interesting case because a) nothing actually uses susp_clk > (it's dead code, presumably waiting for further patches), It does not look like dead code because it is enabled. > and b) there was a > commit not too long ago (42f69a02) upgrading the message from dev_dbg to > dev_info for reasons I don't understand, making the problem worse. > (The commit message had an argument that I could accept for changing _to_ > dbg, but not the other way round. I don't get the rationale behind that change neither... >> Please send an appropriate separate patch for fixing this. Your email >> did not reach people, I think. > > I only sent one patch so far, namely the leak fix. Yeah, but you did not send it to appropriate people. get_maintainer.pl will point you (Felipe Balbi handles the patches for this driver). > >> What other problems exactly do you have? Late binding of S2MPS11 >> regulator driver? That does not look like a problem. If it is built as >> a module then it should be loaded, probably from initramfs because >> these are regulators. > > In this specific case, Debian's initramfs has neglected to include the i2c > driver in the initramfs, so the regulator doesn't come up until the boot is > out of the initramfs. That's probably a bug in its own right, and fixing it > reduces the amount of messages by a _lot_, but even so, it sounds to me like > the kernel should be able to boot without that fix. Apparently the s2mps11 regulator driver can be built as module... but is not a commonly tested configuration. In our testing configs (exynos and multi_v7) it is built-in. Actually most of PMICs are built in. Additionally, if you look at the driver, its init was moved earlier from module_init() to subsys_initcall(). This is kind of manual probe ordering, used mostly because some depending drivers did not support probe deferral. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 15:53 ` Krzysztof Kozlowski @ 2016-05-24 16:44 ` Steinar H. Gunderson [not found] ` <574478FE.4070800-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 1 sibling, 0 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-24 16:44 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: Mark Brown, linux-samsung-soc, 823552 On Tue, May 24, 2016 at 05:53:34PM +0200, Krzysztof Kozlowski wrote: >> Which devm_clk_get() error are you talking about? The one with susp_clk? > Now I saw your original report on Debian bugzilla. Let's stick to v4.5. I'm actually developing on 4.6, but sure. The differences are small from what I can see. > exynos->clk = devm_clk_get(dev, "usbdrd30"); > if (IS_ERR(exynos->clk)) { > + // On each error path since here we need to > + // revert work done by dwc3_exynos_register_phys() > dev_err(dev, "couldn't get clock\n"); > return -EINVAL; > } > clk_prepare_enable(exynos->clk); OK, sounds like these should be changed to the common goto pattern to save on the repeated cleanup. I'll have a stab at that later today. >> That's an interesting case because a) nothing actually uses susp_clk >> (it's dead code, presumably waiting for further patches), > It does not look like dead code because it is enabled. But not a single DT seems to set such a suspend clock, from what I can see? > Yeah, but you did not send it to appropriate people. get_maintainer.pl > will point you (Felipe Balbi handles the patches for this driver). Ack. > Apparently the s2mps11 regulator driver can be built as module... but is > not a commonly tested configuration. In our testing configs (exynos and > multi_v7) it is built-in. Actually most of PMICs are built in. I don't have a lot of control over how Debian chooses to build kernels -- from what I gather from previous conversations, the kernel team are unhappy about building things into the kernel if they can be modules. And in this case, it wasn't even about the s2mps11 module, it was just that it didn't have an I2C bus ready for init. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <574478FE.4070800-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* [PATCH] Re: Endless "supply vcc not found, using dummy regulator" [not found] ` <574478FE.4070800-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> @ 2016-05-24 19:24 ` Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH v2] dwc3-exynos: Fix deferred probing storm Steinar H. Gunderson ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-24 19:24 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mark Brown, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, 823552-61a8vm9lEZVf4u+23C9RwQ, balbi-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA On Tue, May 24, 2016 at 05:53:34PM +0200, Krzysztof Kozlowski wrote: > exynos->clk = devm_clk_get(dev, "usbdrd30"); > if (IS_ERR(exynos->clk)) { > + // On each error path since here we need to > + // revert work done by dwc3_exynos_register_phys() > dev_err(dev, "couldn't get clock\n"); > return -EINVAL; > } > clk_prepare_enable(exynos->clk); OK, so I took Mark's advice and moved the phy instantiation towards the end of the function (after the regulators have successfully come up). It reduced the number of probes, even with the original initramfs, dramatically, so it seems to work quite well. It also reduces the text for each deferred probe by a lot, since we no longer have the dummy regulator message for each one (only the message about “no suspend clk specified” is left). Finally, this arrangement reduced the need for extra error handling to a minimum. Cc-ing Felipe and and linux-usb@, and adding the patch as a reply to this message. /* Steinar */ -- Homepage: https://www.sesse.net/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2] dwc3-exynos: Fix deferred probing storm. 2016-05-24 19:24 ` [PATCH] " Steinar H. Gunderson @ 2016-05-24 18:13 ` Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH] " Steinar H. Gunderson 2016-05-25 12:16 ` [PATCH] Re: Endless "supply vcc not found, using dummy regulator" Anand Moon 2 siblings, 0 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-24 18:13 UTC (permalink / raw) To: k.kozlowski Cc: broonie, linux-samsung-soc, 823552, balbi, linux-usb, stable, gautam.vivek dwc3-exynos has two problems during init if the regulators are slow to come up (for instance if the I2C bus driver is not on the initramfs) and return probe deferral. First, every time this happens, the driver leaks the USB phys created; they need to be deallocated on error. Second, since the phy devices are created before the regulators fail, this means that there's a new device to re-trigger deferred probing, which causes it to essentially go into a busy loop of re-probing the device until the regulators come up. Move the phy creation to after the regulators have succeeded, and also fix cleanup on failure. On my ODROID XU4 system (with Debian's initramfs which doesn't contain the I2C driver), this reduces the number of probe attempts (for each of the two controllers) from more than 2000 to eight. Signed-off-by: Steinar H. Gunderson <sesse@google.com> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Reviewed-by: Vivek Gautam <gautam.vivek@samsung.com> Fixes: d720f057fda4 ("usb: dwc3: exynos: add nop transceiver support") Cc: <stable@vger.kernel.org> --- drivers/usb/dwc3/dwc3-exynos.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index dd5cb55..2f1fb7e 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -128,12 +128,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev) platform_set_drvdata(pdev, exynos); - ret = dwc3_exynos_register_phys(exynos); - if (ret) { - dev_err(dev, "couldn't register PHYs\n"); - return ret; - } - exynos->dev = dev; exynos->clk = devm_clk_get(dev, "usbdrd30"); @@ -183,20 +177,29 @@ static int dwc3_exynos_probe(struct platform_device *pdev) goto err3; } + ret = dwc3_exynos_register_phys(exynos); + if (ret) { + dev_err(dev, "couldn't register PHYs\n"); + goto err4; + } + if (node) { ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { dev_err(dev, "failed to add dwc3 core\n"); - goto err4; + goto err5; } } else { dev_err(dev, "no device node, failed to add dwc3 core\n"); ret = -ENODEV; - goto err4; + goto err5; } return 0; +err5: + platform_device_unregister(exynos->usb2_phy); + platform_device_unregister(exynos->usb3_phy); err4: regulator_disable(exynos->vdd10); err3: -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-24 19:24 ` [PATCH] " Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH v2] dwc3-exynos: Fix deferred probing storm Steinar H. Gunderson @ 2016-05-24 18:13 ` Steinar H. Gunderson 2016-05-25 7:54 ` Krzysztof Kozlowski 2016-05-27 9:53 ` Vivek Gautam 2016-05-25 12:16 ` [PATCH] Re: Endless "supply vcc not found, using dummy regulator" Anand Moon 2 siblings, 2 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-24 18:13 UTC (permalink / raw) To: k.kozlowski; +Cc: broonie, linux-samsung-soc, 823552, balbi, linux-usb dwc3-exynos has two problems during init if the regulators are slow to come up (for instance if the I2C bus driver is not on the initramfs) and return probe deferral. First, every time this happens, the driver leaks the USB phys created; they need to be deallocated on error. Second, since the phy devices are created before the regulators fail, this means that there's a new device to re-trigger deferred probing, which causes it to essentially go into a busy loop of re-probing the device until the regulators come up. Move the phy creation to after the regulators have succeeded, and also fix cleanup on failure. On my ODROID XU4 system (with Debian's initramfs which doesn't contain the I2C driver), this reduces the number of probe attempts (for each of the two controllers) from more than 2000 to eight. Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com> Signed-off-by: Steinar H. Gunderson <sesse@google.com> --- drivers/usb/dwc3/dwc3-exynos.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index dd5cb55..2f1fb7e 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -128,12 +128,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev) platform_set_drvdata(pdev, exynos); - ret = dwc3_exynos_register_phys(exynos); - if (ret) { - dev_err(dev, "couldn't register PHYs\n"); - return ret; - } - exynos->dev = dev; exynos->clk = devm_clk_get(dev, "usbdrd30"); @@ -183,20 +177,29 @@ static int dwc3_exynos_probe(struct platform_device *pdev) goto err3; } + ret = dwc3_exynos_register_phys(exynos); + if (ret) { + dev_err(dev, "couldn't register PHYs\n"); + goto err4; + } + if (node) { ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { dev_err(dev, "failed to add dwc3 core\n"); - goto err4; + goto err5; } } else { dev_err(dev, "no device node, failed to add dwc3 core\n"); ret = -ENODEV; - goto err4; + goto err5; } return 0; +err5: + platform_device_unregister(exynos->usb2_phy); + platform_device_unregister(exynos->usb3_phy); err4: regulator_disable(exynos->vdd10); err3: -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-24 18:13 ` [PATCH] " Steinar H. Gunderson @ 2016-05-25 7:54 ` Krzysztof Kozlowski 2016-05-27 9:53 ` Vivek Gautam 1 sibling, 0 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2016-05-25 7:54 UTC (permalink / raw) To: Steinar H. Gunderson; +Cc: broonie, linux-samsung-soc, 823552, balbi, linux-usb On 05/24/2016 08:13 PM, Steinar H. Gunderson wrote: > dwc3-exynos has two problems during init if the regulators are slow > to come up (for instance if the I2C bus driver is not on the initramfs) > and return probe deferral. First, every time this happens, the driver > leaks the USB phys created; they need to be deallocated on error. > > Second, since the phy devices are created before the regulators fail, > this means that there's a new device to re-trigger deferred probing, > which causes it to essentially go into a busy loop of re-probing the > device until the regulators come up. > > Move the phy creation to after the regulators have succeeded, and also > fix cleanup on failure. On my ODROID XU4 system (with Debian's initramfs > which doesn't contain the I2C driver), this reduces the number of probe > attempts (for each of the two controllers) from more than 2000 to eight. > > Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com> > Signed-off-by: Steinar H. Gunderson <sesse@google.com> This is the same person so no need for "Reported-by". The reported-by is used if someone else submits patch after your bug report. Please (when resubmitting or applying) add following tags: Fixes: d720f057fda4 ("usb: dwc3: exynos: add nop transceiver support") Cc: <stable@vger.kernel.org> This will help downstream (like Debian) using stable kernels. Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof > --- > drivers/usb/dwc3/dwc3-exynos.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index dd5cb55..2f1fb7e 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -128,12 +128,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, exynos); > > - ret = dwc3_exynos_register_phys(exynos); > - if (ret) { > - dev_err(dev, "couldn't register PHYs\n"); > - return ret; > - } > - > exynos->dev = dev; > > exynos->clk = devm_clk_get(dev, "usbdrd30"); > @@ -183,20 +177,29 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > goto err3; > } > > + ret = dwc3_exynos_register_phys(exynos); > + if (ret) { > + dev_err(dev, "couldn't register PHYs\n"); > + goto err4; > + } > + > if (node) { > ret = of_platform_populate(node, NULL, NULL, dev); > if (ret) { > dev_err(dev, "failed to add dwc3 core\n"); > - goto err4; > + goto err5; > } > } else { > dev_err(dev, "no device node, failed to add dwc3 core\n"); > ret = -ENODEV; > - goto err4; > + goto err5; > } > > return 0; > > +err5: > + platform_device_unregister(exynos->usb2_phy); > + platform_device_unregister(exynos->usb3_phy); > err4: > regulator_disable(exynos->vdd10); > err3: > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-24 18:13 ` [PATCH] " Steinar H. Gunderson 2016-05-25 7:54 ` Krzysztof Kozlowski @ 2016-05-27 9:53 ` Vivek Gautam 2016-05-27 11:46 ` Steinar H. Gunderson 1 sibling, 1 reply; 31+ messages in thread From: Vivek Gautam @ 2016-05-27 9:53 UTC (permalink / raw) To: Steinar H. Gunderson Cc: Krzysztof Kozłowski, Mark Brown, linux-samsung-soc, 823552, balbi, Linux USB Mailing List On Tue, May 24, 2016 at 11:43 PM, Steinar H. Gunderson <sesse@google.com> wrote: > dwc3-exynos has two problems during init if the regulators are slow > to come up (for instance if the I2C bus driver is not on the initramfs) > and return probe deferral. First, every time this happens, the driver > leaks the USB phys created; they need to be deallocated on error. > > Second, since the phy devices are created before the regulators fail, > this means that there's a new device to re-trigger deferred probing, > which causes it to essentially go into a busy loop of re-probing the > device until the regulators come up. > > Move the phy creation to after the regulators have succeeded, and also > fix cleanup on failure. On my ODROID XU4 system (with Debian's initramfs > which doesn't contain the I2C driver), this reduces the number of probe > attempts (for each of the two controllers) from more than 2000 to eight. > > Reported-by: Steinar H. Gunderson <sgunderson@bigfoot.com> > Signed-off-by: Steinar H. Gunderson <sesse@google.com> > --- I don't have any concerns with the patch apart from the ones Krzysztof has already pointed out. LGTM. After fixing the patch, Reviewed-by: Vivek Gautam <gautam.vivek@samsung.com> > drivers/usb/dwc3/dwc3-exynos.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index dd5cb55..2f1fb7e 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -128,12 +128,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, exynos); > > - ret = dwc3_exynos_register_phys(exynos); > - if (ret) { > - dev_err(dev, "couldn't register PHYs\n"); > - return ret; > - } > - > exynos->dev = dev; > > exynos->clk = devm_clk_get(dev, "usbdrd30"); > @@ -183,20 +177,29 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > goto err3; > } > > + ret = dwc3_exynos_register_phys(exynos); > + if (ret) { > + dev_err(dev, "couldn't register PHYs\n"); > + goto err4; > + } > + > if (node) { > ret = of_platform_populate(node, NULL, NULL, dev); > if (ret) { > dev_err(dev, "failed to add dwc3 core\n"); > - goto err4; > + goto err5; > } > } else { > dev_err(dev, "no device node, failed to add dwc3 core\n"); > ret = -ENODEV; > - goto err4; > + goto err5; > } > > return 0; > > +err5: > + platform_device_unregister(exynos->usb2_phy); > + platform_device_unregister(exynos->usb3_phy); > err4: > regulator_disable(exynos->vdd10); > err3: > -- > 2.8.0.rc3.226.g39d4020 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-27 9:53 ` Vivek Gautam @ 2016-05-27 11:46 ` Steinar H. Gunderson 2016-05-27 13:13 ` Krzysztof Kozlowski 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-27 11:46 UTC (permalink / raw) To: Vivek Gautam Cc: Steinar H. Gunderson, Krzysztof Kozłowski, Mark Brown, linux-samsung-soc, 823552, balbi, Linux USB Mailing List On Fri, May 27, 2016 at 03:23:35PM +0530, Vivek Gautam wrote: > I don't have any concerns with the patch apart from the ones > Krzysztof has already pointed out. > LGTM. Should I repost the patch, or will people just make these commit message changes for me? I guess balbi@ is the right person to review and merge, since he maintains the driver. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-27 11:46 ` Steinar H. Gunderson @ 2016-05-27 13:13 ` Krzysztof Kozlowski [not found] ` <5748480D.8000501-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2016-05-27 13:13 UTC (permalink / raw) To: Steinar H. Gunderson, Vivek Gautam Cc: Steinar H. Gunderson, Mark Brown, linux-samsung-soc, 823552, balbi, Linux USB Mailing List On 05/27/2016 01:46 PM, Steinar H. Gunderson wrote: > On Fri, May 27, 2016 at 03:23:35PM +0530, Vivek Gautam wrote: >> I don't have any concerns with the patch apart from the ones >> Krzysztof has already pointed out. >> LGTM. > > Should I repost the patch, or will people just make these commit message > changes for me? I guess balbi@ is the right person to review and merge, > since he maintains the driver. Although resend is not strictly needed because maintainer might do the changes by himself... but he might just miss the comments as well (and e.g. use patchwork to apply the patch). I think it would be useful if you send a v2 with fixes pointed by me and accumulated review-tags (mine and Vivek's Reviewed-by). Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <5748480D.8000501-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. [not found] ` <5748480D.8000501-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> @ 2016-05-27 13:12 ` Felipe Balbi [not found] ` <87bn3rprtw.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 31+ messages in thread From: Felipe Balbi @ 2016-05-27 13:12 UTC (permalink / raw) To: Krzysztof Kozlowski, Steinar H. Gunderson, Vivek Gautam Cc: Steinar H. Gunderson, Mark Brown, linux-samsung-soc@vger.kernel.org, 823552-61a8vm9lEZVf4u+23C9RwQ, Linux USB Mailing List [-- Attachment #1: Type: text/plain, Size: 1033 bytes --] Hi, Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> writes: > On 05/27/2016 01:46 PM, Steinar H. Gunderson wrote: >> On Fri, May 27, 2016 at 03:23:35PM +0530, Vivek Gautam wrote: >>> I don't have any concerns with the patch apart from the ones >>> Krzysztof has already pointed out. >>> LGTM. >> >> Should I repost the patch, or will people just make these commit message >> changes for me? I guess balbi@ is the right person to review and merge, >> since he maintains the driver. > > Although resend is not strictly needed because maintainer might do the > changes by himself... but he might just miss the comments as well (and > e.g. use patchwork to apply the patch). > > I think it would be useful if you send a v2 with fixes pointed by me and > accumulated review-tags (mine and Vivek's Reviewed-by). yes, please do that. Keep in mind, also, that we're still in the middle of the merge window and nothing will really happen until v4.7-rc1 is tagged. cheers -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <87bn3rprtw.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. [not found] ` <87bn3rprtw.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2016-05-27 13:25 ` Steinar H. Gunderson 2016-05-27 13:26 ` Felipe Balbi 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-27 13:25 UTC (permalink / raw) To: Felipe Balbi Cc: Krzysztof Kozlowski, Vivek Gautam, Steinar H. Gunderson, Mark Brown, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, 823552-61a8vm9lEZVf4u+23C9RwQ, Linux USB Mailing List On Fri, May 27, 2016 at 04:12:59PM +0300, Felipe Balbi wrote: > yes, please do that. Keep in mind, also, that we're still in the middle > of the merge window and nothing will really happen until v4.7-rc1 is > tagged. Sent. As a fix, there's a chance it could go into 4.7, right? /* Steinar */ -- Homepage: https://www.sesse.net/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-27 13:25 ` Steinar H. Gunderson @ 2016-05-27 13:26 ` Felipe Balbi 2016-05-30 17:46 ` Steinar H. Gunderson 0 siblings, 1 reply; 31+ messages in thread From: Felipe Balbi @ 2016-05-27 13:26 UTC (permalink / raw) To: Steinar H. Gunderson Cc: Krzysztof Kozlowski, Vivek Gautam, Steinar H. Gunderson, Mark Brown, linux-samsung-soc, 823552, Linux USB Mailing List [-- Attachment #1: Type: text/plain, Size: 441 bytes --] Hi, "Steinar H. Gunderson" <sgunderson@bigfoot.com> writes: > On Fri, May 27, 2016 at 04:12:59PM +0300, Felipe Balbi wrote: >> yes, please do that. Keep in mind, also, that we're still in the middle >> of the merge window and nothing will really happen until v4.7-rc1 is >> tagged. > > Sent. As a fix, there's a chance it could go into 4.7, right? yup, shouldn't be a problem. But only after v4.7-rc1 is tagged. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] dwc3-exynos: Fix deferred probing storm. 2016-05-27 13:26 ` Felipe Balbi @ 2016-05-30 17:46 ` Steinar H. Gunderson 0 siblings, 0 replies; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-30 17:46 UTC (permalink / raw) To: Felipe Balbi Cc: Krzysztof Kozlowski, Vivek Gautam, Steinar H. Gunderson, Mark Brown, linux-samsung-soc, 823552, Linux USB Mailing List On Fri, May 27, 2016 at 04:26:42PM +0300, Felipe Balbi wrote: >> Sent. As a fix, there's a chance it could go into 4.7, right? > yup, shouldn't be a problem. But only after v4.7-rc1 is tagged. Seemingly v4.7-rc1 is out today (I was surprised at how quick that was). /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 19:24 ` [PATCH] " Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH v2] dwc3-exynos: Fix deferred probing storm Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH] " Steinar H. Gunderson @ 2016-05-25 12:16 ` Anand Moon 2016-05-25 17:52 ` Steinar H. Gunderson 2 siblings, 1 reply; 31+ messages in thread From: Anand Moon @ 2016-05-25 12:16 UTC (permalink / raw) To: Steinar H. Gunderson Cc: Krzysztof Kozlowski, Mark Brown, linux-samsung-soc, 823552, balbi, linux-usb Hi Steinar, On 25 May 2016 at 00:54, Steinar H. Gunderson <sgunderson@bigfoot.com> wrote: > On Tue, May 24, 2016 at 05:53:34PM +0200, Krzysztof Kozlowski wrote: >> exynos->clk = devm_clk_get(dev, "usbdrd30"); >> if (IS_ERR(exynos->clk)) { >> + // On each error path since here we need to >> + // revert work done by dwc3_exynos_register_phys() >> dev_err(dev, "couldn't get clock\n"); >> return -EINVAL; >> } >> clk_prepare_enable(exynos->clk); > > OK, so I took Mark's advice and moved the phy instantiation towards the end > of the function (after the regulators have successfully come up). It reduced > the number of probes, even with the original initramfs, dramatically, so > it seems to work quite well. It also reduces the text for each deferred probe > by a lot, since we no longer have the dummy regulator message for each one > (only the message about “no suspend clk specified” is left). Finally, this > arrangement reduced the need for extra error handling to a minimum. > > Cc-ing Felipe and and linux-usb@, and adding the patch as a reply to this > message. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Actually their are some missing patches to tune the usb3 phy. https://lkml.org/lkml/2014/10/31/266 Best Regards -Anand Moon ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" 2016-05-25 12:16 ` [PATCH] Re: Endless "supply vcc not found, using dummy regulator" Anand Moon @ 2016-05-25 17:52 ` Steinar H. Gunderson 2016-05-26 12:57 ` Steinar H. Gunderson 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-25 17:52 UTC (permalink / raw) To: Anand Moon Cc: Krzysztof Kozlowski, Mark Brown, linux-samsung-soc, 823552, balbi, linux-usb On Wed, May 25, 2016 at 05:46:51PM +0530, Anand Moon wrote: > Actually their are some missing patches to tune the usb3 phy. > > https://lkml.org/lkml/2014/10/31/266 This explains why the default networking speed refused to go above ~300 Mbit/sec! What happened to the patches, I wonder? /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" 2016-05-25 17:52 ` Steinar H. Gunderson @ 2016-05-26 12:57 ` Steinar H. Gunderson [not found] ` <20160526125757.GA23797-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-26 12:57 UTC (permalink / raw) To: Anand Moon Cc: Krzysztof Kozlowski, Mark Brown, linux-samsung-soc, 823552, balbi, linux-usb, gautam.vivek On Wed, May 25, 2016 at 07:52:36PM +0200, Steinar H. Gunderson wrote: >> Actually their are some missing patches to tune the usb3 phy. >> >> https://lkml.org/lkml/2014/10/31/266 > This explains why the default networking speed refused to go above > ~300 Mbit/sec! What happened to the patches, I wonder? Adding Vivek in case he knows. They certainly don't apply anymore, at least. /* Steinar */ -- Homepage: https://www.sesse.net/ ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <20160526125757.GA23797-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org>]
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" [not found] ` <20160526125757.GA23797-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> @ 2016-05-27 9:32 ` Vivek Gautam [not found] ` <CAFp+6iE_QW0BDO-Z1Ce5zrUJiToWD_8UtnCFnfKLeaYyys7U8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 31+ messages in thread From: Vivek Gautam @ 2016-05-27 9:32 UTC (permalink / raw) To: Steinar H. Gunderson Cc: Anand Moon, Krzysztof Kozlowski, Mark Brown, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, 823552-61a8vm9lEZVf4u+23C9RwQ, balbi-DgEjT+Ai2ygdnm+yROfE0A, Linux USB Mailing List, Vivek Gautam On Thu, May 26, 2016 at 6:27 PM, Steinar H. Gunderson <sgunderson-jG/AHqQBv7lBDgjK7y7TUQ@public.gmane.org> wrote: > On Wed, May 25, 2016 at 07:52:36PM +0200, Steinar H. Gunderson wrote: >>> Actually their are some missing patches to tune the usb3 phy. >>> >>> https://lkml.org/lkml/2014/10/31/266 >> This explains why the default networking speed refused to go above >> ~300 Mbit/sec! What happened to the patches, I wonder? > > Adding Vivek in case he knows. They certainly don't apply anymore, at least. Above mentioned patches were not accepted by the maintainers of generic-phy and usb. I couldn't get any response on them for quite a long time. So, the patches could never make it to the mainline. I can try initiating the entire exercise once again and try to get them merged. >From the thread, i can't make out the configuration of the board you are using. If the network is coming out of the USB 3.0 phy, then definitely you will see a performance drop. AFA dwc3-exynos is concerned, there's definitely a resource leak as pointed out by you. Please post the suggested patch, adding required people in CC. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <CAFp+6iE_QW0BDO-Z1Ce5zrUJiToWD_8UtnCFnfKLeaYyys7U8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" [not found] ` <CAFp+6iE_QW0BDO-Z1Ce5zrUJiToWD_8UtnCFnfKLeaYyys7U8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-05-27 9:39 ` Steinar H. Gunderson [not found] ` <20160527093919.GA34987-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> 0 siblings, 1 reply; 31+ messages in thread From: Steinar H. Gunderson @ 2016-05-27 9:39 UTC (permalink / raw) To: Vivek Gautam Cc: Anand Moon, Krzysztof Kozlowski, Mark Brown, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, 823552-61a8vm9lEZVf4u+23C9RwQ, balbi-DgEjT+Ai2ygdnm+yROfE0A, Linux USB Mailing List, Vivek Gautam On Fri, May 27, 2016 at 03:02:48PM +0530, Vivek Gautam wrote: > Above mentioned patches were not accepted by the maintainers of generic-phy > and usb. I couldn't get any response on them for quite a long time. So, the > patches could never make it to the mainline. > I can try initiating the entire exercise once again and try to get them merged. That would be nice; having USB3 speeds is certainly attractive. > AFA dwc3-exynos is concerned, there's definitely a resource leak as > pointed out by you. > Please post the suggested patch, adding required people in CC. http://www.spinics.net/lists/linux-usb/msg141385.html Is there anyone I should have Cc-ed that I didn't? /* Steinar */ -- Homepage: https://www.sesse.net/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <20160527093919.GA34987-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org>]
* Re: [PATCH] Re: Endless "supply vcc not found, using dummy regulator" [not found] ` <20160527093919.GA34987-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> @ 2016-05-27 9:43 ` Vivek Gautam 0 siblings, 0 replies; 31+ messages in thread From: Vivek Gautam @ 2016-05-27 9:43 UTC (permalink / raw) To: Steinar H. Gunderson Cc: Anand Moon, Krzysztof Kozlowski, Mark Brown, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, 823552-61a8vm9lEZVf4u+23C9RwQ, balbi-DgEjT+Ai2ygdnm+yROfE0A, Linux USB Mailing List, Vivek Gautam On Fri, May 27, 2016 at 3:09 PM, Steinar H. Gunderson <sgunderson-jG/AHqQBv7lBDgjK7y7TUQ@public.gmane.org> wrote: > On Fri, May 27, 2016 at 03:02:48PM +0530, Vivek Gautam wrote: >> Above mentioned patches were not accepted by the maintainers of generic-phy >> and usb. I couldn't get any response on them for quite a long time. So, the >> patches could never make it to the mainline. >> I can try initiating the entire exercise once again and try to get them merged. > > That would be nice; having USB3 speeds is certainly attractive. > >> AFA dwc3-exynos is concerned, there's definitely a resource leak as >> pointed out by you. >> Please post the suggested patch, adding required people in CC. > > http://www.spinics.net/lists/linux-usb/msg141385.html > > Is there anyone I should have Cc-ed that I didn't? That's alright, i missed noticing your patch. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Endless "supply vcc not found, using dummy regulator" 2016-05-24 15:06 ` Krzysztof Kozlowski 2016-05-24 15:26 ` Steinar H. Gunderson @ 2016-05-24 15:39 ` Mark Brown 1 sibling, 0 replies; 31+ messages in thread From: Mark Brown @ 2016-05-24 15:39 UTC (permalink / raw) To: Krzysztof Kozlowski; +Cc: Steinar H. Gunderson, linux-samsung-soc, 823552 [-- Attachment #1: Type: text/plain, Size: 627 bytes --] On Tue, May 24, 2016 at 05:06:43PM +0200, Krzysztof Kozlowski wrote: > What other problems exactly do you have? Late binding of S2MPS11 > regulator driver? That does not look like a problem. If it is built as > a module then it should be loaded, probably from initramfs because > these are regulators. AFAICT the fact that every time the dwc3-exynos driver probes it creates a new child device which successfully instantiates means that we end up constantly running deferred probes. It should only create the child devices if it managed to get the other resources, that way we don't get the constant deferred probe retries. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2016-05-30 17:46 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20160515173900.GC21194@sesse.net> 2016-05-21 14:43 ` Endless "supply vcc not found, using dummy regulator" Steinar H. Gunderson 2016-05-23 13:47 ` Steinar H. Gunderson 2016-05-23 14:40 ` Steinar H. Gunderson 2016-05-23 16:24 ` Mark Brown 2016-05-23 17:06 ` Steinar H. Gunderson 2016-05-23 17:46 ` Steinar H. Gunderson 2016-05-23 18:56 ` Mark Brown 2016-05-23 18:44 ` Mark Brown 2016-05-24 15:06 ` Krzysztof Kozlowski 2016-05-24 15:26 ` Steinar H. Gunderson 2016-05-24 15:45 ` Mark Brown 2016-05-24 15:53 ` Krzysztof Kozlowski 2016-05-24 16:44 ` Steinar H. Gunderson [not found] ` <574478FE.4070800-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 2016-05-24 19:24 ` [PATCH] " Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH v2] dwc3-exynos: Fix deferred probing storm Steinar H. Gunderson 2016-05-24 18:13 ` [PATCH] " Steinar H. Gunderson 2016-05-25 7:54 ` Krzysztof Kozlowski 2016-05-27 9:53 ` Vivek Gautam 2016-05-27 11:46 ` Steinar H. Gunderson 2016-05-27 13:13 ` Krzysztof Kozlowski [not found] ` <5748480D.8000501-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> 2016-05-27 13:12 ` Felipe Balbi [not found] ` <87bn3rprtw.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2016-05-27 13:25 ` Steinar H. Gunderson 2016-05-27 13:26 ` Felipe Balbi 2016-05-30 17:46 ` Steinar H. Gunderson 2016-05-25 12:16 ` [PATCH] Re: Endless "supply vcc not found, using dummy regulator" Anand Moon 2016-05-25 17:52 ` Steinar H. Gunderson 2016-05-26 12:57 ` Steinar H. Gunderson [not found] ` <20160526125757.GA23797-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> 2016-05-27 9:32 ` Vivek Gautam [not found] ` <CAFp+6iE_QW0BDO-Z1Ce5zrUJiToWD_8UtnCFnfKLeaYyys7U8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-05-27 9:39 ` Steinar H. Gunderson [not found] ` <20160527093919.GA34987-gdzBep0Ce9heoWH0uzbU5w@public.gmane.org> 2016-05-27 9:43 ` Vivek Gautam 2016-05-24 15:39 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.