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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
