Re: [PATCH v9 07/22] clk: Add API to get index of the clock parent

[Stephen Boyd <sboyd@xxxxxxxxxx>]

Quoting Sowjanya Komatineni (2019-08-16 12:41:52)
> This patch adds an API clk_hw_get_parent_index to get index of the
> clock parent to use during the clock restore operations on system
> resume.

Is there a reason we can't save the clk hw index at suspend time by
reading the hardware to understand the current parent? The parent index
typically doesn't matter unless we're trying to communicate something
from the framework to the provider driver. Put another way, I would
think the provider driver can figure out the index itself without having
to go through the framework to do so.

> 
> Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
> Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index cbcc333aec84..12ad0e9b8591 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1645,6 +1645,23 @@ static int clk_fetch_parent_index(struct clk_core *core,
>         return i;
>  }
>  
> +/**
> + * clk_hw_get_parent_index - return the index of parent clock
> + * @hw: clk_hw associated with the clk being consumed
> + * @parent_hw: clk_hw associated with the parent of clk
> + *
> + * Fetches and returns the index of parent clock.
> + * If hw or parent_hw is NULL, returns -EINVAL.
> + */
> +int clk_hw_get_parent_index(struct clk_hw *hw, struct clk_hw *parent_hw)
> +{
> +       if (!hw || !parent_hw)
> +               return -EINVAL;

The caller should be ashamed if they call this with NULL arguments.
I'd prefer we skip this check and we get an oops.

> +
> +       return clk_fetch_parent_index(hw->core, parent_hw->core);
> +}
> +EXPORT_SYMBOL_GPL(clk_hw_get_parent_index);
> +
>  /*
>   * Update the orphan status of @core and all its children.
>   */