[PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree was
synchronized from linux and the aliases were dropped.

They need to be kept so that the mmc cards are in the right order.
Without the aliases, u-boot reports:
MMC:   FSL_SDHC: 2, FSL_SDHC: 3

With the aliases, u-boot reports:
MMC:   FSL_SDHC: 0, FSL_SDHC: 1

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi b/arch/arm/dts/imx6qdl-sabrelite.dtsi
index 22f8e2783c..6564e3b82c 100644
--- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
@@ -10,6 +10,13 @@
 #include <dt-bindings/input/input.h>
 
 / {
+	aliases {
+		mmc0 = &usdhc3;
+		mmc1 = &usdhc4;
+		pwm_lcd = &pwm1;
+		pwm_lvds = &pwm4;
+	};
+
 	chosen {
 		stdout-path = &uart2;
 	};
-- 
2.38.1

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

On Thu, Oct 27, 2022 at 11:22:52AM -0400, Detlev Casanova wrote:

> In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree was
> synchronized from linux and the aliases were dropped.
> 
> They need to be kept so that the mmc cards are in the right order.
> Without the aliases, u-boot reports:
> MMC:   FSL_SDHC: 2, FSL_SDHC: 3
> 
> With the aliases, u-boot reports:
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> index 22f8e2783c..6564e3b82c 100644
> --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> @@ -10,6 +10,13 @@
>  #include <dt-bindings/input/input.h>
>  
>  / {
> +	aliases {
> +		mmc0 = &usdhc3;
> +		mmc1 = &usdhc4;
> +		pwm_lcd = &pwm1;
> +		pwm_lvds = &pwm4;
> +	};
> +
>  	chosen {
>  		stdout-path = &uart2;
>  	};

This really belongs in the upstream dts file so they aren't lost again
and again.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

On Thursday, October 27, 2022 2:30:51 P.M. EST Tom Rini wrote:
> On Thu, Oct 27, 2022 at 11:22:52AM -0400, Detlev Casanova wrote:
> > In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree was
> > synchronized from linux and the aliases were dropped.
> > 
> > They need to be kept so that the mmc cards are in the right order.
> > Without the aliases, u-boot reports:
> > MMC:   FSL_SDHC: 2, FSL_SDHC: 3
> > 
> > With the aliases, u-boot reports:
> > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >  arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > b/arch/arm/dts/imx6qdl-sabrelite.dtsi index 22f8e2783c..6564e3b82c 100644
> > --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > @@ -10,6 +10,13 @@
> > 
> >  #include <dt-bindings/input/input.h>
> >  
> >  / {
> > 
> > +	aliases {
> > +		mmc0 = &usdhc3;
> > +		mmc1 = &usdhc4;
> > +		pwm_lcd = &pwm1;
> > +		pwm_lvds = &pwm4;
> > +	};
> > +
> > 
> >  	chosen {
> >  	
> >  		stdout-path = &uart2;
> >  	
> >  	};
> 
> This really belongs in the upstream dts file so they aren't lost again
> and again.

Forwaded to upstream kernel: https://lore.kernel.org/lkml/20221028141811.101122-1-detlev.casanova@collabora.com/[1]

Would it make sense to still add this one in u-boot before the devices trees are copied again ?

Detlev.


--------
[1] https://lore.kernel.org/lkml/20221028141811.101122-1-detlev.casanova@collabora.com/

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]

On Fri, Nov 11, 2022 at 10:28:43AM -0500, Detlev Casanova wrote:
> On Thursday, October 27, 2022 2:30:51 P.M. EST Tom Rini wrote:
> > On Thu, Oct 27, 2022 at 11:22:52AM -0400, Detlev Casanova wrote:
> > > In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree was
> > > synchronized from linux and the aliases were dropped.
> > > 
> > > They need to be kept so that the mmc cards are in the right order.
> > > Without the aliases, u-boot reports:
> > > MMC:   FSL_SDHC: 2, FSL_SDHC: 3
> > > 
> > > With the aliases, u-boot reports:
> > > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > > 
> > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > ---
> > > 
> > >  arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > b/arch/arm/dts/imx6qdl-sabrelite.dtsi index 22f8e2783c..6564e3b82c 100644
> > > --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > @@ -10,6 +10,13 @@
> > > 
> > >  #include <dt-bindings/input/input.h>
> > >  
> > >  / {
> > > 
> > > +	aliases {
> > > +		mmc0 = &usdhc3;
> > > +		mmc1 = &usdhc4;
> > > +		pwm_lcd = &pwm1;
> > > +		pwm_lvds = &pwm4;
> > > +	};
> > > +
> > > 
> > >  	chosen {
> > >  	
> > >  		stdout-path = &uart2;
> > >  	
> > >  	};
> > 
> > This really belongs in the upstream dts file so they aren't lost again
> > and again.
> 
> Forwaded to upstream kernel: https://lore.kernel.org/lkml/20221028141811.101122-1-detlev.casanova@collabora.com/[1]
> 
> Would it make sense to still add this one in u-boot before the devices trees are copied again ?

Yes, it would make sense to pick it up now to fix the regression here.
Thanks for pushing it upstream!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

On Friday, November 11, 2022 10:35:56 A.M. EST Tom Rini wrote:
> On Fri, Nov 11, 2022 at 10:28:43AM -0500, Detlev Casanova wrote:
> > On Thursday, October 27, 2022 2:30:51 P.M. EST Tom Rini wrote:
> > > On Thu, Oct 27, 2022 at 11:22:52AM -0400, Detlev Casanova wrote:
> > > > In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree
> > > > was
> > > > synchronized from linux and the aliases were dropped.
> > > > 
> > > > They need to be kept so that the mmc cards are in the right order.
> > > > Without the aliases, u-boot reports:
> > > > MMC:   FSL_SDHC: 2, FSL_SDHC: 3
> > > > 
> > > > With the aliases, u-boot reports:
> > > > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > > > 
> > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > > ---
> > > > 
> > > >  arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > b/arch/arm/dts/imx6qdl-sabrelite.dtsi index 22f8e2783c..6564e3b82c
> > > > 100644
> > > > --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > @@ -10,6 +10,13 @@
> > > > 
> > > >  #include <dt-bindings/input/input.h>
> > > >  
> > > >  / {
> > > > 
> > > > +	aliases {
> > > > +		mmc0 = &usdhc3;
> > > > +		mmc1 = &usdhc4;
> > > > +		pwm_lcd = &pwm1;
> > > > +		pwm_lvds = &pwm4;
> > > > +	};
> > > > +
> > > > 
> > > >  	chosen {
> > > >  	
> > > >  		stdout-path = &uart2;
> > > >  	
> > > >  	};
> > > 
> > > This really belongs in the upstream dts file so they aren't lost again
> > > and again.
> > 
> > Forwaded to upstream kernel:
> > https://lore.kernel.org/lkml/20221028141811.101122-1-detlev.casanova@coll
> > abora.com/[1]
> > 
> > Would it make sense to still add this one in u-boot before the devices
> > trees are copied again ?
> Yes, it would make sense to pick it up now to fix the regression here.
> Thanks for pushing it upstream!

Upstream linux won't apply it as they want to keep the already established order in linux (the 
SD card has to stay at mmc2). See https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/[1]

I guess we will have to adapt u-boot environment when these dts are synced again.

Detlev.


--------
[1] https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3083 bytes --]

On Fri, Dec 02, 2022 at 02:06:26PM -0500, Detlev Casanova wrote:
> On Friday, November 11, 2022 10:35:56 A.M. EST Tom Rini wrote:
> > On Fri, Nov 11, 2022 at 10:28:43AM -0500, Detlev Casanova wrote:
> > > On Thursday, October 27, 2022 2:30:51 P.M. EST Tom Rini wrote:
> > > > On Thu, Oct 27, 2022 at 11:22:52AM -0400, Detlev Casanova wrote:
> > > > > In commit d0399a46e7cda63c07e3eb8558bef84cfb068028, the device tree
> > > > > was
> > > > > synchronized from linux and the aliases were dropped.
> > > > > 
> > > > > They need to be kept so that the mmc cards are in the right order.
> > > > > Without the aliases, u-boot reports:
> > > > > MMC:   FSL_SDHC: 2, FSL_SDHC: 3
> > > > > 
> > > > > With the aliases, u-boot reports:
> > > > > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > > > > 
> > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > > > > ---
> > > > > 
> > > > >  arch/arm/dts/imx6qdl-sabrelite.dtsi | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > > b/arch/arm/dts/imx6qdl-sabrelite.dtsi index 22f8e2783c..6564e3b82c
> > > > > 100644
> > > > > --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > > +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi
> > > > > @@ -10,6 +10,13 @@
> > > > > 
> > > > >  #include <dt-bindings/input/input.h>
> > > > >  
> > > > >  / {
> > > > > 
> > > > > +	aliases {
> > > > > +		mmc0 = &usdhc3;
> > > > > +		mmc1 = &usdhc4;
> > > > > +		pwm_lcd = &pwm1;
> > > > > +		pwm_lvds = &pwm4;
> > > > > +	};
> > > > > +
> > > > > 
> > > > >  	chosen {
> > > > >  	
> > > > >  		stdout-path = &uart2;
> > > > >  	
> > > > >  	};
> > > > 
> > > > This really belongs in the upstream dts file so they aren't lost again
> > > > and again.
> > > 
> > > Forwaded to upstream kernel:
> > > https://lore.kernel.org/lkml/20221028141811.101122-1-detlev.casanova@coll
> > > abora.com/[1]
> > > 
> > > Would it make sense to still add this one in u-boot before the devices
> > > trees are copied again ?
> > Yes, it would make sense to pick it up now to fix the regression here.
> > Thanks for pushing it upstream!
> 
> Upstream linux won't apply it as they want to keep the already established order in linux (the 
> SD card has to stay at mmc2). See https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/[1]
> 
> I guess we will have to adapt u-boot environment when these dts are synced again.
> 
> Detlev.
> 
> 
> --------
> [1] https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/

Not being overly familiar with the board in question in production (it
was quite a long back when I last was involved in a design based on one
of these), I don't know which order got more widespread real life usage,
so, OK, we'll defer to what upstream kernel has now as indeed following
the data sheet for order is reasonable. Whatever migrations need to be
done to match this should be started on sooner rather than later I would
suggest.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Fabio Estevam]

[Adding Troy]

On Fri, Dec 2, 2022 at 4:06 PM Detlev Casanova
<detlev.casanova@collabora.com> wrote:

> Upstream linux won't apply it as they want to keep the already established order in linux (the SD card has to stay at mmc2). See https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/
>
>
> I guess we will have to adapt u-boot environment when these dts are synced again.

I would suggest adding the mmc alias into
arch/arm/dts/imx6qdl-sabrelite-u-boot.dtsi

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On Fri, Dec 02, 2022 at 08:20:11PM -0300, Fabio Estevam wrote:
> [Adding Troy]
> 
> On Fri, Dec 2, 2022 at 4:06 PM Detlev Casanova
> <detlev.casanova@collabora.com> wrote:
> 
> > Upstream linux won't apply it as they want to keep the already established order in linux (the SD card has to stay at mmc2). See https://lore.kernel.org/all/a13c2e92-cfa6-04fd-c32a-c4d444894660@pengutronix.de/
> >
> >
> > I guess we will have to adapt u-boot environment when these dts are synced again.
> 
> I would suggest adding the mmc alias into
> arch/arm/dts/imx6qdl-sabrelite-u-boot.dtsi

No, upstream has different aliases and doesn't want these. That's the
point of the above thread, right?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Fabio Estevam]

On Fri, Dec 2, 2022 at 8:36 PM Tom Rini <trini@konsulko.com> wrote:

> No, upstream has different aliases and doesn't want these. That's the
> point of the above thread, right?

Upstream is correct in not accepting new alias for this board, as this
could break
existing setups.

In U-Boot, we had alias for this board originally. After the sync with
Linux they are gone.

To fix U-Boot, the less invasive change is to add the alias into
arch/arm/dts/imx6qdl-sabrelite-u-boot.dtsi.

This way we can:

1. Keep the dtsi in sync with Linux

2. Do not break users in U-Boot

This is the same approach I did for wandboard in the following commit:

commit f827f84d3f5607d0b33e927f6127a888e7bd664f
Author: Fabio Estevam <festevam@denx.de>
Date:   Fri Nov 4 08:12:54 2022 -0300

    wandboard: Pass mmc aliases

    Originally, the mmc aliases node was present in imx6qdl-wandboard.dtsi.

    After the sync with Linux in commit d0399a46e7cd ("imx6dl/imx6qdl:
    synchronise device trees with linux"), the aliases node is gone as
    the upstream version does not have it.

    This causes a regression in which the SD card cannot be found anymore:

    Since commit  the aliases node has been removed
    U-Boot 2022.10-00999-gcca41ed3d63f-dirty (Nov 03 2022 - 22:07:38 -0300)

    CPU:   Freescale i.MX6QP rev1.0 at 792 MHz
    Reset cause: POR
    DRAM:  2 GiB
    Core:  62 devices, 17 uclasses, devicetree: separate
    PMIC:  PFUZE100 ID=0x10
    MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
    Loading Environment from MMC... MMC: no card present
    *** Warning - No block device, using default environment

    Fix it by passing the alias node in the u-boot.dtsi file to
    restore the original behaviour where the SD card (esdhc3) was
    mapped to mmc0.

    Fixes: d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with linux")
    Signed-off-by: Fabio Estevam <festevam@denx.de>

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

On Saturday, December 3, 2022 7:23:10 A.M. EST Fabio Estevam wrote:
> On Fri, Dec 2, 2022 at 8:36 PM Tom Rini <trini@konsulko.com> wrote:
> > No, upstream has different aliases and doesn't want these. That's the
> > point of the above thread, right?
> 
> Upstream is correct in not accepting new alias for this board, as this
> could break
> existing setups.
> 
> In U-Boot, we had alias for this board originally. After the sync with
> Linux they are gone.
> 
> To fix U-Boot, the less invasive change is to add the alias into
> arch/arm/dts/imx6qdl-sabrelite-u-boot.dtsi.
> 
> This way we can:
> 
> 1. Keep the dtsi in sync with Linux
> 
> 2. Do not break users in U-Boot
> 
> This is the same approach I did for wandboard in the following commit:
> 
> commit f827f84d3f5607d0b33e927f6127a888e7bd664f
> Author: Fabio Estevam <festevam@denx.de>
> Date:   Fri Nov 4 08:12:54 2022 -0300
> 
>     wandboard: Pass mmc aliases
> 
>     Originally, the mmc aliases node was present in imx6qdl-wandboard.dtsi.
> 
>     After the sync with Linux in commit d0399a46e7cd ("imx6dl/imx6qdl:
>     synchronise device trees with linux"), the aliases node is gone as
>     the upstream version does not have it.
> 
>     This causes a regression in which the SD card cannot be found anymore:
> 
>     Since commit  the aliases node has been removed
>     U-Boot 2022.10-00999-gcca41ed3d63f-dirty (Nov 03 2022 - 22:07:38 -0300)
> 
>     CPU:   Freescale i.MX6QP rev1.0 at 792 MHz
>     Reset cause: POR
>     DRAM:  2 GiB
>     Core:  62 devices, 17 uclasses, devicetree: separate
>     PMIC:  PFUZE100 ID=0x10
>     MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
>     Loading Environment from MMC... MMC: no card present
>     *** Warning - No block device, using default environment
> 
>     Fix it by passing the alias node in the u-boot.dtsi file to
>     restore the original behaviour where the SD card (esdhc3) was
>     mapped to mmc0.
> 
>     Fixes: d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> linux") Signed-off-by: Fabio Estevam <festevam@denx.de>

Ha good idea to use a u-boot specific dtsi. I'll send a new patch with that 
then.

Thanks !

Detlev.

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

On Sat, Dec 03, 2022 at 09:23:10AM -0300, Fabio Estevam wrote:
> On Fri, Dec 2, 2022 at 8:36 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > No, upstream has different aliases and doesn't want these. That's the
> > point of the above thread, right?
> 
> Upstream is correct in not accepting new alias for this board, as this
> could break
> existing setups.
> 
> In U-Boot, we had alias for this board originally. After the sync with
> Linux they are gone.
> 
> To fix U-Boot, the less invasive change is to add the alias into
> arch/arm/dts/imx6qdl-sabrelite-u-boot.dtsi.
> 
> This way we can:
> 
> 1. Keep the dtsi in sync with Linux
> 
> 2. Do not break users in U-Boot
> 
> This is the same approach I did for wandboard in the following commit:
> 
> commit f827f84d3f5607d0b33e927f6127a888e7bd664f
> Author: Fabio Estevam <festevam@denx.de>
> Date:   Fri Nov 4 08:12:54 2022 -0300
> 
>     wandboard: Pass mmc aliases
> 
>     Originally, the mmc aliases node was present in imx6qdl-wandboard.dtsi.
> 
>     After the sync with Linux in commit d0399a46e7cd ("imx6dl/imx6qdl:
>     synchronise device trees with linux"), the aliases node is gone as
>     the upstream version does not have it.
> 
>     This causes a regression in which the SD card cannot be found anymore:
> 
>     Since commit  the aliases node has been removed
>     U-Boot 2022.10-00999-gcca41ed3d63f-dirty (Nov 03 2022 - 22:07:38 -0300)
> 
>     CPU:   Freescale i.MX6QP rev1.0 at 792 MHz
>     Reset cause: POR
>     DRAM:  2 GiB
>     Core:  62 devices, 17 uclasses, devicetree: separate
>     PMIC:  PFUZE100 ID=0x10
>     MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
>     Loading Environment from MMC... MMC: no card present
>     *** Warning - No block device, using default environment
> 
>     Fix it by passing the alias node in the u-boot.dtsi file to
>     restore the original behaviour where the SD card (esdhc3) was
>     mapped to mmc0.
> 
>     Fixes: d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with linux")
>     Signed-off-by: Fabio Estevam <festevam@denx.de>

I'm not really happy with this approach.  It's not that upstream doesn't
have aliases now, it's that it has different aliases, right? That's why
they won't accept these?

But this also highlights that we really need to get these kind of
aliases and similar (a) re-synced with upstream ASAP and (b) do that at
the start. I don't know which group of "users are broken by this change"
is bigger, the group that needs the aliases we have or the group that
needs the other aliases, but the group that gets changes upstream first
"wins" here is how the OSS world works.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Fabio Estevam]

On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:

> I'm not really happy with this approach.  It's not that upstream doesn't
> have aliases now, it's that it has different aliases, right? That's why
> they won't accept these?

imx6q.dtsi does have the default mmc aliases:

mmc0 = &usdhc1;
mmc1 = &usdhc2;
mmc2 = &usdhc3;
mmc3 = &usdhc4;

Upstream does not want to change mmc alias because users may rely on
this mmc aliases.

Changing it now may cause the board not to boot anymore as the rootfs
cannot be found.

It is OK to change mmc alias for a newly introduced board, but please
keep in mind that
wandboard and sabrelite have been launched many many years ago.

So for upstream Linux the message was clear: don't change the mmc
alias for these boards.

Now let's talk about U-Boot.

Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
linux") the mmc alias for sabrelite was present:

               mmc0 = &usdhc3;
               mmc1 = &usdhc4;

After this commit, the mmc alias is gone and causes the boot
regression as reported by Detlev.

We don't want to cause regressions in U-Boot as well, so that's why I
propose just adding the alias into u-boot.dtsi.

There are many boards that does the same.

> But this also highlights that we really need to get these kind of
> aliases and similar (a) re-synced with upstream ASAP and (b) do that at

imx6 dts files are already synced with Linux.

> the start. I don't know which group of "users are broken by this change"
> is bigger, the group that needs the aliases we have or the group that
> needs the other aliases, but the group that gets changes upstream first
> "wins" here is how the OSS world works.

I prefer to not break things for anyone, hence my proposal.

If you are still not happy with it, please feel free to submit a patch
with your proposal.

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

On Monday, December 5, 2022 12:33:48 P.M. EST Fabio Estevam wrote:
> On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:
> > I'm not really happy with this approach.  It's not that upstream doesn't
> > have aliases now, it's that it has different aliases, right? That's why
> > they won't accept these?
> 
> imx6q.dtsi does have the default mmc aliases:
> 
> mmc0 = &usdhc1;
> mmc1 = &usdhc2;
> mmc2 = &usdhc3;
> mmc3 = &usdhc4;
> 
> Upstream does not want to change mmc alias because users may rely on
> this mmc aliases.
> 
> Changing it now may cause the board not to boot anymore as the rootfs
> cannot be found.
> 
> It is OK to change mmc alias for a newly introduced board, but please
> keep in mind that
> wandboard and sabrelite have been launched many many years ago.
> 
> So for upstream Linux the message was clear: don't change the mmc
> alias for these boards.
> 
> Now let's talk about U-Boot.
> 
> Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> linux") the mmc alias for sabrelite was present:
> 
>                mmc0 = &usdhc3;
>                mmc1 = &usdhc4;
> 
> After this commit, the mmc alias is gone and causes the boot
> regression as reported by Detlev.
> 
> We don't want to cause regressions in U-Boot as well, so that's why I
> propose just adding the alias into u-boot.dtsi.
> 
> There are many boards that does the same.
> 
> > But this also highlights that we really need to get these kind of
> > aliases and similar (a) re-synced with upstream ASAP and (b) do that at
> 
> imx6 dts files are already synced with Linux.
> 
> > the start. I don't know which group of "users are broken by this change"
> > is bigger, the group that needs the aliases we have or the group that
> > needs the other aliases, but the group that gets changes upstream first
> > "wins" here is how the OSS world works.
> 
> I prefer to not break things for anyone, hence my proposal.
> 
> If you are still not happy with it, please feel free to submit a patch
> with your proposal.

There is no SPL enabled for the imx.6 Sabrelite, Isn't that needed to use the 
*-u-boot.dtsi files ?

It is not clear to me how those are loaded.

Detlev.

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3015 bytes --]

On Wed, Dec 07, 2022 at 02:53:10PM -0500, Detlev Casanova wrote:
> On Monday, December 5, 2022 12:33:48 P.M. EST Fabio Estevam wrote:
> > On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:
> > > I'm not really happy with this approach.  It's not that upstream doesn't
> > > have aliases now, it's that it has different aliases, right? That's why
> > > they won't accept these?
> > 
> > imx6q.dtsi does have the default mmc aliases:
> > 
> > mmc0 = &usdhc1;
> > mmc1 = &usdhc2;
> > mmc2 = &usdhc3;
> > mmc3 = &usdhc4;
> > 
> > Upstream does not want to change mmc alias because users may rely on
> > this mmc aliases.
> > 
> > Changing it now may cause the board not to boot anymore as the rootfs
> > cannot be found.
> > 
> > It is OK to change mmc alias for a newly introduced board, but please
> > keep in mind that
> > wandboard and sabrelite have been launched many many years ago.
> > 
> > So for upstream Linux the message was clear: don't change the mmc
> > alias for these boards.
> > 
> > Now let's talk about U-Boot.
> > 
> > Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> > linux") the mmc alias for sabrelite was present:
> > 
> >                mmc0 = &usdhc3;
> >                mmc1 = &usdhc4;
> > 
> > After this commit, the mmc alias is gone and causes the boot
> > regression as reported by Detlev.
> > 
> > We don't want to cause regressions in U-Boot as well, so that's why I
> > propose just adding the alias into u-boot.dtsi.
> > 
> > There are many boards that does the same.
> > 
> > > But this also highlights that we really need to get these kind of
> > > aliases and similar (a) re-synced with upstream ASAP and (b) do that at
> > 
> > imx6 dts files are already synced with Linux.
> > 
> > > the start. I don't know which group of "users are broken by this change"
> > > is bigger, the group that needs the aliases we have or the group that
> > > needs the other aliases, but the group that gets changes upstream first
> > > "wins" here is how the OSS world works.
> > 
> > I prefer to not break things for anyone, hence my proposal.
> > 
> > If you are still not happy with it, please feel free to submit a patch
> > with your proposal.
> 
> There is no SPL enabled for the imx.6 Sabrelite, Isn't that needed to use the 
> *-u-boot.dtsi files ?
> 
> It is not clear to me how those are loaded.

The -u-boot.dtsi files are automatically included from scripts/Makefile.lib:
u_boot_dtsi_options = $(strip $(wildcard $(dir $<)$(basename $(notdir $<))-u-boot.dtsi) \
        $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
        $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
        $(wildcard $(dir $<)$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
        $(wildcard $(dir $<)u-boot.dtsi))

And then manually #included at times as the above logic will only pick
one file and sometimes we do need to combine multiple.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2301 bytes --]

On Mon, Dec 05, 2022 at 02:33:48PM -0300, Fabio Estevam wrote:
> On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > I'm not really happy with this approach.  It's not that upstream doesn't
> > have aliases now, it's that it has different aliases, right? That's why
> > they won't accept these?
> 
> imx6q.dtsi does have the default mmc aliases:
> 
> mmc0 = &usdhc1;
> mmc1 = &usdhc2;
> mmc2 = &usdhc3;
> mmc3 = &usdhc4;
> 
> Upstream does not want to change mmc alias because users may rely on
> this mmc aliases.
> 
> Changing it now may cause the board not to boot anymore as the rootfs
> cannot be found.
> 
> It is OK to change mmc alias for a newly introduced board, but please
> keep in mind that
> wandboard and sabrelite have been launched many many years ago.
> 
> So for upstream Linux the message was clear: don't change the mmc
> alias for these boards.
> 
> Now let's talk about U-Boot.
> 
> Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> linux") the mmc alias for sabrelite was present:
> 
>                mmc0 = &usdhc3;
>                mmc1 = &usdhc4;
> 
> After this commit, the mmc alias is gone and causes the boot
> regression as reported by Detlev.
> 
> We don't want to cause regressions in U-Boot as well, so that's why I
> propose just adding the alias into u-boot.dtsi.
> 
> There are many boards that does the same.
> 
> > But this also highlights that we really need to get these kind of
> > aliases and similar (a) re-synced with upstream ASAP and (b) do that at
> 
> imx6 dts files are already synced with Linux.
> 
> > the start. I don't know which group of "users are broken by this change"
> > is bigger, the group that needs the aliases we have or the group that
> > needs the other aliases, but the group that gets changes upstream first
> > "wins" here is how the OSS world works.
> 
> I prefer to not break things for anyone, hence my proposal.
> 
> If you are still not happy with it, please feel free to submit a patch
> with your proposal.

I don't know what we can do about the existing boards, but we need to
NOT do this moving forward. We need to use whatever the aliases are
upstream already, or upstream the aliases we want to use.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Detlev Casanova]

On Wednesday, December 7, 2022 3:03:56 P.M. EST Tom Rini wrote:
> On Mon, Dec 05, 2022 at 02:33:48PM -0300, Fabio Estevam wrote:
> > On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:
> > > I'm not really happy with this approach.  It's not that upstream doesn't
> > > have aliases now, it's that it has different aliases, right? That's why
> > > they won't accept these?
> > 
> > imx6q.dtsi does have the default mmc aliases:
> > 
> > mmc0 = &usdhc1;
> > mmc1 = &usdhc2;
> > mmc2 = &usdhc3;
> > mmc3 = &usdhc4;
> > 
> > Upstream does not want to change mmc alias because users may rely on
> > this mmc aliases.
> > 
> > Changing it now may cause the board not to boot anymore as the rootfs
> > cannot be found.
> > 
> > It is OK to change mmc alias for a newly introduced board, but please
> > keep in mind that
> > wandboard and sabrelite have been launched many many years ago.
> > 
> > So for upstream Linux the message was clear: don't change the mmc
> > alias for these boards.
> > 
> > Now let's talk about U-Boot.
> > 
> > Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> > 
> > linux") the mmc alias for sabrelite was present:
> >                mmc0 = &usdhc3;
> >                mmc1 = &usdhc4;
> > 
> > After this commit, the mmc alias is gone and causes the boot
> > regression as reported by Detlev.
> > 
> > We don't want to cause regressions in U-Boot as well, so that's why I
> > propose just adding the alias into u-boot.dtsi.
> > 
> > There are many boards that does the same.
> > 
> > > But this also highlights that we really need to get these kind of
> > > aliases and similar (a) re-synced with upstream ASAP and (b) do that at
> > 
> > imx6 dts files are already synced with Linux.
> > 
> > > the start. I don't know which group of "users are broken by this change"
> > > is bigger, the group that needs the aliases we have or the group that
> > > needs the other aliases, but the group that gets changes upstream first
> > > "wins" here is how the OSS world works.
> > 
> > I prefer to not break things for anyone, hence my proposal.
> > 
> > If you are still not happy with it, please feel free to submit a patch
> > with your proposal.
> 
> I don't know what we can do about the existing boards, but we need to
> NOT do this moving forward. We need to use whatever the aliases are
> upstream already, or upstream the aliases we want to use.

Yes, I agree that aliases should be kept in sync with the kernel. But this 
case is particular because this board has been using different aliases in u-
boot and in linux for some time (maybe since forever).

So this should be kept to avoid breaking existing u-boot setups when they are 
upgraded.

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

On Wed, Dec 07, 2022 at 03:08:46PM -0500, Detlev Casanova wrote:
> On Wednesday, December 7, 2022 3:03:56 P.M. EST Tom Rini wrote:
> > On Mon, Dec 05, 2022 at 02:33:48PM -0300, Fabio Estevam wrote:
> > > On Mon, Dec 5, 2022 at 12:37 PM Tom Rini <trini@konsulko.com> wrote:
> > > > I'm not really happy with this approach.  It's not that upstream doesn't
> > > > have aliases now, it's that it has different aliases, right? That's why
> > > > they won't accept these?
> > > 
> > > imx6q.dtsi does have the default mmc aliases:
> > > 
> > > mmc0 = &usdhc1;
> > > mmc1 = &usdhc2;
> > > mmc2 = &usdhc3;
> > > mmc3 = &usdhc4;
> > > 
> > > Upstream does not want to change mmc alias because users may rely on
> > > this mmc aliases.
> > > 
> > > Changing it now may cause the board not to boot anymore as the rootfs
> > > cannot be found.
> > > 
> > > It is OK to change mmc alias for a newly introduced board, but please
> > > keep in mind that
> > > wandboard and sabrelite have been launched many many years ago.
> > > 
> > > So for upstream Linux the message was clear: don't change the mmc
> > > alias for these boards.
> > > 
> > > Now let's talk about U-Boot.
> > > 
> > > Prior to d0399a46e7cd ("imx6dl/imx6qdl: synchronise device trees with
> > > 
> > > linux") the mmc alias for sabrelite was present:
> > >                mmc0 = &usdhc3;
> > >                mmc1 = &usdhc4;
> > > 
> > > After this commit, the mmc alias is gone and causes the boot
> > > regression as reported by Detlev.
> > > 
> > > We don't want to cause regressions in U-Boot as well, so that's why I
> > > propose just adding the alias into u-boot.dtsi.
> > > 
> > > There are many boards that does the same.
> > > 
> > > > But this also highlights that we really need to get these kind of
> > > > aliases and similar (a) re-synced with upstream ASAP and (b) do that at
> > > 
> > > imx6 dts files are already synced with Linux.
> > > 
> > > > the start. I don't know which group of "users are broken by this change"
> > > > is bigger, the group that needs the aliases we have or the group that
> > > > needs the other aliases, but the group that gets changes upstream first
> > > > "wins" here is how the OSS world works.
> > > 
> > > I prefer to not break things for anyone, hence my proposal.
> > > 
> > > If you are still not happy with it, please feel free to submit a patch
> > > with your proposal.
> > 
> > I don't know what we can do about the existing boards, but we need to
> > NOT do this moving forward. We need to use whatever the aliases are
> > upstream already, or upstream the aliases we want to use.
> 
> Yes, I agree that aliases should be kept in sync with the kernel. But this 
> case is particular because this board has been using different aliases in u-
> boot and in linux for some time (maybe since forever).
> 
> So this should be kept to avoid breaking existing u-boot setups when they are 
> upgraded.

I wonder just how many users would end up needing to be fixed here,
but... since this is imx6 and not something brand new like 8 or 9, I'll
just let it go, here.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] dts: Re-add aliases for imx6qdl-sabrelite devices

[Michael Walle]

Hi,

>> I'm not really happy with this approach.  It's not that upstream doesn't
>> have aliases now, it's that it has different aliases, right? That's why
>> they won't accept these?
> 
> imx6q.dtsi does have the default mmc aliases:
> 
> mmc0 = &usdhc1;
> mmc1 = &usdhc2;
> mmc2 = &usdhc3;
> mmc3 = &usdhc4;
>
> Upstream does not want to change mmc alias because users may rely on
> this mmc aliases.

This thread seems to be another reason *not* to put any aliases
into a common dtsi file. They are board specific and should
really go into the board dts.

-michael