* 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: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: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: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: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: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 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: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-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: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-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-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
* 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
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).