diff mbox

[v3,1/2] usb: ehci: exynos: Fix multiple FDT decode

Message ID 1362549243-20587-2-git-send-email-gautam.vivek@samsung.com
State New
Headers show

Commit Message

Vivek Gautam March 6, 2013, 5:54 a.m. UTC 
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(-)

Comments

Simon Glass March 6, 2013, 6:11 a.m. UTC | #1 
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
Simon Glass March 6, 2013, 6:16 a.m. UTC | #2 
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
diff mbox

Patch

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;
 }