linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator
@ 2019-04-11  9:30 Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 1/6] ARM: davinci: add missing sentinels to GPIO lookup tables Bartosz Golaszewski
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Historically the power supply management in this driver has been handled
in two separate places in parallel. Device-tree users simply defined an
appropriate regulator, while two boards with no DT support (da830-evm and
omapl138-hawk) passed functions defined in their respective board files
over platform data. These functions simply used legacy GPIO calls to
watch the oc GPIO for interrupts and disable the vbus GPIO when the irq
fires.

Commit d193abf1c913 ("usb: ohci-da8xx: add vbus and overcurrent gpios")
updated these GPIO calls to the modern API and moved them inside the
driver.

This however is not the optimal solution for the vbus GPIO as it
duplicates code. Instead we should model the GPIO as a fixed regulator
that can be controlled with a GPIO.

This series adds fixed regulators for all users of vbus GPIO, adds
overcurrent protection using the existing vbus regulator in the USB
driver and removes the vbus GPIO calls once they're no longer used.

Tested on da830-evm with the module both built-in and loadable.

v1 -> v2:
- add patch 1/6 that fixes an existing problem with missing array
  sentinels in GPIO lookups
- add patch 2/6 that removes an unnecessary use count variable from the
  driver
- reorder the changes: add support for overcurrent protection using the
  existing vbus regulator first, then setup fixed regulators in board
  files and finally remove the vbus GPIO from the ohci driver

v2 -> v3:
- change the device id of the fixed regulator to 0 since it's the first
  one
- drop the interrupt-context handler and only use the thread
- drop Alan's Ack from patch 3/6 as it's changed significantly
- rebase on top of v5.1-rc4

v3 -> v4:
- in patch 2/6: switch to using a simple if/else

Bartosz Golaszewski (6):
  ARM: davinci: add missing sentinels to GPIO lookup tables
  usb: ohci-da8xx: let the regulator framework keep track of use count
  usb: ohci-da8xx: disable the regulator if the overcurrent irq fired
  ARM: davinci: omapl138-hawk: add a fixed regulator for ohci-da8xx
  ARM: davinci: da830-evm: add a fixed regulator for ohci-da8xx
  usb: ohci-da8xx: drop the vbus GPIO

 arch/arm/mach-davinci/board-da830-evm.c     | 51 +++++++++++++++++++--
 arch/arm/mach-davinci/board-da850-evm.c     |  1 +
 arch/arm/mach-davinci/board-dm355-evm.c     |  1 +
 arch/arm/mach-davinci/board-dm644x-evm.c    |  1 +
 arch/arm/mach-davinci/board-omapl138-hawk.c | 50 ++++++++++++++++++--
 drivers/usb/host/ohci-da8xx.c               | 46 ++++++++-----------
 6 files changed, 116 insertions(+), 34 deletions(-)

-- 
2.21.0


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

* [PATCH v4 1/6] ARM: davinci: add missing sentinels to GPIO lookup tables
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 2/6] usb: ohci-da8xx: let the regulator framework keep track of use count Bartosz Golaszewski
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Some GPIO lookup tables defined in davinci board files are missing
array sentinels. If an entry for given device cannot be found, this
will cause a kernel panic.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/board-da830-evm.c     | 2 ++
 arch/arm/mach-davinci/board-da850-evm.c     | 1 +
 arch/arm/mach-davinci/board-dm355-evm.c     | 1 +
 arch/arm/mach-davinci/board-dm644x-evm.c    | 1 +
 arch/arm/mach-davinci/board-omapl138-hawk.c | 1 +
 5 files changed, 6 insertions(+)

diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index ff097ecfa451..e3d74e43c47d 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -58,6 +58,7 @@ static struct gpiod_lookup_table da830_evm_usb_gpio_lookup = {
 	.table = {
 		GPIO_LOOKUP("davinci_gpio", ON_BD_USB_DRV, "vbus", 0),
 		GPIO_LOOKUP("davinci_gpio", ON_BD_USB_OVC, "oc", 0),
+		{ }
 	},
 };
 
