From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Tue, 8 Jan 2019 13:23:38 +0100 Subject: [U-Boot] [PATCH v1 2/4] arm: socfpga: fix compiling with OF_PLATDATA In-Reply-To: References: <20190107211423.10151-1-simon.k.r.goldschmidt@gmail.com> <20190107211423.10151-3-simon.k.r.goldschmidt@gmail.com> <77643167-9ebe-cac6-78fa-0a98151f61e1@denx.de> <62c568b5-b725-16d4-9ad0-afe8ca438c29@denx.de> Message-ID: <5ccade4b-86a4-1218-c2e9-1958f30706fb@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 1/8/19 1:14 PM, Simon Goldschmidt wrote: > On Tue, Jan 8, 2019 at 1:05 PM Marek Vasut wrote: >> >> On 1/8/19 7:32 AM, Simon Goldschmidt wrote: >>> On Mon, Jan 7, 2019 at 11:58 PM Marek Vasut 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 >>>>> --- >>>>> >>>>> 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... Right. I think you can just pull that info from the DT node associated with the DWMAC driver. -- Best regards, Marek Vasut