Re: [PATCH 1/6] include: env: phytec: overlays: Add extension command

[Yannic Moog]

On Tue, 2024-07-09 at 15:47 +0200, Wadim Egorov wrote:
> 
> 
> Am 09.07.24 um 15:31 schrieb Daniel Schultz:
> > Hi Yannic,
> > 
> > On 09.07.24 08:49, Yannic Moog wrote:
> > > Hello Daniel,
> > > 
> > > On Sun, 2024-07-07 at 23:07 -0700, Daniel Schultz wrote:
> > > > Add a new environment routine to apply extensions. Our SOM detection
> > > > adds overlays via the extension framework to alter the kernel
> > > > device-tree according to the flashed EEPROM image.
> > > > 
> > > > Signed-off-by: Daniel Schultz 
> > > > ---
> > > >   include/env/phytec/overlays.env | 20 
> > > >   1 file changed, 20 insertions(+)
> > > > 
> > > > diff --git a/include/env/phytec/overlays.env 
> > > > b/include/env/phytec/overlays.env
> > > > index 78721cde654..50110e731bd 100644
> > > > --- a/include/env/phytec/overlays.env
> > > > +++ b/include/env/phytec/overlays.env
> > > > @@ -23,6 +23,16 @@ mmc_apply_overlays=
> > > >   fi;
> > > >   done;
> > > >   fi;
> > > > +#ifdef CONFIG_CMD_EXTENSION
> > > > +mmc_apply_extensions=
> > > > +    setenv extension_overlay_addr ${fdtoverlay_addr_r};
> > > > +    setenv extension_overlay_cmd 'load mmc ${mmcdev}:${mmcpart}
> > > > +      ${fdtoverlay_addr_r} ${extension_overlay_name}';
> > > > +    extension scan;
> > > > +    extension apply all;
> > > > +#else
> > > > +mmc_apply_extensions=echo "no extension command"
> > > Do you think it makes sense to make this fail? I would think 
> > > "apply_extensions" would be
> > > unsuccessful if extensions are not working.
> > 
> > Technically, you don't need our SOM detection overlays. They just make 
> > the boot nicer and you don't have any probe errors or other error 
> > messages. So, in my opinion, it's better not apply them instead of 
> > stopping the entire boot.
> 
> My recommendation is to embed all SoM variant related overlays into 
> u-boot.img's FIT using binman.

Didn't think binman would be used for that, but I share your opinion regarding 
overlay packaging.

Yannic

> 
> This makes the extension command obsolete, and especially the handling 
> for loading of overlays from different boot sources / storage devices.
> 
> Having them available directly in the u-boot binary resolves the issue 
> of not found overlays.
> 
> We do not know how users will proceed after u-boot booted and if the 
> overlays will be available to the "extension setup". Better to have 
> everything in place and fixup to the best we know using our SoM data 
> stored in the EEPROM.
> 
> This kind of overlays should be applied regardless of the boot device.
> 
> > 
> > Regards,
> > Daniel
> > 
> > > 
> > > Yannic
> > > 
> > > > +#endif
> > > >   net_load_bootenv=${get_cmd} ${bootenv_addr_r} ${bootenv}
> > > >   net_load_overlay=${get_cmd} ${fdtoverlay_addr_r} ${overlay}
> > > >   net_apply_overlays=
> > > > @@ -36,3 +46,13 @@ net_apply_overlays=
> > > >   fi;
> > > >   done;
> > > >   fi;
> > > > +#ifdef CONFIG_CMD_EXTENSION
> > > > +net_apply_extensions=
> > > > +    setenv extension_overlay_addr ${fdtoverlay_addr_r};
> > > > +    setenv extension_overlay_cmd '${get_cmd} ${fdtoverlay_addr_r}
> > > > + ${extension_overlay_name}';
> > > > +    extension scan;
> > > > +    extension apply all;
> > > > +#else
> > > > +net_apply_extensions=echo "no extension command"
> > > > +#endif

