| Message ID | 20200608204347.19685-1-jonathan@marek.ca |
|---|---|
| Headers | show |
| Series | soundwire: qcom: add mmio support | expand |
On 6/8/20 3:43 PM, Jonathan Marek wrote: > Adds support for qcom soundwire devices with memory mapped IO registers. 'device' is an ambiguous term for SoundWire. Seems to me this is a SoundWire Master device directly accessed with mmio registers instead of over a SLIMbus link? > > Signed-off-by: Jonathan Marek <jonathan@marek.ca> > --- > drivers/soundwire/qcom.c | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index f38d1fd3679f..628747df1c75 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -90,6 +90,7 @@ struct qcom_swrm_ctrl { > struct sdw_bus bus; > struct device *dev; > struct regmap *regmap; > + void __iomem *mmio; > struct completion *comp; > struct work_struct slave_work; > /* read/write lock */ > @@ -154,6 +155,20 @@ static int qcom_swrm_ahb_reg_write(struct qcom_swrm_ctrl *ctrl, > return SDW_CMD_OK; > } > > +static int qcom_swrm_cpu_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, > + u32 *val) > +{ > + *val = readl(ctrl->mmio + reg); > + return SDW_CMD_OK; > +} > + > +static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl *ctrl, int reg, > + int val) > +{ > + writel(val, ctrl->mmio + reg); > + return SDW_CMD_OK; > +} > + > static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data, > u8 dev_addr, u16 reg_addr) > { > @@ -746,6 +761,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) > struct sdw_master_prop *prop; > struct sdw_bus_params *params; > struct qcom_swrm_ctrl *ctrl; > + struct resource *res; > int ret; > u32 val; > > @@ -760,8 +776,13 @@ static int qcom_swrm_probe(struct platform_device *pdev) > if (!ctrl->regmap) > return -EINVAL; > } else { > - /* Only WCD based SoundWire controller is supported */ > - return -ENOTSUPP; I would move patch4 before this one, and add the functionality *after* removing the SLIMbus dependency. > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + > + ctrl->reg_read = qcom_swrm_cpu_reg_read; > + ctrl->reg_write = qcom_swrm_cpu_reg_write; > + ctrl->mmio = devm_ioremap_resource(dev, res); > + if (IS_ERR(ctrl->mmio)) > + return PTR_ERR(ctrl->mmio); maybe deal with the resource checks before setting callbacks? There are quite a few drivers who do things this way: clk/qcom/common.c- res = platform_get_resource(pdev, IORESOURCE_MEM, 0); clk/qcom/common.c: base = devm_ioremap_resource(dev, res); -- > } > > ctrl->irq = of_irq_get(dev->of_node, 0); >
On 08/06/2020 21:43, Jonathan Marek wrote: > Adds support for qcom soundwire devices with memory mapped IO registers. > > Signed-off-by: Jonathan Marek <jonathan@marek.ca> > --- In general patch itself looks pretty trivial, but I would like to see what 1.5.1 controller provides in terms of error reporting of SoundWire slave register reads/writes. On WCD based controller we did not have a mechanism to report things like if the read is ignored or not. I was hoping that this version of controller would be able to report that. I will be nice to those patches if that is something which is supported in this version. --srini > drivers/soundwire/qcom.c | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index f38d1fd3679f..628747df1c75 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -90,6 +90,7 @@ struct qcom_swrm_ctrl { > struct sdw_bus bus; > struct device *dev; > struct regmap *regmap; > + void __iomem *mmio; > struct completion *comp; > struct work_struct slave_work; > /* read/write lock */ > @@ -154,6 +155,20 @@ static int qcom_swrm_ahb_reg_write(struct qcom_swrm_ctrl *ctrl, > return SDW_CMD_OK; > } > > +static int qcom_swrm_cpu_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, > + u32 *val) > +{ > + *val = readl(ctrl->mmio + reg); > + return SDW_CMD_OK; > +} > + > +static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl *ctrl, int reg, > + int val) > +{ > + writel(val, ctrl->mmio + reg); > + return SDW_CMD_OK; > +} > + > static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data, > u8 dev_addr, u16 reg_addr) > { > @@ -746,6 +761,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) > struct sdw_master_prop *prop; > struct sdw_bus_params *params; > struct qcom_swrm_ctrl *ctrl; > + struct resource *res; > int ret; > u32 val; > > @@ -760,8 +776,13 @@ static int qcom_swrm_probe(struct platform_device *pdev) > if (!ctrl->regmap) > return -EINVAL; > } else { > - /* Only WCD based SoundWire controller is supported */ > - return -ENOTSUPP; > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + > + ctrl->reg_read = qcom_swrm_cpu_reg_read; > + ctrl->reg_write = qcom_swrm_cpu_reg_write; > + ctrl->mmio = devm_ioremap_resource(dev, res); > + if (IS_ERR(ctrl->mmio)) > + return PTR_ERR(ctrl->mmio); > } > > ctrl->irq = of_irq_get(dev->of_node, 0); >
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! > 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 > 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; >
On 6/9/20 5:18 AM, Srinivas Kandagatla wrote: > > > On 09/06/2020 05:34, Vinod Koul wrote: >> Hi Jonathan, >> >> On 08-06-20, 16:43, Jonathan Marek wrote: >>> Adds support for qcom soundwire devices with memory mapped IO registers. >> >> Please use 'SoundWire Master devices' instead :) >> >>> >>> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >>> --- >>> drivers/soundwire/qcom.c | 25 +++++++++++++++++++++++-- >>> 1 file changed, 23 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c >>> index f38d1fd3679f..628747df1c75 100644 >>> --- a/drivers/soundwire/qcom.c >>> +++ b/drivers/soundwire/qcom.c >>> @@ -90,6 +90,7 @@ struct qcom_swrm_ctrl { >>> struct sdw_bus bus; >>> struct device *dev; >>> struct regmap *regmap; >>> + void __iomem *mmio; >>> struct completion *comp; >>> struct work_struct slave_work; >>> /* read/write lock */ >>> @@ -154,6 +155,20 @@ static int qcom_swrm_ahb_reg_write(struct >>> qcom_swrm_ctrl *ctrl, >>> return SDW_CMD_OK; >>> } >>> +static int qcom_swrm_cpu_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, >>> + u32 *val) >>> +{ >>> + *val = readl(ctrl->mmio + reg); >>> + return SDW_CMD_OK; >>> +} >>> + >>> +static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl *ctrl, int >>> reg, >>> + int val) >>> +{ >>> + writel(val, ctrl->mmio + reg); >>> + return SDW_CMD_OK; >>> +} >> >> this looks fine but regmap supports mmio also, so I am thinking we >> should remove these and set regmap (mmio/slim)... Srini..? > > That is doable, but not going to add great value in this case, unless we > are having another layer of abstraction. So keeping it as readl/writel > seems okay to me. > Adding to this, the slim case doesn't use the regmap directly, it goes through AHB_BRIDGE registers. So using a single regmap path is not "doable" (at least not in a nice way IMO). > --srini > > >>
On 09/06/2020 12:33, Jonathan Marek wrote: > 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. imply is week select, which means that this driver can be built without SLIMBus selected. So removing reference to slimbus_bus is necessary in this case. On the other hand, SLIMBus is going to be used sm8250 for BT audio, so this would not be unnecessary. Also mostly these are build as modules, so not sure why kernel size will increase here! Am not 100% sure if SLIMBus will be kept on all SoCs, but keeping depends or imply in place would enforce or spell out some level of dependency on this. > >>> 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? Other suggestion I had in my mind was to use compatible strings to get reg_read, reg_write callbacks + some flags like (if_type) populated. This can help looking up resources correctly. Thanks, srini > >>> 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; >>>
On 09/06/2020 12:04, Jonathan Marek wrote: > On 6/9/20 5:19 AM, Srinivas Kandagatla wrote: >> >> >> On 08/06/2020 21:43, Jonathan Marek wrote: >>> Adds support for qcom soundwire devices with memory mapped IO registers. >>> >>> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >>> --- >> >> In general patch itself looks pretty trivial, but I would like to see >> what 1.5.1 controller provides in terms of error reporting of >> SoundWire slave register reads/writes. On WCD based controller we did >> not have a mechanism to report things like if the read is ignored or >> not. I was hoping that this version of controller would be able to >> report that. >> >> I will be nice to those patches if that is something which is >> supported in this version. >> >> --srini >> > > It does seem to support additional error reporting (it gets an error > during enumeration after finding the 2 WSA slaves). However the > downstream driver seems to disable this by setting BIT(31) in > FIFO_CFG_ADDR (the comment says "For SWR master version 1.5.1, continue > execute on command ignore"). Outside of the initial enumeration, it > doesn't seem to produce any extra errors (still relying on the timeout > mechanism to know if read/write is ignored). 1.5.* versions support reporting CMD_NACKED in FIFO_STATUS register, so you should use that instead of timeout mechanism which is used for 1.3.0 which did not have support for this. thanks, srini > >>> drivers/soundwire/qcom.c | 25 +++++++++++++++++++++++-- >>> 1 file changed, 23 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c >>> index f38d1fd3679f..628747df1c75 100644 >>> --- a/drivers/soundwire/qcom.c >>> +++ b/drivers/soundwire/qcom.c >>> @@ -90,6 +90,7 @@ struct qcom_swrm_ctrl { >>> struct sdw_bus bus; >>> struct device *dev; >>> struct regmap *regmap; >>> + void __iomem *mmio; >>> struct completion *comp; >>> struct work_struct slave_work; >>> /* read/write lock */ >>> @@ -154,6 +155,20 @@ static int qcom_swrm_ahb_reg_write(struct >>> qcom_swrm_ctrl *ctrl, >>> return SDW_CMD_OK; >>> } >>> +static int qcom_swrm_cpu_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, >>> + u32 *val) >>> +{ >>> + *val = readl(ctrl->mmio + reg); >>> + return SDW_CMD_OK; >>> +} >>> + >>> +static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl *ctrl, int >>> reg, >>> + int val) >>> +{ >>> + writel(val, ctrl->mmio + reg); >>> + return SDW_CMD_OK; >>> +} >>> + >>> static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, >>> u8 cmd_data, >>> u8 dev_addr, u16 reg_addr) >>> { >>> @@ -746,6 +761,7 @@ static int qcom_swrm_probe(struct platform_device >>> *pdev) >>> struct sdw_master_prop *prop; >>> struct sdw_bus_params *params; >>> struct qcom_swrm_ctrl *ctrl; >>> + struct resource *res; >>> int ret; >>> u32 val; >>> @@ -760,8 +776,13 @@ static int qcom_swrm_probe(struct >>> platform_device *pdev) >>> if (!ctrl->regmap) >>> return -EINVAL; >>> } else { >>> - /* Only WCD based SoundWire controller is supported */ >>> - return -ENOTSUPP; >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + >>> + ctrl->reg_read = qcom_swrm_cpu_reg_read; >>> + ctrl->reg_write = qcom_swrm_cpu_reg_write; >>> + ctrl->mmio = devm_ioremap_resource(dev, res); >>> + if (IS_ERR(ctrl->mmio)) >>> + return PTR_ERR(ctrl->mmio); >>> } >>> ctrl->irq = of_irq_get(dev->of_node, 0); >>>
On 6/10/20 6:36 AM, Srinivas Kandagatla wrote: > > > On 09/06/2020 12:33, Jonathan Marek wrote: >> 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. > > imply is week select, which means that this driver can be built without > SLIMBus selected. So removing reference to slimbus_bus is necessary in > this case. > Will change it to "imply", I wasn't aware of this keyword. > On the other hand, SLIMBus is going to be used sm8250 for BT audio, so > this would not be unnecessary. Also mostly these are build as modules, > so not sure why kernel size will increase here! > Not everyone uses modules. I am using a config with CONFIG_MODULES=n and only relevant drivers enabled. > Am not 100% sure if SLIMBus will be kept on all SoCs, but keeping > depends or imply in place would enforce or spell out some level of > dependency on this. > >> >>>> 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? > > Other suggestion I had in my mind was to use compatible strings to get > reg_read, reg_write callbacks + some flags like (if_type) populated. > This can help looking up resources correctly. > This is better than the previous suggestion, but is it safe to say that specific versions will always be used with MMIO or slimbus? (I guess the answer is yes, but I want to confirm) > Thanks, > srini > >> >>>> 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; >>>>
On 08/06/2020 23: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 I'd suggest: depends on SLIMBUS || !SLIMBUS #if SLIMBUS=m, this can not be builtin This would allow building both SLIMBUS and SOUNDWIRE_QCOM as modules > depends on SND_SOC > 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 and then #if IS_ENABLED(CONFIG_SLIBMUS) here > if (dev->parent->bus == &slimbus_bus) { > +#else > + if (false) { > +#endif > 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; > -- With best wishes Dmitry