| Message ID | 877d15clv5.fsf@baylibre.com |
|---|---|
| State | Superseded |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | splash: add more options/support | expand |
Hi Julien, On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: > > By default several types of splash locations are supported and the > user can select one of them through environment var (splashsource). > > However the devpart is still hardcoded and we cannot change it from > the environment. > > This patch add the support of "splashdevpart" which allow the user to > set the devpart though this environment variable. > > Example: image located in splashscreen partition (MMC as raw) > ``` > splashsource=mmc_raw > splashdevpart=0#splashscreen > ``` > > Signed-off-by: Julien Masson <jmasson@baylibre.com> > --- > common/splash_source.c | 5 +++++ > 1 file changed, 5 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/common/splash_source.c b/common/splash_source.c > index b4bf6f1336..1f99f44f78 100644 > --- a/common/splash_source.c > +++ b/common/splash_source.c > @@ -451,6 +451,7 @@ int splash_source_load(struct splash_location *locations, uint size) > { > struct splash_location *splash_location; > char *env_splashimage_value; > + char *env_splashdevpart_value; How about just 'devpar' as it is shorter and easier to read? > u32 bmp_load_addr; > > env_splashimage_value = env_get("splashimage"); > @@ -467,6 +468,10 @@ int splash_source_load(struct splash_location *locations, uint size) > if (!splash_location) > return -EINVAL; > > + env_splashdevpart_value = env_get("splashdevpart"); > + if (env_splashdevpart_value) > + splash_location->devpart = env_splashdevpart_value; > + > if (splash_location->flags == SPLASH_STORAGE_RAW) > return splash_load_raw(splash_location, bmp_load_addr); > else if (splash_location->flags == SPLASH_STORAGE_FS) > -- > 2.37.3 > Regards, Simon
Hi Simon, Thanks for the review. On Thu 13 Oct 2022 at 17:44, Simon Glass <sjg@chromium.org> wrote: > Hi Julien, > > On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: >> >> By default several types of splash locations are supported and the >> user can select one of them through environment var (splashsource). >> >> However the devpart is still hardcoded and we cannot change it from >> the environment. >> >> This patch add the support of "splashdevpart" which allow the user to >> set the devpart though this environment variable. >> >> Example: image located in splashscreen partition (MMC as raw) >> ``` >> splashsource=mmc_raw >> splashdevpart=0#splashscreen >> ``` >> >> Signed-off-by: Julien Masson <jmasson@baylibre.com> >> --- >> common/splash_source.c | 5 +++++ >> 1 file changed, 5 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> >> >> diff --git a/common/splash_source.c b/common/splash_source.c >> index b4bf6f1336..1f99f44f78 100644 >> --- a/common/splash_source.c >> +++ b/common/splash_source.c >> @@ -451,6 +451,7 @@ int splash_source_load(struct splash_location *locations, uint size) >> { >> struct splash_location *splash_location; >> char *env_splashimage_value; >> + char *env_splashdevpart_value; > > How about just 'devpar' as it is shorter and easier to read? > Yes I initially follow the same "syntax" of splashimage var but I agree it can be shorter, what name would you prefer: - char *devpart - char *env_devpart - char *env_devpart_value ? >> u32 bmp_load_addr; >> >> env_splashimage_value = env_get("splashimage"); >> @@ -467,6 +468,10 @@ int splash_source_load(struct splash_location *locations, uint size) >> if (!splash_location) >> return -EINVAL; >> >> + env_splashdevpart_value = env_get("splashdevpart"); >> + if (env_splashdevpart_value) >> + splash_location->devpart = env_splashdevpart_value; >> + >> if (splash_location->flags == SPLASH_STORAGE_RAW) >> return splash_load_raw(splash_location, bmp_load_addr); >> else if (splash_location->flags == SPLASH_STORAGE_FS) >> -- >> 2.37.3 >> > > Regards, > Simon
Hi Julien, On Thu, 13 Oct 2022 at 09:51, Julien Masson <jmasson@baylibre.com> wrote: > > Hi Simon, > > Thanks for the review. > > On Thu 13 Oct 2022 at 17:44, Simon Glass <sjg@chromium.org> wrote: > > > Hi Julien, > > > > On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: > >> > >> By default several types of splash locations are supported and the > >> user can select one of them through environment var (splashsource). > >> > >> However the devpart is still hardcoded and we cannot change it from > >> the environment. > >> > >> This patch add the support of "splashdevpart" which allow the user to > >> set the devpart though this environment variable. > >> > >> Example: image located in splashscreen partition (MMC as raw) > >> ``` > >> splashsource=mmc_raw > >> splashdevpart=0#splashscreen > >> ``` > >> > >> Signed-off-by: Julien Masson <jmasson@baylibre.com> > >> --- > >> common/splash_source.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > >> > >> diff --git a/common/splash_source.c b/common/splash_source.c > >> index b4bf6f1336..1f99f44f78 100644 > >> --- a/common/splash_source.c > >> +++ b/common/splash_source.c > >> @@ -451,6 +451,7 @@ int splash_source_load(struct splash_location *locations, uint size) > >> { > >> struct splash_location *splash_location; > >> char *env_splashimage_value; > >> + char *env_splashdevpart_value; > > > > How about just 'devpar' as it is shorter and easier to read? > > > > Yes I initially follow the same "syntax" of splashimage var but I agree > it can be shorter, what name would you prefer: > - char *devpart > - char *env_devpart > - char *env_devpart_value > ? devpart seems good to me > > >> u32 bmp_load_addr; > >> > >> env_splashimage_value = env_get("splashimage"); > >> @@ -467,6 +468,10 @@ int splash_source_load(struct splash_location *locations, uint size) > >> if (!splash_location) > >> return -EINVAL; > >> > >> + env_splashdevpart_value = env_get("splashdevpart"); > >> + if (env_splashdevpart_value) > >> + splash_location->devpart = env_splashdevpart_value; > >> + > >> if (splash_location->flags == SPLASH_STORAGE_RAW) > >> return splash_load_raw(splash_location, bmp_load_addr); > >> else if (splash_location->flags == SPLASH_STORAGE_FS) > >> -- > >> 2.37.3 > >> Regards, Simon
Hi Simon, On Mon 17 Oct 2022 at 09:49, Simon Glass <sjg@chromium.org> wrote: > Hi Julien, > > > On Thu, 13 Oct 2022 at 09:51, Julien Masson <jmasson@baylibre.com> wrote: >> >> Hi Simon, >> >> Thanks for the review. >> >> On Thu 13 Oct 2022 at 17:44, Simon Glass <sjg@chromium.org> wrote: >> >>> Hi Julien, >>> >>> On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: >>>> >>>> By default several types of splash locations are supported and the >>>> user can select one of them through environment var (splashsource). >>>> >>>> However the devpart is still hardcoded and we cannot change it from >>>> the environment. >>>> >>>> This patch add the support of "splashdevpart" which allow the user to >>>> set the devpart though this environment variable. >>>> >>>> Example: image located in splashscreen partition (MMC as raw) >>>> ``` >>>> splashsource=mmc_raw >>>> splashdevpart=0#splashscreen >>>> ``` >>>> >>>> Signed-off-by: Julien Masson <jmasson@baylibre.com> >>>> --- >>>> common/splash_source.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> diff --git a/common/splash_source.c b/common/splash_source.c >>>> index b4bf6f1336..1f99f44f78 100644 >>>> --- a/common/splash_source.c >>>> +++ b/common/splash_source.c >>>> @@ -451,6 +451,7 @@ int splash_source_load(struct splash_location *locations, uint size) >>>> { >>>> struct splash_location *splash_location; >>>> char *env_splashimage_value; >>>> + char *env_splashdevpart_value; >>> >>> How about just 'devpar' as it is shorter and easier to read? >>> >> >> Yes I initially follow the same "syntax" of splashimage var but I agree >> it can be shorter, what name would you prefer: >> - char *devpart >> - char *env_devpart >> - char *env_devpart_value >> ? > > devpart seems good to me Got it thanks, I'll change that in v2. > > >> >>>> u32 bmp_load_addr; >>>> >>>> env_splashimage_value = env_get("splashimage"); >>>> @@ -467,6 +468,10 @@ int splash_source_load(struct splash_location *locations, uint size) >>>> if (!splash_location) >>>> return -EINVAL; >>>> >>>> + env_splashdevpart_value = env_get("splashdevpart"); >>>> + if (env_splashdevpart_value) >>>> + splash_location->devpart = env_splashdevpart_value; >>>> + >>>> if (splash_location->flags == SPLASH_STORAGE_RAW) >>>> return splash_load_raw(splash_location, bmp_load_addr); >>>> else if (splash_location->flags == SPLASH_STORAGE_FS) >>>> -- >>>> 2.37.3 >>>> > Regards, > Simon
diff --git a/common/splash_source.c b/common/splash_source.c index b4bf6f1336..1f99f44f78 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -451,6 +451,7 @@ int splash_source_load(struct splash_location *locations, uint size) { struct splash_location *splash_location; char *env_splashimage_value; + char *env_splashdevpart_value; u32 bmp_load_addr; env_splashimage_value = env_get("splashimage"); @@ -467,6 +468,10 @@ int splash_source_load(struct splash_location *locations, uint size) if (!splash_location) return -EINVAL; + env_splashdevpart_value = env_get("splashdevpart"); + if (env_splashdevpart_value) + splash_location->devpart = env_splashdevpart_value; + if (splash_location->flags == SPLASH_STORAGE_RAW) return splash_load_raw(splash_location, bmp_load_addr); else if (splash_location->flags == SPLASH_STORAGE_FS)
By default several types of splash locations are supported and the user can select one of them through environment var (splashsource). However the devpart is still hardcoded and we cannot change it from the environment. This patch add the support of "splashdevpart" which allow the user to set the devpart though this environment variable. Example: image located in splashscreen partition (MMC as raw) ``` splashsource=mmc_raw splashdevpart=0#splashscreen ``` Signed-off-by: Julien Masson <jmasson@baylibre.com> --- common/splash_source.c | 5 +++++ 1 file changed, 5 insertions(+)