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?
> + __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?
> + 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?
> + pdev->name, drvdata);
> if (ret) {
> dev_err(&pdev->dev, "irq request failed: %d\n", ret);
> return ret;