@@ -156,6 +157,7 @@ static struct gpiod_lookup_table mmc_gpios_table = {
 			    GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP("davinci_gpio", DA830_MMCSD_WP_PIN, "wp",
 			    GPIO_ACTIVE_LOW),
+		{ }
 	},
 };
 
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 1fdc9283a8c5..4ee65a8a3b80 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -784,6 +784,7 @@ static struct gpiod_lookup_table mmc_gpios_table = {
 			    GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP("davinci_gpio", DA850_MMCSD_WP_PIN, "wp",
 			    GPIO_ACTIVE_HIGH),
+		{ }
 	},
 };
 
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c b/arch/arm/mach-davinci/board-dm355-evm.c
index 64d81fc86f14..5113273fda69 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -121,6 +121,7 @@ static struct gpiod_lookup_table i2c_recovery_gpiod_table = {
 			    GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
 		GPIO_LOOKUP("davinci_gpio", DM355_I2C_SCL_PIN, "scl",
 			    GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+		{ }
 	},
 };
 
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
index de15f782816e..9d87d4e440ea 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -663,6 +663,7 @@ static struct gpiod_lookup_table i2c_recovery_gpiod_table = {
 			    GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
 		GPIO_LOOKUP("davinci_gpio", DM644X_I2C_SCL_PIN, "scl",
 			    GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
+		{ }
 	},
 };
 
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 0896af2bed24..e244c8648594 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -303,6 +303,7 @@ static struct gpiod_lookup_table hawk_usb_gpio_lookup = {
 	.table = {
 		GPIO_LOOKUP("davinci_gpio", DA850_USB1_VBUS_PIN, "vbus", 0),
 		GPIO_LOOKUP("davinci_gpio", DA850_USB1_OC_PIN, "oc", 0),
+		{ }
 	},
 };
 
-- 
2.21.0


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

* [PATCH v4 2/6] usb: ohci-da8xx: let the regulator framework keep track of use count
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 1/6] ARM: davinci: add missing sentinels to GPIO lookup tables Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired Bartosz Golaszewski
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

There's no reason to have a separate variable to keep track of the
regulator state. The regulator core already does that. Remove
reg_enabled from struct da8xx_ohci_hcd.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/host/ohci-da8xx.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index ca8a94f15ac0..209a262b5565 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -40,7 +40,6 @@ struct da8xx_ohci_hcd {
 	struct phy *usb11_phy;
 	struct regulator *vbus_reg;
 	struct notifier_block nb;
-	unsigned int reg_enabled;
 	struct gpio_desc *vbus_gpio;
 	struct gpio_desc *oc_gpio;
 };
@@ -100,21 +99,18 @@ static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
 	if (!da8xx_ohci->vbus_reg)
 		return 0;
 
-	if (on && !da8xx_ohci->reg_enabled) {
+	if (on) {
 		ret = regulator_enable(da8xx_ohci->vbus_reg);
 		if (ret) {
 			dev_err(dev, "Failed to enable regulator: %d\n", ret);
 			return ret;
 		}
-		da8xx_ohci->reg_enabled = 1;
-
-	} else if (!on && da8xx_ohci->reg_enabled) {
+	} else {
 		ret = regulator_disable(da8xx_ohci->vbus_reg);
 		if (ret) {
 			dev_err(dev, "Failed  to disable regulator: %d\n", ret);
 			return ret;
 		}
-		da8xx_ohci->reg_enabled = 0;
 	}
 
 	return 0;
-- 
2.21.0


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

* [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 1/6] ARM: davinci: add missing sentinels to GPIO lookup tables Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 2/6] usb: ohci-da8xx: let the regulator framework keep track of use count Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-12 10:39   ` Sekhar Nori
  2019-04-11  9:30 ` [PATCH v4 4/6] ARM: davinci: omapl138-hawk: add a fixed regulator for ohci-da8xx Bartosz Golaszewski
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Historically the power supply management in this driver has been handled
in two separate places in parallel. Device-tree users simply defined an
appropriate regulator, while two boards with no DT support (da830-evm and
omapl138-hawk) passed functions defined in their respective board files
over platform data. These functions simply used legacy GPIO calls to
watch the oc GPIO for interrupts and disable the vbus GPIO when the irq
fires.

