Re: [PATCH] ASoC: lpass-platform: initialize dma channel number
On Tue, Nov 08, 2016 at 02:38:52PM +0100, Arnd Bergmann wrote:
> A bugfix accidentally removed the implicit initialization of the
> dma channel number, causing undefined behavior when
> v->alloc_dma_channel is NULL:
>
> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> sound/soc/qcom/lpass-platform.c:83:29: error: ‘dma_ch’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds back an explicit initialization to zero, restoring the
> previous behavior for that case.
>
> Fixes: 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
> sound/soc/qcom/lpass-platform.c | 3 +++
> 1 file changed, 3 insertions(+)
Good catch.
Acked-by: Kenneth Westfield <kwest...@codeaurora.org>
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH] ASoC: lpass-platform: initialize dma channel number
On Tue, Nov 08, 2016 at 02:38:52PM +0100, Arnd Bergmann wrote:
> A bugfix accidentally removed the implicit initialization of the
> dma channel number, causing undefined behavior when
> v->alloc_dma_channel is NULL:
>
> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> sound/soc/qcom/lpass-platform.c:83:29: error: ‘dma_ch’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds back an explicit initialization to zero, restoring the
> previous behavior for that case.
>
> Fixes: 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> Signed-off-by: Arnd Bergmann
> ---
> sound/soc/qcom/lpass-platform.c | 3 +++
> 1 file changed, 3 insertions(+)
Good catch.
Acked-by: Kenneth Westfield
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote:
> This patch fixes lpass-platform driver which was broken in v4.9-rc1.
> lpass_pcm_data data structure holds information specific to stream.
> Holding a single private pointer to it in global lpass_data
> will not work, because it would be overwritten by for each pcm instance.
>
> This code was breaking playback when we have both playback and capture
> pcm streams, as playback settings are over written by capture settings.
>
> Fix this by moving channel allocation logic out of pcm_new to pcm_open
> so that we can store the stream specific information private_data of
> snd_pcm_runtime.
>
> Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
> snd_soc_pcm_set_drvdata()")
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>
> ---
> sound/soc/qcom/lpass-platform.c | 151
> ++--
> sound/soc/qcom/lpass.h | 1 -
> 2 files changed, 67 insertions(+), 85 deletions(-)
After you address Mark's comments:
Acked-by: Kenneth Westfield <kwest...@codeaurora.org>
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote:
> This patch fixes lpass-platform driver which was broken in v4.9-rc1.
> lpass_pcm_data data structure holds information specific to stream.
> Holding a single private pointer to it in global lpass_data
> will not work, because it would be overwritten by for each pcm instance.
>
> This code was breaking playback when we have both playback and capture
> pcm streams, as playback settings are over written by capture settings.
>
> Fix this by moving channel allocation logic out of pcm_new to pcm_open
> so that we can store the stream specific information private_data of
> snd_pcm_runtime.
>
> Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
> snd_soc_pcm_set_drvdata()")
> Signed-off-by: Srinivas Kandagatla
> ---
> sound/soc/qcom/lpass-platform.c | 151
> ++--
> sound/soc/qcom/lpass.h | 1 -
> 2 files changed, 67 insertions(+), 85 deletions(-)
After you address Mark's comments:
Acked-by: Kenneth Westfield
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH 1/3] ASoC: lpass-platform: use dma_ch instead of rdma_ch/wrdma_ch
On Fri, Oct 28, 2016 at 04:32:18PM +0100, Srinivas Kandagatla wrote: > This patch cleans up usage of wrdma_ch and rdma_ch variables into a > common variable dma_ch, As there is no real use of tracking the dma > channel in two different variables based on stream direction. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-platform.c | 53 > + > 1 file changed, 17 insertions(+), 36 deletions(-) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/3] ASoC: lpass-platform: use dma_ch instead of rdma_ch/wrdma_ch
On Fri, Oct 28, 2016 at 04:32:18PM +0100, Srinivas Kandagatla wrote: > This patch cleans up usage of wrdma_ch and rdma_ch variables into a > common variable dma_ch, As there is no real use of tracking the dma > channel in two different variables based on stream direction. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-platform.c | 53 > + > 1 file changed, 17 insertions(+), 36 deletions(-) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Mon, Jul 04, 2016 at 10:21:49AM +0100, Srinivas Kandagatla wrote: > Move dma channel allocations to pcmops open and close functions. Reason > to do this is that, lpass_platform_pcm_free() accesses snd_soc_pcm_runtime > via substream->private data, However By this time runtimes are already > freed as part of soc_cleanup_card_resources() sequence. > > This patch moves the channel allocations/deallocations to pcmops open() > and close() respectively, where the code has valid snd_soc_pcm_runtime. > > Without this patch unloading lpass sound card module would result in below > crash: snip... > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- LGTM. Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Mon, Jul 04, 2016 at 10:21:49AM +0100, Srinivas Kandagatla wrote: > Move dma channel allocations to pcmops open and close functions. Reason > to do this is that, lpass_platform_pcm_free() accesses snd_soc_pcm_runtime > via substream->private data, However By this time runtimes are already > freed as part of soc_cleanup_card_resources() sequence. > > This patch moves the channel allocations/deallocations to pcmops open() > and close() respectively, where the code has valid snd_soc_pcm_runtime. > > Without this patch unloading lpass sound card module would result in below > crash: snip... > Signed-off-by: Srinivas Kandagatla > --- LGTM. Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 2/2] ASoC: msm8916: Add msm8916-wcd codec driver
On Wed, Jun 15, 2016 at 10:31:36AM +0100, Mark Brown wrote: > On Wed, Jun 15, 2016 at 10:16:27AM +0100, Srinivas Kandagatla wrote: > > On 14/06/16 16:59, Mark Brown wrote: > > > On Fri, Jun 10, 2016 at 07:18:45PM +0100, Srinivas Kandagatla wrote: > > In theory there are 3 devices, > > one is the pmic-spmi driver, which provides regmap access to analog part of > > codec registers. > > second is syscon driver which provides regmap access to digital parts of > > codec to codec driver. > > third is the codec driver which uses both the above. > > > Codec registers range is just split into two, range 0x0- 0x200 sits in pmic > > address space and range 0x201 - 0x4ff in the SOC address space, > > > Are there any other better ways to model this kinda driver? > > Why not just have separate devices for each of the register maps? Srinivas, Mark has a good point. Also, by having distinct devices and drivers; you should make use of ASoC's supporting wrappers for regmap accesses. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 2/2] ASoC: msm8916: Add msm8916-wcd codec driver
On Wed, Jun 15, 2016 at 10:31:36AM +0100, Mark Brown wrote: > On Wed, Jun 15, 2016 at 10:16:27AM +0100, Srinivas Kandagatla wrote: > > On 14/06/16 16:59, Mark Brown wrote: > > > On Fri, Jun 10, 2016 at 07:18:45PM +0100, Srinivas Kandagatla wrote: > > In theory there are 3 devices, > > one is the pmic-spmi driver, which provides regmap access to analog part of > > codec registers. > > second is syscon driver which provides regmap access to digital parts of > > codec to codec driver. > > third is the codec driver which uses both the above. > > > Codec registers range is just split into two, range 0x0- 0x200 sits in pmic > > address space and range 0x201 - 0x4ff in the SOC address space, > > > Are there any other better ways to model this kinda driver? > > Why not just have separate devices for each of the register maps? Srinivas, Mark has a good point. Also, by having distinct devices and drivers; you should make use of ASoC's supporting wrappers for regmap accesses. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH v2 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Tue, Jun 14, 2016 at 06:34:50AM -0700, Srinivas Kandagatla wrote: > On 14/06/16 13:49, Kenneth Westfield wrote: > >On Tue, Jun 14, 2016 at 09:30:03AM +0100, Srinivas Kandagatla wrote: > >>- data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL); > >>+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > > >Is the reason for changing the dev pointer similar to why it was changed > >for snd_dma_alloc_pages() below? > Yep, Only reason is that I can fit stuff in single line. The original line fit in one as well. If that is the reason, maybe just leave it unchanged. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH v2 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Tue, Jun 14, 2016 at 06:34:50AM -0700, Srinivas Kandagatla wrote: > On 14/06/16 13:49, Kenneth Westfield wrote: > >On Tue, Jun 14, 2016 at 09:30:03AM +0100, Srinivas Kandagatla wrote: > >>- data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL); > >>+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > > >Is the reason for changing the dev pointer similar to why it was changed > >for snd_dma_alloc_pages() below? > Yep, Only reason is that I can fit stuff in single line. The original line fit in one as well. If that is the reason, maybe just leave it unchanged. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 1/2] ASoC: msm8916: Add codec Device Tree bindings.
On Fri, Jun 10, 2016 at 11:18:44AM -0700, Srinivas Kandagatla wrote: > diff --git a/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > new file mode 100644 > index 000..0559c1f > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > @@ -0,0 +1,103 @@ > +msm8916 audio CODEC audio CODEC > + > +Codec IP is divided into two parts, first analog which is integrated in > pmic pm8916 > +and secondly digital part which is integrated into application processor. > Codec register > +controls are also split across pmic an lpass. Analog part is controlled > via spmi bus to pmic. Cleaning up the description: msm8916 audio CODEC Codec IP is divided into two parts: * analog, which is integrated into pmic pm8916 * digital, which is integrated into LPASS on the SOC Codec registers are also split across pmic and LPASS. Analog codec is controlled via spmi bus to pmic. > +## Bindings for codec core on pmic: > + > +Required properties > + - compatible = "qcom,msm8916-pmic-wcd-codec"; OK. Looking at this again, I think its better just to drop the pmic. > + "spk_cnp_int" - Speaker click and pop interrupt > + "spk_clip_int" - Speaker clip interrupt > + "spk_ocp_int" - Speaker over current protect interrupt. > + "ins_rem_det1" - jack insert removal detect interrupt 1. > + "but_rel_det" - button release interrupt > + "but_press_det" - button press event > + "ins_rem_det" - jack insert removal detect interrup > + "mbhc_int" - multi button headset interrupt. > + "ear_ocp_int" - Earphone over current protect interrupt. > + "hphr_ocp_int" - Headphone R over current protect interrupt. > + "hphl_ocp_det" - Headphone L over current protect interrupt > + "ear_cnp_int" - earphone cnp interrupt. > + "hphr_cnp_int" - hphr click and pop interrupt. > + "hphl_cnp_int" - hphl click and pop interrupt Please use labels that more closely match the HW spec: "cdc_spk_cnp_int" "cdc_spk_clip_int" "cdc_spk_ocp_int" "mbhc_ins_rem_det1" "mbhc_but_rel_det" "mbhc_but_press_det" "mbhc_ins_rem_det" "mbhc_switch_int" "cdc_ear_ocp_int" "cdc_hphr_ocp_int" "cdc_hphl_ocp_int" "cdc_ear_cnp_int" "cdc_hphr_cnp_int" "cdc_hphl_cnp_int" -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 1/2] ASoC: msm8916: Add codec Device Tree bindings.
On Fri, Jun 10, 2016 at 11:18:44AM -0700, Srinivas Kandagatla wrote: > diff --git a/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > new file mode 100644 > index 000..0559c1f > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/qcom,msm8916-wcd.txt > @@ -0,0 +1,103 @@ > +msm8916 audio CODEC audio CODEC > + > +Codec IP is divided into two parts, first analog which is integrated in > pmic pm8916 > +and secondly digital part which is integrated into application processor. > Codec register > +controls are also split across pmic an lpass. Analog part is controlled > via spmi bus to pmic. Cleaning up the description: msm8916 audio CODEC Codec IP is divided into two parts: * analog, which is integrated into pmic pm8916 * digital, which is integrated into LPASS on the SOC Codec registers are also split across pmic and LPASS. Analog codec is controlled via spmi bus to pmic. > +## Bindings for codec core on pmic: > + > +Required properties > + - compatible = "qcom,msm8916-pmic-wcd-codec"; OK. Looking at this again, I think its better just to drop the pmic. > + "spk_cnp_int" - Speaker click and pop interrupt > + "spk_clip_int" - Speaker clip interrupt > + "spk_ocp_int" - Speaker over current protect interrupt. > + "ins_rem_det1" - jack insert removal detect interrupt 1. > + "but_rel_det" - button release interrupt > + "but_press_det" - button press event > + "ins_rem_det" - jack insert removal detect interrup > + "mbhc_int" - multi button headset interrupt. > + "ear_ocp_int" - Earphone over current protect interrupt. > + "hphr_ocp_int" - Headphone R over current protect interrupt. > + "hphl_ocp_det" - Headphone L over current protect interrupt > + "ear_cnp_int" - earphone cnp interrupt. > + "hphr_cnp_int" - hphr click and pop interrupt. > + "hphl_cnp_int" - hphl click and pop interrupt Please use labels that more closely match the HW spec: "cdc_spk_cnp_int" "cdc_spk_clip_int" "cdc_spk_ocp_int" "mbhc_ins_rem_det1" "mbhc_but_rel_det" "mbhc_but_press_det" "mbhc_ins_rem_det" "mbhc_switch_int" "cdc_ear_ocp_int" "cdc_hphr_ocp_int" "cdc_hphl_ocp_int" "cdc_ear_cnp_int" "cdc_hphr_cnp_int" "cdc_hphl_cnp_int" -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Tue, Jun 14, 2016 at 09:30:03AM +0100, Srinivas Kandagatla wrote:
> Move dma channel allocations to pcmops open and close functions. Reason
> to do this is that, lpass_platform_pcm_free() accesses snd_soc_pcm_runtime
> via substream->private data, However By this time runtimes are already
> freed as part of soc_cleanup_card_resources() sequence.
>
> This patch moves the channel allocations/deallocations to pcmops open()
> and close() respectively, where the code has valid snd_soc_pcm_runtime.
> ---
> sound/soc/qcom/lpass-platform.c | 155
> +++-
> 1 file changed, 74 insertions(+), 81 deletions(-)
>
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index db000c6..3bd2cd6 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -61,7 +61,36 @@ static int lpass_platform_pcmops_open(struct
> snd_pcm_substream *substream)
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + pcm_data->rdma_ch = ch;
> + reg = LPAIF_RDMACTL_REG(v, ch);
Spacing.
> + } else {
> + pcm_data->wrdma_ch = ch;
> + reg = LPAIF_WRDMACTL_REG(v, ch);
Spacing.
> +static int lpass_platform_pcmops_close(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
> + struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime);
> + struct lpass_data *drvdata =
> + snd_soc_platform_get_drvdata(soc_runtime->platform);
> + struct lpass_variant *v = drvdata->variant;
> + int ch;
> +
> + if (v->free_dma_channel) {
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + ch = pcm_data->rdma_ch;
> + else
> + ch = pcm_data->wrdma_ch;
> +
> + if (ch >= 0)
> + v->free_dma_channel(drvdata, ch);
> + }
Since this is being moved from free() to close(), drvdata->substream[ch]
should be set to NULL. Otherwise, the array could fill up as the PCM is
opened/closed.
> @@ -471,14 +529,12 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
> struct snd_pcm *pcm = soc_runtime->pcm;
> struct snd_pcm_substream *psubstream, *csubstream;
> struct snd_soc_dai *cpu_dai = soc_runtime->cpu_dai;
> - struct lpass_data *drvdata =
> - snd_soc_platform_get_drvdata(soc_runtime->platform);
> - struct lpass_variant *v = drvdata->variant;
> - int ret = -EINVAL;
> + struct device *dev = soc_runtime->platform->dev;
> struct lpass_pcm_data *data;
> size_t size = lpass_platform_pcm_hardware.buffer_bytes_max;
> + int ret = -EINVAL;
>
> - data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
Is the reason for changing the dev pointer similar to why it was changed
for snd_dma_alloc_pages() below?
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
> + >dma_buffer);
> if (ret) {
> - dev_err(soc_runtime->dev,
> - "%s() error writing to rdmactl reg: %d\n",
> - __func__, ret);
> - goto capture_alloc_err;
> + dev_err(dev, "can't alloc playback dma buffer\n");
Print the return value.
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
> + >dma_buffer);
> if (ret) {
> - dev_err(soc_runtime->dev,
> - "%s() error writing to wrdmactl reg: %d\n",
> - __func__, ret);
> - goto capture_reg_err;
> + dev_err(dev, "can't alloc capture dma buffer\n");
Ditto.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v2 2/2] sound: lpass-platform: Move dma channel allocation to pcmops
On Tue, Jun 14, 2016 at 09:30:03AM +0100, Srinivas Kandagatla wrote:
> Move dma channel allocations to pcmops open and close functions. Reason
> to do this is that, lpass_platform_pcm_free() accesses snd_soc_pcm_runtime
> via substream->private data, However By this time runtimes are already
> freed as part of soc_cleanup_card_resources() sequence.
>
> This patch moves the channel allocations/deallocations to pcmops open()
> and close() respectively, where the code has valid snd_soc_pcm_runtime.
> ---
> sound/soc/qcom/lpass-platform.c | 155
> +++-
> 1 file changed, 74 insertions(+), 81 deletions(-)
>
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index db000c6..3bd2cd6 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -61,7 +61,36 @@ static int lpass_platform_pcmops_open(struct
> snd_pcm_substream *substream)
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + pcm_data->rdma_ch = ch;
> + reg = LPAIF_RDMACTL_REG(v, ch);
Spacing.
> + } else {
> + pcm_data->wrdma_ch = ch;
> + reg = LPAIF_WRDMACTL_REG(v, ch);
Spacing.
> +static int lpass_platform_pcmops_close(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
> + struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime);
> + struct lpass_data *drvdata =
> + snd_soc_platform_get_drvdata(soc_runtime->platform);
> + struct lpass_variant *v = drvdata->variant;
> + int ch;
> +
> + if (v->free_dma_channel) {
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + ch = pcm_data->rdma_ch;
> + else
> + ch = pcm_data->wrdma_ch;
> +
> + if (ch >= 0)
> + v->free_dma_channel(drvdata, ch);
> + }
Since this is being moved from free() to close(), drvdata->substream[ch]
should be set to NULL. Otherwise, the array could fill up as the PCM is
opened/closed.
> @@ -471,14 +529,12 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
> struct snd_pcm *pcm = soc_runtime->pcm;
> struct snd_pcm_substream *psubstream, *csubstream;
> struct snd_soc_dai *cpu_dai = soc_runtime->cpu_dai;
> - struct lpass_data *drvdata =
> - snd_soc_platform_get_drvdata(soc_runtime->platform);
> - struct lpass_variant *v = drvdata->variant;
> - int ret = -EINVAL;
> + struct device *dev = soc_runtime->platform->dev;
> struct lpass_pcm_data *data;
> size_t size = lpass_platform_pcm_hardware.buffer_bytes_max;
> + int ret = -EINVAL;
>
> - data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
Is the reason for changing the dev pointer similar to why it was changed
for snd_dma_alloc_pages() below?
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
> + >dma_buffer);
> if (ret) {
> - dev_err(soc_runtime->dev,
> - "%s() error writing to rdmactl reg: %d\n",
> - __func__, ret);
> - goto capture_alloc_err;
> + dev_err(dev, "can't alloc playback dma buffer\n");
Print the return value.
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
> + >dma_buffer);
> if (ret) {
> - dev_err(soc_runtime->dev,
> - "%s() error writing to wrdmactl reg: %d\n",
> - __func__, ret);
> - goto capture_reg_err;
> + dev_err(dev, "can't alloc capture dma buffer\n");
Ditto.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v1 1/2] sound: lpass-cpu: add module licence and description
On Mon, Jun 13, 2016 at 02:23:16PM +0100, Srinivas Kandagatla wrote: > This patch adds module licence to lpass-cpu driver, without this > patch lpass-cpu module would taint with below error, while insmod/modprobe. > > snd_soc_lpass_cpu: module license 'unspecified' taints kernel. > Disabling lock debugging due to kernel taint > snd_soc_lpass_cpu: Unknown symbol regmap_write (err 0) > snd_soc_lpass_cpu: Unknown symbol devm_kmalloc (err 0) > ... > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v1 1/2] sound: lpass-cpu: add module licence and description
On Mon, Jun 13, 2016 at 02:23:16PM +0100, Srinivas Kandagatla wrote: > This patch adds module licence to lpass-cpu driver, without this > patch lpass-cpu module would taint with below error, while insmod/modprobe. > > snd_soc_lpass_cpu: module license 'unspecified' taints kernel. > Disabling lock debugging due to kernel taint > snd_soc_lpass_cpu: Unknown symbol regmap_write (err 0) > snd_soc_lpass_cpu: Unknown symbol devm_kmalloc (err 0) > ... > > Signed-off-by: Srinivas Kandagatla > --- Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [RFC v2 1/3] ASoC: msm8916: Add codec Device Tree bindings.
<0x1 0xf0 0x6 IRQ_TYPE_NONE>,
> + <0x1 0xf0 0x7 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
> + interrupt-names = "spk_cnp_int",
> + "spk_clip_int",
> + "spk_ocp_int",
> + "ins_rem_det1",
> + "but_rel_det",
> + "but_press_det",
> + "ins_rem_det",
> + "mbhc_int",
> + "ear_ocp_int",
> + "hphr_ocp_int",
> + "hphl_ocp_det",
> + "ear_cnp_int",
> + "hphr_cnp_int",
> + "hphl_cnp_int";
> + vddio-supply = <_l5>;
> + vdd-tx-rx-supply = <_l5>;
> + vdd-micbias-supply = <_l13>;
> + qcom,lpass-codec-core = <_codec_core>;
Is there a reason the PMIC codec holds a phandle reference to the SoC
codec node? Rather than having the SoC codec hold a ref to the PMIC
codec?
> + #sound-dai-cells = <1>;
> + };
> +};
> +
> +soc {
> + ...
> + lpass_codec_core: lpass-codec{
> + compatible = "qcom,msm8916-lpass-codec", "syscon";
> + reg = <0x0771c000 0x400>;
> + };
> +
> +};
> --
> 2.8.2
>
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [RFC v2 3/3] ASoC: msm8916: Add msm8916-wcd codec driver
S_RATE_F_48_KHZ;
> + break;
> + case 96000:
> + tx_fs_rate = TX_I2S_CTL_TX_I2S_FS_RATE_F_96_KHZ;
> + break;
> + case 192000:
> + tx_fs_rate = TX_I2S_CTL_TX_I2S_FS_RATE_F_192_KHZ;
> + break;
Supported rates need to match (mentioned above).
> +static const struct snd_soc_dapm_route audio_map[] = {
Please use consistent name prefixes for variables
(msm8916_wcd_audio_map).
> +static struct snd_soc_dai_driver msm8916_wcd_codec_dai[] = {
> + [0] = {
> +.name = "msm8916_wcd_i2s_rx1",
> +.id = MSM8916_WCD_PLAYBACK_DAI,
> +.playback = {
> + .stream_name = "AIF1 Playback",
> + .rates = MSM8916_WCD_RATES,
> + .formats = MSM8916_WCD_FORMATS,
> + .rate_max = 192000,
Supported rates...
> +static int msm8916_wcd_probe(struct platform_device *pdev)
> +{
> + struct msm8916_wcd_chip *chip;
> + struct device *dev = >dev;
> + int ret;
> +
> + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->analog_map = dev_get_regmap(dev->parent, NULL);
> + if (!chip->analog_map)
> + return -ENXIO;
Does the analog_map need to be deallocated on later errors in this
function? or is dev_get_regmap a devm_ type function?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [RFC v2 1/3] ASoC: msm8916: Add codec Device Tree bindings.
<0x1 0xf0 0x7 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x0 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x1 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x2 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x3 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x4 IRQ_TYPE_NONE>,
> + <0x1 0xf1 0x5 IRQ_TYPE_NONE>;
> + interrupt-names = "spk_cnp_int",
> + "spk_clip_int",
> + "spk_ocp_int",
> + "ins_rem_det1",
> + "but_rel_det",
> + "but_press_det",
> + "ins_rem_det",
> + "mbhc_int",
> + "ear_ocp_int",
> + "hphr_ocp_int",
> + "hphl_ocp_det",
> + "ear_cnp_int",
> + "hphr_cnp_int",
> + "hphl_cnp_int";
> + vddio-supply = <_l5>;
> + vdd-tx-rx-supply = <_l5>;
> + vdd-micbias-supply = <_l13>;
> + qcom,lpass-codec-core = <_codec_core>;
Is there a reason the PMIC codec holds a phandle reference to the SoC
codec node? Rather than having the SoC codec hold a ref to the PMIC
codec?
> + #sound-dai-cells = <1>;
> + };
> +};
> +
> +soc {
> + ...
> + lpass_codec_core: lpass-codec{
> + compatible = "qcom,msm8916-lpass-codec", "syscon";
> + reg = <0x0771c000 0x400>;
> + };
> +
> +};
> --
> 2.8.2
>
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [RFC v2 3/3] ASoC: msm8916: Add msm8916-wcd codec driver
case 96000:
> + tx_fs_rate = TX_I2S_CTL_TX_I2S_FS_RATE_F_96_KHZ;
> + break;
> + case 192000:
> + tx_fs_rate = TX_I2S_CTL_TX_I2S_FS_RATE_F_192_KHZ;
> + break;
Supported rates need to match (mentioned above).
> +static const struct snd_soc_dapm_route audio_map[] = {
Please use consistent name prefixes for variables
(msm8916_wcd_audio_map).
> +static struct snd_soc_dai_driver msm8916_wcd_codec_dai[] = {
> + [0] = {
> +.name = "msm8916_wcd_i2s_rx1",
> +.id = MSM8916_WCD_PLAYBACK_DAI,
> +.playback = {
> + .stream_name = "AIF1 Playback",
> + .rates = MSM8916_WCD_RATES,
> + .formats = MSM8916_WCD_FORMATS,
> + .rate_max = 192000,
Supported rates...
> +static int msm8916_wcd_probe(struct platform_device *pdev)
> +{
> + struct msm8916_wcd_chip *chip;
> + struct device *dev = >dev;
> + int ret;
> +
> + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->analog_map = dev_get_regmap(dev->parent, NULL);
> + if (!chip->analog_map)
> + return -ENXIO;
Does the analog_map need to be deallocated on later errors in this
function? or is dev_get_regmap a devm_ type function?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v1 00/14] ASoC: qcom: Add capture support.
On Wed, Feb 10, 2016 at 02:16:58AM -0800, Srinivas Kandagatla wrote: > LPASS IP on QCOM SOC supports both Playback and capture > via I2S, but this feature is missing in existing code. > This patchset aims at adding capture support to lpass IP. > First few patches in this series does cleanup the driver > to make easy to add capture support. > > Most of these patches are acked by Kenneth. > > These patches are tested on DB410C with Headset Mic, secondary mic. With the exception of patch 13 (ASoC: qcom: add mic support), apply my ack to all remaining patches: Acked-by: Kenneth Westfield Once the comments in patch 13 are addressed, apply my ack to that as well. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v1 13/14] ASoC: qcom: add mic support
On Wed, Feb 10, 2016 at 02:20:03AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-platform.c
> b/sound/soc/qcom/lpass-platform.c
> index 26a046a..574aa33 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
...
> @@ -92,7 +93,13 @@ static int lpass_platform_pcmops_hw_params(struct
> snd_pcm_substream *substream,
> unsigned int regval;
> int dir = substream->stream;
> int bitwidth;
> - int ret, rdma_port = pcm_data->i2s_port + v->dmactl_audif_start;
> + int ch, ret, dma_port = pcm_data->i2s_port +
> v->dmactl_audif_start;
IMO, changing rdma_port to dma_port should be in patch 3 (ASoC: qcom:
rename rdmactl_audif_start to dmactrl_audif_start). However, as I
stated in my comments to your RFC submission, this is a nit.
...
> @@ -460,55 +483,106 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
> if (!data)
> return -ENOMEM;
>
> - if (v->alloc_dma_channel)
> - data->rdma_ch = v->alloc_dma_channel(drvdata,
> -
> SNDRV_PCM_STREAM_PLAYBACK);
> + data->i2s_port = cpu_dai->driver->id;
> + snd_soc_pcm_set_drvdata(soc_runtime, data);
>
> - if (IS_ERR_VALUE(data->rdma_ch))
> - return data->rdma_ch;
> + psubstream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
> + if (psubstream) {
> + if (v->alloc_dma_channel)
> + data->rdma_ch = v->alloc_dma_channel(drvdata,
> +
> SNDRV_PCM_STREAM_PLAYBACK);
>
> - drvdata->substream[data->rdma_ch] = substream;
> - data->i2s_port = cpu_dai->driver->id;
> + if (IS_ERR_VALUE(data->rdma_ch))
> + return data->rdma_ch;
>
> - snd_soc_pcm_set_drvdata(soc_runtime, data);
> + drvdata->substream[data->rdma_ch] = psubstream;
>
> - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> - soc_runtime->platform->dev,
> - size, >dma_buffer);
> - if (ret)
> - return ret;
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> pcm->card->dev,
> + size, >dma_buffer);
In patch 1 of this series (ASoC: qcom: use snd_dma_alloc/free* apis), you
correctly used the platform device for memory allocation. However, that is
then replaced by the soundcard device here ...
> + csubstream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
> + if (csubstream) {
> + if (v->alloc_dma_channel)
> + data->wrdma_ch = v->alloc_dma_channel(drvdata,
> + SNDRV_PCM_STREAM_CAPTURE);
> +
> + if (IS_ERR_VALUE(data->wrdma_ch))
> + goto capture_alloc_err;
> +
> + drvdata->substream[data->wrdma_ch] = csubstream;
> +
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> pcm->card->dev,
> + size, >dma_buffer);
> + if (ret)
> + goto capture_alloc_err;
... and here as well.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v1 13/14] ASoC: qcom: add mic support
On Wed, Feb 10, 2016 at 02:20:03AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-platform.c
> b/sound/soc/qcom/lpass-platform.c
> index 26a046a..574aa33 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
...
> @@ -92,7 +93,13 @@ static int lpass_platform_pcmops_hw_params(struct
> snd_pcm_substream *substream,
> unsigned int regval;
> int dir = substream->stream;
> int bitwidth;
> - int ret, rdma_port = pcm_data->i2s_port + v->dmactl_audif_start;
> + int ch, ret, dma_port = pcm_data->i2s_port +
> v->dmactl_audif_start;
IMO, changing rdma_port to dma_port should be in patch 3 (ASoC: qcom:
rename rdmactl_audif_start to dmactrl_audif_start). However, as I
stated in my comments to your RFC submission, this is a nit.
...
> @@ -460,55 +483,106 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
> if (!data)
> return -ENOMEM;
>
> - if (v->alloc_dma_channel)
> - data->rdma_ch = v->alloc_dma_channel(drvdata,
> -
> SNDRV_PCM_STREAM_PLAYBACK);
> + data->i2s_port = cpu_dai->driver->id;
> + snd_soc_pcm_set_drvdata(soc_runtime, data);
>
> - if (IS_ERR_VALUE(data->rdma_ch))
> - return data->rdma_ch;
> + psubstream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
> + if (psubstream) {
> + if (v->alloc_dma_channel)
> + data->rdma_ch = v->alloc_dma_channel(drvdata,
> +
> SNDRV_PCM_STREAM_PLAYBACK);
>
> - drvdata->substream[data->rdma_ch] = substream;
> - data->i2s_port = cpu_dai->driver->id;
> + if (IS_ERR_VALUE(data->rdma_ch))
> + return data->rdma_ch;
>
> - snd_soc_pcm_set_drvdata(soc_runtime, data);
> + drvdata->substream[data->rdma_ch] = psubstream;
>
> - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> - soc_runtime->platform->dev,
> - size, >dma_buffer);
> - if (ret)
> - return ret;
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> pcm->card->dev,
> + size, >dma_buffer);
In patch 1 of this series (ASoC: qcom: use snd_dma_alloc/free* apis), you
correctly used the platform device for memory allocation. However, that is
then replaced by the soundcard device here ...
> + csubstream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
> + if (csubstream) {
> + if (v->alloc_dma_channel)
> + data->wrdma_ch = v->alloc_dma_channel(drvdata,
> + SNDRV_PCM_STREAM_CAPTURE);
> +
> + if (IS_ERR_VALUE(data->wrdma_ch))
> + goto capture_alloc_err;
> +
> + drvdata->substream[data->wrdma_ch] = csubstream;
> +
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV,
> pcm->card->dev,
> + size, >dma_buffer);
> + if (ret)
> + goto capture_alloc_err;
... and here as well.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v1 00/14] ASoC: qcom: Add capture support.
On Wed, Feb 10, 2016 at 02:16:58AM -0800, Srinivas Kandagatla wrote: > LPASS IP on QCOM SOC supports both Playback and capture > via I2S, but this feature is missing in existing code. > This patchset aims at adding capture support to lpass IP. > First few patches in this series does cleanup the driver > to make easy to add capture support. > > Most of these patches are acked by Kenneth. > > These patches are tested on DB410C with Headset Mic, secondary mic. With the exception of patch 13 (ASoC: qcom: add mic support), apply my ack to all remaining patches: Acked-by: Kenneth Westfield <kwest...@codeaurora.org> Once the comments in patch 13 are addressed, apply my ack to that as well. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 15/15] ASoC: qcom: add mic support
On Mon, Feb 01, 2016 at 09:29:52AM -0800, Srinivas Kandagatla wrote: > This patch adds mic support to the lpass driver, most of the driver is > reused as it is, only the register level access is changed depending on > te direction of the stream. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-cpu.c | 113 +++ > sound/soc/qcom/lpass-platform.c | 166 > +--- > 2 files changed, 204 insertions(+), 75 deletions(-) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 14/15] ASoC: qcom: apq8016-sbc: add mic support
On Mon, Feb 01, 2016 at 09:29:46AM -0800, Srinivas Kandagatla wrote: > This patch add mic support on apq8016-sbc board aka db410c. Tested it > with headset mic. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/apq8016_sbc.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 11/15] ASoC: qcom: add wrdma register definations
On Mon, Feb 01, 2016 at 09:29:26AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma registers into the lpaif-reg.h. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-lpaif-reg.h | 11 +++ > 1 file changed, 11 insertions(+) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 09/15] ASoC: qcom: rename rdma_ch_bit_map to dma_ch_bit_map
On Mon, Feb 01, 2016 at 09:29:07AM -0800, Srinivas Kandagatla wrote: > rdma_ch_bit_map can be reused for wrdma channel allocations as wrdma > channel numbering start after rdma channel numbers. > With capture support referring rdma_ch_bit_map for wrdma channel > allocation > is confusing, so renaming rdma_ch_bit_map to dma_ch_bit_map makes sense. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-apq8016.c | 6 +++--- > sound/soc/qcom/lpass.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) As stated in my comments on patch 3 (ASoC: qcom: rename rdmactl_audif_start to dmactrl_audif_start), I believe this change could be combined with that patch. Or at the very least, have this patch be sequenced directly after patch 3. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 13/15] ASoC: qcom: apq8016: set the correct max register for regmap
On Mon, Feb 01, 2016 at 09:29:38AM -0800, Srinivas Kandagatla wrote: > Now that we are ready to access wrdma registers, set the max register > and other regmap related configs to use correct values. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-cpu.c | 33 +++-- > 1 file changed, 31 insertions(+), 2 deletions(-) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 12/15] ASoC: qcom: add generic bit masks for RDMA and WRDMA
On Mon, Feb 01, 2016 at 09:29:32AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-lpaif-reg.h > b/sound/soc/qcom/lpass-lpaif-reg.h > index 2008f9f..2114b3e 100644 > --- a/sound/soc/qcom/lpass-lpaif-reg.h > +++ b/sound/soc/qcom/lpass-lpaif-reg.h ... > +#define __LPAIF_DMA_REG(v, chan, dir, reg) \ > + (dir == SNDRV_PCM_STREAM_PLAYBACK) ? \ > + LPAIF_RDMA##reg##_REG(v, chan) : \ > + LPAIF_WRDMA##reg##_REG(v, chan) > + > +#define LPAIF_DMACTL_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, CTL) > +#define LPAIF_DMABASE_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, > BASE) > +#define LPAIF_DMABUFF_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, BUFF) > +#define LPAIF_DMACURR_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, > CURR) > +#define LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, PER) > +#define LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, PERCNT) > + > + > +#define LPAIF_DMACTL_AUDINTF(id) (id << LPAIF_DMACTL_AUDINTF_SHIFT) IMO, it would make more sense to place this definition here ... > + > +#define LPAIF_DMACTL_BURSTEN_MASK0x800 > +#define LPAIF_DMACTL_BURSTEN_SHIFT 11 > +#define LPAIF_DMACTL_BURSTEN_SINGLE (0 << LPAIF_DMACTL_BURSTEN_SHIFT) > +#define LPAIF_DMACTL_BURSTEN_INCR4 (1 << LPAIF_DMACTL_BURSTEN_SHIFT) > + > +#define LPAIF_DMACTL_WPSCNT_MASK 0x700 > +#define LPAIF_DMACTL_WPSCNT_SHIFT8 > +#define LPAIF_DMACTL_WPSCNT_ONE (0 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_TWO (1 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_THREE(2 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_FOUR (3 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_SIX (5 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_EIGHT(7 << LPAIF_DMACTL_WPSCNT_SHIFT) > + > +#define LPAIF_DMACTL_AUDINTF_MASK0x0F0 > +#define LPAIF_DMACTL_AUDINTF_SHIFT 4 ... #define LPAIF_DMACTL_AUDINTF(id) (id << LPAIF_DMACTL_AUDINTF_SHIFT) -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 07/15] ASoC: qcom: add mic related i2s control register defines
On Mon, Feb 01, 2016 at 09:28:55AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-lpaif-reg.h > b/sound/soc/qcom/lpass-lpaif-reg.h > index 95e22f1..8a64d1a 100644 > --- a/sound/soc/qcom/lpass-lpaif-reg.h > +++ b/sound/soc/qcom/lpass-lpaif-reg.h > @@ -47,6 +47,28 @@ ... > +#define LPAIF_I2SCTL_MICMODE_MASKGENMASK(7, 4) > +#define LPAIF_I2SCTL_MICMODE_SHIFT 4 > +#define LPAIF_I2SCTL_MICMODE_NONE(0 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD0 (1 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD1 (2 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD2 (3 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD3 (4 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_QUAD01 (5 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_QUAD02 (6 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_6CH (7 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_8CH (8 << LPAIF_I2SCTL_MICMODE_SHIFT) LPAIF_I2SCTL_MICMODE_QUAD02 should be LPAIF_I2SCTL_MICMODE_QUAD23, as the suffix numbers refer to SD[0-3]. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 10/15] ASoC: qcom: apq8016: add wrdma support
On Mon, Feb 01, 2016 at 09:29:20AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-apq8016.c
> b/sound/soc/qcom/lpass-apq8016.c
> index df44f09..3eef0c3 100644
> --- a/sound/soc/qcom/lpass-apq8016.c
> +++ b/sound/soc/qcom/lpass-apq8016.c
...
> @@ -213,7 +225,11 @@ static struct lpass_variant apq8016_data = {
> .rdma_reg_base = 0x8400,
> .rdma_reg_stride= 0x1000,
> .rdma_channels = 2,
> - .rdmactl_audif_start= 1,
> + .dmactl_audif_start = 1,
Changing rdma.. to dma.. should be done in patch 3 (ASoC: qcom: rename
rdmactl_audif_start to dmactrl_audif_start). The build should
fail otherwise if bisecting between this patch and patch 3.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 08/15] ASoC: qcom: add wrdma dma channel start
On Mon, Feb 01, 2016 at 09:29:01AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index 30449f3..8475b60 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -80,6 +80,7 @@ struct lpass_variant {
>* at different offset to ipq806x
>**/
> u32 dmactl_audif_start;
> + u32 wrdma_channel_start;
This patch should come before patch 6 (ASoC: qcom: ipq806x: add wrdma
related register offsets) as that patch references this field.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 05/15] ASoC: qcom: ipq806x: add error in dma allocation.
On Mon, Feb 01, 2016 at 09:28:43AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> index 119048c..8bdcdcb 100644
> --- a/sound/soc/qcom/lpass-ipq806x.c
> +++ b/sound/soc/qcom/lpass-ipq806x.c
> @@ -65,7 +65,10 @@ static struct snd_soc_dai_driver
> ipq806x_lpass_cpu_dai_driver = {
>
> static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata,
> int dir)
> {
> - return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
> + if (dir == SNDRV_PCM_STREAM_PLAYBACK)
> + return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
> + else/* Capture not supported */
> + return -EINVAL;
> }
The comment could be read as "Capture not supported by hardware", which
isn't true. Maybe "Capture currently not implemented"?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 06/15] ASoC: qcom: ipq806x: add wrdma related register offsets
On Mon, Feb 01, 2016 at 09:28:49AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma related register offsets to the lpass variant data > of ipq806x. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-ipq806x.c | 4 > 1 file changed, 4 insertions(+) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 04/15] ASoC: qcom: pass direction to dma allocation
On Mon, Feb 01, 2016 at 09:28:34AM -0800, Srinivas Kandagatla wrote: > This patch updates the internal dma allocation callbacks to take the > stream direction so that it can allocate channels suitable for that > stream direction. Before the capture support this was not necessary. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass-apq8016.c | 3 ++- > sound/soc/qcom/lpass-ipq806x.c | 2 +- > sound/soc/qcom/lpass.h | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 03/15] ASoC: qcom: rename rdmactl_audif_start to dmactrl_audif_start
On Mon, Feb 01, 2016 at 09:28:28AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-platform.c > b/sound/soc/qcom/lpass-platform.c > index a6dce1b..bfc9de6 100644 > --- a/sound/soc/qcom/lpass-platform.c > +++ b/sound/soc/qcom/lpass-platform.c > @@ -91,7 +91,7 @@ static int lpass_platform_pcmops_hw_params(struct > snd_pcm_substream *substream, > unsigned int channels = params_channels(params); > unsigned int regval; > int bitwidth; > - int ret, rdma_port = pcm_data->i2s_port + v->rdmactl_audif_start; > + int ret, rdma_port = pcm_data->i2s_port + v->dmactl_audif_start; I was wondering why rdma_port had not been changed as well, until I saw that in a later patch. Would it make sense to combine all changes related to removing read-only indications from identifiers to one patch? -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 02/15] ASoC: qcom: add wrdma register details to lpass_variant
On Mon, Feb 01, 2016 at 09:28:20AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma related register offsets and shifts into lpass > variant structure. > > Signed-off-by: Srinivas Kandagatla > --- > sound/soc/qcom/lpass.h | 3 +++ > 1 file changed, 3 insertions(+) Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 01/15] ASoC: qcom: use snd_dma_alloc/free* apis
On Mon, Feb 01, 2016 at 09:27:59AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-platform.c
> b/sound/soc/qcom/lpass-platform.c
> index 4aeb8e1..a6dce1b 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -439,39 +439,6 @@ static irqreturn_t lpass_platform_lpaif_irq(int irq,
> void *data)
> return IRQ_HANDLED;
> }
>
> -static int lpass_platform_alloc_buffer(struct snd_pcm_substream
> *substream,
> - struct snd_soc_pcm_runtime *rt)
> -{
> - struct snd_dma_buffer *buf = >dma_buffer;
> - size_t size = lpass_platform_pcm_hardware.buffer_bytes_max;
> -
> - buf->dev.type = SNDRV_DMA_TYPE_DEV;
> - buf->dev.dev = rt->platform->dev;
> - buf->private_data = NULL;
> - buf->area = dma_alloc_coherent(rt->platform->dev, size,
> >addr,
> - GFP_KERNEL);
> - if (!buf->area) {
> - dev_err(rt->platform->dev, "%s: Could not allocate DMA
> buffer\n",
> - __func__);
> - return -ENOMEM;
> - }
...
> @@ -499,7 +467,8 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
>
> snd_soc_pcm_set_drvdata(soc_runtime, data);
>
> - ret = lpass_platform_alloc_buffer(substream, soc_runtime);
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->card->dev,
> + size, >dma_buffer);
> if (ret)
> return ret;
>
Is there a particular reason for using the soundcard device (pcm-card->dev)
rather than the platform device (rt->platform->dev) for memory
allocation? Especially considering you posted a fix for this several
weeks ago (ASoC: qcom: use correct device pointer in dma allocation).
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 00/15] ASoC: qcom: Add capture support.
On Mon, Feb 01, 2016 at 09:27:02AM -0800, Srinivas Kandagatla wrote: > LPASS IP on QCOM SOC supports both Playback and capture > via I2S, but this feature is missing in existing code. > This patchset aims at adding capture support to lpass IP. > First few patches in this series does cleanup the driver > to make easy to add capture support. > > These patches are tested on DB410C with Headset Mic. Thanks for posting this. I went through the changes, and it mostly looks good minus some small, easy-to-fix nits. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 09/15] ASoC: qcom: rename rdma_ch_bit_map to dma_ch_bit_map
On Mon, Feb 01, 2016 at 09:29:07AM -0800, Srinivas Kandagatla wrote: > rdma_ch_bit_map can be reused for wrdma channel allocations as wrdma > channel numbering start after rdma channel numbers. > With capture support referring rdma_ch_bit_map for wrdma channel > allocation > is confusing, so renaming rdma_ch_bit_map to dma_ch_bit_map makes sense. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-apq8016.c | 6 +++--- > sound/soc/qcom/lpass.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) As stated in my comments on patch 3 (ASoC: qcom: rename rdmactl_audif_start to dmactrl_audif_start), I believe this change could be combined with that patch. Or at the very least, have this patch be sequenced directly after patch 3. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 07/15] ASoC: qcom: add mic related i2s control register defines
On Mon, Feb 01, 2016 at 09:28:55AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-lpaif-reg.h > b/sound/soc/qcom/lpass-lpaif-reg.h > index 95e22f1..8a64d1a 100644 > --- a/sound/soc/qcom/lpass-lpaif-reg.h > +++ b/sound/soc/qcom/lpass-lpaif-reg.h > @@ -47,6 +47,28 @@ ... > +#define LPAIF_I2SCTL_MICMODE_MASKGENMASK(7, 4) > +#define LPAIF_I2SCTL_MICMODE_SHIFT 4 > +#define LPAIF_I2SCTL_MICMODE_NONE(0 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD0 (1 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD1 (2 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD2 (3 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_SD3 (4 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_QUAD01 (5 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_QUAD02 (6 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_6CH (7 << LPAIF_I2SCTL_MICMODE_SHIFT) > +#define LPAIF_I2SCTL_MICMODE_8CH (8 << LPAIF_I2SCTL_MICMODE_SHIFT) LPAIF_I2SCTL_MICMODE_QUAD02 should be LPAIF_I2SCTL_MICMODE_QUAD23, as the suffix numbers refer to SD[0-3]. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 10/15] ASoC: qcom: apq8016: add wrdma support
On Mon, Feb 01, 2016 at 09:29:20AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-apq8016.c
> b/sound/soc/qcom/lpass-apq8016.c
> index df44f09..3eef0c3 100644
> --- a/sound/soc/qcom/lpass-apq8016.c
> +++ b/sound/soc/qcom/lpass-apq8016.c
...
> @@ -213,7 +225,11 @@ static struct lpass_variant apq8016_data = {
> .rdma_reg_base = 0x8400,
> .rdma_reg_stride= 0x1000,
> .rdma_channels = 2,
> - .rdmactl_audif_start= 1,
> + .dmactl_audif_start = 1,
Changing rdma.. to dma.. should be done in patch 3 (ASoC: qcom: rename
rdmactl_audif_start to dmactrl_audif_start). The build should
fail otherwise if bisecting between this patch and patch 3.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 13/15] ASoC: qcom: apq8016: set the correct max register for regmap
On Mon, Feb 01, 2016 at 09:29:38AM -0800, Srinivas Kandagatla wrote: > Now that we are ready to access wrdma registers, set the max register > and other regmap related configs to use correct values. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-cpu.c | 33 +++-- > 1 file changed, 31 insertions(+), 2 deletions(-) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 03/15] ASoC: qcom: rename rdmactl_audif_start to dmactrl_audif_start
On Mon, Feb 01, 2016 at 09:28:28AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-platform.c > b/sound/soc/qcom/lpass-platform.c > index a6dce1b..bfc9de6 100644 > --- a/sound/soc/qcom/lpass-platform.c > +++ b/sound/soc/qcom/lpass-platform.c > @@ -91,7 +91,7 @@ static int lpass_platform_pcmops_hw_params(struct > snd_pcm_substream *substream, > unsigned int channels = params_channels(params); > unsigned int regval; > int bitwidth; > - int ret, rdma_port = pcm_data->i2s_port + v->rdmactl_audif_start; > + int ret, rdma_port = pcm_data->i2s_port + v->dmactl_audif_start; I was wondering why rdma_port had not been changed as well, until I saw that in a later patch. Would it make sense to combine all changes related to removing read-only indications from identifiers to one patch? -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 12/15] ASoC: qcom: add generic bit masks for RDMA and WRDMA
On Mon, Feb 01, 2016 at 09:29:32AM -0800, Srinivas Kandagatla wrote: > diff --git a/sound/soc/qcom/lpass-lpaif-reg.h > b/sound/soc/qcom/lpass-lpaif-reg.h > index 2008f9f..2114b3e 100644 > --- a/sound/soc/qcom/lpass-lpaif-reg.h > +++ b/sound/soc/qcom/lpass-lpaif-reg.h ... > +#define __LPAIF_DMA_REG(v, chan, dir, reg) \ > + (dir == SNDRV_PCM_STREAM_PLAYBACK) ? \ > + LPAIF_RDMA##reg##_REG(v, chan) : \ > + LPAIF_WRDMA##reg##_REG(v, chan) > + > +#define LPAIF_DMACTL_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, CTL) > +#define LPAIF_DMABASE_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, > BASE) > +#define LPAIF_DMABUFF_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, BUFF) > +#define LPAIF_DMACURR_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, > CURR) > +#define LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, PER) > +#define LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, > dir, PERCNT) > + > + > +#define LPAIF_DMACTL_AUDINTF(id) (id << LPAIF_DMACTL_AUDINTF_SHIFT) IMO, it would make more sense to place this definition here ... > + > +#define LPAIF_DMACTL_BURSTEN_MASK0x800 > +#define LPAIF_DMACTL_BURSTEN_SHIFT 11 > +#define LPAIF_DMACTL_BURSTEN_SINGLE (0 << LPAIF_DMACTL_BURSTEN_SHIFT) > +#define LPAIF_DMACTL_BURSTEN_INCR4 (1 << LPAIF_DMACTL_BURSTEN_SHIFT) > + > +#define LPAIF_DMACTL_WPSCNT_MASK 0x700 > +#define LPAIF_DMACTL_WPSCNT_SHIFT8 > +#define LPAIF_DMACTL_WPSCNT_ONE (0 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_TWO (1 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_THREE(2 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_FOUR (3 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_SIX (5 << LPAIF_DMACTL_WPSCNT_SHIFT) > +#define LPAIF_DMACTL_WPSCNT_EIGHT(7 << LPAIF_DMACTL_WPSCNT_SHIFT) > + > +#define LPAIF_DMACTL_AUDINTF_MASK0x0F0 > +#define LPAIF_DMACTL_AUDINTF_SHIFT 4 ... #define LPAIF_DMACTL_AUDINTF(id) (id << LPAIF_DMACTL_AUDINTF_SHIFT) -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 05/15] ASoC: qcom: ipq806x: add error in dma allocation.
On Mon, Feb 01, 2016 at 09:28:43AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> index 119048c..8bdcdcb 100644
> --- a/sound/soc/qcom/lpass-ipq806x.c
> +++ b/sound/soc/qcom/lpass-ipq806x.c
> @@ -65,7 +65,10 @@ static struct snd_soc_dai_driver
> ipq806x_lpass_cpu_dai_driver = {
>
> static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata,
> int dir)
> {
> - return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
> + if (dir == SNDRV_PCM_STREAM_PLAYBACK)
> + return IPQ806X_LPAIF_RDMA_CHAN_MI2S;
> + else/* Capture not supported */
> + return -EINVAL;
> }
The comment could be read as "Capture not supported by hardware", which
isn't true. Maybe "Capture currently not implemented"?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 06/15] ASoC: qcom: ipq806x: add wrdma related register offsets
On Mon, Feb 01, 2016 at 09:28:49AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma related register offsets to the lpass variant data > of ipq806x. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-ipq806x.c | 4 > 1 file changed, 4 insertions(+) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 08/15] ASoC: qcom: add wrdma dma channel start
On Mon, Feb 01, 2016 at 09:29:01AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index 30449f3..8475b60 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -80,6 +80,7 @@ struct lpass_variant {
>* at different offset to ipq806x
>**/
> u32 dmactl_audif_start;
> + u32 wrdma_channel_start;
This patch should come before patch 6 (ASoC: qcom: ipq806x: add wrdma
related register offsets) as that patch references this field.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 11/15] ASoC: qcom: add wrdma register definations
On Mon, Feb 01, 2016 at 09:29:26AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma registers into the lpaif-reg.h. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-lpaif-reg.h | 11 +++ > 1 file changed, 11 insertions(+) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 04/15] ASoC: qcom: pass direction to dma allocation
On Mon, Feb 01, 2016 at 09:28:34AM -0800, Srinivas Kandagatla wrote: > This patch updates the internal dma allocation callbacks to take the > stream direction so that it can allocate channels suitable for that > stream direction. Before the capture support this was not necessary. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-apq8016.c | 3 ++- > sound/soc/qcom/lpass-ipq806x.c | 2 +- > sound/soc/qcom/lpass.h | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 15/15] ASoC: qcom: add mic support
On Mon, Feb 01, 2016 at 09:29:52AM -0800, Srinivas Kandagatla wrote: > This patch adds mic support to the lpass driver, most of the driver is > reused as it is, only the register level access is changed depending on > te direction of the stream. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass-cpu.c | 113 +++ > sound/soc/qcom/lpass-platform.c | 166 > +--- > 2 files changed, 204 insertions(+), 75 deletions(-) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 14/15] ASoC: qcom: apq8016-sbc: add mic support
On Mon, Feb 01, 2016 at 09:29:46AM -0800, Srinivas Kandagatla wrote: > This patch add mic support on apq8016-sbc board aka db410c. Tested it > with headset mic. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/apq8016_sbc.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 00/15] ASoC: qcom: Add capture support.
On Mon, Feb 01, 2016 at 09:27:02AM -0800, Srinivas Kandagatla wrote: > LPASS IP on QCOM SOC supports both Playback and capture > via I2S, but this feature is missing in existing code. > This patchset aims at adding capture support to lpass IP. > First few patches in this series does cleanup the driver > to make easy to add capture support. > > These patches are tested on DB410C with Headset Mic. Thanks for posting this. I went through the changes, and it mostly looks good minus some small, easy-to-fix nits. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 01/15] ASoC: qcom: use snd_dma_alloc/free* apis
On Mon, Feb 01, 2016 at 09:27:59AM -0800, Srinivas Kandagatla wrote:
> diff --git a/sound/soc/qcom/lpass-platform.c
> b/sound/soc/qcom/lpass-platform.c
> index 4aeb8e1..a6dce1b 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -439,39 +439,6 @@ static irqreturn_t lpass_platform_lpaif_irq(int irq,
> void *data)
> return IRQ_HANDLED;
> }
>
> -static int lpass_platform_alloc_buffer(struct snd_pcm_substream
> *substream,
> - struct snd_soc_pcm_runtime *rt)
> -{
> - struct snd_dma_buffer *buf = >dma_buffer;
> - size_t size = lpass_platform_pcm_hardware.buffer_bytes_max;
> -
> - buf->dev.type = SNDRV_DMA_TYPE_DEV;
> - buf->dev.dev = rt->platform->dev;
> - buf->private_data = NULL;
> - buf->area = dma_alloc_coherent(rt->platform->dev, size,
> >addr,
> - GFP_KERNEL);
> - if (!buf->area) {
> - dev_err(rt->platform->dev, "%s: Could not allocate DMA
> buffer\n",
> - __func__);
> - return -ENOMEM;
> - }
...
> @@ -499,7 +467,8 @@ static int lpass_platform_pcm_new(struct
> snd_soc_pcm_runtime *soc_runtime)
>
> snd_soc_pcm_set_drvdata(soc_runtime, data);
>
> - ret = lpass_platform_alloc_buffer(substream, soc_runtime);
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->card->dev,
> + size, >dma_buffer);
> if (ret)
> return ret;
>
Is there a particular reason for using the soundcard device (pcm-card->dev)
rather than the platform device (rt->platform->dev) for memory
allocation? Especially considering you posted a fix for this several
weeks ago (ASoC: qcom: use correct device pointer in dma allocation).
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH RFC 02/15] ASoC: qcom: add wrdma register details to lpass_variant
On Mon, Feb 01, 2016 at 09:28:20AM -0800, Srinivas Kandagatla wrote: > This patch adds wrdma related register offsets and shifts into lpass > variant structure. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > sound/soc/qcom/lpass.h | 3 +++ > 1 file changed, 3 insertions(+) Acked-by: Kenneth Westfield <kwest...@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 12:54:15PM -0700, Julia Lawall wrote:
> On Sun, 30 Aug 2015, walter harms wrote:
> > Am 30.08.2015 20:05, schrieb Julia Lawall:
> > > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > > dev_err(>dev,
> > > "%s() error getting mi2s-bit-clk: %ld\n",
> > > - __func__,
> PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > > + __func__,
> > > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > > }
> > > }
> > >
> >
> > just a note:
> > using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould
> help to make the code
> > more readable (yes, the other code is alike). something like:
> >
> > struct clk *tmp = devm_clk_get(>dev,clk_name);
>
> Where do you suggest to put this?
>
> Maybe it would be reasonable to declare a variable struct clk *clk; at the
>
> top of the function, and then use that as a temporary variable for all
> three calls.
>
> However, now I see that the first call, unlike the other two doesn't cause
>
> a return from the function.
>
> if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
> dev_warn(>dev,
> "%s() error getting mi2s-osr-clk: %ld\n",
> __func__,
> PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
> }
>
> Is that intentional?
Yes, that was intentional as the presense of the OSR clock in the DT
node is optional.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 11:54:33AM -0700, walter harms wrote:
> Am 30.08.2015 20:05, schrieb Julia Lawall:
> > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > dev_err(>dev,
> > "%s() error getting mi2s-bit-clk: %ld\n",
> > - __func__,
> PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > + __func__,
> > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > }
> > }
> >
>
> just a note:
> using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help
> to make the code
> more readable (yes, the other code is alike). something like:
>
> struct clk *tmp = devm_clk_get(>dev,clk_name);
>
> if (IS_ERR(tmp)) {
> dev_err(>dev,"%s() error getting mi2s-bit-clk:
> %ld\n",__func__, PTR_ERR(tmp));
> return PTR_ERR(tmp);
> }
> drvdata->mi2s_bit_clk[dai_id]=tmp;
Yes, it would make the code more readable.
>
>
> just one minor:
> the dev_warn() just before says: " error getting mi2s-osr-clk" may be it
> should be "warnig ..."
> That will make it more easy to rep for real error in a log.
"error [gs]etting" could be re-phrased to "could not [gs]et". The term
"error" was not meant to indicate the log level, but evidently, can
cause some confusion for someone reading the logs.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 11:05:10AM -0700, Julia Lawall wrote:
> Apply PTR_ERR to the value that was recently assigned.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @@
> expression x,y;
> @@
>
> if (IS_ERR(x) || ...) {
> ... when any
> when != IS_ERR(...)
> (
>PTR_ERR(x)
> |
> * PTR_ERR(y)
> )
> ... when any
> }
> //
>
> Signed-off-by: Julia Lawall
>
> ---
The patch itself looks good. Thanks.
Acked-by: Kenneth Westfield
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 11:54:33AM -0700, walter harms wrote:
> Am 30.08.2015 20:05, schrieb Julia Lawall:
> > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > dev_err(>dev,
> > "%s() error getting mi2s-bit-clk: %ld\n",
> > - __func__,
> PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > + __func__,
> > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > }
> > }
> >
>
> just a note:
> using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help
> to make the code
> more readable (yes, the other code is alike). something like:
>
> struct clk *tmp = devm_clk_get(>dev,clk_name);
>
> if (IS_ERR(tmp)) {
> dev_err(>dev,"%s() error getting mi2s-bit-clk:
> %ld\n",__func__, PTR_ERR(tmp));
> return PTR_ERR(tmp);
> }
> drvdata->mi2s_bit_clk[dai_id]=tmp;
Yes, it would make the code more readable.
>
>
> just one minor:
> the dev_warn() just before says: " error getting mi2s-osr-clk" may be it
> should be "warnig ..."
> That will make it more easy to rep for real error in a log.
"error [gs]etting" could be re-phrased to "could not [gs]et". The term
"error" was not meant to indicate the log level, but evidently, can
cause some confusion for someone reading the logs.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 11:05:10AM -0700, Julia Lawall wrote:
> Apply PTR_ERR to the value that was recently assigned.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @@
> expression x,y;
> @@
>
> if (IS_ERR(x) || ...) {
> ... when any
> when != IS_ERR(...)
> (
>PTR_ERR(x)
> |
> * PTR_ERR(y)
> )
> ... when any
> }
> //
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
>
> ---
The patch itself looks good. Thanks.
Acked-by: Kenneth Westfield <kwest...@codeaurora.org>
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument
On Sun, Aug 30, 2015 at 12:54:15PM -0700, Julia Lawall wrote:
> On Sun, 30 Aug 2015, walter harms wrote:
> > Am 30.08.2015 20:05, schrieb Julia Lawall:
> > > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > > dev_err(>dev,
> > > "%s() error getting mi2s-bit-clk: %ld\n",
> > > - __func__,
> PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > > + __func__,
> > > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > > }
> > > }
> > >
> >
> > just a note:
> > using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould
> help to make the code
> > more readable (yes, the other code is alike). something like:
> >
> > struct clk *tmp = devm_clk_get(>dev,clk_name);
>
> Where do you suggest to put this?
>
> Maybe it would be reasonable to declare a variable struct clk *clk; at the
>
> top of the function, and then use that as a temporary variable for all
> three calls.
>
> However, now I see that the first call, unlike the other two doesn't cause
>
> a return from the function.
>
> if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
> dev_warn(>dev,
> "%s() error getting mi2s-osr-clk: %ld\n",
> __func__,
> PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
> }
>
> Is that intentional?
Yes, that was intentional as the presense of the OSR clock in the DT
node is optional.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ASoC: qcom: remove Kconfig deps from variant configs
From: Kenneth Westfield Remove the SND_SOC_QCOM dependency from the variant configs. The board configs, which select the variants, already have this dependency. Signed-off-by: Kenneth Westfield --- sound/soc/qcom/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index d6ccda7..3cc252e 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -15,13 +15,11 @@ config SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_IPQ806X tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_APQ8016 tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ASoC: qcom: move board Kconfig deps to parent config
From: Kenneth Westfield Rather than have each board define the same set of dependencies; move the common dependencies to the SND_SOC_QCOM parent config. Signed-off-by: Kenneth Westfield --- sound/soc/qcom/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 807fedf..d6ccda7 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -1,5 +1,6 @@ config SND_SOC_QCOM tristate "ASoC support for QCOM platforms" + depends on ARCH_QCOM || COMPILE_TEST help Say Y or M if you want to add support to use audio devices in Qualcomm Technologies SOC-based platforms. @@ -26,7 +27,7 @@ config SND_SOC_LPASS_APQ8016 config SND_SOC_STORM tristate "ASoC I2S support for Storm boards" - depends on SND_SOC_QCOM && (ARCH_QCOM || COMPILE_TEST) + depends on SND_SOC_QCOM select SND_SOC_LPASS_IPQ806X select SND_SOC_MAX98357A help @@ -35,7 +36,7 @@ config SND_SOC_STORM config SND_SOC_APQ8016_SBC tristate "SoC Audio support for APQ8016 SBC platforms" - depends on SND_SOC_QCOM && (ARCH_QCOM || COMPILE_TEST) + depends on SND_SOC_QCOM select SND_SOC_LPASS_APQ8016 help Support for Qualcomm Technologies LPASS audio block in -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] ASoC: qcom: move board Kconfig deps to parent config
From: Kenneth Westfield kwest...@codeaurora.org Rather than have each board define the same set of dependencies; move the common dependencies to the SND_SOC_QCOM parent config. Signed-off-by: Kenneth Westfield kwest...@codeaurora.org --- sound/soc/qcom/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 807fedf..d6ccda7 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -1,5 +1,6 @@ config SND_SOC_QCOM tristate ASoC support for QCOM platforms + depends on ARCH_QCOM || COMPILE_TEST help Say Y or M if you want to add support to use audio devices in Qualcomm Technologies SOC-based platforms. @@ -26,7 +27,7 @@ config SND_SOC_LPASS_APQ8016 config SND_SOC_STORM tristate ASoC I2S support for Storm boards - depends on SND_SOC_QCOM (ARCH_QCOM || COMPILE_TEST) + depends on SND_SOC_QCOM select SND_SOC_LPASS_IPQ806X select SND_SOC_MAX98357A help @@ -35,7 +36,7 @@ config SND_SOC_STORM config SND_SOC_APQ8016_SBC tristate SoC Audio support for APQ8016 SBC platforms - depends on SND_SOC_QCOM (ARCH_QCOM || COMPILE_TEST) + depends on SND_SOC_QCOM select SND_SOC_LPASS_APQ8016 help Support for Qualcomm Technologies LPASS audio block in -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ASoC: qcom: remove Kconfig deps from variant configs
From: Kenneth Westfield kwest...@codeaurora.org Remove the SND_SOC_QCOM dependency from the variant configs. The board configs, which select the variants, already have this dependency. Signed-off-by: Kenneth Westfield kwest...@codeaurora.org --- sound/soc/qcom/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index d6ccda7..3cc252e 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -15,13 +15,11 @@ config SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_IPQ806X tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_APQ8016 tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ASoC: qcom: Remove QCOM dependency from LPASS variants
From: Kenneth Westfield
Building with the following config options ...
CONFIG_COMPILE_TEST=y
# CONFIG_SND_SOC_QCOM is not set
CONFIG_SND_SOC_LPASS_IPQ806X=m
CONFIG_SND_SOC_STORM=m
... causes the following build warning:
warning: (SND_SOC_STORM) selects SND_SOC_LPASS_IPQ806X
which has unmet direct dependencies (SOUND && !M68K &&
!UML && SND && SND_SOC && SND_SOC_QCOM)
To fix this, the dependency on SND_SOC_QCOM from the
user-invisible LPASS variant config options has been
removed. This will allow for successful randconfig
builds that use COMPILE_TEST.
Reported-by: Jim Davis
Fixes: dc1ebd1811e9 ("ASoC: qcom: Add apq8016 lpass driver support")
Fixes: 9bae4880acee ("ASoC: qcom: move ipq806x specific bits out of lpass
driver")
Signed-off-by: Kenneth Westfield
---
Created fix in response to Jim's bug report:
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092870.html
sound/soc/qcom/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 938144c..290b056 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -14,13 +14,11 @@ config SND_SOC_LPASS_PLATFORM
config SND_SOC_LPASS_IPQ806X
tristate
- depends on SND_SOC_QCOM
select SND_SOC_LPASS_CPU
select SND_SOC_LPASS_PLATFORM
config SND_SOC_LPASS_APQ8016
tristate
- depends on SND_SOC_QCOM
select SND_SOC_LPASS_CPU
select SND_SOC_LPASS_PLATFORM
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ASoC: qcom: Remove QCOM dependency from LPASS variants
From: Kenneth Westfield kwest...@codeaurora.org Building with the following config options ... CONFIG_COMPILE_TEST=y # CONFIG_SND_SOC_QCOM is not set CONFIG_SND_SOC_LPASS_IPQ806X=m CONFIG_SND_SOC_STORM=m ... causes the following build warning: warning: (SND_SOC_STORM) selects SND_SOC_LPASS_IPQ806X which has unmet direct dependencies (SOUND !M68K !UML SND SND_SOC SND_SOC_QCOM) To fix this, the dependency on SND_SOC_QCOM from the user-invisible LPASS variant config options has been removed. This will allow for successful randconfig builds that use COMPILE_TEST. Reported-by: Jim Davis jim.ep...@gmail.com Fixes: dc1ebd1811e9 (ASoC: qcom: Add apq8016 lpass driver support) Fixes: 9bae4880acee (ASoC: qcom: move ipq806x specific bits out of lpass driver) Signed-off-by: Kenneth Westfield kwest...@codeaurora.org --- Created fix in response to Jim's bug report: http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092870.html sound/soc/qcom/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 938144c..290b056 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -14,13 +14,11 @@ config SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_IPQ806X tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_APQ8016 tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ASoC: qcom: Remove QCOM dependency from LPASS variants
From: Kenneth Westfield
Remove the dependency on SND_SOC_QCOM from the user-
invisible LPASS variant options. This will fix
randconfig build errors.
Reported-by: Jim Davis
Fixes: dc1ebd1811e9 ("ASoC: qcom: Add apq8016 lpass driver support")
Fixes: 9bae4880acee ("ASoC: qcom: move ipq806x specific bits out of lpass
driver")
Signed-off-by: Kenneth Westfield
---
Created fix in response to Jim's bug report:
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092870.html
Note: I did some searching but didn't see an example of a fix for the
same type of bug introduced by 2 commits. If this needs to be split into
two fixes, let me know.
sound/soc/qcom/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 29fff6d..10837bd 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -14,13 +14,11 @@ config SND_SOC_LPASS_PLATFORM
config SND_SOC_LPASS_IPQ806X
tristate
- depends on SND_SOC_QCOM
select SND_SOC_LPASS_CPU
select SND_SOC_LPASS_PLATFORM
config SND_SOC_LPASS_APQ8016
tristate
- depends on SND_SOC_QCOM
select SND_SOC_LPASS_CPU
select SND_SOC_LPASS_PLATFORM
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ASoC: qcom: Remove QCOM dependency from LPASS variants
From: Kenneth Westfield kwest...@codeaurora.org Remove the dependency on SND_SOC_QCOM from the user- invisible LPASS variant options. This will fix randconfig build errors. Reported-by: Jim Davis jim.ep...@gmail.com Fixes: dc1ebd1811e9 (ASoC: qcom: Add apq8016 lpass driver support) Fixes: 9bae4880acee (ASoC: qcom: move ipq806x specific bits out of lpass driver) Signed-off-by: Kenneth Westfield kwest...@codeaurora.org --- Created fix in response to Jim's bug report: http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092870.html Note: I did some searching but didn't see an example of a fix for the same type of bug introduced by 2 commits. If this needs to be split into two fixes, let me know. sound/soc/qcom/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 29fff6d..10837bd 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -14,13 +14,11 @@ config SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_IPQ806X tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM config SND_SOC_LPASS_APQ8016 tristate - depends on SND_SOC_QCOM select SND_SOC_LPASS_CPU select SND_SOC_LPASS_PLATFORM -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/13] ASoC: qcom: add support to apq8016 audio
On Sat, May 16, 2015 at 05:31:02AM -0700, Srinivas Kandagatla wrote: > Thankyou for reviewing the v1 patches, here is the v2 patchset after > incorporating review comments and testing on Storm Board. > This patchset adds apq8016 audio support into lpass driver. Existing > Lpass driver can not be used as-it-is for apq8016 as it contains code > specific to ipq806x. Also the driver only supports single i2s port, > single dma channel and single bitclk control. > > APQ8016 has 4 MI2S( Primary, Secondary, Tertiary, Quaternary) which can be > routed > to internal wcd codec or external codecs. This routing is controlled by 2 > mux > registers. > > This patch series firstly re-organizes the lpass driver such that the SOC > specific bits are moved away from the driver. And secondly the SOC > specifics > are now passed as lpass variant data which would include various register > offsets, dma channel allocations and SOC specific clock handling. > > Finally the last few patchs add apq8016 lpass and machine driver. > > All these patches are tested for HDMI audio via adv7533 bridge and Analog > audio > on APQ8016-SBC, msm8916-mtp boards and Kenneth tested this patchset on > ipq806x Storm board too. Other than the two comments on patches 5 and 6, everything else looks fine to me. Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 06/13] ASoC: qcom: make osr clock optional
On Sat, May 16, 2015 at 05:32:49AM -0700, Srinivas Kandagatla wrote: > Some LPASS integrations like on APQ8016 do not have OSR clk, so making > osr clk optional would allow such integrations to use lpass driver. > @@ -415,7 +424,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct > platform_device *pdev) > dev_err(>dev, > "%s() error getting mi2s-osr-clk: %ld\n", > __func__, > - return PTR_ERR(drvdata->mi2s_osr_clk[dai_id])); > + PTR_ERR(drvdata->mi2s_osr_clk[dai_id])); > } NIT: Perhaps make this a dev_warn or dev_notice log message, as it's no longer really an error. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 05/13] ASoC: qcom: support bitclk and osrclk per i2s port
On Sat, May 16, 2015 at 05:32:42AM -0700, Srinivas Kandagatla wrote:
> This patch adds support to allow bitclk and osrclk per i2s dai port.
> on APQ8016 there are 4 i2s ports each one has its own bit clks.
>
> Without this patch its not possible to support multiple i2s ports in the
> lpass driver.
> @@ -400,18 +402,34 @@ int asoc_qcom_lpass_cpu_platform_probe(struct
> platform_device *pdev)
> if (variant->init)
> variant->init(pdev);
>
> - drvdata->mi2s_osr_clk = devm_clk_get(>dev, "mi2s-osr-clk");
> - if (IS_ERR(drvdata->mi2s_osr_clk)) {
> - dev_err(>dev, "%s() error getting mi2s-osr-clk:
> %ld\n",
> - __func__, PTR_ERR(drvdata->mi2s_osr_clk));
> - return PTR_ERR(drvdata->mi2s_osr_clk);
> - }
> -
> - drvdata->mi2s_bit_clk = devm_clk_get(>dev, "mi2s-bit-clk");
> - if (IS_ERR(drvdata->mi2s_bit_clk)) {
> - dev_err(>dev, "%s() error getting mi2s-bit-clk:
> %ld\n",
> - __func__, PTR_ERR(drvdata->mi2s_bit_clk));
> - return PTR_ERR(drvdata->mi2s_bit_clk);
> + for (i = 0; i < variant->num_dai; i++) {
> + dai_id = variant->dai_driver[i].id;
> + if (variant->num_dai > 1)
> + sprintf(clk_name, "mi2s-osr-clk%d", i);
> + else
> + sprintf(clk_name, "mi2s-osr-clk");
> +
> + drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(>dev,
> + clk_name);
> + if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
> + dev_err(>dev,
> + "%s() error getting mi2s-osr-clk: %ld\n",
> + __func__,
> + return PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
Should the previous two lines be:
+ __func__, PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
+ return PTR_ERR(drvdata->mi2s_osr_clk[dai_id]);
as you remove the return in patch 6?
> + }
> +
> + if (variant->num_dai > 1)
> + sprintf(clk_name, "mi2s-bit-clk%d", i);
> + else
> + sprintf(clk_name, "mi2s-bit-clk");
> +
> + drvdata->mi2s_bit_clk[dai_id] = devm_clk_get(>dev,
> clk_name);
> + if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> + dev_err(>dev,
> + "%s() error getting mi2s-bit-clk: %ld\n",
> + __func__,
> PTR_ERR(drvdata->mi2s_bit_clk[i]));
> + return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> + }
> }
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 06/13] ASoC: qcom: make osr clock optional
On Sat, May 16, 2015 at 05:32:49AM -0700, Srinivas Kandagatla wrote: Some LPASS integrations like on APQ8016 do not have OSR clk, so making osr clk optional would allow such integrations to use lpass driver. @@ -415,7 +424,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) dev_err(pdev-dev, %s() error getting mi2s-osr-clk: %ld\n, __func__, - return PTR_ERR(drvdata-mi2s_osr_clk[dai_id])); + PTR_ERR(drvdata-mi2s_osr_clk[dai_id])); } NIT: Perhaps make this a dev_warn or dev_notice log message, as it's no longer really an error. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 00/13] ASoC: qcom: add support to apq8016 audio
On Sat, May 16, 2015 at 05:31:02AM -0700, Srinivas Kandagatla wrote: Thankyou for reviewing the v1 patches, here is the v2 patchset after incorporating review comments and testing on Storm Board. This patchset adds apq8016 audio support into lpass driver. Existing Lpass driver can not be used as-it-is for apq8016 as it contains code specific to ipq806x. Also the driver only supports single i2s port, single dma channel and single bitclk control. APQ8016 has 4 MI2S( Primary, Secondary, Tertiary, Quaternary) which can be routed to internal wcd codec or external codecs. This routing is controlled by 2 mux registers. This patch series firstly re-organizes the lpass driver such that the SOC specific bits are moved away from the driver. And secondly the SOC specifics are now passed as lpass variant data which would include various register offsets, dma channel allocations and SOC specific clock handling. Finally the last few patchs add apq8016 lpass and machine driver. All these patches are tested for HDMI audio via adv7533 bridge and Analog audio on APQ8016-SBC, msm8916-mtp boards and Kenneth tested this patchset on ipq806x Storm board too. Other than the two comments on patches 5 and 6, everything else looks fine to me. Acked-by: Kenneth Westfield kwest...@codeaurora.org -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 05/13] ASoC: qcom: support bitclk and osrclk per i2s port
On Sat, May 16, 2015 at 05:32:42AM -0700, Srinivas Kandagatla wrote:
This patch adds support to allow bitclk and osrclk per i2s dai port.
on APQ8016 there are 4 i2s ports each one has its own bit clks.
Without this patch its not possible to support multiple i2s ports in the
lpass driver.
@@ -400,18 +402,34 @@ int asoc_qcom_lpass_cpu_platform_probe(struct
platform_device *pdev)
if (variant-init)
variant-init(pdev);
- drvdata-mi2s_osr_clk = devm_clk_get(pdev-dev, mi2s-osr-clk);
- if (IS_ERR(drvdata-mi2s_osr_clk)) {
- dev_err(pdev-dev, %s() error getting mi2s-osr-clk:
%ld\n,
- __func__, PTR_ERR(drvdata-mi2s_osr_clk));
- return PTR_ERR(drvdata-mi2s_osr_clk);
- }
-
- drvdata-mi2s_bit_clk = devm_clk_get(pdev-dev, mi2s-bit-clk);
- if (IS_ERR(drvdata-mi2s_bit_clk)) {
- dev_err(pdev-dev, %s() error getting mi2s-bit-clk:
%ld\n,
- __func__, PTR_ERR(drvdata-mi2s_bit_clk));
- return PTR_ERR(drvdata-mi2s_bit_clk);
+ for (i = 0; i variant-num_dai; i++) {
+ dai_id = variant-dai_driver[i].id;
+ if (variant-num_dai 1)
+ sprintf(clk_name, mi2s-osr-clk%d, i);
+ else
+ sprintf(clk_name, mi2s-osr-clk);
+
+ drvdata-mi2s_osr_clk[dai_id] = devm_clk_get(pdev-dev,
+ clk_name);
+ if (IS_ERR(drvdata-mi2s_osr_clk[dai_id])) {
+ dev_err(pdev-dev,
+ %s() error getting mi2s-osr-clk: %ld\n,
+ __func__,
+ return PTR_ERR(drvdata-mi2s_osr_clk[dai_id]));
Should the previous two lines be:
+ __func__, PTR_ERR(drvdata-mi2s_osr_clk[dai_id]));
+ return PTR_ERR(drvdata-mi2s_osr_clk[dai_id]);
as you remove the return in patch 6?
+ }
+
+ if (variant-num_dai 1)
+ sprintf(clk_name, mi2s-bit-clk%d, i);
+ else
+ sprintf(clk_name, mi2s-bit-clk);
+
+ drvdata-mi2s_bit_clk[dai_id] = devm_clk_get(pdev-dev,
clk_name);
+ if (IS_ERR(drvdata-mi2s_bit_clk[dai_id])) {
+ dev_err(pdev-dev,
+ %s() error getting mi2s-bit-clk: %ld\n,
+ __func__,
PTR_ERR(drvdata-mi2s_bit_clk[i]));
+ return PTR_ERR(drvdata-mi2s_bit_clk[dai_id]);
+ }
}
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH] ASoC: skip legacy dai naming if dai driver has all the information
On Fri, May 15, 2015 at 02:38:27AM -0700, Srinivas Kandagatla wrote: > Original issue is that the id field in the dai is not same as the id > in dai_driver when dai driver count == 1. This is due to the legacy > dai naming check, which could possibly cause issues if the audio drivers > written in assumption that dai->id would be always equal to > dai_driver->id. > This assumption is true only if the dai driver count is greater than 1, > and false if dai driver count is 1. On Qcom Lpass driver we hit such > issue while adding support to apq8016. > > The code path which falls back to legacy naming for cases where num_dai > == 1 does not check if there is any valid information in the dai_driver. > This patch fixes that by checking if the dai_driver has valid id and > name before falling back to legacy dai naming > Although the drivers can work around this issue by only using > dai->driver->id, but this patch attempts to fix the actual issue. > > Suggested-by: Lars-Peter Clausen > Signed-off-by: Srinivas Kandagatla > --- Acked-by: Kenneth Westfield -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCH] ASoC: skip legacy dai naming if dai driver has all the information
On Fri, May 15, 2015 at 02:38:27AM -0700, Srinivas Kandagatla wrote: Original issue is that the id field in the dai is not same as the id in dai_driver when dai driver count == 1. This is due to the legacy dai naming check, which could possibly cause issues if the audio drivers written in assumption that dai-id would be always equal to dai_driver-id. This assumption is true only if the dai driver count is greater than 1, and false if dai driver count is 1. On Qcom Lpass driver we hit such issue while adding support to apq8016. The code path which falls back to legacy naming for cases where num_dai == 1 does not check if there is any valid information in the dai_driver. This patch fixes that by checking if the dai_driver has valid id and name before falling back to legacy dai naming Although the drivers can work around this issue by only using dai-driver-id, but this patch attempts to fix the actual issue. Suggested-by: Lars-Peter Clausen l...@metafoo.de Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- Acked-by: Kenneth Westfield kwest...@codeaurora.org -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/13] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Wed, May 13, 2015 at 05:00:26AM -0700, Srinivas Kandagatla wrote:
> This patch tries to make the lpass driver more generic by moving the
> ipq806x specific bits out of the cpu and platform driver, also allows the
> SOC specific drivers to add the correct register offsets.
>
> This patch also renames the register definition header file into more
> generic header file.
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 05b9840..865205e 100644
> --- a/sound/soc/qcom/Kconfig
> +++ b/sound/soc/qcom/Kconfig
> @@ -14,11 +14,16 @@ config SND_SOC_LPASS_PLATFORM
> depends on SND_SOC_QCOM && OF
> select REGMAP_MMIO
>
> +config SND_SOC_LPASS_IPQ806X
> + tristate
> + depends on SND_SOC_QCOM
> + select SND_SOC_LPASS_CPU
> + select SND_SOC_LPASS_PLATFORM
Based on moving the of_device_id table from lpass-cpu.c to
lpass-ipq806x.c, shouldn't the OF dependency follow to the
SND_SOC_LPASS_IPQ806X config (and not SND_SOC_LPASS_CPU)?
> +
> config SND_SOC_STORM
> tristate "ASoC I2S support for Storm boards"
> depends on (ARCH_QCOM && SND_SOC_QCOM) || COMPILE_TEST
> - select SND_SOC_LPASS_CPU
> - select SND_SOC_LPASS_PLATFORM
> + select SND_SOC_LPASS_IPQ806X
> select SND_SOC_MAX98357A
> help
>Say Y or M if you want add support for SoC audio on the
> diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> new file mode 100644
> index 000..d1f698c
> --- /dev/null
> +++ b/sound/soc/qcom/lpass-ipq806x.c
> +static struct platform_driver ipq806x_lpass_cpu_platform_driver = {
> + .driver = {
> + .name = "lpass-cpu",
> + .of_match_table =
> of_match_ptr(ipq806x_lpass_cpu_device_id),
> + },
> + .probe = asoc_qcom_lpass_cpu_platform_probe,
> + .remove = asoc_qcom_lpass_cpu_platform_remove,
> +};
> +module_platform_driver(ipq801x_lpass_cpu_platform_driver);
Patch below fixes the above typo (which breaks compilation):
---><-
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
index ad1d67a..2eab828 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -103,7 +103,7 @@ static struct platform_driver
ipq806x_lpass_cpu_platform_driver = {
.probe = asoc_qcom_lpass_cpu_platform_probe,
.remove = asoc_qcom_lpass_cpu_platform_remove,
};
-module_platform_driver(ipq801x_lpass_cpu_platform_driver);
+module_platform_driver(ipq806x_lpass_cpu_platform_driver);
MODULE_DESCRIPTION("QTi LPASS CPU Driver");
MODULE_LICENSE("GPL v2");
---><-
> +
> +MODULE_DESCRIPTION("QTi LPASS CPU Driver");
> +MODULE_LICENSE("GPL v2");
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 11/13] ASoC: qcom: add apq8016 sound card support
On Wed, May 13, 2015 at 05:03:14AM -0700, Srinivas Kandagatla wrote: > This patch adds apq8016 machine driver support. This patch was tested on > two apq8016-sbc and msm8916-mtp board for both hdmi and analog audio > features. > diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig > index 9cc5ed7..e71b0f2 100644 > --- a/sound/soc/qcom/Kconfig > +++ b/sound/soc/qcom/Kconfig > @@ -34,3 +34,12 @@ config SND_SOC_STORM > help >Say Y or M if you want add support for SoC audio on the >Qualcomm Technologies IPQ806X-based Storm board. > + > +config SND_SOC_APQ8016_SBC > + tristate "SoC Audio support for APQ8016 SBC platforms" > + depends on SND_SOC_QCOM || ARCH_QCOM || COMPILE_TEST I believe this should be: depends on (SND_SOC_QCOM && ARCH_QCOM) || COMPILE_TEST > + select SND_SOC_LPASS_APQ8016 > + help > + Support for Qualcomm Technologies LPASS audio block in > + APQ8016 SOC-based systems. > + Say Y if you want to use audio devices on MI2S -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 05/13] ASoC: qcom: support bitclk and osrclk per i2s port
vm_clk_get(>dev,
clk_name);
@@ -427,19 +424,14 @@ int asoc_qcom_lpass_cpu_platform_probe(struct
platform_device *pdev)
}
}
- for (i = 0; i < variant->num_dai; i++) {
-
- if (variant->num_dai > 1)
- sprintf(clk_name, "mi2s-bit-clk%d", i);
- else
- sprintf(clk_name, "mi2s-bit-clk");
+ for (i = 0; i < LPASS_MAX_MI2S_PORTS; i++) {
+ sprintf(clk_name, "mi2s-bit-clk%d", i);
drvdata->mi2s_bit_clk[i] = devm_clk_get(>dev, clk_name);
if (IS_ERR(drvdata->mi2s_bit_clk[i])) {
dev_err(>dev,
"%s() error getting mi2s-bit-clk: %ld\n",
__func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
- return PTR_ERR(drvdata->mi2s_bit_clk[i]);
}
}
---><-
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 10/13] ASoC: qcom: Add apq8016 lpass driver support
On Wed, May 13, 2015 at 05:03:06AM -0700, Srinivas Kandagatla wrote:
> This patch adds apq8016 lpass driver support. APQ8016 has 4 MI2S which
> can be routed to one internal codec and 2 external codec interfaces.
>
> Primary, Secondary, Quaternary I2S can do Rx(playback) and Tertiary and
> Quaternary can do Tx(capture).
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 865205e..9cc5ed7 100644
> --- a/sound/soc/qcom/Kconfig
> +++ b/sound/soc/qcom/Kconfig
> @@ -20,6 +20,12 @@ config SND_SOC_LPASS_IPQ806X
> select SND_SOC_LPASS_CPU
> select SND_SOC_LPASS_PLATFORM
>
> +config SND_SOC_LPASS_APQ8016
> + tristate
> + depends on SND_SOC_QCOM
> + select SND_SOC_LPASS_CPU
> + select SND_SOC_LPASS_PLATFORM
Continuing from my comments on patch 2/13, should an OF dependency be added
here as well?
> +
> config SND_SOC_STORM
> tristate "ASoC I2S support for Storm boards"
> depends on (ARCH_QCOM && SND_SOC_QCOM) || COMPILE_TEST
> diff --git a/sound/soc/qcom/lpass-apq8016.c
> b/sound/soc/qcom/lpass-apq8016.c
> new file mode 100644
> index 000..5cbf17f0
> --- /dev/null
> +++ b/sound/soc/qcom/lpass-apq8016.c
> +static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int
> chan)
> +{
> + clear_bit(chan, >rdma_ch_bit_map);
> +
> + return 0;
> +}
> +
> +static int apq8016_lpass_init(struct platform_device *pdev)
> +{
> + struct lpass_data *drvdata = platform_get_drvdata(pdev);
> + struct device *dev = >dev;
> + int ret;
> +
> + drvdata->pcnoc_mport_clk = devm_clk_get(dev, "pcnoc-mport-clk");
> + if (IS_ERR(drvdata->pcnoc_mport_clk)) {
> + dev_err(>dev, "%s() error getting pcnoc-mport-clk:
> %ld\n",
> + __func__,
> PTR_ERR(drvdata->pcnoc_mport_clk));
> + return PTR_ERR(drvdata->pcnoc_mport_clk);
> + }
> +
> + ret = clk_prepare_enable(drvdata->pcnoc_mport_clk);
> + if (ret) {
> + dev_err(>dev, "%s() Error enabling ahbix_clk: %d\n",
Please correct the clock name in the log message ...
> + __func__, ret);
> + return ret;
> + }
> +
> + drvdata->pcnoc_sway_clk = devm_clk_get(dev, "pcnoc-sway-clk");
> + if (IS_ERR(drvdata->pcnoc_sway_clk)) {
> + dev_err(>dev, "%s() error getting pcnoc-sway-clk:
> %ld\n",
> + __func__,
> PTR_ERR(drvdata->pcnoc_sway_clk));
> + return PTR_ERR(drvdata->pcnoc_sway_clk);
> + }
> +
> + ret = clk_prepare_enable(drvdata->pcnoc_sway_clk);
> + if (ret) {
> + dev_err(>dev, "%s() Error enabling ahbix_clk: %d\n",
... here too.
> + __func__, ret);
> + return ret;
> + }
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 10/13] ASoC: qcom: Add apq8016 lpass driver support
On Wed, May 13, 2015 at 05:03:06AM -0700, Srinivas Kandagatla wrote:
This patch adds apq8016 lpass driver support. APQ8016 has 4 MI2S which
can be routed to one internal codec and 2 external codec interfaces.
Primary, Secondary, Quaternary I2S can do Rx(playback) and Tertiary and
Quaternary can do Tx(capture).
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 865205e..9cc5ed7 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -20,6 +20,12 @@ config SND_SOC_LPASS_IPQ806X
select SND_SOC_LPASS_CPU
select SND_SOC_LPASS_PLATFORM
+config SND_SOC_LPASS_APQ8016
+ tristate
+ depends on SND_SOC_QCOM
+ select SND_SOC_LPASS_CPU
+ select SND_SOC_LPASS_PLATFORM
Continuing from my comments on patch 2/13, should an OF dependency be added
here as well?
+
config SND_SOC_STORM
tristate ASoC I2S support for Storm boards
depends on (ARCH_QCOM SND_SOC_QCOM) || COMPILE_TEST
diff --git a/sound/soc/qcom/lpass-apq8016.c
b/sound/soc/qcom/lpass-apq8016.c
new file mode 100644
index 000..5cbf17f0
--- /dev/null
+++ b/sound/soc/qcom/lpass-apq8016.c
+static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int
chan)
+{
+ clear_bit(chan, drvdata-rdma_ch_bit_map);
+
+ return 0;
+}
+
+static int apq8016_lpass_init(struct platform_device *pdev)
+{
+ struct lpass_data *drvdata = platform_get_drvdata(pdev);
+ struct device *dev = pdev-dev;
+ int ret;
+
+ drvdata-pcnoc_mport_clk = devm_clk_get(dev, pcnoc-mport-clk);
+ if (IS_ERR(drvdata-pcnoc_mport_clk)) {
+ dev_err(pdev-dev, %s() error getting pcnoc-mport-clk:
%ld\n,
+ __func__,
PTR_ERR(drvdata-pcnoc_mport_clk));
+ return PTR_ERR(drvdata-pcnoc_mport_clk);
+ }
+
+ ret = clk_prepare_enable(drvdata-pcnoc_mport_clk);
+ if (ret) {
+ dev_err(pdev-dev, %s() Error enabling ahbix_clk: %d\n,
Please correct the clock name in the log message ...
+ __func__, ret);
+ return ret;
+ }
+
+ drvdata-pcnoc_sway_clk = devm_clk_get(dev, pcnoc-sway-clk);
+ if (IS_ERR(drvdata-pcnoc_sway_clk)) {
+ dev_err(pdev-dev, %s() error getting pcnoc-sway-clk:
%ld\n,
+ __func__,
PTR_ERR(drvdata-pcnoc_sway_clk));
+ return PTR_ERR(drvdata-pcnoc_sway_clk);
+ }
+
+ ret = clk_prepare_enable(drvdata-pcnoc_sway_clk);
+ if (ret) {
+ dev_err(pdev-dev, %s() Error enabling ahbix_clk: %d\n,
... here too.
+ __func__, ret);
+ return ret;
+ }
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 05/13] ASoC: qcom: support bitclk and osrclk per i2s port
, mi2s-bit-clk%d, i);
drvdata-mi2s_bit_clk[i] = devm_clk_get(pdev-dev, clk_name);
if (IS_ERR(drvdata-mi2s_bit_clk[i])) {
dev_err(pdev-dev,
%s() error getting mi2s-bit-clk: %ld\n,
__func__, PTR_ERR(drvdata-mi2s_bit_clk[i]));
- return PTR_ERR(drvdata-mi2s_bit_clk[i]);
}
}
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 11/13] ASoC: qcom: add apq8016 sound card support
On Wed, May 13, 2015 at 05:03:14AM -0700, Srinivas Kandagatla wrote: This patch adds apq8016 machine driver support. This patch was tested on two apq8016-sbc and msm8916-mtp board for both hdmi and analog audio features. diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 9cc5ed7..e71b0f2 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -34,3 +34,12 @@ config SND_SOC_STORM help Say Y or M if you want add support for SoC audio on the Qualcomm Technologies IPQ806X-based Storm board. + +config SND_SOC_APQ8016_SBC + tristate SoC Audio support for APQ8016 SBC platforms + depends on SND_SOC_QCOM || ARCH_QCOM || COMPILE_TEST I believe this should be: depends on (SND_SOC_QCOM ARCH_QCOM) || COMPILE_TEST + select SND_SOC_LPASS_APQ8016 + help + Support for Qualcomm Technologies LPASS audio block in + APQ8016 SOC-based systems. + Say Y if you want to use audio devices on MI2S -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v1 02/13] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Wed, May 13, 2015 at 05:00:26AM -0700, Srinivas Kandagatla wrote:
This patch tries to make the lpass driver more generic by moving the
ipq806x specific bits out of the cpu and platform driver, also allows the
SOC specific drivers to add the correct register offsets.
This patch also renames the register definition header file into more
generic header file.
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 05b9840..865205e 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -14,11 +14,16 @@ config SND_SOC_LPASS_PLATFORM
depends on SND_SOC_QCOM OF
select REGMAP_MMIO
+config SND_SOC_LPASS_IPQ806X
+ tristate
+ depends on SND_SOC_QCOM
+ select SND_SOC_LPASS_CPU
+ select SND_SOC_LPASS_PLATFORM
Based on moving the of_device_id table from lpass-cpu.c to
lpass-ipq806x.c, shouldn't the OF dependency follow to the
SND_SOC_LPASS_IPQ806X config (and not SND_SOC_LPASS_CPU)?
+
config SND_SOC_STORM
tristate ASoC I2S support for Storm boards
depends on (ARCH_QCOM SND_SOC_QCOM) || COMPILE_TEST
- select SND_SOC_LPASS_CPU
- select SND_SOC_LPASS_PLATFORM
+ select SND_SOC_LPASS_IPQ806X
select SND_SOC_MAX98357A
help
Say Y or M if you want add support for SoC audio on the
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
new file mode 100644
index 000..d1f698c
--- /dev/null
+++ b/sound/soc/qcom/lpass-ipq806x.c
+static struct platform_driver ipq806x_lpass_cpu_platform_driver = {
+ .driver = {
+ .name = lpass-cpu,
+ .of_match_table =
of_match_ptr(ipq806x_lpass_cpu_device_id),
+ },
+ .probe = asoc_qcom_lpass_cpu_platform_probe,
+ .remove = asoc_qcom_lpass_cpu_platform_remove,
+};
+module_platform_driver(ipq801x_lpass_cpu_platform_driver);
Patch below fixes the above typo (which breaks compilation):
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
index ad1d67a..2eab828 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -103,7 +103,7 @@ static struct platform_driver
ipq806x_lpass_cpu_platform_driver = {
.probe = asoc_qcom_lpass_cpu_platform_probe,
.remove = asoc_qcom_lpass_cpu_platform_remove,
};
-module_platform_driver(ipq801x_lpass_cpu_platform_driver);
+module_platform_driver(ipq806x_lpass_cpu_platform_driver);
MODULE_DESCRIPTION(QTi LPASS CPU Driver);
MODULE_LICENSE(GPL v2);
+
+MODULE_DESCRIPTION(QTi LPASS CPU Driver);
+MODULE_LICENSE(GPL v2);
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 00/14] ASoC: qcom: add support to apq8016 audio
On Tue, May 05, 2015 at 11:54:16PM -0700, Srinivas Kandagatla wrote:
> Hi Kenneth,
>
> On 06/05/15 06:47, Kenneth Westfield wrote:
> >>>
> >>>I will test the patches and let you know by Wednesday. Also, I posted
> >>>some comments, but Patrick should be posting his comments separately
> >>>later next week.
> >Srinivas,
> >
> >After applying the patches, audio playback is no longer functional on
> >the storm board. Give me some time to debug this and I will get back
> >to you.
> I found atleast one issue with rdma audif bits in ipq806x lpass
>
> could you try change, this will be fixed properly in next version.
>
> ---><-
> diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> index 11a7053..2b00355 100644
> --- a/sound/soc/qcom/lpass-ipq806x.c
> +++ b/sound/soc/qcom/lpass-ipq806x.c
> @@ -69,6 +69,7 @@ struct lpass_variant ipq806x_data = {
> .rdma_reg_base = 0x6000,
> .rdma_reg_stride= 0x1000,
> .rdma_channels = 4,
> + .rdmactl_audif_start= 4,
> .dai_driver = _cpu_dai_driver,
> .num_dai= 1,
>
> ---><-
Srinivas,
I was able to get audio working on the Storm board. There were several
issues. First, the I2S control port was saved in the DAI driver id field
(which was 4), but the DAI id field was used by the macro (which was 0).
The patch below fixed it:
---><-
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 17ad20d..58ae8af 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -146,7 +146,7 @@ static int lpass_cpu_daiops_hw_params(struct
snd_pcm_substream *substream,
}
ret = regmap_write(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id),
+ LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
regval);
if (ret) {
dev_err(dai->dev, "%s() error writing to i2sctl reg: %d\n",
@@ -171,7 +171,7 @@ static int lpass_cpu_daiops_hw_free(struct
snd_pcm_substream *substream,
int ret;
ret = regmap_write(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id), 0);
+ LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
0);
if (ret)
dev_err(dai->dev, "%s() error writing to i2sctl reg: %d\n",
__func__, ret);
@@ -186,7 +186,7 @@ static int lpass_cpu_daiops_prepare(struct
snd_pcm_substream *substream,
int ret;
ret = regmap_update_bits(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id),
+ LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
LPAIF_I2SCTL_SPKEN_MASK, LPAIF_I2SCTL_SPKEN_ENABLE);
if (ret)
dev_err(dai->dev, "%s() error writing to i2sctl reg: %d\n",
@@ -206,7 +206,7 @@ static int lpass_cpu_daiops_trigger(struct
snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
ret = regmap_update_bits(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id),
+ LPAIF_I2SCTL_REG(drvdata->variant,
dai->driver->id),
LPAIF_I2SCTL_SPKEN_MASK,
LPAIF_I2SCTL_SPKEN_ENABLE);
if (ret)
@@ -217,7 +217,7 @@ static int lpass_cpu_daiops_trigger(struct
snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
ret = regmap_update_bits(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id),
+ LPAIF_I2SCTL_REG(drvdata->variant,
dai->driver->id),
LPAIF_I2SCTL_SPKEN_MASK,
LPAIF_I2SCTL_SPKEN_DISABLE);
if (ret)
@@ -247,7 +247,7 @@ int lpass_cpu_dai_probe(struct snd_soc_dai *dai)
/* ensure audio hardware is disabled */
ret = regmap_write(drvdata->lpaif_map,
- LPAIF_I2SCTL_REG(drvdata->variant, dai->id), 0);
+ LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id), 0);
if (ret)
dev_err(dai->dev, "%s() error writing to i2sctl reg: %d\n",
Re: [alsa-devel] [RFC PATCH 00/14] ASoC: qcom: add support to apq8016 audio
On Tue, May 05, 2015 at 11:54:16PM -0700, Srinivas Kandagatla wrote:
Hi Kenneth,
On 06/05/15 06:47, Kenneth Westfield wrote:
I will test the patches and let you know by Wednesday. Also, I posted
some comments, but Patrick should be posting his comments separately
later next week.
Srinivas,
After applying the patches, audio playback is no longer functional on
the storm board. Give me some time to debug this and I will get back
to you.
I found atleast one issue with rdma audif bits in ipq806x lpass
could you try change, this will be fixed properly in next version.
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
index 11a7053..2b00355 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -69,6 +69,7 @@ struct lpass_variant ipq806x_data = {
.rdma_reg_base = 0x6000,
.rdma_reg_stride= 0x1000,
.rdma_channels = 4,
+ .rdmactl_audif_start= 4,
.dai_driver = lpass_cpu_dai_driver,
.num_dai= 1,
Srinivas,
I was able to get audio working on the Storm board. There were several
issues. First, the I2S control port was saved in the DAI driver id field
(which was 4), but the DAI id field was used by the macro (which was 0).
The patch below fixed it:
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 17ad20d..58ae8af 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -146,7 +146,7 @@ static int lpass_cpu_daiops_hw_params(struct
snd_pcm_substream *substream,
}
ret = regmap_write(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id),
+ LPAIF_I2SCTL_REG(drvdata-variant, dai-driver-id),
regval);
if (ret) {
dev_err(dai-dev, %s() error writing to i2sctl reg: %d\n,
@@ -171,7 +171,7 @@ static int lpass_cpu_daiops_hw_free(struct
snd_pcm_substream *substream,
int ret;
ret = regmap_write(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id), 0);
+ LPAIF_I2SCTL_REG(drvdata-variant, dai-driver-id),
0);
if (ret)
dev_err(dai-dev, %s() error writing to i2sctl reg: %d\n,
__func__, ret);
@@ -186,7 +186,7 @@ static int lpass_cpu_daiops_prepare(struct
snd_pcm_substream *substream,
int ret;
ret = regmap_update_bits(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id),
+ LPAIF_I2SCTL_REG(drvdata-variant, dai-driver-id),
LPAIF_I2SCTL_SPKEN_MASK, LPAIF_I2SCTL_SPKEN_ENABLE);
if (ret)
dev_err(dai-dev, %s() error writing to i2sctl reg: %d\n,
@@ -206,7 +206,7 @@ static int lpass_cpu_daiops_trigger(struct
snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
ret = regmap_update_bits(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id),
+ LPAIF_I2SCTL_REG(drvdata-variant,
dai-driver-id),
LPAIF_I2SCTL_SPKEN_MASK,
LPAIF_I2SCTL_SPKEN_ENABLE);
if (ret)
@@ -217,7 +217,7 @@ static int lpass_cpu_daiops_trigger(struct
snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
ret = regmap_update_bits(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id),
+ LPAIF_I2SCTL_REG(drvdata-variant,
dai-driver-id),
LPAIF_I2SCTL_SPKEN_MASK,
LPAIF_I2SCTL_SPKEN_DISABLE);
if (ret)
@@ -247,7 +247,7 @@ int lpass_cpu_dai_probe(struct snd_soc_dai *dai)
/* ensure audio hardware is disabled */
ret = regmap_write(drvdata-lpaif_map,
- LPAIF_I2SCTL_REG(drvdata-variant, dai-id), 0);
+ LPAIF_I2SCTL_REG(drvdata-variant, dai-driver-id), 0);
if (ret)
dev_err(dai-dev, %s() error writing to i2sctl reg: %d\n,
__func__, ret);
In addition to your patch above, I also needed to correct the rdma_port
assignment by removing the i2s port reference:
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index c5907d5..580cae1 100644
--- a/sound/soc/qcom
Re: [RFC PATCH 00/14] ASoC: qcom: add support to apq8016 audio
On Sat, May 02, 2015 at 04:57:04PM -0700, Kenneth Westfield wrote: > On Thu, Apr 30, 2015 at 06:15:48PM +0100, Srinivas Kandagatla wrote: > > Hi All, > > > > This patchset adds apq8016 audio support into lpass driver. Existing > Lpass > > driver can not be used as-it-is for apq8016 as it contains code specific > to > > ipq806x. Also the driver only supports single i2s port, single dma > channel and > > single bitclk control. > > > > APQ8016 has 4 MI2S( Primary, Secondary, Tertiary, Quaternary) which can > be routed > > to internal wcd codec or external codecs. This routing is controlled by > 2 mux > > registers. > > > > This patch series firstly re-organizes the lpass driver such that the > SOC > > specific bits are moved away from the driver. And secondly the SOC > specifics > > are now passed as lpass variant data which would include various > register > > offsets, dma channel allocations and SOC specific clock handling. > > > > Finally the patchset add apq8016 lpass and machine driver. > > > > This patchset also has two trivial cleanup patches which are to do with > > redundant checks and removing unnecessary header files. > > > > All these patches are tested for HDMI audio via adv7533 bridge and > Analog audio > > on APQ8016-SBC and msm8916-mtp boards. I dont have access to ipq806x > boards to > > test these patches. > > > > This is very first version of the patches which was developed with very > > mimimal/no access to IP documentation. I would like to get your opinon > on the > > over all approch. > > > > > > Kenneth/Patrick, > > Could you please try these patches on storm board? > > I will test the patches and let you know by Wednesday. Also, I posted > some comments, but Patrick should be posting his comments separately > later next week. Srinivas, After applying the patches, audio playback is no longer functional on the storm board. Give me some time to debug this and I will get back to you. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Tue, May 05, 2015 at 12:17:23AM -0700, Srinivas Kandagatla wrote:
> On 05/05/15 06:19, Kenneth Westfield wrote:
> >>>> >+enum lpaif_i2s_ports {
> >>>> >+ LPAIF_I2S_PORT_MIN = 0,
> >>>> >+
> >>>> >+ LPAIF_I2S_PORT_CODEC_SPK= 0,
> >>>> >+ LPAIF_I2S_PORT_CODEC_MIC= 1,
> >>>> >+ LPAIF_I2S_PORT_SEC_SPK = 2,
> >>>> >+ LPAIF_I2S_PORT_SEC_MIC = 3,
> >>>> >+ LPAIF_I2S_PORT_MI2S = 4,
> >>>> >+
> >>>> >+ LPAIF_I2S_PORT_MAX = 4,
> >>>> >+ LPAIF_I2S_PORT_NUM = 5,
> >>>> >+};
> >>>
> >>>These port mappings here...
> >>>
> >>>> >+enum lpaif_irq_ports {
> >>>> >+ LPAIF_IRQ_PORT_MIN = 0,
> >>>> >+
> >>>> >+ LPAIF_IRQ_PORT_HOST = 0,
> >>>> >+ LPAIF_IRQ_PORT_ADSP = 1,
> >>>> >+
> >>>> >+ LPAIF_IRQ_PORT_MAX = 2,
> >>>> >+ LPAIF_IRQ_PORT_NUM = 3,
> >>>> >+};
> >>>
> >>>...here...
> >>>
> >>>> >+enum lpaif_dma_channels {
> >>>> >+ LPAIF_RDMA_CHAN_MIN = 0,
> >>>> >+
> >>>> >+ LPAIF_RDMA_CHAN_MI2S= 0,
> >>>> >+ LPAIF_RDMA_CHAN_PCM0= 1,
> >>>> >+ LPAIF_RDMA_CHAN_PCM1= 2,
> >>>> >+
> >>>> >+ LPAIF_RDMA_CHAN_MAX = 4,
> >>>> >+ LPAIF_RDMA_CHAN_NUM = 5,
> >>>> >+};
> >>>
> >>>...and here can be SOC-specific. Should move them to the SOC-specific
> >>>files.
> >Expanding on this, the I2S port mappings for the APQ8016 should replace
> >the ones defined above with the constants you refer to in
> >dt-bindings/sound/apq8016.h:
> >MI2S_PRIMARY
> >MI2S_SECONDARY
> >etc.
> >
> >Maybe defining a corresponding ipq806x.h in the same directory, and
> >moving the above definitions there?
>
> As you pointed out i2s ports definitions can be moved to
> dt-bindings/soc/ipq806x.h but the channels can be directly defined
> in lpass-ipq806x.c as there would be no DT consumers for these
> defines
> anyway.
Moving the I2S ports to dt-bindings and the other definitions to their
SOC-specific source files works for me.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 14/14] ASoC: qcom: document apq8016 machine driver bindings
On Tue, May 05, 2015 at 12:17:01AM -0700, Srinivas Kandagatla wrote:
> On 03/05/15 01:03, Kenneth Westfield wrote:
> >On Thu, Apr 30, 2015 at 06:18:26PM +0100, Srinivas Kandagatla wrote:
> >>This patch adds bindings for apq8016 machine driver.
> >>On APQ8016 4 MI2S can be configured to different sinks like internal
> >>codec/external codec, this connection is controlled via 2 iomux
> >>registers.
> >>
> >>+sound: sound {
> >>+ compatible = "qcom,apq8016-sndcard";
> >>+ reg = <0x07702000 0x4>, <0x07702004 0x4>;
> >>+ reg-names = "mic-iomux", "spkr-iomux";
> >>+ qcom,model = "DB410c";
> >>+
> >>+ /* I2S - Internal codec */
> >>+ internal-dai-link@0 {
> >>+ cpu { /* PRIMARY */
> >>+ sound-dai = < MI2S_PRIMARY>;
> >>+ };
> >>+ codec {
> >>+ sound-dai = <_codec 0>;
> >>+ };
> >>+ };
> >>+
> >>+ /* External Primary or External Secondary -ADV7533 HDMI */
> >>+ external-dai-link@0 {
> >>+ external;
> >>+ cpu { /* QUAT */
> >>+ sound-dai = < MI2S_QUATERNARY>;
> >>+ };
> >>+ codec {
> >>+ sound-dai = <_bridge 0>;
> >>+ };
> >>+ };
> >>+};
> >
> >OK, although I will need to double-check this with the spec, it seems
> >(from the patches) that there are 4 I2S ports, 2 of which are being
> >used. Usually, multi-channel audio is sent to the primary dai (which
> >is MI2S), which then gets sent to the other ports by HW. If that holds
> >true for this SOC, then the external cpu dai should be labelled I2S,
> >not MI2S. If not, then both should be labelled as I2S (and the DAI
> >channel constraints should be reduced to 1-2).
> >
> I have got very limited access to documentation, which obviously
> does not have any info related to channel capabilities. I was
> thinking that all the playback ports support multi-channel. I might
> be wrong though.
After checking the specs, the four playback DAIs are, in fact,
multi-channel. Therefore, MI2S_ is the correct prefix for them.
>
> >Looking at patch 12, the internal DAI is labelled Headset and the
> >external DAI is labelled HDMI.
> This naming is very specific to the SBC board.
>
> I will check the spec to see if the QUAT
> >I2S port can handle multi-channel.
> External HDMI bridge on the other side only supports 2-channels, but
> if you can re-check on the channel capabilities would be good.
Ah, ok. Then no need to worry about multi-channel playback. Just need
to ensure the codec drivers constrain the channels to mono/stereo.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Tue, May 05, 2015 at 12:16:46AM -0700, Srinivas Kandagatla wrote:
> On 03/05/15 00:57, Kenneth Westfield wrote:
> >On Thu, Apr 30, 2015 at 06:16:53PM +0100, Srinivas Kandagatla wrote:
> >>This patch tries to make the lpass driver more generic by moving the
> >>ipq806x specific bits out of the cpu and platform driver, also allows
> the
> >>SOC specific drivers to add the correct register offsets.
> >>
> >>This patch also renames the register definition header file into more
> >>generic header file.
> >
> >>diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> >>new file mode 100644
> >>index 000..8e9a427
> >>--- /dev/null
> >>+++ b/sound/soc/qcom/lpass-ipq806x.c
> >
> >>+static const struct of_device_id ipq806x_lpass_cpu_device_id[] = {
> >>+ { .compatible = "qcom,lpass-cpu", .data = _data },
> >>+ {}
> >>+};
> >>+MODULE_DEVICE_TABLE(of, ipq806x_lpass_cpu_device_id);
> >
> >Surround with #ifdef CONFIG_OF?
>
> I was more of thinking adding "depends OF" in Kconfig.
Works for me.
> Is there any possibility that the driver would support non-DT?
Not that I am aware of.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Tue, May 05, 2015 at 12:17:23AM -0700, Srinivas Kandagatla wrote:
On 05/05/15 06:19, Kenneth Westfield wrote:
+enum lpaif_i2s_ports {
+ LPAIF_I2S_PORT_MIN = 0,
+
+ LPAIF_I2S_PORT_CODEC_SPK= 0,
+ LPAIF_I2S_PORT_CODEC_MIC= 1,
+ LPAIF_I2S_PORT_SEC_SPK = 2,
+ LPAIF_I2S_PORT_SEC_MIC = 3,
+ LPAIF_I2S_PORT_MI2S = 4,
+
+ LPAIF_I2S_PORT_MAX = 4,
+ LPAIF_I2S_PORT_NUM = 5,
+};
These port mappings here...
+enum lpaif_irq_ports {
+ LPAIF_IRQ_PORT_MIN = 0,
+
+ LPAIF_IRQ_PORT_HOST = 0,
+ LPAIF_IRQ_PORT_ADSP = 1,
+
+ LPAIF_IRQ_PORT_MAX = 2,
+ LPAIF_IRQ_PORT_NUM = 3,
+};
...here...
+enum lpaif_dma_channels {
+ LPAIF_RDMA_CHAN_MIN = 0,
+
+ LPAIF_RDMA_CHAN_MI2S= 0,
+ LPAIF_RDMA_CHAN_PCM0= 1,
+ LPAIF_RDMA_CHAN_PCM1= 2,
+
+ LPAIF_RDMA_CHAN_MAX = 4,
+ LPAIF_RDMA_CHAN_NUM = 5,
+};
...and here can be SOC-specific. Should move them to the SOC-specific
files.
Expanding on this, the I2S port mappings for the APQ8016 should replace
the ones defined above with the constants you refer to in
dt-bindings/sound/apq8016.h:
MI2S_PRIMARY
MI2S_SECONDARY
etc.
Maybe defining a corresponding ipq806x.h in the same directory, and
moving the above definitions there?
As you pointed out i2s ports definitions can be moved to
dt-bindings/soc/ipq806x.h but the channels can be directly defined
in lpass-ipq806x.c as there would be no DT consumers for these
defines
anyway.
Moving the I2S ports to dt-bindings and the other definitions to their
SOC-specific source files works for me.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 00/14] ASoC: qcom: add support to apq8016 audio
On Sat, May 02, 2015 at 04:57:04PM -0700, Kenneth Westfield wrote: On Thu, Apr 30, 2015 at 06:15:48PM +0100, Srinivas Kandagatla wrote: Hi All, This patchset adds apq8016 audio support into lpass driver. Existing Lpass driver can not be used as-it-is for apq8016 as it contains code specific to ipq806x. Also the driver only supports single i2s port, single dma channel and single bitclk control. APQ8016 has 4 MI2S( Primary, Secondary, Tertiary, Quaternary) which can be routed to internal wcd codec or external codecs. This routing is controlled by 2 mux registers. This patch series firstly re-organizes the lpass driver such that the SOC specific bits are moved away from the driver. And secondly the SOC specifics are now passed as lpass variant data which would include various register offsets, dma channel allocations and SOC specific clock handling. Finally the patchset add apq8016 lpass and machine driver. This patchset also has two trivial cleanup patches which are to do with redundant checks and removing unnecessary header files. All these patches are tested for HDMI audio via adv7533 bridge and Analog audio on APQ8016-SBC and msm8916-mtp boards. I dont have access to ipq806x boards to test these patches. This is very first version of the patches which was developed with very mimimal/no access to IP documentation. I would like to get your opinon on the over all approch. Kenneth/Patrick, Could you please try these patches on storm board? I will test the patches and let you know by Wednesday. Also, I posted some comments, but Patrick should be posting his comments separately later next week. Srinivas, After applying the patches, audio playback is no longer functional on the storm board. Give me some time to debug this and I will get back to you. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 14/14] ASoC: qcom: document apq8016 machine driver bindings
On Tue, May 05, 2015 at 12:17:01AM -0700, Srinivas Kandagatla wrote:
On 03/05/15 01:03, Kenneth Westfield wrote:
On Thu, Apr 30, 2015 at 06:18:26PM +0100, Srinivas Kandagatla wrote:
This patch adds bindings for apq8016 machine driver.
On APQ8016 4 MI2S can be configured to different sinks like internal
codec/external codec, this connection is controlled via 2 iomux
registers.
+sound: sound {
+ compatible = qcom,apq8016-sndcard;
+ reg = 0x07702000 0x4, 0x07702004 0x4;
+ reg-names = mic-iomux, spkr-iomux;
+ qcom,model = DB410c;
+
+ /* I2S - Internal codec */
+ internal-dai-link@0 {
+ cpu { /* PRIMARY */
+ sound-dai = lpass MI2S_PRIMARY;
+ };
+ codec {
+ sound-dai = wcd_codec 0;
+ };
+ };
+
+ /* External Primary or External Secondary -ADV7533 HDMI */
+ external-dai-link@0 {
+ external;
+ cpu { /* QUAT */
+ sound-dai = lpass MI2S_QUATERNARY;
+ };
+ codec {
+ sound-dai = adv_bridge 0;
+ };
+ };
+};
OK, although I will need to double-check this with the spec, it seems
(from the patches) that there are 4 I2S ports, 2 of which are being
used. Usually, multi-channel audio is sent to the primary dai (which
is MI2S), which then gets sent to the other ports by HW. If that holds
true for this SOC, then the external cpu dai should be labelled I2S,
not MI2S. If not, then both should be labelled as I2S (and the DAI
channel constraints should be reduced to 1-2).
I have got very limited access to documentation, which obviously
does not have any info related to channel capabilities. I was
thinking that all the playback ports support multi-channel. I might
be wrong though.
After checking the specs, the four playback DAIs are, in fact,
multi-channel. Therefore, MI2S_ is the correct prefix for them.
Looking at patch 12, the internal DAI is labelled Headset and the
external DAI is labelled HDMI.
This naming is very specific to the SBC board.
I will check the spec to see if the QUAT
I2S port can handle multi-channel.
External HDMI bridge on the other side only supports 2-channels, but
if you can re-check on the channel capabilities would be good.
Ah, ok. Then no need to worry about multi-channel playback. Just need
to ensure the codec drivers constrain the channels to mono/stereo.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Tue, May 05, 2015 at 12:16:46AM -0700, Srinivas Kandagatla wrote:
On 03/05/15 00:57, Kenneth Westfield wrote:
On Thu, Apr 30, 2015 at 06:16:53PM +0100, Srinivas Kandagatla wrote:
This patch tries to make the lpass driver more generic by moving the
ipq806x specific bits out of the cpu and platform driver, also allows
the
SOC specific drivers to add the correct register offsets.
This patch also renames the register definition header file into more
generic header file.
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
new file mode 100644
index 000..8e9a427
--- /dev/null
+++ b/sound/soc/qcom/lpass-ipq806x.c
+static const struct of_device_id ipq806x_lpass_cpu_device_id[] = {
+ { .compatible = qcom,lpass-cpu, .data = ipq806x_data },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ipq806x_lpass_cpu_device_id);
Surround with #ifdef CONFIG_OF?
I was more of thinking adding depends OF in Kconfig.
Works for me.
Is there any possibility that the driver would support non-DT?
Not that I am aware of.
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Sat, May 02, 2015 at 04:57:38PM -0700, Kenneth Westfield wrote:
> On Thu, Apr 30, 2015 at 06:16:53PM +0100, Srinivas Kandagatla wrote:
> > This patch tries to make the lpass driver more generic by moving the
> > ipq806x specific bits out of the cpu and platform driver, also allows
> the
> > SOC specific drivers to add the correct register offsets.
> >
> > This patch also renames the register definition header file into more
> > generic header file.
>
> > diff --git a/sound/soc/qcom/lpass-ipq806x.c
> b/sound/soc/qcom/lpass-ipq806x.c
> > new file mode 100644
> > index 000..8e9a427
> > --- /dev/null
> > +++ b/sound/soc/qcom/lpass-ipq806x.c
> > +enum lpaif_i2s_ports {
> > + LPAIF_I2S_PORT_MIN = 0,
> > +
> > + LPAIF_I2S_PORT_CODEC_SPK= 0,
> > + LPAIF_I2S_PORT_CODEC_MIC= 1,
> > + LPAIF_I2S_PORT_SEC_SPK = 2,
> > + LPAIF_I2S_PORT_SEC_MIC = 3,
> > + LPAIF_I2S_PORT_MI2S = 4,
> > +
> > + LPAIF_I2S_PORT_MAX = 4,
> > + LPAIF_I2S_PORT_NUM = 5,
> > +};
>
> These port mappings here...
>
> > +enum lpaif_irq_ports {
> > + LPAIF_IRQ_PORT_MIN = 0,
> > +
> > + LPAIF_IRQ_PORT_HOST = 0,
> > + LPAIF_IRQ_PORT_ADSP = 1,
> > +
> > + LPAIF_IRQ_PORT_MAX = 2,
> > + LPAIF_IRQ_PORT_NUM = 3,
> > +};
>
> ...here...
>
> > +enum lpaif_dma_channels {
> > + LPAIF_RDMA_CHAN_MIN = 0,
> > +
> > + LPAIF_RDMA_CHAN_MI2S= 0,
> > + LPAIF_RDMA_CHAN_PCM0= 1,
> > + LPAIF_RDMA_CHAN_PCM1= 2,
> > +
> > + LPAIF_RDMA_CHAN_MAX = 4,
> > + LPAIF_RDMA_CHAN_NUM = 5,
> > +};
>
> ...and here can be SOC-specific. Should move them to the SOC-specific
> files.
Expanding on this, the I2S port mappings for the APQ8016 should replace
the ones defined above with the constants you refer to in
dt-bindings/sound/apq8016.h:
MI2S_PRIMARY
MI2S_SECONDARY
etc.
Maybe defining a corresponding ipq806x.h in the same directory, and
moving the above definitions there?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 03/14] ASoC: qcom: move ipq806x specific bits out of lpass driver.
On Sat, May 02, 2015 at 04:57:38PM -0700, Kenneth Westfield wrote:
On Thu, Apr 30, 2015 at 06:16:53PM +0100, Srinivas Kandagatla wrote:
This patch tries to make the lpass driver more generic by moving the
ipq806x specific bits out of the cpu and platform driver, also allows
the
SOC specific drivers to add the correct register offsets.
This patch also renames the register definition header file into more
generic header file.
diff --git a/sound/soc/qcom/lpass-ipq806x.c
b/sound/soc/qcom/lpass-ipq806x.c
new file mode 100644
index 000..8e9a427
--- /dev/null
+++ b/sound/soc/qcom/lpass-ipq806x.c
+enum lpaif_i2s_ports {
+ LPAIF_I2S_PORT_MIN = 0,
+
+ LPAIF_I2S_PORT_CODEC_SPK= 0,
+ LPAIF_I2S_PORT_CODEC_MIC= 1,
+ LPAIF_I2S_PORT_SEC_SPK = 2,
+ LPAIF_I2S_PORT_SEC_MIC = 3,
+ LPAIF_I2S_PORT_MI2S = 4,
+
+ LPAIF_I2S_PORT_MAX = 4,
+ LPAIF_I2S_PORT_NUM = 5,
+};
These port mappings here...
+enum lpaif_irq_ports {
+ LPAIF_IRQ_PORT_MIN = 0,
+
+ LPAIF_IRQ_PORT_HOST = 0,
+ LPAIF_IRQ_PORT_ADSP = 1,
+
+ LPAIF_IRQ_PORT_MAX = 2,
+ LPAIF_IRQ_PORT_NUM = 3,
+};
...here...
+enum lpaif_dma_channels {
+ LPAIF_RDMA_CHAN_MIN = 0,
+
+ LPAIF_RDMA_CHAN_MI2S= 0,
+ LPAIF_RDMA_CHAN_PCM0= 1,
+ LPAIF_RDMA_CHAN_PCM1= 2,
+
+ LPAIF_RDMA_CHAN_MAX = 4,
+ LPAIF_RDMA_CHAN_NUM = 5,
+};
...and here can be SOC-specific. Should move them to the SOC-specific
files.
Expanding on this, the I2S port mappings for the APQ8016 should replace
the ones defined above with the constants you refer to in
dt-bindings/sound/apq8016.h:
MI2S_PRIMARY
MI2S_SECONDARY
etc.
Maybe defining a corresponding ipq806x.h in the same directory, and
moving the above definitions there?
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
