diff mbox series

dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration

Message ID 20220620111354.448512-1-jh80.chung@samsung.com
State Accepted, archived
Delegated to: Mattijs Korpershoek
Headers show
Series dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration | expand

Commit Message

Jaehoon Chung June 20, 2022, 11:13 a.m. UTC
Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
If name is longer than default size, it can do wrong behavior during updating
image. So it need to change the proper maximum size.

This patch is proviced the solution to change value with configuration.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/dfu/Kconfig | 9 +++++++++
 include/dfu.h       | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Lukasz Majewski June 20, 2022, 11:23 a.m. UTC | #1
Hi Jaehoon,

> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
> If name is longer than default size, it can do wrong behavior during
> updating image. So it need to change the proper maximum size.
> 
> This patch is proviced the solution to change value with
> configuration.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/dfu/Kconfig | 9 +++++++++
>  include/dfu.h       | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index 8d7f13dcb0b5..a181f0b8d7ba 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -111,5 +111,14 @@ config SYS_DFU_MAX_FILE_SIZE
>  	  the buffer once we've been given the whole file.  Define
>  	  this to the maximum filesize (in bytes) for the buffer.
>  	  If undefined it defaults to the
> CONFIG_SYS_DFU_DATA_BUF_SIZE. +
> +config DFU_NAME_MAX_SIZE
> +	int "Size of the name to be added in dfu entity"
> +	default 32
> +	depends on DFU
> +	help
> +	  This value is used to maximum size. If name is longer than
> default size,
> +	  we need to change the proper maximum size.
> +
>  endif
>  endmenu
> diff --git a/include/dfu.h b/include/dfu.h
> index dcb9cd9d799a..948596f367d9 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -99,7 +99,7 @@ struct virt_internal_data {
>  	int dev_num;
>  };
>  
> -#define DFU_NAME_SIZE			32
> +#define DFU_NAME_SIZE			CONFIG_DFU_NAME_MAX_SIZE
>  #ifndef DFU_DEFAULT_POLL_TIMEOUT
>  #define DFU_DEFAULT_POLL_TIMEOUT 0
>  #endif

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Mattijs Korpershoek Oct. 31, 2023, 2:47 p.m. UTC | #2
Hi Jaehoon,

Thank you for your patch.

On lun., juin 20, 2022 at 20:13, Jaehoon Chung <jh80.chung@samsung.com> wrote:

> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
> If name is longer than default size, it can do wrong behavior during updating
> image. So it need to change the proper maximum size.
>
> This patch is proviced the solution to change value with configuration.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

I could not find a maximum size in the DFU 1.1 spec [1] neither in the
dfu-util codebase.

[1] https://www.usb.org/sites/default/files/DFU_1.1.pdf

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  drivers/dfu/Kconfig | 9 +++++++++
>  include/dfu.h       | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index 8d7f13dcb0b5..a181f0b8d7ba 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -111,5 +111,14 @@ config SYS_DFU_MAX_FILE_SIZE
>  	  the buffer once we've been given the whole file.  Define
>  	  this to the maximum filesize (in bytes) for the buffer.
>  	  If undefined it defaults to the CONFIG_SYS_DFU_DATA_BUF_SIZE.
> +
> +config DFU_NAME_MAX_SIZE
> +	int "Size of the name to be added in dfu entity"
> +	default 32
> +	depends on DFU
> +	help
> +	  This value is used to maximum size. If name is longer than default size,
> +	  we need to change the proper maximum size.
> +
>  endif
>  endmenu
> diff --git a/include/dfu.h b/include/dfu.h
> index dcb9cd9d799a..948596f367d9 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -99,7 +99,7 @@ struct virt_internal_data {
>  	int dev_num;
>  };
>  
> -#define DFU_NAME_SIZE			32
> +#define DFU_NAME_SIZE			CONFIG_DFU_NAME_MAX_SIZE
>  #ifndef DFU_DEFAULT_POLL_TIMEOUT
>  #define DFU_DEFAULT_POLL_TIMEOUT 0
>  #endif
> -- 
> 2.25.1
Mattijs Korpershoek Oct. 31, 2023, 2:50 p.m. UTC | #3
Hi,

On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
> If name is longer than default size, it can do wrong behavior during updating
> image. So it need to change the proper maximum size.
> 
> This patch is proviced the solution to change value with configuration.
> 
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)

