Re: [PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Mayuresh Chitale]

On Wed, May 17, 2023 at 8:11 PM Tom Rini  wrote:
>
> On Thu, May 04, 2023 at 03:23:26PM +0530, Mayuresh Chitale wrote:
>
> > Detect a FIT when loading from an ext File system and handle it using
> > the FIT SPL support.
> >
> > Signed-off-by: Mayuresh Chitale 
> > ---
> >  common/spl/spl_ext.c | 33 +
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> > index f117c630bf..7b771c41e9 100644
> > --- a/common/spl/spl_ext.c
> > +++ b/common/spl/spl_ext.c
> > @@ -8,6 +8,26 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +
> > +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> > +   ulong size, void *buf)
> > +{
> > + loff_t filelen = (loff_t)load->priv, actlen;
> > + char *filename = (char *)load->filename;
>
> Please build on 32bit platforms such as j721e_evm_r5 as well:
> +common/spl/spl_ext.c:16:26: error: cast from pointer to integer of different 
> size [-Werror=pointer-to-int-cast]
> +   16 | loff_t filelen = (loff_t)load->priv, actlen;
Ok.
>
> --
> Tom

Re: [PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Mayuresh Chitale]

On Fri, May 5, 2023 at 6:11 AM Simon Glass  wrote:
>
> Hi Mayuresh,
>
> On Thu, 4 May 2023 at 03:53, Mayuresh Chitale  
> wrote:
> >
> > Detect a FIT when loading from an ext File system and handle it using
> > the FIT SPL support.
> >
> > Signed-off-by: Mayuresh Chitale 
> > ---
> >  common/spl/spl_ext.c | 33 +
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> > index f117c630bf..7b771c41e9 100644
> > --- a/common/spl/spl_ext.c
> > +++ b/common/spl/spl_ext.c
> > @@ -8,6 +8,26 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +
> > +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> > + ulong size, void *buf)
> > +{
> > +   loff_t filelen = (loff_t)load->priv, actlen;
> > +   char *filename = (char *)load->filename;
> > +   int ret;
> > +
> > +   ret = ext4fs_read(buf, file_offset, filelen, );
>
> If you use the fs_...() interface instead, can you make this function
> generic for all filesystems?
Ok.
>
> > +   if (ret < 0) {
> > +   if (IS_ENABLED(CONFIG_SPL_LIBCOMMON_SUPPORT)) {
> > +   printf("%s: error reading image %s, err - %d\n",
> > +  __func__, filename, ret);
>
> I could be wrong, but I think printf() is silently dropped if that
> option is not enabled, so maybe you don't need the if() checK?
>
> > +   }
> > +   return ret;
> > +   }
> > +
> > +   return actlen;
> > +}
> >
> >  int spl_load_image_ext(struct spl_image_info *spl_image,
> >struct spl_boot_device *bootdev,
> > @@ -47,6 +67,19 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
>
> Really this should not be different from FAT and other filesystems.
> I'm not sure what is involved in making it common, though.
Yes, it can be made generic.
>
> > goto end;
> > }
> >
> > +   if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> > +   image_get_magic(header) == FDT_MAGIC) {
> > +   struct spl_load_info load;
> > +
> > +   debug("Found FIT\n");
> > +   load.read = spl_fit_read;
> > +   load.bl_len = 1;
> > +   load.filename = (void *)filename;
> > +   load.priv = (void *)filelen;
> > +
> > +   return spl_load_simple_fit(spl_image, , 0, header);
> > +   }
> > +
> > err = spl_parse_image_header(spl_image, bootdev, header);
> > if (err < 0) {
> > puts("spl: ext: failed to parse image header\n");
> > --
> > 2.34.1
> >
>
> Regards,
> Simon

Re: [PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Heinrich Schuchardt]

Am 17. Mai 2023 16:41:47 MESZ schrieb Tom Rini :
>On Thu, May 04, 2023 at 03:23:26PM +0530, Mayuresh Chitale wrote:
>
>> Detect a FIT when loading from an ext File system and handle it using
>> the FIT SPL support.
>> 
>> Signed-off-by: Mayuresh Chitale 
>> ---
>>  common/spl/spl_ext.c | 33 +
>>  1 file changed, 33 insertions(+)
>> 
>> diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
>> index f117c630bf..7b771c41e9 100644
>> --- a/common/spl/spl_ext.c
>> +++ b/common/spl/spl_ext.c
>> @@ -8,6 +8,26 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
>> +  ulong size, void *buf)
>> +{
>> +loff_t filelen = (loff_t)load->priv, actlen;

The comma seems to be incorrect.

>> +char *filename = (char *)load->filename;
>
>Please build on 32bit platforms such as j721e_evm_r5 as well:
>+common/spl/spl_ext.c:16:26: error: cast from pointer to integer of different 
>size [-Werror=pointer-to-int-cast]
>+   16 | loff_t filelen = (loff_t)load->priv, actlen;
>

Re: [PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Tom Rini]

On Thu, May 04, 2023 at 03:23:26PM +0530, Mayuresh Chitale wrote:

> Detect a FIT when loading from an ext File system and handle it using
> the FIT SPL support.
> 
> Signed-off-by: Mayuresh Chitale 
> ---
>  common/spl/spl_ext.c | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> index f117c630bf..7b771c41e9 100644
> --- a/common/spl/spl_ext.c
> +++ b/common/spl/spl_ext.c
> @@ -8,6 +8,26 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> +   ulong size, void *buf)
> +{
> + loff_t filelen = (loff_t)load->priv, actlen;
> + char *filename = (char *)load->filename;

Please build on 32bit platforms such as j721e_evm_r5 as well:
+common/spl/spl_ext.c:16:26: error: cast from pointer to integer of different 
size [-Werror=pointer-to-int-cast]
+   16 | loff_t filelen = (loff_t)load->priv, actlen;

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Simon Glass]

