Messages in this thread | | | Subject | Re: [PATCH v3 3/5] ASoC: qcom: Add support for lpass hdmi driver | From | Srinivasa Rao Mandadapu <> | Date | Fri, 4 Sep 2020 11:21:10 +0000 |
| |
Thanks Srinivas For Review!!!
On 9/4/2020 4:11 PM, Srinivas Kandagatla wrote: > > > On 31/08/2020 07:39, Srinivasa Rao Mandadapu wrote: >> From: V Sujith Kumar Reddy <vsujithk@codeaurora.org> >> >> Upadate lpass cpu and platform driver to support audio over dp. >> Also add lpass-hdmi.c and lpass-hdmi.h. >> >> Signed-off-by: Srinivasa Rao <srivasam@codeaurora.org> >> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org> >> --- >> sound/soc/qcom/Kconfig | 5 + >> sound/soc/qcom/Makefile | 2 + >> sound/soc/qcom/lpass-apq8016.c | 1 + >> sound/soc/qcom/lpass-cpu.c | 64 ++-- >> sound/soc/qcom/lpass-hdmi.c | 684 >> +++++++++++++++++++++++++++++++++++++++ >> sound/soc/qcom/lpass-hdmi.h | 129 ++++++++ >> sound/soc/qcom/lpass-ipq806x.c | 1 + >> sound/soc/qcom/lpass-lpaif-reg.h | 48 ++- >> sound/soc/qcom/lpass-platform.c | 287 ++++++++++++---- >> sound/soc/qcom/lpass.h | 88 ++++- >> 10 files changed, 1225 insertions(+), 84 deletions(-) >> create mode 100644 sound/soc/qcom/lpass-hdmi.c >> create mode 100644 sound/soc/qcom/lpass-hdmi.h >> >> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig >> index a607ace..509584c 100644 >> --- a/sound/soc/qcom/Kconfig >> +++ b/sound/soc/qcom/Kconfig >> @@ -12,6 +12,10 @@ config SND_SOC_LPASS_CPU >> tristate >> select REGMAP_MMIO >> +config SND_SOC_LPASS_HDMI >> + tristate >> + select REGMAP_MMIO >> + >> config SND_SOC_LPASS_PLATFORM >> tristate >> select REGMAP_MMIO >> @@ -30,6 +34,7 @@ config SND_SOC_LPASS_SC7180 >> tristate >> select SND_SOC_LPASS_CPU >> select SND_SOC_LPASS_PLATFORM >> + select SND_SOC_LPASS_HDMI >> config SND_SOC_STORM >> tristate "ASoC I2S support for Storm boards" >> diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile >> index 7972c94..0bd90d7 100644 >> --- a/sound/soc/qcom/Makefile >> +++ b/sound/soc/qcom/Makefile >> @@ -1,12 +1,14 @@ >> # SPDX-License-Identifier: GPL-2.0 >> # Platform >> snd-soc-lpass-cpu-objs := lpass-cpu.o >> +snd-soc-lpass-hdmi-objs := lpass-hdmi.o >> snd-soc-lpass-platform-objs := lpass-platform.o >> snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o >> snd-soc-lpass-apq8016-objs := lpass-apq8016.o >> snd-soc-lpass-sc7180-objs := lpass-sc7180.o >> obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o >> +obj-$(CONFIG_SND_SOC_LPASS_HDMI) += snd-soc-lpass-hdmi.o >> obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o >> obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o >> obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o >> diff --git a/sound/soc/qcom/lpass-apq8016.c >> b/sound/soc/qcom/lpass-apq8016.c >> index 5c8ae22..a1bc7e2 100644 >> --- a/sound/soc/qcom/lpass-apq8016.c >> +++ b/sound/soc/qcom/lpass-apq8016.c >> @@ -289,6 +289,7 @@ static struct lpass_variant apq8016_data = { >> .exit = apq8016_lpass_exit, >> .alloc_dma_channel = apq8016_lpass_alloc_dma_channel, >> .free_dma_channel = apq8016_lpass_free_dma_channel, >> + .id = I2S_INTERFACE, > > Before going into detail review, I see real issue in the overall > approach here to add new interface to exiting lpass!! > > Intention of struct lpass_variant is to address differences between > SoCs or different lpass versions. But you should not duplicate this > and use it for addressing differences between each lpass interfaces! > All the dai related register offsets should still go in to this > structure and driver should be able to know which dai its talking to > based on snd_soc_dai_driver id and select correct register offset. > Do You suggest to use separate structure like struct lpass_hdmi_interface in lpass_data? > Also on the other note, can you please split the patch if possible so > that it will be easy for review. Specially I would like to see header > file changes specific to adding new interface to be separate then > followed by the actual interface implementation and then the user. Okay, will split the patch. > > I also see some unrelated changes like changing buffer sizes, which > should go into different patch! Okay we will separate the patch. > > --srini
-- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
| |