Commit d193abf1c913 ("usb: ohci-da8xx: add vbus and overcurrent gpios")
updated these GPIO calls to the modern API and moved them inside the
driver.

This however is not the optimal solution for the vbus GPIO which should
be modeled as a fixed regulator that can be controlled with a GPIO.

In order to keep the overcurrent protection available once we move the
board files to using fixed regulators we need to disable the enable_reg
regulator when the overcurrent indicator interrupt fires. Since we
cannot call regulator_disable() from interrupt context, we need to
switch to using a oneshot threaded interrupt.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/usb/host/ohci-da8xx.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 209a262b5565..6158408b1ea0 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -202,12 +202,24 @@ static int ohci_da8xx_regulator_event(struct notifier_block *nb,
 	return 0;
 }
 
-static irqreturn_t ohci_da8xx_oc_handler(int irq, void *data)
+static irqreturn_t ohci_da8xx_oc_thread(int irq, void *data)
 {
 	struct da8xx_ohci_hcd *da8xx_ohci = data;
+	struct device *dev = da8xx_ohci->hcd->self.controller;
+	int ret;
 
-	if (gpiod_get_value(da8xx_ohci->oc_gpio))
-		gpiod_set_value(da8xx_ohci->vbus_gpio, 0);
+	if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio)) {
+		if (da8xx_ohci->vbus_gpio) {
+			gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
+		} else if (da8xx_ohci->vbus_reg) {
+			ret = regulator_disable(da8xx_ohci->vbus_reg);
+			if (ret)
+				dev_err(dev,
+					"Failed to disable regulator: %d\n",
+					ret);
+		}
+			
+	}
 
 	return IRQ_HANDLED;
 }
@@ -434,8 +446,9 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
 		if (oc_irq < 0)
 			goto err;
 
