Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/9/20 5:52 AM, Srinivas Kandagatla wrote:


On 08/06/2020 21:43, Jonathan Marek wrote:
The driver may be used without slimbus, so don't depend on slimbus.

Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx>
---
  drivers/soundwire/Kconfig | 1 -
  drivers/soundwire/qcom.c  | 5 +++++
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index fa2b4ab92ed9..d121cf739090 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -33,7 +33,6 @@ config SOUNDWIRE_INTEL
  config SOUNDWIRE_QCOM
      tristate "Qualcomm SoundWire Master driver"
-    depends on SLIMBUS
      depends on SND_SOC

Why not move this to imply SLIMBUS this will give more flexibility!



If you mean to change it to "select SLIMBUS", I'd prefer not to, because this would increase code size unnecessarily in my kernel.

      help
        SoundWire Qualcomm Master driver.
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 14334442615f..ac81c64768ea 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -769,13 +769,18 @@ static int qcom_swrm_probe(struct platform_device *pdev)
      if (!ctrl)
          return -ENOMEM;
+#ifdef CONFIG_SLIMBUS
      if (dev->parent->bus == &slimbus_bus) {
+#else
+    if (false) {
+#endif

May be you can do bit more cleanup here, which could endup like:


ctrl->regmap = dev_get_regmap(dev->parent, NULL);
if (!ctrl->regmap) {
     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
     if (res) {
         ctrl->mmio = devm_ioremap_resource(dev, res);
         if (IS_ERR(ctrl->mmio)) {
             dev_err(dev, "No valid mem resource found\n");
             return PTR_ERR(ctrl->mmio);
         }

         ctrl->reg_read = qcom_swrm_cpu_reg_read;
         ctrl->reg_write = qcom_swrm_cpu_reg_write;
     } else {
         dev_err(dev, "No valid slim resource found\n");
         return -EINVAL;
     }
} else {
     ctrl->reg_read = qcom_swrm_ahb_reg_read;
     ctrl->reg_write = qcom_swrm_ahb_reg_write;
}



thanks,
srini

I don't think this is better, it feels more obfuscated, and I think its possible we may end up with the mmio sdw having a parent with a regmap. (it is not how I did things up in my upstream stack, but in downstream the sdw nodes are inside the "macro" codec nodes)

I understand the '#ifdef CONFIG_SLIMBUS'/'dev->parent->bus == &slimbus_bus' is ugly, but at least its clear what's going on. Unless you have a better suggestion?

          ctrl->reg_read = qcom_swrm_ahb_reg_read;
          ctrl->reg_write = qcom_swrm_ahb_reg_write;
          ctrl->regmap = dev_get_regmap(dev->parent, NULL);
          if (!ctrl->regmap)
              return -EINVAL;
      } else {
+
          res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
          ctrl->reg_read = qcom_swrm_cpu_reg_read;




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux