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