Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi Kishon,
On Wed, Oct 16, 2013 at 01:24:12AM +0530, Kishon Vijay Abraham I wrote:
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
Is there some specific reason for these checks? The driver should not
need to care about the platform (DT, ACPI, platform based).
Just get the phys and check the return value. In case ERR_PTR(-ENODEV)
leave the phy as NULL and let the driver continue normally. With other
errors you make the dwc3 probe fail.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On Tuesday 03 December 2013 05:29 PM, Heikki Krogerus wrote:
Hi Kishon,
On Wed, Oct 16, 2013 at 01:24:12AM +0530, Kishon Vijay Abraham I wrote:
+count = of_property_match_string(node, phy-names, usb2-phy);
+if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+if (IS_ERR(dwc-usb2_generic_phy)) {
+dev_err(dev, no usb2 phy configured yet);
+return PTR_ERR(dwc-usb2_generic_phy);
+}
+dwc-usb2_phy = NULL;
+}
+
+count = of_property_match_string(node, phy-names, usb3-phy);
+if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+if (IS_ERR(dwc-usb3_generic_phy)) {
+dev_err(dev, no usb3 phy configured yet);
+return PTR_ERR(dwc-usb3_generic_phy);
+}
+dwc-usb3_phy = NULL;
+}
Is there some specific reason for these checks? The driver should not
need to care about the platform (DT, ACPI, platform based).
yeah just wanted to throw an error if a platform needs PHY but wasn't able to
get it. Btw this has changed after my v3 of this patch series which I sent
sometime back [1] where we use quirks to know if a PHY is needed for that
platform or not.
http://www.spinics.net/lists/linux-usb/msg98077.html
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On Wednesday 16 October 2013 07:53 PM, Roger Quadros wrote:
On 10/16/2013 04:52 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+ select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_init(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_init(dwc-usb3_generic_phy);
+
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is
wrong.
You overlooked the above?
No. Forgot to comment back.
Yeah, you are right.. that should be fixed.
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK (16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
+
There are a couple of issues here.
- The above code must be executed only if it node is valid.
of_property_match_string will give a error value if the node is not present.
*count = 0* can take care of this.
OK.
- We must either get both PHYs via old method or via new method and not
support mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
Why? As long as both the
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
On 10/21/2013 02:55 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 16 October 2013 07:53 PM, Roger Quadros wrote:
On 10/16/2013 04:52 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+ select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_init(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_init(dwc-usb3_generic_phy);
+
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is
wrong.
You overlooked the above?
No. Forgot to comment back.
Yeah, you are right.. that should be fixed.
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK (16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
+
There are a couple of issues here.
- The above code must be executed only if it node is valid.
of_property_match_string will give a error value if the node is not present.
*count = 0* can take care of this.
OK.
- We must either get both PHYs via old method or via new method and not
support mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
Why? As long as
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+ select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_init(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_init(dwc-usb3_generic_phy);
+
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is wrong.
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK (16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
+
There are a couple of issues here.
- The above code must be executed only if it node is valid.
- We must either get both PHYs via old method or via new method and not support
mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
If you give the new method preference then you don't even need to attempt a
devm_usb_get_phy()
if the generic phy is present in the node.
/* default to superspeed if no maximum_speed passed */
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
@@ -462,6 +493,11 @@ static
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi,
On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+if (dwc-usb2_generic_phy)
+phy_init(dwc-usb2_generic_phy);
+if (dwc-usb3_generic_phy)
+phy_init(dwc-usb3_generic_phy);
+
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is wrong.
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+if (dwc-usb2_generic_phy)
+phy_power_off(dwc-usb2_generic_phy);
+if (dwc-usb3_generic_phy)
+phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK (16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+count = of_property_match_string(node, phy-names, usb2-phy);
+if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+if (IS_ERR(dwc-usb2_generic_phy)) {
+dev_err(dev, no usb2 phy configured yet);
+return PTR_ERR(dwc-usb2_generic_phy);
+}
+dwc-usb2_phy = NULL;
+}
+
+count = of_property_match_string(node, phy-names, usb3-phy);
+if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+if (IS_ERR(dwc-usb3_generic_phy)) {
+dev_err(dev, no usb3 phy configured yet);
+return PTR_ERR(dwc-usb3_generic_phy);
+}
+dwc-usb3_phy = NULL;
+}
+
There are a couple of issues here.
- The above code must be executed only if it node is valid.
of_property_match_string will give a error value if the node is not present.
*count = 0* can take care of this.
- We must either get both PHYs via old method or via new method and not
support mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
Why? As long as both the frameworks co-exist (and we support both in dwc3), I
don't see any reason why we shouldn't allow it. We can avoid adding a few more
checks by leaving it the way
Re: [PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
On 10/16/2013 04:52 PM, Kishon Vijay Abraham I wrote:
Hi,
On Wednesday 16 October 2013 06:48 PM, Roger Quadros wrote:
Hi,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+ select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_init(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_init(dwc-usb3_generic_phy);
+
dwc3_core_soft_reset() is called from dwc3_core_init() which is called from
dwc3_probe() but before the PHYs are initialized.
This will cause phy power_on() to be called before phy_init() which is wrong.
You overlooked the above?
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK(16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
+
There are a couple of issues here.
- The above code must be executed only if it node is valid.
of_property_match_string will give a error value if the node is not present.
*count = 0* can take care of this.
OK.
- We must either get both PHYs via old method or via new method and not
support mix matching
them. e.g. it is possible with this code that usb2_phy is set and
usb3_generic_phy is set.
Why? As long as both the frameworks co-exist (and we support both in dwc3), I
don't see any reason why we shouldn't allow it. We can
[PATCH v2 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Adapted dwc3 core to use the Generic PHY Framework. So for init, exit,
power_on and power_off the following APIs are used phy_init(), phy_exit(),
phy_power_on() and phy_power_off().
However using the old USB phy library wont be removed till the PHYs of all
other SoC's using dwc3 core is adapted to the Generic PHY Framework.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++-
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c| 52
drivers/usb/dwc3/core.h|7
drivers/usb/dwc3/platform_data.h |2 +
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e807635..471366d 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -6,11 +6,13 @@ Required properties:
- compatible: must be snps,dwc3
- reg : Address and length of the register set for the device
- interrupts: Interrupts used by the dwc3 controller.
+
+Optional properties:
- usb-phy : array of phandle for the PHY device. The first element
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
-
-Optional properties:
+ - phys: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8e385b4..64eed6f 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -2,6 +2,7 @@ config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
select USB_PHY
+ select GENERIC_PHY
select USB_XHCI_PLATFORM if USB_SUPPORT USB_XHCI_HCD
help
Say Y or M here if your system has a Dual Role SuperSpeed
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index cb91d70..28bfa5b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -82,6 +82,12 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc)
usb_phy_init(dwc-usb2_phy);
usb_phy_init(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_init(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_init(dwc-usb3_generic_phy);
+
mdelay(100);
/* Clear USB3 PHY reset */
@@ -343,6 +349,11 @@ static void dwc3_core_exit(struct dwc3 *dwc)
{
usb_phy_shutdown(dwc-usb2_phy);
usb_phy_shutdown(dwc-usb3_phy);
+
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_off(dwc-usb3_generic_phy);
}
#define DWC3_ALIGN_MASK(16 - 1)
@@ -439,6 +450,26 @@ static int dwc3_probe(struct platform_device *pdev)
dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
}
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
+
/* default to superspeed if no maximum_speed passed */
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
@@ -462,6 +493,11 @@ static int dwc3_probe(struct platform_device *pdev)
usb_phy_set_suspend(dwc-usb2_phy, 0);
usb_phy_set_suspend(dwc-usb3_phy, 0);
+ if (dwc-usb2_generic_phy)
+ phy_power_on(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+ phy_power_on(dwc-usb3_generic_phy);
+
spin_lock_init(dwc-lock);
platform_set_drvdata(pdev, dwc);
@@ -588,6 +624,11 @@ static int dwc3_remove(struct platform_device *pdev)
usb_phy_set_suspend(dwc-usb2_phy, 1);
usb_phy_set_suspend(dwc-usb3_phy, 1);
+ if (dwc-usb2_generic_phy)
+ phy_power_off(dwc-usb2_generic_phy);
+ if (dwc-usb3_generic_phy)
+
