Patchwork [U-Boot,v2,1/2] usb: ehci: exynos: Fix multiple FDT decode

login
register
mail settings
Submitter Vivek Gautam
Date Feb. 13, 2013, 6:26 a.m.
Message ID <1360736808-27209-2-git-send-email-gautam.vivek@samsung.com>
Download mbox | patch
Permalink /patch/220058/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Vivek Gautam - Feb. 13, 2013, 6:26 a.m.
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>
---

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(-)
Simon Glass - Feb. 14, 2013, 4:52 a.m.
Hi,

On Tue, Feb 12, 2013 at 10:26 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>

Nice patch.

> ---
>
> 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 *?

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

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

Regards,
Simon
Vivek Gautam - March 5, 2013, 1:40 p.m.
Hi,


On Thu, Feb 14, 2013 at 10:22 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On Tue, Feb 12, 2013 at 10:26 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>
>
> 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
>>
>
Simon Glass - March 6, 2013, 1:57 a.m.
On Tue, Mar 5, 2013 at 5:40 AM, Vivek Gautam <gautamvivek1987@gmail.com> wrote:
> Hi,
>
>
> On Thu, Feb 14, 2013 at 10:22 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> On Tue, Feb 12, 2013 at 10:26 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>
>>
>> Nice patch.
>>
> Really sorry for the delay in responding.

No problem, we all have other things to do :-)

>
>>> ---
>>>
>>> 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.

How about having a local variable in your decode routine like:

fdt_addr_t addr;

addr = fdtdec_get_addr()...
if (addr == FDT_ADDR_NONE) {
   debug(...)
   return -1;
}
exynos->hcd = (struct ... *)addr;

I think this is better than holding a value that is the wrong type,
and casting it throughout your driver. Best to get the
casting/checking done at init time,

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

Regards,
Simon
Vivek Gautam - March 6, 2013, 4:56 a.m.
Hi,


On Wed, Mar 6, 2013 at 7:27 AM, Simon Glass <sjg@chromium.org> wrote:
> On Tue, Mar 5, 2013 at 5:40 AM, Vivek Gautam <gautamvivek1987@gmail.com> wrote:
>> Hi,
>>
>>
>> On Thu, Feb 14, 2013 at 10:22 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi,
>>>
>>> On Tue, Feb 12, 2013 at 10:26 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>
>>>
>>> Nice patch.
>>>
>> Really sorry for the delay in responding.
>
> No problem, we all have other things to do :-)
>
>>
>>>> ---
>>>>
>>>> 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.
>
> How about having a local variable in your decode routine like:
>
> fdt_addr_t addr;
>

Yeah, seem a nice idea.

> addr = fdtdec_get_addr()...
> if (addr == FDT_ADDR_NONE) {
>    debug(...)
>    return -1;
> }
> exynos->hcd = (struct ... *)addr;
>
> I think this is better than holding a value that is the wrong type,
> and casting it throughout your driver. Best to get the
> casting/checking done at init time,
>

True, will modify this and post it.

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

Patch

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;
 };
 
+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) {
 		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;
 }