Re: [U-Boot] [PATCH v2] Add support for Microchip LAN78xx

On 03/10/2017 07:38 PM, Yuiko.Oshino@microchip.com wrote:

From: Yuiko Oshino yuiko.oshino@microchip.com

Add support for Microchip LAN7800 and 7850, USB to 10/100/1000 Ethernet Controllers

Signed-off-by: Yuiko Oshino yuiko.oshino@microchip.com Cc: Marek Vasut marex@denx.de

---

Changes for v2:

• lan78xx.c header comment cleanup

drivers/usb/eth/Makefile | 1 + drivers/usb/eth/lan78xx.c | 1319 ++++++++++++++++++++++++++++++++++++++++++ drivers/usb/eth/usb_ether.c | 7 + include/usb_ether.h | 6 + scripts/config_whitelist.txt | 1 + 5 files changed, 1334 insertions(+) create mode 100644 drivers/usb/eth/lan78xx.c

diff --git a/drivers/usb/eth/Makefile b/drivers/usb/eth/Makefile index 4c44efc..aa8a6cf 100644 --- a/drivers/usb/eth/Makefile +++ b/drivers/usb/eth/Makefile @@ -9,4 +9,5 @@ obj-$(CONFIG_USB_ETHER_ASIX) += asix.o obj-$(CONFIG_USB_ETHER_ASIX88179) += asix88179.o obj-$(CONFIG_USB_ETHER_MCS7830) += mcs7830.o obj-$(CONFIG_USB_ETHER_SMSC95XX) += smsc95xx.o +obj-$(CONFIG_USB_ETHER_LAN78XX) += lan78xx.o obj-$(CONFIG_USB_ETHER_RTL8152) += r8152.o r8152_fw.o diff --git a/drivers/usb/eth/lan78xx.c b/drivers/usb/eth/lan78xx.c new file mode 100644 index 0000000..bc25c16 --- /dev/null +++ b/drivers/usb/eth/lan78xx.c @@ -0,0 +1,1319 @@ +/*

• • Copyright (c) 2017 Microchip Technology Inc. All rights reserved.
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <malloc.h> +#include <memalign.h> +#include <usb.h> +#include <linux/mii.h> +#include "usb_ether.h"

+/* USB Vendor Requests */ +#define USB_VENDOR_REQUEST_WRITE_REGISTER 0xA0 +#define USB_VENDOR_REQUEST_READ_REGISTER 0xA1 +#define USB_VENDOR_REQUEST_GET_STATS 0xA2

+/* Tx Command A */ +#define TX_CMD_A_FCS_ (0x00400000) +#define TX_CMD_A_LEN_MASK_ (0x000FFFFF)

Drop all those extra parenthesis around constants ...

+/* Rx Command A */ +#define RX_CMD_A_RX_ERRS_MASK_ (0xC03F0000) +#define RX_CMD_A_RXE_ (0x00040000) +#define RX_CMD_A_LEN_MASK_ (0x00003FFF)

+/* SCSRs */ +#define ID_REV (0x00) +#define ID_REV_CHIP_ID_MASK_ (0xFFFF0000) +#define ID_REV_CHIP_REV_MASK_ (0x0000FFFF) +#define ID_REV_CHIP_ID_7800_ (0x7800) +#define ID_REV_CHIP_ID_7850_ (0x7850)

+#define INT_STS (0x0C)

+#define HW_CFG (0x010) +#define HW_CFG_LED1_EN_ (0x00200000) +#define HW_CFG_LED0_EN_ (0x00100000) /* Muxed with EEDO */ +#define HW_CFG_LRST_ (0x00000002) /* Mused with EECLK */ +#define HW_CFG_SRST_ (0x00000001)

+#define PMT_CTL (0x014) +#define PMT_CTL_READY_ (0x00000080) +#define PMT_CTL_PHY_RST_ (0x00000010)

+#define E2P_CMD (0x040) +#define E2P_CMD_EPC_BUSY_ (0x80000000) +#define E2P_CMD_EPC_CMD_MASK_ (0x70000000) +#define E2P_CMD_EPC_CMD_READ_ (0x00000000) +#define E2P_CMD_EPC_TIMEOUT_ (0x00000400) +#define E2P_CMD_EPC_ADDR_MASK_ (0x000001FF)

Drop the extra trailing underscore ...

