* [PATCH] Revert "reset: microchip-sparx5: allow building as a module" @ 2022-07-13 8:40 Philipp Zabel 2022-07-13 9:03 ` Michael Walle 0 siblings, 1 reply; 9+ messages in thread From: Philipp Zabel @ 2022-07-13 8:40 UTC (permalink / raw) To: linux-kernel Cc: linux-arm-kernel, Lars Povlsen, Michael Walle, Steen Hegelund, Clément Léger, Philipp Zabel This reverts commit b6b9585876da018bdde2d5f15d206a689c0d70f3. This breaks MDIO on kswitch-d10, presumably because the global switch reset is not released early enough anymore. Reported-by: Michael Walle <michael@walle.cc> Cc: Clément Léger <clement.leger@bootlin.com> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/reset/Kconfig | 2 +- drivers/reset/reset-microchip-sparx5.c | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index d9a08ec343e2..f9a7cee01659 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -115,7 +115,7 @@ config RESET_LPC18XX This enables the reset controller driver for NXP LPC18xx/43xx SoCs. config RESET_MCHP_SPARX5 - tristate "Microchip Sparx5 reset driver" + bool "Microchip Sparx5 reset driver" depends on ARCH_SPARX5 || SOC_LAN966 || COMPILE_TEST default y if SPARX5_SWITCH select MFD_SYSCON diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c index 3d54dda3593e..00b612a0effa 100644 --- a/drivers/reset/reset-microchip-sparx5.c +++ b/drivers/reset/reset-microchip-sparx5.c @@ -149,7 +149,6 @@ static const struct of_device_id mchp_sparx5_reset_of_match[] = { }, { } }; -MODULE_DEVICE_TABLE(of, mchp_sparx5_reset_of_match); static struct platform_driver mchp_sparx5_reset_driver = { .probe = mchp_sparx5_reset_probe, @@ -159,7 +158,12 @@ static struct platform_driver mchp_sparx5_reset_driver = { }, }; -module_platform_driver(mchp_sparx5_reset_driver); +static int __init mchp_sparx5_reset_init(void) +{ + return platform_driver_register(&mchp_sparx5_reset_driver); +} + +postcore_initcall(mchp_sparx5_reset_init); MODULE_DESCRIPTION("Microchip Sparx5 switch reset driver"); MODULE_AUTHOR("Steen Hegelund <steen.hegelund@microchip.com>"); -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 8:40 [PATCH] Revert "reset: microchip-sparx5: allow building as a module" Philipp Zabel @ 2022-07-13 9:03 ` Michael Walle 2022-07-13 9:40 ` Steen Hegelund 0 siblings, 1 reply; 9+ messages in thread From: Michael Walle @ 2022-07-13 9:03 UTC (permalink / raw) To: Philipp Zabel Cc: linux-kernel, linux-arm-kernel, Lars Povlsen, Steen Hegelund, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri [+ Claudiu, Kavyasree ] Am 2022-07-13 10:40, schrieb Philipp Zabel: > This reverts commit b6b9585876da018bdde2d5f15d206a689c0d70f3. > > This breaks MDIO on kswitch-d10, presumably because the global switch > reset is not released early enough anymore. > > Reported-by: Michael Walle <michael@walle.cc> > Cc: Clément Léger <clement.leger@bootlin.com> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> Thanks! Tested-by: Michael Walle <michael@walle.cc> And maybe Microchip can chime in here and tell us what subsystems in the SoC will actually be reset by this. I fear this reset will affect almost every part of the SoC. So it would have to be bound to every single device? Or maybe adding that reset to the switch driver was a mistake in the first place? I mean, if it wouldn't be for the guard bit, it would also reset the CPU core.. -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 9:03 ` Michael Walle @ 2022-07-13 9:40 ` Steen Hegelund 2022-07-13 9:52 ` Michael Walle 0 siblings, 1 reply; 9+ messages in thread From: Steen Hegelund @ 2022-07-13 9:40 UTC (permalink / raw) To: Michael Walle, Philipp Zabel Cc: linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri Hi Michael, I am afraid that the exact list of affected modules is not available, so using the RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of resetting as much as possible, and still continue execution. This is what the Sparx5 datasheet has to say about the SYS_RST_PROT_VCORE protect bit: The device can be soft-reset by writing SOFT_RST.SOFT_CHIP_RST. The VCore system and CPU can be protected from a device soft-reset by writing RESET_PROT_STAT.SYS_RST_PROT_VCORE = 1 before initiating soft-reset. In this case, a chip-level soft reset is applied to all other blocks except the VCore system and the VCore CPU. When protecting the VCore system and CPU from a soft reset, the frame DMA must be disabled prior to a chip-level soft reset. The SERDES and PLL blocks can be protected from reset by writing to SOFT_RST.SOFT_SWC_RST instead of SOFT_CHIP_RST. The VCore general purpose registers (CPU::GPR) and GPIO alternate modes (DEVCPU_GCB::GPIO_ALT) are not affected by a soft-reset. These registers are only reset when an external reset is asserted. BR Steen On Wed, 2022-07-13 at 11:03 +0200, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > [+ Claudiu, Kavyasree ] > Am 2022-07-13 10:40, schrieb Philipp Zabel: > > This reverts commit b6b9585876da018bdde2d5f15d206a689c0d70f3. > > > > This breaks MDIO on kswitch-d10, presumably because the global switch > > reset is not released early enough anymore. > > > > Reported-by: Michael Walle <michael@walle.cc> > > Cc: Clément Léger <clement.leger@bootlin.com> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > Thanks! > > Tested-by: Michael Walle <michael@walle.cc> > > And maybe Microchip can chime in here and tell us what > subsystems in the SoC will actually be reset by this. > I fear this reset will affect almost every part of the > SoC. So it would have to be bound to every single > device? Or maybe adding that reset to the switch driver > was a mistake in the first place? > > I mean, if it wouldn't be for the guard bit, it would > also reset the CPU core.. > > -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 9:40 ` Steen Hegelund @ 2022-07-13 9:52 ` Michael Walle 2022-07-13 12:08 ` Philipp Zabel 0 siblings, 1 reply; 9+ messages in thread From: Michael Walle @ 2022-07-13 9:52 UTC (permalink / raw) To: Steen Hegelund Cc: Philipp Zabel, linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur [+ Horatiu, I missed you earlier, sorry] Hi Steen, Am 2022-07-13 11:40, schrieb Steen Hegelund: > I am afraid that the exact list of affected modules is not available, > so using the > RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of > resetting as much as possible, and > still continue execution. Mh, you are designing that chip (at least the LAN966x) no? Shouldn't that information be available anywhere at Microchip? ;) Anyway, it looks like almost the whole chip is reset except some minor things. So the driver has actually a wrong name. Until recently only the switch driver was the sole user of it (at least on the lan966x). So, my question remains, is this correct? I mean the switch driver says, "reset the switch core", but what actually happens is that the the entire SoC except the CPU and maybe the io mux is reset. What about the watchdog for example? Will that be reset, too? -michael > This is what the Sparx5 datasheet has to say about the > SYS_RST_PROT_VCORE protect bit: > > The device can be soft-reset by writing SOFT_RST.SOFT_CHIP_RST. The > VCore system and CPU can be > protected from a device soft-reset by writing > RESET_PROT_STAT.SYS_RST_PROT_VCORE = 1 before > initiating soft-reset. > > In this case, a chip-level soft reset is applied to all other blocks > except the VCore system and the > VCore CPU. When protecting the VCore system and CPU from a soft reset, > the frame DMA must be > disabled prior to a chip-level soft reset. The SERDES and PLL blocks > can be protected from reset by > writing to SOFT_RST.SOFT_SWC_RST instead of SOFT_CHIP_RST. > > The VCore general purpose registers (CPU::GPR) and GPIO alternate > modes (DEVCPU_GCB::GPIO_ALT) are > not affected by a soft-reset. These registers are only reset when an > external reset is asserted. > > BR > Steen > > On Wed, 2022-07-13 at 11:03 +0200, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> [+ Claudiu, Kavyasree ] >> Am 2022-07-13 10:40, schrieb Philipp Zabel: >> > This reverts commit b6b9585876da018bdde2d5f15d206a689c0d70f3. >> > >> > This breaks MDIO on kswitch-d10, presumably because the global switch >> > reset is not released early enough anymore. >> > >> > Reported-by: Michael Walle <michael@walle.cc> >> > Cc: Clément Léger <clement.leger@bootlin.com> >> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> >> Thanks! >> >> Tested-by: Michael Walle <michael@walle.cc> >> >> And maybe Microchip can chime in here and tell us what >> subsystems in the SoC will actually be reset by this. >> I fear this reset will affect almost every part of the >> SoC. So it would have to be bound to every single >> device? Or maybe adding that reset to the switch driver >> was a mistake in the first place? >> >> I mean, if it wouldn't be for the guard bit, it would >> also reset the CPU core.. >> >> -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 9:52 ` Michael Walle @ 2022-07-13 12:08 ` Philipp Zabel 2022-08-04 7:53 ` Michael Walle 2022-08-26 11:37 ` Michael Walle 0 siblings, 2 replies; 9+ messages in thread From: Philipp Zabel @ 2022-07-13 12:08 UTC (permalink / raw) To: Michael Walle, Steen Hegelund Cc: linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur Hi, On Mi, 2022-07-13 at 11:52 +0200, Michael Walle wrote: > [+ Horatiu, I missed you earlier, sorry] > > Hi Steen, > > Am 2022-07-13 11:40, schrieb Steen Hegelund: > > I am afraid that the exact list of affected modules is not available, > > so using the > > RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of > > resetting as much as possible, and > > still continue execution. > > Mh, you are designing that chip (at least the LAN966x) no? Shouldn't > that information be available anywhere at Microchip? ;) > > Anyway, it looks like almost the whole chip is reset > except some minor things. So the driver has actually a > wrong name. Until recently only the switch driver was the > sole user of it (at least on the lan966x). So, my question > remains, is this correct? I mean the switch driver says, > "reset the switch core", but what actually happens is that > the the entire SoC except the CPU and maybe the io mux is reset. > What about the watchdog for example? Will that be reset, too? If [1-3] are to be trusted, RESET_PROT_STAT[VCORE_RST_PROT_WDT], which protects the watchdog from soft reset, is not set by default. So yes? There are also AMBA, PCIe, PDBG protection bits against Vcore soft reset in this register, depending on the platform. [1] https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=cpu,cpu_regs,reset_prot_stat [2] https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,cpu_regs,reset_prot_stat [3] https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html?select=cpu,cpu_regs,reset_prot_stat regards Philipp ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 12:08 ` Philipp Zabel @ 2022-08-04 7:53 ` Michael Walle 2022-08-09 10:19 ` Steen Hegelund 2022-08-26 11:37 ` Michael Walle 1 sibling, 1 reply; 9+ messages in thread From: Michael Walle @ 2022-08-04 7:53 UTC (permalink / raw) To: Philipp Zabel Cc: Steen Hegelund, linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur Am 2022-07-13 14:08, schrieb Philipp Zabel: > Hi, > > On Mi, 2022-07-13 at 11:52 +0200, Michael Walle wrote: >> [+ Horatiu, I missed you earlier, sorry] >> >> Hi Steen, >> >> Am 2022-07-13 11:40, schrieb Steen Hegelund: >> > I am afraid that the exact list of affected modules is not available, >> > so using the >> > RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of >> > resetting as much as possible, and >> > still continue execution. >> >> Mh, you are designing that chip (at least the LAN966x) no? Shouldn't >> that information be available anywhere at Microchip? ;) >> >> Anyway, it looks like almost the whole chip is reset >> except some minor things. So the driver has actually a >> wrong name. Until recently only the switch driver was the >> sole user of it (at least on the lan966x). So, my question >> remains, is this correct? I mean the switch driver says, >> "reset the switch core", but what actually happens is that >> the the entire SoC except the CPU and maybe the io mux is reset. >> What about the watchdog for example? Will that be reset, too? > > If [1-3] are to be trusted, RESET_PROT_STAT[VCORE_RST_PROT_WDT], which > protects the watchdog from soft reset, is not set by default. So yes? > > There are also AMBA, PCIe, PDBG protection bits against Vcore soft > reset in this register, depending on the platform. > > [1] > https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=cpu,cpu_regs,reset_prot_stat > [2] > https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,cpu_regs,reset_prot_stat > [3] > https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html?select=cpu,cpu_regs,reset_prot_stat Ping. any news here? -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-08-04 7:53 ` Michael Walle @ 2022-08-09 10:19 ` Steen Hegelund 2022-08-09 10:27 ` Michael Walle 0 siblings, 1 reply; 9+ messages in thread From: Steen Hegelund @ 2022-08-09 10:19 UTC (permalink / raw) To: Michael Walle, Philipp Zabel Cc: linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur Hi Michael, Sorry, but most people have been OOO (including me), so this has delayed the response. The protection bit protects the VCore Shared Bus (SBA) blocks shown on Figure 5-1. VCore System Block Diagram in the Datasheet. So in this case also the watchdog (which is the WDT block). I hope this clarifies the usage. Best Regards Steen On Thu, 2022-08-04 at 09:53 +0200, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-07-13 14:08, schrieb Philipp Zabel: > > Hi, > > > > On Mi, 2022-07-13 at 11:52 +0200, Michael Walle wrote: > > > [+ Horatiu, I missed you earlier, sorry] > > > > > > Hi Steen, > > > > > > Am 2022-07-13 11:40, schrieb Steen Hegelund: > > > > I am afraid that the exact list of affected modules is not available, > > > > so using the > > > > RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of > > > > resetting as much as possible, and > > > > still continue execution. > > > > > > Mh, you are designing that chip (at least the LAN966x) no? Shouldn't > > > that information be available anywhere at Microchip? ;) > > > > > > Anyway, it looks like almost the whole chip is reset > > > except some minor things. So the driver has actually a > > > wrong name. Until recently only the switch driver was the > > > sole user of it (at least on the lan966x). So, my question > > > remains, is this correct? I mean the switch driver says, > > > "reset the switch core", but what actually happens is that > > > the the entire SoC except the CPU and maybe the io mux is reset. > > > What about the watchdog for example? Will that be reset, too? > > > > If [1-3] are to be trusted, RESET_PROT_STAT[VCORE_RST_PROT_WDT], which > > protects the watchdog from soft reset, is not set by default. So yes? > > > > There are also AMBA, PCIe, PDBG protection bits against Vcore soft > > reset in this register, depending on the platform. > > > > [1] > > https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=cpu,cpu_regs,reset_prot_stat > > [2] > > https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,cpu_regs,reset_prot_stat > > [3] > > https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html?select=cpu,cpu_regs,reset_prot_stat > > Ping. any news here? > > -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-08-09 10:19 ` Steen Hegelund @ 2022-08-09 10:27 ` Michael Walle 0 siblings, 0 replies; 9+ messages in thread From: Michael Walle @ 2022-08-09 10:27 UTC (permalink / raw) To: Steen Hegelund Cc: Philipp Zabel, linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur Hi Steen, Am 2022-08-09 12:19, schrieb Steen Hegelund: > Sorry, but most people have been OOO (including me), so this has > delayed the response. > > The protection bit protects the VCore Shared Bus (SBA) blocks shown on > Figure 5-1. VCore System > Block Diagram in the Datasheet. So in this case also the watchdog > (which is the WDT block). > > I hope this clarifies the usage. Actually, it does not. The watchdog was just an example. I'm questioning the use of this reset for the switch block. Esp. as it turns out that not even microchip knows what blocks are actually reset. This reset was first introduced for the DSA switch, since then more and more spurious errors were discovered and it turns out all of these errors was because the "switch" reset had some severe side effects, like resetting the GPIO core and thus turning all the GPIOs to input mode which then took some peripherals into reset. -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Revert "reset: microchip-sparx5: allow building as a module" 2022-07-13 12:08 ` Philipp Zabel 2022-08-04 7:53 ` Michael Walle @ 2022-08-26 11:37 ` Michael Walle 1 sibling, 0 replies; 9+ messages in thread From: Michael Walle @ 2022-08-26 11:37 UTC (permalink / raw) To: Philipp Zabel Cc: Steen Hegelund, linux-kernel, linux-arm-kernel, Lars Povlsen, Clément Léger, Claudiu Beznea, Kavyasree Kotagiri, Horatiu Vultur Hi, Am 2022-07-13 14:08, schrieb Philipp Zabel: > On Mi, 2022-07-13 at 11:52 +0200, Michael Walle wrote: >> [+ Horatiu, I missed you earlier, sorry] >> >> Hi Steen, >> >> Am 2022-07-13 11:40, schrieb Steen Hegelund: >> > I am afraid that the exact list of affected modules is not available, >> > so using the >> > RESET_PROT_STAT.SYS_RST_PROT_VCORE bit is the best known way of >> > resetting as much as possible, and >> > still continue execution. >> >> Mh, you are designing that chip (at least the LAN966x) no? Shouldn't >> that information be available anywhere at Microchip? ;) >> >> Anyway, it looks like almost the whole chip is reset >> except some minor things. So the driver has actually a >> wrong name. Until recently only the switch driver was the >> sole user of it (at least on the lan966x). So, my question >> remains, is this correct? I mean the switch driver says, >> "reset the switch core", but what actually happens is that >> the the entire SoC except the CPU and maybe the io mux is reset. >> What about the watchdog for example? Will that be reset, too? > > If [1-3] are to be trusted, RESET_PROT_STAT[VCORE_RST_PROT_WDT], which > protects the watchdog from soft reset, is not set by default. So yes? > > There are also AMBA, PCIe, PDBG protection bits against Vcore soft > reset in this register, depending on the platform. *But* this also prevents it from reset by the watchdog. I don't know if we want that?! I.e. what happens if one sets RESET_PROT_STAT[VCORE_RST_PROT_WDT] and the watchdog does a reset? OTHO, I guess it is also bad to reset the watchdog during boot.. IMHO this reset logic doesn't look that well designed. > [1] > https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=cpu,cpu_regs,reset_prot_stat > [2] > https://microchip-ung.github.io/lan9662_reginfo/reginfo_LAN9662.html?select=cpu,cpu_regs,reset_prot_stat > [3] > https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html?select=cpu,cpu_regs,reset_prot_stat -michael ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-26 11:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-13 8:40 [PATCH] Revert "reset: microchip-sparx5: allow building as a module" Philipp Zabel 2022-07-13 9:03 ` Michael Walle 2022-07-13 9:40 ` Steen Hegelund 2022-07-13 9:52 ` Michael Walle 2022-07-13 12:08 ` Philipp Zabel 2022-08-04 7:53 ` Michael Walle 2022-08-09 10:19 ` Steen Hegelund 2022-08-09 10:27 ` Michael Walle 2022-08-26 11:37 ` Michael Walle
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).