* kexec on rk3399
@ 2019-07-22 14:31 Vicente Bergas
2019-07-22 14:46 ` Will Deacon
` (3 more replies)
0 siblings, 4 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-07-22 14:31 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Heiko Stuebner,
linux-rockchip
Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
From 5.2 onwards, there are memory corruption issues as reported here:
http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
kexec has been identified as the principal reason for the issues.
It turns out that kexec has never worked reliably on this platform,
i was just lucky until recently.
Please, can you provide some directions on how to debug the issue?
The latest test performed is:
1.- Boot v5.3-rc1
2.- Kexec into v5.2.2
3.- The kernel reports nothing (earlycon not enabled) and does not boot.
The same kernel v5.2.2 works fine when booted without kexec.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:31 kexec on rk3399 Vicente Bergas
@ 2019-07-22 14:46 ` Will Deacon
2019-07-22 16:08 ` Vicente Bergas
2019-07-22 14:47 ` Matthias Brugger
` (2 subsequent siblings)
3 siblings, 1 reply; 28+ messages in thread
From: Will Deacon @ 2019-07-22 14:46 UTC (permalink / raw)
To: Vicente Bergas
Cc: Heiko Stuebner, Marc Zyngier, Catalin Marinas, Will Deacon,
linux-rockchip, linux-arm-kernel
On Mon, Jul 22, 2019 at 04:31:27PM +0200, Vicente Bergas wrote:
> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
> From 5.2 onwards, there are memory corruption issues as reported here:
> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
> kexec has been identified as the principal reason for the issues.
I thought that you resolved this issue by upgrading u-boot. Did that not
actually work?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:31 kexec on rk3399 Vicente Bergas
2019-07-22 14:46 ` Will Deacon
@ 2019-07-22 14:47 ` Matthias Brugger
2019-07-22 16:56 ` Vicente Bergas
2019-07-22 14:54 ` Marc Zyngier
2019-08-14 12:53 ` Vicente Bergas
3 siblings, 1 reply; 28+ messages in thread
From: Matthias Brugger @ 2019-07-22 14:47 UTC (permalink / raw)
To: Vicente Bergas, linux-arm-kernel
Cc: Marc Zyngier, Catalin Marinas, Will Deacon, Heiko Stuebner,
linux-rockchip
On 22/07/2019 16:31, Vicente Bergas wrote:
> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
> From 5.2 onwards, there are memory corruption issues as reported here:
> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
> kexec has been identified as the principal reason for the issues.
>
> It turns out that kexec has never worked reliably on this platform,
> i was just lucky until recently.
>
> Please, can you provide some directions on how to debug the issue?
>
> The latest test performed is:
> 1.- Boot v5.3-rc1
> 2.- Kexec into v5.2.2
> 3.- The kernel reports nothing (earlycon not enabled) and does not boot.
> The same kernel v5.2.2 works fine when booted without kexec.
Is there a known kernel where kexec was working (v5.2 e.g.)?
If you can find one, you can run git bisect to find the commit that breaks things.
Apart from that you should try to enable earlycon on your kernel you want to
kexec into, so at least you can see if it crashes in the new kernel. If you
still don't see anything you can try to use prugatory printing (if you use
kexec_load and not kexec_file_load). I have a work-in-progress patches that I
can share with you if needed. Just let me know.
Regards,
Matthias
>
> Regards,
> Vicenç.
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:31 kexec on rk3399 Vicente Bergas
2019-07-22 14:46 ` Will Deacon
2019-07-22 14:47 ` Matthias Brugger
@ 2019-07-22 14:54 ` Marc Zyngier
2019-07-22 17:05 ` Vicente Bergas
2019-08-14 12:53 ` Vicente Bergas
3 siblings, 1 reply; 28+ messages in thread
From: Marc Zyngier @ 2019-07-22 14:54 UTC (permalink / raw)
To: Vicente Bergas, linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, Heiko Stuebner, linux-rockchip
On 22/07/2019 15:31, Vicente Bergas wrote:
> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
> From 5.2 onwards, there are memory corruption issues as reported here:
> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
> kexec has been identified as the principal reason for the issues.
>
> It turns out that kexec has never worked reliably on this platform,
> i was just lucky until recently.
>
> Please, can you provide some directions on how to debug the issue?
>
> The latest test performed is:
> 1.- Boot v5.3-rc1
Not the most trusted version, but hey, why not...
> 2.- Kexec into v5.2.2
> 3.- The kernel reports nothing (earlycon not enabled) and does not boot.
> The same kernel v5.2.2 works fine when booted without kexec.
Can you please enable earlycon and let us know how far it goes? Your
previous reports hinting at runtime memory corruption, but this seems to
be much more radical...
FWIW, I was able to kexec using 5.2-rcx (for values of x around 6) on a
NanoPC-T4, using a mainline u-boot and firmware.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:46 ` Will Deacon
@ 2019-07-22 16:08 ` Vicente Bergas
0 siblings, 0 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-07-22 16:08 UTC (permalink / raw)
To: Will Deacon
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-rockchip, linux-arm-kernel
On Monday, July 22, 2019 4:46:36 PM CEST, Will Deacon wrote:
> On Mon, Jul 22, 2019 at 04:31:27PM +0200, Vicente Bergas wrote:
>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>> From 5.2 onwards, there are memory corruption issues as reported here:
>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>> kexec has been identified as the principal reason for the issues.
>
> I thought that you resolved this issue by upgrading u-boot. Did that not
> actually work?
>
> Will
I am now using u-boot, but that is not a solution.
My intention is to use it on the Sapphire board and on gru-kevin.
I would like the bootloader to support HDMI on the sapphire and EDP on
kevin
for the interactive boot menu.
U-Boot on the Sapphire boots, but has no graphic display. Kevin is not
supported at all.
I've achived some progress on the u-boot front, but not enough to have
display.
With a kexec-based bootloader display is already working, out of the box.
From another point of view, I would like to reduce the TCB (trusted
computing base).
Linux is huge, but is already in the TCB, so, executing it twice does not
add to the TCB.
U-Boot, instead, is enlarging the TCB, in this case.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:47 ` Matthias Brugger
@ 2019-07-22 16:56 ` Vicente Bergas
0 siblings, 0 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-07-22 16:56 UTC (permalink / raw)
To: Matthias Brugger
Cc: Heiko Stuebner, Marc Zyngier, Catalin Marinas, Will Deacon,
linux-rockchip, linux-arm-kernel
On Monday, July 22, 2019 4:47:51 PM CEST, Matthias Brugger wrote:
>
> On 22/07/2019 16:31, Vicente Bergas wrote:
>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>> From 5.2 onwards, there are memory corruption issues as reported here:
>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>> kexec has been identified as the principal reason for the issues.
>>
>> It turns out that kexec has never worked reliably on this platform, ...
>
> Is there a known kernel where kexec was working (v5.2 e.g.)?
> If you can find one, you can run git bisect to find the commit
> that breaks things.
Yes, most kernels i have tried before 5.2 worked, more or less.
From time to time there were random freezes at boot time.
Re-booting 2 or 3 times fixed it. This was happenning about 1 out of 10
times.
I've rarely seen kernel traces on dmesg, but these were not affecting the
stability of the system.
I've also used 5.2 (a known affected version) during days with no issues.
So, the randomness of the issue reproducibility makes bisecting too
dificult.
> Apart from that you should try to enable earlycon on your kernel you want to
> kexec into, so at least you can see if it crashes in the new kernel.
Added earlycon to the cmdline. Now it boots fine, although it is not
working, the d_lookup issues are still there.
Basically, it looks like anything that changes the kernel memory layout
(like a single extra alloc) affects the issue, making it completely random.
> If you
> still don't see anything you can try to use prugatory printing (if you use
> kexec_load and not kexec_file_load). I have a work-in-progress
> patches that I
> can share with you if needed. Just let me know.
What is 'prugatory printing' ? I am using kexec_load from kexec-tools. The
option to use kexec_file_load is not used.
> Regards,
> Matthias
>
>
>> Regards,
>> Vicenç.
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org ...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:54 ` Marc Zyngier
@ 2019-07-22 17:05 ` Vicente Bergas
2019-07-22 17:35 ` Robin Murphy
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-07-22 17:05 UTC (permalink / raw)
To: Marc Zyngier
Cc: Matthias Brugger, Heiko Stuebner, Catalin Marinas, Will Deacon,
linux-rockchip, linux-arm-kernel
On Monday, July 22, 2019 4:54:41 PM CEST, Marc Zyngier wrote:
> On 22/07/2019 15:31, Vicente Bergas wrote:
>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>> From 5.2 onwards, there are memory corruption issues as reported here:
>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>> kexec has been identified as the principal reason for the issues.
>>
>> It turns out that kexec has never worked reliably on this platform, ...
>
> Not the most trusted version, but hey, why not...
>
>> 2.- Kexec into v5.2.2
>> 3.- The kernel reports nothing (earlycon not enabled) and does not boot.
>> The same kernel v5.2.2 works fine when booted without kexec.
>
> Can you please enable earlycon and let us know how far it goes? Your
> previous reports hinting at runtime memory corruption, but this seems to
> be much more radical...
Details on previous email, but basically, earlycon affects reproducibility.
If it is a runtime memory corruption, what are we hunting for? an issue in
the kernel before kexec or in the kernel after kexec?
> FWIW, I was able to kexec using 5.2-rcx (for values of x around 6) on a
> NanoPC-T4, using a mainline u-boot and firmware.
>
> Thanks,
>
> M.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 17:05 ` Vicente Bergas
@ 2019-07-22 17:35 ` Robin Murphy
2019-07-22 18:53 ` Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Robin Murphy @ 2019-07-22 17:35 UTC (permalink / raw)
To: Vicente Bergas, Marc Zyngier
Cc: Matthias Brugger, Heiko Stuebner, Catalin Marinas, Will Deacon,
linux-rockchip, linux-arm-kernel
On 22/07/2019 18:05, Vicente Bergas wrote:
> On Monday, July 22, 2019 4:54:41 PM CEST, Marc Zyngier wrote:
>> On 22/07/2019 15:31, Vicente Bergas wrote:
>>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>>> From 5.2 onwards, there are memory corruption issues as reported here:
>>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>>> kexec has been identified as the principal reason for the issues.
>>>
>>> It turns out that kexec has never worked reliably on this platform, ...
>>
>> Not the most trusted version, but hey, why not...
>>
>>> 2.- Kexec into v5.2.2
>>> 3.- The kernel reports nothing (earlycon not enabled) and does not
>>> boot.
>>> The same kernel v5.2.2 works fine when booted without kexec.
>>
>> Can you please enable earlycon and let us know how far it goes? Your
>> previous reports hinting at runtime memory corruption, but this seems to
>> be much more radical...
>
> Details on previous email, but basically, earlycon affects reproducibility.
>
> If it is a runtime memory corruption, what are we hunting for? an issue in
> the kernel before kexec or in the kernel after kexec?
The obvious culprit would be DMA devices left running by the first
kernel scribbling over the second kernel's memory before it's had the
chance to reset them. Since boot-time memory allocation patterns tend to
be relatively repeatable for a given platform and kernel configuration,
"random" may well look a lot less random than you might expect, and it
wouldn't be unheard of for e.g. the second kernel to mostly allocate its
dentry cache from the same area the first kernel was mostly putting a
network Rx descriptor ring.
Robin.
>
>> FWIW, I was able to kexec using 5.2-rcx (for values of x around 6) on a
>> NanoPC-T4, using a mainline u-boot and firmware.
>>
>> Thanks,
>>
>> M.
>
> Regards,
> Vicenç.
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 17:35 ` Robin Murphy
@ 2019-07-22 18:53 ` Vicente Bergas
2019-07-23 13:32 ` Robin Murphy
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-07-22 18:53 UTC (permalink / raw)
To: Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-rockchip, linux-arm-kernel
On Monday, July 22, 2019 7:35:01 PM CEST, Robin Murphy wrote:
> On 22/07/2019 18:05, Vicente Bergas wrote:
>> On Monday, July 22, 2019 4:54:41 PM CEST, Marc Zyngier wrote: ...
>
> The obvious culprit would be DMA devices left running by the
> first kernel scribbling over the second kernel's memory before
> it's had the chance to reset them. Since boot-time memory
> allocation patterns tend to be relatively repeatable for a given
> platform and kernel configuration, "random" may well look a lot
> less random than you might expect, and it wouldn't be unheard of
> for e.g. the second kernel to mostly allocate its dentry cache
> from the same area the first kernel was mostly putting a network
> Rx descriptor ring.
>
> Robin.
Here is another attempted test: on the first kernel disable:
# CONFIG_ZONE_DMA32 is not set
# CONFIG_DMADEVICES is not set
# CONFIG_PL330_DMA is not set
that is, all i could disable matching CONFIG_*DMA*=y, which is not much.
Still enabled are:
CONFIG_HAVE_DMA_CONTIGUOUS=y
CONFIG_DMA_SHARED_BUFFER=y
CONFIG_SCSI_DMA=y
CONFIG_IOMMU_DMA=y
CONFIG_HAS_DMA=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_ARCH_DMA_ADDR_T_64BIT=y
CONFIG_DMA_DECLARE_COHERENT=y
CONFIG_ARCH_HAS_SETUP_DMA_OPS=y
CONFIG_ARCH_HAS_TEARDOWN_DMA_OPS=y
CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE=y
CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU=y
CONFIG_ARCH_HAS_DMA_PREP_COHERENT=y
CONFIG_ARCH_HAS_DMA_COHERENT_TO_PFN=y
CONFIG_ARCH_HAS_DMA_MMAP_PGPROT=y
CONFIG_DMA_REMAP=y
CONFIG_DMA_DIRECT_REMAP=y
Then the second kernel still fails with d_lookup errors.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 18:53 ` Vicente Bergas
@ 2019-07-23 13:32 ` Robin Murphy
0 siblings, 0 replies; 28+ messages in thread
From: Robin Murphy @ 2019-07-23 13:32 UTC (permalink / raw)
To: Vicente Bergas
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-rockchip, linux-arm-kernel
On 22/07/2019 19:53, Vicente Bergas wrote:
> On Monday, July 22, 2019 7:35:01 PM CEST, Robin Murphy wrote:
>> On 22/07/2019 18:05, Vicente Bergas wrote:
>>> On Monday, July 22, 2019 4:54:41 PM CEST, Marc Zyngier wrote: ...
>>
>> The obvious culprit would be DMA devices left running by the first
>> kernel scribbling over the second kernel's memory before it's had the
>> chance to reset them. Since boot-time memory allocation patterns tend
>> to be relatively repeatable for a given platform and kernel
>> configuration, "random" may well look a lot less random than you might
>> expect, and it wouldn't be unheard of for e.g. the second kernel to
>> mostly allocate its dentry cache from the same area the first kernel
>> was mostly putting a network Rx descriptor ring.
>>
>> Robin.
>
> Here is another attempted test: on the first kernel disable:
> # CONFIG_ZONE_DMA32 is not set
> # CONFIG_DMADEVICES is not set
> # CONFIG_PL330_DMA is not set
> that is, all i could disable matching CONFIG_*DMA*=y, which is not much.
> Still enabled are:
> CONFIG_HAVE_DMA_CONTIGUOUS=y
> CONFIG_DMA_SHARED_BUFFER=y
> CONFIG_SCSI_DMA=y
> CONFIG_IOMMU_DMA=y
> CONFIG_HAS_DMA=y
> CONFIG_NEED_SG_DMA_LENGTH=y
> CONFIG_NEED_DMA_MAP_STATE=y
> CONFIG_ARCH_DMA_ADDR_T_64BIT=y
> CONFIG_DMA_DECLARE_COHERENT=y
> CONFIG_ARCH_HAS_SETUP_DMA_OPS=y
> CONFIG_ARCH_HAS_TEARDOWN_DMA_OPS=y
> CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE=y
> CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU=y
> CONFIG_ARCH_HAS_DMA_PREP_COHERENT=y
> CONFIG_ARCH_HAS_DMA_COHERENT_TO_PFN=y
> CONFIG_ARCH_HAS_DMA_MMAP_PGPROT=y
> CONFIG_DMA_REMAP=y
> CONFIG_DMA_DIRECT_REMAP=y
>
> Then the second kernel still fails with d_lookup errors.
Yeah, none of those configs are particularly relevant - I'm thinking
more about the drivers for ethernet, wifi, USB, PCI devices, and any
other peripherals which may be set up to make DMA accesses based on
external stimuli and don't get shut down cleanly by a kexec.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-07-22 14:31 kexec on rk3399 Vicente Bergas
` (2 preceding siblings ...)
2019-07-22 14:54 ` Marc Zyngier
@ 2019-08-14 12:53 ` Vicente Bergas
2019-08-14 13:06 ` Felipe Balbi
2019-08-14 13:12 ` Robin Murphy
3 siblings, 2 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-08-14 12:53 UTC (permalink / raw)
To: Felipe Balbi, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote:
> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
> From 5.2 onwards, there are memory corruption issues as reported here:
> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
> kexec has been identified as the principal reason for the issues.
>
> It turns out that kexec has never worked reliably on this platform,
> i was just lucky until recently.
>
> Please, can you provide some directions on how to debug the issue?
Thank you all for your suggestions on where the issue could be.
It seems that it was the USB driver.
Now using v5.2.8 booted with kexec from v5.2.8 with a workaround and
so far so good. It is being tested on the Sapphire board.
The workaround is:
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -133,6 +133,13 @@
return 0;
}
+static void dwc3_of_simple_shutdown(struct platform_device *pdev)
+{
+ struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
+
+ reset_control_assert(simple->resets);
+}
+
static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device
*dev)
{
struct dwc3_of_simple *simple = dev_get_drvdata(dev);
@@ -190,6 +197,7 @@
static struct platform_driver dwc3_of_simple_driver = {
.probe = dwc3_of_simple_probe,
.remove = dwc3_of_simple_remove,
+ .shutdown = dwc3_of_simple_shutdown,
.driver = {
.name = "dwc3-of-simple",
.of_match_table = of_dwc3_simple_match,
If this patch is OK after review i can resubmit it as a pull request.
Should a similar change be applied to drivers/usb/dwc3/core.c ?
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-14 12:53 ` Vicente Bergas
@ 2019-08-14 13:06 ` Felipe Balbi
2019-08-14 13:15 ` Vicente Bergas
2019-08-14 13:12 ` Robin Murphy
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2019-08-14 13:06 UTC (permalink / raw)
To: Vicente Bergas, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2085 bytes --]
Hi,
Vicente Bergas <vicencb@gmail.com> writes:
> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote:
>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>> From 5.2 onwards, there are memory corruption issues as reported here:
>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>> kexec has been identified as the principal reason for the issues.
>>
>> It turns out that kexec has never worked reliably on this platform,
>> i was just lucky until recently.
>>
>> Please, can you provide some directions on how to debug the issue?
>
> Thank you all for your suggestions on where the issue could be.
>
> It seems that it was the USB driver.
> Now using v5.2.8 booted with kexec from v5.2.8 with a workaround and
> so far so good. It is being tested on the Sapphire board.
>
> The workaround is:
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -133,6 +133,13 @@
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
> +
> + reset_control_assert(simple->resets);
> +}
> +
> static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device
> *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +197,7 @@
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
>
> If this patch is OK after review i can resubmit it as a pull request.
not a pull request, just send a patch using git send-email
> Should a similar change be applied to drivers/usb/dwc3/core.c ?
Is it necessary? We haven't had any bug reports regarding that. Also, if
we have reset control support in the core driver, why do we need it in
of_simple? Seems like of_simple could just rely on what core does.
--
balbi
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-14 12:53 ` Vicente Bergas
2019-08-14 13:06 ` Felipe Balbi
@ 2019-08-14 13:12 ` Robin Murphy
2019-08-15 1:15 ` Vicente Bergas
1 sibling, 1 reply; 28+ messages in thread
From: Robin Murphy @ 2019-08-14 13:12 UTC (permalink / raw)
To: Vicente Bergas, Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
On 14/08/2019 13:53, Vicente Bergas wrote:
> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote:
>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>> From 5.2 onwards, there are memory corruption issues as reported here:
>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>> kexec has been identified as the principal reason for the issues.
>>
>> It turns out that kexec has never worked reliably on this platform,
>> i was just lucky until recently.
>>
>> Please, can you provide some directions on how to debug the issue?
>
> Thank you all for your suggestions on where the issue could be.
>
> It seems that it was the USB driver.
> Now using v5.2.8 booted with kexec from v5.2.8 with a workaround and
> so far so good. It is being tested on the Sapphire board.
>
> The workaround is:
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -133,6 +133,13 @@
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
> +
> + reset_control_assert(simple->resets);
> +}
> +
> static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device
> *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +197,7 @@
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
>
> If this patch is OK after review i can resubmit it as a pull request.
> Should a similar change be applied to drivers/usb/dwc3/core.c ?
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-14 13:06 ` Felipe Balbi
@ 2019-08-14 13:15 ` Vicente Bergas
2019-08-15 6:00 ` Felipe Balbi
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-08-14 13:15 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, Robin Murphy, linux-arm-kernel
On Wednesday, August 14, 2019 3:06:04 PM CEST, Felipe Balbi wrote:
> Hi,
>
> Vicente Bergas <vicencb@gmail.com> writes:
>> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote:
>>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>>> From 5.2 onwards, there are memory corruption issues as reported here:
>>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>>> kexec has been identified as the principal reason for the issues.
>>>
>>> It turns out that kexec has never worked reliably on this platform, ...
>>
>> Thank you all for your suggestions on where the issue could be.
>>
>> It seems that it was the USB driver.
>> Now using v5.2.8 booted with kexec from v5.2.8 with a workaround and
>> so far so good. It is being tested on the Sapphire board.
>>
>> The workaround is:
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -133,6 +133,13 @@
>> return 0;
>> }
>>
>> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
>> +{
>> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
>> +
>> + reset_control_assert(simple->resets);
>> +}
>> +
>> static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device
>> *dev)
>> {
>> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
>> @@ -190,6 +197,7 @@
>> static struct platform_driver dwc3_of_simple_driver = {
>> .probe = dwc3_of_simple_probe,
>> .remove = dwc3_of_simple_remove,
>> + .shutdown = dwc3_of_simple_shutdown,
>> .driver = {
>> .name = "dwc3-of-simple",
>> .of_match_table = of_dwc3_simple_match,
>>
>> If this patch is OK after review i can resubmit it as a pull request.
>
> not a pull request, just send a patch using git send-email
>
>> Should a similar change be applied to drivers/usb/dwc3/core.c ?
>
> Is it necessary? We haven't had any bug reports regarding that. Also, if
> we have reset control support in the core driver, why do we need it in
> of_simple? Seems like of_simple could just rely on what core does.
the workaround has been tested patching only core.c with
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1561,6 +1561,13 @@
return 0;
}
+static void dwc3_shutdown(struct platform_device *pdev)
+{
+ struct dwc3 *dwc = platform_get_drvdata(pdev);
+
+ reset_control_assert(dwc->reset);
+}
+
#ifdef CONFIG_PM
static int dwc3_core_init_for_resume(struct dwc3 *dwc)
{
@@ -1866,6 +1873,7 @@
static struct platform_driver dwc3_driver = {
.probe = dwc3_probe,
.remove = dwc3_remove,
+ .shutdown = dwc3_shutdown,
.driver = {
.name = "dwc3",
.of_match_table = of_match_ptr(of_dwc3_match),
and leaving dwc3-of-simple.c as is, the issue persisted.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-14 13:12 ` Robin Murphy
@ 2019-08-15 1:15 ` Vicente Bergas
2019-08-15 6:06 ` Felipe Balbi
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-08-15 1:15 UTC (permalink / raw)
To: Robin Murphy, Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
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?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-14 13:15 ` Vicente Bergas
@ 2019-08-15 6:00 ` Felipe Balbi
0 siblings, 0 replies; 28+ messages in thread
From: Felipe Balbi @ 2019-08-15 6:00 UTC (permalink / raw)
To: Vicente Bergas
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, Robin Murphy, linux-arm-kernel
Hi,
Vicente Bergas <vicencb@gmail.com> writes:
>> Vicente Bergas <vicencb@gmail.com> writes:
>>> On Monday, July 22, 2019 4:31:27 PM CEST, Vicente Bergas wrote:
>>>> Hi, i have been running linux on rk3399 booted with kexec fine until 5.2
>>>> From 5.2 onwards, there are memory corruption issues as reported here:
>>>> http://lkml.iu.edu/hypermail/linux/kernel/1906.2/07211.html
>>>> kexec has been identified as the principal reason for the issues.
>>>>
>>>> It turns out that kexec has never worked reliably on this platform, ...
>>>
>>> Thank you all for your suggestions on where the issue could be.
>>>
>>> It seems that it was the USB driver.
>>> Now using v5.2.8 booted with kexec from v5.2.8 with a workaround and
>>> so far so good. It is being tested on the Sapphire board.
>>>
>>> The workaround is:
>>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>>> @@ -133,6 +133,13 @@
>>> return 0;
>>> }
>>>
>>> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
>>> +{
>>> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
>>> +
>>> + reset_control_assert(simple->resets);
>>> +}
>>> +
>>> static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device
>>> *dev)
>>> {
>>> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
>>> @@ -190,6 +197,7 @@
>>> static struct platform_driver dwc3_of_simple_driver = {
>>> .probe = dwc3_of_simple_probe,
>>> .remove = dwc3_of_simple_remove,
>>> + .shutdown = dwc3_of_simple_shutdown,
>>> .driver = {
>>> .name = "dwc3-of-simple",
>>> .of_match_table = of_dwc3_simple_match,
>>>
>>> If this patch is OK after review i can resubmit it as a pull request.
>>
>> not a pull request, just send a patch using git send-email
>>
>>> Should a similar change be applied to drivers/usb/dwc3/core.c ?
>>
>> Is it necessary? We haven't had any bug reports regarding that. Also, if
>> we have reset control support in the core driver, why do we need it in
>> of_simple? Seems like of_simple could just rely on what core does.
>
> the workaround has been tested patching only core.c with
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1561,6 +1561,13 @@
> return 0;
> }
>
> +static void dwc3_shutdown(struct platform_device *pdev)
> +{
> + struct dwc3 *dwc = platform_get_drvdata(pdev);
> +
> + reset_control_assert(dwc->reset);
> +}
> +
> #ifdef CONFIG_PM
> static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> {
> @@ -1866,6 +1873,7 @@
> static struct platform_driver dwc3_driver = {
> .probe = dwc3_probe,
> .remove = dwc3_remove,
> + .shutdown = dwc3_shutdown,
> .driver = {
> .name = "dwc3",
> .of_match_table = of_match_ptr(of_dwc3_match),
>
> and leaving dwc3-of-simple.c as is, the issue persisted.
That's because your reset controller is not passed to dwc3 core, only to
your glue layer.
--
balbi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-15 1:15 ` Vicente Bergas
@ 2019-08-15 6:06 ` Felipe Balbi
2019-08-15 11:09 ` Robin Murphy
0 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2019-08-15 6:06 UTC (permalink / raw)
To: Vicente Bergas, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
Hi,
Vicente Bergas <vicencb@gmail.com> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: kexec on rk3399
2019-08-15 6:06 ` Felipe Balbi
@ 2019-08-15 11:09 ` Robin Murphy
2019-08-17 17:41 ` [PATCH] usb: dwc3: Add shutdown to platform_driver Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Robin Murphy @ 2019-08-15 11:09 UTC (permalink / raw)
To: Felipe Balbi, Vicente Bergas
Cc: Matthias Brugger, Heiko Stuebner, Marc Zyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-kernel, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
On 15/08/2019 07:06, Felipe Balbi wrote:
>
> Hi,
>
> Vicente Bergas <vicencb@gmail.com> 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.
Probing/initialisation has never been the problem. The issue for the
kexec case is that when the first kernel shuts down, there is currently
nothing to quiesce the controller (since only driver->shutdown gets
called, not driver->remove), and thus (presumably) external USB activity
causes it to keep writing back over the memory where the
descriptors/command ring used to be while the second kernel boots. The
second kernel will eventually probe and reset it appropriately, but by
that time the damage is already done.
Yanking on a hardware reset line when the first kernel shuts down is
certainly one way to stop any memory accesses if such a control is
available, but presumably there's a general software way to gracefully
disable the controller's DMA functions until a subsequent probe can
fully reset it again - I think that would be the preferable solution.
Robin.
> 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?
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-15 11:09 ` Robin Murphy
@ 2019-08-17 17:41 ` Vicente Bergas
2019-08-27 11:45 ` Vicente Bergas
2019-09-19 11:36 ` Vicente Bergas
0 siblings, 2 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-08-17 17:41 UTC (permalink / raw)
To: Felipe Balbi, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, Vicente Bergas, linux-rockchip,
Greg Kroah-Hartman, linux-arm-kernel
Otherwise the device keeps writing to memory after kexec and disturbs
the next kernel.
Signed-off-by: Vicente Bergas <vicencb@gmail.com>
---
drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
1 file changed, 6 insertions(+)
Hi Felipe, Robin,
this version calls 'remove' from 'shutdown' instead of just asserting
a reset because it looks like a cleaner way to stop the device.
Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
fix the issue either.
It has been tested on the sapphire board, a RK3399 platform.
Regards,
Vicenç.
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index bdac3e7d7b18..d5fd45c64901 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
return 0;
}
+static void dwc3_of_simple_shutdown(struct platform_device *pdev)
+{
+ dwc3_of_simple_remove(pdev);
+}
+
static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device *dev)
{
struct dwc3_of_simple *simple = dev_get_drvdata(dev);
@@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
static struct platform_driver dwc3_of_simple_driver = {
.probe = dwc3_of_simple_probe,
.remove = dwc3_of_simple_remove,
+ .shutdown = dwc3_of_simple_shutdown,
.driver = {
.name = "dwc3-of-simple",
.of_match_table = of_dwc3_simple_match,
--
2.22.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-17 17:41 ` [PATCH] usb: dwc3: Add shutdown to platform_driver Vicente Bergas
@ 2019-08-27 11:45 ` Vicente Bergas
2019-08-27 11:53 ` Felipe Balbi
2019-09-19 11:36 ` Vicente Bergas
1 sibling, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-08-27 11:45 UTC (permalink / raw)
To: Felipe Balbi, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
linux-arm-kernel
On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
> Otherwise the device keeps writing to memory after kexec and disturbs
> the next kernel.
>
> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> ---
> drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> Hi Felipe, Robin,
> this version calls 'remove' from 'shutdown' instead of just asserting
> a reset because it looks like a cleaner way to stop the device.
>
> Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
> fix the issue either.
>
> It has been tested on the sapphire board, a RK3399 platform.
>
> Regards,
> Vicenç.
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index bdac3e7d7b18..d5fd45c64901 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + dwc3_of_simple_remove(pdev);
> +}
> +
> static int __maybe_unused
> dwc3_of_simple_runtime_suspend(struct device *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
Hi,
please, can you provide some feedback on this?
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-27 11:45 ` Vicente Bergas
@ 2019-08-27 11:53 ` Felipe Balbi
2019-08-27 12:16 ` Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2019-08-27 11:53 UTC (permalink / raw)
To: Vicente Bergas, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1852 bytes --]
Hi,
Vicente Bergas <vicencb@gmail.com> writes:
> On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
>> Otherwise the device keeps writing to memory after kexec and disturbs
>> the next kernel.
>>
>> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
>> ---
>> drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> Hi Felipe, Robin,
>> this version calls 'remove' from 'shutdown' instead of just asserting
>> a reset because it looks like a cleaner way to stop the device.
>>
>> Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
>> fix the issue either.
>>
>> It has been tested on the sapphire board, a RK3399 platform.
>>
>> Regards,
>> Vicenç.
>>
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
>> b/drivers/usb/dwc3/dwc3-of-simple.c
>> index bdac3e7d7b18..d5fd45c64901 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct
>> platform_device *pdev)
>> return 0;
>> }
>>
>> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
>> +{
>> + dwc3_of_simple_remove(pdev);
>> +}
>> +
>> static int __maybe_unused
>> dwc3_of_simple_runtime_suspend(struct device *dev)
>> {
>> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
>> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
>> static struct platform_driver dwc3_of_simple_driver = {
>> .probe = dwc3_of_simple_probe,
>> .remove = dwc3_of_simple_remove,
>> + .shutdown = dwc3_of_simple_shutdown,
why don't you just have shutdown use the same exact function as remove?
Frankly, though, I still don't fully understand what's going wrong
here. Why is the device still alive during kexec?
cheers
--
balbi
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-27 11:53 ` Felipe Balbi
@ 2019-08-27 12:16 ` Vicente Bergas
2019-09-09 15:07 ` Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-08-27 12:16 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
On Tuesday, August 27, 2019 1:53:04 PM CEST, Felipe Balbi wrote:
> Hi,
>
> Vicente Bergas <vicencb@gmail.com> writes:
>> On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
>>> Otherwise the device keeps writing to memory after kexec and disturbs
>>> the next kernel.
...
>
> why don't you just have shutdown use the same exact function as remove?
> Frankly, though, I still don't fully understand what's going wrong
> here. Why is the device still alive during kexec?
>
> cheers
Hi Felipe,
the remove and shutdown functions have different prototypes, so
shutdown is wrapping remove.
Would it be preferable to cast remove as shutdown?
The issue with kexec is that the device is being used during the livetime
of the first kernel. When the first kernel executes kexec it calls the
shutdown function of drivers (instead of remove). Because of this the dwc3
device keeps doing things like DMA.
While the second kernel is taking over, it gets its memory corrupted with
such DMA accesses from the device. When the second kernel reaches the point
of taking over the dwc3 device, re-initializes it, but it is already too
late. Still worse, if the second kernel did not have the dwc3 driver, it
would get endless memory corruptions.
All in all, devices that can do DMA need to stop doing it on shutdown.
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-27 12:16 ` Vicente Bergas
@ 2019-09-09 15:07 ` Vicente Bergas
2019-10-23 6:31 ` Felipe Balbi
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-09-09 15:07 UTC (permalink / raw)
To: Felipe Balbi, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
linux-arm-kernel
On Tuesday, August 27, 2019 2:16:20 PM CEST, Vicente Bergas wrote:
> On Tuesday, August 27, 2019 1:53:04 PM CEST, Felipe Balbi wrote:
>> Hi,
>>
>> Vicente Bergas <vicencb@gmail.com> writes:
>>> On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
>>>> Otherwise the device keeps writing to memory after kexec and disturbs
>>>> the next kernel.
> ...
>>
>> why don't you just have shutdown use the same exact function as remove?
>> Frankly, though, I still don't fully understand what's going wrong
>> here. Why is the device still alive during kexec?
>>
>> cheers
>
> Hi Felipe,
> the remove and shutdown functions have different prototypes, so
> shutdown is wrapping remove.
> Would it be preferable to cast remove as shutdown?
>
> The issue with kexec is that the device is being used during the livetime
> of the first kernel. When the first kernel executes kexec it calls the
> shutdown function of drivers (instead of remove). Because of this the dwc3
> device keeps doing things like DMA.
> While the second kernel is taking over, it gets its memory corrupted with
> such DMA accesses from the device. When the second kernel reaches the point
> of taking over the dwc3 device, re-initializes it, but it is already too
> late. Still worse, if the second kernel did not have the dwc3 driver, it
> would get endless memory corruptions.
> All in all, devices that can do DMA need to stop doing it on shutdown.
>
> Regards,
> Vicenç.
Hi,
please, can you provide some feedback on this?
Regards,
Vicenç.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-08-17 17:41 ` [PATCH] usb: dwc3: Add shutdown to platform_driver Vicente Bergas
2019-08-27 11:45 ` Vicente Bergas
@ 2019-09-19 11:36 ` Vicente Bergas
1 sibling, 0 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-09-19 11:36 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
Ping?
On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
> Otherwise the device keeps writing to memory after kexec and disturbs
> the next kernel.
>
> Signed-off-by: Vicente Bergas <vicencb@gmail.com>
> ---
> drivers/usb/dwc3/dwc3-of-simple.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> Hi Felipe, Robin,
> this version calls 'remove' from 'shutdown' instead of just asserting
> a reset because it looks like a cleaner way to stop the device.
>
> Calling remove from shutdown in core.c instead of dwc3-of-simple.c does not
> fix the issue either.
>
> It has been tested on the sapphire board, a RK3399 platform.
>
> Regards,
> Vicenç.
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index bdac3e7d7b18..d5fd45c64901 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -133,6 +133,11 @@ static int dwc3_of_simple_remove(struct
> platform_device *pdev)
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + dwc3_of_simple_remove(pdev);
> +}
> +
> static int __maybe_unused
> dwc3_of_simple_runtime_suspend(struct device *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-09-09 15:07 ` Vicente Bergas
@ 2019-10-23 6:31 ` Felipe Balbi
2019-10-24 12:15 ` Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2019-10-23 6:31 UTC (permalink / raw)
To: Vicente Bergas, Robin Murphy
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 3709 bytes --]
Hi,
(sorry for the long delay)
Vicente Bergas <vicencb@gmail.com> writes:
> On Tuesday, August 27, 2019 2:16:20 PM CEST, Vicente Bergas wrote:
>> On Tuesday, August 27, 2019 1:53:04 PM CEST, Felipe Balbi wrote:
>>> Hi,
>>>
>>> Vicente Bergas <vicencb@gmail.com> writes:
>>>> On Saturday, August 17, 2019 7:41:40 PM CEST, Vicente Bergas wrote:
>>>>> Otherwise the device keeps writing to memory after kexec and disturbs
>>>>> the next kernel.
>> ...
>>>
>>> why don't you just have shutdown use the same exact function as remove?
>>> Frankly, though, I still don't fully understand what's going wrong
>>> here. Why is the device still alive during kexec?
>>>
>>> cheers
>>
>> Hi Felipe,
>> the remove and shutdown functions have different prototypes, so
>> shutdown is wrapping remove.
>> Would it be preferable to cast remove as shutdown?
>>
>> The issue with kexec is that the device is being used during the livetime
>> of the first kernel. When the first kernel executes kexec it calls the
>> shutdown function of drivers (instead of remove). Because of this the dwc3
>> device keeps doing things like DMA.
>> While the second kernel is taking over, it gets its memory corrupted with
>> such DMA accesses from the device. When the second kernel reaches the point
>> of taking over the dwc3 device, re-initializes it, but it is already too
>> late. Still worse, if the second kernel did not have the dwc3 driver, it
>> would get endless memory corruptions.
>> All in all, devices that can do DMA need to stop doing it on shutdown.
>>
>> Regards,
>> Vicenç.
>
> Hi,
> please, can you provide some feedback on this?
I meant something like this:
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index bdac3e7d7b18..e64754be47b4 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -110,12 +110,9 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
return ret;
}
-static int dwc3_of_simple_remove(struct platform_device *pdev)
+static void __dwc3_of_simple_teardown(struct dwc3_of_simple *simple)
{
- struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
- struct device *dev = &pdev->dev;
-
- of_platform_depopulate(dev);
+ of_platform_depopulate(simple->dev);
clk_bulk_disable_unprepare(simple->num_clocks, simple->clks);
clk_bulk_put_all(simple->num_clocks, simple->clks);
@@ -126,13 +123,27 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
reset_control_put(simple->resets);
- pm_runtime_disable(dev);
- pm_runtime_put_noidle(dev);
- pm_runtime_set_suspended(dev);
+ pm_runtime_disable(simple->dev);
+ pm_runtime_put_noidle(simple->dev);
+ pm_runtime_set_suspended(simple->dev);
+}
+
+static int dwc3_of_simple_remove(struct platform_device *pdev)
+{
+ struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
+
+ __dwc3_of_simple_teardown(simple);
return 0;
}
+static void dwc3_of_simple_shutdown(struct platform_device *pdev)
+{
+ struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
+
+ __dwc3_of_simple_teardown(simple);
+}
+
static int __maybe_unused dwc3_of_simple_runtime_suspend(struct device *dev)
{
struct dwc3_of_simple *simple = dev_get_drvdata(dev);
@@ -190,6 +201,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
static struct platform_driver dwc3_of_simple_driver = {
.probe = dwc3_of_simple_probe,
.remove = dwc3_of_simple_remove,
+ .shutdown = dwc3_of_simple_shutdown,
.driver = {
.name = "dwc3-of-simple",
.of_match_table = of_dwc3_simple_match,
Can you make sure it works as you intended?
--
balbi
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-10-23 6:31 ` Felipe Balbi
@ 2019-10-24 12:15 ` Vicente Bergas
2019-10-25 10:25 ` Felipe Balbi
0 siblings, 1 reply; 28+ messages in thread
From: Vicente Bergas @ 2019-10-24 12:15 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
On Wednesday, October 23, 2019 8:31:21 AM CEST, Felipe Balbi wrote:
> Hi,
>
> (sorry for the long delay)
>
> Vicente Bergas <vicencb@gmail.com> writes:
>
>> On Tuesday, August 27, 2019 2:16:20 PM CEST, Vicente Bergas wrote: ...
>
> I meant something like this:
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index bdac3e7d7b18..e64754be47b4 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -110,12 +110,9 @@ static int dwc3_of_simple_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> -static int dwc3_of_simple_remove(struct platform_device *pdev)
> +static void __dwc3_of_simple_teardown(struct dwc3_of_simple *simple)
> {
> - struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
> - struct device *dev = &pdev->dev;
> -
> - of_platform_depopulate(dev);
> + of_platform_depopulate(simple->dev);
>
> clk_bulk_disable_unprepare(simple->num_clocks, simple->clks);
> clk_bulk_put_all(simple->num_clocks, simple->clks);
> @@ -126,13 +123,27 @@ static int dwc3_of_simple_remove(struct
> platform_device *pdev)
>
> reset_control_put(simple->resets);
>
> - pm_runtime_disable(dev);
> - pm_runtime_put_noidle(dev);
> - pm_runtime_set_suspended(dev);
> + pm_runtime_disable(simple->dev);
> + pm_runtime_put_noidle(simple->dev);
> + pm_runtime_set_suspended(simple->dev);
> +}
> +
> +static int dwc3_of_simple_remove(struct platform_device *pdev)
> +{
> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
> +
> + __dwc3_of_simple_teardown(simple);
>
> return 0;
> }
>
> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
> +{
> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
> +
> + __dwc3_of_simple_teardown(simple);
> +}
> +
> static int __maybe_unused
> dwc3_of_simple_runtime_suspend(struct device *dev)
> {
> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
> @@ -190,6 +201,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> static struct platform_driver dwc3_of_simple_driver = {
> .probe = dwc3_of_simple_probe,
> .remove = dwc3_of_simple_remove,
> + .shutdown = dwc3_of_simple_shutdown,
> .driver = {
> .name = "dwc3-of-simple",
> .of_match_table = of_dwc3_simple_match,
>
> Can you make sure it works as you intended?
Hi Felipe,
just applied your approach to v5.3.7 and it is working properly.
Thanks,
Vicente.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-10-24 12:15 ` Vicente Bergas
@ 2019-10-25 10:25 ` Felipe Balbi
2019-10-25 10:42 ` Vicente Bergas
0 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2019-10-25 10:25 UTC (permalink / raw)
To: Vicente Bergas
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
hi,
Vicente Bergas <vicencb@gmail.com> writes:
> On Wednesday, October 23, 2019 8:31:21 AM CEST, Felipe Balbi wrote:
>> Hi,
>>
>> (sorry for the long delay)
>>
>> Vicente Bergas <vicencb@gmail.com> writes:
>>
>>> On Tuesday, August 27, 2019 2:16:20 PM CEST, Vicente Bergas wrote: ...
>>
>> I meant something like this:
>>
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c
>> b/drivers/usb/dwc3/dwc3-of-simple.c
>> index bdac3e7d7b18..e64754be47b4 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -110,12 +110,9 @@ static int dwc3_of_simple_probe(struct
>> platform_device *pdev)
>> return ret;
>> }
>>
>> -static int dwc3_of_simple_remove(struct platform_device *pdev)
>> +static void __dwc3_of_simple_teardown(struct dwc3_of_simple *simple)
>> {
>> - struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
>> - struct device *dev = &pdev->dev;
>> -
>> - of_platform_depopulate(dev);
>> + of_platform_depopulate(simple->dev);
>>
>> clk_bulk_disable_unprepare(simple->num_clocks, simple->clks);
>> clk_bulk_put_all(simple->num_clocks, simple->clks);
>> @@ -126,13 +123,27 @@ static int dwc3_of_simple_remove(struct
>> platform_device *pdev)
>>
>> reset_control_put(simple->resets);
>>
>> - pm_runtime_disable(dev);
>> - pm_runtime_put_noidle(dev);
>> - pm_runtime_set_suspended(dev);
>> + pm_runtime_disable(simple->dev);
>> + pm_runtime_put_noidle(simple->dev);
>> + pm_runtime_set_suspended(simple->dev);
>> +}
>> +
>> +static int dwc3_of_simple_remove(struct platform_device *pdev)
>> +{
>> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
>> +
>> + __dwc3_of_simple_teardown(simple);
>>
>> return 0;
>> }
>>
>> +static void dwc3_of_simple_shutdown(struct platform_device *pdev)
>> +{
>> + struct dwc3_of_simple *simple = platform_get_drvdata(pdev);
>> +
>> + __dwc3_of_simple_teardown(simple);
>> +}
>> +
>> static int __maybe_unused
>> dwc3_of_simple_runtime_suspend(struct device *dev)
>> {
>> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
>> @@ -190,6 +201,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
>> static struct platform_driver dwc3_of_simple_driver = {
>> .probe = dwc3_of_simple_probe,
>> .remove = dwc3_of_simple_remove,
>> + .shutdown = dwc3_of_simple_shutdown,
>> .driver = {
>> .name = "dwc3-of-simple",
>> .of_match_table = of_dwc3_simple_match,
>>
>> Can you make sure it works as you intended?
>
> Hi Felipe,
> just applied your approach to v5.3.7 and it is working properly.
Do you want to send it as a formal patch or shall I do it?
--
balbi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] usb: dwc3: Add shutdown to platform_driver
2019-10-25 10:25 ` Felipe Balbi
@ 2019-10-25 10:42 ` Vicente Bergas
0 siblings, 0 replies; 28+ messages in thread
From: Vicente Bergas @ 2019-10-25 10:42 UTC (permalink / raw)
To: Felipe Balbi
Cc: Matthias Brugger, Heiko Stuebner, MarcZyngier, Catalin Marinas,
linux-usb, Will Deacon, linux-rockchip, Greg Kroah-Hartman,
Robin Murphy, linux-arm-kernel
On Friday, October 25, 2019 12:25:17 PM CEST, Felipe Balbi wrote:
> hi,
>
> Vicente Bergas <vicencb@gmail.com> writes:
>
>> On Wednesday, October 23, 2019 8:31:21 AM CEST, Felipe Balbi wrote: ...
>
> Do you want to send it as a formal patch or shall I do it?
All yours. Thank you for reviewing and proposing this solution.
Regards,
Vicente.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2019-10-25 10:42 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 14:31 kexec on rk3399 Vicente Bergas
2019-07-22 14:46 ` Will Deacon
2019-07-22 16:08 ` Vicente Bergas
2019-07-22 14:47 ` Matthias Brugger
2019-07-22 16:56 ` Vicente Bergas
2019-07-22 14:54 ` Marc Zyngier
2019-07-22 17:05 ` Vicente Bergas
2019-07-22 17:35 ` Robin Murphy
2019-07-22 18:53 ` Vicente Bergas
2019-07-23 13:32 ` Robin Murphy
2019-08-14 12:53 ` Vicente Bergas
2019-08-14 13:06 ` Felipe Balbi
2019-08-14 13:15 ` Vicente Bergas
2019-08-15 6:00 ` Felipe Balbi
2019-08-14 13:12 ` Robin Murphy
2019-08-15 1:15 ` Vicente Bergas
2019-08-15 6:06 ` Felipe Balbi
2019-08-15 11:09 ` Robin Murphy
2019-08-17 17:41 ` [PATCH] usb: dwc3: Add shutdown to platform_driver Vicente Bergas
2019-08-27 11:45 ` Vicente Bergas
2019-08-27 11:53 ` Felipe Balbi
2019-08-27 12:16 ` Vicente Bergas
2019-09-09 15:07 ` Vicente Bergas
2019-10-23 6:31 ` Felipe Balbi
2019-10-24 12:15 ` Vicente Bergas
2019-10-25 10:25 ` Felipe Balbi
2019-10-25 10:42 ` Vicente Bergas
2019-09-19 11:36 ` Vicente Bergas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).