Hi Simon,
On 26.06.2016 04:53, Simon Glass wrote:
On 22 June 2016 at 01:18, Stefan Roese sr@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 sr@denx.de Cc: Stephen Warren swarren@nvidia.com Cc: Ted Chen tedchen@realtek.com Cc: Simon Glass sjg@chromium.org Cc: Joe Hershberger joe.hershberger@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 sjg@chromium.org
With a few comments below.
diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c index 325b70c..a148267 100644 --- a/drivers/usb/eth/r8152.c +++ b/drivers/usb/eth/r8152.c @@ -3,9 +3,10 @@
- SPDX-License-Identifier: GPL-2.0
- */
*/
#include <common.h>
+#include <dm.h> #include <errno.h> #include <malloc.h> #include <usb.h> @@ -15,6 +16,7 @@ #include "usb_ether.h" #include "r8152.h"
+#ifndef CONFIG_DM_ETH /* local vars */ static int curr_eth_dev; /* index for name of next device detected */
@@ -23,12 +25,6 @@ struct r8152_dongle { unsigned short product; };
-struct r8152_version {
unsigned short tcr;unsigned short version;bool gmii;-};
- static const struct r8152_dongle const r8152_dongles[] = { /* Realtek */ { 0x0bda, 0x8050 },
@@ -54,6 +50,13 @@ static const struct r8152_dongle const r8152_dongles[] = { /* Nvidia */ { 0x0955, 0x09ff }, }; +#endif
+struct r8152_version {
unsigned short tcr;unsigned short version;bool gmii;+};
static const struct r8152_version const r8152_versions[] = { { 0x4c00, RTL_VER_01, 0 }, @@ -1176,11 +1179,8 @@ static int rtl_ops_init(struct r8152 *tp) return ret; }
-static int r8152_init(struct eth_device *eth, bd_t *bd) +static int r8152_init_common(struct r8152 *tp) {
struct ueth_data *dev = (struct ueth_data *)eth->priv;struct r8152 *tp = (struct r8152 *)dev->dev_priv;u8 speed; int timeout = 0; int link_detected;@@ -1210,14 +1210,11 @@ static int r8152_init(struct eth_device *eth, bd_t *bd) return 0; }
-static int r8152_send(struct eth_device *eth, void *packet, int length) +static int r8152_send_common(struct ueth_data *ueth, void *packet, int length) {
struct ueth_data *dev = (struct ueth_data *)eth->priv;
struct usb_device *udev = ueth->pusb_dev; u32 opts1, opts2 = 0;
int err;int actual_len; unsigned char msg[PKTSIZE + sizeof(struct tx_desc)]; struct tx_desc *tx_desc = (struct tx_desc *)msg;@@ -1231,18 +1228,31 @@ static int r8152_send(struct eth_device *eth, void *packet, int length)
memcpy(msg + sizeof(struct tx_desc), (void *)packet, length);
err = usb_bulk_msg(dev->pusb_dev,usb_sndbulkpipe(dev->pusb_dev, dev->ep_out),(void *)msg,length + sizeof(struct tx_desc),&actual_len,USB_BULK_SEND_TIMEOUT);
err = usb_bulk_msg(udev, usb_sndbulkpipe(udev, ueth->ep_out),(void *)msg, length + sizeof(struct tx_desc),&actual_len, USB_BULK_SEND_TIMEOUT); debug("Tx: len = %zu, actual = %u, err = %d\n", length + sizeof(struct tx_desc), actual_len, err); return err;}
+#ifndef CONFIG_DM_ETH +static int r8152_init(struct eth_device *eth, bd_t *bd) +{
struct ueth_data *dev = (struct ueth_data *)eth->priv;struct r8152 *tp = (struct r8152 *)dev->dev_priv;return r8152_init_common(tp);+}
+static int r8152_send(struct eth_device *eth, void *packet, int length) +{
struct ueth_data *dev = (struct ueth_data *)eth->priv;return r8152_send_common(dev, packet, length);+}
- static int r8152_recv(struct eth_device *eth) { struct ueth_data *dev = (struct ueth_data *)eth->priv;
@@ -1454,3 +1464,184 @@ int r8152_eth_get_info(struct usb_device *dev, struct ueth_data *ss, debug("MAC %pM\n", eth->enetaddr); return 1; } +#endif /* !CONFIG_DM_ETH */
+#ifdef CONFIG_DM_ETH +static int r8152_eth_start(struct udevice *dev) +{
struct r8152 *tp = dev_get_priv(dev);debug("** %s (%d)\n", __func__, __LINE__);return r8152_init_common(tp);+}
+void r8152_eth_stop(struct udevice *dev) +{
struct r8152 *tp = dev_get_priv(dev);debug("** %s (%d)\n", __func__, __LINE__);tp->rtl_ops.disable(tp);+}
+int r8152_eth_send(struct udevice *dev, void *packet, int length) +{
struct r8152 *tp = dev_get_priv(dev);return r8152_send_common(&tp->ueth, packet, length);+}
+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.
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?
+}
+static int r8152_free_pkt(struct udevice *dev, uchar *packet, int packet_len) +{
struct r8152 *tp = dev_get_priv(dev);packet_len += sizeof(struct rx_desc) + CRC_SIZE;packet_len = ALIGN(packet_len, 8);usb_ether_advance_rxbuf(&tp->ueth, packet_len);return 0;+}
+static int r8152_write_hwaddr(struct udevice *dev) +{
struct eth_pdata *pdata = dev_get_platdata(dev);struct r8152 *tp = dev_get_priv(dev);unsigned char enetaddr[8] = { 0 };debug("** %s (%d)\n", __func__, __LINE__);memcpy(enetaddr, pdata->enetaddr, ETH_ALEN);ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_CONFIG);pla_ocp_write(tp, PLA_IDR, BYTE_EN_SIX_BYTES, 8, enetaddr);ocp_write_byte(tp, MCU_TYPE_PLA, PLA_CRWECR, CRWECR_NORAML);debug("MAC %pM\n", pdata->enetaddr);return 0;+}
+int r8152_read_rom_hwaddr(struct udevice *dev) +{
struct eth_pdata *pdata = dev_get_platdata(dev);struct r8152 *tp = dev_get_priv(dev);debug("** %s (%d)\n", __func__, __LINE__);r8152_read_mac(tp, pdata->enetaddr);return 0;+}
+static int r8152_eth_probe(struct udevice *dev) +{
struct usb_device *udev = dev_get_parent_priv(dev);struct eth_pdata *pdata = dev_get_platdata(dev);struct r8152 *tp = dev_get_priv(dev);struct ueth_data *ueth = &tp->ueth;tp->udev = udev;r8152_read_mac(tp, pdata->enetaddr);r8152b_get_version(tp);if (rtl_ops_init(tp))return 0;Why 0? Isn't this an error?
Right. Will fix in v2.
Thanks for the review, Stefan