USB Hid Timeout and r8169 module to trust HW if present.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I got this timeout patch and patch for r8168 to work, both are situations that i've had, and fixes work for me ok.


Where do I send them ?


--Lauri


On 4.2.2020 9.18, Dan Carpenter wrote:
On Tue, Feb 04, 2020 at 09:00:46AM +0200, Lauri Jakku wrote:
Hi,

I'm quite newbie publicly patching stuff, but at anyway:

On 4.2.2020 8.27, Dan Carpenter wrote:
Hi Lauri,

There are a bunch of issues with this patch.

1)  There is no way we're going to add a retry loop to
     usb_control_msg().  We might add it to a driver, but probably we
     would suspect that the hardware is bad.  Maybe the cable is faulty?
     Or I could be wrong on that...
For example there is lot of USB HID related oddities reported

by multiple users, that have said that after (kernel) update

of their system USB devices stopped working or work randomly.


I have problem with my mouse, and the problem went away

with my patch. I think that the change to driver by driver is

just code duplication, and with my patch the 'good' HW still

is not going to sleep even that 20ms, the time that I tought

to be good. Max retry time is 400ms.

That doesn't sound totally unreasonable.  Go ahead and try sending the
patch.  Maybe they apply it or maybe they don't but either way there
is no harm in sending a patch.

regards,
dan carpenter

>From 8e9898889c035e6301c7e92b1a63717d441eded6 Mon Sep 17 00:00:00 2001
From: Lauri Jakku <lja@xxxxxx>
Date: Mon, 9 Aug 2021 21:44:53 +0300
Subject: [PATCH 1/2] net:realtek:r8169, driver load fix

   net:realtek:r8169

     Problem:

       The driver seems not to work at first run, not even when proper HW
       is detected by it at the first probe.

     Solution:

       the driver will trust the HW that reports valid ID and then make
       re-loading of the module as it would being reloaded manually

Signed-off-by: Lauri Jakku <lja@xxxxxx>
---
 .../drivers/net/ethernet/realtek/r8169_main.c | 35 ++++++++++++++---
 linux-5.14-rc4/drivers/net/phy/phy_device.c   | 38 +++++++++++++++++++
 2 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/linux-5.14-rc4/drivers/net/ethernet/realtek/r8169_main.c b/linux-5.14-rc4/drivers/net/ethernet/realtek/r8169_main.c
index c7af5bc3b..d8e602527 100644
--- a/linux-5.14-rc4/drivers/net/ethernet/realtek/r8169_main.c
+++ b/linux-5.14-rc4/drivers/net/ethernet/realtek/r8169_main.c
@@ -634,6 +634,8 @@ struct rtl8169_private {
 	struct rtl_fw *rtl_fw;
 
 	u32 ocp_base;
+
+	int retry_probeing;
 };
 
 typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -5097,13 +5099,16 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 	tp->phydev = mdiobus_get_phy(new_bus, 0);
 	if (!tp->phydev) {
 		return -ENODEV;
-	} else if (!tp->phydev->drv) {
-		/* Most chip versions fail with the genphy driver.
-		 * Therefore ensure that the dedicated PHY driver is loaded.
+	} else if (tp->phydev->phy_id != RTL_GIGA_MAC_NONE) {
+		/* Most chip versions fail with the genphy driver, BUT do rerport valid IW
+		 * ID. Re-starting the module seem to fix the issue of non-functional driver.
 		 */
-		dev_err(&pdev->dev, "no dedicated PHY driver found for PHY ID 0x%08x, maybe realtek.ko needs to be added to initramfs?\n",
+		dev_err(&pdev->dev,
+			"no dedicated driver, but HW found: PHY PHY ID 0x%08x\n",
 			tp->phydev->phy_id);
-		return -EUNATCH;
+
+		dev_err(&pdev->dev, "trying re-probe few times..\n");
+
 	}
 
 	tp->phydev->mac_managed_pm = 1;
@@ -5250,6 +5255,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	enum mac_version chipset;
 	struct net_device *dev;
 	u16 xid;
+	int savederr = 0;
 
 	dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
 	if (!dev)
@@ -5261,6 +5267,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->dev = dev;
 	tp->pci_dev = pdev;
 	tp->supports_gmii = ent->driver_data == RTL_CFG_NO_GBIT ? 0 : 1;
+	tp->retry_probeing = 0;
 	tp->eee_adv = -1;
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
@@ -5410,7 +5417,15 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, tp);
 
-	rc = r8169_mdio_register(tp);
+	savederr = r8169_mdio_register(tp);
+
+	if (
+		(tp->retry_probeing > 0) &&
+		(savederr == -EAGAIN)
+	   ) {
+		netdev_info(dev, " retry of probe requested..............");
+	}
+
 	if (rc)
 		return rc;
 
@@ -5435,6 +5450,14 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_put_sync(&pdev->dev);
 
+	if (
+		(tp->retry_probeing > 0) &&
+		(savederr == -EAGAIN)
+	   ) {
+		netdev_info(dev, " retry of probe requested..............");
+		return savederr;
+	}
+
 	return 0;
 }
 
