* [bug report] clk: ti: divider: add driver internal API for parsing divider data
@ 2017-04-11 22:16 Dan Carpenter
2017-04-12 7:20 ` Tero Kristo
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2017-04-11 22:16 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:457 ti_clk_register_divider()
warn: 'table' isn't an ERR_PTR
drivers/clk/ti/divider.c
453 if (div->flags & CLKF_SET_RATE_PARENT)
454 flags |= CLK_SET_RATE_PARENT;
455
456 table = _get_div_table_from_setup(div, &width);
457 if (IS_ERR(table))
^^^^^^^^^^^^
This needs to be updated to a NULL check.
458 return (struct clk *)table;
459
460 clk = _register_divider(NULL, setup->name, div->parent,
461 flags, (void __iomem *)reg, div->bit_shift,
462 width, div_flags, table);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] clk: ti: divider: add driver internal API for parsing divider data
2017-04-11 22:16 [bug report] clk: ti: divider: add driver internal API for parsing divider data Dan Carpenter
@ 2017-04-12 7:20 ` Tero Kristo
2017-04-12 16:38 ` Stephen Boyd
0 siblings, 1 reply; 5+ messages in thread
From: Tero Kristo @ 2017-04-12 7:20 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-clk
On 12/04/17 01:16, Dan Carpenter wrote:
> 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:457 ti_clk_register_divider()
> warn: 'table' isn't an ERR_PTR
>
> drivers/clk/ti/divider.c
> 453 if (div->flags & CLKF_SET_RATE_PARENT)
> 454 flags |= CLK_SET_RATE_PARENT;
> 455
> 456 table = _get_div_table_from_setup(div, &width);
> 457 if (IS_ERR(table))
> ^^^^^^^^^^^^
> This needs to be updated to a NULL check.
Thanks for catching.
I'll send a fix for this against 4.12-rc1. Only case where this can fail
is if we run out of memory during boot which is pretty unlikely.
-Tero
>
> 458 return (struct clk *)table;
> 459
> 460 clk = _register_divider(NULL, setup->name, div->parent,
> 461 flags, (void __iomem *)reg, div->bit_shift,
> 462 width, div_flags, table);
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] clk: ti: divider: add driver internal API for parsing divider data
2017-04-12 7:20 ` Tero Kristo
@ 2017-04-12 16:38 ` Stephen Boyd
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2017-04-12 16:38 UTC (permalink / raw)
To: Tero Kristo; +Cc: Dan Carpenter, linux-clk
On 04/12, Tero Kristo wrote:
> On 12/04/17 01:16, Dan Carpenter wrote:
> >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:457 ti_clk_register_divider()
> > warn: 'table' isn't an ERR_PTR
> >
> >drivers/clk/ti/divider.c
> > 453 if (div->flags & CLKF_SET_RATE_PARENT)
> > 454 flags |= CLK_SET_RATE_PARENT;
> > 455
> > 456 table = _get_div_table_from_setup(div, &width);
> > 457 if (IS_ERR(table))
> > ^^^^^^^^^^^^
> >This needs to be updated to a NULL check.
>
> Thanks for catching.
>
> I'll send a fix for this against 4.12-rc1. Only case where this can
> fail is if we run out of memory during boot which is pretty
> unlikely.
>
Ok.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [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, 0 replies; 5+ 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] 5+ messages in thread
* [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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2019-01-09 19:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 22:16 [bug report] clk: ti: divider: add driver internal API for parsing divider data Dan Carpenter
2017-04-12 7:20 ` Tero Kristo
2017-04-12 16:38 ` Stephen Boyd
2018-12-17 14:45 Dan Carpenter
2019-01-09 19:45 ` Stephen Boyd
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).