Re: [RFC PATCH 04/16] ASoC: Intel: sof-pcm512x: detect Hifiberry DAC+ PRO

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Apr 15, 2020 at 09:07:20AM -0500, Pierre-Louis Bossart wrote:
> On 4/15/20 4:55 AM, Andy Shevchenko wrote:
> > On Tue, Apr 14, 2020 at 01:02:12PM -0500, Pierre-Louis Bossart wrote:
> > > On 4/14/20 12:20 PM, Andy Shevchenko wrote:
> > > > On Thu, Apr 09, 2020 at 02:58:29PM -0500, Pierre-Louis Bossart wrote:

...

> > > > > +	if (IS_ERR(ctx->sclk)) {
> > > > 
> > > > > +		dev_info(dev, "Could not get SCLK, will operate in SOC master mode\n");
> > > > 
> > > > Sounds like devm_clk_get_optional().
> > > 
> > > I am not sure about the semantic here. This driver selects the one which
> > > implements this clock, so if we get a -ENOENT return it's a very bad sign.
> > > Not sure what suppressing the error and converting to NULL would do?
> > 
> > Same as per GPIO.
> > Can it work without this clock? How did it work before your change?
> > 
> > When you add any hard dependency always ask yourself above questions.
> 
> The clock is not required in codec slave mode, it's provided by the SOC.
> 
> The clock is required when the codec is configured as master. Without these
> GPIO selection there will be no audio. So yes we could move this to
> devm_clk_get_optional() and change the test to IS_ERR_OR_NULL.

Hmm... I do not understand. If it's optional, why to check for NULL?

Perhaps you need to split code to show explicitly master / slave paths and for
the first one call everything w/o _optinal() suffix?

> > > > > +		goto skip_dacpro;
> > > > > +	}

-- 
With Best Regards,
Andy Shevchenko