| Message ID | 20200428142225.2041132-1-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | efa1a62ad2ddbb47dcaed9fdbdd34c315fdcd883 |
| Headers | show |
| Series | [1/5] ARM: imx: Do not define do_reset() if sysreset is enabled | expand |
Hello Marek, On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote: > The SPL can also be compiled with sysreset drivers just fine, so > update the condition to cater for that option. Me and Claudius solved the same problem in a different way a while back (see [1] and [2]). The two approaches overlap but both contain some unique code that is useful. I'll merge the two patchsets and send them anew if that is ok with you. [1]: https://lists.denx.de/pipermail/u-boot/2020-March/401935.html [2]: https://patchwork.ozlabs.org/project/uboot/list/?series=162379 > Signed-off-by: Marek Vasut <marex at denx.de> > Cc: Fabio Estevam <festevam at gmail.com> > Cc: Flavio Suligoi <f.suligoi at asem.it> > Cc: Harald Seiler <hws at denx.de> > Cc: Igor Opaniuk <igor.opaniuk at toradex.com> > Cc: Marcel Ziswiler <marcel.ziswiler at toradex.com> > Cc: Oleksandr Suvorov <oleksandr.suvorov at toradex.com> > Cc: Peng Fan <peng.fan at nxp.com> > Cc: Stefano Babic <sbabic at denx.de> > --- > arch/arm/mach-imx/imx8m/soc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c > index 7fcbd53f30..0f17252e80 100644 > --- a/arch/arm/mach-imx/imx8m/soc.c > +++ b/arch/arm/mach-imx/imx8m/soc.c > @@ -382,7 +382,7 @@ int ft_system_setup(void *blob, bd_t *bd) > } > #endif > > -#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET) > +#if !CONFIG_IS_ENABLED(SYSRESET) > void reset_cpu(ulong addr) > { > struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
On 4/28/20 11:23 PM, Harald Seiler wrote: > Hello Marek, Hi, > On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote: >> The SPL can also be compiled with sysreset drivers just fine, so >> update the condition to cater for that option. > > Me and Claudius solved the same problem in a different way a while back > (see [1] and [2]). > > The two approaches overlap but both contain some unique code that is > useful. I'll merge the two patchsets and send them anew if that is ok > with you. Can you be more specific about what is your plan ? Since the MX8M already has DM enabled in SPL for quite a few things and has enough SRAM space left, I would be inclined to just go with DM-based implementation of the reset over a non-DM one.
Hello Marek, On Wed, 2020-04-29 at 00:26 +0200, Marek Vasut wrote: > On 4/28/20 11:23 PM, Harald Seiler wrote: > > Hello Marek, > > Hi, > > > On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote: > > > The SPL can also be compiled with sysreset drivers just fine, so > > > update the condition to cater for that option. > > > > Me and Claudius solved the same problem in a different way a while back > > (see [1] and [2]). > > > > The two approaches overlap but both contain some unique code that is > > useful. I'll merge the two patchsets and send them anew if that is ok > > with you. > > Can you be more specific about what is your plan ? > > Since the MX8M already has DM enabled in SPL for quite a few things and > has enough SRAM space left, I would be inclined to just go with DM-based > implementation of the reset over a non-DM one. For me personally, the patch I care most about is the first of Claudius and my series ("ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them") [1]. It is necessary to make SPL_USB_SDP_SUPPORT work on the DH imx6 board (which does not use SYSRESET in SPL at the moment). As this patch breaks the imx8m* boards in their present form, we tried fixing them in their current non-DM state in "imx: imx8m*: Remove do_reset from board files" [2] and "imx: imx8m: Don't use the addr parameter of reset_cpu" [3]. These patches are theoretically superseeded by your series which solves the problem much better and more future-proof. But I think it would not hurt to pull in Claudius and my changes as well so the non-DM version isn't lying around, silently broken. Additionally, "imx: imx8m: Don't use the addr parameter of reset_cpu" [3] is groundwork for removing this addr parameter altogether from the whole tree which I want to do after this imx8 story is over. So to summarize, the final state I'd want is this: - "ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them" is applied for my own needs. - No board defines do_reset(). - The imx8m boards use DM_SYSRESET in SPL (from your patches). - The non-DM reset code for imx8 is also fixed, even though it is not used in any mainline board anymore. [1]: https://patchwork.ozlabs.org/project/uboot/patch/62c163018998fcf476f0ad2edf83d1787d69445d.1583328917.git.hws at denx.de/ [2]: https://patchwork.ozlabs.org/project/uboot/patch/25277ba3658920ff3be7464020438070844f05da.1583328917.git.hws at denx.de/ [3]: https://patchwork.ozlabs.org/project/uboot/patch/b31a2cbb66950bff2e09ab07b710d53d9d46cf09.1583328917.git.hws at denx.de/
On 4/29/20 10:57 AM, Harald Seiler wrote: > Hello Marek, Hi, > On Wed, 2020-04-29 at 00:26 +0200, Marek Vasut wrote: >> On 4/28/20 11:23 PM, Harald Seiler wrote: >>> Hello Marek, >> >> Hi, >> >>> On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote: >>>> The SPL can also be compiled with sysreset drivers just fine, so >>>> update the condition to cater for that option. >>> >>> Me and Claudius solved the same problem in a different way a while back >>> (see [1] and [2]). >>> >>> The two approaches overlap but both contain some unique code that is >>> useful. I'll merge the two patchsets and send them anew if that is ok >>> with you. >> >> Can you be more specific about what is your plan ? >> >> Since the MX8M already has DM enabled in SPL for quite a few things and >> has enough SRAM space left, I would be inclined to just go with DM-based >> implementation of the reset over a non-DM one. > > For me personally, the patch I care most about is the first of Claudius > and my series ("ARM: reset: use do_reset in SPL/TPL if SYSRESET was not > enabled for them") [1]. It is necessary to make SPL_USB_SDP_SUPPORT work > on the DH imx6 board (which does not use SYSRESET in SPL at the moment). > > As this patch breaks the imx8m* boards in their present form, we tried > fixing them in their current non-DM state in "imx: imx8m*: Remove do_reset > from board files" [2] and "imx: imx8m: Don't use the addr parameter of > reset_cpu" [3]. These patches are theoretically superseeded by your > series which solves the problem much better and more future-proof. > But I think it would not hurt to pull in Claudius and my changes as well > so the non-DM version isn't lying around, silently broken. > > Additionally, "imx: imx8m: Don't use the addr parameter of reset_cpu" [3] > is groundwork for removing this addr parameter altogether from the whole > tree which I want to do after this imx8 story is over. > > So to summarize, the final state I'd want is this: > > - "ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for > them" is applied for my own needs. > - No board defines do_reset(). > - The imx8m boards use DM_SYSRESET in SPL (from your patches). > - The non-DM reset code for imx8 is also fixed, even though it is not > used in any mainline board anymore. All right, I totally forgot about the MX6 SDP. So go ahead, prepare a series and let's see how that looks.
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 7fcbd53f30..0f17252e80 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -382,7 +382,7 @@ int ft_system_setup(void *blob, bd_t *bd) } #endif -#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET) +#if !CONFIG_IS_ENABLED(SYSRESET) void reset_cpu(ulong addr) { struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
The SPL can also be compiled with sysreset drivers just fine, so update the condition to cater for that option. Signed-off-by: Marek Vasut <marex at denx.de> Cc: Fabio Estevam <festevam at gmail.com> Cc: Flavio Suligoi <f.suligoi at asem.it> Cc: Harald Seiler <hws at denx.de> Cc: Igor Opaniuk <igor.opaniuk at toradex.com> Cc: Marcel Ziswiler <marcel.ziswiler at toradex.com> Cc: Oleksandr Suvorov <oleksandr.suvorov at toradex.com> Cc: Peng Fan <peng.fan at nxp.com> Cc: Stefano Babic <sbabic at denx.de> --- arch/arm/mach-imx/imx8m/soc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)