lkml.org 
[lkml]   [2018]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/9] clkdev: add managed clkdev lookup registration
Hi,

On Mon, Dec 03, 2018 at 02:17:56PM +0200, Matti Vaittinen wrote:
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 9ab3db8b3988..68a1e55a60d2 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -401,6 +401,25 @@ static struct clk_lookup *__clk_register_clkdev(struct clk_hw *hw,
> return cl;
> }
>
> +static int do_clk_register_clkdev(struct clk_hw *hw,
> + struct clk_lookup **cl, const char *con_id, const char *dev_id)
> +{
> +

No need for this blank line.

> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> + /*
> + * Since dev_id can be NULL, and NULL is handled specially, we must
> + * pass it as either a NULL format string, or with "%s".
> + */
> + if (dev_id)
> + *cl = __clk_register_clkdev(hw, con_id, "%s",
> + dev_id);

Please move "dev_id);" onto the line above.

> @@ -420,20 +439,10 @@ int clk_register_clkdev(struct clk *clk, const char *con_id,
> {
> struct clk_lookup *cl;
>
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> -
> - /*
> - * Since dev_id can be NULL, and NULL is handled specially, we must
> - * pass it as either a NULL format string, or with "%s".
> - */
> - if (dev_id)
> - cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, "%s",
> - dev_id);
> - else
> - cl = __clk_register_clkdev(__clk_get_hw(clk), con_id, NULL);
> -
> - return cl ? 0 : -ENOMEM;
> + if (!IS_ERR(clk))
> + return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
> + dev_id);
> + return PTR_ERR(clk);

Please keep this as:

if (IS_ERR(clk))
return PTR_ERR(clk);

return do_clk_register_clkdev(__clk_get_hw(clk), &cl, con_id,
dev_id);

rather than inverting it - it makes it more consistent with code elsewhere.

> +static int devm_clk_match_clkdev(struct device *dev, void *res, void *data)
> +{
> + struct clk_lookup **l = res;
> +
> + if (!l || !*l) {
> + WARN_ON(!l || !*l);

How can "l" be NULL here? How can *l be NULL?

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

\
 
 \ /
  Last update: 2018-12-03 19:07    [W:0.094 / U:4.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site