Re: question about clk_get_parent

[Julia Lawall]

On Thu, 10 Nov 2016, Russell King - ARM Linux wrote:

> On Thu, Nov 10, 2016 at 09:55:16PM +0100, Julia Lawall wrote:
> > As far as I can see in the various definitions of clk_get_parent, they all
> > return either NULL or a value stored in a structure field.  But the
> > documentation with the prototype in includ/linux/clk.h says that it
> > returns a valid IS_ERR() condition containing errno.  Are ERR_PTR values
> > stored in the structure fields?
>
> The API documentation (in clk.h) is correct.  The API (from the user
> perspective) considers invalid clocks to be the set of pointers for
> which IS_ERR() is true.
>
> By implication, valid clocks are those for which IS_ERR() returns
> false.
>
> Hence, in order for clk_get_parent() to indicate an error, it has to
> return a pointer value which corresponds with IS_ERR() being true.
>
> The question over the NULL clock pointer is left to the implementation
> to decide whether it's an error or not as far as the API design goes,
> but practically everyone treats it as "there is no clock" which is
> entirely reasonable.
>
> Also, remember from the clk API design point of view, users of the
> API should never dereference the clk pointer, it is a cookie as far
> as users should be concerned. (The clk structure was not available to
> drivers in the early days.)  Only clk implementations and clk drivers
> should dereference, and these should not dereference anything but
> their own clocks.

Thanks for the explanation, but I'm not sure how to relate it to what is
in the code.  For example, in drivers/clk/clk.c, there is:

{
        struct clk *parent;

        if (!clk)
                return NULL;

        clk_prepare_lock();
        /* TODO: Create a per-user clk and change callers to call clk_put */
        parent = !clk->core->parent ? NULL : clk->core->parent->hw->clk;
        clk_prepare_unlock();

        return parent;
}

Could clk->core->parent->hw->clk return an ERR_PTR value?  Or is the point
that from the clock point of view, this definition never fails?

thanks,
julia

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