Re: [PATCH v3 2/2] ASoC: Add HP iPAQ RX1950 support

From: Vasily Khoruzhick <anarsoul@gmail.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel <alsa-devel@alsa-project.org>,
	Philipp Zabel <philipp.zabel@gmail.com>,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [PATCH v3 2/2] ASoC: Add HP iPAQ RX1950 support
Date: Sat, 28 Aug 2010 12:28:19 +0300	[thread overview]
Message-ID: <201008281228.24097.anarsoul@gmail.com> (raw)
In-Reply-To: <20100828090604.GB9324@opensource.wolfsonmicro.com>

[-- Attachment #1.1: Type: Text/Plain, Size: 2477 bytes --]

В сообщении от 28 августа 2010 12:06:17 автор Mark Brown написал:
> On Sat, Aug 28, 2010 at 11:54:26AM +0300, Vasily Khoruzhick wrote:
> 
> This is basically fine but needs to be redone against current ASoC due
> to Liam's multi-component work - the way the machine driver is
> registered with the system has changed.  See either -next or
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git
> for-2.6.37
> 
> for the latest code.
> 
> > +static int rx1950_startup(struct snd_pcm_substream *substream)
> > +{
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +
> > +	runtime->hw.rate_min = hw_rates.list[0];
> > +	runtime->hw.rate_max = hw_rates.list[hw_rates.count - 1];
> > +	runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
> > +
> > +	return snd_pcm_hw_constraint_list(runtime, 0,
> > +					SNDRV_PCM_HW_PARAM_RATE,
> > +					&hw_rates);
> > +}
> 
> Where does this constraint come from?  It's the sort of thing that
> sounds like it comes from one of the chips so I'd expect their drivers
> to be figuring it out.  Let's see below...
> 
> > +static int rx1950_spk_power(struct snd_soc_dapm_widget *w,
> > +				struct snd_kcontrol *kcontrol, int event)
> > +{
> > +	if (SND_SOC_DAPM_EVENT_ON(event))
> > +		gpio_direction_output(S3C2410_GPA(1), 1);
> > +	else
> > +		gpio_direction_output(S3C2410_GPA(1), 0);
> > +
> > +	return 0;
> > +}
> 
> We should really make a standard GPIO DAPM thing.  Not a problem for
> this driver, though.

Ok

> > +	switch (rate) {
> > +	case 16000:
> > +	case 48000:
> > +		clk_source = S3C24XX_CLKSRC_PCLK;
> > +		fs_mode = S3C2410_IISMOD_384FS;
> > +		div = s3c24xx_i2s_get_clockrate() / (384 * rate);
> > +		if (s3c24xx_i2s_get_clockrate() % (384 * rate) > (182 * rate))
> > +			div++;
> > +		break;
> > +	case 44100:
> > +	case 88200:
> > +		clk_source = S3C24XX_CLKSRC_MPLL;
> > +		fs_mode = S3C2410_IISMOD_256FS;
> > +		div = clk_get_rate(xtal) / (256 * rate);
> > +		if (clk_get_rate(xtal) % (256 * rate) > (128 * rate))
> > +			div++;
> > +		break;
> 
> ...right, it's the CPU driver which is a bit more manual than is ideal.
> OK, again not a problem for this driver.

These constraints are machine-specific. They depends on what clock source for 
i2s machine can provide, for rx1950 it's ~49.9mhz and 299mhz, so I doubt that 
these constraints can be integrated into s3c24xx-i2s driver

Regards
Vasily

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel