Hi Stefan,
On 28 June 2016 at 08:28, Stefan Roese <s...@denx.de> wrote:
> Hi Simon,
>
> On 26.06.2016 04:53, Simon Glass wrote:
>> On 22 June 2016 at 01:18, Stefan Roese <s...@denx.de> wrote:
>>> Add support for driver model, so that CONFIG_DM_ETH can be defined and
>>> used with this driver.
>>>
>>> This patch also adds the read_rom_hwaddr() callback so that the ROM MAC
>>> address will be used to the DM part of this driver.
>>>
>>> Signed-off-by: Stefan Roese <s...@denx.de>
>>> Cc: Stephen Warren <swar...@nvidia.com>
>>> Cc: Ted Chen <tedc...@realtek.com>
>>> Cc: Simon Glass <s...@chromium.org>
>>> Cc: Joe Hershberger <joe.hershber...@ni.com>
>>> ---
>>> drivers/usb/eth/r8152.c | 235
>>> ++++++++++++++++++++++++++++++++++++++++-----
>>> drivers/usb/eth/r8152.h | 4 +
>>> drivers/usb/eth/r8152_fw.c | 2 +
>>> 3 files changed, 219 insertions(+), 22 deletions(-)
>>
>> Reviewed-by: Simon Glass <s...@chromium.org>
>>
>> With a few comments below.
[snip]
>>> +int r8152_eth_recv(struct udevice *dev, int flags, uchar **packetp)
>>> +{
>>> + struct r8152 *tp = dev_get_priv(dev);
>>> + struct ueth_data *ueth = &tp->ueth;
>>> + uint8_t *ptr;
>>> + int ret, len;
>>> + struct rx_desc *rx_desc;
>>> + u16 packet_len;
>>> +
>>> + len = usb_ether_get_rx_bytes(ueth, &ptr);
>>> + debug("%s: first try, len=%d\n", __func__, len);
>>> + if (!len) {
>>> + if (!(flags & ETH_RECV_CHECK_DEVICE))
>>> + return -EAGAIN;
>>> + ret = usb_ether_receive(ueth, RTL8152_AGG_BUF_SZ);
>>> + if (ret == -EAGAIN)
>>> + return ret;
>>
>> What about if ret returns some other error?
>
> Good catch. Will fix in v2.
>
> BTW: I've cloned this part from other drivers (e.g. smsc95xx.c
> and asix.c). So they would need some additional error handling here
> as well.
OK
>
>>> +
>>> + len = usb_ether_get_rx_bytes(ueth, &ptr);
>>> + debug("%s: second try, len=%d\n", __func__, len);
>>> + }
>>> +
>>> + rx_desc = (struct rx_desc *)ptr;
>>> + packet_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
>>> + packet_len -= CRC_SIZE;
>>> +
>>> + if (packet_len > len - (sizeof(struct rx_desc) + CRC_SIZE)) {
>>> + debug("Rx: too large packet: %d\n", packet_len);
>>> + goto err;
>>> + }
>>> +
>>> + *packetp = ptr + sizeof(struct rx_desc);
>>> + return packet_len;
>>> +
>>> +err:
>>> + usb_ether_advance_rxbuf(ueth, -1);
>>> + return -EINVAL;
>>
>> -E2BIG?
>
> Hmmm. This does not seem appropriate here:
>
> #define E2BIG 7 /* Argument list too long */
>
> Or is it commonly used also for non argument list return failure?
Not sure...:-)
I was probably thinking of -ENOSPC.
[snip]
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
