[alsa-devel] [PATCH, RFC 2/3] ASoC: UDA1380: register from board files, configure via pdata

[pHilipp Zabel]

On Sun, Jun 14, 2009 at 4:51 PM, Mark
Brown<broonie at opensource.wolfsonmicro.com> wrote:
> On Sun, Jun 14, 2009 at 12:51:31PM +0200, Philipp Zabel wrote:
>> This patch removes the I2C board info registration from the codec driver.
>> Instead, device instantiation will be done via global i2c_board_info from
>> board files. At the same time, platform specific configuration is moved
>> to platform data and common power/reset GPIO handling moves into the
>> codec driver.
>
> Thanks for doing this.  This should probably either be squashed in with
> the arch patch that follows.

Ok. If Eric agrees, I'll squash as you propose.

>> +struct uda1380_platform_data {
>> +     int gpio_power;
>> +     int gpio_reset;
>> +     int dac_clk;
>> +#define UDA1380_DAC_CLK_SYSCLK 0
>> +#define UDA1380_DAC_CLK_WSPLL  1
>
> Is it worth changing this to make both options non-zero so that there's
> no possibility that it could be left zero by default?

I think this is fine as-is: SYSCLK is the default setting for DAC
clock input when the chip comes out of reset.

>> +static int uda1380_add_i2c_device(struct platform_device *pdev)
>>  {
>
> ...
>
>>       ret = i2c_add_driver(&uda1380_i2c_driver);
>
> If you're doing this conversion the ideal thing is to convert the driver
> completely so that the driver is registered when the module loads and
> does the DAI registration once the I2C device probes.  The I2C device
> probe should also do any other initialisation that can be done without
> the rest of the ASoC subsystem such as registering the GPIOs.

Ok.

> This implies a bit more rearrangment of the registration functions -
> most of the initialisation can stay in the I2C function but anything
> that uses the entire ASoC device and the final instantiation of the card
> should be moved into the ASoC probe function which will be called once
> the entire card has started.  WM8731 is a good template to look at here.

I'll look at WM8731 for guidance, thanks.

>> --- a/sound/soc/codecs/uda1380.h
>> +++ b/sound/soc/codecs/uda1380.h
>> @@ -8,9 +8,6 @@
>>   * Copyright (c) 2005 Giorgio Padrin <giorgio at mandarinlogiq.org>
>>   */
>>
>> -#ifndef _UDA1380_H
>> -#define _UDA1380_H
>> -
>
> Best practice would be to keep these with a different name.

uda1380.h is only a one-time private header file, but I won't say no
to best practice.

cheers
Philipp