| Message ID | 20220915094444.11434-1-povik+lin@cutebit.org |
|---|---|
| Headers | show |
| Series | Support for CS42L83 on Apple machines | expand |
On 15/09/2022 10:44, Martin Povišer wrote: > From: Richard Fitzgerald <rf@opensource.cirrus.com> > > To prepare for adding SoundWire the probe must be split into three > parts: > > 1) The bus-specific probe > 2) Common bus-agnostic probe steps > 3) Initialization of the peripheral registers > > Step (3) must be separate because on SoundWire devices the probe must > enable power supplies and release reset so that the peripheral can be > enumerated by the bus, but it isn't possible to access registers until > enumeration has completed. > > The call to devm_snd_soc_register_component() must be done at stage (2) > so that it can EPROBE_DEFER if necessary. In SoundWire systems stage (3) > is not a probe event so a deferral at this stage would not result in > re-probing dependencies. > > A new init_done flag indicates that the chip has been identified and > initialized. This is used to prevent cs42l42_remove(), cs42l42_suspend(), > cs42l42_restore() and cs42l42_irq_thread() from attempting register > accesses if the chip was not successfully initialized. Although this > cannot happen on I2C, because the entire probe would fail, it is > possible on SoundWire if probe succeeds but the cs42l42 is never > enumerated. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > Signed-off-by: Martin Povišer <povik+lin@cutebit.org> > --- > sound/soc/codecs/cs42l42.c | 127 +++++++++++++++++++++++++------------ > sound/soc/codecs/cs42l42.h | 2 + > 2 files changed, 87 insertions(+), 42 deletions(-) > > diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c > index 11cb659f03e0..427b29db2252 100644 > --- a/sound/soc/codecs/cs42l42.c > +++ b/sound/soc/codecs/cs42l42.c > @@ -1627,7 +1627,7 @@ static irqreturn_t cs42l42_irq_thread(int irq, void *data) > int report = 0; > > mutex_lock(&cs42l42->irq_lock); > - if (cs42l42->suspended) { > + if (cs42l42->suspended || !cs42l42->init_done) { > mutex_unlock(&cs42l42->irq_lock); > return IRQ_NONE; > } This doesn't apply to broonie/for-next. Needs rebasing onto commit: ea75deef1a73 ("ASoC: cs42l42: Only report button state if there was a button interrupt")
> On 15. 9. 2022, at 14:20, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > > On 15/09/2022 10:44, Martin Povišer wrote: >> From: Richard Fitzgerald <rf@opensource.cirrus.com> >> To prepare for adding SoundWire the probe must be split into three >> parts: >> 1) The bus-specific probe >> 2) Common bus-agnostic probe steps >> 3) Initialization of the peripheral registers >> Step (3) must be separate because on SoundWire devices the probe must >> enable power supplies and release reset so that the peripheral can be >> enumerated by the bus, but it isn't possible to access registers until >> enumeration has completed. >> The call to devm_snd_soc_register_component() must be done at stage (2) >> so that it can EPROBE_DEFER if necessary. In SoundWire systems stage (3) >> is not a probe event so a deferral at this stage would not result in >> re-probing dependencies. >> A new init_done flag indicates that the chip has been identified and >> initialized. This is used to prevent cs42l42_remove(), cs42l42_suspend(), >> cs42l42_restore() and cs42l42_irq_thread() from attempting register >> accesses if the chip was not successfully initialized. Although this >> cannot happen on I2C, because the entire probe would fail, it is >> possible on SoundWire if probe succeeds but the cs42l42 is never >> enumerated. >> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> >> Signed-off-by: Martin Povišer <povik+lin@cutebit.org> >> --- >> sound/soc/codecs/cs42l42.c | 127 +++++++++++++++++++++++++------------ >> sound/soc/codecs/cs42l42.h | 2 + >> 2 files changed, 87 insertions(+), 42 deletions(-) >> diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c >> index 11cb659f03e0..427b29db2252 100644 >> --- a/sound/soc/codecs/cs42l42.c >> +++ b/sound/soc/codecs/cs42l42.c >> @@ -1627,7 +1627,7 @@ static irqreturn_t cs42l42_irq_thread(int irq, void *data) >> int report = 0; >> mutex_lock(&cs42l42->irq_lock); >> - if (cs42l42->suspended) { >> + if (cs42l42->suspended || !cs42l42->init_done) { >> mutex_unlock(&cs42l42->irq_lock); >> return IRQ_NONE; >> } > > This doesn't apply to broonie/for-next. Needs rebasing onto commit: > ea75deef1a73 ("ASoC: cs42l42: Only report button state if there was a > button interrupt") It did apply to broonie/for-6.1 roughly at the time of posting, but since then the merge commit 12e51866c79f ("ASoC/qcom/arm64: Qualcomm ADSP DTS and binding fixes”) brought in changes into broonie/for-6.1 from elsewhere, I guess from broonie/for-next. (I am not sure this was intentional.) Since the series still applies in a 3-way automerge (git am -3) on top of the new for-6.1 or for-next, I checked the automerge result and also considering the other commit ea75deef1a73 touches an unrelated part of cs42l42_irq_thread, I am not considering this an issue, that is unless Mark says it is. Martin