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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
