Hi,
On Thu, Feb 14, 2013 at 10:22 AM, Simon Glass <[email protected]> wrote: > Hi, > > On Tue, Feb 12, 2013 at 10:26 PM, Vivek Gautam <[email protected]> > 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 <[email protected]> > > Nice patch. > Really sorry for the delay in responding. >> --- >> >> This patch comes up as a fix for earlier problem of multiple FDT >> decode. Actually no 'v1' present for this patch. >> >> drivers/usb/host/ehci-exynos.c | 40 >> +++++++++++----------------------------- >> 1 files changed, 11 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c >> index 3ca4c5c..68f12fc 100644 >> --- a/drivers/usb/host/ehci-exynos.c >> +++ b/drivers/usb/host/ehci-exynos.c >> @@ -42,9 +42,11 @@ DECLARE_GLOBAL_DATA_PTR; >> */ >> struct exynos_ehci { >> struct exynos_usb_phy *usb; >> - unsigned int *hcd; >> + unsigned int hcd; > > I think this should be a pointer. Perhaps a (struct ehci_hccr *? > Isn't it better to maintain it as unsigned int ? Since later when fdtdec_get_addr() is used, which returns u32 or u64, we can easily check against FDT_ADDR_T_NONE. ehci_hcd_init() can later typecast it to (struct ehci_hccr *) when needed. >> }; >> >> +static struct exynos_ehci exynos; >> + >> static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) >> { >> unsigned int node; >> @@ -59,8 +61,8 @@ 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) { >> + exynos->hcd = fdtdec_get_addr(blob, node, "reg"); >> + if (exynos->hcd < 0) { > > You need to check for FDT_ADDR_NONE here. > Sure. >> debug("Can't get the EHCI register address\n"); >> return -ENXIO; >> } >> @@ -144,20 +146,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; >> - >> - exynos = (struct exynos_ehci *) >> - kzalloc(sizeof(struct exynos_ehci), GFP_KERNEL); >> - if (!exynos) { >> - debug("failed to allocate exynos ehci context\n"); >> - return -ENOMEM; >> - } >> + struct exynos_ehci *ctx = &exynos; >> >> - exynos_usb_parse_dt(gd->fdt_blob, exynos); >> + exynos_usb_parse_dt(gd->fdt_blob, ctx); >> >> - setup_usb_phy(exynos->usb); >> + setup_usb_phy(ctx->usb); >> >> - *hccr = (struct ehci_hccr *)(exynos->hcd); >> + *hccr = (struct ehci_hccr *)(ctx->hcd); >> *hcor = (struct ehci_hcor *)((uint32_t) *hccr >> + >> HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase))); >> >> @@ -165,8 +160,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 +169,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 >> > -- Thanks & Regards Vivek _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
