Hi Andrey,
On Thu, Feb 09, 2017 at 11:13:25AM -0800, Andrey Smirnov wrote:
> On Tue, Feb 7, 2017 at 12:43 AM, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote:
> > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > ---
> > arch/arm/cpu/psci.c | 13 +++++++++
> > arch/arm/mach-imx/imx7.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 89 insertions(+)
> >
> > diff --git a/arch/arm/cpu/psci.c b/arch/arm/cpu/psci.c
> > index 745b8495e..d650c23ea 100644
> > --- a/arch/arm/cpu/psci.c
> > +++ b/arch/arm/cpu/psci.c
> > @@ -22,6 +22,17 @@
> > #include <magicvar.h>
> >
> > #ifdef CONFIG_ARM_PSCI_DEBUG
> > +
> > +/*
> > + * PSCI debugging functions. Board code can specify a putc() function
> > + * which is used for debugging output. Beware that this function is
> > + * called while the kernel is running. This means the kernel could have
> > + * turned off clocks, configured other baudrates and other stuff that
> > + * might confuse the putc function. So it can well be that the debugging
> > + * code itself is the problem when somethings not working. You have been
> > + * warned.
> > + */
> > +
> > static void (*__putc)(void *ctx, int c);
> > static void *putc_ctx;
> >
> > @@ -220,6 +231,8 @@ int psci_cpu_entry_c(void)
> > if (bootm_arm_security_state() == ARM_STATE_HYP)
> > armv7_switch_to_hyp();
> >
> > + psci_printf("core #%d enter function 0x%p\n", cpu, entry);
> > +
> > entry(context_id);
> >
> > while (1);
> > diff --git a/arch/arm/mach-imx/imx7.c b/arch/arm/mach-imx/imx7.c
> > index 1cd27a0db..c4b9b2815 100644
> > --- a/arch/arm/mach-imx/imx7.c
> > +++ b/arch/arm/mach-imx/imx7.c
> > @@ -92,6 +92,80 @@ static void imx7_init_csu(void)
> > writel(CSU_INIT_SEC_LEVEL0, csu + i * 4);
> > }
> >
> > +#define GPC_CPU_PGC_SW_PDN_REQ 0xfc
> > +#define GPC_CPU_PGC_SW_PUP_REQ 0xf0
> > +#define GPC_PGC_C1 0x840
>
> Maybe convert this to something like:
>
> #define PGC_C(n) (0x800 + (n) * 0x40)
>
> ... more domain offsets if needed ...
>
> #define GPC_PGC_nCTRL(d) ((d) + 0x00)
>
> and use as:
>
> GPC_PGC_nCTRL(PGC_C(1))
>
> ?
>
> > +
> > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 0x2
> > +
> > +/* below is for i.MX7D */
> > +#define SRC_GPR1_MX7D 0x074
> > +#define SRC_A7RCR0 0x004
>
> This constant doesn't seem to be used anywhere, is that intentional?
>
> > +#define SRC_A7RCR1 0x008
> > +
> > +static void imx_gpcv2_set_core1_power(bool pdn)
> > +{
>
> Did you not make this function more generic and take core number as a
> parameter on purpose? It just seems like it would be trivial code
> change, but maybe I am mistaken
>
> > + void __iomem *gpc = IOMEM(MX7_GPC_BASE_ADDR);
> > +
> > + u32 reg = pdn ? GPC_CPU_PGC_SW_PUP_REQ : GPC_CPU_PGC_SW_PDN_REQ;
> > + u32 val;
> > +
> > + writel(1, gpc + GPC_PGC_C1);
>
> GPC_PGC_nCTRL_PCR instead of "1"?
>
> > +
> > + val = readl(gpc + reg);
> > + val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7;
> > + writel(val, gpc + reg);
> > +
> > + while ((readl(gpc + reg) &
> > + BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0)
> > + ;
> > +
> > + writel(0, gpc + GPC_PGC_C1);
> > +}
> > +
> > +static int imx7_cpu_on(u32 cpu_id)
> > +{
> > + void __iomem *src = IOMEM(MX7_SRC_BASE_ADDR);
> > + u32 val;
> > +
> > + writel(psci_cpu_entry, src + cpu_id * 8 + SRC_GPR1_MX7D);
> > + imx_gpcv2_set_core1_power(true);
> > +
> > + val = readl(src + SRC_A7RCR1);
> > + val |= 1 << cpu_id;
>
> BIT(cpu_id)?
>
> > + writel(val, src + SRC_A7RCR1);
>
> Hmm, this function doesn't look like it supports turning on CPU 0, am
> I missing something? If not shouldn't it return NOT_SUPPORTED in case
> cpu_id is 0?
>
> > +
> > + return 0;
> > +}
> > +
> > +static int imx7_cpu_off(void)
> > +{
> > + void __iomem *src = IOMEM(MX7_SRC_BASE_ADDR);
> > + u32 val;
> > + int cpu_id = 1;
> > +
>
> I have only a very brief familiarity with PCSI, so pleasw bear with me
> if what I am asking is dumb, but isn't CPU_OFF operation supposed to
> power off current CPU? This function looks like it will power down
> core 1 regardless of who's executing the code.
This patch is only tested up to the point where the secondary CPU comes
up. In a dual core system we only ever have to enable the CPU1. I didn't
test CPU hotplug, so Linux won't turn off a CPU, be it core 0 or core 1.
I updated the patch to pass the core number around so it *shoul* work,
but I haven't tested it.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
