[U-Boot] [PATCH 3/3] rockchip: evb-rv1108: add usb init function for dwc2 gadget

[wlf]

Dear Simon,

在 2017年09月09日 12:54, Simon Glass 写道:
> Hi,
>
> On 14 August 2017 at 04:05, wlf <wulf at rock-chips.com> wrote:
>> Dear Simon,
>>
>>
>> 在 2017年08月14日 05:35, Simon Glass 写道:
>>> On 8 August 2017 at 21:36, William Wu <william.wu at rock-chips.com> wrote:
>>>> This patch implements board_usb_init() for dwc2 gadget, it
>>>> generally called from do_fastboot to do dwc2 udc probe and
>>>> support fastboot over USB.
>>>>
>>>> Signed-off-by: William Wu <william.wu at rock-chips.com>
>>>> ---
>>>>    board/rockchip/evb_rv1108/evb_rv1108.c | 47
>>>> ++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 47 insertions(+)
>>>>
>>>> diff --git a/board/rockchip/evb_rv1108/evb_rv1108.c
>>>> b/board/rockchip/evb_rv1108/evb_rv1108.c
>>>> index fe37eac..8ca5ee6 100644
>>>> --- a/board/rockchip/evb_rv1108/evb_rv1108.c
>>>> +++ b/board/rockchip/evb_rv1108/evb_rv1108.c
>>>> @@ -50,3 +50,50 @@ int dram_init_banksize(void)
>>>>
>>>>           return 0;
>>>>    }
>>>> +
>>>> +#if defined(CONFIG_USB_GADGET) && defined(CONFIG_USB_GADGET_DWC2_OTG)
>>>> +#include <usb.h>
>>>> +#include <usb/dwc2_udc.h>
>>>> +
>>>> +static struct dwc2_plat_otg_data rv1108_otg_data = {
>>>> +       .rx_fifo_sz     = 512,
>>>> +       .np_tx_fifo_sz  = 16,
>>>> +       .tx_fifo_sz     = 128,
>>>> +};
>>>> +
>>>> +int board_usb_init(int index, enum usb_init_type init)
>>>> +{
>>>> +       int node;
>>>> +       const char *mode;
>>>> +       bool matched = false;
>>>> +       const void *blob = gd->fdt_blob;
>>>> +
>>>> +       /* find the usb_otg node */
>>>> +       node = fdt_node_offset_by_compatible(blob, -1,
>>>> +                                       "rockchip,rv1108-usb");
>>>> +
>>>> +       while (node > 0) {
>>>> +               mode = fdt_getprop(blob, node, "dr_mode", NULL);
>>>> +               if (mode && strcmp(mode, "otg") == 0) {
>>>> +                       matched = true;
>>>> +                       break;
>>>> +               }
>>>> +
>>>> +               node = fdt_node_offset_by_compatible(blob, node,
>>>> +                                       "rockchip,rv1108-usb");
>>>> +       }
>>>> +       if (!matched) {
>>>> +               debug("Not found usb_otg device\n");
>>>> +               return -ENODEV;
>>>> +       }
>>>> +
>>>> +       rv1108_otg_data.regs_otg = fdtdec_get_addr(blob, node, "reg");
>>>> +
>>>> +       return dwc2_udc_probe(&rv1108_otg_data);
>>> These USB init things have been bothering me for a while. Do you think
>>> this could be changed into a driver that you could probe with
>>> device_probe()? Then much of the code in here would not be needed and
>>> it might be easier to tidy it up when we have proper driver-model
>>> support for USB device mode.
>> Yes,  on rockchip platforms, it did most of the same USB init things in
>> different board special drivers.
>> I think may be we can try to optimize the code in two different ways.
>>
>> Method1.
>> 1. Move the USB init things from board_usb_init(), just simply call
>> dwc2_udc_probe().
>> 2. Create a new USB init  function in drivers/usb/gadget/dwc2_udc_otg.c，
>>       and call it in dwc2_udc_probe().
>> 3. Use fdt (Flat Device Tree manipulation )  to parse the dts, and get the
>> regs_phy, regs_otg and so on.
>>
>> Method2.
>> Just like your  suggestion， use driver-model method to probe with
>> device_probe,
>> actually, I don't know much about the driver-model, as far as I know, we may
>> need
>> to do the following work:
>> 1. Create a new uclass id for usb udc, and declare a new uclass_driver with
>> UCLASS_DRIVER,
>>      and also need to create a  new U-boot driver for dwc2 controller with
>> U_BOOT_DRIVER.
>> 2. How to call device_probe()? Maybe it's better to create a new driver
>> (like drivers/usb/host/usb-uclass.c)
>>      in drivers/usb/gadget to match different usb gadget controllers, and
>> wrap the device_probe()
>>      to a new function (like udc_pre_probe()), and then different udc driver
>> can call it,
>>      e.g.
>>      dwc2_udc_probe() --> udc_pre_probe() --> device_probe()
>>
>> In summary, method1 is easily to be done, but method2 is more generic.
>>
>> I hope I haven't misunderstood your proposal, and hope to get your reply.
>> Thank you!
> Yes I think method 2 is better.
>
> You will have to call device_probe() as you say. I think what you
> suggest is reasonable, since something has to decide that the USB
> device mode should be started.
Thanks for your reply.  I will try to update a new patch according to 
method 2.

>
> Regards,
> Simon
>
>
>

-- 
吴良峰 William.Wu
福建省福州市铜盘路软件大道89号软件园A区21号楼
No.21 Building, A District, No.89,software Boulevard Fuzhou,Fujian, PRC
手机: 13685012275
座机: 0591-83991906-8520
邮件:wulf at rock-chips.com