Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi, On Fri, Dec 06, 2013 at 02:15:30PM -0600, Felipe Balbi wrote: On Tue, Dec 03, 2013 at 12:39:50PM +0200, Heikki Krogerus wrote: On Thu, Oct 17, 2013 at 09:54:26AM -0500, Felipe Balbi wrote: On Wed, Oct 16, 2013 at 04:27:26PM +0300, Roger Quadros wrote: On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote: Do you know if there are users of dwc3 other than exynos5250 and omap5? If not, we should get rid of the old USB PHY altogether. Intel's Baytrail, at least. But they don't use DT. I don't see any use for the generic-phy when the device is enumerated from PCI. If dwc3 can live without phys, there should not be any problem dropping the old USB PHY support. yeah, I don't want to drop PHY support, I want to require everybody to provide a PHY, otherwise we have too many options to handle and that's never clean. I have to clarify in case there was a misunderstanding. When I said generic-phy I meant drivers/usb/phy/phy-generic.c and I was not talking about Kishon's new generic PHY framework. So I was only saying that if the dwc3-pci.c is the only thing that needs the old usb phy support, then there should not be any problem dropping the support for it. 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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi,
On Tuesday 26 November 2013 03:02 AM, Felipe Balbi wrote:
Hi,
On Mon, Nov 11, 2013 at 08:06:12PM +0530, Kishon Vijay Abraham I wrote:
diff --git a/drivers/usb/dwc3/platform_data.h
b/drivers/usb/dwc3/platform_data.h
index 7db34f0..49ffa6d 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -24,4 +24,6 @@ struct dwc3_platform_data {
enum usb_device_speed maximum_speed;
enum usb_dr_mode dr_mode;
bool tx_fifo_resize;
+ bool usb2_phy;
+ bool usb3_phy;
This would look better as a quirks flag, then we could:
unsigned long quirks;
#define DWC3_QUIRK_NO_USB3_PHY BIT(0)
#define DWC3_QUIRK_NO_USB2_PHY BIT(1)
Should this quirk be used for dt also? Currently we find if it has usb3 phy
or
usb2 phy from the dt data only. But if we add a quirk, we'll have to add a
property to populate the quirk no?
either we use the quirk, or use the fact that no usb2_phy phandle is
defined, would work both ways, no ?
In my v3, I've made both to use quirks since we don't want to have separate
mechanism for dt and non-dt stuff to know the presence of a particular PHY.
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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi, On Thu, Oct 17, 2013 at 09:54:26AM -0500, Felipe Balbi wrote: On Wed, Oct 16, 2013 at 04:27:26PM +0300, Roger Quadros wrote: On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote: Do you know if there are users of dwc3 other than exynos5250 and omap5? If not, we should get rid of the old USB PHY altogether. Intel's Baytrail, at least. But they don't use DT. I don't see any use for the generic-phy when the device is enumerated from PCI. If dwc3 can live without phys, there should not be any problem dropping the old USB PHY support. Br, -- 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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi,
On Mon, Nov 11, 2013 at 08:06:12PM +0530, Kishon Vijay Abraham I wrote:
diff --git a/drivers/usb/dwc3/platform_data.h
b/drivers/usb/dwc3/platform_data.h
index 7db34f0..49ffa6d 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -24,4 +24,6 @@ struct dwc3_platform_data {
enum usb_device_speed maximum_speed;
enum usb_dr_mode dr_mode;
bool tx_fifo_resize;
+ bool usb2_phy;
+ bool usb3_phy;
This would look better as a quirks flag, then we could:
unsigned long quirks;
#define DWC3_QUIRK_NO_USB3_PHY BIT(0)
#define DWC3_QUIRK_NO_USB2_PHY BIT(1)
Should this quirk be used for dt also? Currently we find if it has usb3 phy or
usb2 phy from the dt data only. But if we add a quirk, we'll have to add a
property to populate the quirk no?
either we use the quirk, or use the fact that no usb2_phy phandle is
defined, would work both ways, no ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi Felipe,
On Thursday 17 October 2013 10:08 PM, Felipe Balbi wrote:
Hi,
On Wed, Oct 16, 2013 at 01:24:11AM +0530, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot
or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
I'm fine with this patch, but one comment below:
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
-dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
-dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+count = of_count_phandle_with_args(node, usb-phy, NULL);
+switch (count) {
+case 2:
+dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 1);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+case 1:
+dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 0);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+break;
+default:
+dev_err(dev, usb phy nodes not configured\n);
+}
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
-dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+if (pdata-usb2_phy) {
+dwc-usb2_phy = devm_usb_get_phy(dev,
+USB_PHY_TYPE_USB2);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+}
+
+if (pdata-usb3_phy) {
+dwc-usb3_phy = devm_usb_get_phy(dev,
+USB_PHY_TYPE_USB3);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+}
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
-if (IS_ERR(dwc-usb2_phy)) {
-ret = PTR_ERR(dwc-usb2_phy);
-
-/*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
-if (ret == -ENXIO)
-return ret;
-
-dev_err(dev, no usb2 phy configured\n);
-return -EPROBE_DEFER;
-}
-
-if (IS_ERR(dwc-usb3_phy)) {
-ret = PTR_ERR(dwc-usb3_phy);
-
-/*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
-if (ret == -ENXIO)
-return ret;
-
-dev_err(dev, no usb3 phy configured\n);
-return -EPROBE_DEFER;
-}
the idea for doing the error checking here was actually to avoid
duplicating it in all previous cases, as you did. Please keep the error
checking here and you're good to go.
-
dwc-xhci_resources[0].start = res-start;
dwc-xhci_resources[0].end = dwc-xhci_resources[0].start +
DWC3_XHCI_REGS_END;
diff --git a/drivers/usb/dwc3/platform_data.h
b/drivers/usb/dwc3/platform_data.h
index 7db34f0..49ffa6d 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -24,4 +24,6 @@ struct dwc3_platform_data {
enum usb_device_speed maximum_speed;
enum usb_dr_mode dr_mode;
bool tx_fifo_resize;
+bool usb2_phy;
+bool usb3_phy;
This would look better as a quirks flag, then we could:
unsigned long quirks;
#define DWC3_QUIRK_NO_USB3_PHY
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
On Tue, Nov 5, 2013 at 11:41 PM, Vivek Gautam gautamvivek1...@gmail.com wrote:
Dear Kishon, Roger
On Wed, Oct 16, 2013 at 6:40 PM, Kishon Vijay Abraham I kis...@ti.com wrote:
Hi roger,
On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
Hi Kishon,
Apologies for missing this thread for so long.
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt
boot or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy
layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
always
refers to usb3 phy. Since we've lived so long with this, this patch will
make
an assumption that if only one entry is populated in *usb-phy* property,
it will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72
--
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
-dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy,
0);
-dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy,
1);
+count = of_count_phandle_with_args(node, usb-phy, NULL);
+switch (count) {
+case 2:
+dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 1);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+case 1:
+dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 0);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
Thank you Roger for your concern regarding Exynos case.
It's true that, Exynos5 series of SoCs have got only one IP block for
DWC3'c PHY.
This block is actually a combo of USB2 phy (UTMI+) as well as USB3 phy
(PIPE3).
And the same is served by a single USB phy driver. This is also
clarified in the thread : https://lkml.org/lkml/2013/11/5/160
That was the case even before this patch no? Unfortunately the old USB PHY
library doesn't have APIs to get PHYs in a better way. If we try modifying
the
USB PHY library, it'll be kind of duplicating what is already there in the
Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
we have tried moving Samsung's USB phy controller driver for DWC3, to
generic phy framework
so that things look clearer on Samsung's side too. The necessary
patches are availble at:
https://lkml.org/lkml/2013/10/31/79
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
--
Best Regards
Vivek Gautam
Samsung RD Institute, Bangalore
India
--
Best Regards
Vivek Gautam
Samsung RD Institute, Bangalore
India
--
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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi,
On Thursday 17 October 2013 10:08 PM, Felipe Balbi wrote:
Hi,
On Wed, Oct 16, 2013 at 01:24:11AM +0530, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot
or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
I'm fine with this patch, but one comment below:
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
-dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
-dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+count = of_count_phandle_with_args(node, usb-phy, NULL);
+switch (count) {
+case 2:
+dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 1);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+case 1:
+dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 0);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+break;
+default:
+dev_err(dev, usb phy nodes not configured\n);
+}
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
-dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+if (pdata-usb2_phy) {
+dwc-usb2_phy = devm_usb_get_phy(dev,
+USB_PHY_TYPE_USB2);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+}
+
+if (pdata-usb3_phy) {
+dwc-usb3_phy = devm_usb_get_phy(dev,
+USB_PHY_TYPE_USB3);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+}
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
-if (IS_ERR(dwc-usb2_phy)) {
-ret = PTR_ERR(dwc-usb2_phy);
-
-/*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
-if (ret == -ENXIO)
-return ret;
-
-dev_err(dev, no usb2 phy configured\n);
-return -EPROBE_DEFER;
-}
-
-if (IS_ERR(dwc-usb3_phy)) {
-ret = PTR_ERR(dwc-usb3_phy);
-
-/*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
-if (ret == -ENXIO)
-return ret;
-
-dev_err(dev, no usb3 phy configured\n);
-return -EPROBE_DEFER;
-}
the idea for doing the error checking here was actually to avoid
duplicating it in all previous cases, as you did. Please keep the error
checking here and you're good to go.
Ok.
-
dwc-xhci_resources[0].start = res-start;
dwc-xhci_resources[0].end = dwc-xhci_resources[0].start +
DWC3_XHCI_REGS_END;
diff --git a/drivers/usb/dwc3/platform_data.h
b/drivers/usb/dwc3/platform_data.h
index 7db34f0..49ffa6d 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -24,4 +24,6 @@ struct dwc3_platform_data {
enum usb_device_speed maximum_speed;
enum usb_dr_mode dr_mode;
bool tx_fifo_resize;
+bool usb2_phy;
+bool usb3_phy;
This would look better as a quirks flag, then we could:
unsigned long quirks;
#define DWC3_QUIRK_NO_USB3_PHY
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
On Wed, Oct 16, 2013 at 04:27:26PM +0300, Roger Quadros wrote:
On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote:
Hi roger,
On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
Hi Kishon,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt
boot or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy
layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
always
refers to usb3 phy. Since we've lived so long with this, this patch will
make
an assumption that if only one entry is populated in *usb-phy* property,
it will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72
--
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+ select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+ int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ count = of_count_phandle_with_args(node, usb-phy, NULL);
+ switch (count) {
+ case 2:
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ case 1:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
That was the case even before this patch no? Unfortunately the old USB PHY
library doesn't have APIs to get PHYs in a better way. If we try modifying
the
USB PHY library, it'll be kind of duplicating what is already there in the
Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
OK. I agree with you.
Do you know if there are users of dwc3 other than exynos5250 and omap5?
If not, we should get rid of the old USB PHY altogether.
Intel's Baytrail, at least. But they don't use DT.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi,
On Wed, Oct 16, 2013 at 01:24:11AM +0530, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot
or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
I'm fine with this patch, but one comment below:
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ count = of_count_phandle_with_args(node, usb-phy, NULL);
+ switch (count) {
+ case 2:
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ case 1:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ break;
+ default:
+ dev_err(dev, usb phy nodes not configured\n);
+ }
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ if (pdata-usb2_phy) {
+ dwc-usb2_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB2);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ }
+
+ if (pdata-usb3_phy) {
+ dwc-usb3_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB3);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ }
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
- if (IS_ERR(dwc-usb2_phy)) {
- ret = PTR_ERR(dwc-usb2_phy);
-
- /*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
- if (ret == -ENXIO)
- return ret;
-
- dev_err(dev, no usb2 phy configured\n);
- return -EPROBE_DEFER;
- }
-
- if (IS_ERR(dwc-usb3_phy)) {
- ret = PTR_ERR(dwc-usb3_phy);
-
- /*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
- if (ret == -ENXIO)
- return ret;
-
- dev_err(dev, no usb3 phy configured\n);
- return -EPROBE_DEFER;
- }
the idea for doing the error checking here was actually to avoid
duplicating it in all previous cases, as you did. Please keep the error
checking here and you're good to go.
-
dwc-xhci_resources[0].start = res-start;
dwc-xhci_resources[0].end = dwc-xhci_resources[0].start +
DWC3_XHCI_REGS_END;
diff --git a/drivers/usb/dwc3/platform_data.h
b/drivers/usb/dwc3/platform_data.h
index 7db34f0..49ffa6d 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -24,4 +24,6 @@ struct dwc3_platform_data {
enum usb_device_speed maximum_speed;
enum usb_dr_mode dr_mode;
bool tx_fifo_resize;
+ bool usb2_phy;
+ bool usb3_phy;
This would look better as a quirks flag, then we could:
unsigned long quirks;
#define DWC3_QUIRK_NO_USB3_PHY BIT(0)
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi Kishon,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot
or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
always
refers to usb3 phy. Since we've lived so long with this, this patch will make
an assumption that if only one entry is populated in *usb-phy* property, it
will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72
--
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+ select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+ int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ count = of_count_phandle_with_args(node, usb-phy, NULL);
+ switch (count) {
+ case 2:
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ case 1:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
+ default:
+ dev_err(dev, usb phy nodes not configured\n);
+ }
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ if (pdata-usb2_phy) {
+ dwc-usb2_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB2);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ }
+
+ if (pdata-usb3_phy) {
+ dwc-usb3_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB3);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ }
This part looks fine to me.
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
- if (IS_ERR(dwc-usb2_phy)) {
- ret = PTR_ERR(dwc-usb2_phy);
-
- /*
- * if -ENXIO is returned, it means PHY layer wasn't
- * enabled, so it makes no sense to return -EPROBE_DEFER
- * in that case, since no PHY driver will ever probe.
- */
- if (ret ==
Re: [PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi roger,
On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
Hi Kishon,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot
or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
always
refers to usb3 phy. Since we've lived so long with this, this patch will make
an assumption that if only one entry is populated in *usb-phy* property, it
will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72
--
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
-dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
-dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+count = of_count_phandle_with_args(node, usb-phy, NULL);
+switch (count) {
+case 2:
+dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 1);
+if (IS_ERR(dwc-usb3_phy)) {
+dev_err(dev, usb3 phy not found\n);
+return PTR_ERR(dwc-usb3_phy);
+}
+case 1:
+dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+usb-phy, 0);
+if (IS_ERR(dwc-usb2_phy)) {
+dev_err(dev, usb2 phy not found\n);
+return PTR_ERR(dwc-usb2_phy);
+}
+break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
That was the case even before this patch no? Unfortunately the old USB PHY
library doesn't have APIs to get PHYs in a better way. If we try modifying the
USB PHY library, it'll be kind of duplicating what is already there in the
Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote:
Hi roger,
On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
Hi Kishon,
On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt
boot or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy
layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1
always
refers to usb3 phy. Since we've lived so long with this, this patch will
make
an assumption that if only one entry is populated in *usb-phy* property, it
will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72
--
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+ select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+ int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ count = of_count_phandle_with_args(node, usb-phy, NULL);
+ switch (count) {
+ case 2:
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ case 1:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ break;
In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
will wrongly treat it as usb2_phy.
That was the case even before this patch no? Unfortunately the old USB PHY
library doesn't have APIs to get PHYs in a better way. If we try modifying the
USB PHY library, it'll be kind of duplicating what is already there in the
Generic PHY library. I'd rather prefer Exynos guys to use the new framework.
OK. I agree with you.
Do you know if there are users of dwc3 other than exynos5250 and omap5?
If not, we should get rid of the old USB PHY altogether.
cheers,
-roger
--
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
[PATCH v2 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
There can be systems which does not have a external usb_phy, so get
usb_phy only if dt data indicates the presence of PHY in the case of dt boot or
if platform_data indicates the presence of PHY. Also remove checking if
return value is -ENXIO since it's now changed to always enable usb_phy layer.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1 always
refers to usb3 phy. Since we've lived so long with this, this patch will make
an assumption that if only one entry is populated in *usb-phy* property, it will
be usb2 phy and the next entry will be usb3 phy.
drivers/usb/dwc3/Kconfig |1 +
drivers/usb/dwc3/core.c | 72 --
drivers/usb/dwc3/platform_data.h |2 ++
3 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 70fc430..8e385b4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -1,6 +1,7 @@
config USB_DWC3
tristate DesignWare USB3 DRD Core Support
depends on (USB || USB_GADGET) HAS_DMA
+ select USB_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 474162e..cb91d70 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct device_node *node = dev-of_node;
struct resource *res;
struct dwc3 *dwc;
+ int count;
int ret = -ENOMEM;
@@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ count = of_count_phandle_with_args(node, usb-phy, NULL);
+ switch (count) {
+ case 2:
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ case 1:
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ break;
+ default:
+ dev_err(dev, usb phy nodes not configured\n);
+ }
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ if (pdata-usb2_phy) {
+ dwc-usb2_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB2);
+ if (IS_ERR(dwc-usb2_phy)) {
+ dev_err(dev, usb2 phy not found\n);
+ return PTR_ERR(dwc-usb2_phy);
+ }
+ }
+
+ if (pdata-usb3_phy) {
+ dwc-usb3_phy = devm_usb_get_phy(dev,
+ USB_PHY_TYPE_USB3);
+ if (IS_ERR(dwc-usb3_phy)) {
+ dev_err(dev, usb3 phy not found\n);
+ return PTR_ERR(dwc-usb3_phy);
+ }
+ }
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -409,36 +443,6 @@ static int dwc3_probe(struct platform_device *pdev)
if (dwc-maximum_speed == USB_SPEED_UNKNOWN)
dwc-maximum_speed = USB_SPEED_SUPER;
- if (IS_ERR(dwc-usb2_phy)) {
- ret = PTR_ERR(dwc-usb2_phy);
-
- /*
-* if -ENXIO is returned, it means PHY layer wasn't
-* enabled, so it makes no sense to return -EPROBE_DEFER
-* in that case, since no PHY driver will ever probe.
-*/
- if (ret == -ENXIO)
- return ret;
-
- dev_err(dev, no usb2 phy configured\n);
- return -EPROBE_DEFER;
- }
-
- if (IS_ERR(dwc-usb3_phy)) {
