* [bug report] clk: ti: divider: add driver internal API for parsing divider data
@ 2018-12-17 14:45 Dan Carpenter
2019-01-09 19:45 ` Stephen Boyd
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2018-12-17 14:45 UTC (permalink / raw)
To: t-kristo; +Cc: linux-clk
Hello Tero Kristo,
The patch 4f6be5655dc9: "clk: ti: divider: add driver internal API
for parsing divider data" from Feb 9, 2017, leads to the following
static checker warning:
drivers/clk/ti/divider.c:491 ti_clk_register_divider()
warn: 'table' isn't an ERR_PTR
drivers/clk/ti/divider.c
468 struct clk *ti_clk_register_divider(struct ti_clk *setup)
469 {
470 struct ti_clk_divider *div = setup->data;
471 struct clk_omap_reg reg = {
472 .index = div->module,
473 .offset = div->reg,
474 };
475 u8 width;
476 u32 flags = 0;
477 u8 div_flags = 0;
478 const struct clk_div_table *table;
479 struct clk *clk;
480
481 if (div->flags & CLKF_INDEX_STARTS_AT_ONE)
482 div_flags |= CLK_DIVIDER_ONE_BASED;
483
484 if (div->flags & CLKF_INDEX_POWER_OF_TWO)
485 div_flags |= CLK_DIVIDER_POWER_OF_TWO;
486
487 if (div->flags & CLKF_SET_RATE_PARENT)
488 flags |= CLK_SET_RATE_PARENT;
489
490 table = _get_div_table_from_setup(div, &width);
491 if (IS_ERR(table))
^^^^^
NULL is actually allowed here so we can't just change this to a check
for NULL. Prior to this commit if the:
tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
allocation failed then table was PTR_ERR(-ENOMEM). I guess we should
change it back. We could probably do sothing like the diff below?
492 return (struct clk *)table;
493
494 clk = _register_divider(NULL, setup->name, div->parent,
495 flags, ®, div->bit_shift,
496 width, -EINVAL, div_flags, table);
497
498 if (IS_ERR(clk))
499 kfree(table);
500
501 return clk;
502 }
diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
index 8d77090ad94a..c4335eba2b2e 100644
--- a/drivers/clk/ti/divider.c
+++ b/drivers/clk/ti/divider.c
@@ -403,8 +403,10 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
num_dividers = i;
tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
- if (!tmp)
+ if (!tmp) {
+ *table = PTR_ERR(-ENOMEM);
return -ENOMEM;
+ }
valid_div = 0;
*width = 0;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [bug report] clk: ti: divider: add driver internal API for parsing divider data
2018-12-17 14:45 [bug report] clk: ti: divider: add driver internal API for parsing divider data Dan Carpenter
@ 2019-01-09 19:45 ` Stephen Boyd
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2019-01-09 19:45 UTC (permalink / raw)
To: Dan Carpenter, t-kristo; +Cc: linux-clk
Quoting Dan Carpenter (2018-12-17 06:45:53)
> Hello Tero Kristo,
>
> The patch 4f6be5655dc9: "clk: ti: divider: add driver internal API
> for parsing divider data" from Feb 9, 2017, leads to the following
> static checker warning:
>
> drivers/clk/ti/divider.c:491 ti_clk_register_divider()
> warn: 'table' isn't an ERR_PTR
>
> drivers/clk/ti/divider.c
> 468 struct clk *ti_clk_register_divider(struct ti_clk *setup)
> 469 {
> 470 struct ti_clk_divider *div = setup->data;
> 471 struct clk_omap_reg reg = {
> 472 .index = div->module,
> 473 .offset = div->reg,
> 474 };
> 475 u8 width;
> 476 u32 flags = 0;
> 477 u8 div_flags = 0;
> 478 const struct clk_div_table *table;
> 479 struct clk *clk;
> 480
> 481 if (div->flags & CLKF_INDEX_STARTS_AT_ONE)
> 482 div_flags |= CLK_DIVIDER_ONE_BASED;
> 483
> 484 if (div->flags & CLKF_INDEX_POWER_OF_TWO)
> 485 div_flags |= CLK_DIVIDER_POWER_OF_TWO;
> 486
> 487 if (div->flags & CLKF_SET_RATE_PARENT)
> 488 flags |= CLK_SET_RATE_PARENT;
> 489
> 490 table = _get_div_table_from_setup(div, &width);
> 491 if (IS_ERR(table))
> ^^^^^
>
> NULL is actually allowed here so we can't just change this to a check
> for NULL. Prior to this commit if the:
>
> tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
>
> allocation failed then table was PTR_ERR(-ENOMEM). I guess we should
> change it back. We could probably do sothing like the diff below?
Patch looks fine to me. Are you going to send a proper change?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-09 19:45 ` Stephen Boyd
@ 2019-01-15 7:01 ` Dan Carpenter
2019-01-15 8:36 ` Tero Kristo
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Dan Carpenter @ 2019-01-15 7:01 UTC (permalink / raw)
To: Tero Kristo, Stephen Boyd
Cc: Michael Turquette, linux-omap, linux-clk, kernel-janitors
The ti_clk_parse_divider_data() function is only called from
_get_div_table_from_setup(). That function doesn't look at the return
value but instead looks at the "*table" pointer. In this case, if the
kcalloc() fails then *table is NULL (which means success). It should
instead be an error pointer.
The ti_clk_parse_divider_data() function has two callers. One checks
for errors and the other doesn't. I have fixed it so now both handle
errors.
Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/clk/ti/divider.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
index 8d77090ad94a..4c48ef424ad5 100644
--- a/drivers/clk/ti/divider.c
+++ b/drivers/clk/ti/divider.c
@@ -403,8 +403,10 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
num_dividers = i;
tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
- if (!tmp)
+ if (!tmp) {
+ *table = PTR_ERR(-ENOMEM);
return -ENOMEM;
+ }
valid_div = 0;
*width = 0;
@@ -439,6 +441,7 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
{
struct clk_omap_divider *div;
struct clk_omap_reg *reg;
+ int ret;
if (!setup)
return NULL;
@@ -458,6 +461,12 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
div->flags |= CLK_DIVIDER_POWER_OF_TWO;
div->table = _get_div_table_from_setup(setup, &div->width);
+ if (IS_ERR(div->table)) {
+ ret = PTR_ERR(div->table);
+ kfree(div);
+ return ret;
+ }
+
div->shift = setup->bit_shift;
div->latch = -EINVAL;
--
2.11.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
@ 2019-01-15 8:36 ` Tero Kristo
2019-01-15 13:21 ` kbuild test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Tero Kristo @ 2019-01-15 8:36 UTC (permalink / raw)
To: Dan Carpenter, Stephen Boyd
Cc: Michael Turquette, linux-omap, linux-clk, kernel-janitors
On 15/01/2019 09:01, Dan Carpenter wrote:
> The ti_clk_parse_divider_data() function is only called from
> _get_div_table_from_setup(). That function doesn't look at the return
> value but instead looks at the "*table" pointer. In this case, if the
> kcalloc() fails then *table is NULL (which means success). It should
> instead be an error pointer.
>
> The ti_clk_parse_divider_data() function has two callers. One checks
> for errors and the other doesn't. I have fixed it so now both handle
> errors.
>
> Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Looks fine to me:
Acked-by: Tero Kristo <t-kristo@ti.com>
> ---
> drivers/clk/ti/divider.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
> index 8d77090ad94a..4c48ef424ad5 100644
> --- a/drivers/clk/ti/divider.c
> +++ b/drivers/clk/ti/divider.c
> @@ -403,8 +403,10 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
> num_dividers = i;
>
> tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
> - if (!tmp)
> + if (!tmp) {
> + *table = PTR_ERR(-ENOMEM);
> return -ENOMEM;
> + }
>
> valid_div = 0;
> *width = 0;
> @@ -439,6 +441,7 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
> {
> struct clk_omap_divider *div;
> struct clk_omap_reg *reg;
> + int ret;
>
> if (!setup)
> return NULL;
> @@ -458,6 +461,12 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
> div->flags |= CLK_DIVIDER_POWER_OF_TWO;
>
> div->table = _get_div_table_from_setup(setup, &div->width);
> + if (IS_ERR(div->table)) {
> + ret = PTR_ERR(div->table);
> + kfree(div);
> + return ret;
> + }
> +
>
> div->shift = setup->bit_shift;
> div->latch = -EINVAL;
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
2019-01-15 8:36 ` Tero Kristo
@ 2019-01-15 13:21 ` kbuild test robot
2019-01-15 13:42 ` Dan Carpenter
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-01-15 13:21 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild-all, Tero Kristo, Stephen Boyd, Michael Turquette,
linux-omap, linux-clk, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 4983 bytes --]
Hi Dan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on v5.0-rc2 next-20190115]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/clk-ti-Fix-error-handling-in-ti_clk_parse_divider_data/20190115-175541
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All warnings (new ones prefixed by >>):
drivers/clk/ti/divider.c: In function 'ti_clk_parse_divider_data':
>> drivers/clk/ti/divider.c:407:20: warning: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
*table = PTR_ERR(-ENOMEM);
^
In file included from include/linux/io.h:24:0,
from include/linux/clk-provider.h:9,
from drivers/clk/ti/divider.c:18:
include/linux/err.h:29:33: note: expected 'const void *' but argument is of type 'int'
static inline long __must_check PTR_ERR(__force const void *ptr)
^~~~~~~
>> drivers/clk/ti/divider.c:407:10: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
*table = PTR_ERR(-ENOMEM);
^
drivers/clk/ti/divider.c: In function 'ti_clk_build_component_div':
>> drivers/clk/ti/divider.c:467:10: warning: return makes pointer from integer without a cast [-Wint-conversion]
return ret;
^~~
vim +/PTR_ERR +407 drivers/clk/ti/divider.c
360
361 int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
362 u8 flags, u8 *width,
363 const struct clk_div_table **table)
364 {
365 int valid_div = 0;
366 u32 val;
367 int div;
368 int i;
369 struct clk_div_table *tmp;
370
371 if (!div_table) {
372 if (flags & CLKF_INDEX_STARTS_AT_ONE)
373 val = 1;
374 else
375 val = 0;
376
377 div = 1;
378
379 while (div < max_div) {
380 if (flags & CLKF_INDEX_POWER_OF_TWO)
381 div <<= 1;
382 else
383 div++;
384 val++;
385 }
386
387 *width = fls(val);
388 *table = NULL;
389
390 return 0;
391 }
392
393 i = 0;
394
395 while (!num_dividers || i < num_dividers) {
396 if (div_table[i] == -1)
397 break;
398 if (div_table[i])
399 valid_div++;
400 i++;
401 }
402
403 num_dividers = i;
404
405 tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
406 if (!tmp) {
> 407 *table = PTR_ERR(-ENOMEM);
408 return -ENOMEM;
409 }
410
411 valid_div = 0;
412 *width = 0;
413
414 for (i = 0; i < num_dividers; i++)
415 if (div_table[i] > 0) {
416 tmp[valid_div].div = div_table[i];
417 tmp[valid_div].val = i;
418 valid_div++;
419 *width = i;
420 }
421
422 *width = fls(*width);
423 *table = tmp;
424
425 return 0;
426 }
427
428 static const struct clk_div_table *
429 _get_div_table_from_setup(struct ti_clk_divider *setup, u8 *width)
430 {
431 const struct clk_div_table *table = NULL;
432
433 ti_clk_parse_divider_data(setup->dividers, setup->num_dividers,
434 setup->max_div, setup->flags, width,
435 &table);
436
437 return table;
438 }
439
440 struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
441 {
442 struct clk_omap_divider *div;
443 struct clk_omap_reg *reg;
444 int ret;
445
446 if (!setup)
447 return NULL;
448
449 div = kzalloc(sizeof(*div), GFP_KERNEL);
450 if (!div)
451 return ERR_PTR(-ENOMEM);
452
453 reg = (struct clk_omap_reg *)&div->reg;
454 reg->index = setup->module;
455 reg->offset = setup->reg;
456
457 if (setup->flags & CLKF_INDEX_STARTS_AT_ONE)
458 div->flags |= CLK_DIVIDER_ONE_BASED;
459
460 if (setup->flags & CLKF_INDEX_POWER_OF_TWO)
461 div->flags |= CLK_DIVIDER_POWER_OF_TWO;
462
463 div->table = _get_div_table_from_setup(setup, &div->width);
464 if (IS_ERR(div->table)) {
465 ret = PTR_ERR(div->table);
466 kfree(div);
> 467 return ret;
468 }
469
470
471 div->shift = setup->bit_shift;
472 div->latch = -EINVAL;
473
474 return &div->hw;
475 }
476
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68457 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
2019-01-15 8:36 ` Tero Kristo
2019-01-15 13:21 ` kbuild test robot
@ 2019-01-15 13:42 ` Dan Carpenter
2019-01-15 13:53 ` kbuild test robot
2019-01-15 19:46 ` [PATCH v2] " Dan Carpenter
4 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2019-01-15 13:42 UTC (permalink / raw)
To: Tero Kristo, Stephen Boyd
Cc: Michael Turquette, linux-omap, linux-clk, kernel-janitors
On Tue, Jan 15, 2019 at 10:01:49AM +0300, Dan Carpenter wrote:
> The ti_clk_parse_divider_data() function is only called from
> _get_div_table_from_setup(). That function doesn't look at the return
> value but instead looks at the "*table" pointer. In this case, if the
> kcalloc() fails then *table is NULL (which means success). It should
> instead be an error pointer.
>
> The ti_clk_parse_divider_data() function has two callers. One checks
> for errors and the other doesn't. I have fixed it so now both handle
> errors.
>
> Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/clk/ti/divider.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
> index 8d77090ad94a..4c48ef424ad5 100644
> --- a/drivers/clk/ti/divider.c
> +++ b/drivers/clk/ti/divider.c
> @@ -403,8 +403,10 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
> num_dividers = i;
>
> tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
> - if (!tmp)
> + if (!tmp) {
> + *table = PTR_ERR(-ENOMEM);
Oh wow... I don't know how I screwed that up. :(
Let me resend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
` (2 preceding siblings ...)
2019-01-15 13:42 ` Dan Carpenter
@ 2019-01-15 13:53 ` kbuild test robot
2019-01-15 19:46 ` [PATCH v2] " Dan Carpenter
4 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-01-15 13:53 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild-all, Tero Kristo, Stephen Boyd, Michael Turquette,
linux-omap, linux-clk, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 5080 bytes --]
Hi Dan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on v5.0-rc2 next-20190115]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/clk-ti-Fix-error-handling-in-ti_clk_parse_divider_data/20190115-175541
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=arm
All warnings (new ones prefixed by >>):
drivers/clk/ti/divider.c: In function 'ti_clk_parse_divider_data':
drivers/clk/ti/divider.c:407:20: warning: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
*table = PTR_ERR(-ENOMEM);
In file included from include/linux/io.h:24,
from include/linux/clk-provider.h:9,
from drivers/clk/ti/divider.c:18:
include/linux/err.h:29:61: note: expected 'const void *' but argument is of type 'int'
static inline long __must_check PTR_ERR(__force const void *ptr)
~~~~~~~~~~~~^~~
>> drivers/clk/ti/divider.c:407:10: warning: assignment to 'const struct clk_div_table *' from 'long int' makes pointer from integer without a cast [-Wint-conversion]
*table = PTR_ERR(-ENOMEM);
^
drivers/clk/ti/divider.c: In function 'ti_clk_build_component_div':
>> drivers/clk/ti/divider.c:467:10: warning: returning 'int' from a function with return type 'struct clk_hw *' makes pointer from integer without a cast [-Wint-conversion]
return ret;
^~~
vim +407 drivers/clk/ti/divider.c
360
361 int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
362 u8 flags, u8 *width,
363 const struct clk_div_table **table)
364 {
365 int valid_div = 0;
366 u32 val;
367 int div;
368 int i;
369 struct clk_div_table *tmp;
370
371 if (!div_table) {
372 if (flags & CLKF_INDEX_STARTS_AT_ONE)
373 val = 1;
374 else
375 val = 0;
376
377 div = 1;
378
379 while (div < max_div) {
380 if (flags & CLKF_INDEX_POWER_OF_TWO)
381 div <<= 1;
382 else
383 div++;
384 val++;
385 }
386
387 *width = fls(val);
388 *table = NULL;
389
390 return 0;
391 }
392
393 i = 0;
394
395 while (!num_dividers || i < num_dividers) {
396 if (div_table[i] == -1)
397 break;
398 if (div_table[i])
399 valid_div++;
400 i++;
401 }
402
403 num_dividers = i;
404
405 tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
406 if (!tmp) {
> 407 *table = PTR_ERR(-ENOMEM);
408 return -ENOMEM;
409 }
410
411 valid_div = 0;
412 *width = 0;
413
414 for (i = 0; i < num_dividers; i++)
415 if (div_table[i] > 0) {
416 tmp[valid_div].div = div_table[i];
417 tmp[valid_div].val = i;
418 valid_div++;
419 *width = i;
420 }
421
422 *width = fls(*width);
423 *table = tmp;
424
425 return 0;
426 }
427
428 static const struct clk_div_table *
429 _get_div_table_from_setup(struct ti_clk_divider *setup, u8 *width)
430 {
431 const struct clk_div_table *table = NULL;
432
433 ti_clk_parse_divider_data(setup->dividers, setup->num_dividers,
434 setup->max_div, setup->flags, width,
435 &table);
436
437 return table;
438 }
439
440 struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
441 {
442 struct clk_omap_divider *div;
443 struct clk_omap_reg *reg;
444 int ret;
445
446 if (!setup)
447 return NULL;
448
449 div = kzalloc(sizeof(*div), GFP_KERNEL);
450 if (!div)
451 return ERR_PTR(-ENOMEM);
452
453 reg = (struct clk_omap_reg *)&div->reg;
454 reg->index = setup->module;
455 reg->offset = setup->reg;
456
457 if (setup->flags & CLKF_INDEX_STARTS_AT_ONE)
458 div->flags |= CLK_DIVIDER_ONE_BASED;
459
460 if (setup->flags & CLKF_INDEX_POWER_OF_TWO)
461 div->flags |= CLK_DIVIDER_POWER_OF_TWO;
462
463 div->table = _get_div_table_from_setup(setup, &div->width);
464 if (IS_ERR(div->table)) {
465 ret = PTR_ERR(div->table);
466 kfree(div);
> 467 return ret;
468 }
469
470
471 div->shift = setup->bit_shift;
472 div->latch = -EINVAL;
473
474 return &div->hw;
475 }
476
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68400 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
` (3 preceding siblings ...)
2019-01-15 13:53 ` kbuild test robot
@ 2019-01-15 19:46 ` Dan Carpenter
2019-01-24 19:24 ` Stephen Boyd
4 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2019-01-15 19:46 UTC (permalink / raw)
To: Tero Kristo
Cc: Michael Turquette, Stephen Boyd, linux-omap, linux-clk, kernel-janitors
The ti_clk_parse_divider_data() function is only called from
_get_div_table_from_setup(). That function doesn't look at the return
value but instead looks at the "*table" pointer. In this case, if the
kcalloc() fails then *table is NULL (which means success). It should
instead be an error pointer.
The ti_clk_parse_divider_data() function has two callers. One checks
for errors and the other doesn't. I have fixed it so now both handle
errors.
Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: I somehow didn't compile the v1 of this patch so it had a couple
stupid bugs. I'm very sorry.
drivers/clk/ti/divider.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c
index 8d77090ad94a..0241450f3eb3 100644
--- a/drivers/clk/ti/divider.c
+++ b/drivers/clk/ti/divider.c
@@ -403,8 +403,10 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div,
num_dividers = i;
tmp = kcalloc(valid_div + 1, sizeof(*tmp), GFP_KERNEL);
- if (!tmp)
+ if (!tmp) {
+ *table = ERR_PTR(-ENOMEM);
return -ENOMEM;
+ }
valid_div = 0;
*width = 0;
@@ -439,6 +441,7 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
{
struct clk_omap_divider *div;
struct clk_omap_reg *reg;
+ int ret;
if (!setup)
return NULL;
@@ -458,6 +461,12 @@ struct clk_hw *ti_clk_build_component_div(struct ti_clk_divider *setup)
div->flags |= CLK_DIVIDER_POWER_OF_TWO;
div->table = _get_div_table_from_setup(setup, &div->width);
+ if (IS_ERR(div->table)) {
+ ret = PTR_ERR(div->table);
+ kfree(div);
+ return ERR_PTR(ret);
+ }
+
div->shift = setup->bit_shift;
div->latch = -EINVAL;
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-15 19:46 ` [PATCH v2] " Dan Carpenter
@ 2019-01-24 19:24 ` Stephen Boyd
2019-01-24 19:50 ` Tero Kristo
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2019-01-24 19:24 UTC (permalink / raw)
To: Dan Carpenter, Tero Kristo
Cc: Michael Turquette, linux-omap, linux-clk, kernel-janitors
Quoting Dan Carpenter (2019-01-15 11:46:25)
> The ti_clk_parse_divider_data() function is only called from
> _get_div_table_from_setup(). That function doesn't look at the return
> value but instead looks at the "*table" pointer. In this case, if the
> kcalloc() fails then *table is NULL (which means success). It should
> instead be an error pointer.
>
> The ti_clk_parse_divider_data() function has two callers. One checks
> for errors and the other doesn't. I have fixed it so now both handle
> errors.
>
> Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
Applied to clk-fixes
I'm going to add Tero's ack to this because it isn't really that
different.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clk: ti: Fix error handling in ti_clk_parse_divider_data()
2019-01-24 19:24 ` Stephen Boyd
@ 2019-01-24 19:50 ` Tero Kristo
0 siblings, 0 replies; 10+ messages in thread
From: Tero Kristo @ 2019-01-24 19:50 UTC (permalink / raw)
To: Stephen Boyd, Dan Carpenter
Cc: Michael Turquette, linux-omap, linux-clk, kernel-janitors
On 24/01/2019 21:24, Stephen Boyd wrote:
> Quoting Dan Carpenter (2019-01-15 11:46:25)
>> The ti_clk_parse_divider_data() function is only called from
>> _get_div_table_from_setup(). That function doesn't look at the return
>> value but instead looks at the "*table" pointer. In this case, if the
>> kcalloc() fails then *table is NULL (which means success). It should
>> instead be an error pointer.
>>
>> The ti_clk_parse_divider_data() function has two callers. One checks
>> for errors and the other doesn't. I have fixed it so now both handle
>> errors.
>>
>> Fixes: 4f6be5655dc9 ("clk: ti: divider: add driver internal API for parsing divider data")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>
> Applied to clk-fixes
>
> I'm going to add Tero's ack to this because it isn't really that
> different.
>
Yeah, thats ok. Thanks Stephen.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-01-24 20:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-17 14:45 [bug report] clk: ti: divider: add driver internal API for parsing divider data Dan Carpenter
2019-01-09 19:45 ` Stephen Boyd
2019-01-15 7:01 ` [PATCH] clk: ti: Fix error handling in ti_clk_parse_divider_data() Dan Carpenter
2019-01-15 8:36 ` Tero Kristo
2019-01-15 13:21 ` kbuild test robot
2019-01-15 13:42 ` Dan Carpenter
2019-01-15 13:53 ` kbuild test robot
2019-01-15 19:46 ` [PATCH v2] " Dan Carpenter
2019-01-24 19:24 ` Stephen Boyd
2019-01-24 19:50 ` Tero Kristo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).