Re: [U-Boot] [PATCH 2/4] arm: mvebu: Add Marvell's integrated CPUs

2019-02-15 Thread Stefan Roese

Hi Chris,

On 15.02.19 10:41, Chris Packham wrote:

Marvell's switch chips with integrated CPUs (collectively referred to as
MSYS) share common ancestry with the Armada SoCs. Some of the IP blocks
(e.g. xor) are located at different addresses and DFX server exists as a
separate target on the MBUS (on Armada-38x it's just part of the core
complex registers).

Signed-off-by: Chris Packham 
---

  arch/arm/mach-mvebu/Kconfig   | 18 -
  arch/arm/mach-mvebu/Makefile  |  1 +
  arch/arm/mach-mvebu/cpu.c | 32 +--
  arch/arm/mach-mvebu/include/mach/config.h |  2 +-
  arch/arm/mach-mvebu/include/mach/cpu.h|  3 +++
  arch/arm/mach-mvebu/include/mach/soc.h| 20 ++
  drivers/ddr/marvell/axp/xor_regs.h|  4 +++
  7 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 7dda04e0e34e..05aa2ade0499 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -46,7 +46,7 @@ config ARMADA_8K
  # Armada PLL frequency (used for NAND clock generation)
  config SYS_MVEBU_PLL_CLOCK
int
-   default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K
+   default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K || MSYS


I personally find this "MSYS" abbreviation quite short and not
descriptive. How is this handled (if at all yet) in Linux?


default "10" if ARMADA_38X || ARMADA_375
  
  # Armada XP/38x SoC types...

@@ -63,6 +63,22 @@ config MV78460
bool
select ARMADA_XP
  
+config MSYS

+   bool
+   select ARMADA_32BIT
+
+config 98DX4251
+   bool
+   select MSYS
+
+config 98DX3336
+   bool
+   select MSYS
+
+config 98DX3236
+   bool
+   select MSYS
+
  config 88F6820
bool
select ARMADA_38X
diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index ee2eca913484..e7f1c59e6351 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -24,6 +24,7 @@ ifndef CONFIG_SPL_BUILD
  obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
  obj-$(CONFIG_ARMADA_38X) += ../../../drivers/ddr/marvell/a38x/xor.o
  obj-$(CONFIG_ARMADA_XP) += ../../../drivers/ddr/marvell/axp/xor.o
+obj-$(CONFIG_MSYS) += ../../../drivers/ddr/marvell/axp/xor.o
  obj-$(CONFIG_MVEBU_EFUSE) += efuse.o
  
  extra-y += kwbimage.cfg

diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
index 919d05c88c77..e80f9a86c483 100644
--- a/arch/arm/mach-mvebu/cpu.c
+++ b/arch/arm/mach-mvebu/cpu.c
@@ -23,6 +23,12 @@ static struct mbus_win windows[] = {
/* NOR */
{ MBUS_BOOTROM_BASE, MBUS_BOOTROM_SIZE,
  CPU_TARGET_DEVICEBUS_BOOTROM_SPI, CPU_ATTR_BOOTROM },
+
+#ifdef CONFIG_MSYS
+   /* DFX */
+   { MBUS_DFX_BASE, MBUS_DFX_SIZE,
+ CPU_TARGET_DFX, 0 },


Nitpicking: Doesn't this fit into one single line?


+#endif
  };
  
  void lowlevel_init(void)

@@ -121,6 +127,14 @@ static const struct sar_freq_modes sar_freq_tab[] = {
{ 0x13,  0x0, 2000, 1000, 933 },
{ 0xff, 0xff,0,0,   0 } /* 0xff marks end of array */
  };
