* 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; 8+ 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] 8+ 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; 8+ messages in thread
From: kbuild test robot @ 2019-01-15 13:21 UTC (permalink / raw)
To: 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] 8+ 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; 8+ 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] 8+ 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; 8+ messages in thread
From: kbuild test robot @ 2019-01-15 13:53 UTC (permalink / raw)
To: 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread