[alsa-devel] [PATCH] [ALSA] ASoC: Add max98925 codec driver

[anish]

On Fri, Jan 16, 2015 at 1:12 PM, Lars-Peter Clausen <lars at metafoo.de> wrote:
> On 01/16/2015 10:00 PM, anish wrote:
>>
>> On Fri, Jan 16, 2015 at 9:11 AM, Lars-Peter Clausen <lars at metafoo.de>
>> wrote:
>>>
>>> On 01/15/2015 10:17 PM, Anish Kumar wrote:
>>>>
>>>>
>>>> From: Anish Kumar <anish.kumar at maximintegrated.com>
>>>>
>>>> This patch adds the max98925 codec driver.
>>>>
>>>> Signed-off-by: Anish Kumar <anish.kumar at maximintegrated.com>
>>>
>>>
>>>
>>> Please make sure to submit drivers against the latest development version
>>> of
>>> the ASoC tree
>>>
>>> (http://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/log/?h=for-next).
>>> This has a couple of outdated things and won't even compile.
>>
>>
>> When i checked on IRC i was told that linux-next is good. Anyway will
>> rebase
>> it against this one.
>
>
> Yes, that was me who said that. But this driver will neither compile against
> linux-next nor asoc/for-next.
>
> Which raises another issue, the driver doesn't add any Makefile or Kconfig
> entries, so even if it would compile you still can compile it.
>
>
>>>> +static struct snd_soc_codec_driver soc_codec_dev_max98925 = {
>>>
>>>
>>>
>>> const
>>>
>>>> +       .probe            = max98925_probe,
>>>> +       .set_bias_level   = max98925_set_bias_level,
>>>
>>>
>>>
>>> You'll need at least some kind of DAPM, otherwise you CODEC won't do
>>> anything.

As already mentioned over IRC this is just a basic power amplifier connected
through i2s interface. Currently we do all the mixer settings by using
the mixer control
and ALSA callbacks such as prepare, hw_params are doing the other jobs.

I can create dapm path to represent the internals of amplifier but i
was wondering
can i tie it to the codec dai?
If yes i can represent codec dai as AIF_IN and DAC as output. So that whenever
codec dai is started it will trigger PA as well. Is that ok?

>>
>>
>> Yes this is most important comment. I wrongly mentioned this as codec
>> but rather it is just mono Class DG audio amplifier  so it doesn't need
>> any dapm connections. Mixer commands is good enough for this and i will
>> change this in commit text as well.
>
>
> Even a amplifier will have a output input and a audio output. Those should
> be properly modeled in DAPM.
>
>
>>>
>>>> +};
>>>> +
>>>
>>>
>>> [...]
>>>>
>>>>
>>>> +/* There should be a second MAX98925 on the board */
>>>> +static struct i2c_board_info max98925_i2c_second[] = {
>>>> +       {
>>>> +               I2C_BOARD_INFO("max98925R", 0x32),
>>>> +       }
>>>> +};
>>>> +
>>>> +struct i2c_client *add_second_device(int busnum)
>>>> +{
>>>> +       struct i2c_client *i2c = NULL;
>>>> +       struct i2c_adapter *adapter;
>>>> +
>>>> +       adapter = i2c_get_adapter(busnum);
>>>> +       if (adapter != NULL)
>>>> +               i2c = i2c_new_device(adapter, max98925_i2c_second);
>>>> +
>>>> +       return i2c;
>>>> +}
>>>
>>>
>>> [...]
>>>>
>>>>
>>>> +       /* Check for second MAX98925 */
>>>> +       i2c_r = add_second_device(2);
>>>
>>>
>>>
>>> If there are two devices instantiate them properly and create two CODEC
>>> devices, not some kind of hackery like this.
>>
>>
>> Ok but this is just audio amplifier do we  still need that?
>
>
> Yes! There is no conceptual difference between a CODEC or an amplifier
> driver at that level.
>
> - Lars