On Sun, Jun 14, 2009 at 4:51 PM, Mark Brown<broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx> >> */ >> >> -#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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel