[U-Boot] [PATCH v1 2/4] arm: socfpga: fix compiling with OF_PLATDATA

[Simon Goldschmidt]

On Tue, Jan 8, 2019 at 1:05 PM Marek Vasut <marex at denx.de> wrote:
>
> On 1/8/19 7:32 AM, Simon Goldschmidt wrote:
> > On Mon, Jan 7, 2019 at 11:58 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> On 1/7/19 10:14 PM, Simon Goldschmidt wrote:
> >>> 'socfpga_eth_reset_common' must not be compiled when OF_PLATDATA is enabled
> >>> since it uses functions accessing the devicetree.
> >>>
> >>> Since this function is not used in SPL anyway, change the compile guard
> >>> to reflect this and fix compiling SPL with OF_PLATDATA.
> >>
> >> Doesn't this break ethernet in SPL ? I think it does. The real fix is to
> >> move this PHY mode configuration into the driver.
> >
> > Hmm, ethernet in SPL. Haven't thought of that :-)
> >
> > While it could be easy to move the DTS part of  PHY mode configuration into
> > the driver, moving the reset portion of that code into the driver would be ugly
> > unless using proper reset management?
>
> Well, it's still better than having it live in arch/arm/ , which was at
> that point the best option, but right now it's no longer. And I think
> Joyce ran into similar topic with some other Altera IP.

Hmm, ok, I could search for that.

> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>> ---
> >>>
> >>>  arch/arm/mach-socfpga/misc.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> >>> index 78fbe28724..30f74d9940 100644
> >>> --- a/arch/arm/mach-socfpga/misc.c
> >>> +++ b/arch/arm/mach-socfpga/misc.c
> >>> @@ -120,7 +120,7 @@ int arch_cpu_init(void)
> >>>       return 0;
> >>>  }
> >>>
> >>> -#ifdef CONFIG_ETH_DESIGNWARE
> >>> +#if defined CONFIG_ETH_DESIGNWARE && !defined CONFIG_SPL_BUILD
> >
> > Would it work for you to change this into:
> >
> > #if defined CONFIG_ETH_DESIGNWARE && !CONFIG_IS_ENABLED(OF_PLATDATA)
> >
> > We could still fix the reset code when actually using OF_PLATDATA with
> > ethernet...
>
> Is it too much work to fix it properly ?

Well, just moving the functions I commented out into the eth driver would mean
it would at some point call back into arch/arm/mach-socfpga code.

If that is acceptable, it wouldn't be too much work and I could certainly do it.
But the question remains whether such a change would "fix it properly". In my
opinion, it wouldn't. But it would probably still be better than the
current state
where some code in arch/arm parses ETH DTS nodes...

Regards,
Simon