Re: [PATCH 2/2] ASoC: fsl_asrc: Add support for imx8qm
On Wed, Oct 30, 2019 at 03:20:35AM +, S.j. Wang wrote:
> Hi
>
> >
> > On Tue, Oct 29, 2019 at 05:17:09PM +0800, Shengjiu Wang wrote:
> > > There are two asrc module in imx8qm, each module has different clock
> > > configuration, and the DMA type is EDMA.
> > >
> > > So in this patch, we define the new clocks, refine the clock map, and
> > > include struct fsl_asrc_soc_data for different soc usage.
> > >
> > > The EDMA channel is fixed with each dma request, one dma request
> > > corresponding to one dma channel. So we need to request dma channel
> > > with dma request of asrc module.
> > >
> > > Signed-off-by: Shengjiu Wang
> > > ---
> > > sound/soc/fsl/fsl_asrc.c | 91 +---
> > > sound/soc/fsl/fsl_asrc.h | 65 +-
> > > sound/soc/fsl/fsl_asrc_dma.c | 39
> > > 3 files changed, 167 insertions(+), 28 deletions(-)
> >
> > > diff --git a/sound/soc/fsl/fsl_asrc_dma.c
> > > b/sound/soc/fsl/fsl_asrc_dma.c index d6146de9acd2..dbb07a486504
> > 100644
> > > --- a/sound/soc/fsl/fsl_asrc_dma.c
> > > +++ b/sound/soc/fsl/fsl_asrc_dma.c
> > > @@ -199,19 +199,40 @@ static int fsl_asrc_dma_hw_params(struct
> > > snd_soc_component *component,
> > >
> > > /* Get DMA request of Back-End */
> > > tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
> > > - tmp_data = tmp_chan->private;
> > > - pair->dma_data.dma_request = tmp_data->dma_request;
> > > - dma_release_channel(tmp_chan);
> > > + /* tmp_chan may be NULL for it is already allocated by Back-End */
> > > + if (tmp_chan) {
> > > + tmp_data = tmp_chan->private;
> > > + if (tmp_data)
> > > + pair->dma_data.dma_request =
> > > + tmp_data->dma_request;
> >
> > If this patch is supposed to add a !tmp_chan case for EDMA, we probably
> > shouldn't mute the !tmp_data case because dma_request will be NULL,
> > although the code previously didn't have a check either. I mean we might
> > need to error-out for !tmp_chan. Or...
> > is this intentional?
> >
>
> Yes, intentional. May be we can change to
>
> if (!asrc_priv->soc->use_edma) {
> /* Get DMA request of Back-End */
> tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" :
> "rx");
> tmp_data = tmp_chan->private;
> pair->dma_data.dma_request = tmp_data->dma_request;
> dma_release_channel(tmp_chan);
>
> /* Get DMA request of Front-End */
> tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> tmp_data = tmp_chan->private;
> pair->dma_data.dma_request2 = tmp_data->dma_request;
> pair->dma_data.peripheral_type = tmp_data->peripheral_type;
> pair->dma_data.priority = tmp_data->priority;
> dma_release_channel(tmp_chan);
> }
Oh...now I understand..yea, I think this would be better.
Would you please change it in v2?
I am fine with other places, so may add:
Acked-by: Nicolin Chen
Thanks
Re: [PATCH 2/2] ASoC: fsl_asrc: Add support for imx8qm
Hi
>
> On Tue, Oct 29, 2019 at 05:17:09PM +0800, Shengjiu Wang wrote:
> > There are two asrc module in imx8qm, each module has different clock
> > configuration, and the DMA type is EDMA.
> >
> > So in this patch, we define the new clocks, refine the clock map, and
> > include struct fsl_asrc_soc_data for different soc usage.
> >
> > The EDMA channel is fixed with each dma request, one dma request
> > corresponding to one dma channel. So we need to request dma channel
> > with dma request of asrc module.
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/fsl_asrc.c | 91 +---
> > sound/soc/fsl/fsl_asrc.h | 65 +-
> > sound/soc/fsl/fsl_asrc_dma.c | 39
> > 3 files changed, 167 insertions(+), 28 deletions(-)
>
> > diff --git a/sound/soc/fsl/fsl_asrc_dma.c
> > b/sound/soc/fsl/fsl_asrc_dma.c index d6146de9acd2..dbb07a486504
> 100644
> > --- a/sound/soc/fsl/fsl_asrc_dma.c
> > +++ b/sound/soc/fsl/fsl_asrc_dma.c
> > @@ -199,19 +199,40 @@ static int fsl_asrc_dma_hw_params(struct
> > snd_soc_component *component,
> >
> > /* Get DMA request of Back-End */
> > tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
> > - tmp_data = tmp_chan->private;
> > - pair->dma_data.dma_request = tmp_data->dma_request;
> > - dma_release_channel(tmp_chan);
> > + /* tmp_chan may be NULL for it is already allocated by Back-End */
> > + if (tmp_chan) {
> > + tmp_data = tmp_chan->private;
> > + if (tmp_data)
> > + pair->dma_data.dma_request =
> > + tmp_data->dma_request;
>
> If this patch is supposed to add a !tmp_chan case for EDMA, we probably
> shouldn't mute the !tmp_data case because dma_request will be NULL,
> although the code previously didn't have a check either. I mean we might
> need to error-out for !tmp_chan. Or...
> is this intentional?
>
Yes, intentional. May be we can change to
if (!asrc_priv->soc->use_edma) {
/* Get DMA request of Back-End */
tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
tmp_data = tmp_chan->private;
pair->dma_data.dma_request = tmp_data->dma_request;
dma_release_channel(tmp_chan);
/* Get DMA request of Front-End */
tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
tmp_data = tmp_chan->private;
pair->dma_data.dma_request2 = tmp_data->dma_request;
pair->dma_data.peripheral_type = tmp_data->peripheral_type;
pair->dma_data.priority = tmp_data->priority;
dma_release_channel(tmp_chan);
}
Best regards
Wang shengjiu
Re: [PATCH 2/2] ASoC: fsl_asrc: Add support for imx8qm
On Tue, Oct 29, 2019 at 05:17:09PM +0800, Shengjiu Wang wrote:
> There are two asrc module in imx8qm, each module has different
> clock configuration, and the DMA type is EDMA.
>
> So in this patch, we define the new clocks, refine the clock map,
> and include struct fsl_asrc_soc_data for different soc usage.
>
> The EDMA channel is fixed with each dma request, one dma request
> corresponding to one dma channel. So we need to request dma
> channel with dma request of asrc module.
>
> Signed-off-by: Shengjiu Wang
> ---
> sound/soc/fsl/fsl_asrc.c | 91 +---
> sound/soc/fsl/fsl_asrc.h | 65 +-
> sound/soc/fsl/fsl_asrc_dma.c | 39
> 3 files changed, 167 insertions(+), 28 deletions(-)
> diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
> index d6146de9acd2..dbb07a486504 100644
> --- a/sound/soc/fsl/fsl_asrc_dma.c
> +++ b/sound/soc/fsl/fsl_asrc_dma.c
> @@ -199,19 +199,40 @@ static int fsl_asrc_dma_hw_params(struct
> snd_soc_component *component,
>
> /* Get DMA request of Back-End */
> tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
> - tmp_data = tmp_chan->private;
> - pair->dma_data.dma_request = tmp_data->dma_request;
> - dma_release_channel(tmp_chan);
> + /* tmp_chan may be NULL for it is already allocated by Back-End */
> + if (tmp_chan) {
> + tmp_data = tmp_chan->private;
> + if (tmp_data)
> + pair->dma_data.dma_request = tmp_data->dma_request;
If this patch is supposed to add a !tmp_chan case for EDMA, we
probably shouldn't mute the !tmp_data case because dma_request
will be NULL, although the code previously didn't have a check
either. I mean we might need to error-out for !tmp_chan. Or...
is this intentional?
> + dma_release_channel(tmp_chan);
> + }
>
> /* Get DMA request of Front-End */
> tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> - tmp_data = tmp_chan->private;
> - pair->dma_data.dma_request2 = tmp_data->dma_request;
> - pair->dma_data.peripheral_type = tmp_data->peripheral_type;
> - pair->dma_data.priority = tmp_data->priority;
> - dma_release_channel(tmp_chan);
> + if (tmp_chan) {
> + tmp_data = tmp_chan->private;
> + if (tmp_data) {
Same question here.
> + pair->dma_data.dma_request2 = tmp_data->dma_request;
> + pair->dma_data.peripheral_type =
> + tmp_data->peripheral_type;
> + pair->dma_data.priority = tmp_data->priority;
> + }
> + dma_release_channel(tmp_chan);
> + }
[PATCH 2/2] ASoC: fsl_asrc: Add support for imx8qm
There are two asrc module in imx8qm, each module has different
clock configuration, and the DMA type is EDMA.
So in this patch, we define the new clocks, refine the clock map,
and include struct fsl_asrc_soc_data for different soc usage.
The EDMA channel is fixed with each dma request, one dma request
corresponding to one dma channel. So we need to request dma
channel with dma request of asrc module.
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_asrc.c | 91 +---
sound/soc/fsl/fsl_asrc.h | 65 +-
sound/soc/fsl/fsl_asrc_dma.c | 39
3 files changed, 167 insertions(+), 28 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index a3cfceea7d2f..9dc5b5a93fb0 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -43,24 +43,55 @@ static struct snd_pcm_hw_constraint_list
fsl_asrc_rate_constraints = {
*/
static unsigned char input_clk_map_imx35[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
+ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
};
static unsigned char output_clk_map_imx35[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
+ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
};
/* i.MX53 uses the same map for input and output */
static unsigned char input_clk_map_imx53[] = {
/* 0x0 0x1 0x2 0x3 0x4 0x5 0x6 0x7 0x8 0x9 0xa 0xb 0xc 0xd
0xe 0xf */
0x0, 0x1, 0x2, 0x7, 0x4, 0x5, 0x6, 0x3, 0x8, 0x9, 0xa, 0xb, 0xc, 0xf,
0xe, 0xd,
+ 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7,
0x7, 0x7,
+ 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7,
0x7, 0x7,
};
static unsigned char output_clk_map_imx53[] = {
/* 0x0 0x1 0x2 0x3 0x4 0x5 0x6 0x7 0x8 0x9 0xa 0xb 0xc 0xd
0xe 0xf */
0x8, 0x9, 0xa, 0x7, 0xc, 0x5, 0x6, 0xb, 0x0, 0x1, 0x2, 0x3, 0x4, 0xf,
0xe, 0xd,
+ 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7,
0x7, 0x7,
+ 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7, 0x7,
0x7, 0x7,
};
-static unsigned char *clk_map[2];
+/* i.MX8QM uses the same map for input and output */
+static unsigned char input_clk_map_imx8_0[] = {
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0x0,
+ 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd,
0xe, 0xf,
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+};
+
+static unsigned char output_clk_map_imx8_0[] = {
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0x0,
+ 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd,
0xe, 0xf,
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+};
+
+static unsigned char input_clk_map_imx8_1[] = {
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0x7, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0x0,
+ 0x0, 0x1, 0x2, 0x3, 0xb, 0xc, 0xf, 0xf, 0xd, 0xe, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+ 0x4, 0x5, 0x6, 0xf, 0x8, 0x9, 0xa, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+};
+
+static unsigned char output_clk_map_imx8_1[] = {
+ 0xf, 0xf, 0xf, 0xf, 0xf, 0x7, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0x0,
+ 0x0, 0x1, 0x2, 0x3, 0xb, 0xc, 0xf, 0xf, 0xd, 0xe, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+ 0x4, 0x5, 0x6, 0xf, 0x8, 0x9, 0xa, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf, 0xf,
0xf, 0xf,
+};
/**
* Select the pre-processing and post-processing options
@@ -353,8 +384,8 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair,
bool use_ideal_rate)
}
/* Validate input and output clock sources */
- clk_index[IN] = clk_map[IN][config->inclk];
- clk_index[OUT] = clk_map[OUT][config->outclk];
+ clk_index[IN] = asrc_priv->soc->inclk_map[config->inclk];
+ clk_index[OUT] = asrc_priv->soc->outclk_map[config->outclk];
/* We only have output clock for ideal ratio mode */
clk = asrc_priv->asrck_clk[clk_index[ideal ? OUT : IN]];
@@ -398,13 +429,13 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
*pair, bool use_ideal_rate)
/* Set the channel number */
channels = config->channel_num;
- if (asrc_priv->channel_bits < 4)
+ if (asrc_priv->soc->channel_bits < 4)
channels /= 2;
/* Update channels for current pair */
regmap_update_bits(asrc_priv->regmap, REG_ASRCNCR,
- ASRCNCR_ANCi_MASK(index, asrc_priv->channel_bits),
- ASRCNCR_ANCi(index, channels,
asrc_priv->channel_bits));
+ ASRCNCR_ANCi_MASK(index,
asrc_priv->soc->channel_bits),
+ ASRCNCR_ANCi(index, channels,
