Re: [U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
On Tuesday, December 30, 2014 at 02:34:47 PM, Stefano Babic wrote: > On 16/12/2014 14:09, Marek Vasut wrote: > > The malloc() calls are unnecessary, just allocate the stuff on stack. > > While at it, reorder the code a little, so that only one variable is > > used for the text, use snprintf() instead of sprintf() and use %01d > > as a formatting string to avoid any possible overflows. > > > > Signed-off-by: Marek Vasut > > Cc: Igor Grinberg > > Cc: Nikita Kiryanov > > Cc: Sean Cross > > Cc: Simon Glass > > Cc: Stefano Babic > > Cc: Tim Harvey > > --- > > Applied to u-boot-imx, thanks ! Hey! hope you had a nice holiday :) You might want to apply 2/8 and 3/8 to current codebase and send it to Tom, since they fix real problem and the board doesn't boot without this. I should have separated them out, sorry. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
On 16/12/2014 14:09, Marek Vasut wrote: > The malloc() calls are unnecessary, just allocate the stuff on stack. > While at it, reorder the code a little, so that only one variable is > used for the text, use snprintf() instead of sprintf() and use %01d > as a formatting string to avoid any possible overflows. > > Signed-off-by: Marek Vasut > Cc: Igor Grinberg > Cc: Nikita Kiryanov > Cc: Sean Cross > Cc: Simon Glass > Cc: Stefano Babic > Cc: Tim Harvey > --- Applied to u-boot-imx, thanks ! Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
On Tuesday, December 16, 2014 at 05:27:53 PM, Simon Glass wrote:
> Hi Marek,
>
> On 16 December 2014 at 06:09, Marek Vasut wrote:
> > The malloc() calls are unnecessary, just allocate the stuff on stack.
> > While at it, reorder the code a little, so that only one variable is
> > used for the text, use snprintf() instead of sprintf() and use %01d
> > as a formatting string to avoid any possible overflows.
> >
> > Signed-off-by: Marek Vasut
> > Cc: Igor Grinberg
> > Cc: Nikita Kiryanov
> > Cc: Sean Cross
> > Cc: Simon Glass
> > Cc: Stefano Babic
> > Cc: Tim Harvey
> > ---
> >
> > arch/arm/imx-common/i2c-mxv7.c | 24
> > 1 file changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/arm/imx-common/i2c-mxv7.c
> > b/arch/arm/imx-common/i2c-mxv7.c index 34f5387..1a632e7 100644
> > --- a/arch/arm/imx-common/i2c-mxv7.c
> > +++ b/arch/arm/imx-common/i2c-mxv7.c
> > @@ -73,26 +73,21 @@ static void * const i2c_bases[] = {
> >
> > int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> >
> > struct i2c_pads_info *p)
> >
> > {
> >
> > - char *name1, *name2;
> > + char name[9];
> >
> > int ret;
> >
> > if (i2c_index >= ARRAY_SIZE(i2c_bases))
> >
> > return -EINVAL;
> >
> > - name1 = malloc(9);
> > - name2 = malloc(9);
> > - if (!name1 || !name2)
> > - return -ENOMEM;
> > -
> > - sprintf(name1, "i2c_sda%d", i2c_index);
> > - sprintf(name2, "i2c_scl%d", i2c_index);
> > - ret = gpio_request(p->sda.gp, name1);
> > + snprintf(name, sizeof(name), "i2c_sda%01d", i2c_index);
> > + ret = gpio_request(p->sda.gp, name);
>
> Does this board use driver model? If not it should be easy to convert
> since one MX6 board supports it. With driver model there is
> gpio_requestf("i2c_sda%01d", i2c_index);
No, not yet, but it's in the pipeline. I am already keeping an eye on a few
conversion patches to get this done.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
Hi Marek,
On 16 December 2014 at 06:09, Marek Vasut wrote:
> The malloc() calls are unnecessary, just allocate the stuff on stack.
> While at it, reorder the code a little, so that only one variable is
> used for the text, use snprintf() instead of sprintf() and use %01d
> as a formatting string to avoid any possible overflows.
>
> Signed-off-by: Marek Vasut
> Cc: Igor Grinberg
> Cc: Nikita Kiryanov
> Cc: Sean Cross
> Cc: Simon Glass
> Cc: Stefano Babic
> Cc: Tim Harvey
> ---
> arch/arm/imx-common/i2c-mxv7.c | 24
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
> index 34f5387..1a632e7 100644
> --- a/arch/arm/imx-common/i2c-mxv7.c
> +++ b/arch/arm/imx-common/i2c-mxv7.c
> @@ -73,26 +73,21 @@ static void * const i2c_bases[] = {
> int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> struct i2c_pads_info *p)
> {
> - char *name1, *name2;
> + char name[9];
> int ret;
>
> if (i2c_index >= ARRAY_SIZE(i2c_bases))
> return -EINVAL;
>
> - name1 = malloc(9);
> - name2 = malloc(9);
> - if (!name1 || !name2)
> - return -ENOMEM;
> -
> - sprintf(name1, "i2c_sda%d", i2c_index);
> - sprintf(name2, "i2c_scl%d", i2c_index);
> - ret = gpio_request(p->sda.gp, name1);
> + snprintf(name, sizeof(name), "i2c_sda%01d", i2c_index);
> + ret = gpio_request(p->sda.gp, name);
Does this board use driver model? If not it should be easy to convert
since one MX6 board supports it. With driver model there is
gpio_requestf("i2c_sda%01d", i2c_index);
> if (ret)
> - goto err_req1;
> + return ret;
>
> - ret = gpio_request(p->scl.gp, name2);
> + snprintf(name, sizeof(name), "i2c_scl%01d", i2c_index);
> + ret = gpio_request(p->scl.gp, name);
> if (ret)
> - goto err_req2;
> + goto err_req;
>
> /* Enable i2c clock */
> ret = enable_i2c_clk(1, i2c_index);
> @@ -112,11 +107,8 @@ int setup_i2c(unsigned i2c_index, int speed, int
> slave_addr,
> err_idle:
> err_clk:
> gpio_free(p->scl.gp);
> -err_req2:
> +err_req:
> gpio_free(p->sda.gp);
> -err_req1:
> - free(name1);
> - free(name2);
>
> return ret;
> }
> --
> 2.1.3
>
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
2014-12-16 14:09 GMT+01:00 Marek Vasut :
> The malloc() calls are unnecessary, just allocate the stuff on stack.
> While at it, reorder the code a little, so that only one variable is
> used for the text, use snprintf() instead of sprintf() and use %01d
> as a formatting string to avoid any possible overflows.
>
> Signed-off-by: Marek Vasut
> Cc: Igor Grinberg
> Cc: Nikita Kiryanov
> Cc: Sean Cross
> Cc: Simon Glass
> Cc: Stefano Babic
> Cc: Tim Harvey
> ---
> arch/arm/imx-common/i2c-mxv7.c | 24
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
> index 34f5387..1a632e7 100644
> --- a/arch/arm/imx-common/i2c-mxv7.c
> +++ b/arch/arm/imx-common/i2c-mxv7.c
> @@ -73,26 +73,21 @@ static void * const i2c_bases[] = {
> int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> struct i2c_pads_info *p)
> {
> - char *name1, *name2;
> + char name[9];
> int ret;
>
> if (i2c_index >= ARRAY_SIZE(i2c_bases))
> return -EINVAL;
>
> - name1 = malloc(9);
> - name2 = malloc(9);
> - if (!name1 || !name2)
> - return -ENOMEM;
> -
> - sprintf(name1, "i2c_sda%d", i2c_index);
> - sprintf(name2, "i2c_scl%d", i2c_index);
> - ret = gpio_request(p->sda.gp, name1);
> + snprintf(name, sizeof(name), "i2c_sda%01d", i2c_index);
> + ret = gpio_request(p->sda.gp, name);
> if (ret)
> - goto err_req1;
> + return ret;
>
> - ret = gpio_request(p->scl.gp, name2);
> + snprintf(name, sizeof(name), "i2c_scl%01d", i2c_index);
> + ret = gpio_request(p->scl.gp, name);
> if (ret)
> - goto err_req2;
> + goto err_req;
>
> /* Enable i2c clock */
> ret = enable_i2c_clk(1, i2c_index);
> @@ -112,11 +107,8 @@ int setup_i2c(unsigned i2c_index, int speed, int
> slave_addr,
> err_idle:
> err_clk:
> gpio_free(p->scl.gp);
> -err_req2:
> +err_req:
> gpio_free(p->sda.gp);
> -err_req1:
> - free(name1);
> - free(name2);
>
> return ret;
> }
> --
> 2.1.3
>
Reviewed-by: Christian Gmeiner
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/8] imx: i2c: Zap unnecessary malloc() calls
The malloc() calls are unnecessary, just allocate the stuff on stack.
While at it, reorder the code a little, so that only one variable is
used for the text, use snprintf() instead of sprintf() and use %01d
as a formatting string to avoid any possible overflows.
Signed-off-by: Marek Vasut
Cc: Igor Grinberg
Cc: Nikita Kiryanov
Cc: Sean Cross
Cc: Simon Glass
Cc: Stefano Babic
Cc: Tim Harvey
---
arch/arm/imx-common/i2c-mxv7.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
index 34f5387..1a632e7 100644
--- a/arch/arm/imx-common/i2c-mxv7.c
+++ b/arch/arm/imx-common/i2c-mxv7.c
@@ -73,26 +73,21 @@ static void * const i2c_bases[] = {
int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
struct i2c_pads_info *p)
{
- char *name1, *name2;
+ char name[9];
int ret;
if (i2c_index >= ARRAY_SIZE(i2c_bases))
return -EINVAL;
- name1 = malloc(9);
- name2 = malloc(9);
- if (!name1 || !name2)
- return -ENOMEM;
-
- sprintf(name1, "i2c_sda%d", i2c_index);
- sprintf(name2, "i2c_scl%d", i2c_index);
- ret = gpio_request(p->sda.gp, name1);
+ snprintf(name, sizeof(name), "i2c_sda%01d", i2c_index);
+ ret = gpio_request(p->sda.gp, name);
if (ret)
- goto err_req1;
+ return ret;
- ret = gpio_request(p->scl.gp, name2);
+ snprintf(name, sizeof(name), "i2c_scl%01d", i2c_index);
+ ret = gpio_request(p->scl.gp, name);
if (ret)
- goto err_req2;
+ goto err_req;
/* Enable i2c clock */
ret = enable_i2c_clk(1, i2c_index);
@@ -112,11 +107,8 @@ int setup_i2c(unsigned i2c_index, int speed, int
slave_addr,
err_idle:
err_clk:
gpio_free(p->scl.gp);
-err_req2:
+err_req:
gpio_free(p->sda.gp);
-err_req1:
- free(name1);
- free(name2);
return ret;
}
--
2.1.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
