| Message ID | 1481654490-31846-1-git-send-email-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 13 Dec 2016 19:41:30 +0100, Pierre-Louis Bossart wrote: > > Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") > was merged but the corresponding clock framework patches have not, > after being bumped from audio to clock to x86 domains. The missing > clock-related patches result in a regression starting with 4.9 with > the audio card not being created. > > Rather than reverting this commit and all following updates already > queued up for 4.10, handle run-time dependency on MCLK and fall back > to the previous bit-clock mode. This provides the same functionality > as in 4.8 for Baytrail devices. On Baytrail-CR most devices remain > silent with this fallback but additional patches are needed anyway. > > This patch should be applied to -stable as well as ASoC 4.10 fixes > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Takashi Iwai <tiwai@suse.de> Cc: <stable@vger.kernel.org> # v4.9 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=117141 Takashi > --- > sound/soc/intel/boards/bytcr_rt5640.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c > index 507a86a..4a8b509 100644 > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -828,7 +828,9 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > dev_err(&pdev->dev, > "Failed to get MCLK from pmc_plt_clk_3: %ld\n", > PTR_ERR(priv->mclk)); > - return PTR_ERR(priv->mclk); > + > + /* Fall back to bitclock only */ > + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; > } > } > > -- > 2.9.3 >
On Tue, Dec 13, 2016 at 12:41:30PM -0600, Pierre-Louis Bossart wrote: > Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") > was merged but the corresponding clock framework patches have not, > after being bumped from audio to clock to x86 domains. The missing > clock-related patches result in a regression starting with 4.9 with > the audio card not being created. When I asked about this previously you said that this only affected new boards? > dev_err(&pdev->dev, > "Failed to get MCLK from pmc_plt_clk_3: %ld\n", > PTR_ERR(priv->mclk)); > - return PTR_ERR(priv->mclk); > + > + /* Fall back to bitclock only */ > + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; This is broken for probe deferral, I'd expect this to be explicitly checking for a no clock mapped error code (probably -ENOENT) rather than just ignoring all errors.
On 12/14/16 7:19 AM, Mark Brown wrote: > On Tue, Dec 13, 2016 at 12:41:30PM -0600, Pierre-Louis Bossart wrote: >> Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") >> was merged but the corresponding clock framework patches have not, >> after being bumped from audio to clock to x86 domains. The missing >> clock-related patches result in a regression starting with 4.9 with >> the audio card not being created. > > When I asked about this previously you said that this only affected new > boards? see http://mailman.alsa-project.org/pipermail/alsa-devel/2016-August/111865.html all boards will be affected but newer boards never worked before. Only older BYT-T are affected by a regression. And given the hair-splitting debates on what constitutes pdata or what the locations of the PMC files should be I am no longer expecting a quick resolution. At this rate it'll take 9 months to program 2 registers. > >> dev_err(&pdev->dev, >> "Failed to get MCLK from pmc_plt_clk_3: %ld\n", >> PTR_ERR(priv->mclk)); >> - return PTR_ERR(priv->mclk); >> + >> + /* Fall back to bitclock only */ >> + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; > > This is broken for probe deferral, I'd expect this to be explicitly > checking for a no clock mapped error code (probably -ENOENT) rather than > just ignoring all errors. I don't understand the probe deferral concept here. Are you saying fall back to the bit clock mode if the status is -ENOENT and fail otherwise?
On Wed, 14 Dec 2016 23:08:45 +0100, Pierre-Louis Bossart wrote: > > On 12/14/16 7:19 AM, Mark Brown wrote: > > On Tue, Dec 13, 2016 at 12:41:30PM -0600, Pierre-Louis Bossart wrote: > >> Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") > >> was merged but the corresponding clock framework patches have not, > >> after being bumped from audio to clock to x86 domains. The missing > >> clock-related patches result in a regression starting with 4.9 with > >> the audio card not being created. > > > > When I asked about this previously you said that this only affected new > > boards? > > see > http://mailman.alsa-project.org/pipermail/alsa-devel/2016-August/111865.html > all boards will be affected but newer boards never worked before. Only > older BYT-T are affected by a regression. And given the hair-splitting > debates on what constitutes pdata or what the locations of the PMC > files should be I am no longer expecting a quick resolution. At this > rate it'll take 9 months to program 2 registers. > > > > >> dev_err(&pdev->dev, > >> "Failed to get MCLK from pmc_plt_clk_3: %ld\n", > >> PTR_ERR(priv->mclk)); > >> - return PTR_ERR(priv->mclk); > >> + > >> + /* Fall back to bitclock only */ > >> + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; > > > > This is broken for probe deferral, I'd expect this to be explicitly > > checking for a no clock mapped error code (probably -ENOENT) rather than > > just ignoring all errors. > > I don't understand the probe deferral concept here. Are you saying > fall back to the bit clock mode if the status is -ENOENT and fail > otherwise? The clk driver may return -EPROBE_DEFER, and in that case, the intel driver should also defer the probe as well. Or, like -ENOMEM, it indicates a serious error, and then better bail out. So, we need to figure out which error code should fall back to the legacy mode. (Or vice versa, which error code shouldn't be ignored.) Takashi
On Thu, Dec 15, 2016 at 08:42:22AM +0100, Takashi Iwai wrote: > Pierre-Louis Bossart wrote: > > >> dev_err(&pdev->dev, > > >> "Failed to get MCLK from pmc_plt_clk_3: %ld\n", > > >> PTR_ERR(priv->mclk)); > > >> - return PTR_ERR(priv->mclk); > > >> + > > >> + /* Fall back to bitclock only */ > > >> + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; > > > This is broken for probe deferral, I'd expect this to be explicitly > > > checking for a no clock mapped error code (probably -ENOENT) rather than > > > just ignoring all errors. > > I don't understand the probe deferral concept here. Are you saying > > fall back to the bit clock mode if the status is -ENOENT and fail > > otherwise? > The clk driver may return -EPROBE_DEFER, and in that case, the intel > driver should also defer the probe as well. Or, like -ENOMEM, it > indicates a serious error, and then better bail out. > So, we need to figure out which error code should fall back to the > legacy mode. (Or vice versa, which error code shouldn't be ignored.) Exactly. -ENOENT is what other subsystems return, I've not specifically checked the clock API.
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 507a86a..4a8b509 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -828,7 +828,9 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to get MCLK from pmc_plt_clk_3: %ld\n", PTR_ERR(priv->mclk)); - return PTR_ERR(priv->mclk); + + /* Fall back to bitclock only */ + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; } }
Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") was merged but the corresponding clock framework patches have not, after being bumped from audio to clock to x86 domains. The missing clock-related patches result in a regression starting with 4.9 with the audio card not being created. Rather than reverting this commit and all following updates already queued up for 4.10, handle run-time dependency on MCLK and fall back to the previous bit-clock mode. This provides the same functionality as in 4.8 for Baytrail devices. On Baytrail-CR most devices remain silent with this fallback but additional patches are needed anyway. This patch should be applied to -stable as well as ASoC 4.10 fixes Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/intel/boards/bytcr_rt5640.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)