Hi Pierre-Louis
Thank you for the review
> > From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> >
> > We have bit field to control snd_soc_card.
> > Let's add probed field on it instead of local variable.
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> > ---
(snip)
> > @@ -1760,12 +1760,10 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card,
> > static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
> > {
> > if (card->instantiated) {
> > - int card_probed = 1;
> > -
>
> This looks like a change, now soc_cleanup_card_resources() is called
> without setting the card_probed bitfield?
>
> everywhere else I see a 1:1 mapping between variable and bitfield
> usage, not here, is this intentional?
This is a little bit confusable point for reviewing.
I will explain, so, please double check it.
This "soc_cleanup_card_resources()" will be called from
snd_soc_unbind_card() as "formal cleanup" or,
snd_soc_bind_card() as "error handling" (D).
For "error handling" purpose, it needed to have "card_probed"
flags as parameter.
This "card->instantiated" will be set at (A) if
all probe functions were called (B) without error.
By this patch, "probed" is handled by "card".
Thus, it already has card->probed flag if
card has instantiated flag.
static int snd_soc_bind_card(struct snd_soc_card *card)
{
...
(B) ret = snd_soc_card_probe(card);
if (ret < 0)
goto probe_end;
...
(B) ret = snd_soc_card_late_probe(card);
if (ret < 0)
goto probe_end;
...
(A) card->instantiated = 1;
...
probe_end:
if (ret < 0)
(D) soc_cleanup_card_resources();
...
}
But indeed patch and log are confusable.
I will try to add such explanation at git-log area.
Thank you for your help !!
Best regards
---
Kuninori Morimoto
