Hi Eddie, On 4 May 2017 at 19:15, Eddie Cai <eddie.cai.li...@gmail.com> wrote: > 2017-05-05 0:49 GMT+08:00 Simon Glass <s...@chromium.org>: >> 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! > Does it means i can keep it in rk3288_common.h?
Anything new you add must go into defconfig. If it depends on a non-Kconfig option, then just leave out the depend. We cannot add new things to the config whitelist, so you should get a build error if you try to add a new option outside Kconfig. >> >>> >>> 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 Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot