Re: [PATCH 09/13] MIPS: mscc: Add initial support for Microsemi MIPS SoCs

[James Hogan]

On Wed, Nov 29, 2017 at 05:38:19PM +0100, Alexandre Belloni wrote:
> Hi Paul,
> 
> On 28/11/2017 at 11:50:02 -0800, Paul Burton wrote:
> > On Tue, Nov 28, 2017 at 05:31:51PM +0000, James Hogan wrote:
> > > On Tue, Nov 28, 2017 at 05:53:59PM +0100, Alexandre Belloni wrote:
> > > > On 28/11/2017 at 16:01:38 +0000, James Hogan wrote:
> > > > > On Tue, Nov 28, 2017 at 04:26:39PM +0100, Alexandre Belloni wrote:
> > > > > > Introduce support for the MIPS based Microsemi Ocelot SoCs.
> > > > > > As the plan is to have all SoCs supported only using device tree, the
> > > > > > mach directory is simply called mscc.
> > > > > 
> > > > > Nice. Have you considered adding this to the existing multiplatform
> > > > > "generic" platform? See for example commit b35565bb16a5 ("MIPS: generic:
> > > > > Add support for MIPSfpga") for the latest platform to be converted.
> > > > > 
> > > > 
> > > > I didn't because we are currently booting using an old redboot with its
> > > > own boot protocol and at boot, the register read by the sead3 code is
> > > > completely random (it actually matched once).
> > > > 
> > > > Do you consider that mandatory to get the platform upstream?
> > > 
> > > No, however if it is practical to do so I think it might be the best way
> > > forward (even if generic+YAMON support is mutually exclusive of
> > > generic+redboot, though hopefully there is some way to avoid that).
> > > 
> > > Paul on Cc, he may have thoughts on this one.
> > 
> > We could certainly look at tightening the checks in the SEAD-3 code to
> > avoid the false positive.
> > 
> > Could you share any details of the boot protocol you're using with
> > redboot? One option might be for the SEAD-3 code to check that the
> > arguments the bootloader provided look "YAMON-like", so long as the 2
> > protocols differ sufficiently.
> > 
> 
> I didn't look closely at the redboot code yet but it ends up with
> something like:
>  - argc == fw_arg0
>  - argv == fw_arg1
>     - not sure yet what is in argv[0]
>     - kernel commande line in argv[1]
>  - fw_arg2 is a pointer to a structure like:
>         struct parmblock {
>             t_env_var memsize;
>         };
>     with:
>         typedef struct
>         {
>             char *name;
>             char *val;
>         } t_env_var;
>    this is the size of the RAM but I'm not using it because it is in the
>    device tree.
> 
> Does that help?

That basically matches what YAMON provides. I can't see a nice way to
support both in the same kernel.

Processor ID is no good since Malta (not yet mainline added to
"generic") uses the same address for the ID, and can support a much
bigger range of cores.

Poking at random I/O always feels a bit risky.

Some safety checked environment checking (Paul says modetty0 should
always be in there for YAMON) might work.

Does Ocelot have a read-only ID register with a specific value? We'd
have to add prioritisation of the legacy board detection to rely on
that.

If all else fails, we could still make them mutually exclusive,
something roughly like below would work but its a bit clumsy as all the
ocelot config options would still get enabled when sead3 is enabled,
even though some of the drivers may not be useful. The detection &
co-existence can always be improved later. What do you think?

We can't #require CONFIG_LEGACY_BOARD_SEAD3=n unfortunately since it
only checks the base config, not the already merged board configs.

Cheers
James

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 0f20f84de53b..bfdefc013358 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -537,6 +537,10 @@ generic_defconfig:
 # now that the boards have been converted to use the generic kernel they are
 # wrappers around the generic rules above.
 #
+.PHONY: ocelot_defconfig
+ocelot_defconfig:
+	$(Q)$(MAKE) -f $(srctree)/Makefile 32r2el_defconfig BOARDS=ocelot
+
 .PHONY: sead3_defconfig
 sead3_defconfig:
 	$(Q)$(MAKE) -f $(srctree)/Makefile 32r2el_defconfig BOARDS=sead-3
diff --git a/arch/mips/configs/generic/board-ocelot.config b/arch/mips/configs/generic/board-ocelot.config
new file mode 100644
index 000000000000..b22a4570d05c
--- /dev/null
+++ b/arch/mips/configs/generic/board-ocelot.config
@@ -0,0 +1,3 @@
+# require CONFIG_32BIT=y
+
+CONFIG_LEGACY_BOARD_OCELOT=y
diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
index 52e0286a1612..fac8b936c468 100644
--- a/arch/mips/generic/Kconfig
+++ b/arch/mips/generic/Kconfig
@@ -27,6 +27,14 @@ config LEGACY_BOARD_SEAD3
 	  Enable this to include support for booting on MIPS SEAD-3 FPGA-based
 	  development boards, which boot using a legacy boot protocol.
 
+comment "MSCC Ocelot doesn't work with SEAD3 enabled"
+	depends on LEGACY_BOARD_SEAD3
+
+config LEGACY_BOARD_OCELOT
+	bool "Support MSCC Ocelot boards"
+	depends on LEGACY_BOARD_SEAD3=n
+	select LEGACY_BOARDS
+
 comment "FIT/UHI Boards"
 
 config FIT_IMAGE_FDT_BOSTON

Attachment: signature.asc
Description: Digital signature