Re: [PATCH 1/6] include: env: phytec: overlays: Add extension command

[Wadim Egorov]

Am 09.07.24 um 15:31 schrieb Daniel Schultz:

Hi Yannic,

On 09.07.24 08:49, Yannic Moog wrote:

Hello Daniel,

On Sun, 2024-07-07 at 23:07 -0700, Daniel Schultz wrote:

Add a new environment routine to apply extensions. Our SOM detection
adds overlays via the extension framework to alter the kernel
device-tree according to the flashed EEPROM image.

Signed-off-by: Daniel Schultz 
---
  include/env/phytec/overlays.env | 20 
  1 file changed, 20 insertions(+)

diff --git a/include/env/phytec/overlays.env 
b/include/env/phytec/overlays.env

index 78721cde654..50110e731bd 100644
--- a/include/env/phytec/overlays.env
+++ b/include/env/phytec/overlays.env
@@ -23,6 +23,16 @@ mmc_apply_overlays=
  fi;
  done;
  fi;
+#ifdef CONFIG_CMD_EXTENSION
+mmc_apply_extensions=
+    setenv extension_overlay_addr ${fdtoverlay_addr_r};
+    setenv extension_overlay_cmd 'load mmc ${mmcdev}:${mmcpart}
+      ${fdtoverlay_addr_r} ${extension_overlay_name}';
+    extension scan;
+    extension apply all;
+#else
+mmc_apply_extensions=echo "no extension command"
Do you think it makes sense to make this fail? I would think 
"apply_extensions" would be

unsuccessful if extensions are not working.


Technically, you don't need our SOM detection overlays. They just make 
the boot nicer and you don't have any probe errors or other error 
messages. So, in my opinion, it's better not apply them instead of 
stopping the entire boot.


My recommendation is to embed all SoM variant related overlays into 
u-boot.img's FIT using binman.


This makes the extension command obsolete, and especially the handling 
for loading of overlays from different boot sources / storage devices.


Having them available directly in the u-boot binary resolves the issue 
of not found overlays.


We do not know how users will proceed after u-boot booted and if the 
overlays will be available to the "extension setup". Better to have 
everything in place and fixup to the best we know using our SoM data 
stored in the EEPROM.


This kind of overlays should be applied regardless of the boot device.



Regards,
Daniel



Yannic


+#endif
  net_load_bootenv=${get_cmd} ${bootenv_addr_r} ${bootenv}
  net_load_overlay=${get_cmd} ${fdtoverlay_addr_r} ${overlay}
  net_apply_overlays=
@@ -36,3 +46,13 @@ net_apply_overlays=
  fi;
  done;
  fi;
+#ifdef CONFIG_CMD_EXTENSION
+net_apply_extensions=
+    setenv extension_overlay_addr ${fdtoverlay_addr_r};
+    setenv extension_overlay_cmd '${get_cmd} ${fdtoverlay_addr_r}
+ ${extension_overlay_name}';
+    extension scan;
+    extension apply all;
+#else
+net_apply_extensions=echo "no extension command"
+#endif

Re: [PATCH 1/6] include: env: phytec: overlays: Add extension command

[Daniel Schultz]

Hi Yannic,

On 09.07.24 08:49, Yannic Moog wrote:

Hello Daniel,

On Sun, 2024-07-07 at 23:07 -0700, Daniel Schultz wrote:

Add a new environment routine to apply extensions. Our SOM detection
adds overlays via the extension framework to alter the kernel
device-tree according to the flashed EEPROM image.

Signed-off-by: Daniel Schultz 
---
  include/env/phytec/overlays.env | 20 
  1 file changed, 20 insertions(+)