+#define E2P_DATA (0x044) +#define E2P_DATA_EEPROM_DATA_MASK_ (0x000000FF)

Why does a lot of this stuff look like the SMSC95xx driver ? Do we even need a separate driver or can the SMSC95xx driver be updated to support the LAN78xx ?

+#define USB_CFG0 (0x080) +#define USB_CFG_BIR_ (0x00000040)

Looks like a BIT() macro can be used a lot in here ...

+#define BURST_CAP (0x090) +#define BURST_CAP_SIZE_MASK_ (0x000000FF)

[...]

+/* local defines */ +#define LAN78XX_BASE_NAME "78xx" +#define USB_CTRL_SET_TIMEOUT 5000 +#define USB_CTRL_GET_TIMEOUT 5000 +#define USB_BULK_SEND_TIMEOUT 5000 +#define USB_BULK_RECV_TIMEOUT 5000

the macro should have a unit in it, like _MS or such ...

+#define RX_URB_SIZE 2048 +#define PHY_CONNECT_TIMEOUT 5000

+#ifndef CONFIG_DM_ETH +/* local vars */ +static int curr_eth_dev; /* index for name of next device detected */ +#endif

+/* driver private */ +struct lan78xx_private { +#ifdef CONFIG_DM_ETH

• struct ueth_data ueth;

+#endif

• size_t rx_urb_size; /* maximum USB URB size */
• u32 mac_cr; /* MAC control register value */
• int have_hwaddr; /* 1 if we have a hardware MAC address */

+};

+/*

• • Lan78xx infrastructure commands
• */

+static int lan78xx_write_reg(struct usb_device *udev, u32 index, u32 data) +{

• int len;
• ALLOC_CACHE_ALIGN_BUFFER(u32, tmpbuf, 1);
• cpu_to_le32s(&data);
• tmpbuf[0] = data;
• len = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),

• 	USB_VENDOR_REQUEST_WRITE_REGISTER,
  

• 	USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
  

• 	0, index, tmpbuf, sizeof(data),
  

• 	USB_CTRL_SET_TIMEOUT);
  

• if (len != sizeof(data)) {

• debug("lan78xx_write_reg failed: index=%d, data=%d, len=%d",
  

^^ just use __func__

•       index, data, len);
  

• return -EIO;
  

• }
• return 0;

+}

+static int lan78xx_read_reg(struct usb_device *udev, u32 index, u32 *data) +{

• int len;
• ALLOC_CACHE_ALIGN_BUFFER(u32, tmpbuf, 1);
• len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),

• 	USB_VENDOR_REQUEST_READ_REGISTER,
  

• 	USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
  

• 	0, index, tmpbuf, sizeof(*data),
  

• 	USB_CTRL_GET_TIMEOUT);
  

• *data = tmpbuf[0];
• if (len != sizeof(*data)) {

• debug("lan78xx_read_reg failed: index=%d, len=%d", index, len);
  

DTTO

• return -EIO;
  

• }
• le32_to_cpus(data);
• return 0;

+}

+/* Loop until the read is completed with timeout */ +static int lan78xx_phy_wait_not_busy(struct usb_device *udev) +{

• unsigned long start_time = get_timer(0);
• u32 val;
• do {

• lan78xx_read_reg(udev, MII_ACC, &val);
  

• if (!(val & MII_ACC_MII_BUSY_))
  

• 	return 0;
  

• } while (get_timer(start_time) < 1000);
• return -ETIMEDOUT;

+}

+static int lan78xx_mdio_read(struct usb_device *udev, int phy_id, int idx) +{

• u32 val, addr;
• /* confirm MII not busy */
• if (lan78xx_phy_wait_not_busy(udev)) {

• debug("MII is busy in lan78xx_mdio_read\n");
  

• return -ETIMEDOUT;
  

• }
• /* set the address, index & direction (read from PHY) */
• addr = (phy_id << 11) | (idx << 6) |

• MII_ACC_MII_READ_ | MII_ACC_MII_BUSY_;
  

• lan78xx_write_reg(udev, MII_ACC, addr);
• if (lan78xx_phy_wait_not_busy(udev)) {

• debug("Timed out reading MII reg %02X\n", idx);
  

• return -ETIMEDOUT;
  

• }
• lan78xx_read_reg(udev, MII_DATA, &val);
• return (u16) (val & 0xFFFF);

+}

