[alsa-devel] [PATCH] ASoC: Intel: Add Baytrail byt-max98090 machine driver
jarkko.nikula at linux.intel.com
Mon May 19 09:56:11 CEST 2014
On 05/16/2014 09:54 PM, Mark Brown wrote:
>> + /*
>> + * ASoC still uses legacy GPIOs so we look both GPIOs using
>> + * descriptors here, convert them to numbers and release the
>> + * acquired descriptors. Once ASoC switches over to GPIO descriptor
>> + * APIs we can pass them directly.
>> + */
> You could just add the required support to the framework... it seems
> quicker and simpler.
Looks like Mika was already proposing it :-)
>> + hp_desc = gpiod_get_index(card->dev->parent, NULL, 0);
>> + if (IS_ERR(hp_desc))
>> + return 0;
> This just eats errors, it's broken for deferred probe.
>> + snd_soc_jack_report(jack, SND_JACK_LINEOUT | SND_JACK_LINEIN,
>> + SND_JACK_HEADSET | SND_JACK_LINEOUT |
>> + SND_JACK_LINEIN);
> Why is this here?
Hmm.. I'd guess either a left over from a test code or attempt to send a
wrong initial jack state. I'll check.
>> +static int byt_max98090_remove(struct platform_device *pdev)
>> + struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
>> + struct byt_max98090_private *drv = snd_soc_card_get_drvdata(soc_card);
>> + snd_soc_jack_free_gpios(&drv->jack, ARRAY_SIZE(hs_jack_gpios),
>> + hs_jack_gpios);
>> + snd_soc_card_set_drvdata(soc_card, NULL);
>> + snd_soc_unregister_card(soc_card);
>> + platform_set_drvdata(pdev, NULL);
> Setting the data to NULL on removal is just a waste of time and is done
> by the core anyway.
> You're freeing these in the device level remove path but allocating them
> in the ASoC level probe path. They should be managed consistently.
Right, thanks for pointing, I was blind to see this.
Your other comments are valid to byt-rt5640 too. I'll update it also
More information about the Alsa-devel