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

[Julien Masson]

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
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