On Sat, Nov 7, 2009 at 1:52 AM, Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, Nov 05, 2009 at 10:35:00AM +0900, Jassi Brar wrote: > >> +/** >> + * struct s3c_audio_pdata - common platform data for audio device drivers >> + * @pdata: Pointer to protocol(I2S, PCM or AC97) specific platform data >> + * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode >> + */ >> +struct s3c_audio_pdata { >> + void *pdata; >> + >> + int (*cfg_gpio)(struct platform_device *); >> +}; > > I don't see much benefit in this structure with the pdata embedded in it > - you just end up with an additional layer of indirection in your > platform data if you need to add device specific stuff which is annoying > to specify in machine drivers. Well, since the abstraction(dev-audio) is supposed to be common for I2S, PCM and AC97 protocol, we need to have another layer which communicate protocol specific platform data. This pdata is meant to serve that purpose in future. > I'd just embed the GPIO configuration callback in a driver-specific > platform data. If not for pdata, the cfg_gpio wudn't be enough for future SoCs which not only configure the MUX'ed pins but also set the drive strength. Some might also need configuring the SoC which may not fall into either Clock or Power domain. So, if I am to remove pdata, maybe cfg_gpio can be renamed config_controller? _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel