11 Sep
2019
11 Sep
'19
12:12 p.m.
On Wed, Sep 11, 2019 at 10:42:39AM -0400, Shengjiu Wang wrote:
This looks good, a few minor comments below but nothing major - it's mostly nits with the DT binding.
--- /dev/null +++ b/sound/soc/fsl/fsl_mqs.c @@ -0,0 +1,336 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- ALSA SoC IMX MQS driver
- Copyright (C) 2014-2019 Freescale Semiconductor, Inc.
Please make the entire comment block a C++ comment so things look neater.
- /* On i.MX6sx the MQS control register is in GPR domain
* But in i.MX8QM/i.MX8QXP the control register is moved* to its own domain.*/- if (of_device_is_compatible(np, "fsl,imx8qm-mqs"))
mqs_priv->use_gpr = false;- else
mqs_priv->use_gpr = true;
The GPR was listed as a required property in the binding document but it is only needed here so the binding document should say "required if compatible is...".
- } else {
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);regs = devm_ioremap_resource(&pdev->dev, res);if (IS_ERR(regs))
You can use devm_platform_ioremap_resource() here.
mqs_priv->ipg = devm_clk_get(&pdev->dev, "core");if (IS_ERR(mqs_priv->ipg)) {dev_err(&pdev->dev, "failed to get the clock: %ld\n",PTR_ERR(mqs_priv->ipg));goto out;}
The core clock wasn't listed in the bindings document.