[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[Laurent Pinchart]

Hello,

The PXA27x OHCI implementation doesn't perform automatic control of port power
supplies for all ports. While the PPS and LSDA bits of the HcRhPortStatus
register are implemented, only a subset of ports have an external power enable
pin controlled by the port status register. Other ports need their power supply
to be controlled manually.

In order to do so I've implemented manual regulator control in the ohci-pxa27x
driver. This requires overriding the default behaviour of the CLEAR_FEATURE
and SET_FEATURE requests for USB_PORT_FEAT_POWER with a custom hub control
operation. In turn this requires calling the currently static ohci_hub_control
function from the ohci-pxa27x driver.

The ohci-s3c2410 driver already implements a similar feature, and accesses the
ohci_hub_control function by saving the struct hc_driver hub_control value to
a global variable right after calling ohci_init_driver. This is a bit of a
hack, and as I need to call that function as well I've decided to export it
instead.

Another option would have been to handle the regulators directly inside the
ohci-hub implementation, but I'm not sure whether it's worth it yet. Comments
(or acks :-)) will be appreciated.

Please not that I haven't been able to test the third patch due to lack of
hardware. I've however tested a similar implementation on an out of tree
PXA270 board.

Laurent Pinchart (3):
  USB: OHCI: Export the ohci_hub_control function
  USB: ohci-pxa27x: Add support for external vbus regulators
  ARM: pxa: zeus: Replace OHCI init/exit functions with a regulator

 arch/arm/mach-pxa/zeus.c        | 89 ++++++++++++++++++++++-------------------
 drivers/usb/host/ohci-hub.c     |  4 +-
 drivers/usb/host/ohci-pxa27x.c  | 67 +++++++++++++++++++++++++++++++
 drivers/usb/host/ohci-s3c2410.c |  7 +---
 drivers/usb/host/ohci.h         |  2 +
 5 files changed, 121 insertions(+), 48 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 1/3] USB: OHCI: Export the ohci_hub_control function

[Laurent Pinchart]

Platform drivers sometimes need to perform specific handling of hub
control requests. Make this possible by exporting the ohci_hub_control()
function which can then be called from a custom hub control handler in
the default case.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/usb/host/ohci-hub.c     | 4 ++--
 drivers/usb/host/ohci-s3c2410.c | 7 ++-----
 drivers/usb/host/ohci.h         | 2 ++
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index c81c872..70013c6 100644
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -646,7 +646,7 @@ static inline int root_port_reset (struct ohci_hcd *ohci, unsigned port)
 	return 0;
 }
 
-static int ohci_hub_control (
+int ohci_hub_control (
 	struct usb_hcd	*hcd,
 	u16		typeReq,
 	u16		wValue,
@@ -772,4 +772,4 @@ error:
 	}
 	return retval;
 }
-
+EXPORT_SYMBOL_GPL(ohci_hub_control);
diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c
index ff7c8f1..95809c5 100644
--- a/drivers/usb/host/ohci-s3c2410.c
+++ b/drivers/usb/host/ohci-s3c2410.c
@@ -45,8 +45,6 @@ static struct clk *usb_clk;
 
 /* forward definitions */
 
-static int (*orig_ohci_hub_control)(struct usb_hcd  *hcd, u16 typeReq,
-			u16 wValue, u16 wIndex, char *buf, u16 wLength);
 static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
 
 static void s3c2410_hcd_oc(struct s3c2410_hcd_info *info, int port_oc);
