[bug report] clk: renesas: Add CPG core wrapper for RZ/G2L SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Lad Prabhakar,

The patch ef3c613ccd68: "clk: renesas: Add CPG core wrapper for
RZ/G2L SoC" from Jun 9, 2021, leads to the following static checker
warning:

	drivers/clk/renesas/renesas-rzg2l-cpg.c:204 rzg2l_cpg_pll_clk_register()
	warn: passing devm_ allocated variable to kfree. 'pll_clk'

drivers/clk/renesas/renesas-rzg2l-cpg.c
   166  static struct clk * __init
   167  rzg2l_cpg_pll_clk_register(const struct cpg_core_clk *core,
   168                             struct clk **clks,
   169                             void __iomem *base,
   170                             struct rzg2l_cpg_priv *priv)
   171  {
   172          struct device *dev = priv->dev;
   173          const struct clk *parent;
   174          struct clk_init_data init;
   175          const char *parent_name;
   176          struct pll_clk *pll_clk;
   177          struct clk *clk;
   178  
   179          parent = clks[core->parent & 0xffff];
   180          if (IS_ERR(parent))
   181                  return ERR_CAST(parent);
   182  
   183          pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL);
   184          if (!pll_clk) {
   185                  clk = ERR_PTR(-ENOMEM);
   186                  return NULL;

Obviously, "return PTR_ERR(-ENOMEM); is intended.  But I looked at the
caller and it has a check for NULL as a kind of work around for this
bug.

When a function returns only NULL and valid pointers, the NULL is an
error.  But when a function returns *BOTH* NULL and error pointers the
NULL means that the feature is deliberately disabled by the user.  It's
not an error.  The caller should not print an error, but instead the
code needs to have NULL checks to avoid the NULL dereference.

For example, maybe the user has disabled blinking lights.

	lights = get_blinking_lights();

If get_blinking_lights() fails then we print an error message and return
to the user.  We don't try to continue.  The user can fix their problem
and try again.

But if the get_blinking_lights() returns NULL that means we have to add
NULL checks:

	if (lights)
		lights->blink();

   187          }
   188  
   189          parent_name = __clk_get_name(parent);
   190          init.name = core->name;
   191          init.ops = &rzg2l_cpg_pll_ops;
   192          init.flags = 0;
   193          init.parent_names = &parent_name;
   194          init.num_parents = 1;
   195  
   196          pll_clk->hw.init = &init;
   197          pll_clk->conf = core->conf;
   198          pll_clk->base = base;
   199          pll_clk->priv = priv;
   200          pll_clk->type = core->type;
   201  
   202          clk = clk_register(NULL, &pll_clk->hw);
   203          if (IS_ERR(clk))
   204                  kfree(pll_clk);

Delete this line.

   205  
   206          return clk;
   207  }

regards,
dan carpenter



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux