[U-Boot] [PATCH v3 0/2] usb: ehci: exynos: Fixes for non-fdt systems

Based on 'master' branch of u-boot-samsung.
V3 for patch-series "[PATCH v2 0/2] usb: ehci: exynos: Enable non-dt path". Changing the series title to "usb: ehci: exynos: Fixes for non-fdt systems" to avoid confision with the patch included.
Changes from v2: - Using hcd as "struct ehci_hccr *" rather than "unsigned int" to avoid unnecessary typecasting later in the code. - Further using local variable of type "fdt_addr_t" in exynos_usb_parse_dt() and modifying the code as required.
Changes from v1: - Added patch to fix problem of multiple FDT decode. - Addressing warnings when compiling non-dt way.
Vivek Gautam (2): usb: ehci: exynos: Fix multiple FDT decode usb: ehci: exynos: Enable non-dt path
drivers/usb/host/ehci-exynos.c | 50 ++++++++++++++++----------------------- 1 files changed, 21 insertions(+), 29 deletions(-)

With current FDT support driver tries to parse device node twice in ehci_hcd_init() and ehci_hcd_stop(), which shouldn't happen ideally. Making provision to store data in a global structure and thereby passing its pointer when needed.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- drivers/usb/host/ehci-exynos.c | 43 +++++++++++++--------------------------- 1 files changed, 14 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 3ca4c5c..c6b7a5e 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -42,11 +42,14 @@ DECLARE_GLOBAL_DATA_PTR; */ struct exynos_ehci { struct exynos_usb_phy *usb; - unsigned int *hcd; + struct ehci_hccr *hcd; };
+static struct exynos_ehci exynos; + static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) { + fdt_addr_t addr; unsigned int node; int depth;
@@ -59,12 +62,14 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) /* * Get the base address for EHCI controller from the device node */ - exynos->hcd = (unsigned int *)fdtdec_get_addr(blob, node, "reg"); - if (exynos->hcd == NULL) { + addr = fdtdec_get_addr(blob, node, "reg"); + if (addr == FDT_ADDR_T_NONE) { debug("Can't get the EHCI register address\n"); return -ENXIO; }
+ exynos->hcd = (struct ehci_hccr *)addr; + depth = 0; node = fdtdec_next_compatible_subnode(blob, node, COMPAT_SAMSUNG_EXYNOS_USB_PHY, &depth); @@ -144,20 +149,13 @@ static void reset_usb_phy(struct exynos_usb_phy *usb) */ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { - struct exynos_ehci *exynos = NULL; + struct exynos_ehci *ctx = &exynos;
- exynos = (struct exynos_ehci *) - kzalloc(sizeof(struct exynos_ehci), GFP_KERNEL); - if (!exynos) { - debug("failed to allocate exynos ehci context\n"); - return -ENOMEM; - } + exynos_usb_parse_dt(gd->fdt_blob, ctx);
- exynos_usb_parse_dt(gd->fdt_blob, exynos); + setup_usb_phy(ctx->usb);
- setup_usb_phy(exynos->usb); - - *hccr = (struct ehci_hccr *)(exynos->hcd); + *hccr = (ctx->hcd); *hcor = (struct ehci_hcor *)((uint32_t) *hccr + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
@@ -165,8 +163,6 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) (uint32_t)*hccr, (uint32_t)*hcor, (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
- kfree(exynos); - return 0; }
@@ -176,20 +172,9 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) */ int ehci_hcd_stop(int index) { - struct exynos_ehci *exynos = NULL; - - exynos = (struct exynos_ehci *) - kzalloc(sizeof(struct exynos_ehci), GFP_KERNEL); - if (!exynos) { - debug("failed to allocate exynos ehci context\n"); - return -ENOMEM; - } - - exynos_usb_parse_dt(gd->fdt_blob, exynos); - - reset_usb_phy(exynos->usb); + struct exynos_ehci *ctx = &exynos;
- kfree(exynos); + reset_usb_phy(ctx->usb);
return 0; }

Hi,
On Tue, Mar 5, 2013 at 9:54 PM, Vivek Gautam gautam.vivek@samsung.com wrote:
With current FDT support driver tries to parse device node twice in ehci_hcd_init() and ehci_hcd_stop(), which shouldn't happen ideally. Making provision to store data in a global structure and thereby passing its pointer when needed.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Please can you add a list of changes in each revision? You could try using patman if you like (tools/patman).
drivers/usb/host/ehci-exynos.c | 43 +++++++++++++--------------------------- 1 files changed, 14 insertions(+), 29 deletions(-)
Regards, Simon

Hi Vivek,
On Tue, Mar 5, 2013 at 9:54 PM, Vivek Gautam gautam.vivek@samsung.com wrote:
With current FDT support driver tries to parse device node twice in ehci_hcd_init() and ehci_hcd_stop(), which shouldn't happen ideally. Making provision to store data in a global structure and thereby passing its pointer when needed.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
drivers/usb/host/ehci-exynos.c | 43 +++++++++++++--------------------------- 1 files changed, 14 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 3ca4c5c..c6b7a5e 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -42,11 +42,14 @@ DECLARE_GLOBAL_DATA_PTR; */ struct exynos_ehci { struct exynos_usb_phy *usb;
unsigned int *hcd;
struct ehci_hccr *hcd;
};
+static struct exynos_ehci exynos;
static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) {
fdt_addr_t addr; unsigned int node; int depth;
@@ -59,12 +62,14 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) /* * Get the base address for EHCI controller from the device node */
exynos->hcd = (unsigned int *)fdtdec_get_addr(blob, node, "reg");
if (exynos->hcd == NULL) {
addr = fdtdec_get_addr(blob, node, "reg");
if (addr == FDT_ADDR_T_NONE) { debug("Can't get the EHCI register address\n"); return -ENXIO; }
exynos->hcd = (struct ehci_hccr *)addr;
depth = 0; node = fdtdec_next_compatible_subnode(blob, node, COMPAT_SAMSUNG_EXYNOS_USB_PHY, &depth);
@@ -144,20 +149,13 @@ static void reset_usb_phy(struct exynos_usb_phy *usb) */ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) {
struct exynos_ehci *exynos = NULL;
struct exynos_ehci *ctx = &exynos;
exynos = (struct exynos_ehci *)
kzalloc(sizeof(struct exynos_ehci), GFP_KERNEL);
if (!exynos) {
debug("failed to allocate exynos ehci context\n");
return -ENOMEM;
}
exynos_usb_parse_dt(gd->fdt_blob, ctx);
Need to check for error return here.
exynos_usb_parse_dt(gd->fdt_blob, exynos);
setup_usb_phy(ctx->usb);
setup_usb_phy(exynos->usb);
*hccr = (struct ehci_hccr *)(exynos->hcd);
*hccr = (ctx->hcd);
Remove ()
*hcor = (struct ehci_hcor *)((uint32_t) *hccr + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
@@ -165,8 +163,6 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) (uint32_t)*hccr, (uint32_t)*hcor, (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
kfree(exynos);
return 0;
}
@@ -176,20 +172,9 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) */ int ehci_hcd_stop(int index) {
struct exynos_ehci *exynos = NULL;
exynos = (struct exynos_ehci *)
kzalloc(sizeof(struct exynos_ehci), GFP_KERNEL);
if (!exynos) {
debug("failed to allocate exynos ehci context\n");
return -ENOMEM;
}
exynos_usb_parse_dt(gd->fdt_blob, exynos);
reset_usb_phy(exynos->usb);
struct exynos_ehci *ctx = &exynos;
kfree(exynos);
reset_usb_phy(ctx->usb); return 0;
}
1.7.6.5
Regards, Simon

