question about clk_get_parent

question about clk_get_parent

[Julia Lawall]

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?

thanks,
julia

Re: question about clk_get_parent

[Russell King - ARM Linux]

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.

-- 
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.

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.
>