From: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
To: Stephen Boyd <swboyd@chromium.org>,
agross@kernel.org, alsa-devel@alsa-project.org,
bgoswami@codeaurora.org, bjorn.andersson@linaro.org,
broonie@kernel.org, devicetree@vger.kernel.org,
lgirdwood@gmail.com, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, perex@perex.cz,
plai@codeaurora.org, robh+dt@kernel.org, rohitkr@codeaurora.org,
srinivas.kandagatla@linaro.org, tiwai@suse.com
Cc: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
Subject: Re: [PATCH v3 3/5] ASoC: qcom: Add support for lpass hdmi driver
Date: Thu, 3 Sep 2020 14:59:41 +0530 [thread overview]
Message-ID: <36672d15-78bf-ffe7-71a3-4a7ed8664149@codeaurora.org> (raw)
In-Reply-To: <159894579447.334488.1604209140945740724@swboyd.mtv.corp.google.com>
Thanks Stephen for your time for review!!!
On 9/1/2020 1:06 PM, Stephen Boyd wrote:
> Quoting Srinivasa Rao Mandadapu (2020-08-30 23:39:22)
>> diff --git a/sound/soc/qcom/lpass-hdmi.c b/sound/soc/qcom/lpass-hdmi.c
>> new file mode 100644
>> index 0000000..7e18113
>> --- /dev/null
>> +++ b/sound/soc/qcom/lpass-hdmi.c
>> @@ -0,0 +1,684 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
> [...]
>> +
>> +static int lpass_hdmi_daiops_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>> +{
> [...]
>> + data_format = LPASS_DATA_FORMAT_LINEAR;
>> + ch_sts_buf0 = (((data_format << LPASS_DATA_FORMAT_SHIFT) & LPASS_DATA_FORMAT_MASK)
>> + | ((sampling_freq << LPASS_FREQ_BIT_SHIFT) & LPASS_FREQ_BIT_MASK));
>> + ch_sts_buf1 = (word_length) & LPASS_WORDLENGTH_MASK;
>> +
>> + ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_RESET);
>> + if (ret) {
>> + dev_err(dai->dev, "%s error writing to softreset enable : %d\n",
> All of these strings bloat the kernel image. Can we just return ret
> instead and if something goes wrong we can use a debug patch to figure
> out which register write failed? Would a register write even fail to
> begin with?
Will remove wherever possible.
>
>> + __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = regmap_field_write(drvdata->tx_ctl->soft_reset, LPASS_TX_CTL_CLEAR);
>> + if (ret) {
>> + dev_err(dai->dev, "%s error writing to softreset disable : %d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = regmap_field_write(drvdata->legacy->legacy_en,
>> + LPASS_HDMITX_LEGACY_DISABLE);
>> + if (ret) {
>> + dev_err(dai->dev, "%s error writing to legacy_en field : %d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> +
> [...]
>> #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)
>> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
>> index df692ed..607f4c4 100644
>> --- a/sound/soc/qcom/lpass-platform.c
>> +++ b/sound/soc/qcom/lpass-platform.c
>> @@ -553,7 +702,21 @@ static irqreturn_t lpass_platform_lpaif_irq(int irq, void *data)
>>
>> /* Handle per channel interrupts */
>> for (chan = 0; chan < LPASS_MAX_DMA_CHANNELS; chan++) {
>> - if (irqs & LPAIF_IRQ_ALL(chan) && drvdata->substream[chan]) {
>> + switch (v->id) {
>> + case HDMI_INTERFACE:
>> + val = LPAIF_IRQ_HDMI_REQ_ON_PRELOAD(chan) |
>> + LPAIF_IRQ_HDMI_METADONE |
>> + LPAIF_IRQ_HDMI_SDEEP_AUD_DIS(chan);
>> + break;
>> + case I2S_INTERFACE:
>> + val = 0;
>> + break;
>> + default:
>> + pr_err("%s: invalid %d interface\n", __func__, v->id);
> Any reason we can't use dev_err() here?
It's interrupt handler. Here dev variable is not available.
So we shall remove print as per your previous comment or keep as it is.
>
>> + return -EINVAL;
>> + }
>> + if (irqs & (LPAIF_IRQ_ALL(chan) | val)
>> + && drvdata->substream[chan]) {
>> rv = lpass_dma_interrupt_handler(
>> drvdata->substream[chan],
>> drvdata, chan, irqs);
>> @@ -644,15 +807,15 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
>>
>> /* ensure audio hardware is disabled */
>> ret = regmap_write(drvdata->lpaif_map,
>> - LPAIF_IRQEN_REG(v, LPAIF_IRQ_PORT_HOST), 0);
>> + IRQ_EN(v, LPAIF_IRQ_PORT_HOST), 0);
>> if (ret) {
>> dev_err(&pdev->dev, "error writing to irqen reg: %d\n", ret);
>> return ret;
>> }
>>
>> ret = devm_request_irq(&pdev->dev, drvdata->lpaif_irq,
>> - lpass_platform_lpaif_irq, IRQF_TRIGGER_RISING,
>> - "lpass-irq-lpaif", drvdata);
>> + lpass_platform_lpaif_irq, IRQF_TRIGGER_RISING,
> Can we use the irq flags from the firmware, i.e. whatever the DT or ACPI
> tables say?
okay.
>
>> + pdev->name, drvdata);
>> if (ret) {
>> dev_err(&pdev->dev, "irq request failed: %d\n", ret);
>> return ret;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
next prev parent reply other threads:[~2020-09-03 9:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-31 6:39 [PATCH v3 0/5] Qualcomm's lpass-hdmi ASoC driver to support audio over dp port Srinivasa Rao Mandadapu
2020-08-31 6:39 ` [PATCH v3 1/5] ASoC: Add sc7180-lpass binding header hdmi define Srinivasa Rao Mandadapu
2020-08-31 6:39 ` [PATCH v3 2/5] ASoC: dt-bindings: Add dt binding for lpass hdmi Srinivasa Rao Mandadapu
2020-09-04 10:41 ` Srinivas Kandagatla
2020-09-04 11:02 ` Srinivasa Rao Mandadapu
2020-08-31 6:39 ` [PATCH v3 3/5] ASoC: qcom: Add support for lpass hdmi driver Srinivasa Rao Mandadapu
2020-09-01 7:36 ` Stephen Boyd
2020-09-03 9:29 ` Srinivasa Rao Mandadapu [this message]
2020-09-04 10:41 ` Srinivas Kandagatla
2020-09-04 11:21 ` Srinivasa Rao Mandadapu
[not found] ` <0101017458d94c82-96bbcff0-018e-4f5d-8273-7869c3599d32-000000@us-west-2.amazonses.com>
2020-09-04 11:24 ` Srinivas Kandagatla
2020-09-08 17:42 ` Srinivasa Rao Mandadapu
2020-08-31 6:39 ` [PATCH v3 4/5] ASoC: qcom: Add support for audio over DP Srinivasa Rao Mandadapu
2020-09-01 7:32 ` Stephen Boyd
2020-09-03 9:23 ` Srinivasa Rao Mandadapu
2020-08-31 6:39 ` [PATCH v3 5/5] ASoC: qcom: Optimise lpass variant structure Srinivasa Rao Mandadapu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=36672d15-78bf-ffe7-71a3-4a7ed8664149@codeaurora.org \
--to=srivasam@codeaurora.org \
--cc=agross@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=bgoswami@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=plai@codeaurora.org \
--cc=robh+dt@kernel.org \
--cc=rohitkr@codeaurora.org \
--cc=srinivas.kandagatla@linaro.org \
--cc=swboyd@chromium.org \
--cc=tiwai@suse.com \
--cc=vsujithk@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).