Hi Mayuresh,

On Thu, 4 May 2023 at 03:53, Mayuresh Chitale  wrote:
>
> Detect a FIT when loading from an ext File system and handle it using
> the FIT SPL support.
>
> Signed-off-by: Mayuresh Chitale 
> ---
>  common/spl/spl_ext.c | 33 +
>  1 file changed, 33 insertions(+)
>
> diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> index f117c630bf..7b771c41e9 100644
> --- a/common/spl/spl_ext.c
> +++ b/common/spl/spl_ext.c
> @@ -8,6 +8,26 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> + ulong size, void *buf)
> +{
> +   loff_t filelen = (loff_t)load->priv, actlen;
> +   char *filename = (char *)load->filename;
> +   int ret;
> +
> +   ret = ext4fs_read(buf, file_offset, filelen, );

If you use the fs_...() interface instead, can you make this function
generic for all filesystems?

> +   if (ret < 0) {
> +   if (IS_ENABLED(CONFIG_SPL_LIBCOMMON_SUPPORT)) {
> +   printf("%s: error reading image %s, err - %d\n",
> +  __func__, filename, ret);

I could be wrong, but I think printf() is silently dropped if that
option is not enabled, so maybe you don't need the if() checK?

> +   }
> +   return ret;
> +   }
> +
> +   return actlen;
> +}
>
>  int spl_load_image_ext(struct spl_image_info *spl_image,
>struct spl_boot_device *bootdev,
> @@ -47,6 +67,19 @@ int spl_load_image_ext(struct spl_image_info *spl_image,

Really this should not be different from FAT and other filesystems.
I'm not sure what is involved in making it common, though.

> goto end;
> }
>
> +   if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> +   image_get_magic(header) == FDT_MAGIC) {
> +   struct spl_load_info load;
> +
> +   debug("Found FIT\n");
> +   load.read = spl_fit_read;
> +   load.bl_len = 1;
> +   load.filename = (void *)filename;
> +   load.priv = (void *)filelen;
> +
> +   return spl_load_simple_fit(spl_image, , 0, header);
> +   }
> +
> err = spl_parse_image_header(spl_image, bootdev, header);
> if (err < 0) {
> puts("spl: ext: failed to parse image header\n");
> --
> 2.34.1
>

Regards,
Simon

[PATCH v3 4/5] spl: Support loading a FIT from ext FS

[Mayuresh Chitale]

Detect a FIT when loading from an ext File system and handle it using
the FIT SPL support.

Signed-off-by: Mayuresh Chitale 
---
 common/spl/spl_ext.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
index f117c630bf..7b771c41e9 100644
--- a/common/spl/spl_ext.c
+++ b/common/spl/spl_ext.c
@@ -8,6 +8,26 @@
 #include 
 #include 
 #include 
+#include 
+
+static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
+ ulong size, void *buf)
+{
+   loff_t filelen = (loff_t)load->priv, actlen;
+   char *filename = (char *)load->filename;
+   int ret;
+
+   ret = ext4fs_read(buf, file_offset, filelen, );
+   if (ret < 0) {
+   if (IS_ENABLED(CONFIG_SPL_LIBCOMMON_SUPPORT)) {
+   printf("%s: error reading image %s, err - %d\n",
+  __func__, filename, ret);
+   }
+   return ret;
+   }
+
+   return actlen;
+}
 
 int spl_load_image_ext(struct spl_image_info *spl_image,
   struct spl_boot_device *bootdev,
@@ -47,6 +67,19 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
goto end;
}
 
+   if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
+   image_get_magic(header) == FDT_MAGIC) {
+   struct spl_load_info load;
+
+   debug("Found FIT\n");
+   load.read = spl_fit_read;
+   load.bl_len = 1;
+   load.filename = (void *)filename;
+   load.priv = (void *)filelen;
+
+   return spl_load_simple_fit(spl_image, , 0, header);
+   }
+
err = spl_parse_image_header(spl_image, bootdev, header);
if (err < 0) {
puts("spl: ext: failed to parse image header\n");
-- 
2.34.1