@@ -181,7 +179,7 @@ static int ohci_s3c2410_hub_control(
 	 * process the request straight away and exit */
 
 	if (info == NULL) {
-		ret = orig_ohci_hub_control(hcd, typeReq, wValue,
+		ret = ohci_hub_control(hcd, typeReq, wValue,
 				       wIndex, buf, wLength);
 		goto out;
 	}
@@ -231,7 +229,7 @@ static int ohci_s3c2410_hub_control(
 		break;
 	}
 
-	ret = orig_ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+	ret = ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
 	if (ret)
 		goto out;
 
@@ -489,7 +487,6 @@ static int __init ohci_s3c2410_init(void)
 	 * override these functions by making it too easy.
 	 */
 
-	orig_ohci_hub_control = ohci_s3c2410_hc_driver.hub_control;
 	orig_ohci_hub_status_data = ohci_s3c2410_hc_driver.hub_status_data;
 
 	ohci_s3c2410_hc_driver.hub_status_data	= ohci_s3c2410_hub_status_data;
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 9250cad..f317de6 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -727,3 +727,5 @@ extern int	ohci_setup(struct usb_hcd *hcd);
 extern int	ohci_suspend(struct usb_hcd *hcd, bool do_wakeup);
 extern int	ohci_resume(struct usb_hcd *hcd, bool hibernated);
 #endif
+extern int	ohci_hub_control(struct usb_hcd	*hcd, u16 typeReq, u16 wValue,
+				 u16 wIndex, char *buf, u16 wLength);
-- 
1.8.3.2

[PATCH/RFC 2/3] USB: ohci-pxa27x: Add support for external vbus regulators

[Laurent Pinchart]

Override the hub control operation to enable and disable external
regulators for the ports vbus power supply in response to clear/set
USB_PORT_FEAT_POWER requests.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/usb/host/ohci-pxa27x.c | 67 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index d21d5fe..0c31265 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -30,6 +30,7 @@
 #include <linux/platform_data/usb-ohci-pxa27x.h>
 #include <linux/platform_data/usb-pxa3xx-ulpi.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/signal.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
@@ -120,6 +121,8 @@ static struct hc_driver __read_mostly ohci_pxa27x_hc_driver;
 struct pxa27x_ohci {
 	struct clk	*clk;
 	void __iomem	*mmio_base;
+	struct regulator *vbus[3];
+	bool		vbus_enabled[3];
 };
 
 #define to_pxa27x_ohci(hcd)	(struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -166,6 +169,52 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *pxa_ohci, int mode)
 	return 0;
 }
 
+static int pxa27x_ohci_set_vbus_power(struct pxa27x_ohci *pxa_ohci,
+				      unsigned int port, bool enable)
+{
+	struct regulator *vbus = pxa_ohci->vbus[port];
+	int ret = 0;
+
+	if (IS_ERR_OR_NULL(vbus))
+		return 0;
+
+	if (enable && !pxa_ohci->vbus_enabled[port])
+		ret = regulator_enable(vbus);
+	else if (!enable && pxa_ohci->vbus_enabled[port])
+		ret = regulator_disable(vbus);
+
+	if (ret < 0)
+		return ret;
+
+	pxa_ohci->vbus_enabled[port] = enable;
+
+	return 0;
+}
+
+static int pxa27x_ohci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
+				   u16 wIndex, char *buf, u16 wLength)
+{
+	struct pxa27x_ohci *pxa_ohci = to_pxa27x_ohci(hcd);
+	int ret;
+
+	switch (typeReq) {
+	case SetPortFeature:
+	case ClearPortFeature:
+		if (!wIndex || wIndex > 3)
+			return -EPIPE;
+
+		if (wValue != USB_PORT_FEAT_POWER)
+			break;
+
+		ret = pxa27x_ohci_set_vbus_power(pxa_ohci, wIndex - 1,
+						 typeReq == SetPortFeature);
+		if (ret)
+			return ret;
+		break;
+	}
+
+	return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+}
 /*-------------------------------------------------------------------------*/
 
 static inline void pxa27x_setup_hc(struct pxa27x_ohci *pxa_ohci,
@@ -372,6 +421,7 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct platform_device
 	struct ohci_hcd *ohci;
 	struct resource *r;
 	struct clk *usb_clk;
+	unsigned int i;
 
 	retval = ohci_pxa_of_init(pdev);
 	if (retval)
@@ -417,6 +467,16 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct platform_device
 	pxa_ohci->clk = usb_clk;
 	pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
 
+	for (i = 0; i < 3; ++i) {
+		char name[6];
+
+		if (!(inf->flags & (ENABLE_PORT1 << i)))
+			continue;
+
+		sprintf(name, "vbus%u", i + 1);
+		pxa_ohci->vbus[i] = devm_regulator_get(&pdev->dev, name);
+	}
+
 	retval = pxa27x_start_hc(pxa_ohci, &pdev->dev);
 	if (retval < 0) {
 		pr_debug("pxa27x_start_hc failed");
@@ -462,6 +522,10 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct platform_device
 void usb_hcd_pxa27x_remove (struct usb_hcd *hcd, struct platform_device *pdev)
 {
 	struct pxa27x_ohci *pxa_ohci = to_pxa27x_ohci(hcd);
+	unsigned int i;
+
+	for (i = 0; i < 3; ++i)
+		pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
 
 	usb_remove_hcd(hcd);
 	pxa27x_stop_hc(pxa_ohci, &pdev->dev);
@@ -563,7 +627,10 @@ static int __init ohci_pxa27x_init(void)
 		return -ENODEV;
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+
 	ohci_init_driver(&ohci_pxa27x_hc_driver, &pxa27x_overrides);
+	ohci_pxa27x_hc_driver.hub_control = pxa27x_ohci_hub_control;
+
 	return platform_driver_register(&ohci_hcd_pxa27x_driver);
 }
 module_init(ohci_pxa27x_init);
-- 
1.8.3.2

[PATCH/RFC 3/3] ARM: pxa: zeus: Replace OHCI init/exit functions with a regulator

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-pxa/zeus.c | 89 ++++++++++++++++++++++++++----------------------
 1 file changed, 48 insertions(+), 41 deletions(-)

diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
index b19d1c3..205f9bf 100644
--- a/arch/arm/mach-pxa/zeus.c
+++ b/arch/arm/mach-pxa/zeus.c
@@ -413,7 +413,7 @@ static struct fixed_voltage_config can_regulator_pdata = {
 
 static struct platform_device can_regulator_device = {
 	.name	= "reg-fixed-volage",
-	.id	= -1,
+	.id	= 0,
 	.dev	= {
 		.platform_data	= &can_regulator_pdata,
 	},
@@ -510,18 +510,6 @@ struct platform_device zeus_max6369_device = {
 	.num_resources	= 1,
 };
 
-static struct platform_device *zeus_devices[] __initdata = {
-	&zeus_serial_device,
-	&zeus_mtd_devices[0],
-	&zeus_dm9k0_device,
-	&zeus_dm9k1_device,
-	&zeus_sram_device,
-	&zeus_leds_device,
-	&zeus_pcmcia_device,
-	&zeus_max6369_device,
-	&can_regulator_device,
-};
-
 /* AC'97 */
 static pxa2xx_audio_ops_t zeus_ac97_info = {
 	.reset_gpio = 95,
@@ -532,44 +520,50 @@ static pxa2xx_audio_ops_t zeus_ac97_info = {
  * USB host
  */
 
-static int zeus_ohci_init(struct device *dev)
-{
-	int err;
-
-	/* Switch on port 2. */
-	if ((err = gpio_request(ZEUS_USB2_PWREN_GPIO, "USB2_PWREN"))) {
-		dev_err(dev, "Can't request USB2_PWREN\n");
-		return err;
-	}
-
-	if ((err = gpio_direction_output(ZEUS_USB2_PWREN_GPIO, 1))) {
-		gpio_free(ZEUS_USB2_PWREN_GPIO);
-		dev_err(dev, "Can't enable USB2_PWREN\n");
-		return err;
-	}
+static struct regulator_consumer_supply zeus_ohci_regulator_supplies[] = {
+	REGULATOR_SUPPLY("vbus2", "pxa27x-ohci"),
+};
 
-	/* Port 2 is shared between host and client interface. */
-	UP2OCR = UP2OCR_HXOE | UP2OCR_HXS | UP2OCR_DMPDE | UP2OCR_DPPDE;
+static struct regulator_init_data zeus_ohci_regulator_data = {
+	.constraints = {
+		.valid_ops_mask		= REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(zeus_ohci_regulator_supplies),
+	.consumer_supplies	= zeus_ohci_regulator_supplies,
+};
 
-	return 0;
-}
+static struct fixed_voltage_config zeus_ohci_regulator_config = {
+	.supply_name		= "vbus2",
+	.microvolts		= 5000000, /* 5.0V */
+	.gpio			= ZEUS_USB2_PWREN_GPIO,
+	.enable_high		= 1,
+	.startup_delay		= 0,
+	.init_data		= &zeus_ohci_regulator_data,
+};
 
-static void zeus_ohci_exit(struct device *dev)
-{
-	/* Power-off port 2 */
-	gpio_direction_output(ZEUS_USB2_PWREN_GPIO, 0);
-	gpio_free(ZEUS_USB2_PWREN_GPIO);
-}
+static struct platform_device zeus_ohci_regulator_device = {
+	.name		= "reg-fixed-voltage",
+	.id		= 1,
+	.dev = {
+		.platform_data = &zeus_ohci_regulator_config,
+	},
+};
 
 static struct pxaohci_platform_data zeus_ohci_platform_data = {
 	.port_mode	= PMM_NPS_MODE,
 	/* Clear Power Control Polarity Low and set Power Sense
 	 * Polarity Low. Supply power to USB ports. */
 	.flags		= ENABLE_PORT_ALL | POWER_SENSE_LOW,
-	.init		= zeus_ohci_init,
-	.exit		= zeus_ohci_exit,
 };
 
+static void zeus_register_ohci(void)
+{
+	/* Port 2 is shared between host and client interface. */
+	UP2OCR = UP2OCR_HXOE | UP2OCR_HXS | UP2OCR_DMPDE | UP2OCR_DPPDE;
+
+	pxa_set_ohci_info(&zeus_ohci_platform_data);
+}
+
 /*
  * Flat Panel
  */
@@ -677,6 +671,19 @@ static struct pxa2xx_udc_mach_info zeus_udc_info = {
 	.udc_command = zeus_udc_command,
 };
 
+static struct platform_device *zeus_devices[] __initdata = {
+	&zeus_serial_device,
+	&zeus_mtd_devices[0],
+	&zeus_dm9k0_device,
+	&zeus_dm9k1_device,
+	&zeus_sram_device,
+	&zeus_leds_device,
+	&zeus_pcmcia_device,
+	&zeus_max6369_device,
+	&can_regulator_device,
+	&zeus_ohci_regulator_device,
+};
+
 #ifdef CONFIG_PM
 static void zeus_power_off(void)
 {
@@ -847,7 +854,7 @@ static void __init zeus_init(void)
 
 	platform_add_devices(zeus_devices, ARRAY_SIZE(zeus_devices));
 
-	pxa_set_ohci_info(&zeus_ohci_platform_data);
+	zeus_register_ohci();
 
 	if (zeus_setup_fb_gpios())
 		pr_err("Failed to setup fb gpios\n");
-- 
1.8.3.2

[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[Alan Stern]

On Mon, 14 Apr 2014, Laurent Pinchart wrote:

> Hello,
> 
> The PXA27x OHCI implementation doesn't perform automatic control of port power
> supplies for all ports. While the PPS and LSDA bits of the HcRhPortStatus
> register are implemented, only a subset of ports have an external power enable
> pin controlled by the port status register. Other ports need their power supply
> to be controlled manually.
> 
> In order to do so I've implemented manual regulator control in the ohci-pxa27x
> driver. This requires overriding the default behaviour of the CLEAR_FEATURE
> and SET_FEATURE requests for USB_PORT_FEAT_POWER with a custom hub control
> operation. In turn this requires calling the currently static ohci_hub_control
> function from the ohci-pxa27x driver.
> 
> The ohci-s3c2410 driver already implements a similar feature, and accesses the
> ohci_hub_control function by saving the struct hc_driver hub_control value to
> a global variable right after calling ohci_init_driver. This is a bit of a
> hack, and as I need to call that function as well I've decided to export it
> instead.

It only seems like a hack if you don't think about it the right way.  :-)

The intention was to imitate an object-oriented style, by allowing a
child class to override member functions in the parent class.  The
vtable mechanism in C++, for example, does essentially the same thing
as ohci-s3c2410 (except that C++ doesn't save the original function
pointer in a global variable; instead it reads the function pointer
from the parent's vtable as needed -- ohci-s3c2410 can't do that 
because ohci_hc_driver isn't exported).

I'm open to the idea of exporting the hub functions.  In the end, it
doesn't make all that much difference (it would save a little object
code).

> Another option would have been to handle the regulators directly inside the
> ohci-hub implementation, but I'm not sure whether it's worth it yet. Comments
> (or acks :-)) will be appreciated.
> 
> Please not that I haven't been able to test the third patch due to lack of
> hardware. I've however tested a similar implementation on an out of tree
> PXA270 board.
> 
> Laurent Pinchart (3):
>   USB: OHCI: Export the ohci_hub_control function
>   USB: ohci-pxa27x: Add support for external vbus regulators
>   ARM: pxa: zeus: Replace OHCI init/exit functions with a regulator
> 
>  arch/arm/mach-pxa/zeus.c        | 89 ++++++++++++++++++++++-------------------
>  drivers/usb/host/ohci-hub.c     |  4 +-
>  drivers/usb/host/ohci-pxa27x.c  | 67 +++++++++++++++++++++++++++++++
>  drivers/usb/host/ohci-s3c2410.c |  7 +---
>  drivers/usb/host/ohci.h         |  2 +
>  5 files changed, 121 insertions(+), 48 deletions(-)

You missed ohci-at91.c.  And ehci-tegra.c plays similar games.  I'd 
prefer to do the same thing for both ehci-hcd and ohci-hcd.

Alan Stern

[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[Laurent Pinchart]

Hi Alan,

Thank you for your comments.

On Monday 14 April 2014 10:35:43 Alan Stern wrote:
> On Mon, 14 Apr 2014, Laurent Pinchart wrote:
> > Hello,
> > 
> > The PXA27x OHCI implementation doesn't perform automatic control of port
> > power supplies for all ports. While the PPS and LSDA bits of the
> > HcRhPortStatus register are implemented, only a subset of ports have an
> > external power enable pin controlled by the port status register. Other
> > ports need their power supply to be controlled manually.
> > 
> > In order to do so I've implemented manual regulator control in the
> > ohci-pxa27x driver. This requires overriding the default behaviour of the
> > CLEAR_FEATURE and SET_FEATURE requests for USB_PORT_FEAT_POWER with a
> > custom hub control operation. In turn this requires calling the currently
> > static ohci_hub_control function from the ohci-pxa27x driver.
> > 
> > The ohci-s3c2410 driver already implements a similar feature, and accesses
> > the ohci_hub_control function by saving the struct hc_driver hub_control
> > value to a global variable right after calling ohci_init_driver. This is
> > a bit of a hack, and as I need to call that function as well I've decided
> > to export it instead.
> 
> It only seems like a hack if you don't think about it the right way.  :-)
> 
> The intention was to imitate an object-oriented style, by allowing a child
> class to override member functions in the parent class. The vtable mechanism
> in C++, for example, does essentially the same thing as ohci-s3c2410 (except
> that C++ doesn't save the original function pointer in a global variable;
> instead it reads the function pointer from the parent's vtable as needed --
> ohci-s3c2410 can't do that because ohci_hc_driver isn't exported).

gcc 4.9 has been released with a new speculative devirtualization optimization 
pass. As we don't use C++ we can't make use of that feature, so we need to 
perform the optimization manually, hence this change :-) 

> I'm open to the idea of exporting the hub functions. In the end, it doesn't
> make all that much difference (it would save a little object code).

I would have agreed to keep the code as it is today if you had thought that 
exporting the hub functions was a really bad idea, but as you're open to it, 
and as it removes a bit of code without much of a drawback, I think it makes 
sense to perform that optimization.

> > Another option would have been to handle the regulators directly inside
> > the ohci-hub implementation, but I'm not sure whether it's worth it yet.
> > Comments (or acks :-)) will be appreciated.
> > 
> > Please not that I haven't been able to test the third patch due to lack of
> > hardware. I've however tested a similar implementation on an out of tree
> > PXA270 board.
> > 
> > Laurent Pinchart (3):
> >   USB: OHCI: Export the ohci_hub_control function
> >   USB: ohci-pxa27x: Add support for external vbus regulators
> >   ARM: pxa: zeus: Replace OHCI init/exit functions with a regulator
> >  
> >  arch/arm/mach-pxa/zeus.c        | 89 ++++++++++++++++++------------------
> >  drivers/usb/host/ohci-hub.c     |  4 +-
> >  drivers/usb/host/ohci-pxa27x.c  | 67 +++++++++++++++++++++++++++++++
> >  drivers/usb/host/ohci-s3c2410.c |  7 +---
> >  drivers/usb/host/ohci.h         |  2 +
> >  5 files changed, 121 insertions(+), 48 deletions(-)
> 
> You missed ohci-at91.c.  And ehci-tegra.c plays similar games.  I'd prefer
> to do the same thing for both ehci-hcd and ohci-hcd.

OK. I'll work on a v2 of patch 1/3 that modifies ohci-at91.c as well, and I'll 
add a new patch to perform the same change for ehci-hub.c and ehci-tegra.c.

By the way, as a second step, do you think it would make sense to handle the 
vbus regulators directly in ohci_hub_control() ?

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[Alan Stern]

On Tue, 15 Apr 2014, Laurent Pinchart wrote:

> > I'm open to the idea of exporting the hub functions. In the end, it doesn't
> > make all that much difference (it would save a little object code).
> 
> I would have agreed to keep the code as it is today if you had thought that 
> exporting the hub functions was a really bad idea, but as you're open to it, 
> and as it removes a bit of code without much of a drawback, I think it makes 
> sense to perform that optimization.

Agreed.  We aren't really _encouraging_ people to do this, because it 
still takes some manual work to override the default operation.

> By the way, as a second step, do you think it would make sense to handle the 
> vbus regulators directly in ohci_hub_control() ?

I don't see any good way of doing this generically.  Since it is
currently needed only for PXA27x, we don't know what other
architectures will require in the future.

Alan Stern

[PATCH/RFC 0/3] Allow OHCI drivers to override hub control operations

[Laurent Pinchart]

Hi Alan,

On Tuesday 15 April 2014 14:05:14 Alan Stern wrote:
> On Tue, 15 Apr 2014, Laurent Pinchart wrote:
> > > I'm open to the idea of exporting the hub functions. In the end, it
> > > doesn't make all that much difference (it would save a little object
> > > code).
> > 
> > I would have agreed to keep the code as it is today if you had thought
> > that exporting the hub functions was a really bad idea, but as you're open
> > to it, and as it removes a bit of code without much of a drawback, I think
> > it makes sense to perform that optimization.
> 
> Agreed.  We aren't really _encouraging_ people to do this, because it
> still takes some manual work to override the default operation.
> 
> > By the way, as a second step, do you think it would make sense to handle
> > the vbus regulators directly in ohci_hub_control() ?
> 
> I don't see any good way of doing this generically.  Since it is
> currently needed only for PXA27x, we don't know what other
> architectures will require in the future.

Fine with me, let's revisit this later if needed then.

-- 
Regards,

Laurent Pinchart