From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Hershberger Date: Tue, 30 May 2017 11:54:13 -0500 Subject: [U-Boot] [PATCH] Add support for Microchip LAN75xx and LAN78xx In-Reply-To: <6AF1B4F019F8804DB17BB6F30B56F026C0B293@CHN-SV-EXMX02.mchp-main.com> References: <6AF1B4F019F8804DB17BB6F30B56F026BD37CF@CHN-SV-EXMX02.mchp-main.com> <6AF1B4F019F8804DB17BB6F30B56F026BF06EC@CHN-SV-EXMX02.mchp-main.com> <6AF1B4F019F8804DB17BB6F30B56F026C0B293@CHN-SV-EXMX02.mchp-main.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, May 24, 2017 at 10:14 AM, wrote: > From: Yuiko Oshino > >>-----Original Message----- >>From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of >>Yuiko.Oshino at microchip.com >>Sent: Wednesday, May 10, 2017 11:25 AM >>To: joe.hershberger at gmail.com >>Cc: marex at denx.de; u-boot at lists.denx.de >>Subject: Re: [U-Boot] [PATCH] Add support for Microchip LAN75xx and >>LAN78xx >> >>From: Yuiko Oshino >>>-----Original Message----- >>>From: Joe Hershberger [mailto:joe.hershberger at gmail.com] >>>Sent: Friday, May 5, 2017 4:59 PM >>>To: Yuiko Oshino - C18177 >>>Cc: u-boot; Marek Vasut >>>Subject: Re: [U-Boot] [PATCH] Add support for Microchip LAN75xx and >>>LAN78xx >>> >>>Hi Yuiko, >> >>Hi Joe! >> >>[...] [...] >>>> + >>>> +/* Loop until the read is completed with timeout */ int >>>> +lan7x_wait_for_bit(struct usb_device *udev, >>>> + const char *prefix, const u32 index, >>>> + const u32 mask, const bool set, >>>> + unsigned int timeout_ms) >>> >>>Can you not use the generic one? include/wait_bit.h >> >>We need to use our own register read function as our device is an USB device. >>It looks like wait_bit.h does not support function pointer to register read, >>therefore we need our own. At least copy the real one. >>> >>>> +{ >>>> + u32 val; >>>> + >>>> + while (--timeout_ms) { >>>> + lan7x_read_reg(udev, index, &val); >>>> + >>>> + if (!set) >>>> + val = ~val; >>>> + >>>> + if ((val & mask) == mask) >>>> + return 0; >>>> + >>>> + mdelay(1); >>>> + } >>>> + >>>> + debug("%s: Timeout (reg=0x%x mask=%08x wait_set=%i)\n", >>>> + prefix, index, mask, set); >>>> + >>>> + return -ETIMEDOUT; >>>> +} >>>> + >>>> +static int lan7x_phy_wait_not_busy(struct usb_device *udev) { >>>> + return lan7x_wait_for_bit(udev, __func__, >>>> + MII_ACC, MII_ACC_MII_BUSY, >>>> + false, 100); } >>>> + >>>> +int lan7x_mdio_read(struct usb_device *udev, int phy_id, int idx) { >>>> + u32 val, addr; >>>> + >>>> + /* confirm MII not busy */ >>>> + if (lan7x_phy_wait_not_busy(udev)) { >>>> + debug("MII is busy in %s\n", __func__); >>>> + return -ETIMEDOUT; >>>> + } >>>> + >>>> + /* set the address, index & direction (read from PHY) */ >>>> + addr = (phy_id << 11) | (idx << 6) | >>>> + MII_ACC_MII_READ | MII_ACC_MII_BUSY; >>>> + lan7x_write_reg(udev, MII_ACC, addr); >>>> + >>>> + if (lan7x_phy_wait_not_busy(udev)) { >>>> + debug("Timed out reading MII reg %02X\n", idx); >>>> + return -ETIMEDOUT; >>>> + } >>>> + >>>> + lan7x_read_reg(udev, MII_DATA, &val); >>>> + >>>> + return (u16) (val & 0xFFFF); >>>> +} >>>> + >>>> +int lan7x_mdio_wait_for_bit(struct usb_device *udev, >>>> + const char *prefix, >>>> + int phy_id, const u32 index, >>>> + const u32 mask, const bool set, >>>> + unsigned int timeout_ms) { >>>> + u32 val; >>>> + >>>> + while (--timeout_ms) { >>>> + val = lan7x_mdio_read(udev, phy_id, index); >>>> + >>>> + if (!set) >>>> + val = ~val; >>>> + >>>> + if ((val & mask) == mask) >>>> + return 0; >>>> + >>>> + mdelay(1); >>>> + } >>>> + >>>> + debug("%s: Timeout (reg=0x%x mask=%08x wait_set=%i)\n", >>>> + prefix, index, mask, set); >>>> + >>>> + return -ETIMEDOUT; >>>> +} >>>> + >> >>[...] >> >>>> +static int lan7x_mii_get_an(uint32_t advertising_reg) { >>>> + int advertising = 0; >>>> + >>>> + if (advertising_reg & LPA_LPACK) >>>> + advertising |= ADVERTISED_Autoneg; >>>> + if (advertising_reg & ADVERTISE_10HALF) >>>> + advertising |= ADVERTISED_10baseT_Half; >>>> + if (advertising_reg & ADVERTISE_10FULL) >>>> + advertising |= ADVERTISED_10baseT_Full; >>>> + if (advertising_reg & ADVERTISE_100HALF) >>>> + advertising |= ADVERTISED_100baseT_Half; >>>> + if (advertising_reg & ADVERTISE_100FULL) >>>> + advertising |= ADVERTISED_100baseT_Full; >>>> + >>>> + return advertising; >>>> +} >>>> + >>>> +int lan7x_update_flowcontrol(struct usb_device *udev, >>>> + struct ueth_data *dev, >>>> + uint32_t *flow, uint32_t *fct_flow) { >>>> + uint32_t lcladv, rmtadv, ctrl1000, stat1000; >>>> + uint32_t advertising = 0, lp_advertising = 0, nego = 0; >>>> + uint32_t duplex = 0; >>>> + u8 cap = 0; >>>> + >>>> + lcladv = lan7x_mdio_read(udev, dev->phy_id, MII_ADVERTISE); >>>> + advertising = lan7x_mii_get_an(lcladv); >>>> + >>>> + rmtadv = lan7x_mdio_read(udev, dev->phy_id, MII_LPA); >>>> + lp_advertising = lan7x_mii_get_an(rmtadv); >>>> + >>>> + ctrl1000 = lan7x_mdio_read(udev, dev->phy_id, MII_CTRL1000); >>>> + stat1000 = lan7x_mdio_read(udev, dev->phy_id, MII_STAT1000); >>>> + >>>> + if (ctrl1000 & ADVERTISE_1000HALF) >>>> + advertising |= ADVERTISED_1000baseT_Half; >>>> + >>>> + if (ctrl1000 & ADVERTISE_1000FULL) >>>> + advertising |= ADVERTISED_1000baseT_Full; >>>> + >>>> + if (stat1000 & LPA_1000HALF) >>>> + lp_advertising |= ADVERTISED_1000baseT_Half; >>>> + >>>> + if (stat1000 & LPA_1000FULL) >>>> + lp_advertising |= ADVERTISED_1000baseT_Full; >>>> + >>>> + nego = advertising & lp_advertising; >>>> + >>>> + debug("LAN7x linked at "); >>>> + >>>> + if (nego & (ADVERTISED_1000baseT_Full | >>>ADVERTISED_1000baseT_Half)) { >>>> + debug("1000 "); >>>> + duplex = !!(nego & ADVERTISED_1000baseT_Full); >>>> + >>>> + } else if (nego & (ADVERTISED_100baseT_Full | >>>> + ADVERTISED_100baseT_Half)) { >>>> + debug("100 "); >>>> + duplex = !!(nego & ADVERTISED_100baseT_Full); >>>> + } else { >>>> + debug("10 "); >>>> + duplex = !!(nego & ADVERTISED_10baseT_Full); >>>> + } >>>> + >>>> + if (duplex == DUPLEX_FULL) >>>> + debug("full dup "); >>>> + else >>>> + debug("half dup "); >>>> + >>>> + if (duplex == DUPLEX_FULL) { >>>> + if (lcladv & rmtadv & ADVERTISE_PAUSE_CAP) { >>>> + cap = FLOW_CTRL_TX | FLOW_CTRL_RX; >>>> + } else if (lcladv & rmtadv & ADVERTISE_PAUSE_ASYM) { >>>> + if (lcladv & ADVERTISE_PAUSE_CAP) >>>> + cap = FLOW_CTRL_RX; >>>> + else if (rmtadv & LPA_PAUSE_CAP) >>>> + cap = FLOW_CTRL_TX; >>>> + } >>>> + debug("TX Flow "); >>>> + if (cap & FLOW_CTRL_TX) { >>>> + *flow = (FLOW_CR_TX_FCEN | 0xFFFF); >>>> + /* set fct_flow thresholds to 20% and 80% */ >>>> + *fct_flow = (((MAX_RX_FIFO_SIZE * 2) / (10 * 512)) >>>> + & 0x7FUL); >>>> + *fct_flow <<= 8UL; >>>> + *fct_flow |= (((MAX_RX_FIFO_SIZE * 8) / (10 * 512)) >>>> + & 0x7FUL); >>>> + debug("EN "); >>>> + } else { >>>> + debug("DIS "); >>>> + } >>>> + debug("RX Flow "); >>>> + if (cap & FLOW_CTRL_RX) { >>>> + *flow |= FLOW_CR_RX_FCEN; >>>> + debug("EN"); >>>> + } else { >>>> + debug("DIS"); >>>> + } >>>> + } >>>> + debug("\n"); >>>> + return 0; >>>> +} >>> >>>I see where Marek is coming from wrt thisall being in phylib already. >>>I guess you always have a fixed phy internal, so there's no need of the >>>flexibility of phylib. Maybe there's at least opportunity to >>>consolidate subroutines even if not using phylib the normal way. >> >>I am a bit confused what you say. Do you mean that I can keep the current >>code as is in this area? >>Please confirm? >>The access to PHY also needs our own register read/write through USB, so >>using the phylib is more complicated. The thought here is to do a minor refactor of the phy.c code such that all this parsing of the bits can be shared code, while access to the MDIO interface is specialized for your driver. >>>> + >>>> +int lan7x_eth_probe(struct usb_device *dev, unsigned int ifnum, >>>> + struct ueth_data *ss) { >>>> + struct usb_interface *iface; >>>> + struct usb_interface_descriptor *iface_desc; >>>> + int i; >>>> + >>>> + iface = &dev->config.if_desc[ifnum]; >>>> + iface_desc = &dev->config.if_desc[ifnum].desc; >>>> + >>>> + memset(ss, '\0', sizeof(struct ueth_data)); >>>> + >>>> + /* Initialize the ueth_data structure with some useful info */ >>>> + ss->ifnum = ifnum; >>>> + ss->pusb_dev = dev; >>>> + ss->subclass = iface_desc->bInterfaceSubClass; >>>> + ss->protocol = iface_desc->bInterfaceProtocol; >>>> + >>>> + /* >>>> + * We are expecting a minimum of 3 endpoints >>>> + * - in, out (bulk), and int. >>>> + * We will ignore any others. >>>> + */ >>>> + for (i = 0; i < iface_desc->bNumEndpoints; i++) { >>>> + /* is it an BULK endpoint? */ >>>> + if ((iface->ep_desc[i].bmAttributes & >>>> + USB_ENDPOINT_XFERTYPE_MASK) == >>>USB_ENDPOINT_XFER_BULK) { >>>> + if (iface->ep_desc[i].bEndpointAddress & USB_DIR_IN) >>>> + ss->ep_in = >>>> + iface->ep_desc[i].bEndpointAddress & >>>> + USB_ENDPOINT_NUMBER_MASK; >>>> + else >>>> + ss->ep_out = >>>> + iface->ep_desc[i].bEndpointAddress & >>>> + USB_ENDPOINT_NUMBER_MASK; >>>> + } >>>> + >>>> + /* is it an interrupt endpoint? */ >>>> + if ((iface->ep_desc[i].bmAttributes & >>>> + USB_ENDPOINT_XFERTYPE_MASK) == >>>USB_ENDPOINT_XFER_INT) { >>>> + ss->ep_int = iface->ep_desc[i].bEndpointAddress & >>>> + USB_ENDPOINT_NUMBER_MASK; >>>> + ss->irqinterval = iface->ep_desc[i].bInterval; >>>> + } >>>> + } >>>> + debug("Endpoints In %d Out %d Int %d\n", >>>> + ss->ep_in, ss->ep_out, ss->ep_int); >>>> + >>>> + /* Do some basic sanity checks, and bail if we find a problem */ >>>> + if (usb_set_interface(dev, iface_desc->bInterfaceNumber, 0) || >>>> + !ss->ep_in || !ss->ep_out || !ss->ep_int) { >>>> + debug("Problems with device\n"); >>>> + return 0; >>> >>>Seems this should return an error. >> >>The caller probe_valid_drivers() in usb_ether.c expects 0 for skipping a bad >>device. OK. >>>> + } >>>> + dev->privptr = (void *)ss; >>>> + >>>> +#ifndef CONFIG_DM_ETH >>>> + /* alloc driver private */ >>>> + ss->dev_priv = calloc(1, sizeof(struct lan7x_private)); >>>> + if (!ss->dev_priv) >>>> + return 0; >>>> +#endif >>>> + >>>> + return 1; >>>> +} >>>> + >> >>Thank you. >>Yuiko >>_______________________________________________ >>U-Boot mailing list >>U-Boot at lists.denx.de >>https://lists.denx.de/listinfo/u-boot > > Joe, > Please reply so that I can re-submit the patch. > Thank you in advance. > Best regards, > Yuiko