Enabling the non-dt path for the driver so that we don't get any build errors for non-dt configuration.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- drivers/usb/host/ehci-exynos.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index c6b7a5e..6de23ec 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -47,6 +47,7 @@ struct exynos_ehci {
static struct exynos_ehci exynos;
+#ifdef CONFIG_OF_CONTROL static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) { fdt_addr_t addr; @@ -90,6 +91,7 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos)
return 0; } +#endif
/* Setup the EHCI host controller. */ static void setup_usb_phy(struct exynos_usb_phy *usb) @@ -151,7 +153,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { struct exynos_ehci *ctx = &exynos;
+#ifdef CONFIG_OF_CONTROL exynos_usb_parse_dt(gd->fdt_blob, ctx); +#else + ctx->usb = (struct exynos_usb_phy *)samsung_get_base_usb_phy(); + ctx->hcd = (struct ehci_hccr *)samsung_get_base_usb_ehci(); +#endif
setup_usb_phy(ctx->usb);

On Tue, Mar 5, 2013 at 9:54 PM, Vivek Gautam gautam.vivek@samsung.com wrote:
Enabling the non-dt path for the driver so that we don't get any build errors for non-dt configuration.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Change log aside:
Acked-by: Simon Glass sjg@chromium.org
drivers/usb/host/ehci-exynos.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index c6b7a5e..6de23ec 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -47,6 +47,7 @@ struct exynos_ehci {
static struct exynos_ehci exynos;
+#ifdef CONFIG_OF_CONTROL static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) { fdt_addr_t addr; @@ -90,6 +91,7 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos)
return 0;
} +#endif
/* Setup the EHCI host controller. */ static void setup_usb_phy(struct exynos_usb_phy *usb) @@ -151,7 +153,12 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { struct exynos_ehci *ctx = &exynos;
+#ifdef CONFIG_OF_CONTROL exynos_usb_parse_dt(gd->fdt_blob, ctx); +#else
ctx->usb = (struct exynos_usb_phy *)samsung_get_base_usb_phy();
ctx->hcd = (struct ehci_hccr *)samsung_get_base_usb_ehci();
+#endif
setup_usb_phy(ctx->usb);
-- 1.7.6.5

Hi Vivek,
On Tue, Mar 5, 2013 at 9:54 PM, Vivek Gautam gautam.vivek@samsung.com wrote:
Based on 'master' branch of u-boot-samsung.
V3 for patch-series "[PATCH v2 0/2] usb: ehci: exynos: Enable non-dt path". Changing the series title to "usb: ehci: exynos: Fixes for non-fdt systems" to avoid confision with the patch included.
Changes from v2:
- Using hcd as "struct ehci_hccr *" rather than "unsigned int" to avoid unnecessary typecasting later in the code.
- Further using local variable of type "fdt_addr_t" in exynos_usb_parse_dt() and modifying the code as required.
OK I see the change log here. Still, it is good to have it with each patch.
Regards, Simon
participants (2)
-
Simon Glass
-
Vivek Gautam