[U-Boot] [PATCH v3 1/6] drivers: usb: gadget: ether: adopt to usb driver model

[Joe Hershberger]

On Sun, Nov 27, 2016 at 11:02 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mugunthan,
>
> On 24 November 2016 at 01:11, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>> Hi Simon
>>
>> On Thursday 24 November 2016 07:51 AM, Simon Glass wrote:
>>> Hi Mugunthan,
>>>
>>> On 20 November 2016 at 22:38, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>>>> Hi Simon,
>>>>
>>>> On Saturday 19 November 2016 01:04 AM, Simon Glass wrote:
>>>>> Hi Mugunthan,
>>>>>
>>>>> On 17 November 2016 at 01:09, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>>>>>> Convert usb ether gadget to adopt usb driver model
>>>>>>
>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
>>>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>>
>>>>> Sorry, but I'd like to 'un-review' this.
>>>>>
>>>>>> ---
>>>>>>  drivers/usb/gadget/ether.c | 36 ++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 36 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
>>>>>> index 497b981129..9bc61186cf 100644
>>>>>> --- a/drivers/usb/gadget/ether.c
>>>>>> +++ b/drivers/usb/gadget/ether.c
>>>>>> @@ -24,6 +24,10 @@
>>>>>>  #include "gadget_chips.h"
>>>>>>  #include "rndis.h"
>>>>>>
>>>>>> +#include <dm.h>
>>>>>> +#include <dm/uclass-internal.h>
>>>>>> +#include <dm/device-internal.h>
>>>>>> +
>>>>>>  #define USB_NET_NAME "usb_ether"
>>>>>>
>>>>>>  #define atomic_read
>>>>>> @@ -101,6 +105,9 @@ struct eth_dev {
>>>>>>         struct usb_gadget       *gadget;
>>>>>>         struct usb_request      *req;           /* for control responses */
>>>>>>         struct usb_request      *stat_req;      /* for cdc & rndis status */
>>>>>> +#ifdef CONFIG_DM_USB
>>>>>> +       struct udevice          *usb_udev;
>>>>>> +#endif
>>>>>>
>>>>>>         u8                      config;
>>>>>>         struct usb_ep           *in_ep, *out_ep, *status_ep;
>>>>>> @@ -2303,6 +2310,24 @@ fail:
>>>>>>
>>>>>>  /*-------------------------------------------------------------------------*/
>>>>>>
>>>>>> +#ifdef CONFIG_DM_USB
>>>>>> +int dm_usb_init(struct eth_dev *e_dev)
>>>>>> +{
>>>>>> +       struct udevice *dev = NULL;
>>>>>> +       int ret;
>>>>>> +
>>>>>> +       ret = uclass_first_device(UCLASS_USB_DEV_GENERIC, &dev);
>>>>>> +       if (!dev || ret) {
>>>>>> +               error("No USB device found\n");
>>>>>> +               return -ENODEV;
>>>>>> +       }
>>>>>> +
>>>>>> +       e_dev->usb_udev = dev;
>>>>>> +
>>>>>> +       return ret;
>>>>>> +}
>>>>>> +#endif
>>>>>> +
>>>>>>  static int usb_eth_init(struct eth_device *netdev, bd_t *bd)
>>>>>>  {
>>>>>>         struct eth_dev *dev = &l_ethdev;
>>>>>> @@ -2315,7 +2340,14 @@ static int usb_eth_init(struct eth_device *netdev, bd_t *bd)
>>>>>>                 goto fail;
>>>>>>         }
>>>>>>
>>>>>> +#ifdef CONFIG_DM_USB
>>>>>> +       if (dm_usb_init(dev)) {
>>>>>> +               error("USB ether not found\n");
>>>>>> +               return -ENODEV;
>>>>>> +       }
>>>>>> +#else
>>>>>>         board_usb_init(0, USB_INIT_DEVICE);
>>>>>> +#endif
>>>>>>
>>>>>>         /* Configure default mac-addresses for the USB ethernet device */
>>>>>>  #ifdef CONFIG_USBNET_DEV_ADDR
>>>>>> @@ -2497,7 +2529,11 @@ void usb_eth_halt(struct eth_device *netdev)
>>>>>>         }
>>>>>>
>>>>>>         usb_gadget_unregister_driver(&eth_driver);
>>>>>> +#ifdef CONFIG_DM_USB
>>>>>> +       device_remove(dev->usb_udev);
>>>>>> +#else
>>>>>>         board_usb_cleanup(0, USB_INIT_DEVICE);
>>>>>> +#endif
>>>>>
>>>>> This doesn't look right to me. If your board is to be an Ethernet
>>>>> device then it should do:
>>>>>
>>>>> uclass_first_device(UCLASS_ETH, &dev)
>>>>>
>>>>> to get the device. That could be in the device tree under the USB
>>>>> node, or perhaps you want to have a setup function which manualy binds
>>>>> the device, a bit like usb_find_and_bind_driver().
>>>>>
>>>>
>>>> This patch is to get usb device for the ether gadget. It uses the same
>>>> api with UCLASS_USB_DEV_GENERIC to get usb device. The patch hadn't done
>>>> for eth driver model adoption.
>>>
>>> So can you do that one first, or is it coming soon?
>>>
>>
>> Its already implemented above in the function dm_usb_init()
>
> So are you saying that the Ethernet 'USB device' driver needs to be
> converted to driver model? Otherwise what stops us from using
> UCLASS_ETH here?

I think the confusion might be that this is a USB gadget and it
doesn't support ETH DM yet. This code needs lots of clean-up. It is
still using the old Ethernet stack. This just makes the driver able to
function if you have USB DM enabled.

At least that how I understand this patch.

-Joe