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

[Adam Ford]

On Tue, Feb 1, 2022 at 5:22 AM Marek Vasut <marex at denx.de> wrote:
>
> On 2/1/22 12:16, Adam Ford wrote:
> > On Mon, Jan 31, 2022 at 9:18 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> 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 ?
> >
> > For me, patch 1/4 fixed the issue of not being able to define a fixed
> > environment location.  It now sits where I put it.  It also allows me
> > to boot over USB without having to define ENV_IS_NOWHERE.
>
> Sure, it also likely breaks every other MX8M board which does not define
> that env_get_location() on board level, i.e. every MX8M board.

env_get_location is defined in env/env.c