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

[Marek Vasut]

On 2/1/22 01:20, Adam Ford wrote:
> On Mon, Jan 31, 2022 at 4:16 PM Tommaso Merciai
> <tommaso.merciai at amarulasolutions.com> 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.
> 
> The original version (before it was added in 2707faf01f04
> ("imx8mn/imx8mp: override env_get_offset and env_get_location") is
> located in env/env.c and for my board, this is the preferred method.
> This replacement method actually is the opposite behavior from what I
> want, which is to force the environment to a fixed location regardless
> of the boot device.
> 
> I think Tommaso's method is better, because as it is, the users cannot
> override it any more.

Doesn't 1/4 patch break env on pretty much every single imx8m board ?

See my other reply regarding arch and board level __weak symbols, that's 
likely the way to go here.