On Fri, Dec 20, 2013 at 12:39:38PM +0200, Jyri Sarha wrote:
> Add machine driver support for BeagleBone-Black and other boards with
> tilcdc support and NXP TDA998X HDMI transmitter connected to McASP
> port in I2S mode. The 44100 Hz sample-rate and it's multiples can not
> be supported on Beaglebone-Black because of limited clock-rate
Can the drivers infer this from the clocks?
> support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE.
> The 8 least significant bits are ignored.
Where does this constraint come from?
> + struct snd_soc_card_drvdata_davinci *drvdata =
> + (struct snd_soc_card_drvdata_davinci *)
> + snd_soc_card_get_drvdata(soc_card);
Again with the casting.
> + runtime->hw.rate_min = drvdata->rate_constraint->list[0];
> + runtime->hw.rate_max = drvdata->rate_constraint->list[
> + drvdata->rate_constraint->count - 1];
> + runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
> +
> + snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
> + drvdata->rate_constraint);
> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS,
> + 2, 2);
Why not just set all this statically when registering the DAI?
> +static unsigned int evm_get_bclk(struct snd_pcm_hw_params *params)
> +{
> + int sample_size = snd_pcm_format_width(params_format(params));
> + int rate = params_rate(params);
> + int channels = params_channels(params);
> +
> + return sample_size * channels * rate;
> +}
snd_soc_params_to_frame_size().
> +static int evm_tda998x_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> + struct snd_soc_codec *codec = rtd->codec;
> + struct snd_soc_card *soc_card = codec->card;
> + struct platform_device *pdev = to_platform_device(soc_card->dev);
> + unsigned int bclk_freq = evm_get_bclk(params);
> + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *)
> + snd_soc_card_get_drvdata(soc_card))->sysclk;
> + int ret;
> +
> + ret = snd_soc_dai_set_clkdiv(cpu_dai, 1, sysclk / bclk_freq);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "can't set CPU DAI clock divider %d\n",
> + ret);
> + return ret;
> + }
This looks like something the DAI driver ought to be able to work out
for itself based on the clock rate and sample format.
> +static unsigned int tda998x_hdmi_rates[] = {
> + 32000,
> + 44100,
> + 48000,
> + 88200,
> + 96000,
> +};
The changelog said that 44.1kHz and its multiples couldn't be supported
- is that just the multiples?
> +static struct snd_pcm_hw_constraint_list *evm_tda998x_rate_constraint(
> + struct snd_soc_card *soc_card)
> +{
> + struct platform_device *pdev = to_platform_device(soc_card->dev);
> + unsigned sysclk = ((struct snd_soc_card_drvdata_davinci *)
> + snd_soc_card_get_drvdata(soc_card))->sysclk;
> + struct snd_pcm_hw_constraint_list *ret;
> + unsigned int *rates;
> + int i = 0, j = 0;
> +
> + ret = devm_kzalloc(soc_card->dev, sizeof(*ret) +
> + sizeof(tda998x_hdmi_rates), GFP_KERNEL);
> + if (!ret) {
> + dev_err(&pdev->dev, "Unable to allocate rate constraint!\n");
OOM is already very verbose, don't bother.
> + return NULL;
> + }
> +
> + rates = (unsigned int *)&ret[1];
> + ret->list = rates;
> + ret->mask = 0;
> + for (; i < ARRAY_SIZE(tda998x_hdmi_rates); i++) {
This is all very hard to read. Why has the assignment of i been moved
up to the declaration rather than put here as is idiomatic, what's all
the casting going on with ret and in general?
Attachment:
signature.asc
Description: Digital signature
