[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEnQRZCzUkpE-ppSDqBzhTXZHphr+twSju=snSiMii9gR=v0nA@mail.gmail.com>
Date: Tue, 1 Aug 2023 16:58:24 +0300
From: Daniel Baluta <daniel.baluta@...il.com>
To: Shengjiu Wang <shengjiu.wang@....com>
Cc: hverkuil@...all.nl, sakari.ailus@....fi, tfiga@...omium.org,
m.szyprowski@...sung.com, mchehab@...nel.org,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
shengjiu.wang@...il.com, Xiubo.Lee@...il.com, festevam@...il.com,
nicoleotsuka@...il.com, lgirdwood@...il.com, broonie@...nel.org,
perex@...ex.cz, tiwai@...e.com, alsa-devel@...a-project.org,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH v2 1/7] ASoC: fsl_asrc: define functions for memory to
memory usage
> +static int fsl_asrc_m2m_check_format(u8 dir, u32 rate, u32 channels, u32 format)
> +{
> + u64 support_format = FSL_ASRC_FORMATS;
> +
> + if (channels < 1 || channels > 10)
> + return -EINVAL;
> +
> + if (rate < 5512 || rate > 192000)
> + return -EINVAL;
> +
I think we can avoid using magic numbers. Instead we could do:
#define FSL_ASRC_MIN_CHANNELS 1
/...
#define FSL_ASRC_MAX_RATE 192000
> + if (dir == IN)
> + support_format |= SNDRV_PCM_FMTBIT_S8;
> +
> + if (!(1 << format & support_format))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/* calculate capture data length according to output data length and sample rate */
> +static int fsl_asrc_m2m_calc_out_len(struct fsl_asrc_pair *pair, int input_buffer_length)
> +{
> + unsigned int in_width, out_width;
> + unsigned int channels = pair->channels;
> + unsigned int in_samples, out_samples;
> + unsigned int out_length;
> +
> + in_width = snd_pcm_format_physical_width(pair->sample_format[IN]) / 8;
> + out_width = snd_pcm_format_physical_width(pair->sample_format[OUT]) / 8;
> +
> + in_samples = input_buffer_length / in_width / channels;
> + out_samples = pair->rate[OUT] * in_samples / pair->rate[IN];
> + out_length = (out_samples - ASRC_OUTPUT_LAST_SAMPLE) * out_width * channels;
> +
> + return out_length;
> +}
> +
> +static int fsl_asrc_m2m_get_maxburst(u8 dir, struct fsl_asrc_pair *pair)
> +{
> + struct fsl_asrc *asrc = pair->asrc;
> + struct fsl_asrc_priv *asrc_priv = asrc->private;
> + int wml = (dir == IN) ? ASRC_M2M_INPUTFIFO_WML : ASRC_M2M_OUTPUTFIFO_WML;
> +
> + if (!asrc_priv->soc->use_edma)
> + return wml * pair->channels;
> + else
> + return 1;
> +}
> +
> +static int fsl_asrc_m2m_pair_resume(struct fsl_asrc_pair *pair)
> +{
> + struct fsl_asrc *asrc = pair->asrc;
> + int i;
> +
> + for (i = 0; i < pair->channels * 4; i++)
> + regmap_write(asrc->regmap, REG_ASRDI(pair->index), 0);
> +
> + return 0;
> +}
> +
> static int fsl_asrc_runtime_resume(struct device *dev);
> static int fsl_asrc_runtime_suspend(struct device *dev);
<snip>
There is no implementation for _suspend although you mention it
in the commit message.
> + * @complete: dma task complete
> + * @sample_format: format of m2m
> + * @rate: rate of m2m
> + * @buf_len: buffer length of m2m
> + * @req_pair: flag for request pair
For example @complete field is not used in this patch. Maybe add it in the patch
that uses it?
I think is the same for other fields.
Powered by blists - more mailing lists