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
