[U-Boot] [U-Boot 3/3] rockchip: rk3288: enable rockusb support on rk3288 based device

[Eddie Cai]

2017-05-05 0:49 GMT+08:00 Simon Glass <sjg at chromium.org>:
> Hi,
>
> On 4 May 2017 at 03:19, Lukasz Majewski <lukma at denx.de> wrote:
>> Hi Eddie, Simon,
>>
>>> Hi Simon
>>>
>>> 2017-05-03 18:09 GMT+08:00 Simon Glass <sjg at chromium.org>:
>>> > Hi Eddie,
>>> >
>>> > On 2 May 2017 at 04:37, Eddie Cai <eddie.cai.linux at gmail.com> wrote:
>>> >> Hi Simon
>>> >> 2017-03-20 10:30 GMT+08:00 Simon Glass <sjg at chromium.org>:
>>> >>> Hi Eddie.
>>> >>>
>>> >>> On 15 March 2017 at 01:56, Eddie Cai <eddie.cai.linux at gmail.com>
>>> >>> wrote:
>>> >>>> this patch enable rockusb support on rk3288 based device.
>>> >>>>
>>> >>>> Signed-off-by: Eddie Cai <eddie.cai.linux at 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?
>
>>
>> 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