-		error = devm_request_irq(dev, oc_irq, ohci_da8xx_oc_handler,
-				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+		error = devm_request_threaded_irq(dev, oc_irq, NULL,
+				ohci_da8xx_oc_thread, IRQF_TRIGGER_RISING |
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 				"OHCI over-current indicator", da8xx_ohci);
 		if (error)
 			goto err;
-- 
2.21.0


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

* [PATCH v4 4/6] ARM: davinci: omapl138-hawk: add a fixed regulator for ohci-da8xx
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2019-04-11  9:30 ` [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 5/6] ARM: davinci: da830-evm: " Bartosz Golaszewski
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Instead of directly using the vbus GPIO we should model it as a fixed
regulator. Add all necessary fix-ups for the regulator to be registered
and configure the vbus GPIO as its enable pin.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/board-omapl138-hawk.c | 49 +++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index e244c8648594..db177a6a7e48 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -21,6 +21,7 @@
 #include <linux/platform_data/mtd-davinci.h>
 #include <linux/platform_data/mtd-davinci-aemif.h>
 #include <linux/platform_data/ti-aemif.h>
+#include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 
 #include <asm/mach-types.h>
@@ -298,15 +299,50 @@ static const short da850_hawk_usb11_pins[] = {
 	-1
 };
 
-static struct gpiod_lookup_table hawk_usb_gpio_lookup = {
+static struct regulator_consumer_supply hawk_usb_supplies[] = {
+	REGULATOR_SUPPLY("vbus", NULL),
+};
+
+static struct regulator_init_data hawk_usb_vbus_data = {
+	.consumer_supplies	= hawk_usb_supplies,
+	.num_consumer_supplies	= ARRAY_SIZE(hawk_usb_supplies),
+};
+
+static struct fixed_voltage_config hawk_usb_vbus = {
+	.supply_name		= "vbus",
+	.microvolts		= 3300000,
+	.init_data		= &hawk_usb_vbus_data,
+};
+
+static struct platform_device hawk_usb_vbus_device = {
+	.name		= "reg-fixed-voltage",
+	.id		= 0,
+	.dev		= {
+		.platform_data = &hawk_usb_vbus,
+	},
+};
+
+static struct gpiod_lookup_table hawk_usb_oc_gpio_lookup = {
 	.dev_id		= "ohci-da8xx",
 	.table = {
-		GPIO_LOOKUP("davinci_gpio", DA850_USB1_VBUS_PIN, "vbus", 0),
 		GPIO_LOOKUP("davinci_gpio", DA850_USB1_OC_PIN, "oc", 0),
 		{ }
 	},
 };
 
+static struct gpiod_lookup_table hawk_usb_vbus_gpio_lookup = {
+	.dev_id		= "reg-fixed-voltage.0",
+	.table = {
+		GPIO_LOOKUP("davinci_gpio", DA850_USB1_VBUS_PIN, NULL, 0),
+		{ }
+	},
+};
+
+static struct gpiod_lookup_table *hawk_usb_gpio_lookups[] = {
+	&hawk_usb_oc_gpio_lookup,
+	&hawk_usb_vbus_gpio_lookup,
+};
+
 static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
 	/* TPS2087 switch @ 5V */
 	.potpgt         = (3 + 1) / 2,  /* 3 ms max */
@@ -327,12 +363,19 @@ static __init void omapl138_hawk_usb_init(void)
 		pr_warn("%s: USB PHY CLK registration failed: %d\n",
 			__func__, ret);
 
+	gpiod_add_lookup_tables(hawk_usb_gpio_lookups,
+				ARRAY_SIZE(hawk_usb_gpio_lookups));
+
 	ret = da8xx_register_usb_phy();
 	if (ret)
 		pr_warn("%s: USB PHY registration failed: %d\n",
 			__func__, ret);
 
-	gpiod_add_lookup_table(&hawk_usb_gpio_lookup);
+	ret = platform_device_register(&hawk_usb_vbus_device);
+	if (ret) {
+		pr_warn("%s: Unable to register the vbus supply\n", __func__);
+		return;
+	}
 
 	ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
 	if (ret)
-- 
2.21.0


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

* [PATCH v4 5/6] ARM: davinci: da830-evm: add a fixed regulator for ohci-da8xx
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2019-04-11  9:30 ` [PATCH v4 4/6] ARM: davinci: omapl138-hawk: add a fixed regulator for ohci-da8xx Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-11  9:30 ` [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO Bartosz Golaszewski
  2019-04-12 10:40 ` [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Sekhar Nori
  6 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Instead of directly using the vbus GPIO we should model it as a fixed
regulator. Add all necessary fix-ups for the regulator to be registered
and configure the vbus GPIO as its enable pin.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 arch/arm/mach-davinci/board-da830-evm.c | 49 +++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index e3d74e43c47d..51a892702e27 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -29,6 +29,7 @@
 #include <linux/platform_data/spi-davinci.h>
 #include <linux/platform_data/usb-davinci.h>
 #include <linux/platform_data/ti-aemif.h>
+#include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/nvmem-provider.h>
 
@@ -53,15 +54,50 @@ static const short da830_evm_usb11_pins[] = {
 	-1
 };
 
-static struct gpiod_lookup_table da830_evm_usb_gpio_lookup = {
+static struct regulator_consumer_supply da830_evm_usb_supplies[] = {
+	REGULATOR_SUPPLY("vbus", NULL),
+};
+
+static struct regulator_init_data da830_evm_usb_vbus_data = {
+	.consumer_supplies	= da830_evm_usb_supplies,
+	.num_consumer_supplies	= ARRAY_SIZE(da830_evm_usb_supplies),
+};
+
+static struct fixed_voltage_config da830_evm_usb_vbus = {
+	.supply_name		= "vbus",
+	.microvolts		= 33000000,
+	.init_data		= &da830_evm_usb_vbus_data,
+};
+
+static struct platform_device da830_evm_usb_vbus_device = {
+	.name		= "reg-fixed-voltage",
+	.id		= 0,
+	.dev		= {
+		.platform_data = &da830_evm_usb_vbus,
+	},
+};
+
+static struct gpiod_lookup_table da830_evm_usb_oc_gpio_lookup = {
 	.dev_id		= "ohci-da8xx",
 	.table = {
-		GPIO_LOOKUP("davinci_gpio", ON_BD_USB_DRV, "vbus", 0),
 		GPIO_LOOKUP("davinci_gpio", ON_BD_USB_OVC, "oc", 0),
 		{ }
 	},
 };
 
+static struct gpiod_lookup_table da830_evm_usb_vbus_gpio_lookup = {
+	.dev_id		= "reg-fixed-voltage.0",
+	.table = {
+		GPIO_LOOKUP("davinci_gpio", ON_BD_USB_DRV, "vbus", 0),
+		{ }
+	},
+};
+
+static struct gpiod_lookup_table *da830_evm_usb_gpio_lookups[] = {
+	&da830_evm_usb_oc_gpio_lookup,
+	&da830_evm_usb_vbus_gpio_lookup,
+};
+
 static struct da8xx_ohci_root_hub da830_evm_usb11_pdata = {
 	/* TPS2065 switch @ 5V */
 	.potpgt		= (3 + 1) / 2,	/* 3 ms max */
@@ -76,6 +112,9 @@ static __init void da830_evm_usb_init(void)
 		pr_warn("%s: USB PHY CLK registration failed: %d\n",
 			__func__, ret);
 
+	gpiod_add_lookup_tables(da830_evm_usb_gpio_lookups,
+				ARRAY_SIZE(da830_evm_usb_gpio_lookups));
+
 	ret = da8xx_register_usb_phy();
 	if (ret)
 		pr_warn("%s: USB PHY registration failed: %d\n",
@@ -101,7 +140,11 @@ static __init void da830_evm_usb_init(void)
 		return;
 	}
 
-	gpiod_add_lookup_table(&da830_evm_usb_gpio_lookup);
+	ret = platform_device_register(&da830_evm_usb_vbus_device);
+	if (ret) {
+		pr_warn("%s: Unable to register the vbus supply\n", __func__);
+		return;
+	}
 
 	ret = da8xx_register_usb11(&da830_evm_usb11_pdata);
 	if (ret)
-- 
2.21.0


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

* [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
                   ` (4 preceding siblings ...)
  2019-04-11  9:30 ` [PATCH v4 5/6] ARM: davinci: da830-evm: " Bartosz Golaszewski
@ 2019-04-11  9:30 ` Bartosz Golaszewski
  2019-04-12 10:32   ` Sekhar Nori
  2019-04-12 10:40 ` [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Sekhar Nori
  6 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-11  9:30 UTC (permalink / raw)
  To: Sekhar Nori, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

All users now setup a fixed regulator for the vbus supply. We can drop
the vbus GPIO code.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/host/ohci-da8xx.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 6158408b1ea0..674641c94c9e 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -40,7 +40,6 @@ struct da8xx_ohci_hcd {
 	struct phy *usb11_phy;
 	struct regulator *vbus_reg;
 	struct notifier_block nb;
-	struct gpio_desc *vbus_gpio;
 	struct gpio_desc *oc_gpio;
 };
 
@@ -91,11 +90,6 @@ static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
 	struct device *dev = hcd->self.controller;
 	int ret;
 
-	if (da8xx_ohci->vbus_gpio) {
-		gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, on);
-		return 0;
-	}
-
 	if (!da8xx_ohci->vbus_reg)
 		return 0;
 
@@ -120,9 +114,6 @@ static int ohci_da8xx_get_power(struct usb_hcd *hcd)
 {
 	struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
 
-	if (da8xx_ohci->vbus_gpio)
-		return gpiod_get_value_cansleep(da8xx_ohci->vbus_gpio);
-
 	if (da8xx_ohci->vbus_reg)
 		return regulator_is_enabled(da8xx_ohci->vbus_reg);
 
@@ -155,9 +146,6 @@ static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
 {
 	struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
 
-	if (da8xx_ohci->vbus_gpio)
-		return 1;
-
 	if (da8xx_ohci->vbus_reg)
 		return 1;
 
@@ -209,9 +197,7 @@ static irqreturn_t ohci_da8xx_oc_thread(int irq, void *data)
 	int ret;
 
 	if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio)) {
-		if (da8xx_ohci->vbus_gpio) {
-			gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
-		} else if (da8xx_ohci->vbus_reg) {
+		if (da8xx_ohci->vbus_reg) {
 			ret = regulator_disable(da8xx_ohci->vbus_reg);
 			if (ret)
 				dev_err(dev,
@@ -432,11 +418,6 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
 		}
 	}
 
-	da8xx_ohci->vbus_gpio = devm_gpiod_get_optional(dev, "vbus",
-							GPIOD_OUT_HIGH);
-	if (IS_ERR(da8xx_ohci->vbus_gpio))
-		goto err;
-
 	da8xx_ohci->oc_gpio = devm_gpiod_get_optional(dev, "oc", GPIOD_IN);
 	if (IS_ERR(da8xx_ohci->oc_gpio))
 		goto err;
-- 
2.21.0


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

* Re: [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO
  2019-04-11  9:30 ` [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO Bartosz Golaszewski
@ 2019-04-12 10:32   ` Sekhar Nori
  0 siblings, 0 replies; 11+ messages in thread
From: Sekhar Nori @ 2019-04-12 10:32 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

Hi Bartosz,

On 11/04/19 3:00 PM, Bartosz Golaszewski wrote:
> @@ -209,9 +197,7 @@ static irqreturn_t ohci_da8xx_oc_thread(int irq, void *data)
>  	int ret;
>  
>  	if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio)) {
> -		if (da8xx_ohci->vbus_gpio) {
> -			gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
> -		} else if (da8xx_ohci->vbus_reg) {
> +		if (da8xx_ohci->vbus_reg) {
>  			ret = regulator_disable(da8xx_ohci->vbus_reg);
>  			if (ret)
>  				dev_err(dev,

You can now reduce a level of indentation here by doing

	if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio) &&
	    da8xx_ohci->vbus_reg) {
		ret = regulator_disable(da8xx_ohci->vbus_reg);
		if (ret)
			dev_err(dev, "Failed to disable regulator: %d\n",
				ret);
	}

Thanks
Sekhar

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

* Re: [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired
  2019-04-11  9:30 ` [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired Bartosz Golaszewski
@ 2019-04-12 10:39   ` Sekhar Nori
  2019-04-12 12:12     ` Bartosz Golaszewski
  0 siblings, 1 reply; 11+ messages in thread
From: Sekhar Nori @ 2019-04-12 10:39 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

Hi Bartosz,

On 11/04/19 3:00 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Historically the power supply management in this driver has been handled
> in two separate places in parallel. Device-tree users simply defined an
> appropriate regulator, while two boards with no DT support (da830-evm and
> omapl138-hawk) passed functions defined in their respective board files
> over platform data. These functions simply used legacy GPIO calls to
> watch the oc GPIO for interrupts and disable the vbus GPIO when the irq
> fires.
> 
> Commit d193abf1c913 ("usb: ohci-da8xx: add vbus and overcurrent gpios")
> updated these GPIO calls to the modern API and moved them inside the
> driver.
> 
> This however is not the optimal solution for the vbus GPIO which should
> be modeled as a fixed regulator that can be controlled with a GPIO.
> 
> In order to keep the overcurrent protection available once we move the
> board files to using fixed regulators we need to disable the enable_reg
> regulator when the overcurrent indicator interrupt fires. Since we
> cannot call regulator_disable() from interrupt context, we need to
> switch to using a oneshot threaded interrupt.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

I thought a bit on whether it makes sense to merge this patch into 6/6
since that is modifying some code you introduce here. But I think its
probably better to keep it separate like you have it now.

6/6 is about dropping support for vbus gpio, where as here you are
letting regulator be used for handing overcurrent detected using oci gpio.

Having it separate makes it possible to revert dropping support for vbus
gpio while keeping this functionality ;)

> -static irqreturn_t ohci_da8xx_oc_handler(int irq, void *data)
> +static irqreturn_t ohci_da8xx_oc_thread(int irq, void *data)
>  {
>  	struct da8xx_ohci_hcd *da8xx_ohci = data;
> +	struct device *dev = da8xx_ohci->hcd->self.controller;
> +	int ret;
>  
> -	if (gpiod_get_value(da8xx_ohci->oc_gpio))
> -		gpiod_set_value(da8xx_ohci->vbus_gpio, 0);
> +	if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio)) {
> +		if (da8xx_ohci->vbus_gpio) {
> +			gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
> +		} else if (da8xx_ohci->vbus_reg) {
> +			ret = regulator_disable(da8xx_ohci->vbus_reg);
> +			if (ret)
> +				dev_err(dev,
> +					"Failed to disable regulator: %d\n",
> +					ret);
> +		}
> +			

Unnecessary empty line here, that too one that introduces white space
error. Please drop.

Thanks,
Sekhar

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

* Re: [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator
  2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
                   ` (5 preceding siblings ...)
  2019-04-11  9:30 ` [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO Bartosz Golaszewski
@ 2019-04-12 10:40 ` Sekhar Nori
  6 siblings, 0 replies; 11+ messages in thread
From: Sekhar Nori @ 2019-04-12 10:40 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Alan Stern, Greg Kroah-Hartman
  Cc: linux-arm-kernel, linux-kernel, linux-usb, Bartosz Golaszewski

On 11/04/19 3:00 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Historically the power supply management in this driver has been handled
> in two separate places in parallel. Device-tree users simply defined an
> appropriate regulator, while two boards with no DT support (da830-evm and
> omapl138-hawk) passed functions defined in their respective board files
> over platform data. These functions simply used legacy GPIO calls to
> watch the oc GPIO for interrupts and disable the vbus GPIO when the irq
> fires.
> 
> Commit d193abf1c913 ("usb: ohci-da8xx: add vbus and overcurrent gpios")
> updated these GPIO calls to the modern API and moved them inside the
> driver.
> 
> This however is not the optimal solution for the vbus GPIO as it
> duplicates code. Instead we should model the GPIO as a fixed regulator
> that can be controlled with a GPIO.
> 
> This series adds fixed regulators for all users of vbus GPIO, adds
> overcurrent protection using the existing vbus regulator in the USB
> driver and removes the vbus GPIO calls once they're no longer used.
> 
> Tested on da830-evm with the module both built-in and loadable.

Looks good to me except some minor issues pointed out. If you can spin
the next version quickly, I would try merging for v5.2

Thanks,
Sekhar

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

* Re: [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired
  2019-04-12 10:39   ` Sekhar Nori
@ 2019-04-12 12:12     ` Bartosz Golaszewski
  0 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2019-04-12 12:12 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Kevin Hilman, Alan Stern, Greg Kroah-Hartman, Linux ARM,
	Linux Kernel Mailing List, linux-usb, Bartosz Golaszewski

pt., 12 kwi 2019 o 12:39 Sekhar Nori <nsekhar@ti.com> napisał(a):
>
> Hi Bartosz,
>
> On 11/04/19 3:00 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Historically the power supply management in this driver has been handled
> > in two separate places in parallel. Device-tree users simply defined an
> > appropriate regulator, while two boards with no DT support (da830-evm and
> > omapl138-hawk) passed functions defined in their respective board files
> > over platform data. These functions simply used legacy GPIO calls to
> > watch the oc GPIO for interrupts and disable the vbus GPIO when the irq
> > fires.
> >
> > Commit d193abf1c913 ("usb: ohci-da8xx: add vbus and overcurrent gpios")
> > updated these GPIO calls to the modern API and moved them inside the
> > driver.
> >
> > This however is not the optimal solution for the vbus GPIO which should
> > be modeled as a fixed regulator that can be controlled with a GPIO.
> >
> > In order to keep the overcurrent protection available once we move the
> > board files to using fixed regulators we need to disable the enable_reg
> > regulator when the overcurrent indicator interrupt fires. Since we
> > cannot call regulator_disable() from interrupt context, we need to
> > switch to using a oneshot threaded interrupt.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> I thought a bit on whether it makes sense to merge this patch into 6/6
> since that is modifying some code you introduce here. But I think its
> probably better to keep it separate like you have it now.
>
> 6/6 is about dropping support for vbus gpio, where as here you are
> letting regulator be used for handing overcurrent detected using oci gpio.
>
> Having it separate makes it possible to revert dropping support for vbus
> gpio while keeping this functionality ;)
>

These two changes were together in v1, but I too think that having it
separate keeps the series bisectable.

Bart

> > -static irqreturn_t ohci_da8xx_oc_handler(int irq, void *data)
> > +static irqreturn_t ohci_da8xx_oc_thread(int irq, void *data)
> >  {
> >       struct da8xx_ohci_hcd *da8xx_ohci = data;
> > +     struct device *dev = da8xx_ohci->hcd->self.controller;
> > +     int ret;
> >
> > -     if (gpiod_get_value(da8xx_ohci->oc_gpio))
> > -             gpiod_set_value(da8xx_ohci->vbus_gpio, 0);
> > +     if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio)) {
> > +             if (da8xx_ohci->vbus_gpio) {
> > +                     gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
> > +             } else if (da8xx_ohci->vbus_reg) {
> > +                     ret = regulator_disable(da8xx_ohci->vbus_reg);
> > +                     if (ret)
> > +                             dev_err(dev,
> > +                                     "Failed to disable regulator: %d\n",
> > +                                     ret);
> > +             }
> > +
>
> Unnecessary empty line here, that too one that introduces white space
> error. Please drop.
>
> Thanks,
> Sekhar

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

end of thread, other threads:[~2019-04-12 12:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-11  9:30 [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 1/6] ARM: davinci: add missing sentinels to GPIO lookup tables Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 2/6] usb: ohci-da8xx: let the regulator framework keep track of use count Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 3/6] usb: ohci-da8xx: disable the regulator if the overcurrent irq fired Bartosz Golaszewski
2019-04-12 10:39   ` Sekhar Nori
2019-04-12 12:12     ` Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 4/6] ARM: davinci: omapl138-hawk: add a fixed regulator for ohci-da8xx Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 5/6] ARM: davinci: da830-evm: " Bartosz Golaszewski
2019-04-11  9:30 ` [PATCH v4 6/6] usb: ohci-da8xx: drop the vbus GPIO Bartosz Golaszewski
2019-04-12 10:32   ` Sekhar Nori
2019-04-12 10:40 ` [PATCH v4 0/6] ARM: davinci: ohci-da8xx: model the vbus GPIO as a fixed regulator Sekhar Nori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).