+static void lan78xx_mdio_write(struct usb_device *udev, int phy_id, int idx,

• 		int regval)
  

+{

• u32 val, addr;
• /* confirm MII not busy */
• if (lan78xx_phy_wait_not_busy(udev)) {

• debug("MII is busy in lan78xx_mdio_write\n");
  

• return;
  

• }
• val = regval;
• lan78xx_write_reg(udev, MII_DATA, val);
• /* set the address, index & direction (write to PHY) */
• addr = (phy_id << 11) | (idx << 6) |

• MII_ACC_MII_WRITE_ | MII_ACC_MII_BUSY_;
  

• lan78xx_write_reg(udev, MII_ACC, addr);
• if (lan78xx_phy_wait_not_busy(udev))

• debug("Timed out writing MII reg %02X\n", idx);
  

+}

+static int lan78xx_eeprom_confirm_not_busy(struct usb_device *udev) +{

• unsigned long start_time = get_timer(0);
• u32 val;
• do {

• lan78xx_read_reg(udev, E2P_CMD, &val);
  

• if (!(val & E2P_CMD_EPC_BUSY_))
  

• 	return 0;
  

• udelay(40);
  

• } while (get_timer(start_time) < 1000);

Looks like some wait_for_bit() alike function could reduce the duplication here, this lan78xx_wait_foo functions are here at least thrice.

• debug("EEPROM is busy\n");
• return -ETIMEDOUT;

+}

+static int lan78xx_wait_eeprom(struct usb_device *udev) +{

• unsigned long start_time = get_timer(0);
• u32 val;
• do {

• lan78xx_read_reg(udev, E2P_CMD, &val);
  

• if (!(val & E2P_CMD_EPC_BUSY_) || (val & E2P_CMD_EPC_TIMEOUT_))
  

• 	break;
  

• udelay(40);
  

• } while (get_timer(start_time) < 1000);
• if (val & (E2P_CMD_EPC_TIMEOUT_ | E2P_CMD_EPC_BUSY_)) {

• debug("EEPROM read operation timeout\n");
  

• return -ETIMEDOUT;
  

• }
• return 0;

+}

[...]

+static int lan78xx_read_mac(unsigned char *enetaddr,

• 			struct usb_device *udev)
  

+{

• int ret;
• memset(enetaddr, 0, 6);
• /* refer to the doc/README.enetaddr and doc/README.usb for

• * the U-Boot MAC address policy
  

• */
  

/* * multi * line * comment looks like this */

• /* try reading mac address from EEPROM, then from OTP */
• ret = lan78xx_read_eeprom(udev, 0, 1, enetaddr);
• if ((ret == 0) && (enetaddr[0] == EEPROM_INDICATOR)) {

• ret = lan78xx_read_eeprom(udev,
  

• 			  EEPROM_MAC_OFFSET, ETH_ALEN,
  

• 			  enetaddr);
  

• if ((ret == 0) && (is_valid_ethaddr(enetaddr))) {
  

• 	/* eeprom values are valid so use them */
  

• 	debug("MAC address read from EEPROM %pM\n",
  

• 	      enetaddr);
  

• 	return 0;
  

• }
  

• }
• debug("MAC address read from EEPROM invalid %pM\n", enetaddr);
• memset(enetaddr, 0, 6);
• /* try reading mac address from OTP */
• ret = lan78xx_read_otp(udev,

• 	       EEPROM_MAC_OFFSET,
  

• 	       ETH_ALEN,
  

• 	       enetaddr);
  

• if ((ret == 0) && (is_valid_ethaddr(enetaddr))) {

No need for parenthesis around function invocation

• /* eeprom values are valid so use them */
  

• debug("MAC address read from OTP %pM\n", enetaddr);
  

• return 0;
  

• }
• debug("MAC address read from OTP invalid %pM\n", enetaddr);
• memset(enetaddr, 0, 6);
• return 0;

+}

[...]

+static int lan78xx_basic_reset(struct usb_device *udev,

• 		struct ueth_data *dev)
  

