ARM: EXYNOS4: Enable WM8994 MFD on I2S selection

On 05/24/11 21:39, Angus Ainslie wrote:
> From: Giridhar Maruthy<giridhar.maruthy@linaro.org>
>
> Whenever I2S is selected, select MFD for WM8994 and I2C
> to avoid manual enabling of the codec and I2C.
>
> Signed-off-by: Giridhar Maruthy<giridhar.maruthy@linaro.org>
> Signed-off-by: Angus Ainslie<angus.ainslie@linaro.org>
> ---
>   sound/soc/samsung/Kconfig |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>

Hi Angus :)

Hmm...first of all, should add proper maintainers in Cc. In this case, 
Jassi, Liam, and Mark should be added in Cc. (Cc'ed them)

> diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
> index a3fdfb6..a80eeb1 100644
> --- a/sound/soc/samsung/Kconfig
> +++ b/sound/soc/samsung/Kconfig
> @@ -65,6 +65,9 @@ config SND_SOC_SAMSUNG_SMDK_WM8994
>   	tristate "SoC I2S Audio support for WM8994 on SMDK"
>   	depends on SND_SOC_SAMSUNG&&  (MACH_SMDKV310 || MACH_SMDKC210)
>   	select SND_SOC_WM8994
> +	select MFD_SUPPORT
> +	select I2C_S3C2410
> +	select MFD_WM8994

I'm not sure about MFD configs, but if "select I2C_S3C2410" is required, 
it should be moved into regarding Kconfig of arch/arm, because it is 
related to dependency of ARCH. Maybe in this case, 
arch/arm/mach-exynos4/Kconfig?

>   	select SND_SAMSUNG_I2S
>   	help
>   		Say Y if you want to add support for SoC audio on the SMDKs.

>> diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
>> index a3fdfb6..a80eeb1 100644
>> --- a/sound/soc/samsung/Kconfig
>> +++ b/sound/soc/samsung/Kconfig
>> @@ -65,6 +65,9 @@ config SND_SOC_SAMSUNG_SMDK_WM8994
>>        tristate "SoC I2S Audio support for WM8994 on SMDK"
>>        depends on SND_SOC_SAMSUNG&&  (MACH_SMDKV310 || MACH_SMDKC210)
>>        select SND_SOC_WM8994
>> +       select MFD_SUPPORT
>> +       select I2C_S3C2410
>> +       select MFD_WM8994
>
> I'm not sure about MFD configs, but if "select I2C_S3C2410" is required, it
> should be moved into regarding Kconfig of arch/arm, because it is related to
> dependency of ARCH. Maybe in this case, arch/arm/mach-exynos4/Kconfig?

I2C here is a SMDK specific requirement and even if most devices need I2C,
selecting I2C for every EXYNOS4210 based device is not ideal. IMO it'd better
be tied to some SMDK specific config option.

Cheer!
Jassi

On 05/25/11 11:15, Jassi Brar wrote:
>>> diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
>>> index a3fdfb6..a80eeb1 100644
>>> --- a/sound/soc/samsung/Kconfig
>>> +++ b/sound/soc/samsung/Kconfig
>>> @@ -65,6 +65,9 @@ config SND_SOC_SAMSUNG_SMDK_WM8994
>>>         tristate "SoC I2S Audio support for WM8994 on SMDK"
>>>         depends on SND_SOC_SAMSUNG&&    (MACH_SMDKV310 || MACH_SMDKC210)
>>>         select SND_SOC_WM8994
>>> +       select MFD_SUPPORT
>>> +       select I2C_S3C2410
>>> +       select MFD_WM8994
>>
>> I'm not sure about MFD configs, but if "select I2C_S3C2410" is required, it
>> should be moved into regarding Kconfig of arch/arm, because it is related to
>> dependency of ARCH. Maybe in this case, arch/arm/mach-exynos4/Kconfig?
>
> I2C here is a SMDK specific requirement and even if most devices need I2C,
> selecting I2C for every EXYNOS4210 based device is not ideal. IMO it'd better
> be tied to some SMDK specific config option.
>

Hi Jassi :)

How are you?

Hmm...need to think again because as you know, basically all of boards 
can be compiled with one exynos4_defconfig.

Ok, so you agreed it should be moved into some Kconfig of arch/arm/ :)
I will think again which one is more reasonable then let you know.

On Wed, May 25, 2011 at 11:45:37PM +0530, Jassi Brar wrote:

> I2C here is a SMDK specific requirement and even if most devices need I2C,
> selecting I2C for every EXYNOS4210 based device is not ideal. IMO it'd better
> be tied to some SMDK specific config option.

Given that it's the audio machine driver the dependency on I2C doesn't
seem unreasonable - the CODEC will generally be I2C connected so I2C is
going to be required for the driver to do anything useful.  Might be
better to just add a dependency on the WM8994 MFD, though, which does
roughly the same thing.

On Thu, May 26, 2011 at 7:54 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 25, 2011 at 11:45:37PM +0530, Jassi Brar wrote:
>
>> I2C here is a SMDK specific requirement and even if most devices need I2C,
>> selecting I2C for every EXYNOS4210 based device is not ideal. IMO it'd better
>> be tied to some SMDK specific config option.
>
> Given that it's the audio machine driver the dependency on I2C doesn't
> seem unreasonable
Yup, actually I am not dead against the submitted patch... it's just
that historically
Samsung ASoC doesn't include I2C dependency, so why now.

> - the CODEC will generally be I2C connected so I2C is
> going to be required for the driver to do anything useful.  Might be
> better to just add a dependency on the WM8994 MFD, though, which does
> roughly the same thing.
Should do too.