[1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/de9b2e10f10996050a10998a0836abe2f9e425e3

--
Mattijs
Mattijs Korpershoek Nov. 2, 2023, 9:15 a.m. UTC | #4
Hi Jaehoon,

On mar., oct. 31, 2023 at 15:50, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:

> Hi,
>
> On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
>> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
>> If name is longer than default size, it can do wrong behavior during updating
>> image. So it need to change the proper maximum size.
>> 
>> This patch is proviced the solution to change value with configuration.
>> 
>> 
>> [...]
>
> Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>
> [1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/de9b2e10f10996050a10998a0836abe2f9e425e3


This patch breaks CI for both arm32 and arm64 platforms:

* https://source.denx.de/u-boot/custodians/u-boot-dfu/-/jobs/725163
* https://source.denx.de/u-boot/custodians/u-boot-dfu/-/jobs/725164

It breaks because multiple boards which include dfu.h without having
CONFIG_DFU being set.

Tom attempted to fix this with:
https://patchwork.ozlabs.org/project/uboot/list/?series=369275

But that had some different issues, see:
https://libera.irclog.whitequark.org/u-boot/2023-10-31#35154532;

I've fixed it up with the following diff:

diff --git a/include/dfu.h b/include/dfu.h
index 4573f753c565..2c3ffa3f9297 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -99,7 +99,12 @@ struct virt_internal_data {
        int dev_num;
 };
 
+
+#if defined(CONFIG_DFU_NAME_MAX_SIZE)
 #define DFU_NAME_SIZE                  CONFIG_DFU_NAME_MAX_SIZE
+#else
+#define DFU_NAME_SIZE                  32
+#endif
 #ifndef DFU_DEFAULT_POLL_TIMEOUT
 #define DFU_DEFAULT_POLL_TIMEOUT 0
 #endif

If you have a better idea to fix this, can you please let me know?
Otherwise, I will squash this to keep CI green.

Thank you

>
> --
> Mattijs
Jaehoon Chung Nov. 3, 2023, 1:07 a.m. UTC | #5
Hi Mathtjjs,

> -----Original Message-----
> From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Sent: Thursday, November 2, 2023 6:15 PM
> To: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: lukma@denx.de; u-boot@lists.denx.de
> Subject: Re: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
> 
> Hi Jaehoon,
> 
> On mar., oct. 31, 2023 at 15:50, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:
> 
> > Hi,
> >
> > On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
> >> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
> >> If name is longer than default size, it can do wrong behavior during updating
> >> image. So it need to change the proper maximum size.
> >>
> >> This patch is proviced the solution to change value with configuration.
> >>
> >>
> >> [...]
> >
> > Thanks, Applied to https://protect2.fireeye.com/v1/url?k=195a8ba3-78d19e99-195b00ec-74fe4860008a-
> e609da47a4bcadff&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> boot%2Fcustodians%2Fu-boot-dfu (u-boot-dfu-next)
> >
> > [1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
> >       https://protect2.fireeye.com/v1/url?k=b04b2ae8-d1c03fd2-b04aa1a7-74fe4860008a-
> d8ec8502221dcca0&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fcommit%2Fde9b2e10f10996050a10998a0836abe2f9e425e3
> 
> 
> This patch breaks CI for both arm32 and arm64 platforms:
> 
> * https://protect2.fireeye.com/v1/url?k=3179f7cc-50f2e2f6-31787c83-74fe4860008a-
> a4f5d2c73833c4d5&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725163
> * https://protect2.fireeye.com/v1/url?k=22773f8e-43fc2ab4-2276b4c1-74fe4860008a-
> f6dc3d75fe9b283a&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725164

I couldn't access the above CI URL.

> 
> It breaks because multiple boards which include dfu.h without having
> CONFIG_DFU being set.
> 
> Tom attempted to fix this with:
> https://protect2.fireeye.com/v1/url?k=452a9007-24a1853d-452b1b48-74fe4860008a-
> a88f870953497071&q=1&e=b6fd9ce6-85b6-46a3-81aa-
> 8aa89d16a9ed&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D369275
> 
> But that had some different issues, see:
> https://protect2.fireeye.com/v1/url?k=f1c007bd-904b1287-f1c18cf2-74fe4860008a-
> a305de1bab5bdff1&q=1&e=b6fd9ce6-85b6-46a3-81aa-
> 8aa89d16a9ed&u=https%3A%2F%2Flibera.irclog.whitequark.org%2Fu-boot%2F2023-10-31%2335154532;
> 
> I've fixed it up with the following diff:
> 
> diff --git a/include/dfu.h b/include/dfu.h
> index 4573f753c565..2c3ffa3f9297 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -99,7 +99,12 @@ struct virt_internal_data {
>         int dev_num;
>  };
> 
> +
> +#if defined(CONFIG_DFU_NAME_MAX_SIZE)
>  #define DFU_NAME_SIZE                  CONFIG_DFU_NAME_MAX_SIZE
> +#else
> +#define DFU_NAME_SIZE                  32
> +#endif
>  #ifndef DFU_DEFAULT_POLL_TIMEOUT
>  #define DFU_DEFAULT_POLL_TIMEOUT 0
>  #endif
> 
> If you have a better idea to fix this, can you please let me know?

After checking this, let you inform.

> Otherwise, I will squash this to keep CI green.
> 
> Thank you
> 
> >
> > --
> > Mattijs
Jaehoon Chung Nov. 3, 2023, 5:49 a.m. UTC | #6
Hi,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Friday, November 3, 2023 10:07 AM
> To: 'Mattijs Korpershoek' <mkorpershoek@baylibre.com>
> Cc: lukma@denx.de; u-boot@lists.denx.de
> Subject: RE: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
> 
> Hi Mathtjjs,
> 
> > -----Original Message-----
> > From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> > Sent: Thursday, November 2, 2023 6:15 PM
> > To: Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: lukma@denx.de; u-boot@lists.denx.de
> > Subject: Re: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
> >
> > Hi Jaehoon,
> >
> > On mar., oct. 31, 2023 at 15:50, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:
> >
> > > Hi,
> > >
> > > On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
> > >> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
> > >> If name is longer than default size, it can do wrong behavior during updating
> > >> image. So it need to change the proper maximum size.
> > >>
> > >> This patch is proviced the solution to change value with configuration.
> > >>
> > >>
> > >> [...]
> > >
> > > Thanks, Applied to https://protect2.fireeye.com/v1/url?k=195a8ba3-78d19e99-195b00ec-74fe4860008a-
> > e609da47a4bcadff&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> > boot%2Fcustodians%2Fu-boot-dfu (u-boot-dfu-next)
> > >
> > > [1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
> > >       https://protect2.fireeye.com/v1/url?k=b04b2ae8-d1c03fd2-b04aa1a7-74fe4860008a-
> > d8ec8502221dcca0&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fcommit%2Fde9b2e10f10996050a10998a0836abe2f9e425e3
> >
> >
> > This patch breaks CI for both arm32 and arm64 platforms:
> >
> > * https://protect2.fireeye.com/v1/url?k=3179f7cc-50f2e2f6-31787c83-74fe4860008a-
> > a4f5d2c73833c4d5&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725163
> > * https://protect2.fireeye.com/v1/url?k=22773f8e-43fc2ab4-2276b4c1-74fe4860008a-
> > f6dc3d75fe9b283a&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725164
> 
> I couldn't access the above CI URL.
> 
> >
> > It breaks because multiple boards which include dfu.h without having
> > CONFIG_DFU being set.
> >
> > Tom attempted to fix this with:
> > https://protect2.fireeye.com/v1/url?k=452a9007-24a1853d-452b1b48-74fe4860008a-
> > a88f870953497071&q=1&e=b6fd9ce6-85b6-46a3-81aa-
> > 8aa89d16a9ed&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D369275
> >
> > But that had some different issues, see:
> > https://protect2.fireeye.com/v1/url?k=f1c007bd-904b1287-f1c18cf2-74fe4860008a-
> > a305de1bab5bdff1&q=1&e=b6fd9ce6-85b6-46a3-81aa-
> > 8aa89d16a9ed&u=https%3A%2F%2Flibera.irclog.whitequark.org%2Fu-boot%2F2023-10-31%2335154532;
> >
> > I've fixed it up with the following diff:
> >
> > diff --git a/include/dfu.h b/include/dfu.h
> > index 4573f753c565..2c3ffa3f9297 100644
> > --- a/include/dfu.h
> > +++ b/include/dfu.h
> > @@ -99,7 +99,12 @@ struct virt_internal_data {
> >         int dev_num;
> >  };
> >
> > +
> > +#if defined(CONFIG_DFU_NAME_MAX_SIZE)
> >  #define DFU_NAME_SIZE                  CONFIG_DFU_NAME_MAX_SIZE
> > +#else
> > +#define DFU_NAME_SIZE                  32
> > +#endif
> >  #ifndef DFU_DEFAULT_POLL_TIMEOUT
> >  #define DFU_DEFAULT_POLL_TIMEOUT 0
> >  #endif
> >
> > If you have a better idea to fix this, can you please let me know?
> 
> After checking this, let you inform.

It seems that it is depended on USB_GADGET and SPL_USB_GADGET. How about this?
About <dfu.h> is included in some board files, It needs to check more.

diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
index 6502aba96f54..9922da56e704 100644
--- a/drivers/dfu/Kconfig
+++ b/drivers/dfu/Kconfig
@@ -13,6 +13,14 @@ config DFU_OVER_TFTP
        bool
        depends on NET

+config DFU_NAME_MAX_SIZE
+       int "Size of the name to be added in dfu entity"
+       default 32
+       depends on DFU || USB_GADGET_DOWNLOAD || SPL_USB_GADGET
+       help
+         This value is used to maximum size. If name is longer than default size,
+         we need to change the proper maximum size.
+
 if DFU
 config DFU_WRITE_ALT
        bool
@@ -112,13 +120,5 @@ config SYS_DFU_MAX_FILE_SIZE
          this to the maximum filesize (in bytes) for the buffer.
          If undefined it defaults to the CONFIG_SYS_DFU_DATA_BUF_SIZE.

-config DFU_NAME_MAX_SIZE
-       int "Size of the name to be added in dfu entity"
-       default 32
-       depends on DFU
-       help
-         This value is used to maximum size. If name is longer than default size,
-         we need to change the proper maximum size.

I'm running CI with some patch. After checked, I will inform.

Best Regards,
Jaehoon Chung

> 
> > Otherwise, I will squash this to keep CI green.
> >
> > Thank you
> >
> > >
> > > --
> > > Mattijs
Mattijs Korpershoek Nov. 3, 2023, 9:43 a.m. UTC | #7
Hi Jaehoon,

On ven., nov. 03, 2023 at 10:07, "Jaehoon Chung" <jh80.chung@samsung.com> wrote:

> Hi Mathtjjs,
>
>> -----Original Message-----
>> From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> Sent: Thursday, November 2, 2023 6:15 PM
>> To: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: lukma@denx.de; u-boot@lists.denx.de
>> Subject: Re: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>> 
>> Hi Jaehoon,
>> 
>> On mar., oct. 31, 2023 at 15:50, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:
>> 
>> > Hi,
>> >
>> > On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
>> >> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
>> >> If name is longer than default size, it can do wrong behavior during updating
>> >> image. So it need to change the proper maximum size.
>> >>
>> >> This patch is proviced the solution to change value with configuration.
>> >>
>> >>
>> >> [...]
>> >
>> > Thanks, Applied to https://protect2.fireeye.com/v1/url?k=195a8ba3-78d19e99-195b00ec-74fe4860008a-
>> e609da47a4bcadff&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> boot%2Fcustodians%2Fu-boot-dfu (u-boot-dfu-next)
>> >
>> > [1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>> >       https://protect2.fireeye.com/v1/url?k=b04b2ae8-d1c03fd2-b04aa1a7-74fe4860008a-
>> d8ec8502221dcca0&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fcommit%2Fde9b2e10f10996050a10998a0836abe2f9e425e3
>> 
>> 
>> This patch breaks CI for both arm32 and arm64 platforms:
>> 
>> * https://protect2.fireeye.com/v1/url?k=3179f7cc-50f2e2f6-31787c83-74fe4860008a-
>> a4f5d2c73833c4d5&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725163
>> * https://protect2.fireeye.com/v1/url?k=22773f8e-43fc2ab4-2276b4c1-74fe4860008a-
>> f6dc3d75fe9b283a&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725164
>
> I couldn't access the above CI URL.

Sorry about that. CI was not public. That is fixed now.

>
>> 
>> It breaks because multiple boards which include dfu.h without having
>> CONFIG_DFU being set.
>> 
>> Tom attempted to fix this with:
>> https://protect2.fireeye.com/v1/url?k=452a9007-24a1853d-452b1b48-74fe4860008a-
>> a88f870953497071&q=1&e=b6fd9ce6-85b6-46a3-81aa-
>> 8aa89d16a9ed&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D369275
>> 
>> But that had some different issues, see:
>> https://protect2.fireeye.com/v1/url?k=f1c007bd-904b1287-f1c18cf2-74fe4860008a-
>> a305de1bab5bdff1&q=1&e=b6fd9ce6-85b6-46a3-81aa-
>> 8aa89d16a9ed&u=https%3A%2F%2Flibera.irclog.whitequark.org%2Fu-boot%2F2023-10-31%2335154532;
>> 
>> I've fixed it up with the following diff:
>> 
>> diff --git a/include/dfu.h b/include/dfu.h
>> index 4573f753c565..2c3ffa3f9297 100644
>> --- a/include/dfu.h
>> +++ b/include/dfu.h
>> @@ -99,7 +99,12 @@ struct virt_internal_data {
>>         int dev_num;
>>  };
>> 
>> +
>> +#if defined(CONFIG_DFU_NAME_MAX_SIZE)
>>  #define DFU_NAME_SIZE                  CONFIG_DFU_NAME_MAX_SIZE
>> +#else
>> +#define DFU_NAME_SIZE                  32
>> +#endif
>>  #ifndef DFU_DEFAULT_POLL_TIMEOUT
>>  #define DFU_DEFAULT_POLL_TIMEOUT 0
>>  #endif
>> 
>> If you have a better idea to fix this, can you please let me know?
>
> After checking this, let you inform.
>
>> Otherwise, I will squash this to keep CI green.
>> 
>> Thank you
>> 
>> >
>> > --
>> > Mattijs
Mattijs Korpershoek Nov. 7, 2023, 9:08 a.m. UTC | #8
Hi Jaehoon,

On ven., nov. 03, 2023 at 14:49, "Jaehoon Chung" <jh80.chung@samsung.com> wrote:

> Hi,
>
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
>> Sent: Friday, November 3, 2023 10:07 AM
>> To: 'Mattijs Korpershoek' <mkorpershoek@baylibre.com>
>> Cc: lukma@denx.de; u-boot@lists.denx.de
>> Subject: RE: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>> 
>> Hi Mathtjjs,
>> 
>> > -----Original Message-----
>> > From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> > Sent: Thursday, November 2, 2023 6:15 PM
>> > To: Jaehoon Chung <jh80.chung@samsung.com>
>> > Cc: lukma@denx.de; u-boot@lists.denx.de
>> > Subject: Re: [PATCH] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>> >
>> > Hi Jaehoon,
>> >
>> > On mar., oct. 31, 2023 at 15:50, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:
>> >
>> > > Hi,
>> > >
>> > > On Mon, 20 Jun 2022 20:13:54 +0900, Jaehoon Chung wrote:
>> > >> Add CONFIG_DFU_NAME_MAX_SIZE to change the proper size.
>> > >> If name is longer than default size, it can do wrong behavior during updating
>> > >> image. So it need to change the proper maximum size.
>> > >>
>> > >> This patch is proviced the solution to change value with configuration.
>> > >>
>> > >>
>> > >> [...]
>> > >
>> > > Thanks, Applied to https://protect2.fireeye.com/v1/url?k=195a8ba3-78d19e99-195b00ec-74fe4860008a-
>> > e609da47a4bcadff&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> > boot%2Fcustodians%2Fu-boot-dfu (u-boot-dfu-next)
>> > >
>> > > [1/1] dfu: add CONFIG_DFU_NAME_MAX_SIZE configuration
>> > >       https://protect2.fireeye.com/v1/url?k=b04b2ae8-d1c03fd2-b04aa1a7-74fe4860008a-
>> > d8ec8502221dcca0&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fcommit%2Fde9b2e10f10996050a10998a0836abe2f9e425e3
>> >
>> >
>> > This patch breaks CI for both arm32 and arm64 platforms:
>> >
>> > * https://protect2.fireeye.com/v1/url?k=3179f7cc-50f2e2f6-31787c83-74fe4860008a-
>> > a4f5d2c73833c4d5&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725163
>> > * https://protect2.fireeye.com/v1/url?k=22773f8e-43fc2ab4-2276b4c1-74fe4860008a-
>> > f6dc3d75fe9b283a&q=1&e=b6fd9ce6-85b6-46a3-81aa-8aa89d16a9ed&u=https%3A%2F%2Fsource.denx.de%2Fu-
>> > boot%2Fcustodians%2Fu-boot-dfu%2F-%2Fjobs%2F725164
>> 
>> I couldn't access the above CI URL.
>> 
>> >
>> > It breaks because multiple boards which include dfu.h without having
>> > CONFIG_DFU being set.
>> >
>> > Tom attempted to fix this with:
>> > https://protect2.fireeye.com/v1/url?k=452a9007-24a1853d-452b1b48-74fe4860008a-
>> > a88f870953497071&q=1&e=b6fd9ce6-85b6-46a3-81aa-
>> > 8aa89d16a9ed&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Flist%2F%3Fseries%3D369275
>> >
>> > But that had some different issues, see:
>> > https://protect2.fireeye.com/v1/url?k=f1c007bd-904b1287-f1c18cf2-74fe4860008a-
>> > a305de1bab5bdff1&q=1&e=b6fd9ce6-85b6-46a3-81aa-
>> > 8aa89d16a9ed&u=https%3A%2F%2Flibera.irclog.whitequark.org%2Fu-boot%2F2023-10-31%2335154532;
>> >
>> > I've fixed it up with the following diff:
>> >
>> > diff --git a/include/dfu.h b/include/dfu.h
>> > index 4573f753c565..2c3ffa3f9297 100644
>> > --- a/include/dfu.h
>> > +++ b/include/dfu.h
>> > @@ -99,7 +99,12 @@ struct virt_internal_data {
>> >         int dev_num;
>> >  };
>> >
>> > +
>> > +#if defined(CONFIG_DFU_NAME_MAX_SIZE)
>> >  #define DFU_NAME_SIZE                  CONFIG_DFU_NAME_MAX_SIZE
>> > +#else
>> > +#define DFU_NAME_SIZE                  32
>> > +#endif
>> >  #ifndef DFU_DEFAULT_POLL_TIMEOUT
>> >  #define DFU_DEFAULT_POLL_TIMEOUT 0
>> >  #endif
>> >
>> > If you have a better idea to fix this, can you please let me know?
>> 
>> After checking this, let you inform.
>
> It seems that it is depended on USB_GADGET and SPL_USB_GADGET. How about this?
> About <dfu.h> is included in some board files, It needs to check more.
>
> diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> index 6502aba96f54..9922da56e704 100644
> --- a/drivers/dfu/Kconfig
> +++ b/drivers/dfu/Kconfig
> @@ -13,6 +13,14 @@ config DFU_OVER_TFTP
>         bool
>         depends on NET
>
> +config DFU_NAME_MAX_SIZE
> +       int "Size of the name to be added in dfu entity"
> +       default 32
> +       depends on DFU || USB_GADGET_DOWNLOAD || SPL_USB_GADGET
> +       help
> +         This value is used to maximum size. If name is longer than default size,
> +         we need to change the proper maximum size.
> +
>  if DFU
>  config DFU_WRITE_ALT
>         bool
> @@ -112,13 +120,5 @@ config SYS_DFU_MAX_FILE_SIZE
>           this to the maximum filesize (in bytes) for the buffer.
>           If undefined it defaults to the CONFIG_SYS_DFU_DATA_BUF_SIZE.
>
> -config DFU_NAME_MAX_SIZE
> -       int "Size of the name to be added in dfu entity"
> -       default 32
> -       depends on DFU
> -       help
> -         This value is used to maximum size. If name is longer than default size,
> -         we need to change the proper maximum size.
>
> I'm running CI with some patch. After checked, I will inform.

Did your CI run finish ? Is your solution working or should we pick up
the diff I send ?

>
> Best Regards,
> Jaehoon Chung
>
>> 
>> > Otherwise, I will squash this to keep CI green.
>> >
>> > Thank you
>> >
>> > >
>> > > --
>> > > Mattijs
diff mbox series

Patch

diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
index 8d7f13dcb0b5..a181f0b8d7ba 100644
--- a/drivers/dfu/Kconfig
+++ b/drivers/dfu/Kconfig
@@ -111,5 +111,14 @@  config SYS_DFU_MAX_FILE_SIZE
 	  the buffer once we've been given the whole file.  Define
 	  this to the maximum filesize (in bytes) for the buffer.
 	  If undefined it defaults to the CONFIG_SYS_DFU_DATA_BUF_SIZE.
+
+config DFU_NAME_MAX_SIZE
+	int "Size of the name to be added in dfu entity"
+	default 32
+	depends on DFU
+	help
+	  This value is used to maximum size. If name is longer than default size,
+	  we need to change the proper maximum size.
+
 endif
 endmenu
diff --git a/include/dfu.h b/include/dfu.h
index dcb9cd9d799a..948596f367d9 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -99,7 +99,7 @@  struct virt_internal_data {
 	int dev_num;
 };
 
-#define DFU_NAME_SIZE			32
+#define DFU_NAME_SIZE			CONFIG_DFU_NAME_MAX_SIZE
 #ifndef DFU_DEFAULT_POLL_TIMEOUT
 #define DFU_DEFAULT_POLL_TIMEOUT 0
 #endif