Hi Mark,
thanks for review.
On 18-09-25 10:17, Mark Brown wrote:
> On Tue, Sep 25, 2018 at 04:23:51PM +0200, Marco Felsch wrote:
> > From: Andreas Färber <afaerber@xxxxxxx>
> >
> > If master clock is provided through device tree, then update
> > the master clock frequency during set_sysclk.
>
> This changelog suggests that the clock is optional...
Your are right, it was my fail to make it not optional.
>
> > + if (!IS_ERR(max98088->mclk)) {
> > + freq = clk_round_rate(max98088->mclk, freq);
> > + clk_set_rate(max98088->mclk, freq);
> > + }
>
> ...as does much of the code (note BTw that this ignores errors setting
> the clock)...
>
> > + max98088->mclk = devm_clk_get(&i2c->dev, "mclk");
> > + if (IS_ERR(max98088->mclk)) {
> > + if (PTR_ERR(max98088->mclk) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > + dev_err(&i2c->dev, "Invalid MCLK\n");
> > + return -EINVAL;
> > + }
> > +
>
> ...but the probe function makes it mandatory (and also throws away the
> error code from clk_get() if it's not -EPROBE_DEFER). Is this the best
> choice? It seems inconsistent anyway.
One question, should it optional or not? If not I will fix it to return
the clk_get error else I will drop it. It shouldn't be optional In my
point of view, since it is needed by the chip.
Regards,
Marco
