mbox series

[RFC,0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

Message ID 20211126174331.5928-1-tomm.merciai@gmail.com
Headers show
Series imx8m: move env_get_location for imx8mn and imx8mp at board level | expand

Message

Tommaso Merciai Nov. 26, 2021, 5:43 p.m. UTC 
This series move env_get_location from soc to board level. As suggested
by Michael <michael@amarulasolutions.com> make no sense to define an 
unique way for multiple board. One board can boot from emmc and having
env on spi flash etc.. Anyways, this function is kept in both imx8mn 
and imx8mp evk boards instead of being completely dropped.
(as suggested by Andrey <andrey.zhizhikin@leica-geosystems.com>)

Tommaso Merciai (3):
  imx8m: drop env_get_location for imx8mn and imx8mp
  imx: imx8mn_evk: override env_get_location
  imx: imx8mp_evk: override env_get_location

 arch/arm/mach-imx/imx8m/soc.c           | 39 -----------------------
 board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +++++++++++++++++++++++++
 board/freescale/imx8mp_evk/imx8mp_evk.c | 41 ++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 39 deletions(-)

Comments

ZHIZHIKIN Andrey Nov. 29, 2021, 8:38 a.m. UTC | #1 
Hello Tommaso,

> -----Original Message-----
> From: Tommaso Merciai <tomm.merciai@gmail.com>
> Sent: Friday, November 26, 2021 6:43 PM
> Cc: michael@amarulasolutions.com; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-
> geosystems.com>; Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team
> <uboot-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>; Marek
> Vasut <marex@denx.de>; Simon Glass <sjg@chromium.org>; Frieder Schrempf
> <frieder.schrempf@kontron.de>; Marek Behún <marek.behun@nic.cz>; Ying-Chun Liu
> (PaulLiu) <paulliu@debian.org>; u-boot@lists.denx.de
> Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at
> board level
> 
> 
> This series move env_get_location from soc to board level. As suggested
> by Michael <michael@amarulasolutions.com> make no sense to define an
> unique way for multiple board. One board can boot from emmc and having
> env on spi flash etc.. Anyways, this function is kept in both imx8mn
> and imx8mp evk boards instead of being completely dropped.
> (as suggested by Andrey <andrey.zhizhikin@leica-geosystems.com>)

I believe there has been another suggestion from my side regarding this patch:
Since it look like that Michael Trimarchi submitted another part to drop
env_get_offset() in [1], combined with the first patch in this series - it is
a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
env_get_location").

I suggest you to submit a revert instead of your first patch and deprecate the
patch from Michael, instead of having 2 separate patches for this.

> 
> Tommaso Merciai (3):
>   imx8m: drop env_get_location for imx8mn and imx8mp
>   imx: imx8mn_evk: override env_get_location
>   imx: imx8mp_evk: override env_get_location
> 
>  arch/arm/mach-imx/imx8m/soc.c           | 39 -----------------------
>  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +++++++++++++++++++++++++
>  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 ++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 39 deletions(-)
> 
> --
> 2.25.1

Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211117143456.34441-1-michael@amarulasolutions.com/

-- andrey
Michael Nazzareno Trimarchi Nov. 29, 2021, 8:40 a.m. UTC | #2 
HI

On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Tommaso,
>
> > -----Original Message-----
> > From: Tommaso Merciai <tomm.merciai@gmail.com>
> > Sent: Friday, November 26, 2021 6:43 PM
> > Cc: michael@amarulasolutions.com; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-
> > geosystems.com>; Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team
> > <uboot-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>; Marek
> > Vasut <marex@denx.de>; Simon Glass <sjg@chromium.org>; Frieder Schrempf
> > <frieder.schrempf@kontron.de>; Marek Behún <marek.behun@nic.cz>; Ying-Chun Liu
> > (PaulLiu) <paulliu@debian.org>; u-boot@lists.denx.de
> > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at
> > board level
> >
> >
> > This series move env_get_location from soc to board level. As suggested
> > by Michael <michael@amarulasolutions.com> make no sense to define an
> > unique way for multiple board. One board can boot from emmc and having
> > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > and imx8mp evk boards instead of being completely dropped.
> > (as suggested by Andrey <andrey.zhizhikin@leica-geosystems.com>)
>
> I believe there has been another suggestion from my side regarding this patch:
> Since it look like that Michael Trimarchi submitted another part to drop
> env_get_offset() in [1], combined with the first patch in this series - it is
> a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
> env_get_location").
>
> I suggest you to submit a revert instead of your first patch and deprecate the
> patch from Michael, instead of having 2 separate patches for this.
>

I think they are totally different problems. One is code not used and
the other moves that implementation
in specific parts.

Michael

> >
> > Tommaso Merciai (3):
> >   imx8m: drop env_get_location for imx8mn and imx8mp
> >   imx: imx8mn_evk: override env_get_location
> >   imx: imx8mp_evk: override env_get_location
> >
> >  arch/arm/mach-imx/imx8m/soc.c           | 39 -----------------------
> >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +++++++++++++++++++++++++
> >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 ++++++++++++++++++++++++
> >  3 files changed, 83 insertions(+), 39 deletions(-)
> >
> > --
> > 2.25.1
>
> Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211117143456.34441-1-michael@amarulasolutions.com/
>
> -- andrey
ZHIZHIKIN Andrey Nov. 29, 2021, 8:46 a.m. UTC | #3 
Hello Michael,

> -----Original Message-----
> From: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
> Sent: Monday, November 29, 2021 9:40 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Cc: Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic <sbabic@denx.de>;
> Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team <uboot-imx@nxp.com>;
> Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>; Marek Vasut <marex@denx.de>;
> Simon Glass <sjg@chromium.org>; Frieder Schrempf <frieder.schrempf@kontron.de>;
> Marek Behún <marek.behun@nic.cz>; Ying-Chun Liu (PaulLiu) <paulliu@debian.org>;
> u-boot@lists.denx.de
> Subject: Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp
> at board level
> 
> 
> HI
> 
> On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
> <andrey.zhizhikin@leica-geosystems.com> wrote:
> >
> > Hello Tommaso,
> >
> > > -----Original Message-----
> > > From: Tommaso Merciai <tomm.merciai@gmail.com>
> > > Sent: Friday, November 26, 2021 6:43 PM
> > > Cc: michael@amarulasolutions.com; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-
> > > geosystems.com>; Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic
> > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team
> > > <uboot-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>;
> Marek
> > > Vasut <marex@denx.de>; Simon Glass <sjg@chromium.org>; Frieder Schrempf
> > > <frieder.schrempf@kontron.de>; Marek Behún <marek.behun@nic.cz>; Ying-Chun
> Liu
> > > (PaulLiu) <paulliu@debian.org>; u-boot@lists.denx.de
> > > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp
> at
> > > board level
> > >
> > >
> > > This series move env_get_location from soc to board level. As suggested
> > > by Michael <michael@amarulasolutions.com> make no sense to define an
> > > unique way for multiple board. One board can boot from emmc and having
> > > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > > and imx8mp evk boards instead of being completely dropped.
> > > (as suggested by Andrey <andrey.zhizhikin@leica-geosystems.com>)
> >
> > I believe there has been another suggestion from my side regarding this patch:
> > Since it look like that Michael Trimarchi submitted another part to drop
> > env_get_offset() in [1], combined with the first patch in this series - it is
> > a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
> > env_get_location").
> >
> > I suggest you to submit a revert instead of your first patch and deprecate the
> > patch from Michael, instead of having 2 separate patches for this.
> >
> 
> I think they are totally different problems. One is code not used and
> the other moves that implementation
> in specific parts.

They might be logically different, but 2 commits combined together - is a full
revert to me.

I'd leave this up to maintainer to decide, but for me it would be logical to see
a revert instead of 2 separate commits - this makes tracking more transparent.

> 
> Michael
> 
> > >
> > > Tommaso Merciai (3):
> > >   imx8m: drop env_get_location for imx8mn and imx8mp
> > >   imx: imx8mn_evk: override env_get_location
> > >   imx: imx8mp_evk: override env_get_location
> > >
> > >  arch/arm/mach-imx/imx8m/soc.c           | 39 -----------------------
> > >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +++++++++++++++++++++++++
> > >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 ++++++++++++++++++++++++
> > >  3 files changed, 83 insertions(+), 39 deletions(-)
> > >
> > > --
> > > 2.25.1
> >
> > Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211117143456.34441-1-michael@amarulasolutions.com/
> >
> > -- andrey
> 
> 
> 
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
> 
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolut
> ions.com%2F&amp;data=04%7C01%7C%7C666e97d4bb5f425c9a0408d9b313dd84%7C1b16ab3eb8f6
> 4fe39f3e2db7fe549f6a%7C0%7C0%7C637737720180120994%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=NJQ7
> qjpMWu%2BhGoIcqwmD%2BLc4Ekq1oHjqPSCqkiCr4DA%3D&amp;reserved=0

-- andrey
Michael Nazzareno Trimarchi Nov. 29, 2021, 8:48 a.m. UTC | #4 
Hi

On Mon, Nov 29, 2021 at 9:46 AM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Michael,
>
> > -----Original Message-----
> > From: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
> > Sent: Monday, November 29, 2021 9:40 AM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > Cc: Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic <sbabic@denx.de>;
> > Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team <uboot-imx@nxp.com>;
> > Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>; Marek Vasut <marex@denx.de>;
> > Simon Glass <sjg@chromium.org>; Frieder Schrempf <frieder.schrempf@kontron.de>;
> > Marek Behún <marek.behun@nic.cz>; Ying-Chun Liu (PaulLiu) <paulliu@debian.org>;
> > u-boot@lists.denx.de
> > Subject: Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp
> > at board level
> >
> >
> > HI
> >
> > On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > >
> > > Hello Tommaso,
> > >
> > > > -----Original Message-----
> > > > From: Tommaso Merciai <tomm.merciai@gmail.com>
> > > > Sent: Friday, November 26, 2021 6:43 PM
> > > > Cc: michael@amarulasolutions.com; ZHIZHIKIN Andrey <andrey.zhizhikin@leica-
> > > > geosystems.com>; Tommaso Merciai <tomm.merciai@gmail.com>; Stefano Babic
> > > > <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>; NXP i.MX U-Boot Team
> > > > <uboot-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>; Ye Li <ye.li@nxp.com>;
> > Marek
> > > > Vasut <marex@denx.de>; Simon Glass <sjg@chromium.org>; Frieder Schrempf
> > > > <frieder.schrempf@kontron.de>; Marek Behún <marek.behun@nic.cz>; Ying-Chun
> > Liu
> > > > (PaulLiu) <paulliu@debian.org>; u-boot@lists.denx.de
> > > > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp
> > at
> > > > board level
> > > >
> > > >
> > > > This series move env_get_location from soc to board level. As suggested
> > > > by Michael <michael@amarulasolutions.com> make no sense to define an
> > > > unique way for multiple board. One board can boot from emmc and having
> > > > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > > > and imx8mp evk boards instead of being completely dropped.
> > > > (as suggested by Andrey <andrey.zhizhikin@leica-geosystems.com>)
> > >
> > > I believe there has been another suggestion from my side regarding this patch:
> > > Since it look like that Michael Trimarchi submitted another part to drop
> > > env_get_offset() in [1], combined with the first patch in this series - it is
> > > a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
> > > env_get_location").
> > >
> > > I suggest you to submit a revert instead of your first patch and deprecate the
> > > patch from Michael, instead of having 2 separate patches for this.
> > >
> >
> > I think they are totally different problems. One is code not used and
> > the other moves that implementation
> > in specific parts.
>
> They might be logically different, but 2 commits combined together - is a full
> revert to me.
>
> I'd leave this up to maintainer to decide, but for me it would be logical to see
> a revert instead of 2 separate commits - this makes tracking more transparent.
>

The first one (mine) is not a logical change. It means that nothing
get wrong. The other
is anywway some change so if you needs to revert then you can pick
specific part.

Michael

> >
> > Michael
> >
> > > >
> > > > Tommaso Merciai (3):
> > > >   imx8m: drop env_get_location for imx8mn and imx8mp
> > > >   imx: imx8mn_evk: override env_get_location
> > > >   imx: imx8mp_evk: override env_get_location
> > > >
> > > >  arch/arm/mach-imx/imx8m/soc.c           | 39 -----------------------
> > > >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +++++++++++++++++++++++++
> > > >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 ++++++++++++++++++++++++
> > > >  3 files changed, 83 insertions(+), 39 deletions(-)
> > > >
> > > > --
> > > > 2.25.1
> > >
> > > Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211117143456.34441-1-michael@amarulasolutions.com/
> > >
> > > -- andrey
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > michael@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > info@amarulasolutions.com
> > https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolut
> > ions.com%2F&amp;data=04%7C01%7C%7C666e97d4bb5f425c9a0408d9b313dd84%7C1b16ab3eb8f6
> > 4fe39f3e2db7fe549f6a%7C0%7C0%7C637737720180120994%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> > iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=NJQ7
> > qjpMWu%2BhGoIcqwmD%2BLc4Ekq1oHjqPSCqkiCr4DA%3D&amp;reserved=0
>
> -- andrey