+{

• int ret;
• u32 write_buf;
• u32 read_buf;
• int timeout;

+#define TIMEOUT_RESOLUTION 50 /* ms */

Yet another ad-hoc macro ?

• dev->phy_id = LAN78XX_INTERNAL_PHY_ID; /* fixed phy id */
• write_buf = HW_CFG_LRST_;
• ret = lan78xx_write_reg(udev, HW_CFG, write_buf);
• if (ret < 0)

• return ret;
  

• timeout = 0;
• do {

• ret = lan78xx_read_reg(udev, HW_CFG, &read_buf);
  

• if (ret < 0)
  

• 	return ret;
  

• udelay(10 * 1000);
  

• timeout++;
  

• } while ((read_buf & HW_CFG_LRST_) && (timeout < 100));

aka wait_for_bit() , again ?

• if (timeout >= 100) {

• debug("timeout waiting for completion of Lite Reset\n");
  

• return -ETIMEDOUT;
  

• }
• write_buf = PMT_CTL_PHY_RST_;
• ret = lan78xx_write_reg(udev, PMT_CTL, write_buf);
• if (ret < 0)

• return ret;
  

• timeout = 0;
• do {

• ret = lan78xx_read_reg(udev, PMT_CTL, &read_buf);
  

• if (ret < 0)
  

• 	return ret;
  

• udelay(10 * 1000);
  

• timeout++;
  

• if (timeout >= 100)
  

• 	break;
  

And again ?

• /* for LAN78xx, we need to check PMT_CTL_READY_ asserted */
• } while ((read_buf & PMT_CTL_PHY_RST_) || !(read_buf & PMT_CTL_READY_));
• if (timeout >= 100) {

• debug("timeout waiting for PHY Reset\n");
  

• return -ETIMEDOUT;
  

• }
• /* Respond to the IN token with a NAK */
• ret = lan78xx_read_reg(udev, USB_CFG0, &write_buf);
• if (ret < 0)

• return ret;
  

• write_buf |= USB_CFG_BIR_;
• ret = lan78xx_write_reg(udev, USB_CFG0, write_buf);
• if (ret < 0)

• return ret;
  

• return 0;

+}

+static int lan78xx_init_common(struct usb_device *udev,

• 		struct ueth_data *dev,
  

• 		struct lan78xx_private *priv,
  

• 		unsigned char *enetaddr)
  

