Re: [PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"
On 13 April 2018 5:59:51 AM IST, Greg Hackmannwrote: >Pixel 2 field testers reported that when they tried to reboot their >phones with some USB devices plugged in, the reboot would get wedged >and >eventually trigger watchdog reset. Once the Pixel kernel team found a >reliable repro case, they narrowed it down to this commit's 4.4.y >backport. Reverting the change made the issue go away. Are you allowed to make the repro steps public? I'm writing this from a walleye and would be grateful if I could test for this in the modifed tree I'm running atm. -- Harsh Shandilya, PRJKT Development LLC -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT/PATCH 18/38] usb: dwc3: gadget: check for Missed Isoc from event status
Hi, On 4/12/2018 12:18 AM, Felipe Balbi wrote: > > Hi, > > Thinh Nguyenwrites: >> Hi, >> >> On 4/11/2018 1:21 AM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Felipe Balbi writes: >> Without XferNotReady, we won't have a reliable way to know the uFrame >> number. Read the Isochronous programming sequence from your databook. > > Right. We need XferNotReady to know when to start isoc transfer. But if > there are still queued requests, DWC3 can just wait to see if any of > them will succeed to continue with the transfer just as how DWC3 is > handling it now. That's not what the databook says though. And that's also not intention of how the code is written as of now either. The way the code is written is the following: queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() -> queue() -> end_transfer. That's not really waiting for the queue to be consumed, it's just delaying end transfer until we get another queue(). IOW, it just *happens* to give the controller time to go through the list of started requests. > If we end and restart the transfer right away, then we may lose more > isoc data than necessary (due to isoc scheduling at least 4 uFrame > ahead of time). Is there something you see that doesn't work with the > current implementation? Not _really_, I'm just trying to make the code easier to read and, I think, I've achieved that. Now, if we need to delay end transfer in the case where we have more requests in the controller's queue, that's easy enough to implement: @@ -2371,7 +2371,8 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_BUSERR) status = -ECONNRESET; - if (event->status & DEPEVT_STATUS_MISSED_ISOC) { + if (event->status & DEPEVT_STATUS_MISSED_ISOC && + list_empty(>started_list) { status = -EXDEV; >> >> Maybe we should return the -EXDEV status every time there's a missed isoc. > > you mean like this? Yes, this will work. > > @@ -2358,10 +2358,11 @@ static void > dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, > if (event->status & DEPEVT_STATUS_BUSERR) > status = -ECONNRESET; > > - if (event->status & DEPEVT_STATUS_MISSED_ISOC && > - list_empty(>started_list)) { > + if (event->status & DEPEVT_STATUS_MISSED_ISOC) { > status = -EXDEV; > - stop = true; > + > + if (list_empty(>started_list)) > + stop = true; > } > > dwc3_gadget_ep_cleanup_completed_requests(dep, event, status); > BR, Thinh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Revert "xhci: plat: Register shutdown for xhci_plat"
Pixel 2 field testers reported that when they tried to reboot their phones with some USB devices plugged in, the reboot would get wedged and eventually trigger watchdog reset. Once the Pixel kernel team found a reliable repro case, they narrowed it down to this commit's 4.4.y backport. Reverting the change made the issue go away. This reverts commit b07c12517f2aed0add8ce18146bb426b14099392. Cc: sta...@vger.kernel.org Signed-off-by: Greg Hackmann--- drivers/usb/host/xhci-plat.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index df327dcc2bac..ea089fdda611 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -420,7 +420,6 @@ MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match); static struct platform_driver usb_xhci_driver = { .probe = xhci_plat_probe, .remove = xhci_plat_remove, - .shutdown = usb_hcd_platform_shutdown, .driver = { .name = "xhci-hcd", .pm = _plat_pm_ops, -- 2.17.0.484.g0c8726318c-goog -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
> > @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > > (void)lan78xx_set_eee(dev->net, ); > > } > > > > + if (!of_property_read_u32_array(dev->udev->dev.of_node, > > + "microchip,led-modes", > > + led_modes, ARRAY_SIZE(led_modes))) { > > + u32 reg; > > + int i; > > + > > + reg = phy_read(phydev, 0x1d); > > + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { > > + reg &= ~(0xf << (i * 4)); > > + reg |= (led_modes[i] & 0xf) << (i * 4); > > + } > > + (void)phy_write(phydev, 0x1d, reg); > > Poking PHY registers directly from the MAC driver is not always a good > idea. This MAC driver does that in a few places :-( Agree but, some are for workaround unfortunately. > What do we know about the PHY? It is built into the device or is it > external? If it is external, how do you know the LED register is at > 0x1d? This register is not defined in include/linux/microchipphy.h. :( Also agree that there parts should be applied to internal PHY only. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
Andrew, On 12/04/2018 15:16, Andrew Lunn wrote: > On Thu, Apr 12, 2018 at 02:55:34PM +0100, Phil Elwell wrote: >> Add two new Device Tree properties: >> * microchip,eee-enabled - a boolean to enable EEE >> * microchip,tx-lpi-timer - time in microseconds to wait after TX goes >>idle before entering the low power state >>(default 600) > > Hi Phil > > This looks wrong. > > What should happen is that the MAC driver calls phy_init_eee() to find > out if the PHY supports EEE. There should be no need to look in device > tree. If the driver should be calling phy_init_eee to initialise EEE operation then I'm fine with that (although I notice that the TI cpsw calls phy_ethtool_set_eee but I don't see it calling phy_init_eee). However, it sounds like I need to keep my DT toggle of the EEE enablement and parameters downstream. Phil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > (void)lan78xx_set_eee(dev->net, ); > } > > + if (!of_property_read_u32_array(dev->udev->dev.of_node, > + "microchip,led-modes", > + led_modes, ARRAY_SIZE(led_modes))) { > + u32 reg; > + int i; > + > + reg = phy_read(phydev, 0x1d); > + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { > + reg &= ~(0xf << (i * 4)); > + reg |= (led_modes[i] & 0xf) << (i * 4); > + } > + (void)phy_write(phydev, 0x1d, reg); Poking PHY registers directly from the MAC driver is not always a good idea. This MAC driver does that in a few places :-( What do we know about the PHY? It is built into the device or is it external? If it is external, how do you know the LED register is at 0x1d? The safest place to do this is in the PHY driver, and place these OF properties into the PHY node. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
Hi Andrew, On 12/04/2018 15:30, Andrew Lunn wrote: > On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: >> The Microchip LAN78XX family of devices are Ethernet controllers with >> a USB interface. Despite being discoverable devices it can be useful to >> be able to configure them from Device Tree, particularly in low-cost >> applications without an EEPROM or programmed OTP. > > It would be good to document what happens when there is an EEPROM. Is > OF used in preference to the EEPROM? Yes it is. I'll mention it in V2. Phil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > The Microchip LAN78XX family of devices are Ethernet controllers with > a USB interface. Despite being discoverable devices it can be useful to > be able to configure them from Device Tree, particularly in low-cost > applications without an EEPROM or programmed OTP. It would be good to document what happens when there is an EEPROM. Is OF used in preference to the EEPROM? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
Hi Andrew, On 12/04/2018 15:26, Andrew Lunn wrote: > On Thu, Apr 12, 2018 at 02:55:35PM +0100, Phil Elwell wrote: >> Add support for DT property "microchip,led-modes", a vector of two >> cells (u32s) in the range 0-15, each of which sets the mode for one >> of the two LEDs. Some possible values are: >> >> 0=link/activity 1=link1000/activity >> 2=link100/activity 3=link10/activity >> 4=link100/1000/activity 5=link10/1000/activity >> 6=link10/100/activity14=off15=on >> >> Also use the presence of the DT property to indicate that the >> LEDs should be enabled - necessary in the event that no valid OTP >> or EEPROM is available. > > I'm not a fan of this, but at the moment, we don't have anything > better. > > Please follow what mscc does, add a header file for the LED settings. Good idea. > >> >> Signed-off-by: Phil Elwell>> --- >> drivers/net/usb/lan78xx.c | 20 >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c >> index d98397b..ffb483d 100644 >> --- a/drivers/net/usb/lan78xx.c >> +++ b/drivers/net/usb/lan78xx.c >> @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) >> { >> int ret; >> u32 mii_adv; >> +u32 led_modes[2]; >> struct phy_device *phydev; >> >> phydev = phy_find_first(dev->mdiobus); >> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) >> (void)lan78xx_set_eee(dev->net, ); >> } >> >> +if (!of_property_read_u32_array(dev->udev->dev.of_node, >> +"microchip,led-modes", >> +led_modes, ARRAY_SIZE(led_modes))) { >> +u32 reg; >> +int i; >> + >> +reg = phy_read(phydev, 0x1d); >> +for (i = 0; i < ARRAY_SIZE(led_modes); i++) { >> +reg &= ~(0xf << (i * 4)); >> +reg |= (led_modes[i] & 0xf) << (i * 4); >> +} > > Please add range checks for led_modes[i] and return -EINVAL if the > check fails. Will do. Thanks, Phil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
On Thu, Apr 12, 2018 at 02:55:35PM +0100, Phil Elwell wrote: > Add support for DT property "microchip,led-modes", a vector of two > cells (u32s) in the range 0-15, each of which sets the mode for one > of the two LEDs. Some possible values are: > > 0=link/activity 1=link1000/activity > 2=link100/activity 3=link10/activity > 4=link100/1000/activity 5=link10/1000/activity > 6=link10/100/activity14=off15=on > > Also use the presence of the DT property to indicate that the > LEDs should be enabled - necessary in the event that no valid OTP > or EEPROM is available. I'm not a fan of this, but at the moment, we don't have anything better. Please follow what mscc does, add a header file for the LED settings. Andrew > > Signed-off-by: Phil Elwell> --- > drivers/net/usb/lan78xx.c | 20 > 1 file changed, 20 insertions(+) > > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c > index d98397b..ffb483d 100644 > --- a/drivers/net/usb/lan78xx.c > +++ b/drivers/net/usb/lan78xx.c > @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > { > int ret; > u32 mii_adv; > + u32 led_modes[2]; > struct phy_device *phydev; > > phydev = phy_find_first(dev->mdiobus); > @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) > (void)lan78xx_set_eee(dev->net, ); > } > > + if (!of_property_read_u32_array(dev->udev->dev.of_node, > + "microchip,led-modes", > + led_modes, ARRAY_SIZE(led_modes))) { > + u32 reg; > + int i; > + > + reg = phy_read(phydev, 0x1d); > + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { > + reg &= ~(0xf << (i * 4)); > + reg |= (led_modes[i] & 0xf) << (i * 4); > + } Please add range checks for led_modes[i] and return -EINVAL if the check fails. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] lan78xx: Read MAC address from DT if present
Hi Andrew, On 12/04/2018 15:06, Andrew Lunn wrote: > Hi Phil > >> -ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo); >> -ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi); >> +mac_addr = of_get_mac_address(dev->udev->dev.of_node); > > It might be better to use the higher level eth_platform_get_mac_address(). OK - I'll take your word for it. Phil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 03:10:57PM +0100, Phil Elwell wrote: > Hi Andrew, > > On 12/04/2018 15:04, Andrew Lunn wrote: > > On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > >> The Microchip LAN78XX family of devices are Ethernet controllers with > >> a USB interface. Despite being discoverable devices it can be useful to > >> be able to configure them from Device Tree, particularly in low-cost > >> applications without an EEPROM or programmed OTP. > >> > >> Document the supported properties in a bindings file, adding it to > >> MAINTAINERS at the same time. > > > > Hi Phil > > > > How you link an OF node to a USB device is not obvious. Could you > > please include either a pointer to some binding documentation, or make > > your example show it. > > Thanks for the feedback. Would you consider this (lifted from the Pi 3B+ > Device Tree) > a sufficient example? Yes, this is good. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
On Thu, Apr 12, 2018 at 02:55:34PM +0100, Phil Elwell wrote: > Add two new Device Tree properties: > * microchip,eee-enabled - a boolean to enable EEE > * microchip,tx-lpi-timer - time in microseconds to wait after TX goes >idle before entering the low power state >(default 600) Hi Phil This looks wrong. What should happen is that the MAC driver calls phy_init_eee() to find out if the PHY supports EEE. There should be no need to look in device tree. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
Hi Andrew, On 12/04/2018 15:04, Andrew Lunn wrote: > On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: >> The Microchip LAN78XX family of devices are Ethernet controllers with >> a USB interface. Despite being discoverable devices it can be useful to >> be able to configure them from Device Tree, particularly in low-cost >> applications without an EEPROM or programmed OTP. >> >> Document the supported properties in a bindings file, adding it to >> MAINTAINERS at the same time. > > Hi Phil > > How you link an OF node to a USB device is not obvious. Could you > please include either a pointer to some binding documentation, or make > your example show it. Thanks for the feedback. Would you consider this (lifted from the Pi 3B+ Device Tree) a sufficient example? { usb1@1 { compatible = "usb424,2514"; reg = <1>; #address-cells = <1>; #size-cells = <0>; usb1_1@1 { compatible = "usb424,2514"; reg = <1>; #address-cells = <1>; #size-cells = <0>; ethernet: usbether@1 { compatible = "usb424,7800"; reg = <1>; microchip,eee-enabled; microchip,tx-lpi-timer = <600>; /* non-aggressive*/ /* * led0 = 1:link1000/activity * led1 = 6:link10/100/activity */ microchip,led-modes = <1 6>; }; }; }; }; Phil -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] lan78xx: Read MAC address from DT if present
Hi Phil > - ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo); > - ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi); > + mac_addr = of_get_mac_address(dev->udev->dev.of_node); It might be better to use the higher level eth_platform_get_mac_address(). Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote: > The Microchip LAN78XX family of devices are Ethernet controllers with > a USB interface. Despite being discoverable devices it can be useful to > be able to configure them from Device Tree, particularly in low-cost > applications without an EEPROM or programmed OTP. > > Document the supported properties in a bindings file, adding it to > MAINTAINERS at the same time. Hi Phil How you link an OF node to a USB device is not obvious. Could you please include either a pointer to some binding documentation, or make your example show it. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] lan78xx: Read configuration from Device Tree
The Microchip LAN78XX family of devices are Ethernet controllers with a USB interface. Despite being discoverable devices it can be useful to be able to configure them from Device Tree, particularly in low-cost applications without an EEPROM or programmed OTP. This patch set adds support for reading the MAC address, EEE setting and LED modes from Device Tree. Phil Elwell (4): lan78xx: Read MAC address from DT if present lan78xx: Read initial EEE setting from Device Tree lan78xx: Read LED modes from Device Tree dt-bindings: Document the DT bindings for lan78xx .../devicetree/bindings/net/microchip,lan78xx.txt | 44 MAINTAINERS| 1 + drivers/net/usb/lan78xx.c | 81 -- 3 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/microchip,lan78xx.txt -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] lan78xx: Read MAC address from DT if present
There is a standard mechanism for locating and using a MAC address from the Device Tree. Use this facility in the lan78xx driver to support applications without programmed EEPROM or OTP. At the same time, regularise the handling of the different address sources. Signed-off-by: Phil Elwell--- drivers/net/usb/lan78xx.c | 44 +++- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 55a78eb..d2727b5 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "lan78xx.h" #define DRIVER_AUTHOR "WOOJUNG HUH " @@ -1651,34 +1652,35 @@ static void lan78xx_init_mac_address(struct lan78xx_net *dev) addr[5] = (addr_hi >> 8) & 0xFF; if (!is_valid_ether_addr(addr)) { - /* reading mac address from EEPROM or OTP */ - if ((lan78xx_read_eeprom(dev, EEPROM_MAC_OFFSET, ETH_ALEN, -addr) == 0) || - (lan78xx_read_otp(dev, EEPROM_MAC_OFFSET, ETH_ALEN, - addr) == 0)) { - if (is_valid_ether_addr(addr)) { - /* eeprom values are valid so use them */ - netif_dbg(dev, ifup, dev->net, - "MAC address read from EEPROM"); - } else { - /* generate random MAC */ - random_ether_addr(addr); - netif_dbg(dev, ifup, dev->net, - "MAC address set to random addr"); - } + const u8 *mac_addr; - addr_lo = addr[0] | (addr[1] << 8) | - (addr[2] << 16) | (addr[3] << 24); - addr_hi = addr[4] | (addr[5] << 8); - - ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo); - ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi); + mac_addr = of_get_mac_address(dev->udev->dev.of_node); + if (mac_addr) { + /* valid address present in Device Tree */ + ether_addr_copy(addr, mac_addr); + netif_dbg(dev, ifup, dev->net, + "MAC address read from Device Tree"); + } else if (((lan78xx_read_eeprom(dev, EEPROM_MAC_OFFSET, +ETH_ALEN, addr) == 0) || + (lan78xx_read_otp(dev, EEPROM_MAC_OFFSET, + ETH_ALEN, addr) == 0)) && + is_valid_ether_addr(addr)) { + /* eeprom values are valid so use them */ + netif_dbg(dev, ifup, dev->net, + "MAC address read from EEPROM"); } else { /* generate random MAC */ random_ether_addr(addr); netif_dbg(dev, ifup, dev->net, "MAC address set to random addr"); } + + addr_lo = addr[0] | (addr[1] << 8) | + (addr[2] << 16) | (addr[3] << 24); + addr_hi = addr[4] | (addr[5] << 8); + + ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo); + ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi); } ret = lan78xx_write_reg(dev, MAF_LO(0), addr_lo); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
Add two new Device Tree properties: * microchip,eee-enabled - a boolean to enable EEE * microchip,tx-lpi-timer - time in microseconds to wait after TX goes idle before entering the low power state (default 600) Signed-off-by: Phil Elwell--- drivers/net/usb/lan78xx.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index d2727b5..d98397b 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2080,6 +2080,23 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control); phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv); + if (of_property_read_bool(dev->udev->dev.of_node, + "microchip,eee-enabled")) { + struct ethtool_eee edata; + + memset(, 0, sizeof(edata)); + edata.cmd = ETHTOOL_SEEE; + edata.advertised = ADVERTISED_1000baseT_Full | + ADVERTISED_100baseT_Full; + edata.eee_enabled = true; + edata.tx_lpi_enabled = true; + if (of_property_read_u32(dev->udev->dev.of_node, +"microchip,tx-lpi-timer", +_lpi_timer)) + edata.tx_lpi_timer = 600; /* non-aggressive */ + (void)lan78xx_set_eee(dev->net, ); + } + genphy_config_aneg(phydev); dev->fc_autoneg = phydev->autoneg; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] lan78xx: Read LED modes from Device Tree
Add support for DT property "microchip,led-modes", a vector of two cells (u32s) in the range 0-15, each of which sets the mode for one of the two LEDs. Some possible values are: 0=link/activity 1=link1000/activity 2=link100/activity 3=link10/activity 4=link100/1000/activity 5=link10/1000/activity 6=link10/100/activity14=off15=on Also use the presence of the DT property to indicate that the LEDs should be enabled - necessary in the event that no valid OTP or EEPROM is available. Signed-off-by: Phil Elwell--- drivers/net/usb/lan78xx.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index d98397b..ffb483d 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) { int ret; u32 mii_adv; + u32 led_modes[2]; struct phy_device *phydev; phydev = phy_find_first(dev->mdiobus); @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) (void)lan78xx_set_eee(dev->net, ); } + if (!of_property_read_u32_array(dev->udev->dev.of_node, + "microchip,led-modes", + led_modes, ARRAY_SIZE(led_modes))) { + u32 reg; + int i; + + reg = phy_read(phydev, 0x1d); + for (i = 0; i < ARRAY_SIZE(led_modes); i++) { + reg &= ~(0xf << (i * 4)); + reg |= (led_modes[i] & 0xf) << (i * 4); + } + (void)phy_write(phydev, 0x1d, reg); + + /* Ensure the LEDs are enabled */ + lan78xx_read_reg(dev, HW_CFG, ); + reg |= HW_CFG_LED0_EN_ | HW_CFG_LED1_EN_; + lan78xx_write_reg(dev, HW_CFG, reg); + } + genphy_config_aneg(phydev); dev->fc_autoneg = phydev->autoneg; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
The Microchip LAN78XX family of devices are Ethernet controllers with a USB interface. Despite being discoverable devices it can be useful to be able to configure them from Device Tree, particularly in low-cost applications without an EEPROM or programmed OTP. Document the supported properties in a bindings file, adding it to MAINTAINERS at the same time. Signed-off-by: Phil Elwell--- .../devicetree/bindings/net/microchip,lan78xx.txt | 44 ++ MAINTAINERS| 1 + 2 files changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/microchip,lan78xx.txt diff --git a/Documentation/devicetree/bindings/net/microchip,lan78xx.txt b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt new file mode 100644 index 000..e7d7850 --- /dev/null +++ b/Documentation/devicetree/bindings/net/microchip,lan78xx.txt @@ -0,0 +1,44 @@ +Microchip LAN78xx Gigabit Ethernet controller + +The LAN78XX devices are usually configured by programming their OTP or with +an external EEPROM, but some platforms (e.g. Raspberry Pi 3 B+) have neither. + +Please refer to ethernet.txt for a description of common Ethernet bindings. + +Optional properties: +- microchip,eee-enabled: if present, enable Energy Efficient Ethernet support; +- microchip,led-modes: a two-element vector, with each element configuring + the operating mode of an LED. The values supported by the device are; + 0: Link/Activity + 1: Link1000/Activity + 2: Link100/Activity + 3: Link10/Activity + 4: Link100/1000/Activity + 5: Link10/1000/Activity + 6: Link10/100/Activity + 7: RESERVED + 8: Duplex/Collision + 9: Collision + 10: Activity + 11: RESERVED + 12: Auto-negotiation Fault + 13: RESERVED + 14: Off + 15: On +- microchip,tx-lpi-timer: the delay (in microseconds) between the TX fifo + becoming empty and invoking Low Power Idles (default 600). + +Example: + + /* Standard configuration for a Raspberry Pi 3 B+ */ + ethernet: usbether@1 { + compatible = "usb424,7800"; + reg = <1>; + microchip,eee-enabled; + microchip,tx-lpi-timer = <600>; + /* +* led0 = 1:link1000/activity +* led1 = 6:link10/100/activity +*/ + microchip,led-modes = <1 6>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 2328eed..b637aad 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14482,6 +14482,7 @@ M: Microchip Linux Driver Support L: net...@vger.kernel.org S: Maintained F: drivers/net/usb/lan78xx.* +F: Documentation/devicetree/bindings/net/microchip,lan78xx.txt USB MASS STORAGE DRIVER M: Alan Stern -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: typec: ucsi: fix link error on randconfig
On Tue, Apr 10, 2018 at 10:51:13AM +0300, Heikki Krogerus wrote: > If building a kernel without FTRACE but with TRACING, > ucsi.ko fails to link due to missing trace events. Fix this > by using the correct Kconfig symbol on Makefile. > > Reported-by: Tobias Regnery> Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface") > Cc: > Signed-off-by: Heikki Krogerus > --- > drivers/usb/typec/ucsi/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > index b57891c1fd31..7afbea512207 100644 > --- a/drivers/usb/typec/ucsi/Makefile > +++ b/drivers/usb/typec/ucsi/Makefile > @@ -5,6 +5,6 @@ obj-$(CONFIG_TYPEC_UCSI) += typec_ucsi.o > > typec_ucsi-y := ucsi.o > > -typec_ucsi-$(CONFIG_FTRACE) += trace.o > +typec_ucsi-$(CONFIG_TRACING) += trace.o > > obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o Tobias send his own version for this. His patch has a better explanation, so let's use that instead. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: typec: ucsi: fix tracepoint related build error
On Tue, Apr 10, 2018 at 10:38:06AM +0200, Tobias Regnery wrote: > There is the following build error with CONFIG_TYPEC_UCSI=m, CONFIG_FTRACE=y > and CONFIG_TRACING=n: > > ERROR: "__tracepoint_ucsi_command" [drivers/usb/typec/ucsi/typec_ucsi.ko] > undefined! > ERROR: "__tracepoint_ucsi_register_port" > [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! > ERROR: "__tracepoint_ucsi_notify" [drivers/usb/typec/ucsi/typec_ucsi.ko] > undefined! > ERROR: "__tracepoint_ucsi_reset_ppm" [drivers/usb/typec/ucsi/typec_ucsi.ko] > undefined! > ERROR: "__tracepoint_ucsi_run_command" [drivers/usb/typec/ucsi/typec_ucsi.ko] > undefined! > ERROR: "__tracepoint_ucsi_ack" [drivers/usb/typec/ucsi/typec_ucsi.ko] > undefined! > ERROR: "__tracepoint_ucsi_connector_change" > [drivers/usb/typec/ucsi/typec_ucsi.ko] undefined! > > This compination is quite hard to create because CONFIG_TRACING gets selected > only in rare cases without CONFIG_FTRACE. > > The build failure is caused by conditionally compiling trace.c depending on > the wrong option CONFIG_FTRACE. Change this to depend on CONFIG_TRACING like > other users of tracepoints do. > > Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface") > Signed-off-by: Tobias RegneryAcked-by: Heikki Krogerus Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT/PATCH 18/38] usb: dwc3: gadget: check for Missed Isoc from event status
Hi, Thinh Nguyenwrites: > Hi, > > On 4/11/2018 1:21 AM, Felipe Balbi wrote: >> >> Hi, >> >> Felipe Balbi writes: > Without XferNotReady, we won't have a reliable way to know the uFrame > number. Read the Isochronous programming sequence from your databook. Right. We need XferNotReady to know when to start isoc transfer. But if there are still queued requests, DWC3 can just wait to see if any of them will succeed to continue with the transfer just as how DWC3 is handling it now. >>> >>> That's not what the databook says though. And that's also not intention >>> of how the code is written as of now either. The way the code is written >>> is the following: >>> >>> queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() -> >>> queue() -> end_transfer. >>> >>> That's not really waiting for the queue to be consumed, it's just >>> delaying end transfer until we get another queue(). IOW, it just >>> *happens* to give the controller time to go through the list of started >>> requests. >>> If we end and restart the transfer right away, then we may lose more isoc data than necessary (due to isoc scheduling at least 4 uFrame ahead of time). Is there something you see that doesn't work with the current implementation? >>> >>> Not _really_, I'm just trying to make the code easier to read and, I >>> think, I've achieved that. Now, if we need to delay end transfer in the >>> case where we have more requests in the controller's queue, that's easy >>> enough to implement: >>> >>> @@ -2371,7 +2371,8 @@ static void >>> dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, >>> if (event->status & DEPEVT_STATUS_BUSERR) >>> status = -ECONNRESET; >>> >>> - if (event->status & DEPEVT_STATUS_MISSED_ISOC) { >>> + if (event->status & DEPEVT_STATUS_MISSED_ISOC && >>> + list_empty(>started_list) { >>> status = -EXDEV; > > Maybe we should return the -EXDEV status every time there's a missed isoc. you mean like this? @@ -2358,10 +2358,11 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_BUSERR) status = -ECONNRESET; - if (event->status & DEPEVT_STATUS_MISSED_ISOC && - list_empty(>started_list)) { + if (event->status & DEPEVT_STATUS_MISSED_ISOC) { status = -EXDEV; - stop = true; + + if (list_empty(>started_list)) + stop = true; } dwc3_gadget_ep_cleanup_completed_requests(dep, event, status); >>> stop = true; >>> } >>> >>> I'm not sure this is a good idea though. Once we miss an interval, don't >>> we need to know the next frame when transfer needs to be scheduled? >>> >>> Meaning we would need XferNotReady to properly schedule the new >>> transfer? >> >> thinking about this a little more. This extra list_empty() check is not >> wrong at all :-) I've amended this series with the 3 patches below. I'll >> resend the series once I've given more time for people to test. Patches >> have been updated to the branch on kernel.org as well, btw. > > Great! :) > Thanks for all the new updates. I'll test it out when I have a chance. sure, thanks a lot. -- balbi signature.asc Description: PGP signature