diff --git a/include/env/phytec/overlays.env b/include/env/phytec/overlays.env
index 78721cde654..50110e731bd 100644
--- a/include/env/phytec/overlays.env
+++ b/include/env/phytec/overlays.env
@@ -23,6 +23,16 @@ mmc_apply_overlays=
    fi;
    done;
    fi;
+#ifdef CONFIG_CMD_EXTENSION
+mmc_apply_extensions=
+   setenv extension_overlay_addr ${fdtoverlay_addr_r};
+   setenv extension_overlay_cmd 'load mmc ${mmcdev}:${mmcpart}
+     ${fdtoverlay_addr_r} ${extension_overlay_name}';
+   extension scan;
+   extension apply all;
+#else
+mmc_apply_extensions=echo "no extension command"

Do you think it makes sense to make this fail? I would think "apply_extensions" 
would be
unsuccessful if extensions are not working.


Technically, you don't need our SOM detection overlays. They just make 
the boot nicer and you don't have any probe errors or other error 
messages. So, in my opinion, it's better not apply them instead of 
stopping the entire boot.


Regards,
Daniel



Yannic


+#endif
  net_load_bootenv=${get_cmd} ${bootenv_addr_r} ${bootenv}
  net_load_overlay=${get_cmd} ${fdtoverlay_addr_r} ${overlay}
  net_apply_overlays=
@@ -36,3 +46,13 @@ net_apply_overlays=
    fi;
    done;
    fi;
+#ifdef CONFIG_CMD_EXTENSION
+net_apply_extensions=
+   setenv extension_overlay_addr ${fdtoverlay_addr_r};
+   setenv extension_overlay_cmd '${get_cmd} ${fdtoverlay_addr_r}
+${extension_overlay_name}';
+   extension scan;
+   extension apply all;
+#else
+net_apply_extensions=echo "no extension command"
+#endif

Re: [PATCH 1/6] include: env: phytec: overlays: Add extension command

[Yannic Moog]

Hello Daniel,

On Sun, 2024-07-07 at 23:07 -0700, Daniel Schultz wrote:
> Add a new environment routine to apply extensions. Our SOM detection
> adds overlays via the extension framework to alter the kernel
> device-tree according to the flashed EEPROM image.
> 
> Signed-off-by: Daniel Schultz 
> ---
>  include/env/phytec/overlays.env | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/env/phytec/overlays.env b/include/env/phytec/overlays.env
> index 78721cde654..50110e731bd 100644
> --- a/include/env/phytec/overlays.env
> +++ b/include/env/phytec/overlays.env
> @@ -23,6 +23,16 @@ mmc_apply_overlays=
>   fi;
>   done;
>   fi;
> +#ifdef CONFIG_CMD_EXTENSION
> +mmc_apply_extensions=
> + setenv extension_overlay_addr ${fdtoverlay_addr_r};
> + setenv extension_overlay_cmd 'load mmc ${mmcdev}:${mmcpart}
> +   ${fdtoverlay_addr_r} ${extension_overlay_name}';
> + extension scan;
> + extension apply all;
> +#else
> +mmc_apply_extensions=echo "no extension command"

Do you think it makes sense to make this fail? I would think "apply_extensions" 
would be
unsuccessful if extensions are not working.

Yannic

> +#endif
>  net_load_bootenv=${get_cmd} ${bootenv_addr_r} ${bootenv}
>  net_load_overlay=${get_cmd} ${fdtoverlay_addr_r} ${overlay}
>  net_apply_overlays=
> @@ -36,3 +46,13 @@ net_apply_overlays=
>   fi;
>   done;
>   fi;
> +#ifdef CONFIG_CMD_EXTENSION
> +net_apply_extensions=
> + setenv extension_overlay_addr ${fdtoverlay_addr_r};
> + setenv extension_overlay_cmd '${get_cmd} ${fdtoverlay_addr_r}
> +  ${extension_overlay_name}';
> + extension scan;
> + extension apply all;
> +#else
> +net_apply_extensions=echo "no extension command"
> +#endif