+{

• int ret;
• u32 write_buf;
• u32 read_buf;
• u32 burst_cap;
• int timeout;

+#define TIMEOUT_RESOLUTION 50 /* ms */

This is redefined ...

• int link_detected;
• /* Reset and read Mac addr were done in get_info() or in probe() */
• ret = lan78xx_read_reg(udev, MAC_CR, &write_buf);
• if (ret < 0)

• return ret;
  

• write_buf |= MAC_CR_AUTO_DUPLEX_ | MAC_CR_AUTO_SPEED_ | MAC_CR_ADP_;
• ret = lan78xx_write_reg(udev, MAC_CR, write_buf);
• if (ret < 0)

• return ret;
  

+#ifndef CONFIG_DM_ETH

• if (!priv->have_hwaddr && (is_valid_ethaddr(enetaddr))) {

• priv->have_hwaddr = 1;
  

• debug("LAN78xx: MAC address found and set %pM\n", enetaddr);
  

• }

+#endif

• if (!priv->have_hwaddr) {

• printf("Error: LAN78xx: No MAC address set - set usbethaddr\n");
  

• return -EADDRNOTAVAIL;
  

• }
• ret = lan78xx_write_hwaddr_common(udev, priv, enetaddr);
• if (ret < 0)

• return ret;
  

• burst_cap = 0;
• priv->rx_urb_size = RX_URB_SIZE;
• debug("rx_urb_size=%ld\n", (ulong) priv->rx_urb_size);
• ret = lan78xx_write_reg(udev, BURST_CAP, burst_cap);
• if (ret < 0)

• return ret;
  

• read_buf = DEFAULT_BULK_IN_DELAY;
• ret = lan78xx_write_reg(udev, BULK_IN_DLY, read_buf);
• if (ret < 0)

• return ret;
  

• write_buf = 0xFFFFFFFF;
• ret = lan78xx_write_reg(udev, INT_STS, write_buf);

Just use the value directly ?

• if (ret < 0)

• return ret;
  

• ret = lan78xx_read_reg(udev, ID_REV, &read_buf);
• if (ret < 0)

• return ret;
  

• debug("LAN78xx ID_REV = 0x%08x\n", read_buf);
• /* set FIFO sizes */
• write_buf = (MAX_RX_FIFO_SIZE - 512) / 512;
• ret = lan78xx_write_reg(udev, FCT_RX_FIFO_END, write_buf);
• if (ret < 0)

• return ret;
  

• write_buf = (MAX_TX_FIFO_SIZE - 512) / 512;
• ret = lan78xx_write_reg(udev, FCT_TX_FIFO_END, write_buf);
• if (ret < 0)

• return ret;
  

• /* Init Tx */
• write_buf = 0;
• ret = lan78xx_write_reg(udev, FLOW, write_buf);
• if (ret < 0)

• return ret;
  

• ret = lan78xx_read_reg(udev, MAC_CR, &priv->mac_cr);
• if (ret < 0)

• return ret;
  

• /* Init Rx. Set Vlan, keep defult for VLAN on 78xx */
• ret = lan78xx_set_multicast(udev, priv);
• if (ret < 0)

• return ret;
  

• ret = lan78xx_phy_initialize(udev, dev);
• if (ret < 0)

• return ret;
  

• lan78xx_start_tx_path(udev, priv);
• lan78xx_start_rx_path(udev, priv);
• timeout = 0;
• do {

• link_detected = lan78xx_mdio_read(udev, dev->phy_id, MII_BMSR)
  

• 	& BMSR_LSTATUS;
  

• if (!link_detected) {
  

• 	if (timeout == 0) {
  

• 		printf("LAN78xx: ");
  

• 		printf("Waiting for Ethernet connection... ");
  

• 	}
  

• 	udelay(TIMEOUT_RESOLUTION * 1000);
  

• 	timeout += TIMEOUT_RESOLUTION;
  

• }
  

• } while (!link_detected && timeout < PHY_CONNECT_TIMEOUT);

We had this wait_for_bit() before ...

• if (link_detected) {

• if (timeout != 0)
  

• 	printf("done.\n");
  

• else
  

• 	printf("LAN78xx: Ethernet connection done.\n");
  

• link_detected = lan78xx_mdio_read(udev, dev->phy_id, MII_BMSR);
  

• debug("MII_BMSR=0x%08x\n", link_detected);
  

• ret = lan78xx_update_flowcontrol(udev, dev);
  

• if (ret < 0)
  

• 	return ret;
  

• } else {

• printf("unable to connect.\n");
  

• return -EIO;
  

• }
• return 0;

+}

+static int lan78xx_send_common(struct ueth_data *dev, void *packet, int length) +{

• int err;
• int actual_len;
• u32 tx_cmd_a;
• u32 tx_cmd_b;
• ALLOC_CACHE_ALIGN_BUFFER(unsigned char, msg,

• 		 PKTSIZE + sizeof(tx_cmd_a) + sizeof(tx_cmd_b));
  

• debug("** %s(), len %d, buf %#x\n", __func__, length,

•      (unsigned int)(ulong) msg);
  

• if (length > PKTSIZE)

• return -ENOSPC;
  

• /* LAN78xx disable all TX offload features for u-boot */
• tx_cmd_a = (u32) (length & TX_CMD_A_LEN_MASK_) | TX_CMD_A_FCS_;
• tx_cmd_b = 0;
• cpu_to_le32s(&tx_cmd_a);
• cpu_to_le32s(&tx_cmd_b);
• /* prepend cmd_a and cmd_b */
• memcpy(msg, &tx_cmd_a, sizeof(tx_cmd_a));
• memcpy(msg + sizeof(tx_cmd_a), &tx_cmd_b, sizeof(tx_cmd_b));
• memcpy(msg + sizeof(tx_cmd_a) + sizeof(tx_cmd_b), (void *)packet,

•       length);
  

• err = usb_bulk_msg(dev->pusb_dev,

• 	usb_sndbulkpipe(dev->pusb_dev, dev->ep_out),
  

• 	(void *)msg,
  

• 	length + sizeof(tx_cmd_a) +
  

• 	sizeof(tx_cmd_b),
  

• 	&actual_len, USB_BULK_SEND_TIMEOUT);
  

• debug("Tx: len = %u, actual = %u, err = %d\n",

•      (unsigned int)(length + sizeof(tx_cmd_a) + sizeof(tx_cmd_b)),
  

•      (unsigned int)actual_len, err);
  

• return err;

+}

[...]