Messages in this thread | | | Subject | Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS | From | Jonathan Marek <> | Date | Tue, 9 Jun 2020 07:33:11 -0400 |
| |
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@marek.ca> >> --- >> 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; >>
| |