Re: [PATCH 12/19] ASoC: soc-card: add probed bit field to snd_soc_card

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux