| Message ID | 20161117080909.14856-2-mugunthanvnm@ti.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Mugunthan, On 17 November 2016 at 01:09, Mugunthan V N <mugunthanvnm@ti.com> wrote: > Convert usb ether gadget to adopt usb driver model > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> > Reviewed-by: Simon Glass <sjg@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(ð_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(). > } > > static struct usb_gadget_driver eth_driver = { > -- > 2.11.0.rc1 > Regards, Simon
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@ti.com> wrote: >> Convert usb ether gadget to adopt usb driver model >> >> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >> Reviewed-by: Simon Glass <sjg@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(ð_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. Regards Mugunthan V N
Hi Mugunthan, On 20 November 2016 at 22:38, Mugunthan V N <mugunthanvnm@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@ti.com> wrote: >>> Convert usb ether gadget to adopt usb driver model >>> >>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>> Reviewed-by: Simon Glass <sjg@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(ð_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? Regards, Simon
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@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@ti.com> wrote: >>>> Convert usb ether gadget to adopt usb driver model >>>> >>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>> Reviewed-by: Simon Glass <sjg@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(ð_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() Regards Mugunthan V N
Hi Mugunthan, On 24 November 2016 at 01:11, Mugunthan V N <mugunthanvnm@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@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@ti.com> wrote: >>>>> Convert usb ether gadget to adopt usb driver model >>>>> >>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>>> Reviewed-by: Simon Glass <sjg@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(ð_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? Regards, Simon
On Sun, Nov 27, 2016 at 11:02 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Mugunthan, > > On 24 November 2016 at 01:11, Mugunthan V N <mugunthanvnm@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@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@ti.com> wrote: >>>>>> Convert usb ether gadget to adopt usb driver model >>>>>> >>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>>>> Reviewed-by: Simon Glass <sjg@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(ð_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
On Wednesday 30 November 2016 04:43 AM, Joe Hershberger wrote: > On Sun, Nov 27, 2016 at 11:02 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Mugunthan, >> >> On 24 November 2016 at 01:11, Mugunthan V N <mugunthanvnm@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@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@ti.com> wrote: >>>>>>> Convert usb ether gadget to adopt usb driver model >>>>>>> >>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>>>>> Reviewed-by: Simon Glass <sjg@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(ð_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. > Yes, this patch adopts the driver to use DM_USB and still use only non DM ETH. Patch 6/6 actually converts the driver to adopt DM_ETH. Regards Mugunthan V N
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(ð_driver); +#ifdef CONFIG_DM_USB + device_remove(dev->usb_udev); +#else board_usb_cleanup(0, USB_INIT_DEVICE); +#endif } static struct usb_gadget_driver eth_driver = {