Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot

[Tony Lindgren <tony@atomide.com>]

* Andrew F. Davis <afd@ti.com> [170328 10:55]:
> On 03/28/2017 12:09 PM, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
> >> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> >>> Yes problems still remains. I think the immediate fix there is to
> >>> disable kexec during runtime based on some criteria for your use
> >>> case rather than BUG() though. Somehow kexec needs to know if CPU1
> >>> reset is acceptable, then reset CPU1 before kexec.
> >>
> >> The only acceptable way to do that is to make the decision when loading
> >> the image(s), and refuse to load the image(s) if kexec is not possible.
> >> (As is already done when dealing with whether we can CPU hot-unplug the
> >> secondary CPUs.)
> > 
> > OK so how about the following to disable kexec for Andrew's
> > test case?
> > 
> > That is assuming that omap_type() test works for HS dra7
> > like it does for the other omap2 variants.
> > 
> > Regards,
> > 
> > Tony
> > 
> > 8< ---------------------
> > diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> > --- a/arch/arm/mach-omap2/common.h
> > +++ b/arch/arm/mach-omap2/common.h
> > @@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
> >  extern void omap_auxcoreboot_addr(u32 cpu_addr);
> >  extern u32 omap_read_auxcoreboot0(void);
> >  
> > +extern bool omap4_cpu_can_disable(unsigned int cpu);
> >  extern void omap4_cpu_die(unsigned int cpu);
> >  extern int omap4_cpu_kill(unsigned int cpu);
> >  
> > diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> > --- a/arch/arm/mach-omap2/omap-hotplug.c
> > +++ b/arch/arm/mach-omap2/omap-hotplug.c
> > @@ -22,6 +22,15 @@
> >  #include "omap-wakeupgen.h"
> >  #include "common.h"
> >  #include "powerdomain.h"
> > +#include "soc.h"
> > +
> > +bool omap4_cpu_can_disable(unsigned int cpu)
> > +{
> > +	if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
> > +		return false;
> > +
> 
> NAK!
> 
> HS variants can have aux cores disabled, we are already doing it right
> now, this is another hack to avoid having to fix kexec.

Oh sorry the intention was to not break things for your. So probably
just having a separate dra7_hs_smp_ops with no cpu_kill initialized
should do the trick for kexec.

> You are failing to park cores correctly, so instead of fixing that you
> hack around it on non-HS devices by hard-resetting those cores, now
> because you can't do that hack on HS devices you suggest we disable
> hotplug on HS devices altogether!?

Me failing to park cores? The current code has always assumed that
CPU1 enters a low power state for hot-unplug. And AFAIK, there's no
way to have CPU1 continue at a different address at that point
without a reset. The need for preserving CPU1 state through kexec
boot without a reset is something new that you need.

> Kexec *does* work on HS devices. It only fails if we fail to park a core
> correctly, something that should never happen, the check here must be
> for that, unconditionally disabling Kexec for all HS devices is not a
> valid option.

Well sounds like you have to come up with checks for when we
need to disable kexec and use dra7_hs_smp_ops with no cpu_kill.

Regards,

Tony