Re: [PATCH v5 4/6] configs: am62x_evm_*: Enable USB and DFU support

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Tue, May 07, 2024 at 10:14:28AM +0200, Francesco Dolcini wrote:
> Hello Martyn,
> first thanks for your series, with this we might be able to drop some
> downstream branch.
> 
> On Mon, May 06, 2024 at 03:38:44PM +0100, Martyn Welch wrote:
> > From: Sjoerd Simons <sjoerd@collabora.com>
> > 
> > Provide config fragments to enable USB host as well as USB gadget and DFU
> > support for a53 and r5. This relevant fragment is included into the
> > am62x EVM a53 defconfig. For the r5, due to the smaller available size,
> > the config fragment also disables support for persistent storage to free
> > up space for USB support. This fragment needs to be included is DFU
> > booting is desired.
> > 
> > The CONFIG_DFU_SF option is placed in the defconfig rather than the
> > fragment as this is known not to be supported on all boards that can
> > support DFU.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> > Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
> 
> ...
> 
> > diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
> > new file mode 100644
> > index 0000000000..772bb2ab93
> > --- /dev/null
> > +++ b/configs/am62x_r5_usbdfu.config
> > @@ -0,0 +1,28 @@
> 
> ...
> 
> > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> 
> This is making this fragment TI _board_ specific, while the file name seems
> to imply that this is generic for the TI SoC. Other vendors, using TI
> SoCs, will likely want to use their own USB IDs.
> 
> Not a big deal and no need to change it, we'll handle this in our own
> defconfig when we'll enable this, but I wanted to mention this.

One thing about fragments, at least so long as it's being processed
correctly, is that "make fooboard_config fragA.config fragB.config"
means that fragB.config will override fragA.config values. Not that it
shouldn't also possibly be re-done as either of:
1) am62x_r5_usbdfu.config + am62x_evm_r5_usbdfu.config
2) better use of imply keyword perhaps in the SoC stanza in one of the
K3 Kconfig files.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]