[alsa-devel] [PATCH 4/5] ASoC: max98088: Add master clock handling

[Marco Felsch]

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 at suse.de>
> > 
> > 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