| Message ID | 20181217144553.GA11066@kadam (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [bug,report] clk: ti: divider: add driver internal API for parsing divider data | expand |
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?
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;
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 }