[2/3] clk: composite: allow fixed rates & fixed dividers

The composite clock assumes that any clock implementing the .recalc_rate
callback will also implement .round_rate and .set_rate.  This is not
always true; the basic fixed-rate clock will only implement .recalc_rate
and a fixed-divider clock may choose to implement .recalc_rate and
.round_rate but not .set_rate.

Fix this by conditionally registering .round_rate and .set_rate
callbacks based on the rate_ops passed in to clk_composite_register.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Cc: Emilio López <emilio@elopez.com.ar>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/clk-composite.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

On Thursday 11 April 2013 03:32 AM, Mike Turquette wrote:
> The composite clock assumes that any clock implementing the .recalc_rate
> callback will also implement .round_rate and .set_rate.  This is not
> always true; the basic fixed-rate clock will only implement .recalc_rate
> and a fixed-divider clock may choose to implement .recalc_rate and
> .round_rate but not .set_rate.
>
> Fix this by conditionally registering .round_rate and .set_rate
> callbacks based on the rate_ops passed in to clk_composite_register.
>
> Signed-off-by: Mike Turquette <mturquette@linaro.org>
> Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
> Cc: Emilio López <emilio@elopez.com.ar>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>   drivers/clk/clk-composite.c |   13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index 6f4728c..2779fb3 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -150,17 +150,22 @@ struct clk *clk_register_composite(struct device *dev, const char *name,
>   	}
>   
>   	if (rate_hw && rate_ops) {
> -		if (!rate_ops->recalc_rate || !rate_ops->round_rate ||
> -		    !rate_ops->set_rate) {
> +		if (!rate_ops->recalc_rate) {
>   			clk = ERR_PTR(-EINVAL);
>   			goto err;
>   		}
>   
> +		/* .round_rate is a prerequisite for .set_rate */
> +		if (rate_ops->round_rate) {
> +			clk_composite_ops->round_rate = clk_composite_round_rate;
> +			if (rate_ops->set_rate) {
> +				clk_composite_ops->set_rate = clk_composite_set_rate;
> +			}
> +		}
> +

What if rate_ops has only recalc_rate and set_rate? clk_composite_ops 
will have only recalc_rate and it will pass the sanity check in clk_init 
and caller of clk_register_composite will expect set_rate to work.
We should check here if rate_ops has set_rate but not round_rate then 
return error.

>   		composite->rate_hw = rate_hw;
>   		composite->rate_ops = rate_ops;
>   		clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
> -		clk_composite_ops->round_rate = clk_composite_round_rate;
> -		clk_composite_ops->set_rate = clk_composite_set_rate;
>   	}
>   
>   	if (gate_hw && gate_ops) {

Quoting Prashant Gaikwad (2013-04-11 04:47:50)
> On Thursday 11 April 2013 03:32 AM, Mike Turquette wrote:
> > The composite clock assumes that any clock implementing the .recalc_rate
> > callback will also implement .round_rate and .set_rate.  This is not
> > always true; the basic fixed-rate clock will only implement .recalc_rate
> > and a fixed-divider clock may choose to implement .recalc_rate and
> > .round_rate but not .set_rate.
> >
> > Fix this by conditionally registering .round_rate and .set_rate
> > callbacks based on the rate_ops passed in to clk_composite_register.
> >
> > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> > Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
> > Cc: Emilio López <emilio@elopez.com.ar>
> > Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > ---
> >   drivers/clk/clk-composite.c |   13 +++++++++----
> >   1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> > index 6f4728c..2779fb3 100644
> > --- a/drivers/clk/clk-composite.c
> > +++ b/drivers/clk/clk-composite.c
> > @@ -150,17 +150,22 @@ struct clk *clk_register_composite(struct device *dev, const char *name,
> >       }
> >   
> >       if (rate_hw && rate_ops) {
> > -             if (!rate_ops->recalc_rate || !rate_ops->round_rate ||
> > -                 !rate_ops->set_rate) {
> > +             if (!rate_ops->recalc_rate) {
> >                       clk = ERR_PTR(-EINVAL);
> >                       goto err;
> >               }
> >   
> > +             /* .round_rate is a prerequisite for .set_rate */
> > +             if (rate_ops->round_rate) {
> > +                     clk_composite_ops->round_rate = clk_composite_round_rate;
> > +                     if (rate_ops->set_rate) {
> > +                             clk_composite_ops->set_rate = clk_composite_set_rate;
> > +                     }
> > +             }
> > +
> 
> What if rate_ops has only recalc_rate and set_rate? clk_composite_ops 
> will have only recalc_rate and it will pass the sanity check in clk_init 
> and caller of clk_register_composite will expect set_rate to work.
> We should check here if rate_ops has set_rate but not round_rate then 
> return error.
> 

I think I will instead put a WARN here instead of completely bailing out
of the clock registration.

Thanks,
Mike

> >               composite->rate_hw = rate_hw;
> >               composite->rate_ops = rate_ops;
> >               clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
> > -             clk_composite_ops->round_rate = clk_composite_round_rate;
> > -             clk_composite_ops->set_rate = clk_composite_set_rate;
> >       }
> >   
> >       if (gate_hw && gate_ops) {