| Message ID | 1513702819-42310-12-git-send-email-nicoleotsuka@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ASoC: fsl_ssi: Clean up - program flow level | expand |
On 19.12.2017 18:00, Nicolin Chen wrote: > AC97 configures some registers earlier to start a communication > with CODECs, so this patch moves those register settings to the > dai_probe() as well, along with other register configurations. > > It also applies _fsl_ssi_set_dai_fmt() to AC97 only since other > formats would be configured via fsl_ssi_set_dai_fmt() directly. > > Meanwhile, this patch adds fsl_ssi_dai_ac97_remove() to cleanup > some control bits for AC97. > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> This patch breaks AC'97 CODEC probing. Namely, the fsl_ssi DAI probe callback is only called after the AC'97 CODEC probe callback, so when you move SSI AC'97 startup to its DAI probe callback it won't be done yet when the CODEC is probed (and this requires a working AC'97 interface to successfully complete). Maciej
On Mon, Jan 01, 2018 at 04:17:20PM +0100, Maciej S. Szmigiero wrote: > > AC97 configures some registers earlier to start a communication > > with CODECs, so this patch moves those register settings to the > > dai_probe() as well, along with other register configurations. > This patch breaks AC'97 CODEC probing. > > Namely, the fsl_ssi DAI probe callback is only called after the AC'97 > CODEC probe callback, so when you move SSI AC'97 startup to its DAI > probe callback it won't be done yet when the CODEC is probed (and this > requires a working AC'97 interface to successfully complete). Hmm...What's the dependency here? Why is it required like this? I am okay to put everything to a separate fsl_ssi_hw_init() and move it back to the platform probe() though.
On 04.01.2018 20:07, Nicolin Chen wrote: > On Mon, Jan 01, 2018 at 04:17:20PM +0100, Maciej S. Szmigiero wrote: >>> AC97 configures some registers earlier to start a communication >>> with CODECs, so this patch moves those register settings to the >>> dai_probe() as well, along with other register configurations. > >> This patch breaks AC'97 CODEC probing. >> >> Namely, the fsl_ssi DAI probe callback is only called after the AC'97 >> CODEC probe callback, so when you move SSI AC'97 startup to its DAI >> probe callback it won't be done yet when the CODEC is probed (and this >> requires a working AC'97 interface to successfully complete). > > Hmm...What's the dependency here? Why is it required like this? This patch moves enabling AC'97 communication (done by fsl_ssi_setup_ac97() ) from SSI _platform device_ probe path to SSI _DAI_ probe path. However, it turns out that a SSI _DAI_ probe happens after a AC'97 CODEC probe (that is, ac97_soc_probe() in sound/soc/codecs/ac97.c). And a AC'97 CODEC probe needs AC'97 communication to be working, since it has to detect the CODEC model, configure it, etc. > I am okay to put everything to a separate fsl_ssi_hw_init() and > move it back to the platform probe() though. > This could be a solution - I assume that by "everything" in the above sentence you mean (at least) enabling the AC'97 communication at the SSI. Maciej
On Thu, Jan 04, 2018 at 09:38:52PM +0100, Maciej S. Szmigiero wrote: > > Hmm...What's the dependency here? Why is it required like this? > And a AC'97 CODEC probe needs AC'97 communication to be working, > since it has to detect the CODEC model, configure it, etc. Okay. If the CODEC configurations depend on (or are done via) the AC link, it has to be in this way then. > > I am okay to put everything to a separate fsl_ssi_hw_init() and > > move it back to the platform probe() though. > > This could be a solution - I assume that by "everything" in the above > sentence you mean (at least) enabling the AC'97 communication at the > SSI. All register configurations -- I was trying to move them to dai probe() so a deferring probe or the platform remove() does not need to revert these register configurations.
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 4673709..2fdae78 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -977,9 +977,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev, regmap_write(regs, REG_SSI_SRCR, srcr); regmap_write(regs, REG_SSI_SCR, scr); - if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97) - fsl_ssi_setup_ac97(ssi); - return 0; } @@ -1108,6 +1105,26 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) regmap_update_bits(ssi->regs, REG_SSI_SCR, SSI_SCR_TCH_EN, SSI_SCR_TCH_EN); + /* AC97 should start earlier to communicate with CODECs */ + if (fsl_ssi_is_ac97(ssi)) { + _fsl_ssi_set_dai_fmt(ssi->dev, ssi, ssi->dai_fmt); + fsl_ssi_setup_ac97(ssi); + } + + return 0; +} + +/** + * Disable register bits for AC97 + */ +static int fsl_ssi_dai_ac97_remove(struct snd_soc_dai *dai) +{ + struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai); + + regmap_write(ssi->regs, REG_SSI_SCR, 0); + regmap_write(ssi->regs, REG_SSI_SACNT, 0); + regmap_write(ssi->regs, REG_SSI_SOR, 0); + return 0; } @@ -1147,6 +1164,7 @@ static const struct snd_soc_component_driver fsl_ssi_component = { static struct snd_soc_dai_driver fsl_ssi_ac97_dai = { .bus_control = true, .probe = fsl_ssi_dai_probe, + .remove = fsl_ssi_dai_ac97_remove, .playback = { .stream_name = "AC97 Playback", .channels_min = 2, @@ -1524,9 +1542,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) } done: - if (ssi->dai_fmt) - _fsl_ssi_set_dai_fmt(dev, ssi, ssi->dai_fmt); - if (fsl_ssi_is_ac97(ssi)) { u32 ssi_idx;
AC97 configures some registers earlier to start a communication with CODECs, so this patch moves those register settings to the dai_probe() as well, along with other register configurations. It also applies _fsl_ssi_set_dai_fmt() to AC97 only since other formats would be configured via fsl_ssi_set_dai_fmt() directly. Meanwhile, this patch adds fsl_ssi_dai_ac97_remove() to cleanup some control bits for AC97. Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- sound/soc/fsl/fsl_ssi.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)