+#elif defined(CONFIG_MSYS)
+static const struct sar_freq_modes sar_freq_tab[] = {
+   {  0x0, 0x0,  400,  400, 400 },
+   {  0x2, 0x0,  667,  333, 667 },
+   {  0x3, 0x0,  800,  400, 800 },
+   {  0x5, 0x0,  800,  400, 800 },
+   { 0xff, 0xff,0,   0,   0 }  /* 0xff marks end of array */
+};
  #else
  /* SAR frequency values for Armada XP */
  static const struct sar_freq_modes sar_freq_tab[] = {
@@ -144,7 +158,7 @@ void get_sar_freq(struct sar_freq_modes *sar_freq)
u32 freq;
int i;
  
-#if defined(CONFIG_ARMADA_375)

+#if defined(CONFIG_ARMADA_375) || defined(CONFIG_MSYS)
val = readl(CONFIG_SAR2_REG);   /* SAR - Sample At Reset */
  #else
val = readl(CONFIG_SAR_REG);/* SAR - Sample At Reset */
@@ -160,7 +174,7 @@ void get_sar_freq(struct sar_freq_modes *sar_freq)
  #endif
for (i = 0; sar_freq_tab[i].val != 0xff; i++) {
if (sar_freq_tab[i].val == freq) {
-#if defined(CONFIG_ARMADA_375) || defined(CONFIG_ARMADA_38X)
+#if defined(CONFIG_ARMADA_375) || defined(CONFIG_ARMADA_38X) || defined 
(CONFIG_MSYS)
*sar_freq = sar_freq_tab[i];
return;
  #else
@@ -270,6 +284,20 @@ int print_cpuinfo(void)
}
}
  
+	if (mvebu_soc_family() == MVEBU_SOC_MSYS) {

+   switch (revid) {
+   case 3:
+   puts("A0");
+   break;
+   case 4:
+   puts("A1");
+   break;
+   default:
+   printf("?? (%x)", revid);
+   break;
+   }
+   }
+
get_sar_freq(&sar_freq);
printf(" at %d MHz\n", sar_freq.p_clk);
  
diff --git a/arch/arm/mach-mvebu/include/mac

Re: [U-Boot] [PATCH 2/4] arm: mvebu: Add Marvell's integrated CPUs

2019-02-15 Thread Chris Packham
On Fri, 15 Feb 2019, 11:06 PM Stefan Roese  Hi Chris,
>
> On 15.02.19 10:41, Chris Packham wrote:
> > Marvell's switch chips with integrated CPUs (collectively referred to as
> > MSYS) share common ancestry with the Armada SoCs. Some of the IP blocks
> > (e.g. xor) are located at different addresses and DFX server exists as a
> > separate target on the MBUS (on Armada-38x it's just part of the core
> > complex registers).
> >
> > Signed-off-by: Chris Packham 
> > ---
> >
> >   arch/arm/mach-mvebu/Kconfig   | 18 -
> >   arch/arm/mach-mvebu/Makefile  |  1 +
> >   arch/arm/mach-mvebu/cpu.c | 32 +--
> >   arch/arm/mach-mvebu/include/mach/config.h |  2 +-
> >   arch/arm/mach-mvebu/include/mach/cpu.h|  3 +++
> >   arch/arm/mach-mvebu/include/mach/soc.h| 20 ++
> >   drivers/ddr/marvell/axp/xor_regs.h|  4 +++
> >   7 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 7dda04e0e34e..05aa2ade0499 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -46,7 +46,7 @@ config ARMADA_8K
> >   # Armada PLL frequency (used for NAND clock generation)
> >   config SYS_MVEBU_PLL_CLOCK
> >   int
> > - default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K
> > + default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K ||
> MSYS
>
> I personally find this "MSYS" abbreviation quite short and not
> descriptive. How is this handled (if at all yet) in Linux?
>

I did briefly consider ARMADA_MSYS. But settled on MSYS because that's how
Marvell tend to refer to it. Marvells code uses MSYS, XCAT3, AC3 and BC2
depending on how specific they need to be (e.g. the ddr code needs to
distinguish AC3 and BC2).

In Linux I originally proposed MSYS but eventually we went with MV98DX3326
as the base and used the other chip names where needed. But that mostly
works out because the arch code is generic so the things that use these
names are mostly compat strings.


