[PATCH v4 4/4] phytec: phycore_imx8mp: override env_get_location

[Marek Vasut]

On 1/31/22 23:15, Tommaso Merciai wrote:
> On Mon, Jan 31, 2022 at 06:03:58PM +0100, Marek Vasut wrote:
>> On 1/31/22 17:58, Tommaso Merciai wrote:
>>> Override env_get_location function at board level, previously dropped
>>> down from arch/arm/mach-imx/imx8m/soc.c
>>>
>>> References:
>>>    - commit 98af80d3c969e69a1b8ce98bb20e5ad844022da2
>>>
>>> Signed-off-by: Tommaso Merciai <tommaso.merciai at amarulasolutions.com>
>>> ---
>>>    board/phytec/phycore_imx8mp/phycore-imx8mp.c | 33 ++++++++++++++++++++
>>>    1 file changed, 33 insertions(+)
>>>
>>> diff --git a/board/phytec/phycore_imx8mp/phycore-imx8mp.c b/board/phytec/phycore_imx8mp/phycore-imx8mp.c
>>> index a8f0821437..05926eefa3 100644
>>> --- a/board/phytec/phycore_imx8mp/phycore-imx8mp.c
>>> +++ b/board/phytec/phycore_imx8mp/phycore-imx8mp.c
>>> @@ -11,9 +11,42 @@
>>>    #include <asm/mach-imx/boot_mode.h>
>>>    #include <env.h>
>>>    #include <miiphy.h>
>>> +#include <env_internal.h>
>>>    DECLARE_GLOBAL_DATA_PTR;
>>> +enum env_location env_get_location(enum env_operation op, int prio)
>>> +{
>>
>> Why don't you just turn this into default __weak function and override it on
>> board level when it is really needed to be overridden ?
> 
> Hi Marek,
> env_get_location is already declared as __weak, check env/env.c. We
> can't override it 2 times.

Oh, it is this problem with missing ability to define multiple levels of 
symbol strength.

__weak enum env_location arch_env_get_location(enum env_operation op, 
int prio)
{
         if (prio >= ARRAY_SIZE(env_locations))
                 return ENVL_UNKNOWN;

         return env_locations[prio];
}

__weak enum env_location board_env_get_location(enum env_operation op, 
int prio)
{
	return arch_env_get_location(op, prio);
}

__weak enum env_location env_get_location(enum env_operation op, int prio)
{
	return board_env_get_location(op, prio);
}

By default, the compiler will optimize it all out. If you have 
arch-specific default (like imx does), implement 
arch_env_get_location(), if you have even board-specific default (like 
your board likely does), implement board_env_get_location(), if you need 
to override both, then override env_get_location() (unlikely).

This is also inline with all the other arch_*() and board_*() functions 
we have, and you won't have much duplication either.