On Wednesday 09 May 2018 10:55 PM, David Lechner wrote: > @@ -261,10 +263,14 @@ davinci_lpsc_clk_register(struct device *dev, const > char *name, > lpsc->pd = pd; > lpsc->flags = flags; > > - ret = devm_clk_hw_register(dev, &lpsc->hw); > + ret = clk_hw_register(dev, &lpsc->hw); > if (ret < 0) > return ERR_PTR(ret); > > + /* for now, genpd is only registered when using device-tree */ > + if (!dev || !dev->of_node) > + return lpsc; > + > /* genpd attach needs a way to look up this clock */ > ret = clk_hw_register_clkdev(&lpsc->hw, name, best_dev_name(dev)); > > @@ -378,11 +384,11 @@ __davinci_psc_register_clocks(struct device *dev, > struct regmap *regmap; > int i, ret; > > - psc = devm_kzalloc(dev, sizeof(*psc), GFP_KERNEL); > + psc = kzalloc(sizeof(*psc), GFP_KERNEL); > if (!psc) > return ERR_PTR(-ENOMEM); > > - clks = devm_kmalloc_array(dev, num_clks, sizeof(*clks), GFP_KERNEL); > + clks = kmalloc_array(num_clks, sizeof(*clks), GFP_KERNEL); > if (!clks) > return ERR_PTR(-ENOMEM); > > @@ -396,14 +402,14 @@ __davinci_psc_register_clocks(struct device *dev, > for (i = 0; i < num_clks; i++) > clks[i] = ERR_PTR(-ENOENT); > > - pm_domains = devm_kcalloc(dev, num_clks, sizeof(*pm_domains), > GFP_KERNEL); > + pm_domains = kcalloc(num_clks, sizeof(*pm_domains), GFP_KERNEL); > if (!pm_domains) > return ERR_PTR(-ENOMEM); > > psc->pm_data.domains = pm_domains; > psc->pm_data.num_domains = num_clks; > > - regmap = devm_regmap_init_mmio(dev, base, &davinci_psc_regmap_config); > + regmap = regmap_init_mmio(dev, base, &davinci_psc_regmap_config); > if (IS_ERR(regmap)) > return ERR_CAST(regmap);
Here and in the PLL driver, you have dropped the devm_* variants (like agreed upon), but not added any error path handling. For the clocks needed for boot, its probably fine, but the same code path is used for non-essential clocks too. Just from a code completeness perspective, it will be nice to see the error path correctly handled. Apart from that both 1/27 and 3/27 look good to me. Thanks, Sekhar
