Hi Philipp,
Thank you for your review, please check my comments inline.
/Viorel
-----Original Message----- From: Philipp Zabel [mailto:pza@pengutronix.de] Sent: Tuesday, September 22, 2020 3:09 PM To: Viorel Suman (OSS) viorel.suman@oss.nxp.com Cc: Liam Girdwood lgirdwood@gmail.com; Mark Brown broonie@kernel.org; Rob Herring robh+dt@kernel.org; Jaroslav Kysela perex@perex.cz; Takashi Iwai tiwai@suse.com; Timur Tabi timur@kernel.org; Nicolin Chen nicoleotsuka@gmail.com; Xiubo Li Xiubo.Lee@gmail.com; Fabio Estevam festevam@gmail.com; Shengjiu Wang shengjiu.wang@gmail.com; Viorel Suman viorel.suman@nxp.com; Matthias Schiffer matthias.schiffer@ew.tq-group.com; Cosmin-Gabriel Samoila cosmin.samoila@nxp.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linuxppc- dev@lists.ozlabs.org; dl-linux-imx linux-imx@nxp.com; Viorel Suman viorel.suman@gmail.com Subject: Re: [PATCH v2 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
On Mon, Sep 21, 2020 at 10:08:11PM +0300, Viorel Suman (OSS) wrote:
From: Viorel Suman viorel.suman@nxp.com
XCVR (Audio Transceiver) is a on-chip functional module found on i.MX8MP. It support HDMI2.1 eARC, HDMI1.4 ARC and SPDIF.
Signed-off-by: Viorel Suman viorel.suman@nxp.com
sound/soc/fsl/Kconfig | 10 + sound/soc/fsl/Makefile | 2 + sound/soc/fsl/fsl_xcvr.c | 1343 ++++++++++++++++++++++++++++++++++++++++++++++ sound/soc/fsl/fsl_xcvr.h | 266 +++++++++ 4 files changed, 1621 insertions(+) create mode 100644 sound/soc/fsl/fsl_xcvr.c create mode 100644 sound/soc/fsl/fsl_xcvr.h
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index 3f76ff7..d04b64d 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -95,6 +95,16 @@ config SND_SOC_FSL_EASRC destination sample rate. It is a new design module compare with the old ASRC.
+config SND_SOC_FSL_XCVR
- tristate "NXP Audio Transceiver (XCVR) module support"
- select REGMAP_MMIO
- select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
- select SND_SOC_GENERIC_DMAENGINE_PCM
- help
Say Y if you want to add Audio Transceiver (XCVR) support for NXPiMX CPUs. XCVR is a digital module that supports HDMI2.1 eARC,HDMI1.4 ARC and SPDIF.config SND_SOC_FSL_UTILS tristate
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index b835eeb..1d2231f 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -25,6 +25,7 @@ snd-soc-fsl-utils-objs := fsl_utils.o snd-soc-fsl-dma-objs := fsl_dma.o snd-soc-fsl-mqs-objs := fsl_mqs.o snd-soc-fsl-easrc-objs := fsl_easrc.o +snd-soc-fsl-xcvr-objs := fsl_xcvr.o
obj-$(CONFIG_SND_SOC_FSL_AUDMIX) += snd-soc-fsl-audmix.o obj-$(CONFIG_SND_SOC_FSL_ASOC_CARD) += snd-soc-fsl-asoc-card.o @@ -38,6 +39,7 @@ obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o obj-$(CONFIG_SND_SOC_FSL_MQS) += snd-soc-fsl-mqs.o obj-$(CONFIG_SND_SOC_FSL_EASRC) += snd-soc-fsl-easrc.o obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o +obj-$(CONFIG_SND_SOC_FSL_XCVR) += snd-soc-fsl-xcvr.o
# MPC5200 Platform Support obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c new file mode 100644 index 00000000..7391bca --- /dev/null +++ b/sound/soc/fsl/fsl_xcvr.c @@ -0,0 +1,1343 @@
[...]
+static int fsl_xcvr_probe(struct platform_device *pdev) {
- struct device *dev = &pdev->dev;
- struct device_node *np = dev->of_node;
- const struct of_device_id *of_id;
- struct fsl_xcvr *xcvr;
- struct resource *ram_res, *regs_res, *rx_res, *tx_res;
- void __iomem *regs;
- int ret, irq;
- of_id = of_match_device(fsl_xcvr_dt_ids, dev);
- if (!of_id)
return -EINVAL;- xcvr = devm_kzalloc(dev, sizeof(*xcvr), GFP_KERNEL);
- if (!xcvr)
return -ENOMEM;- xcvr->pdev = pdev;
- xcvr->ipg_clk = devm_clk_get(dev, "ipg");
- if (IS_ERR(xcvr->ipg_clk)) {
dev_err(dev, "failed to get ipg clock\n");return PTR_ERR(xcvr->ipg_clk);- }
- xcvr->phy_clk = devm_clk_get(dev, "phy");
- if (IS_ERR(xcvr->phy_clk)) {
dev_err(dev, "failed to get phy clock\n");return PTR_ERR(xcvr->phy_clk);- }
- xcvr->spba_clk = devm_clk_get(dev, "spba");
- if (IS_ERR(xcvr->spba_clk)) {
dev_err(dev, "failed to get spba clock\n");return PTR_ERR(xcvr->spba_clk);- }
- xcvr->pll_ipg_clk = devm_clk_get(dev, "pll_ipg");
- if (IS_ERR(xcvr->pll_ipg_clk)) {
dev_err(dev, "failed to get pll_ipg clock\n");return PTR_ERR(xcvr->pll_ipg_clk);- }
- ram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"ram");
- xcvr->ram_addr = devm_ioremap_resource(dev, ram_res);
- if (IS_ERR(xcvr->ram_addr))
return PTR_ERR(xcvr->ram_addr);- regs_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"regs");
- regs = devm_ioremap_resource(dev, regs_res);
- if (IS_ERR(regs))
return PTR_ERR(regs);- xcvr->regmap = devm_regmap_init_mmio_clk(dev, NULL, regs,
&fsl_xcvr_regmap_cfg);- if (IS_ERR(xcvr->regmap)) {
dev_err(dev, "failed to init XCVR regmap: %ld\n",PTR_ERR(xcvr->regmap));return PTR_ERR(xcvr->regmap);- }
- xcvr->reset = of_reset_control_get(np, NULL);
Please use devm_reset_control_get_exclusive().
Done in V3.
[...]
+static __maybe_unused int fsl_xcvr_runtime_resume(struct device *dev) +{
- struct fsl_xcvr *xcvr = dev_get_drvdata(dev);
- int ret;
- ret = clk_prepare_enable(xcvr->ipg_clk);
- if (ret) {
dev_err(dev, "failed to start IPG clock.\n");return ret;- }
- ret = clk_prepare_enable(xcvr->pll_ipg_clk);
- if (ret) {
dev_err(dev, "failed to start PLL IPG clock.\n");goto stop_ipg_clk;- }
- ret = clk_prepare_enable(xcvr->phy_clk);
- if (ret) {
dev_err(dev, "failed to start PHY clock: %d\n", ret);goto stop_pll_ipg_clk;- }
- ret = clk_prepare_enable(xcvr->spba_clk);
- if (ret) {
dev_err(dev, "failed to start SPBA clock.\n");goto stop_phy_clk;- }
- regcache_cache_only(xcvr->regmap, false);
- regcache_mark_dirty(xcvr->regmap);
- ret = regcache_sync(xcvr->regmap);
- if (ret) {
dev_err(dev, "failed to sync regcache.\n");goto stop_spba_clk;- }
- reset_control_assert(xcvr->reset);
- reset_control_deassert(xcvr->reset);
No delay required between the two?
Not required. Just to keep things in proper context - in V3 I moved reset_control_assert call into runtime_suspend.
/Viorel