diff --git a/linux-5.14-rc4/drivers/net/phy/phy_device.c b/linux-5.14-rc4/drivers/net/phy/phy_device.c
index 5d5f9a9ee..59c6ac031 100644
--- a/linux-5.14-rc4/drivers/net/phy/phy_device.c
+++ b/linux-5.14-rc4/drivers/net/phy/phy_device.c
@@ -2980,6 +2980,9 @@ struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(fwnode_get_phy_node);
 
+
+static int phy_remove(struct device *dev);
+
 /**
  * phy_probe - probe and init a PHY device
  * @dev: device to probe and init
@@ -2988,13 +2991,22 @@ EXPORT_SYMBOL_GPL(fwnode_get_phy_node);
  *   set the state to READY (the driver's init function should
  *   set it to STARTING if needed).
  */
+#define REDO_PROBE_TIMES	5
 static int phy_probe(struct device *dev)
 {
 	struct phy_device *phydev = to_phy_device(dev);
 	struct device_driver *drv = phydev->mdio.dev.driver;
 	struct phy_driver *phydrv = to_phy_driver(drv);
+	int again = 0;
+	int savederr = 0;
+again_retry:
 	int err = 0;
 
+	if (again > 0) {
+		pr_err("%s: Re-probe %d of driver.....\n",
+		       phydrv->name, again);
+	}
+
 	phydev->drv = phydrv;
 
 	/* Disable the interrupt if the PHY doesn't support it
@@ -3013,6 +3025,17 @@ static int phy_probe(struct device *dev)
 
 	if (phydev->drv->probe) {
 		err = phydev->drv->probe(phydev);
+
+		/* If again requested. */
+		if (err == -EAGAIN) {
+			again++;
+			savederr = err;
+			err = 0;
+
+			pr_info("%s: EAGAIN: %d ...\n",
+				phydrv->name, again);
+		}
+
 		if (err)
 			goto out;
 	}
@@ -3081,6 +3104,20 @@ static int phy_probe(struct device *dev)
 
 	mutex_unlock(&phydev->lock);
 
+	if ((savederr == -EAGAIN) &&
+	    ((again > 0) && (again < REDO_PROBE_TIMES))
+	   ) {
+		pr_err("%s: Retry removal driver..\n",
+			phydrv->name);
+
+		phy_remove(dev);
+
+		pr_err("%s: Re-probe driver..\n",
+			phydrv->name);
+		savederr = 0;
+		goto again_retry;
+	}
+
 	return err;
 }
 
@@ -3108,6 +3145,7 @@ static int phy_remove(struct device *dev)
 	return 0;
 }
 
+
 static void phy_shutdown(struct device *dev)
 {
 	struct phy_device *phydev = to_phy_device(dev);
-- 
2.17.1

>From b9838664ed2ac62eb2a957f477bf57d79f09da73 Mon Sep 17 00:00:00 2001
From: Lauri Jakku <lja@xxxxxx>
Date: Mon, 9 Aug 2021 21:44:53 +0300
Subject: [PATCH 2/2] USB core tinkering:

   USB:core:message:

     Problem:

       Timeout happening in USB transfer some times.

     Solution:

       Linger a bit, prior to the next control message or if return
       value is timeout, but do try few times before quitting.

Signed-off-by: Lauri Jakku <lja@xxxxxx>
---
 linux-5.14-rc4/drivers/usb/core/message.c | 28 +++++++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/linux-5.14-rc4/drivers/usb/core/message.c b/linux-5.14-rc4/drivers/usb/core/message.c
index 4d59d927a..ba95e5995 100644
--- a/linux-5.14-rc4/drivers/usb/core/message.c
+++ b/linux-5.14-rc4/drivers/usb/core/message.c
@@ -21,6 +21,7 @@
 #include <linux/usb/hcd.h>	/* for usbcore internals */
 #include <linux/usb/of.h>
 #include <asm/byteorder.h>
+#include <uapi/asm-generic/errno.h>
 
 #include "usb.h"
 
@@ -138,7 +139,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request,
 		    __u16 size, int timeout)
 {
 	struct usb_ctrlrequest *dr;
-	int ret;
+	int ret = -ETIMEDOUT;
+	int retry_cnt = 5;
+
+	/* By default: while done at least one time */
+	int retryStill = 1;
+	int isTimeout = 1;
 
 	dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_NOIO);
 	if (!dr)
@@ -150,11 +156,23 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request,
 	dr->wIndex = cpu_to_le16(index);
 	dr->wLength = cpu_to_le16(size);
 
-	ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
 
-	/* Linger a bit, prior to the next control message. */
-	if (dev->quirks & USB_QUIRK_DELAY_CTRL_MSG)
-		msleep(200);
+	while (retryStill && isTimeout) {
+		ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
+
+		/*
+		 * Linger a bit, prior to the next control message or if return
+		 * value is timeout, but do try few times before quitting.
+		 */
+
+		retryStill = (retry_cnt > 0);
+		isTimeout = (ret == -ETIMEDOUT);
+
+		if ((dev->quirks & USB_QUIRK_DELAY_CTRL_MSG) || isTimeout)
+			msleep(200);
+
+		retry_cnt--
+	}
 
 	kfree(dr);
 
-- 
2.17.1


[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux