| Message ID | 878rllclv6.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: > > The user has now the choice to specify the splash location in the MMC > as a raw storage. > > Signed-off-by: Julien Masson <jmasson@baylibre.com> > --- > common/splash.c | 6 ++++++ > common/splash_source.c | 29 +++++++++++++++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/common/splash.c b/common/splash.c > index 0e520cc103..5206e35f74 100644 > --- a/common/splash.c > +++ b/common/splash.c > @@ -39,6 +39,12 @@ static struct splash_location default_splash_locations[] = { > .flags = SPLASH_STORAGE_FS, > .devpart = "0:1", > }, > + { > + .name = "mmc_raw", > + .storage = SPLASH_STORAGE_MMC, > + .flags = SPLASH_STORAGE_RAW, > + .devpart = "0:1", > + }, > { > .name = "usb_fs", > .storage = SPLASH_STORAGE_USB, > diff --git a/common/splash_source.c b/common/splash_source.c > index 87e55a54f8..b4bf6f1336 100644 > --- a/common/splash_source.c > +++ b/common/splash_source.c > @@ -65,6 +65,33 @@ static int splash_nand_read_raw(u32 bmp_load_addr, int offset, size_t read_size) > } > #endif > > +#ifdef CONFIG_CMD_MMC > +static int splash_mmc_read_raw(u32 bmp_load_addr, struct splash_location *location, > + size_t read_size) > +{ > + struct disk_partition partition; > + struct blk_desc *desc; > + lbaint_t blkcnt; > + int ret, n; > + > + ret = part_get_info_by_dev_and_name_or_num("mmc", location->devpart, &desc, > + &partition, 1); > + if (ret < 0) > + return ret; > + > + blkcnt = DIV_ROUND_UP(read_size, partition.blksz); > + n = blk_dread(desc, partition.start, blkcnt, (void *)(uintptr_t)bmp_load_addr); > + > + return (n == blkcnt) ? 0 : -EINVAL; -EIO would be better > +} > +#else > +static int splash_mmc_read_raw(u32 bmp_load_addr, int offset, size_t read_size) > +{ > + debug("%s: mmc support not available\n", __func__); > + return -ENOSYS; Rather than the #ifdef you should be able to put these two lines in the first function. Does patman not warn you about this sort of thing? > +} > +#endif > + > static int splash_storage_read_raw(struct splash_location *location, > u32 bmp_load_addr, size_t read_size) > { > @@ -75,6 +102,8 @@ static int splash_storage_read_raw(struct splash_location *location, > > offset = location->offset; > switch (location->storage) { > + case SPLASH_STORAGE_MMC: > + return splash_mmc_read_raw(bmp_load_addr, location, read_size); > case SPLASH_STORAGE_NAND: > return splash_nand_read_raw(bmp_load_addr, offset, read_size); > case SPLASH_STORAGE_SF: > -- > 2.37.3 > Regards, SImon
Hi Simon, On Thu 13 Oct 2022 at 17:51, Simon Glass <sjg@chromium.org> wrote: > HI Julien, > > On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: >> >> The user has now the choice to specify the splash location in the MMC >> as a raw storage. >> >> Signed-off-by: Julien Masson <jmasson@baylibre.com> >> --- >> common/splash.c | 6 ++++++ >> common/splash_source.c | 29 +++++++++++++++++++++++++++++ >> 2 files changed, 35 insertions(+) >> >> diff --git a/common/splash.c b/common/splash.c >> index 0e520cc103..5206e35f74 100644 >> --- a/common/splash.c >> +++ b/common/splash.c >> @@ -39,6 +39,12 @@ static struct splash_location default_splash_locations[] = { >> .flags = SPLASH_STORAGE_FS, >> .devpart = "0:1", >> }, >> + { >> + .name = "mmc_raw", >> + .storage = SPLASH_STORAGE_MMC, >> + .flags = SPLASH_STORAGE_RAW, >> + .devpart = "0:1", >> + }, >> { >> .name = "usb_fs", >> .storage = SPLASH_STORAGE_USB, >> diff --git a/common/splash_source.c b/common/splash_source.c >> index 87e55a54f8..b4bf6f1336 100644 >> --- a/common/splash_source.c >> +++ b/common/splash_source.c >> @@ -65,6 +65,33 @@ static int splash_nand_read_raw(u32 bmp_load_addr, int offset, size_t read_size) >> } >> #endif >> >> +#ifdef CONFIG_CMD_MMC >> +static int splash_mmc_read_raw(u32 bmp_load_addr, struct splash_location *location, >> + size_t read_size) >> +{ >> + struct disk_partition partition; >> + struct blk_desc *desc; >> + lbaint_t blkcnt; >> + int ret, n; >> + >> + ret = part_get_info_by_dev_and_name_or_num("mmc", location->devpart, &desc, >> + &partition, 1); >> + if (ret < 0) >> + return ret; >> + >> + blkcnt = DIV_ROUND_UP(read_size, partition.blksz); >> + n = blk_dread(desc, partition.start, blkcnt, (void *)(uintptr_t)bmp_load_addr); >> + >> + return (n == blkcnt) ? 0 : -EINVAL; > > -EIO would be better Yes I will change this in V2. > >> +} >> +#else >> +static int splash_mmc_read_raw(u32 bmp_load_addr, int offset, size_t read_size) I just discovered that my the second argument type is wrong. I will fix that in V2, sorry. >> +{ >> + debug("%s: mmc support not available\n", __func__); >> + return -ENOSYS; > > Rather than the #ifdef you should be able to put these two lines in > the first function. Does patman not warn you about this sort of thing? Yes that is true I can place the #ifdef inside the function but I've "voluntary" done that to stay consistent with other functions: splash_(sf|nand).* No I did not use patman, I've run checkpatch first and git send-email: $ ./scripts/checkpatch.pl --strict --u-boot But even with patman I don't see the warning you are talking: $ ./tools/patman/patman -n Cleaned 2 patches 0 errors, 1 warnings, 0 checks for 0002-splash-support-raw-image-from-MMC.patch: common/splash_source.c:68: warning: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible checkpatch.pl found 0 error(s), 1 warning(s), 0 checks(s) Alias 'splash' not found Alias 'splash' not found Not sending emails due to errors/warnings Dry run, so not doing much. But I would do this: ... What command do you use to have this warning ? Thanks > >> +} >> +#endif >> + >> static int splash_storage_read_raw(struct splash_location *location, >> u32 bmp_load_addr, size_t read_size) >> { >> @@ -75,6 +102,8 @@ static int splash_storage_read_raw(struct splash_location *location, >> >> offset = location->offset; >> switch (location->storage) { >> + case SPLASH_STORAGE_MMC: >> + return splash_mmc_read_raw(bmp_load_addr, location, read_size); >> case SPLASH_STORAGE_NAND: >> return splash_nand_read_raw(bmp_load_addr, offset, read_size); >> case SPLASH_STORAGE_SF: >> -- >> 2.37.3 >> > > Regards, > SImon
Hi Julien, On Thu, 13 Oct 2022 at 10:08, Julien Masson <jmasson@baylibre.com> wrote: > > Hi Simon, > > On Thu 13 Oct 2022 at 17:51, Simon Glass <sjg@chromium.org> wrote: > > > HI Julien, > > > > On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson@baylibre.com> wrote: > >> > >> The user has now the choice to specify the splash location in the MMC > >> as a raw storage. > >> > >> Signed-off-by: Julien Masson <jmasson@baylibre.com> > >> --- > >> common/splash.c | 6 ++++++ > >> common/splash_source.c | 29 +++++++++++++++++++++++++++++ > >> 2 files changed, 35 insertions(+) > >> > >> diff --git a/common/splash.c b/common/splash.c > >> index 0e520cc103..5206e35f74 100644 > >> --- a/common/splash.c > >> +++ b/common/splash.c > >> @@ -39,6 +39,12 @@ static struct splash_location default_splash_locations[] = { > >> .flags = SPLASH_STORAGE_FS, > >> .devpart = "0:1", > >> }, > >> + { > >> + .name = "mmc_raw", > >> + .storage = SPLASH_STORAGE_MMC, > >> + .flags = SPLASH_STORAGE_RAW, > >> + .devpart = "0:1", > >> + }, > >> { > >> .name = "usb_fs", > >> .storage = SPLASH_STORAGE_USB, > >> diff --git a/common/splash_source.c b/common/splash_source.c > >> index 87e55a54f8..b4bf6f1336 100644 > >> --- a/common/splash_source.c > >> +++ b/common/splash_source.c > >> @@ -65,6 +65,33 @@ static int splash_nand_read_raw(u32 bmp_load_addr, int offset, size_t read_size) > >> } > >> #endif > >> > >> +#ifdef CONFIG_CMD_MMC > >> +static int splash_mmc_read_raw(u32 bmp_load_addr, struct splash_location *location, > >> + size_t read_size) > >> +{ > >> + struct disk_partition partition; > >> + struct blk_desc *desc; > >> + lbaint_t blkcnt; > >> + int ret, n; > >> + > >> + ret = part_get_info_by_dev_and_name_or_num("mmc", location->devpart, &desc, > >> + &partition, 1); > >> + if (ret < 0) > >> + return ret; > >> + > >> + blkcnt = DIV_ROUND_UP(read_size, partition.blksz); > >> + n = blk_dread(desc, partition.start, blkcnt, (void *)(uintptr_t)bmp_load_addr); > >> + > >> + return (n == blkcnt) ? 0 : -EINVAL; > > > > -EIO would be better > > Yes I will change this in V2. > > > > >> +} > >> +#else > >> +static int splash_mmc_read_raw(u32 bmp_load_addr, int offset, size_t read_size) > > I just discovered that my the second argument type is wrong. > I will fix that in V2, sorry. > > >> +{ > >> + debug("%s: mmc support not available\n", __func__); > >> + return -ENOSYS; > > > > Rather than the #ifdef you should be able to put these two lines in > > the first function. Does patman not warn you about this sort of thing? > > Yes that is true I can place the #ifdef inside the function but I've > "voluntary" done that to stay consistent with other functions: > splash_(sf|nand).* > > No I did not use patman, I've run checkpatch first and git send-email: > $ ./scripts/checkpatch.pl --strict --u-boot > > But even with patman I don't see the warning you are talking: > $ ./tools/patman/patman -n > Cleaned 2 patches > 0 errors, 1 warnings, 0 checks for 0002-splash-support-raw-image-from-MMC.patch: > common/splash_source.c:68: warning: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible ^ That is the warning > checkpatch.pl found 0 error(s), 1 warning(s), 0 checks(s) > Alias 'splash' not found > Alias 'splash' not found > Not sending emails due to errors/warnings > Dry run, so not doing much. But I would do this: > ... > > What command do you use to have this warning ? > > Thanks > > > > >> +} > >> +#endif > >> + > >> static int splash_storage_read_raw(struct splash_location *location, > >> u32 bmp_load_addr, size_t read_size) > >> { > >> @@ -75,6 +102,8 @@ static int splash_storage_read_raw(struct splash_location *location, > >> > >> offset = location->offset; > >> switch (location->storage) { > >> + case SPLASH_STORAGE_MMC: > >> + return splash_mmc_read_raw(bmp_load_addr, location, read_size); > >> case SPLASH_STORAGE_NAND: > >> return splash_nand_read_raw(bmp_load_addr, offset, read_size); > >> case SPLASH_STORAGE_SF: > >> -- > >> 2.37.3 > >> > > > > Regards, > > SImon > > -- > Julien Masson Regards, Simon
diff --git a/common/splash.c b/common/splash.c index 0e520cc103..5206e35f74 100644 --- a/common/splash.c +++ b/common/splash.c @@ -39,6 +39,12 @@ static struct splash_location default_splash_locations[] = { .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, + { + .name = "mmc_raw", + .storage = SPLASH_STORAGE_MMC, + .flags = SPLASH_STORAGE_RAW, + .devpart = "0:1", + }, { .name = "usb_fs", .storage = SPLASH_STORAGE_USB, diff --git a/common/splash_source.c b/common/splash_source.c index 87e55a54f8..b4bf6f1336 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -65,6 +65,33 @@ static int splash_nand_read_raw(u32 bmp_load_addr, int offset, size_t read_size) } #endif +#ifdef CONFIG_CMD_MMC +static int splash_mmc_read_raw(u32 bmp_load_addr, struct splash_location *location, + size_t read_size) +{ + struct disk_partition partition; + struct blk_desc *desc; + lbaint_t blkcnt; + int ret, n; + + ret = part_get_info_by_dev_and_name_or_num("mmc", location->devpart, &desc, + &partition, 1); + if (ret < 0) + return ret; + + blkcnt = DIV_ROUND_UP(read_size, partition.blksz); + n = blk_dread(desc, partition.start, blkcnt, (void *)(uintptr_t)bmp_load_addr); + + return (n == blkcnt) ? 0 : -EINVAL; +} +#else +static int splash_mmc_read_raw(u32 bmp_load_addr, int offset, size_t read_size) +{ + debug("%s: mmc support not available\n", __func__); + return -ENOSYS; +} +#endif + static int splash_storage_read_raw(struct splash_location *location, u32 bmp_load_addr, size_t read_size) { @@ -75,6 +102,8 @@ static int splash_storage_read_raw(struct splash_location *location, offset = location->offset; switch (location->storage) { + case SPLASH_STORAGE_MMC: + return splash_mmc_read_raw(bmp_load_addr, location, read_size); case SPLASH_STORAGE_NAND: return splash_nand_read_raw(bmp_load_addr, offset, read_size); case SPLASH_STORAGE_SF:
The user has now the choice to specify the splash location in the MMC as a raw storage. Signed-off-by: Julien Masson <jmasson@baylibre.com> --- common/splash.c | 6 ++++++ common/splash_source.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+)