On lun., sept. 04, 2023 at 11:10, Marek Vasut <ma...@denx.de> wrote:
> On 9/4/23 09:56, Mattijs Korpershoek wrote:
>> On ven., sept. 01, 2023 at 11:50, Marek Vasut <ma...@denx.de> wrote:
>>
>>> Convert to plain udevice interaction with UDC controller
>>> device, avoid the use of UDC uclass dev_array .
>>>
>>> Signed-off-by: Marek Vasut <ma...@denx.de>
>>> ---
>>> Cc: Angus Ainslie <an...@akkea.ca>
>>> Cc: Dmitrii Merkurev <dimori...@google.com>
>>> Cc: Eddie Cai <eddie.cai.li...@gmail.com>
>>> Cc: Kever Yang <kever.y...@rock-chips.com>
>>> Cc: Lukasz Majewski <lu...@denx.de>
>>> Cc: Miquel Raynal <miquel.ray...@bootlin.com>
>>> Cc: Mattijs Korpershoek <mkorpersh...@baylibre.com>
>>> Cc: Nishanth Menon <n...@ti.com>
>>> Cc: Patrice Chotard <patrice.chot...@foss.st.com>
>>> Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
>>> Cc: Philipp Tomsich <philipp.toms...@vrull.eu>
>>> Cc: Simon Glass <s...@chromium.org>
>>> Cc: Stefan Roese <s...@denx.de>
>>> Cc: ker...@puri.sm
>>> ---
>>> V2: Use udev->parent for UDC access
>>> ---
>>> drivers/usb/gadget/ether.c | 20 +++++++++++---------
>>> 1 file changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
>>> index 5ff06d3814b..11b1a6221b3 100644
>>> --- a/drivers/usb/gadget/ether.c
>>> +++ b/drivers/usb/gadget/ether.c
>>> @@ -1880,8 +1880,10 @@ static void eth_start(struct eth_dev *dev, gfp_t
>>> gfp_flags)
>>> }
>>> }
>>>
>>> -static int eth_stop(struct eth_dev *dev)
>>> +static int eth_stop(struct udevice *udev)
>>> {
>>> + struct ether_priv *priv = dev_get_priv(udev);
>>> + struct eth_dev *dev = &priv->ethdev;
>>> #ifdef RNDIS_COMPLETE_SIGNAL_DISCONNECT
>>> unsigned long ts;
>>> unsigned long timeout = CONFIG_SYS_HZ; /* 1 sec to stop RNDIS */
>>> @@ -1895,7 +1897,7 @@ static int eth_stop(struct eth_dev *dev)
>>> /* Wait until host receives OID_GEN_MEDIA_CONNECT_STATUS */
>>> ts = get_timer(0);
>>> while (get_timer(ts) < timeout)
>>> - usb_gadget_handle_interrupts(0);
>>> + dm_usb_gadget_handle_interrupts(udev->parent);
>>> #endif
>>>
>>> rndis_uninit(dev->rndis_config);
>>> @@ -2300,7 +2302,7 @@ static int usb_eth_start(struct udevice *udev)
>>> pr_err("The remote end did not respond in time.");
>>> goto fail;
>>> }
>>> - usb_gadget_handle_interrupts(0);
>>> + dm_usb_gadget_handle_interrupts(udev->parent);
>>> }
>>>
>>> packet_received = 0;
>>> @@ -2370,7 +2372,7 @@ static int usb_eth_send(struct udevice *udev, void
>>> *packet, int length)
>>> printf("timeout sending packets to usb ethernet\n");
>>> return -1;
>>> }
>>> - usb_gadget_handle_interrupts(0);
>>> + dm_usb_gadget_handle_interrupts(udev->parent);
>>> }
>>> free(rndis_pkt);
>>>
>>> @@ -2400,13 +2402,13 @@ static void usb_eth_stop(struct udevice *udev)
>>> * 2) 'pullup' callback in your UDC driver can be improved to perform
>>> * this deinitialization.
>>> */
>>> - eth_stop(dev);
>>> + eth_stop(udev);
>>>
>>> usb_gadget_disconnect(dev->gadget);
>>>
>>> /* Clear pending interrupt */
>>> if (dev->network_started) {
>>> - usb_gadget_handle_interrupts(0);
>>> + dm_usb_gadget_handle_interrupts(udev->parent);
>>> dev->network_started = 0;
>>> }
>>> }
>>> @@ -2416,7 +2418,7 @@ static int usb_eth_recv(struct udevice *dev, int
>>> flags, uchar **packetp)
>>> struct ether_priv *priv = dev_get_priv(dev);
>>> struct eth_dev *ethdev = &priv->ethdev;
>>>
>>> - usb_gadget_handle_interrupts(0);
>>> + dm_usb_gadget_handle_interrupts(dev->parent);
>>>
>>> if (packet_received) {
>>> if (ethdev->rx_req) {
>>> @@ -2467,7 +2469,7 @@ int usb_ether_init(void)
>>> return ret;
>>> }
>>>
>>> - return usb_gadget_initialize(0);
>>> + return 0;
>>
>> No udc_device_get() here?
>>
>> This looks a little suspicious, because in the unbind() we call
>> udc_device_put().
>
> Look at how udc_device_get_by_index() is implemented and what it does,
> compared to what usb_ether_init() does (hint: they are basically doing
> the same thing). This udc_device_put() here is only a workaround for
> platforms which are not using the bind command to bind USB ethernet yet.
Oh right. Thank you for explaining.
Reviewed-by: Mattijs Korpershoek <mkorpersh...@baylibre.com>
