lkml.org 
[lkml]   [2011]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 3/5] clk: introduce the common clock framework
Hi

a few initial comments on clk_get_parent().

On Mon, 21 Nov 2011, Mike Turquette wrote:

> +/**
> + * clk_get_parent - return the parent of a clk
> + * @clk: the clk whose parent gets returned
> + *
> + * Simply returns clk->parent. It is up to the caller to hold the
> + * prepare_lock, if desired. Returns NULL if clk is NULL.
> + */
> +struct clk *clk_get_parent(struct clk *clk)
> +{
> + if (!clk)
> + return NULL;
> +
> + return clk->parent;
> +}
> +EXPORT_SYMBOL_GPL(clk_get_parent);

This implementation of clk_get_parent() has similar problems to the
clk_get_rate() implementation:

http://lkml.org/lkml/2011/11/30/403

clk_get_parent() should return an error if some other entity can change
the clock's parent between the time that clk_get_parent() returns, and the
time that the returned struct clk * is used.

For this to work, we need to define when clk_get_parent() should succeed.
If we follow the protocol outlined in the above URL about clk_get_rate(),
and we stipulate that when we block rate changes, we also block parent
changes, then this should be fairly straightforward. clk_get_parent() can
succeed:

1. between a clk_enable() and a clk_disable() or clk_allow_rate_changes(),

2. between a clk_block_rate_changes() and a clk_enable(), clk_disable(),
or clk_allow_rate_changes()

As with clk_get_rate(), include/linux/clk.h is missing documentation of
what the error return value should be. This should be a little easier to
define than with clk_get_rate(), but I don't think the error value should
be NULL. This is because NULL is a valid return value when
clk_get_parent() is called on root clocks. Better to use
ERR_PTR(-EINVAL/-EBUSY/etc.).


- Paul


\
 
 \ /
  Last update: 2011-12-01 03:17    [W:0.392 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site