Re: [PATCH v2] clk: ti: Add support for dm814x ADPLL

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Thu, Dec 10, 2015 at 06:26:32PM -0800, Tony Lindgren wrote:
> +	/* Released with kfree() by clkdev_drop() */
> +	cl = kzalloc(sizeof(*cl), GFP_KERNEL);
> +	if (!cl)
> +		return -ENOMEM;
> +
> +	/* Use clkdev_add, clk_register_clkdev limits length to MAX_CON_ID */
> +	cl->con_id = name;
> +	cl->clk = clock;
> +	cl->clk_hw = __clk_get_hw(clock);
> +	clkdev_add(cl);
> +	d->clocks[index].cl = cl;

NAK.  I've no idea why you're open-coding the clkdev internals (which
seems to have been a historical habbit in OMAP code.)  Please stop
doing this.

You are provided with clkdev_alloc() which will allocate the structure
and initialise it for you, and clkdev_add() which will add the allocated
and initialised struct to the list of lookups.  Everything you're doing
above can be done with clkdev_alloc() + clkdev_add() which have been
there for a _very_ long time.  They're even documented (thanks for
providing me with more proof that documentation is nothing but a waste
of time. :))

Even better is clkdev_create() which eliminates the two step clkdev_alloc()
and clkdev_add() process.

So, the whole of the above can be reduced down to:

	cl = clkdev_create(clock, name, NULL);
	if (!cl)
		return -ENOMEM;

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.