| Message ID | 20160523134737.GA21683@sesse.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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 */
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.
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 */
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 */
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.
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.
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 -- 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
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 */
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.
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.
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 -- 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
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 */
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 */
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 -- 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
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 */
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 */
On Thu, May 26, 2016 at 6:27 PM, Steinar H. Gunderson <sgunderson@bigfoot.com> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
On Fri, May 27, 2016 at 3:09 PM, Steinar H. Gunderson <sgunderson@bigfoot.com> 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/
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);