Hi, On 4 May 2017 at 03:19, Lukasz Majewski <lu...@denx.de> wrote: > Hi Eddie, Simon, > >> Hi Simon >> >> 2017-05-03 18:09 GMT+08:00 Simon Glass <s...@chromium.org>: >> > Hi Eddie, >> > >> > On 2 May 2017 at 04:37, Eddie Cai <eddie.cai.li...@gmail.com> wrote: >> >> Hi Simon >> >> 2017-03-20 10:30 GMT+08:00 Simon Glass <s...@chromium.org>: >> >>> Hi Eddie. >> >>> >> >>> On 15 March 2017 at 01:56, Eddie Cai <eddie.cai.li...@gmail.com> >> >>> wrote: >> >>>> this patch enable rockusb support on rk3288 based device. >> >>>> >> >>>> Signed-off-by: Eddie Cai <eddie.cai.li...@gmail.com> >> >>>> --- >> >>>> include/configs/rk3288_common.h | 4 ++++ >> >>>> 1 file changed, 4 insertions(+) >> >>> >> >>> I think this should be done in Kconfig. >> >> since rockusb used so widely on rockchip soc based devices. every >> >> rockchip soc based >> >> device should support it. So I would like to put it in >> >> rk3288_common.h or even rockchip-common.h. >> >> what do you think? >> > >> > We are moving to removing the board config headers so cannot add new >> > non-Kconfig CONFIG options. >> > >> > You can add it to arch/arm/Kconfig - e.g. with 'imply CONFIG_....' >> > under 'config ARCH_ROCKCHIP'. >> USB_FUNCTION_ROCKUSB depend on USB_GADGET, USB_GADGET_DOWNLOAD, >> CONFIG_G_DNL_VENDOR_NUM, CONFIG_G_DNL_PRODUCT_NUM >> should i move those config to arch/arm/Kconfig? how to define >> CONFIG_G_DNL_VENDOR_NUM >> and CONFIG_G_DNL_PRODUCT_NUM when select it? > > I would prefer to keep those CONFIG options as they are now (and are in > sync with other gadgets).
These two are new though right? CONFIG_CMD_ROCKUSB CONFIG_USB_FUNCTION_ROCKUSB So they should be added to Kconfig. If they 'depend' on non-Kconfig options, that's fine IMO. We can't express that until the other options are converted. Let's convert them soon! > > The problem here is that the gadget infrastructure is a bit > "problematic" from the device model point of view. > > It has his own "structure" borrowed from Linux kernel. > > I've poked around and it seems like adding it to DM requires calling > "g_dnl_register()" with embedded name of the gadget (like > "usb_dnl_dfu"). > > And such call is required for each different gadget (like mass storage, > dfu, fastboot). > > Or maybe we should add "stub" DM support to only indicate supported > gadgets (like dfu, ums, etc) and leave as much code untouched as > possible? I'm sure this can be resolved in some way. I admit I have not looked at it but was thinking of attacking usb_tty sometime, so perhaps might see if I can figure it out. > >> > >> > Please help to remove any options you can from the headers. >> > >> >>> >> >>>> >> >>>> diff --git a/include/configs/rk3288_common.h >> >>>> b/include/configs/rk3288_common.h index b5606d4..b19a34d 100644 >> >>>> --- a/include/configs/rk3288_common.h >> >>>> +++ b/include/configs/rk3288_common.h >> >>>> @@ -74,6 +74,10 @@ >> >>>> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR >> >>>> #define CONFIG_FASTBOOT_BUF_SIZE 0x08000000 >> >>>> >> >>>> +/* rockusb */ >> >>>> +#define CONFIG_CMD_ROCKUSB >> >>>> +#define CONFIG_USB_FUNCTION_ROCKUSB >> >>>> + >> >>>> /* usb mass storage */ >> >>>> #define CONFIG_USB_FUNCTION_MASS_STORAGE >> >>>> #define CONFIG_CMD_USB_MASS_STORAGE >> >>>> -- >> >>>> 2.7.4 >> > >> > Regards, >> > Simon Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
