On Thu, Aug 4, 2016 at 8:51 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
On Tue, Aug 2, 2016 at 6:31 AM, Max Filippov jcmvbkbc@gmail.com wrote:
Extract reusable parts from ethoc_init, ethoc_set_mac_address, ethoc_send and ethoc_receive, move the rest under #ifdef CONFIG_DM_ETH. Add U_BOOT_DRIVER, eth_ops structure and implement required methods.
Signed-off-by: Max Filippov jcmvbkbc@gmail.com
A few small nits below, but otherwise,
Acked-by: Joe Hershberger joe.hershberger@ni.com
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c index f5bd1ab..0225595 100644 --- a/drivers/net/ethoc.c +++ b/drivers/net/ethoc.c
[...]
@@ -216,11 +217,8 @@ static inline void ethoc_write_bd(struct ethoc *priv, int index, ethoc_write(priv, offset + 4, bd->addr); }
-static int ethoc_set_mac_address(struct eth_device *dev) +static int ethoc_set_mac_address(struct ethoc *priv, u8 *mac)
Please use ethoc_write_hwaddr_common() instead.
Ok.
[...]
+static int ethoc_recv_common(struct ethoc *priv)
Please use a better name for this, something like ethoc_is_rx_pkt_rdy().
It's a bit more complex than that, it would only return 1 if new packets arrived since its last invocation. When it returns 0 there still may be received packets in the RX queue. I'll call it ethoc_is_new_packet_received.
[...]
+#ifndef CONFIG_DM_ETH
Please use positive logic and reverse the order of these code snippets.
#ifdef CONFIG_DM_ETH ... #else ... #endif
Ok.
[...]
+static int ethoc_recv(struct udevice *dev, int flags, uchar **packetp) +{
struct ethoc *priv = dev_get_priv(dev);if (flags & ETH_RECV_CHECK_DEVICE)ethoc_recv_common(priv);Kinda seems like you should skip the next call (especially when this is renamed to be clearer). The code flow seems OK, though. Up to you.
Ok.