Hi,
Vicente Bergas <vicencb@xxxxxxxxx> writes:
> On Wednesday, August 14, 2019 3:12:26 PM CEST, Robin Murphy wrote:
>> On 14/08/2019 13:53, Vicente Bergas wrote:
>>> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote: ...
>>
>> This particular change looks like it's implicitly specific to
>> RK3399, which wouldn't be ideal. Presumably if the core dwc3
>> driver implemented shutdown correctly (echoing parts of
>> dwc3_remove(), I guess) then the glue layers shouldn't need
>> anything special anyway.
>>
>> Robin.
>
> I just checked simple->resets from dwc3-of-simple.c and it is an array
> with multiple resets whereas dwc->reset from core.c is NULL.
> So the reset seems specific to the glue layers.
> Is there another way than resetting the thing that is
> generic enough to go to core.c and allows kexec?
This is a really odd 'failure'. We do full soft reset during driver
initialization on dwc3. We shouldn't need to assert reset on shutdown,
really.
I think the problem is here:
if (simple->pulse_resets) {
ret = reset_control_reset(simple->resets);
if (ret)
goto err_resetc_put;
} else {
ret = reset_control_deassert(simple->resets);
if (ret)
goto err_resetc_put;
}
Note that if pulse_resets is set, we will run a reset. But if
pulse_resets is false and need_reset is true, we deassert the reset.
I think below patch is enough:
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index bdac3e7d7b18..9a2f3e09aa2e 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -72,7 +72,15 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
ret = reset_control_reset(simple->resets);
if (ret)
goto err_resetc_put;
- } else {
+ }
+
+ if (simple->need_reset) {
+ ret = reset_control_assert(simple->resets);
+ if (ret)
+ goto err_resetc_put;
+
+ usleep_range(1000, 2000);
+
ret = reset_control_deassert(simple->resets);
if (ret)
goto err_resetc_put;
@@ -121,9 +129,6 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
clk_bulk_put_all(simple->num_clocks, simple->clks);
simple->num_clocks = 0;
- if (!simple->pulse_resets)
- reset_control_assert(simple->resets);
-
reset_control_put(simple->resets);
pm_runtime_disable(dev);
Can you test?
--
balbi
