[RESEND PATCH 1/2] splash: support raw image from MMC

[Simon Glass]

Hi Julien,

On Thu, 13 Oct 2022 at 10:08, Julien Masson <jmasson at baylibre.com> wrote:
>
> Hi Simon,
>
> On Thu 13 Oct 2022 at 17:51, Simon Glass <sjg at chromium.org> wrote:
>
> > HI Julien,
> >
> > On Wed, 12 Oct 2022 at 05:38, Julien Masson <jmasson at 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 at 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