From: Jonathan Marek <jonathan@marek.ca>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
alsa-devel@alsa-project.org
Cc: Vinod Koul <vkoul@kernel.org>,
Sanyog Kale <sanyog.r.kale@intel.com>,
Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
open list <linux-kernel@vger.kernel.org>,
"open list:ARM/QUALCOMM SUPPORT" <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
Date: Tue, 9 Jun 2020 07:33:11 -0400 [thread overview]
Message-ID: <dc8f59c6-2fa9-f3a3-6d77-2d03a6d2776b@marek.ca> (raw)
In-Reply-To: <f3eabdb9-da43-9502-352a-1696babc114d@linaro.org>
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;
>>
next prev parent reply other threads:[~2020-06-09 11:32 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-08 20:43 [PATCH 0/5] soundwire: qcom: add mmio support Jonathan Marek
2020-06-08 20:43 ` [PATCH 1/5] soundwire: qcom: fix abh/ahb typo Jonathan Marek
2020-06-09 9:18 ` Srinivas Kandagatla
2020-06-08 20:43 ` [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices Jonathan Marek
2020-06-08 21:31 ` Pierre-Louis Bossart
2020-06-09 4:34 ` Vinod Koul
2020-06-09 9:18 ` Srinivas Kandagatla
2020-06-09 11:20 ` Jonathan Marek
2020-06-09 9:19 ` Srinivas Kandagatla
2020-06-09 11:04 ` Jonathan Marek
2020-06-10 10:55 ` Srinivas Kandagatla
2020-06-08 20:43 ` [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible Jonathan Marek
2020-06-09 5:26 ` Vinod Koul
2020-06-09 11:17 ` Jonathan Marek
2020-06-10 10:40 ` Srinivas Kandagatla
2020-06-08 20:43 ` [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS Jonathan Marek
2020-06-08 20:58 ` Jonathan Marek
2020-06-08 21:20 ` Pierre-Louis Bossart
2020-06-08 21:46 ` Jonathan Marek
2020-06-09 9:52 ` Srinivas Kandagatla
2020-06-09 11:33 ` Jonathan Marek [this message]
2020-06-10 10:36 ` Srinivas Kandagatla
2020-06-10 12:06 ` Jonathan Marek
2020-08-27 18:16 ` Dmitry Baryshkov
2020-06-08 20:43 ` [PATCH 5/5] soundwire: qcom: enable CPU interrupts for mmio devices Jonathan Marek
2020-06-09 9:26 ` [PATCH 0/5] soundwire: qcom: add mmio support Srinivas Kandagatla
2020-06-09 11:11 ` Jonathan Marek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dc8f59c6-2fa9-f3a3-6d77-2d03a6d2776b@marek.ca \
--to=jonathan@marek.ca \
--cc=agross@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=bjorn.andersson@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).