When booting using device tree, we can not make use of
platform callbacks to handle vbus and over current gpios.
This series allows the ohci-da8xx driver to use a regulator
instead of the platform callbacks to control vbus and adds
the device tree bindings to be able to probe using DT.
Once all users of the platform callbacks will be converted to
use a regulator, we will be able to remove platform data completely.
Changes from v5->v6
* Fix regulator over current flag check (David)
* Spelling fixes and code cleanups (David)
* add Ack for device tree binding
Changes from v4->v5
* Append the Device tree patches to v4.
* Submit only the first part of the series (no dependencies).
this can be applied and merged through the usb tree.
Changes from v3->v4
* separate the series into smaller series for driver and arch/arm code,
to ease review and merging to different trees.
Changes form v2->v3
* drop patches that have been integrated to arch/arm
* drop regulator patches which will be integrated through regulator tree
* use of the accepted regulator API to get over current status
* better patch separation with the use of wrappers
Changes from v1->v2
* Rebased and added patch to make ohci a separate driver
* Use a regulator instead of handling Gpios (David Lechner)
* Add an over current mode to regulator framework
* Fixed regulator is able to register for and over current irq
* Added patch by Alexandre to remove build warnings
* Moved global variables into private hcd structure.
Axel Haslam (5):
USB: ohci: da8xx: use ohci priv data instead of globals
USB: ohci: da8xx: Add wrappers for platform callbacks
USB: ohci: da8xx: Allow a regulator to handle VBUS
USB: ohci: da8xx: Add devicetree bindings
USB: ohci: da8xx: Allow probing from DT
.../devicetree/bindings/usb/ohci-da8xx.txt | 23 ++
drivers/usb/host/ohci-da8xx.c | 291 +++++++++++++++++----
2 files changed, 264 insertions(+), 50 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
--
2.9.3
Instead of global variables, use the extra_priv_size of
the ohci driver.
We cannot yet move the ocic mask because this is used on
the interrupt handler which is registerded through platform
data and does not have an hcd pointer. This will be moved
on a later patch.
Signed-off-by: Axel Haslam <[email protected]>
---
drivers/usb/host/ohci-da8xx.c | 73 +++++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index b3de8bc..aa6f904f 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -35,43 +35,50 @@ 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 struct clk *usb11_clk;
-static struct phy *usb11_phy;
+struct da8xx_ohci_hcd {
+ struct clk *usb11_clk;
+ struct phy *usb11_phy;
+};
+
+#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
/* Over-current indicator change bitmask */
static volatile u16 ocic_mask;
-static int ohci_da8xx_enable(void)
+static int ohci_da8xx_enable(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret;
- ret = clk_prepare_enable(usb11_clk);
+ ret = clk_prepare_enable(da8xx_ohci->usb11_clk);
if (ret)
return ret;
- ret = phy_init(usb11_phy);
+ ret = phy_init(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_init;
- ret = phy_power_on(usb11_phy);
+ ret = phy_power_on(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_power_on;
return 0;
err_phy_power_on:
- phy_exit(usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
err_phy_init:
- clk_disable_unprepare(usb11_clk);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
return ret;
}
-static void ohci_da8xx_disable(void)
+static void ohci_da8xx_disable(struct usb_hcd *hcd)
{
- phy_power_off(usb11_phy);
- phy_exit(usb11_phy);
- clk_disable_unprepare(usb11_clk);
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+ phy_power_off(da8xx_ohci->usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
}
/*
@@ -97,7 +104,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
dev_dbg(dev, "starting USB controller\n");
- result = ohci_da8xx_enable();
+ result = ohci_da8xx_enable(hcd);
if (result < 0)
return result;
@@ -109,7 +116,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
result = ohci_setup(hcd);
if (result < 0) {
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
return result;
}
@@ -231,6 +238,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
+ struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
@@ -238,25 +246,29 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
if (hub == NULL)
return -ENODEV;
- usb11_clk = devm_clk_get(&pdev->dev, "usb11");
- if (IS_ERR(usb11_clk)) {
- if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
+ hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
+ dev_name(&pdev->dev));
+ if (!hcd)
+ return -ENOMEM;
+
+ da8xx_ohci = to_da8xx_ohci(hcd);
+
+ da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
+ if (IS_ERR(da8xx_ohci->usb11_clk)) {
+ error = PTR_ERR(da8xx_ohci->usb11_clk);
+ if (error != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get clock.\n");
- return PTR_ERR(usb11_clk);
+ goto err;
}
- usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
- if (IS_ERR(usb11_phy)) {
- if (PTR_ERR(usb11_phy) != -EPROBE_DEFER)
+ da8xx_ohci->usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
+ if (IS_ERR(da8xx_ohci->usb11_phy)) {
+ error = PTR_ERR(da8xx_ohci->usb11_phy);
+ if (error != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get phy.\n");
- return PTR_ERR(usb11_phy);
+ goto err;
}
- hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
- dev_name(&pdev->dev));
- if (!hcd)
- return -ENOMEM;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(hcd->regs)) {
@@ -320,7 +332,7 @@ static int ohci_da8xx_suspend(struct platform_device *pdev,
if (ret)
return ret;
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
hcd->state = HC_STATE_SUSPENDED;
return ret;
@@ -336,7 +348,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
msleep(5);
ohci->next_statechange = jiffies;
- ret = ohci_da8xx_enable();
+ ret = ohci_da8xx_enable(hcd);
if (ret)
return ret;
@@ -348,7 +360,8 @@ static int ohci_da8xx_resume(struct platform_device *dev)
#endif
static const struct ohci_driver_overrides da8xx_overrides __initconst = {
- .reset = ohci_da8xx_reset,
+ .reset = ohci_da8xx_reset,
+ .extra_priv_size = sizeof(struct da8xx_ohci_hcd),
};
/*
--
2.9.3
This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Axel Haslam <[email protected]>
---
.../devicetree/bindings/usb/ohci-da8xx.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 0000000..2dc8f67
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,23 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg: Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys: Phandle for the PHY device
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: phandle of regulator that controls vbus power / over-current
+
+Example:
+
+ohci: usb@0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ vbus-supply = <®_usb_ohci>;
+};
--
2.9.3
This adds the compatible string to the ohci driver
to be able to probe from DT
Signed-off-by: Axel Haslam <[email protected]>
---
drivers/usb/host/ohci-da8xx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index d0eb754..8b7479b 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -396,6 +396,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
/*-------------------------------------------------------------------------*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+ { .compatible = "ti,da830-ohci" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+#endif
static int ohci_da8xx_probe(struct platform_device *pdev)
{
@@ -547,6 +554,7 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
#endif
.driver = {
.name = DRV_NAME,
+ .of_match_table = of_match_ptr(da8xx_ohci_ids),
},
};
--
2.9.3
Using a regulator to handle VBUS will eliminate the need for
platform data and callbacks, and make the driver more generic
allowing different types of regulators to handle VBUS.
The regulator equivalents to the platform callbacks are:
set_power -> regulator_enable/regulator_disable
get_power -> regulator_is_enabled
get_oci -> regulator_get_error_flags
ocic_notify -> regulator event notification
Signed-off-by: Axel Haslam <[email protected]>
---
drivers/usb/host/ohci-da8xx.c | 97 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 94 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 90763ad..d0eb754 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/regulator/consumer.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
#include <asm/unaligned.h>
@@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
struct da8xx_ohci_hcd {
+ struct usb_hcd *hcd;
struct clk *usb11_clk;
struct phy *usb11_phy;
+ struct regulator *vbus_reg;
+ struct notifier_block nb;
+ unsigned int reg_enabled;
};
#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
@@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ int ret;
if (hub && hub->set_power)
return hub->set_power(1, on);
+ if (!da8xx_ohci->vbus_reg)
+ return 0;
+
+ if (on && !da8xx_ohci->reg_enabled) {
+ 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) {
+ 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;
}
static int ohci_da8xx_get_power(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->get_power)
return hub->get_power(1);
+ if (da8xx_ohci->vbus_reg)
+ return regulator_is_enabled(da8xx_ohci->vbus_reg);
+
return 1;
}
static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ unsigned int flags;
+ int ret;
if (hub && hub->get_oci)
return hub->get_oci(1);
+ if (!da8xx_ohci->vbus_reg)
+ return 0;
+
+ ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, &flags);
+ if (ret)
+ return ret;
+
+ if (flags & REGULATOR_ERROR_OVER_CURRENT)
+ return 1;
+
return 0;
}
static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->set_power)
return 1;
+ if (da8xx_ohci->vbus_reg)
+ return 1;
+
return 0;
}
static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->get_oci)
return 1;
+ if (da8xx_ohci->vbus_reg)
+ return 1;
+
return 0;
}
@@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
}
+static int ohci_da8xx_regulator_event(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct da8xx_ohci_hcd *da8xx_ohci =
+ container_of(nb, struct da8xx_ohci_hcd, nb);
+ struct device *dev = da8xx_ohci->hcd->self.controller;
+
+ if (event & REGULATOR_EVENT_OVER_CURRENT) {
+ dev_warn(dev, "over current event\n");
+ ocic_mask |= 1;
+ ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
+ }
+
+ return 0;
+}
+
static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ int ret = 0;
+
+ if (hub && hub->ocic_notify) {
+ ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
+ } else if (da8xx_ohci->vbus_reg) {
+ da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
+ ret = devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
+ &da8xx_ohci->nb);
+ }
- if (hub && hub->ocic_notify)
- return hub->ocic_notify(ohci_da8xx_ocic_handler);
+ if (ret)
+ dev_err(dev, "Failed to register notifier: %d\n", ret);
- return 0;
+ return ret;
}
static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
@@ -331,6 +409,7 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
return -ENOMEM;
da8xx_ohci = to_da8xx_ohci(hcd);
+ da8xx_ohci->hcd = hcd;
da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
if (IS_ERR(da8xx_ohci->usb11_clk)) {
@@ -348,6 +427,18 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
goto err;
}
+ da8xx_ohci->vbus_reg = devm_regulator_get_optional(&pdev->dev, "vbus");
+ if (IS_ERR(da8xx_ohci->vbus_reg)) {
+ error = PTR_ERR(da8xx_ohci->vbus_reg);
+ if (error == -ENODEV) {
+ da8xx_ohci->vbus_reg = NULL;
+ } else {
+ dev_err(&pdev->dev,
+ "Failed to get regulator\n");
+ goto err;
+ }
+ }
+
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(hcd->regs)) {
--
2.9.3
To migrate to a DT based boot, we will remove the use of platform
callbacks, in favor of using the regulator framework to handle
vbus and over current.
In preparation to use a regulator instead of callbacks, move the platform
data callbacks into separate functions. This provides well defined place
to for the regulator API to coexist with the platform callbacks before
all users are converted.
Signed-off-by: Axel Haslam <[email protected]>
---
drivers/usb/host/ohci-da8xx.c | 125 ++++++++++++++++++++++++++++++++++--------
1 file changed, 102 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index aa6f904f..90763ad 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -81,6 +81,72 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
clk_disable_unprepare(da8xx_ohci->usb11_clk);
}
+static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->set_power)
+ return hub->set_power(1, on);
+
+ return 0;
+}
+
+static int ohci_da8xx_get_power(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_power)
+ return hub->get_power(1);
+
+ return 1;
+}
+
+static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_oci)
+ return hub->get_oci(1);
+
+ return 0;
+}
+
+static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->set_power)
+ return 1;
+
+ return 0;
+}
+
+static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_oci)
+ return 1;
+
+ return 0;
+}
+
+static int ohci_da8xx_has_potpgt(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->potpgt)
+ return 1;
+
+ return 0;
+}
+
/*
* Handle the port over-current indicator change.
*/
@@ -94,6 +160,26 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
}
+static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->ocic_notify)
+ return hub->ocic_notify(ohci_da8xx_ocic_handler);
+
+ return 0;
+}
+
+static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->ocic_notify)
+ hub->ocic_notify(NULL);
+}
+
static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
@@ -127,16 +213,18 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
* the correct hub descriptor...
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (hub->set_power) {
+ if (ohci_da8xx_has_set_power(hcd)) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
}
- if (hub->get_oci) {
+ if (ohci_da8xx_has_oci(hcd)) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
- rh_a &= ~RH_A_POTPGT;
- rh_a |= hub->potpgt << 24;
+ if (ohci_da8xx_has_potpgt(hcd)) {
+ rh_a &= ~RH_A_POTPGT;
+ rh_a |= hub->potpgt << 24;
+ }
ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
@@ -169,7 +257,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int temp;
switch (typeReq) {
@@ -183,11 +270,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (hub->get_power && hub->get_power(wIndex) == 0)
+ if (!ohci_da8xx_get_power(hcd))
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (hub->get_oci && hub->get_oci(wIndex) > 0)
+ if (ohci_da8xx_get_oci(hcd) > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
@@ -212,10 +299,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- if (!hub->set_power)
- return -EPIPE;
-
- return hub->set_power(wIndex, temp) ? -EPIPE : 0;
+ return ohci_da8xx_set_power(hcd, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
@@ -237,15 +321,10 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
-
- if (hub == NULL)
- return -ENODEV;
-
hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
dev_name(&pdev->dev));
if (!hcd)
@@ -290,12 +369,13 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
device_wakeup_enable(hcd->self.controller);
- if (hub->ocic_notify) {
- error = hub->ocic_notify(ohci_da8xx_ocic_handler);
- if (!error)
- return 0;
- }
+ error = ohci_da8xx_register_notify(hcd);
+ if (error)
+ goto err_remove_hcd;
+
+ return 0;
+err_remove_hcd:
usb_remove_hcd(hcd);
err:
usb_put_hcd(hcd);
@@ -305,9 +385,8 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
static int ohci_da8xx_remove(struct platform_device *pdev)
{
struct usb_hcd *hcd = platform_get_drvdata(pdev);
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
- hub->ocic_notify(NULL);
+ ohci_da8xx_unregister_notify(hcd);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
--
2.9.3
On 11/21/2016 10:30 AM, Axel Haslam wrote:
> Using a regulator to handle VBUS will eliminate the need for
> platform data and callbacks, and make the driver more generic
> allowing different types of regulators to handle VBUS.
>
> The regulator equivalents to the platform callbacks are:
> set_power -> regulator_enable/regulator_disable
> get_power -> regulator_is_enabled
> get_oci -> regulator_get_error_flags
> ocic_notify -> regulator event notification
>
> Signed-off-by: Axel Haslam <[email protected]>
> ---
> drivers/usb/host/ohci-da8xx.c | 97 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index 90763ad..d0eb754 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_device.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_data/usb-davinci.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/usb.h>
> #include <linux/usb/hcd.h>
> #include <asm/unaligned.h>
> @@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
> static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
>
> struct da8xx_ohci_hcd {
> + struct usb_hcd *hcd;
> struct clk *usb11_clk;
> struct phy *usb11_phy;
> + struct regulator *vbus_reg;
> + struct notifier_block nb;
> + unsigned int reg_enabled;
> };
>
> #define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
> @@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
>
> static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
> + int ret;
>
> if (hub && hub->set_power)
> return hub->set_power(1, on);
>
> + if (!da8xx_ohci->vbus_reg)
> + return 0;
> +
> + if (on && !da8xx_ohci->reg_enabled) {
> + 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) {
> + 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;
> }
>
> static int ohci_da8xx_get_power(struct usb_hcd *hcd)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>
> if (hub && hub->get_power)
> return hub->get_power(1);
>
> + if (da8xx_ohci->vbus_reg)
> + return regulator_is_enabled(da8xx_ohci->vbus_reg);
> +
> return 1;
> }
>
> static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
> + unsigned int flags;
> + int ret;
>
> if (hub && hub->get_oci)
> return hub->get_oci(1);
>
> + if (!da8xx_ohci->vbus_reg)
> + return 0;
> +
> + ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, &flags);
> + if (ret)
> + return ret;
> +
> + if (flags & REGULATOR_ERROR_OVER_CURRENT)
> + return 1;
> +
> return 0;
> }
>
> static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>
> if (hub && hub->set_power)
> return 1;
>
> + if (da8xx_ohci->vbus_reg)
> + return 1;
> +
> return 0;
> }
>
> static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>
> if (hub && hub->get_oci)
> return 1;
>
> + if (da8xx_ohci->vbus_reg)
> + return 1;
> +
> return 0;
> }
>
> @@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
> hub->set_power(port, 0);
> }
>
> +static int ohci_da8xx_regulator_event(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct da8xx_ohci_hcd *da8xx_ohci =
> + container_of(nb, struct da8xx_ohci_hcd, nb);
> + struct device *dev = da8xx_ohci->hcd->self.controller;
> +
> + if (event & REGULATOR_EVENT_OVER_CURRENT) {
> + dev_warn(dev, "over current event\n");
> + ocic_mask |= 1;
Following up from a v5 thread that is still applicable here, Axel said:
> I did a couple of tests, and i don't get those prints do you get them?.
The problem here is that ocic_mask |= 1; is wrong. It needs to be...
ocic_mask |= (1 << 1);
If you look at the other uses of ocic_mask, you will see why this it
needs to be this way. Once you make this change, then you will see this
in the kernel log:
usb 1-1: USB disconnect, device number 2
usb 1-1: may be reset is needed?..
ohci ohci: over current event
usb usb1-port1: over-current condition
So, we don't need the dev_warn() here.
More from Axel:
> What i understand is that they happen when we get a hub event that is
> reporting the over current. Which is what the root hub of the davinci
system
> does not have, and why we use gpios instead).
Even though the hardware is not capable of detecting the overcurrent by
itself, we are poking the registers in ohci_da8xx_hub_control(), so the
core hub driver sees it just the same as if the hardware itself changed
the register.
> + ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
> + }
> +
> + return 0;
> +}
> +
> static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
> {
> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
> struct device *dev = hcd->self.controller;
> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
> + int ret = 0;
> +
> + if (hub && hub->ocic_notify) {
> + ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
> + } else if (da8xx_ohci->vbus_reg) {
> + da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
> + ret = devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
> + &da8xx_ohci->nb);
> + }
>
> - if (hub && hub->ocic_notify)
> - return hub->ocic_notify(ohci_da8xx_ocic_handler);
> + if (ret)
> + dev_err(dev, "Failed to register notifier: %d\n", ret);
>
> - return 0;
> + return ret;
> }
>
> static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
> @@ -331,6 +409,7 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> da8xx_ohci = to_da8xx_ohci(hcd);
> + da8xx_ohci->hcd = hcd;
>
> da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
> if (IS_ERR(da8xx_ohci->usb11_clk)) {
> @@ -348,6 +427,18 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
> goto err;
> }
>
> + da8xx_ohci->vbus_reg = devm_regulator_get_optional(&pdev->dev, "vbus");
> + if (IS_ERR(da8xx_ohci->vbus_reg)) {
> + error = PTR_ERR(da8xx_ohci->vbus_reg);
> + if (error == -ENODEV) {
> + da8xx_ohci->vbus_reg = NULL;
> + } else {
It might be good to skip the dev_err() if we get -EPROBE_DEFER
> + dev_err(&pdev->dev,
> + "Failed to get regulator\n");
> + goto err;
> + }
> + }
> +
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
> if (IS_ERR(hcd->regs)) {
>
On 11/21/2016 10:30 AM, Axel Haslam wrote:
> Instead of global variables, use the extra_priv_size of
> the ohci driver.
>
> We cannot yet move the ocic mask because this is used on
> the interrupt handler which is registerded through platform
s/registerded/registered/
> data and does not have an hcd pointer. This will be moved
> on a later patch.
>
> Signed-off-by: Axel Haslam <[email protected]>
> ---
Looks good to me (other than the spelling error above).
Tested-by: David Lechner <[email protected]>
On Tue, Nov 22, 2016 at 9:37 PM, David Lechner <[email protected]> wrote:
> On 11/21/2016 10:30 AM, Axel Haslam wrote:
>>
>> Using a regulator to handle VBUS will eliminate the need for
>> platform data and callbacks, and make the driver more generic
>> allowing different types of regulators to handle VBUS.
>>
>> The regulator equivalents to the platform callbacks are:
>> set_power -> regulator_enable/regulator_disable
>> get_power -> regulator_is_enabled
>> get_oci -> regulator_get_error_flags
>> ocic_notify -> regulator event notification
>>
>> Signed-off-by: Axel Haslam <[email protected]>
>> ---
>> drivers/usb/host/ohci-da8xx.c | 97
>> +++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 94 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
>> index 90763ad..d0eb754 100644
>> --- a/drivers/usb/host/ohci-da8xx.c
>> +++ b/drivers/usb/host/ohci-da8xx.c
>> @@ -20,6 +20,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/phy/phy.h>
>> #include <linux/platform_data/usb-davinci.h>
>> +#include <linux/regulator/consumer.h>
>> #include <linux/usb.h>
>> #include <linux/usb/hcd.h>
>> #include <asm/unaligned.h>
>> @@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd
>> *hcd, u16 typeReq,
>> static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
>>
>> struct da8xx_ohci_hcd {
>> + struct usb_hcd *hcd;
>> struct clk *usb11_clk;
>> struct phy *usb11_phy;
>> + struct regulator *vbus_reg;
>> + struct notifier_block nb;
>> + unsigned int reg_enabled;
>> };
>>
>> #define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd
>> *)(hcd_to_ohci(hcd)->priv)
>> @@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
>>
>> static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>> + int ret;
>>
>> if (hub && hub->set_power)
>> return hub->set_power(1, on);
>>
>> + if (!da8xx_ohci->vbus_reg)
>> + return 0;
>> +
>> + if (on && !da8xx_ohci->reg_enabled) {
>> + 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) {
>> + 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;
>> }
>>
>> static int ohci_da8xx_get_power(struct usb_hcd *hcd)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>
>> if (hub && hub->get_power)
>> return hub->get_power(1);
>>
>> + if (da8xx_ohci->vbus_reg)
>> + return regulator_is_enabled(da8xx_ohci->vbus_reg);
>> +
>> return 1;
>> }
>>
>> static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>> + unsigned int flags;
>> + int ret;
>>
>> if (hub && hub->get_oci)
>> return hub->get_oci(1);
>>
>> + if (!da8xx_ohci->vbus_reg)
>> + return 0;
>> +
>> + ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, &flags);
>> + if (ret)
>> + return ret;
>> +
>> + if (flags & REGULATOR_ERROR_OVER_CURRENT)
>> + return 1;
>> +
>> return 0;
>> }
>>
>> static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>
>> if (hub && hub->set_power)
>> return 1;
>>
>> + if (da8xx_ohci->vbus_reg)
>> + return 1;
>> +
>> return 0;
>> }
>>
>> static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>
>> if (hub && hub->get_oci)
>> return 1;
>>
>> + if (da8xx_ohci->vbus_reg)
>> + return 1;
>> +
>> return 0;
>> }
>>
>> @@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct
>> da8xx_ohci_root_hub *hub,
>> hub->set_power(port, 0);
>> }
>>
>> +static int ohci_da8xx_regulator_event(struct notifier_block *nb,
>> + unsigned long event, void *data)
>> +{
>> + struct da8xx_ohci_hcd *da8xx_ohci =
>> + container_of(nb, struct da8xx_ohci_hcd,
>> nb);
>> + struct device *dev = da8xx_ohci->hcd->self.controller;
>> +
>> + if (event & REGULATOR_EVENT_OVER_CURRENT) {
>> + dev_warn(dev, "over current event\n");
>> + ocic_mask |= 1;
>
>
> Following up from a v5 thread that is still applicable here, Axel said:
>
>> I did a couple of tests, and i don't get those prints do you get them?.
>
> The problem here is that ocic_mask |= 1; is wrong. It needs to be...
>
> ocic_mask |= (1 << 1);
>
i see, i will fix it thanks!
> If you look at the other uses of ocic_mask, you will see why this it needs
> to be this way. Once you make this change, then you will see this in the
> kernel log:
>
> usb 1-1: USB disconnect, device number 2
> usb 1-1: may be reset is needed?..
> ohci ohci: over current event
> usb usb1-port1: over-current condition
>
> So, we don't need the dev_warn() here.
agree!
>
>
> More from Axel:
>
>> What i understand is that they happen when we get a hub event that is
>> reporting the over current. Which is what the root hub of the davinci
>> system
>> does not have, and why we use gpios instead).
>
> Even though the hardware is not capable of detecting the overcurrent by
> itself, we are poking the registers in ohci_da8xx_hub_control(), so the core
> hub driver sees it just the same as if the hardware itself changed the
> register.
>
>
>> + ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
>> {
>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>> + int ret = 0;
>> +
>> + if (hub && hub->ocic_notify) {
>> + ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
>> + } else if (da8xx_ohci->vbus_reg) {
>> + da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
>> + ret =
>> devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
>> + &da8xx_ohci->nb);
>> + }
>>
>> - if (hub && hub->ocic_notify)
>> - return hub->ocic_notify(ohci_da8xx_ocic_handler);
>> + if (ret)
>> + dev_err(dev, "Failed to register notifier: %d\n", ret);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
>> @@ -331,6 +409,7 @@ static int ohci_da8xx_probe(struct platform_device
>> *pdev)
>> return -ENOMEM;
>>
>> da8xx_ohci = to_da8xx_ohci(hcd);
>> + da8xx_ohci->hcd = hcd;
>>
>> da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
>> if (IS_ERR(da8xx_ohci->usb11_clk)) {
>> @@ -348,6 +427,18 @@ static int ohci_da8xx_probe(struct platform_device
>> *pdev)
>> goto err;
>> }
>>
>> + da8xx_ohci->vbus_reg = devm_regulator_get_optional(&pdev->dev,
>> "vbus");
>> + if (IS_ERR(da8xx_ohci->vbus_reg)) {
>> + error = PTR_ERR(da8xx_ohci->vbus_reg);
>> + if (error == -ENODEV) {
>> + da8xx_ohci->vbus_reg = NULL;
>> + } else {
>
>
> It might be good to skip the dev_err() if we get -EPROBE_DEFER
>
>> + dev_err(&pdev->dev,
>> + "Failed to get regulator\n");
>> + goto err;
>> + }
>> + }
>> +
>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
>> if (IS_ERR(hcd->regs)) {
>>
>
On 11/22/2016 02:46 PM, Axel Haslam wrote:
> On Tue, Nov 22, 2016 at 9:37 PM, David Lechner <[email protected]> wrote:
>> On 11/21/2016 10:30 AM, Axel Haslam wrote:
>>>
>>> Using a regulator to handle VBUS will eliminate the need for
>>> platform data and callbacks, and make the driver more generic
>>> allowing different types of regulators to handle VBUS.
>>>
>>> The regulator equivalents to the platform callbacks are:
>>> set_power -> regulator_enable/regulator_disable
>>> get_power -> regulator_is_enabled
>>> get_oci -> regulator_get_error_flags
>>> ocic_notify -> regulator event notification
>>>
>>> Signed-off-by: Axel Haslam <[email protected]>
>>> ---
>>> drivers/usb/host/ohci-da8xx.c | 97
>>> +++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 94 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
>>> index 90763ad..d0eb754 100644
>>> --- a/drivers/usb/host/ohci-da8xx.c
>>> +++ b/drivers/usb/host/ohci-da8xx.c
>>> @@ -20,6 +20,7 @@
>>> #include <linux/platform_device.h>
>>> #include <linux/phy/phy.h>
>>> #include <linux/platform_data/usb-davinci.h>
>>> +#include <linux/regulator/consumer.h>
>>> #include <linux/usb.h>
>>> #include <linux/usb/hcd.h>
>>> #include <asm/unaligned.h>
>>> @@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd
>>> *hcd, u16 typeReq,
>>> static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
>>>
>>> struct da8xx_ohci_hcd {
>>> + struct usb_hcd *hcd;
>>> struct clk *usb11_clk;
>>> struct phy *usb11_phy;
>>> + struct regulator *vbus_reg;
>>> + struct notifier_block nb;
>>> + unsigned int reg_enabled;
>>> };
>>>
>>> #define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd
>>> *)(hcd_to_ohci(hcd)->priv)
>>> @@ -83,56 +88,103 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
>>>
>>> static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>> + int ret;
>>>
>>> if (hub && hub->set_power)
>>> return hub->set_power(1, on);
>>>
>>> + if (!da8xx_ohci->vbus_reg)
>>> + return 0;
>>> +
>>> + if (on && !da8xx_ohci->reg_enabled) {
>>> + 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) {
>>> + 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;
>>> }
>>>
>>> static int ohci_da8xx_get_power(struct usb_hcd *hcd)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>>
>>> if (hub && hub->get_power)
>>> return hub->get_power(1);
>>>
>>> + if (da8xx_ohci->vbus_reg)
>>> + return regulator_is_enabled(da8xx_ohci->vbus_reg);
>>> +
>>> return 1;
>>> }
>>>
>>> static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>> + unsigned int flags;
>>> + int ret;
>>>
>>> if (hub && hub->get_oci)
>>> return hub->get_oci(1);
>>>
>>> + if (!da8xx_ohci->vbus_reg)
>>> + return 0;
>>> +
>>> + ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, &flags);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (flags & REGULATOR_ERROR_OVER_CURRENT)
>>> + return 1;
>>> +
>>> return 0;
>>> }
>>>
>>> static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>>
>>> if (hub && hub->set_power)
>>> return 1;
>>>
>>> + if (da8xx_ohci->vbus_reg)
>>> + return 1;
>>> +
>>> return 0;
>>> }
>>>
>>> static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>>
>>> if (hub && hub->get_oci)
>>> return 1;
>>>
>>> + if (da8xx_ohci->vbus_reg)
>>> + return 1;
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct
>>> da8xx_ohci_root_hub *hub,
>>> hub->set_power(port, 0);
>>> }
>>>
>>> +static int ohci_da8xx_regulator_event(struct notifier_block *nb,
>>> + unsigned long event, void *data)
>>> +{
>>> + struct da8xx_ohci_hcd *da8xx_ohci =
>>> + container_of(nb, struct da8xx_ohci_hcd,
>>> nb);
>>> + struct device *dev = da8xx_ohci->hcd->self.controller;
>>> +
>>> + if (event & REGULATOR_EVENT_OVER_CURRENT) {
>>> + dev_warn(dev, "over current event\n");
>>> + ocic_mask |= 1;
>>
>>
>> Following up from a v5 thread that is still applicable here, Axel said:
>>
>>> I did a couple of tests, and i don't get those prints do you get them?.
>>
>> The problem here is that ocic_mask |= 1; is wrong. It needs to be...
>>
>> ocic_mask |= (1 << 1);
Actually parentheses are not needed
ocic_mask |= 1 << 1;
>>
>
> i see, i will fix it thanks!
>
>> If you look at the other uses of ocic_mask, you will see why this it needs
>> to be this way. Once you make this change, then you will see this in the
>> kernel log:
>>
>> usb 1-1: USB disconnect, device number 2
>> usb 1-1: may be reset is needed?..
>> ohci ohci: over current event
>> usb usb1-port1: over-current condition
>>
>> So, we don't need the dev_warn() here.
>
> agree!
>
>>
>>
>> More from Axel:
>>
>>> What i understand is that they happen when we get a hub event that is
>>> reporting the over current. Which is what the root hub of the davinci
>>> system
>>> does not have, and why we use gpios instead).
>>
>> Even though the hardware is not capable of detecting the overcurrent by
>> itself, we are poking the registers in ohci_da8xx_hub_control(), so the core
>> hub driver sees it just the same as if the hardware itself changed the
>> register.
>>
>>
>>> + ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
>>> {
>>> + struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
>>> struct device *dev = hcd->self.controller;
>>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>>> + int ret = 0;
>>> +
>>> + if (hub && hub->ocic_notify) {
>>> + ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
>>> + } else if (da8xx_ohci->vbus_reg) {
>>> + da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
>>> + ret =
>>> devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
>>> + &da8xx_ohci->nb);
>>> + }
>>>
>>> - if (hub && hub->ocic_notify)
>>> - return hub->ocic_notify(ohci_da8xx_ocic_handler);
>>> + if (ret)
>>> + dev_err(dev, "Failed to register notifier: %d\n", ret);
>>>
>>> - return 0;
>>> + return ret;
>>> }
>>>
>>> static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
>>> @@ -331,6 +409,7 @@ static int ohci_da8xx_probe(struct platform_device
>>> *pdev)
>>> return -ENOMEM;
>>>
>>> da8xx_ohci = to_da8xx_ohci(hcd);
>>> + da8xx_ohci->hcd = hcd;
>>>
>>> da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
>>> if (IS_ERR(da8xx_ohci->usb11_clk)) {
>>> @@ -348,6 +427,18 @@ static int ohci_da8xx_probe(struct platform_device
>>> *pdev)
>>> goto err;
>>> }
>>>
>>> + da8xx_ohci->vbus_reg = devm_regulator_get_optional(&pdev->dev,
>>> "vbus");
>>> + if (IS_ERR(da8xx_ohci->vbus_reg)) {
>>> + error = PTR_ERR(da8xx_ohci->vbus_reg);
>>> + if (error == -ENODEV) {
>>> + da8xx_ohci->vbus_reg = NULL;
>>> + } else {
>>
>>
>> It might be good to skip the dev_err() if we get -EPROBE_DEFER
>>
>>> + dev_err(&pdev->dev,
>>> + "Failed to get regulator\n");
>>> + goto err;
>>> + }
>>> + }
>>> +
>>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
>>> if (IS_ERR(hcd->regs)) {
>>>
>>