mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] usb: ohci-omap: implement overcurrent handling for GPIO-based power control
@ 2026-10-18 22:33 Muhammad Mahdi Amirpour
  2026-10-19  6:46 ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Muhammad Mahdi Amirpour @ 2026-10-18 22:33 UTC (permalink / raw)
  To: gregkh
  Cc: stern, linux-usb, linux-omap, linux-kernel, Muhammad Mahdi Amirpour

The OMAP OHCI driver has had optional GPIO lines for VBUS power control
and overcurrent detection since the gpiolib conversion, but the
overcurrent line was never wired up — it was marked with a TODO comment.

On boards like the OMAP1 OSK, the VBUS power switch (TPS2045) is
controlled via a TPS65010 GPIO, and overcurrent is reported on a
separate GPIO. Without this patch, an overcurrent condition is not
detected by software and VBUS power is not cut, relying solely on the
hardware current limiter.

This patch implements the missing overcurrent handling:
- Requests a threaded IRQ on the overcurrent GPIO (both edges)
- Cuts VBUS power via the power GPIO when overcurrent is detected
- Reports overcurrent status and change through the root hub port
  status callbacks so the USB hub core can react
- Handles SetPortFeature/ClearPortFeature(POWER) to drive the VBUS
  GPIO alongside the OHCI hardware register
- Turns off VBUS on driver removal and probe failure

Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>
---
v4:
  - Remove spinlock entirely; use READ_ONCE/WRITE_ONCE for the
    overcurrent flags (Greg KH).  This also fixes the bug where
    gpiod_set_value_cansleep() was called under spin_lock_irqsave(),
    which sleeps on the I2C-attached TPS65010 (Sashiko).
  - Rename overcurrent_status to overcurrent_active and
    overcurrent_changed to overcurrent_change_pending (Greg KH)
  - Fix include order to be alphabetical (Greg KH)
  - Add comment explaining BIT(1) = port 1 (Greg KH)
  - Add comment explaining overcurrent_irq = -1 sentinel (Greg KH)
  - Add VBUS power-off in err_remove_hcd and ohci_hcd_omap_remove()
    so VBUS is not left enabled after driver unbind (Sashiko)
  - Re-check overcurrent_active after enabling VBUS in
    SetPortFeature(POWER) to close the race window (Sashiko)
  - Rewrite commit message to explain motivation (Greg KH)

v3:
  - Fix power GPIO polarity: 1 = VBUS off, 0 = VBUS on
  - Fix wPortChange bit: shift USB_PORT_STAT_C_OVERCURRENT by 16
  - Add clock and PHY cleanup to probe error path
  - Use IRQF_NO_AUTOEN to close race window during probe
  - Only set overcurrent_changed at probe when overcurrent is active
  - Guard hub_status_data and hub_control behind priv->overcurrent
  - Handle SetPortFeature/ClearPortFeature(POWER) for VBUS GPIO
  - Remove shadowed flags variable in hub_control

v2:
  - Use real name in From: and Signed-off-by: lines

 drivers/usb/host/ohci-omap.c | 282 +++++++++++++++++++++++++++++------
 1 file changed, 237 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
index f6e56c4b9914..c71721243905 100644
--- a/drivers/usb/host/ohci-omap.c
+++ b/drivers/usb/host/ohci-omap.c
@@ -19,20 +19,23 @@
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/irqreturn.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/usb/otg.h>
-#include <linux/platform_device.h>
 #include <linux/platform_data/usb-omap1.h>
-#include <linux/soc/ti/omap1-usb.h>
+#include <linux/platform_device.h>
+#include <linux/signal.h>
+#include <linux/soc/ti/omap1-io.h>
 #include <linux/soc/ti/omap1-mux.h>
 #include <linux/soc/ti/omap1-soc.h>
-#include <linux/soc/ti/omap1-io.h>
-#include <linux/signal.h>
+#include <linux/soc/ti/omap1-usb.h>
 #include <linux/usb.h>
+#include <linux/usb/ch11.h>
 #include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
 
 #include "ohci.h"
 
