Re: [PATCH RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi, On Wed, Apr 16, 2014 at 7:42 PM, Heikki Krogerus heikki.kroge...@linux.intel.com wrote: Hi, On Tue, Apr 15, 2014 at 06:24:11PM +0530, Vivek Gautam wrote: I had seen your patches in the mailing list, but i don't see any updated version of these patches. Are you planning to work on this above mentioned patch-series any time soon ? I'm sorry, I forgot this completely. I have not prepared new version of those patches as the drivers I need them for are not ready yet, but I guess I can also use this case as justification for them. Or, should i try to find a different approach for handling the phy from the host controller (child of DWC3 in this case, which has the phys). Well, there is now an issue with the lookup method I'm suggesting in this case. Since the device ID is now generated automatically for xhci-hcd in dwc3, we don't know the xhci-hcd device name before platform_device_add(), and that is too late. True, the xhci-hcd are getting AUTO ID, so it might not be possible to get their device names in dwc3. I don't see why the device could not be named when platform_device_alloc() is called, so I think I'll suggest something like the attached patch to fix this issue Ok, i had a look at the patch, and it looks promising. In any case, I'll send an updated version of the phy patches soon. Thanks for your efforts. -- 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 RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Apr 15, 2014 at 06:24:11PM +0530, Vivek Gautam wrote:
I had seen your patches in the mailing list, but i don't see any
updated version of these patches.
Are you planning to work on this above mentioned patch-series any time soon ?
I'm sorry, I forgot this completely. I have not prepared new version
of those patches as the drivers I need them for are not ready yet, but
I guess I can also use this case as justification for them.
Or, should i try to find a different approach for handling the phy
from the host controller (child of DWC3 in this case, which has the
phys).
Well, there is now an issue with the lookup method I'm suggesting in
this case. Since the device ID is now generated automatically for
xhci-hcd in dwc3, we don't know the xhci-hcd device name before
platform_device_add(), and that is too late. I don't see why the
device could not be named when platform_device_alloc() is called, so I
think I'll suggest something like the attached patch to fix this
issue.
In any case, I'll send an updated version of the phy patches soon.
Thanks,
--
heikki
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e714709..13f8edb 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -169,11 +169,47 @@ struct platform_object {
*/
void platform_device_put(struct platform_device *pdev)
{
- if (pdev)
- put_device(pdev-dev);
+ if (!pdev)
+ return;
+
+ if (pdev-id_auto) {
+ ida_simple_remove(platform_devid_ida, pdev-id);
+ pdev-id = PLATFORM_DEVID_AUTO;
+ }
+
+ put_device(pdev-dev);
}
EXPORT_SYMBOL_GPL(platform_device_put);
+static int pdev_set_name(struct platform_device *pdev)
+{
+ int ret;
+
+ switch (pdev-id) {
+ default:
+ dev_set_name(pdev-dev, %s.%d, pdev-name, pdev-id);
+ break;
+ case PLATFORM_DEVID_NONE:
+ dev_set_name(pdev-dev, %s, pdev-name);
+ break;
+ case PLATFORM_DEVID_AUTO:
+ /*
+ * Automatically allocated device ID. We mark it as such so
+ * that we remember it must be freed, and we append a suffix
+ * to avoid namespace collision with explicit IDs.
+ */
+ ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
+ if (ret 0)
+ return ret;
+ pdev-id = ret;
+ pdev-id_auto = true;
+ dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
+ break;
+ }
+
+ return 0;
+}
+
static void platform_device_release(struct device *dev)
{
struct platform_object *pa = container_of(dev, struct platform_object,
@@ -206,6 +242,10 @@ struct platform_device *platform_device_alloc(const char *name, int id)
device_initialize(pa-pdev.dev);
pa-pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(pa-pdev);
+ if (pdev_set_name(pa-pdev)) {
+ kfree(pa);
+ return NULL;
+ }
}
return pa ? pa-pdev : NULL;
@@ -286,28 +326,6 @@ int platform_device_add(struct platform_device *pdev)
pdev-dev.bus = platform_bus_type;
- switch (pdev-id) {
- default:
- dev_set_name(pdev-dev, %s.%d, pdev-name, pdev-id);
- break;
- case PLATFORM_DEVID_NONE:
- dev_set_name(pdev-dev, %s, pdev-name);
- break;
- case PLATFORM_DEVID_AUTO:
- /*
- * Automatically allocated device ID. We mark it as such so
- * that we remember it must be freed, and we append a suffix
- * to avoid namespace collision with explicit IDs.
- */
- ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
- if (ret 0)
- goto err_out;
- pdev-id = ret;
- pdev-id_auto = true;
- dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
- break;
- }
-
for (i = 0; i pdev-num_resources; i++) {
struct resource *p, *r = pdev-resource[i];
@@ -350,7 +368,6 @@ int platform_device_add(struct platform_device *pdev)
release_resource(r);
}
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(platform_device_add);
@@ -370,11 +387,6 @@ void platform_device_del(struct platform_device *pdev)
if (pdev) {
device_del(pdev-dev);
- if (pdev-id_auto) {
- ida_simple_remove(platform_devid_ida, pdev-id);
- pdev-id = PLATFORM_DEVID_AUTO;
- }
-
for (i = 0; i pdev-num_resources; i++) {
struct resource *r = pdev-resource[i];
unsigned long type = resource_type(r);
@@ -392,8 +404,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
*/
int platform_device_register(struct platform_device *pdev)
{
+ int ret;
+
device_initialize(pdev-dev);
arch_setup_pdev_archdata(pdev);
+
+ ret = pdev_set_name(pdev);
+ if (ret)
+ return ret;
+
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
Re: [PATCH RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi Heikki,
On Tue, Dec 10, 2013 at 7:25 PM, Heikki Krogerus
heikki.kroge...@linux.intel.com wrote:
Giving life to this thread after long time.
Hi,
On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
@@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
/*
+ * The parent of the xhci-plat device may pass in a PHY via
+ * platform data. If it exists, store it in our struct usb_hcd
+ * so that we can use it later.
+ */
+ phy_generic = dev_get_platdata(pdev-dev);
+ if (phy_generic)
+ xhci-shared_hcd-phy_generic = *phy_generic;
Getting the handle to the phy from platform data like this is not
going to work for long. It should be possible to get it normally with
phy_get(). It's not going to be possible to get the handle from the
platform data like this if the xhci-hcd platform device is created
from ACPI or DT. You are also not considering case where you have two
phys.
Vivek, I have made a patch set for the phy framework allowing
associations between the phys and their users to be made in same way
gpios and clk make them. With those you should be able to create a
lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
could use phy_get() here already. Please check them. Subject of the
thread:
phy: remove the need for the phys to know about their users
I had seen your patches in the mailing list, but i don't see any
updated version of these patches.
Are you planning to work on this above mentioned patch-series any time soon ?
Or, should i try to find a different approach for handling the phy
from the host controller (child of DWC3 in this case, which has the
phys).
The lookup table can then be added in drivers/usb/dwc3/host.c with
something like this:
int dwc3_host_init(struct dwc3 *dwc)
{
struct platform_device *xhci;
struct phy_lookup_table *table;
...
table-dev_id = dev_name(xhci-dev);
if (dwc-usb2_generic_phy)
table-table[0].phy_name =
dev_name(dwc-usb2_generic_phy-dev);
if (dwc-usb3_generic_phy)
table-table[1].phy_name =
dev_name(dwc-usb3_generic_phy-dev);
phy_add_lookup_table(table);
...
--
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 RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
The DWC3-exynos eXtensible host controller on Exynos5420 SoC
is quirky in a way that the PHY needs to be tuned to get it
working at SuperSpeed.
Add relevant calls for tuning the PHY for DWC3-Exynos's
host controller, for that matter passing just USB3 PHY
from DWC3 core, which is saved in secondary HCD of XHCI.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/usb/dwc3/host.c |7 ++
drivers/usb/host/xhci-plat.c | 43 -
include/linux/usb/hcd.h |1 +
3 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 32db328..cc1f6ff 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -46,6 +46,13 @@ int dwc3_host_init(struct dwc3 *dwc)
goto err1;
}
+ ret = platform_device_add_data(xhci, dwc-usb3_generic_phy,
+ sizeof(dwc-usb3_generic_phy));
+ if (ret) {
+ dev_err(dwc-dev, failed to add platform data\n);
+ goto err1;
+ }
+
ret = platform_device_add(xhci);
if (ret) {
dev_err(dwc-dev, failed to register xHCI device\n);
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 395c9e9..a0f3cbc 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@
#include linux/slab.h
#include linux/of.h
#include linux/dma-mapping.h
+#include linux/phy/phy.h
#include xhci.h
@@ -51,7 +52,24 @@ static void xhci_plat_quirks(struct device *dev, struct
xhci_hcd *xhci)
/* called during probe() after chip reset completes */
static int xhci_plat_setup(struct usb_hcd *hcd)
{
- return xhci_gen_setup(hcd, xhci_plat_quirks);
+ struct xhci_hcd *xhci;
+ int ret = 0;
+
+ ret = xhci_gen_setup(hcd, xhci_plat_quirks);
+ if (ret) {
+ dev_err(hcd-self.controller, xhci setup failed\n);
+ goto err0;
+ }
+
+ /* Valid for secondary HCD only which gives SuperSpeed ports */
+ if (!usb_hcd_is_primary_hcd(hcd)) {
+ xhci = hcd_to_xhci(hcd);
+ if (xhci-quirks XHCI_DWC3_EXYNOS)
+ phy_tune(hcd-phy_generic);
+ }
+
+err0:
+ return ret;
}
static const struct hc_driver xhci_plat_xhci_driver = {
@@ -111,6 +129,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
struct usb_hcd *hcd;
int ret;
int irq;
+ struct phy **phy_generic;
if (usb_disabled())
return -ENODEV;
@@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
/*
+* The parent of the xhci-plat device may pass in a PHY via
+* platform data. If it exists, store it in our struct usb_hcd
+* so that we can use it later.
+*/
+ phy_generic = dev_get_platdata(pdev-dev);
+ if (phy_generic)
+ xhci-shared_hcd-phy_generic = *phy_generic;
+
+ /*
* Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)
* is called by usb_add_hcd().
*/
@@ -229,8 +257,19 @@ static int xhci_plat_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ int ret;
+
+ ret = xhci_resume(xhci, 0);
+ if (ret)
+ return ret;
- return xhci_resume(xhci, 0);
+ /* Valid for secondary HCD only which gives SuperSpeed ports */
+ if (!usb_hcd_is_primary_hcd(hcd)) {
+ if (xhci-quirks XHCI_DWC3_EXYNOS)
+ phy_tune(hcd-phy_generic);
+ }
+
+ return 0;
}
static const struct dev_pm_ops xhci_plat_pm_ops = {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index b8aba19..241ed2b 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -107,6 +107,7 @@ struct usb_hcd {
* other external phys should be software-transparent
*/
struct usb_phy *phy;
+ struct phy *phy_generic;
/* Flags that need to be manipulated atomically because they can
* change while the host controller is running. Always use
--
1.7.6.5
--
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 RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
@@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
/*
+ * The parent of the xhci-plat device may pass in a PHY via
+ * platform data. If it exists, store it in our struct usb_hcd
+ * so that we can use it later.
+ */
+ phy_generic = dev_get_platdata(pdev-dev);
+ if (phy_generic)
+ xhci-shared_hcd-phy_generic = *phy_generic;
Getting the handle to the phy from platform data like this is not
going to work for long. It should be possible to get it normally with
phy_get(). It's not going to be possible to get the handle from the
platform data like this if the xhci-hcd platform device is created
from ACPI or DT. You are also not considering case where you have two
phys.
Vivek, I have made a patch set for the phy framework allowing
associations between the phys and their users to be made in same way
gpios and clk make them. With those you should be able to create a
lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
could use phy_get() here already. Please check them. Subject of the
thread:
phy: remove the need for the phys to know about their users
The lookup table can then be added in drivers/usb/dwc3/host.c with
something like this:
int dwc3_host_init(struct dwc3 *dwc)
{
struct platform_device *xhci;
struct phy_lookup_table *table;
...
table-dev_id = dev_name(xhci-dev);
if (dwc-usb2_generic_phy)
table-table[0].phy_name =
dev_name(dwc-usb2_generic_phy-dev);
if (dwc-usb3_generic_phy)
table-table[1].phy_name =
dev_name(dwc-usb3_generic_phy-dev);
phy_add_lookup_table(table);
...
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
