| Message ID | 1407235148-31790-1-git-send-email-gautam.vivek@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, August 05, 2014 7:39 PM, Vivek Gautam wrote: > > Since we want to keep support for both older usb-phys as well as the > newer generic phys, lets first get the generic PHYs and fallback to > older USB-PHYs only when we fail to get the former. > This should fix the issue with ehci-exynos and ohci-exynos, wherein > in the absence of SAMSUNG_USB2PHY config symbol, we end up getting > the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs > are not configured properly. > > Reported-by: Sachin Kamat <sachin.kamat@samsung.com> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Jingoo Han <jg1.han@samsung.com> Acked-by: Jingoo Han <jg1.han@samsung.com> Right, we would get the generic PHYs first, then get the older usb-phys. Then, the older one will be removed from the kernel. Thank you. Best regards, Jingoo Han > --- > > Based on 'usb-next' branch. > Resending it after adding 'Reported-by' tag. > > drivers/usb/host/ehci-exynos.c | 40 +++++++++++++++++----------------- > drivers/usb/host/ohci-exynos.c | 47 +++++++++++++++++++--------------------- > 2 files changed, 42 insertions(+), 45 deletions(-) > > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index cda0a2f..2eed9a4 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -62,18 +62,6 @@ static int exynos_ehci_get_phy(struct device *dev, > int phy_number; > int ret = 0; > > - exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > - if (IS_ERR(exynos_ehci->phy)) { > - ret = PTR_ERR(exynos_ehci->phy); > - if (ret != -ENXIO && ret != -ENODEV) { > - dev_err(dev, "no usb2 phy configured\n"); > - return ret; > - } > - dev_dbg(dev, "Failed to get usb2 phy\n"); > - } else { > - exynos_ehci->otg = exynos_ehci->phy->otg; > - } > - > for_each_available_child_of_node(dev->of_node, child) { > ret = of_property_read_u32(child, "reg", &phy_number); > if (ret) { > @@ -90,15 +78,27 @@ static int exynos_ehci_get_phy(struct device *dev, > > phy = devm_of_phy_get(dev, child, NULL); > of_node_put(child); > - if (IS_ERR(phy)) { > - ret = PTR_ERR(phy); > - if (ret != -ENOSYS && ret != -ENODEV) { > - dev_err(dev, "no usb2 phy configured\n"); > - return ret; > - } > - dev_dbg(dev, "Failed to get usb2 phy\n"); > - } > + if (IS_ERR(phy)) > + /* Lets fallback to older USB-PHYs */ > + goto usb_phy_old; > exynos_ehci->phy_g[phy_number] = phy; > + /* Make the older PHYs unavailable */ > + exynos_ehci->phy = ERR_PTR(-ENXIO); > + } > + > + return 0; > + > +usb_phy_old: > + exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > + if (IS_ERR(exynos_ehci->phy)) { > + ret = PTR_ERR(exynos_ehci->phy); > + if (ret != -ENXIO && ret != -ENODEV) { > + dev_err(dev, "no usb2 phy configured\n"); > + return ret; > + } > + dev_dbg(dev, "Failed to get usb2 phy\n"); > + } else { > + exynos_ehci->otg = exynos_ehci->phy->otg; > } > > return ret; > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c > index a72ab8f..7c48e3f 100644 > --- a/drivers/usb/host/ohci-exynos.c > +++ b/drivers/usb/host/ohci-exynos.c > @@ -51,27 +51,12 @@ static int exynos_ohci_get_phy(struct device *dev, > int phy_number; > int ret = 0; > > - exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > - if (IS_ERR(exynos_ohci->phy)) { > - ret = PTR_ERR(exynos_ohci->phy); > - if (ret != -ENXIO && ret != -ENODEV) { > - dev_err(dev, "no usb2 phy configured\n"); > - return ret; > - } > - dev_dbg(dev, "Failed to get usb2 phy\n"); > - } else { > - exynos_ohci->otg = exynos_ohci->phy->otg; > - } > - > /* > * Getting generic phy: > * We are keeping both types of phys as a part of transiting OHCI > * to generic phy framework, so as to maintain backward compatibilty > - * with old DTB. > - * If there are existing devices using DTB files built from them, > - * to remove the support for old bindings in this driver, > - * we need to make sure that such devices have their DTBs > - * updated to ones built from new DTS. > + * with old DTB too. > + * We fallback to older USB-PHYs when we fail to get generic PHYs. > */ > for_each_available_child_of_node(dev->of_node, child) { > ret = of_property_read_u32(child, "reg", &phy_number); > @@ -89,15 +74,27 @@ static int exynos_ohci_get_phy(struct device *dev, > > phy = devm_of_phy_get(dev, child, NULL); > of_node_put(child); > - if (IS_ERR(phy)) { > - ret = PTR_ERR(phy); > - if (ret != -ENOSYS && ret != -ENODEV) { > - dev_err(dev, "no usb2 phy configured\n"); > - return ret; > - } > - dev_dbg(dev, "Failed to get usb2 phy\n"); > - } > + if (IS_ERR(phy)) > + /* Lets fallback to older USB-PHYs */ > + goto usb_phy_old; > exynos_ohci->phy_g[phy_number] = phy; > + /* Make the older PHYs unavailable */ > + exynos_ohci->phy = ERR_PTR(-ENXIO); > + } > + > + return 0; > + > +usb_phy_old: > + exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > + if (IS_ERR(exynos_ohci->phy)) { > + ret = PTR_ERR(exynos_ohci->phy); > + if (ret != -ENXIO && ret != -ENODEV) { > + dev_err(dev, "no usb2 phy configured\n"); > + return ret; > + } > + dev_dbg(dev, "Failed to get usb2 phy\n"); > + } else { > + exynos_ohci->otg = exynos_ohci->phy->otg; > } > > return ret; > -- > 1.7.10.4
Hi Vivek, On Tue, Aug 5, 2014 at 4:09 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote: > Since we want to keep support for both older usb-phys as well as the > newer generic phys, lets first get the generic PHYs and fallback to > older USB-PHYs only when we fail to get the former. > This should fix the issue with ehci-exynos and ohci-exynos, wherein > in the absence of SAMSUNG_USB2PHY config symbol, we end up getting > the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs > are not configured properly. > > Reported-by: Sachin Kamat <sachin.kamat@samsung.com> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Jingoo Han <jg1.han@samsung.com> Tested this patch on Exynos5250 based Arndale and Snow boards. Fixes the said issue. Thanks. Tested-by: Sachin Kamat <sachin.kamat@samsung.com>
Hi Jingoo, On Tuesday, August 05, 2014 4:28 PM, Jingoo Han wrote: > On Tuesday, August 05, 2014 7:39 PM, Vivek Gautam wrote: >> >> Since we want to keep support for both older usb-phys as well as the >> newer generic phys, lets first get the generic PHYs and fallback to >> older USB-PHYs only when we fail to get the former. >> This should fix the issue with ehci-exynos and ohci-exynos, wherein >> in the absence of SAMSUNG_USB2PHY config symbol, we end up getting >> the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs >> are not configured properly. >> >> Reported-by: Sachin Kamat <sachin.kamat@samsung.com> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Jingoo Han <jg1.han@samsung.com> > > Acked-by: Jingoo Han <jg1.han@samsung.com> > > Right, we would get the generic PHYs first, then get the older > usb-phys. Then, the older one will be removed from the kernel. Right, I have a set of patches to remove the older phy support for Exynos SoC series, keeping the support for S3C64XX, which I shall send soon. [snip] Thanks Vivek Gautam
On Tue, 5 Aug 2014, Vivek Gautam wrote: > Since we want to keep support for both older usb-phys as well as the > newer generic phys, lets first get the generic PHYs and fallback to > older USB-PHYs only when we fail to get the former. > This should fix the issue with ehci-exynos and ohci-exynos, wherein > in the absence of SAMSUNG_USB2PHY config symbol, we end up getting > the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs > are not configured properly. > > Reported-by: Sachin Kamat <sachin.kamat@samsung.com> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > > Based on 'usb-next' branch. > Resending it after adding 'Reported-by' tag. > > drivers/usb/host/ehci-exynos.c | 40 +++++++++++++++++----------------- > drivers/usb/host/ohci-exynos.c | 47 +++++++++++++++++++--------------------- > 2 files changed, 42 insertions(+), 45 deletions(-) > > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > index cda0a2f..2eed9a4 100644 > --- a/drivers/usb/host/ehci-exynos.c > +++ b/drivers/usb/host/ehci-exynos.c > @@ -62,18 +62,6 @@ static int exynos_ehci_get_phy(struct device *dev, > int phy_number; > int ret = 0; > > - exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); > - if (IS_ERR(exynos_ehci->phy)) { > - ret = PTR_ERR(exynos_ehci->phy); > - if (ret != -ENXIO && ret != -ENODEV) { > - dev_err(dev, "no usb2 phy configured\n"); > - return ret; > - } > - dev_dbg(dev, "Failed to get usb2 phy\n"); > - } else { > - exynos_ehci->otg = exynos_ehci->phy->otg; > - } > - > for_each_available_child_of_node(dev->of_node, child) { > ret = of_property_read_u32(child, "reg", &phy_number); > if (ret) { > @@ -90,15 +78,27 @@ static int exynos_ehci_get_phy(struct device *dev, What about the code that you skipped here? Suppose the of_property_read_32() call returns an error. Do you then want to abort the whole thing, or should you go on to try the old usb_phy? Alan Stern
Hi Alan, On Tue, Aug 5, 2014 at 7:41 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Tue, 5 Aug 2014, Vivek Gautam wrote: > >> Since we want to keep support for both older usb-phys as well as the >> newer generic phys, lets first get the generic PHYs and fallback to >> older USB-PHYs only when we fail to get the former. >> This should fix the issue with ehci-exynos and ohci-exynos, wherein >> in the absence of SAMSUNG_USB2PHY config symbol, we end up getting >> the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs >> are not configured properly. >> >> Reported-by: Sachin Kamat <sachin.kamat@samsung.com> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> Cc: Alan Stern <stern@rowland.harvard.edu> >> Cc: Jingoo Han <jg1.han@samsung.com> >> --- >> >> Based on 'usb-next' branch. >> Resending it after adding 'Reported-by' tag. >> >> drivers/usb/host/ehci-exynos.c | 40 +++++++++++++++++----------------- >> drivers/usb/host/ohci-exynos.c | 47 +++++++++++++++++++--------------------- >> 2 files changed, 42 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c >> index cda0a2f..2eed9a4 100644 >> --- a/drivers/usb/host/ehci-exynos.c >> +++ b/drivers/usb/host/ehci-exynos.c >> @@ -62,18 +62,6 @@ static int exynos_ehci_get_phy(struct device *dev, >> int phy_number; >> int ret = 0; >> >> - exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); >> - if (IS_ERR(exynos_ehci->phy)) { >> - ret = PTR_ERR(exynos_ehci->phy); >> - if (ret != -ENXIO && ret != -ENODEV) { >> - dev_err(dev, "no usb2 phy configured\n"); >> - return ret; >> - } >> - dev_dbg(dev, "Failed to get usb2 phy\n"); >> - } else { >> - exynos_ehci->otg = exynos_ehci->phy->otg; >> - } >> - >> for_each_available_child_of_node(dev->of_node, child) { >> ret = of_property_read_u32(child, "reg", &phy_number); >> if (ret) { >> @@ -90,15 +78,27 @@ static int exynos_ehci_get_phy(struct device *dev, > > What about the code that you skipped here? Suppose the > of_property_read_32() call returns an error. Do you then want to abort > the whole thing, or should you go on to try the old usb_phy? I have posted a patch-series [1] which cleans up the entire samsug usb2 phy from older usb-phy layer, and further removes the support for these older usb-phy from ehci-exynos and ohci-exynos. Please let me know your comments. [1]: [PATCH 0/7] usb-phy: samsung: Cleanup the unused drivers
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index cda0a2f..2eed9a4 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -62,18 +62,6 @@ static int exynos_ehci_get_phy(struct device *dev, int phy_number; int ret = 0; - exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); - if (IS_ERR(exynos_ehci->phy)) { - ret = PTR_ERR(exynos_ehci->phy); - if (ret != -ENXIO && ret != -ENODEV) { - dev_err(dev, "no usb2 phy configured\n"); - return ret; - } - dev_dbg(dev, "Failed to get usb2 phy\n"); - } else { - exynos_ehci->otg = exynos_ehci->phy->otg; - } - for_each_available_child_of_node(dev->of_node, child) { ret = of_property_read_u32(child, "reg", &phy_number); if (ret) { @@ -90,15 +78,27 @@ static int exynos_ehci_get_phy(struct device *dev, phy = devm_of_phy_get(dev, child, NULL); of_node_put(child); - if (IS_ERR(phy)) { - ret = PTR_ERR(phy); - if (ret != -ENOSYS && ret != -ENODEV) { - dev_err(dev, "no usb2 phy configured\n"); - return ret; - } - dev_dbg(dev, "Failed to get usb2 phy\n"); - } + if (IS_ERR(phy)) + /* Lets fallback to older USB-PHYs */ + goto usb_phy_old; exynos_ehci->phy_g[phy_number] = phy; + /* Make the older PHYs unavailable */ + exynos_ehci->phy = ERR_PTR(-ENXIO); + } + + return 0; + +usb_phy_old: + exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + if (IS_ERR(exynos_ehci->phy)) { + ret = PTR_ERR(exynos_ehci->phy); + if (ret != -ENXIO && ret != -ENODEV) { + dev_err(dev, "no usb2 phy configured\n"); + return ret; + } + dev_dbg(dev, "Failed to get usb2 phy\n"); + } else { + exynos_ehci->otg = exynos_ehci->phy->otg; } return ret; diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index a72ab8f..7c48e3f 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -51,27 +51,12 @@ static int exynos_ohci_get_phy(struct device *dev, int phy_number; int ret = 0; - exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); - if (IS_ERR(exynos_ohci->phy)) { - ret = PTR_ERR(exynos_ohci->phy); - if (ret != -ENXIO && ret != -ENODEV) { - dev_err(dev, "no usb2 phy configured\n"); - return ret; - } - dev_dbg(dev, "Failed to get usb2 phy\n"); - } else { - exynos_ohci->otg = exynos_ohci->phy->otg; - } - /* * Getting generic phy: * We are keeping both types of phys as a part of transiting OHCI * to generic phy framework, so as to maintain backward compatibilty - * with old DTB. - * If there are existing devices using DTB files built from them, - * to remove the support for old bindings in this driver, - * we need to make sure that such devices have their DTBs - * updated to ones built from new DTS. + * with old DTB too. + * We fallback to older USB-PHYs when we fail to get generic PHYs. */ for_each_available_child_of_node(dev->of_node, child) { ret = of_property_read_u32(child, "reg", &phy_number); @@ -89,15 +74,27 @@ static int exynos_ohci_get_phy(struct device *dev, phy = devm_of_phy_get(dev, child, NULL); of_node_put(child); - if (IS_ERR(phy)) { - ret = PTR_ERR(phy); - if (ret != -ENOSYS && ret != -ENODEV) { - dev_err(dev, "no usb2 phy configured\n"); - return ret; - } - dev_dbg(dev, "Failed to get usb2 phy\n"); - } + if (IS_ERR(phy)) + /* Lets fallback to older USB-PHYs */ + goto usb_phy_old; exynos_ohci->phy_g[phy_number] = phy; + /* Make the older PHYs unavailable */ + exynos_ohci->phy = ERR_PTR(-ENXIO); + } + + return 0; + +usb_phy_old: + exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + if (IS_ERR(exynos_ohci->phy)) { + ret = PTR_ERR(exynos_ohci->phy); + if (ret != -ENXIO && ret != -ENODEV) { + dev_err(dev, "no usb2 phy configured\n"); + return ret; + } + dev_dbg(dev, "Failed to get usb2 phy\n"); + } else { + exynos_ohci->otg = exynos_ohci->phy->otg; } return ret;
Since we want to keep support for both older usb-phys as well as the newer generic phys, lets first get the generic PHYs and fallback to older USB-PHYs only when we fail to get the former. This should fix the issue with ehci-exynos and ohci-exynos, wherein in the absence of SAMSUNG_USB2PHY config symbol, we end up getting the NOP_USB_XCEIV phy when the same is enabled. And thus the PHYs are not configured properly. Reported-by: Sachin Kamat <sachin.kamat@samsung.com> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Jingoo Han <jg1.han@samsung.com> --- Based on 'usb-next' branch. Resending it after adding 'Reported-by' tag. drivers/usb/host/ehci-exynos.c | 40 +++++++++++++++++----------------- drivers/usb/host/ohci-exynos.c | 47 +++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 45 deletions(-)