@@ -46,13 +49,23 @@ struct ohci_omap_priv {
 	struct clk *usb_dc_ck;
 	struct gpio_desc *power;
 	struct gpio_desc *overcurrent;
+	/*
+	 * Accessed with READ_ONCE/WRITE_ONCE from the threaded IRQ
+	 * handler and from hub_control in process context.  No lock
+	 * is needed: the IRQ handler is the sole writer of
+	 * overcurrent_active (IRQF_ONESHOT prevents re-entry), and
+	 * overcurrent_change_pending is a single-byte flag where a
+	 * missed clear just causes one extra poll cycle.
+	 */
+	bool overcurrent_active;
+	bool overcurrent_change_pending;
+	int overcurrent_irq; /* -1 means no IRQ has been allocated yet */
 };
 
 static const char hcd_name[] = "ohci-omap";
 static struct hc_driver __read_mostly ohci_omap_hc_driver;
 
-#define hcd_to_ohci_omap_priv(h) \
-	((struct ohci_omap_priv *)hcd_to_ohci(h)->priv)
+#define hcd_to_ohci_omap_priv(h) ((struct ohci_omap_priv *)hcd_to_ohci(h)->priv)
 
 static void omap_ohci_clock_power(struct ohci_omap_priv *priv, int on)
 {
@@ -70,15 +83,15 @@ static void omap_ohci_clock_power(struct ohci_omap_priv *priv, int on)
 static void start_hnp(struct ohci_hcd *ohci)
 {
 	struct usb_hcd *hcd = ohci_to_hcd(ohci);
-	const unsigned	port = hcd->self.otg_port - 1;
-	unsigned long	flags;
+	const unsigned port = hcd->self.otg_port - 1;
+	unsigned long flags;
 	u32 l;
 
 	otg_start_hnp(hcd->usb_phy->otg);
 
 	local_irq_save(flags);
 	hcd->usb_phy->otg->state = OTG_STATE_A_SUSPEND;
-	writel (RH_PS_PSS, &ohci->regs->roothub.portstatus [port]);
+	writel(RH_PS_PSS, &ohci->regs->roothub.portstatus[port]);
 	l = omap_readl(OTG_CTRL);
 	l &= ~OTG_A_BUSREQ;
 	omap_writel(l, OTG_CTRL);
@@ -89,11 +102,11 @@ static void start_hnp(struct ohci_hcd *ohci)
 
 static int ohci_omap_reset(struct usb_hcd *hcd)
 {
-	struct ohci_hcd		*ohci = hcd_to_ohci(hcd);
-	struct omap_usb_config	*config = dev_get_platdata(hcd->self.controller);
-	struct ohci_omap_priv	*priv = hcd_to_ohci_omap_priv(hcd);
-	int			need_transceiver = (config->otg != 0);
-	int			ret;
+	struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+	struct omap_usb_config *config = dev_get_platdata(hcd->self.controller);
+	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
+	int need_transceiver = (config->otg != 0);
+	int ret;
 
 	dev_dbg(hcd->self.controller, "starting USB Controller\n");
 
@@ -110,10 +123,11 @@ static int ohci_omap_reset(struct usb_hcd *hcd)
 	if (IS_ENABLED(CONFIG_USB_OTG) && need_transceiver) {
 		hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB2);
 		if (!IS_ERR_OR_NULL(hcd->usb_phy)) {
-			int	status = otg_set_host(hcd->usb_phy->otg,
-						&ohci_to_hcd(ohci)->self);
-			dev_dbg(hcd->self.controller, "init %s phy, status %d\n",
-					hcd->usb_phy->label, status);
+			int status = otg_set_host(hcd->usb_phy->otg,
+						  &ohci_to_hcd(ohci)->self);
+			dev_dbg(hcd->self.controller,
+				"init %s phy, status %d\n", hcd->usb_phy->label,
+				status);
 			if (status) {
 				usb_put_phy(hcd->usb_phy);
 				return status;
@@ -141,7 +155,7 @@ static int ohci_omap_reset(struct usb_hcd *hcd)
 
 	/* board-specific power switching and overcurrent support */
 	if (machine_is_omap_osk()) {
-		u32	rh = roothub_a (ohci);
+		u32 rh = roothub_a(ohci);
 
 		/* power switching (ganged by default) */
 		rh &= ~RH_A_NPS;
@@ -181,6 +195,128 @@ static int ohci_omap_reset(struct usb_hcd *hcd)
 	return 0;
 }
 
+static irqreturn_t ohci_omap_overcurrent_irq(int irq, void *data)
+{
+	struct usb_hcd *hcd = data;
+	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
+	int status;
+	bool was_active;
+
+	status = gpiod_get_value_cansleep(priv->overcurrent);
+	if (status < 0)
+		return IRQ_HANDLED;
+
+	if (status && priv->power)
+		gpiod_set_value_cansleep(priv->power, 1);
+
+	was_active = READ_ONCE(priv->overcurrent_active);
+	WRITE_ONCE(priv->overcurrent_active, !!status);
+	if (was_active != !!status)
+		WRITE_ONCE(priv->overcurrent_change_pending, true);
+
+	usb_hcd_poll_rh_status(hcd);
+
+	return IRQ_HANDLED;
+}
+
+static int ohci_omap_hub_status_data(struct usb_hcd *hcd, char *buf)
+{
+	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
+	int length;
+
+	length = ohci_hub_status_data(hcd, buf);
+
+	if (!priv->overcurrent)
+		return length;
+
+	if (READ_ONCE(priv->overcurrent_change_pending)) {
+		if (!length) {
+			buf[0] = 0;
+			length = 1;
+		}
+		buf[0] |= BIT(1); /* port 1 */
+	}
+
+	return length;
+}
+
+static int ohci_omap_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
+				 u16 wIndex, char *buf, u16 wLength)
+{
+	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
+	int ret;
+
+	/*
+	 * Drive the VBUS power GPIO alongside the OHCI hardware register.
+	 * On boards like the OSK, the OHCI PPS bit does not control the
+	 * real power switch — the GPIO does.
+	 */
+	if (priv->power && wIndex == 1) {
+		if (typeReq == SetPortFeature &&
+		    wValue == USB_PORT_FEAT_POWER) {
+			ret = ohci_hub_control(hcd, typeReq, wValue, wIndex,
+					       buf, wLength);
+			if (!ret && !READ_ONCE(priv->overcurrent_active))
+				gpiod_set_value_cansleep(priv->power, 0);
+			/*
+			 * Re-check: the overcurrent IRQ may have fired
+			 * between the check above and the GPIO call.
+			 * A mutex would close this fully, but the window
+			 * is microseconds and the hardware current limiter
+			 * provides the final safety net.
+			 */
+			if (!ret && READ_ONCE(priv->overcurrent_active))
+				gpiod_set_value_cansleep(priv->power, 1);
+			return ret;
+		}
+		if (typeReq == ClearPortFeature &&
+		    wValue == USB_PORT_FEAT_POWER) {
+			ret = ohci_hub_control(hcd, typeReq, wValue, wIndex,
+					       buf, wLength);
+			if (!ret)
+				gpiod_set_value_cansleep(priv->power, 1);
+			return ret;
+		}
+	}
+
+	ret = ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+	if (ret)
+		return ret;
+
+	if (!priv->overcurrent)
+		return ret;
+
+	if (typeReq == ClearPortFeature && wIndex == 1 &&
+	    wValue == USB_PORT_FEAT_C_OVER_CURRENT) {
+		WRITE_ONCE(priv->overcurrent_change_pending, false);
+		return 0;
+	}
+
+	if (typeReq != GetPortStatus || wIndex != 1)
+		return ret;
+
+	{
+		bool active = READ_ONCE(priv->overcurrent_active);
+		bool changed = READ_ONCE(priv->overcurrent_change_pending);
+		__le32 *portstatus = (__le32 *)buf;
+		u32 value = le32_to_cpu(*portstatus);
+
+		if (active)
+			value |= USB_PORT_STAT_OVERCURRENT;
+		else
+			value &= ~USB_PORT_STAT_OVERCURRENT;
+
+		if (changed)
+			value |= (USB_PORT_STAT_C_OVERCURRENT << 16);
+		else
+			value &= ~(USB_PORT_STAT_C_OVERCURRENT << 16);
+
+		*portstatus = cpu_to_le32(value);
+	}
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 /**
@@ -201,18 +337,18 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
 
 	if (pdev->num_resources != 2) {
 		dev_err(&pdev->dev, "invalid num_resources: %i\n",
-		       pdev->num_resources);
+			pdev->num_resources);
 		return -ENODEV;
 	}
 
-	if (pdev->resource[0].flags != IORESOURCE_MEM
-			|| pdev->resource[1].flags != IORESOURCE_IRQ) {
+	if (pdev->resource[0].flags != IORESOURCE_MEM ||
+	    pdev->resource[1].flags != IORESOURCE_IRQ) {
 		dev_err(&pdev->dev, "invalid resource type\n");
 		return -ENODEV;
 	}
 
 	hcd = usb_create_hcd(&ohci_omap_hc_driver, &pdev->dev,
-			dev_name(&pdev->dev));
+			     dev_name(&pdev->dev));
 	if (!hcd)
 		return -ENOMEM;
 
@@ -221,6 +357,10 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
 	priv = hcd_to_ohci_omap_priv(hcd);
 
 	/* Obtain two optional GPIO lines */
+	WRITE_ONCE(priv->overcurrent_active, false);
+	WRITE_ONCE(priv->overcurrent_change_pending, false);
+	priv->overcurrent_irq = -1;
+
 	priv->power = devm_gpiod_get_optional(&pdev->dev, "power", GPIOD_ASIS);
 	if (IS_ERR(priv->power)) {
 		retval = PTR_ERR(priv->power);
@@ -229,13 +369,8 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
 	if (priv->power)
 		gpiod_set_consumer_name(priv->power, "OHCI power");
 
-	/*
-	 * This "overcurrent" GPIO line isn't really used in the code,
-	 * but has a designated hardware function.
-	 * TODO: implement proper overcurrent handling.
-	 */
-	priv->overcurrent = devm_gpiod_get_optional(&pdev->dev, "overcurrent",
-						    GPIOD_IN);
+	priv->overcurrent =
+		devm_gpiod_get_optional(&pdev->dev, "overcurrent", GPIOD_IN);
 	if (IS_ERR(priv->overcurrent)) {
 		retval = PTR_ERR(priv->overcurrent);
 		goto err_put_hcd;
@@ -277,23 +412,73 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
 	if (!hcd->regs) {
 		dev_err(&pdev->dev, "can't ioremap OHCI HCD\n");
 		retval = -ENOMEM;
-		goto err2;
+		goto err_release_mem;
 	}
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		retval = irq;
-		goto err3;
+		goto err_iounmap;
 	}
 	retval = usb_add_hcd(hcd, irq, 0);
 	if (retval)
-		goto err3;
+		goto err_iounmap;
+
+	if (priv->overcurrent) {
+		int status;
+
+		priv->overcurrent_irq = gpiod_to_irq(priv->overcurrent);
+		if (priv->overcurrent_irq < 0) {
+			retval = priv->overcurrent_irq;
+			goto err_remove_hcd;
+		}
+
+		retval = request_threaded_irq(
+			priv->overcurrent_irq, NULL, ohci_omap_overcurrent_irq,
+			IRQF_ONESHOT | IRQF_NO_AUTOEN | IRQF_TRIGGER_RISING |
+				IRQF_TRIGGER_FALLING,
+			"ohci-omap-overcurrent", hcd);
+		if (retval) {
+			priv->overcurrent_irq = -1;
+			goto err_remove_hcd;
+		}
+
+		status = gpiod_get_value_cansleep(priv->overcurrent);
+		if (status < 0) {
+			retval = status;
+			goto err_remove_hcd;
+		}
+
+		WRITE_ONCE(priv->overcurrent_active, !!status);
+		WRITE_ONCE(priv->overcurrent_change_pending, !!status);
+
+		if (status && priv->power)
+			gpiod_set_value_cansleep(priv->power, 1);
+
+		enable_irq(priv->overcurrent_irq);
+		if (status)
+			usb_hcd_poll_rh_status(hcd);
+	}
 
 	device_wakeup_enable(hcd->self.controller);
 	return 0;
-err3:
+
+err_remove_hcd:
+	if (priv->overcurrent_irq >= 0) {
+		free_irq(priv->overcurrent_irq, hcd);
+		priv->overcurrent_irq = -1;
+	}
+	usb_remove_hcd(hcd);
+	omap_ohci_clock_power(priv, 0);
+	if (!IS_ERR_OR_NULL(hcd->usb_phy)) {
+		(void)otg_set_host(hcd->usb_phy->otg, NULL);
+		usb_put_phy(hcd->usb_phy);
+	}
+	if (priv->power)
+		gpiod_set_value_cansleep(priv->power, 1);
+err_iounmap:
 	iounmap(hcd->regs);
-err2:
+err_release_mem:
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err_unprepare_dc_ck:
 	clk_unprepare(priv->usb_dc_ck);
@@ -308,7 +493,6 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
 	return retval;
 }
 
-
 /* may be called with controller, bus, and devices active */
 
 /**
@@ -323,16 +507,22 @@ static int ohci_hcd_omap_probe(struct platform_device *pdev)
  */
 static void ohci_hcd_omap_remove(struct platform_device *pdev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(pdev);
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
 
 	dev_dbg(hcd->self.controller, "stopping USB Controller\n");
+	if (priv->overcurrent_irq >= 0) {
+		free_irq(priv->overcurrent_irq, hcd);
+		priv->overcurrent_irq = -1;
+	}
 	usb_remove_hcd(hcd);
 	omap_ohci_clock_power(priv, 0);
 	if (!IS_ERR_OR_NULL(hcd->usb_phy)) {
-		(void) otg_set_host(hcd->usb_phy->otg, 0);
+		(void)otg_set_host(hcd->usb_phy->otg, 0);
 		usb_put_phy(hcd->usb_phy);
 	}
+	if (priv->power)
+		gpiod_set_value_cansleep(priv->power, 1);
 	iounmap(hcd->regs);
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 	clk_unprepare(priv->usb_dc_ck);
@@ -344,7 +534,7 @@ static void ohci_hcd_omap_remove(struct platform_device *pdev)
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef	CONFIG_PM
+#ifdef CONFIG_PM
 
 static int ohci_omap_suspend(struct platform_device *pdev, pm_message_t message)
 {
@@ -368,8 +558,8 @@ static int ohci_omap_suspend(struct platform_device *pdev, pm_message_t message)
 
 static int ohci_omap_resume(struct platform_device *dev)
 {
-	struct usb_hcd	*hcd = platform_get_drvdata(dev);
-	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	struct usb_hcd *hcd = platform_get_drvdata(dev);
+	struct ohci_hcd *ohci = hcd_to_ohci(hcd);
 	struct ohci_omap_priv *priv = hcd_to_ohci_omap_priv(hcd);
 
 	if (time_before(jiffies, ohci->next_statechange))
@@ -392,7 +582,7 @@ static struct platform_driver ohci_hcd_omap_driver = {
 	.probe		= ohci_hcd_omap_probe,
 	.remove		= ohci_hcd_omap_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
-#ifdef	CONFIG_PM
+#ifdef CONFIG_PM
 	.suspend	= ohci_omap_suspend,
 	.resume		= ohci_omap_resume,
 #endif
@@ -402,8 +592,8 @@ static struct platform_driver ohci_hcd_omap_driver = {
 };
 
 static const struct ohci_driver_overrides omap_overrides __initconst = {
-	.product_desc	= "OMAP OHCI",
-	.reset		= ohci_omap_reset,
+	.product_desc = "OMAP OHCI",
+	.reset = ohci_omap_reset,
 	.extra_priv_size = sizeof(struct ohci_omap_priv),
 };
 
@@ -413,6 +603,8 @@ static int __init ohci_omap_init(void)
 		return -ENODEV;
 
 	ohci_init_driver(&ohci_omap_hc_driver, &omap_overrides);
+	ohci_omap_hc_driver.hub_status_data = ohci_omap_hub_status_data;
+	ohci_omap_hc_driver.hub_control = ohci_omap_hub_control;
 	return platform_driver_register(&ohci_hcd_omap_driver);
 }
 module_init(ohci_omap_init);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] usb: ohci-omap: implement overcurrent handling for GPIO-based power control
  2026-10-18 22:33 [PATCH v4] usb: ohci-omap: implement overcurrent handling for GPIO-based power control Muhammad Mahdi Amirpour
@ 2026-10-19  6:46 ` Greg KH
  0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2026-10-19  6:46 UTC (permalink / raw)
  To: Muhammad Mahdi Amirpour; +Cc: stern, linux-usb, linux-omap, linux-kernel

On Mon, Oct 19, 2026 at 02:03:12AM +0330, Muhammad Mahdi Amirpour wrote:
> The OMAP OHCI driver has had optional GPIO lines for VBUS power control
> and overcurrent detection since the gpiolib conversion, but the
> overcurrent line was never wired up — it was marked with a TODO comment.
> 
> On boards like the OMAP1 OSK, the VBUS power switch (TPS2045) is
> controlled via a TPS65010 GPIO, and overcurrent is reported on a
> separate GPIO. Without this patch, an overcurrent condition is not
> detected by software and VBUS power is not cut, relying solely on the
> hardware current limiter.
> 
> This patch implements the missing overcurrent handling:
> - Requests a threaded IRQ on the overcurrent GPIO (both edges)
> - Cuts VBUS power via the power GPIO when overcurrent is detected
> - Reports overcurrent status and change through the root hub port
>   status callbacks so the USB hub core can react
> - Handles SetPortFeature/ClearPortFeature(POWER) to drive the VBUS
>   GPIO alongside the OHCI hardware register
> - Turns off VBUS on driver removal and probe failure
> 
> Signed-off-by: Muhammad Mahdi Amirpour <ampr.mhd@gmail.com>
> ---
> v4:
>   - Remove spinlock entirely; use READ_ONCE/WRITE_ONCE for the
>     overcurrent flags (Greg KH).  This also fixes the bug where
>     gpiod_set_value_cansleep() was called under spin_lock_irqsave(),
>     which sleeps on the I2C-attached TPS65010 (Sashiko).

Are you sure this is correct?  You never responded to my previous review
comments here, just submitted a new version which is a bit odd...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] usb: ohci-omap: implement overcurrent handling for GPIO-based power control
@ 2026-10-19 16:15 Muhammad Mahdi Amirpour
  0 siblings, 0 replies; 3+ messages in thread
From: Muhammad Mahdi Amirpour @ 2026-10-19 16:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alan Stern, linux-usb, linux-omap, linux-kernel

[resending to the full list as requested]

Dear Mr. Kroah-Hartman,

You are right, I apologize. I should have replied to your v3 review
before sending v4. I was not familiar with the mailing list workflow
and will make sure to reply to review comments before sending new
versions in the future.

To address your v3 review points:

1. Lock: You were right that the spinlock was unnecessary. It was
   actually harmful — gpiod_set_value_cansleep() sleeps during the
   I2C transfer to the TPS65010, so calling it under
   spin_lock_irqsave() was a bug. v4 removes the lock entirely and
   uses READ_ONCE/WRITE_ONCE instead.

2. Naming: Renamed overcurrent_status to overcurrent_active and
   overcurrent_changed to overcurrent_change_pending.

3. Include order: Fixed to be alphabetical.

4. Regarding the code origin: I did use an AI assistant to help me
   understand some of the kernel APIs (especially the OHCI hub status
   register layout and the gpiod subsystem) and to review my earlier
   versions for bugs. The code is mine, but I want to be transparent
   about the assistance.

I do not have the hardware to test this patch, so I cannot verify it
at runtime. Given that this is a legacy driver, I am happy to drop
this patch and work on something else if you think that would be
better. Otherwise, I can send a v5 addressing the Sashiko bot findings
(the IRQ handler ordering and the probe falling-edge race).

Please let me know how you would like to proceed.

Best regards,
Muhammad Mahdi Amirpour

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-19 16:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-18 22:33 [PATCH v4] usb: ohci-omap: implement overcurrent handling for GPIO-based power control Muhammad Mahdi Amirpour
2026-10-19  6:46 ` Greg KH
2026-10-19 16:15 Muhammad Mahdi Amirpour

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®