> >   default "10" if ARMADA_38X || ARMADA_375
> >
> >   # Armada XP/38x SoC types...
> > @@ -63,6 +63,22 @@ config MV78460
> >   bool
> >   select ARMADA_XP
> >
> > +config MSYS
> > + bool
> > + select ARMADA_32BIT
> > +
> > +config 98DX4251
> > + bool
> > + select MSYS
> > +
> > +config 98DX3336
> > + bool
> > + select MSYS
> > +
> > +config 98DX3236
> > + bool
> > + select MSYS
> > +
> >   config 88F6820
> >   bool
> >   select ARMADA_38X
> > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
> > index ee2eca913484..e7f1c59e6351 100644
> > --- a/arch/arm/mach-mvebu/Makefile
> > +++ b/arch/arm/mach-mvebu/Makefile
> > @@ -24,6 +24,7 @@ ifndef CONFIG_SPL_BUILD
> >   obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o
> >   obj-$(CONFIG_ARMADA_38X) += ../../../drivers/ddr/marvell/a38x/xor.o
> >   obj-$(CONFIG_ARMADA_XP) += ../../../drivers/ddr/marvell/axp/xor.o
> > +obj-$(CONFIG_MSYS) += ../../../drivers/ddr/marvell/axp/xor.o
> >   obj-$(CONFIG_MVEBU_EFUSE) += efuse.o
> >
> >   extra-y += kwbimage.cfg
> > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> > index 919d05c88c77..e80f9a86c483 100644
> > --- a/arch/arm/mach-mvebu/cpu.c
> > +++ b/arch/arm/mach-mvebu/cpu.c
> > @@ -23,6 +23,12 @@ static struct mbus_win windows[] = {
> >   /* NOR */
> >   { MBUS_BOOTROM_BASE, MBUS_BOOTROM_SIZE,
> > CPU_TARGET_DEVICEBUS_BOOTROM_SPI, CPU_ATTR_BOOTROM },
> > +
> > +#ifdef CONFIG_MSYS
> > + /* DFX */
> > + { MBUS_DFX_BASE, MBUS_DFX_SIZE,
> > +   CPU_TARGET_DFX, 0 },
>
> Nitpicking: Doesn't this fit into one single line?
>

Will fix.


> > +#endif
> >   };
> >
> >   void lowlevel_init(void)
> > @@ -121,6 +127,14 @@ static const struct sar_freq_modes sar_freq_tab[] =
> {
> >   { 0x13,  0x0, 2000, 1000, 933 },
> >   { 0xff, 0xff,0,0,   0 } /* 0xff marks end of array */
> >   };
> > +#elif defined(CONFIG_MSYS)
> > +static const struct sar_freq_modes sar_freq_tab[] = {
> > + {  0x0, 0x0,  400,  400, 400 },
> > + {  0x2, 0x0,  667,  333, 667 },
> > + {  0x3, 0x0,  800,  400, 800 },
> > + {  0x5, 0x0,  800,  400, 800 },
> > + { 0xff, 0xff,0,   0,   0 }  /* 0xff marks end of array */
> > +};
> >   #else
> >   /* SAR frequency values for Armada XP */
> >   static const struct sar_freq_modes sar_freq_tab[] = {
> > @@ -144,7 +158,7 @@ void get_sar_freq(struct sar_freq_modes *sar_freq)
> >   u32 freq;
> >   int i;
> >
> > -#if defined(CONFIG_ARMADA_375)
> > +#if defined(CONFIG_ARMADA_375) || defined(CONFIG_MSYS)
> >   val = readl(CONFIG_SAR2_REG);   /* SAR - Sample At Reset */
> >   #else
> >   val = readl(CONFIG_SAR_REG);/* SAR - Sample At Reset */
> > @@ -160,7 +174,7 @@ void get_sar_freq(struct sar_freq_modes *sar_freq)
> >   #endif
> >   for (i = 0; sar_freq_tab[i]

Re: [U-Boot] [PATCH 2/4] arm: mvebu: Add Marvell's integrated CPUs

2019-02-15 Thread Stefan Roese

Hi Chis,

On 15.02.19 12:57, Chris Packham wrote:



On Fri, 15 Feb 2019, 11:06 PM Stefan Roese mailto:s...@denx.de> 
wrote:

Hi Chris,

On 15.02.19 10:41, Chris Packham wrote:
 > Marvell's switch chips with integrated CPUs (collectively referred to as
 > MSYS) share common ancestry with the Armada SoCs. Some of the IP blocks
 > (e.g. xor) are located at different addresses and DFX server exists as a
 > separate target on the MBUS (on Armada-38x it's just part of the core
 > complex registers).
 >
 > Signed-off-by: Chris Packham mailto:judge.pack...@gmail.com>>
 > ---
 >
 >   arch/arm/mach-mvebu/Kconfig               | 18 -
 >   arch/arm/mach-mvebu/Makefile              |  1 +
 >   arch/arm/mach-mvebu/cpu.c                 | 32 +--
 >   arch/arm/mach-mvebu/include/mach/config.h |  2 +-
 >   arch/arm/mach-mvebu/include/mach/cpu.h    |  3 +++
 >   arch/arm/mach-mvebu/include/mach/soc.h    | 20 ++
 >   drivers/ddr/marvell/axp/xor_regs.h        |  4 +++
 >   7 files changed, 76 insertions(+), 4 deletions(-)
 >
 > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
 > index 7dda04e0e34e..05aa2ade0499 100644
 > --- a/arch/arm/mach-mvebu/Kconfig
 > +++ b/arch/arm/mach-mvebu/Kconfig
 > @@ -46,7 +46,7 @@ config ARMADA_8K
 >   # Armada PLL frequency (used for NAND clock generation)
 >   config SYS_MVEBU_PLL_CLOCK
 >       int
 > -     default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K
 > +     default "20" if ARMADA_XP || ARMADA_3700 || ARMADA_8K || 
MSYS

I personally find this "MSYS" abbreviation quite short and not
descriptive. How is this handled (if at all yet) in Linux?


I did briefly consider ARMADA_MSYS. But settled on MSYS because that's
how Marvell tend to refer to it. Marvells code uses MSYS, XCAT3, AC3
and BC2 depending on how specific they need to be (e.g. the ddr code
needs to distinguish AC3 and BC2).


I see. But now CONFIG_MSYS is mostly used vs CONFIG_ARMADA_38X etc.
I personally find this now clear and would prefer CONFIG_ARMADA_MSYS
over CONFIG_MSYS. You can always define CONFIG_MSYS in some DDR code
header, once this should be integrated.
 

In Linux I originally proposed MSYS but eventually we went with
MV98DX3326 as the base and used the other chip names where needed.
But that mostly works out because the arch code is generic so the
things that use these names are mostly compat strings.


I see. Isn't there any CONFIG_MACH_foo set for this SoC type (like
CONFIG_MACH_ARMADA_38X) in Linux? Or is CONFIG_MACH_ARMADA_XP selected
for this SoC as well?

Still, if we need to make this distinction in U-Boot (compared to the
more generic arch code in Linux), I would prefer CONFIG_ARMADA_MSYS
compared to CONFIG_MSYS or CONFIG_MV98DX3326.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot