[PATCH 4/4] ASoC: enable wm8753 in aspenite

[PATCH 4/4] ASoC: enable wm8753 in aspenite

[Haojian Zhuang]

[PATCH 4/4] ASoC: enable wm8753 in aspenite

[Mark Brown]

On Wed, Mar 31, 2010 at 08:48:58AM -0400, Haojian Zhuang wrote:

> +/* Seek the index of MCLK configuration table */
> +static int pxa168_seek_mclk_conf(int rate, int format, int channel)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mclk_conf); i++) {
> +		if ((mclk_conf[i].rate == rate)
> +			&& (mclk_conf[i].format == format)
> +			&& (mclk_conf[i].channel == channel))
> +			return i;
> +	}
> +	return -EINVAL;
> +}

> +/* Get the MCLK frequency */
> +static int pxa168_get_mclk(int i)
> +{
> +	if ((i < 0) || (i >= ARRAY_SIZE(mclk_conf)))
> +		return -EINVAL;
> +	return mclk_conf[i].mclk;
> +}

This stuff probably shouldn't be in the machine driver since pretty much
all machine drivers are going to want exactly the same code - it should
be in the library code.  Probably best to have them take hw_params
rather than require the machine driver to decode the format, rate and
channel since that'll save a bit of per driver boiler plate if they
don't otherwise need that information.

As I said last time ideally machine drivers shouldn't have to see this
at all, of course.

Otherwise this series all looks basically OK.  Liam is travelling at the
minute so it'll probably be the weekend at the earliest (more likely
early next week) before he has a chance to look at it.

[PATCH 4/4] ASoC: enable wm8753 in aspenite

[Haojian Zhuang]

On Fri, Apr 2, 2010 at 2:21 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Mar 31, 2010 at 08:48:58AM -0400, Haojian Zhuang wrote:
>
>> +/* Seek the index of MCLK configuration table */
>> +static int pxa168_seek_mclk_conf(int rate, int format, int channel)
>> +{
>> + ? ? int i;
>> +
>> + ? ? for (i = 0; i < ARRAY_SIZE(mclk_conf); i++) {
>> + ? ? ? ? ? ? if ((mclk_conf[i].rate == rate)
>> + ? ? ? ? ? ? ? ? ? ? && (mclk_conf[i].format == format)
>> + ? ? ? ? ? ? ? ? ? ? && (mclk_conf[i].channel == channel))
>> + ? ? ? ? ? ? ? ? ? ? return i;
>> + ? ? }
>> + ? ? return -EINVAL;
>> +}
>
>> +/* Get the MCLK frequency */
>> +static int pxa168_get_mclk(int i)
>> +{
>> + ? ? if ((i < 0) || (i >= ARRAY_SIZE(mclk_conf)))
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? return mclk_conf[i].mclk;
>> +}
>
> This stuff probably shouldn't be in the machine driver since pretty much
> all machine drivers are going to want exactly the same code - it should
> be in the library code. ?Probably best to have them take hw_params
> rather than require the machine driver to decode the format, rate and
> channel since that'll save a bit of per driver boiler plate if they
> don't otherwise need that information.
>
> As I said last time ideally machine drivers shouldn't have to see this
> at all, of course.
>
This functions is used to get the sysclk. It's the parameter that is
used in hw_params() of machine driver since we need set codec and cpu
sysclock. But hw_params() of machine driver is invoked before
hw_params() of cpu_dai.

That's the main reason. If hw_params() of cpu_dai is invoked before
machine driver. This issue won't exist.

[PATCH 4/4] ASoC: enable wm8753 in aspenite

[Mark Brown]

On Fri, Apr 02, 2010 at 12:40:30PM +0800, Haojian Zhuang wrote:
> On Fri, Apr 2, 2010 at 2:21 AM, Mark Brown

> >> +/* Get the MCLK frequency */
> >> +static int pxa168_get_mclk(int i)

> > This stuff probably shouldn't be in the machine driver since pretty much
> > all machine drivers are going to want exactly the same code - it should
> > be in the library code. ?Probably best to have them take hw_params
> > rather than require the machine driver to decode the format, rate and
> > channel since that'll save a bit of per driver boiler plate if they
> > don't otherwise need that information.

> > As I said last time ideally machine drivers shouldn't have to see this
> > at all, of course.

> This functions is used to get the sysclk. It's the parameter that is
> used in hw_params() of machine driver since we need set codec and cpu
> sysclock. But hw_params() of machine driver is invoked before
> hw_params() of cpu_dai.

> That's the main reason. If hw_params() of cpu_dai is invoked before
> machine driver. This issue won't exist.

That's not the main point here - the code itself shouldn't be in the
machine driver since pretty much all machine drivers need exactly the
same code.  If they need to do this they should be calling functions
provided by the CPU driver rather than cut'n'pasting the code.

[PATCH 4/4] ASoC: enable wm8753 in aspenite

[Haojian Zhuang]