* [PATCH 0/5] soundwire: qcom: add mmio support
@ 2020-06-08 20:43 Jonathan Marek
2020-06-08 20:43 ` [PATCH 1/5] soundwire: qcom: fix abh/ahb typo Jonathan Marek
` (5 more replies)
0 siblings, 6 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Andy Gross, Bjorn Andersson, open list:ARM/QUALCOMM SUPPORT,
open list, Pierre-Louis Bossart, Sanyog Kale, Vinod Koul
This adds initial support for soundwire device on sm8250.
Tested with the "wsa" sdw device, which is simpler than the others.
Jonathan Marek (5):
soundwire: qcom: fix abh/ahb typo
soundwire: qcom: add support for mmio soundwire devices
soundwire: qcom: add v1.5.1 compatible
soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
soundwire: qcom: enable CPU interrupts for mmio devices
drivers/soundwire/Kconfig | 1 -
drivers/soundwire/qcom.c | 42 +++++++++++++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 5 deletions(-)
--
2.26.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/5] soundwire: qcom: fix abh/ahb typo
2020-06-08 20:43 [PATCH 0/5] soundwire: qcom: add mmio support Jonathan Marek
@ 2020-06-08 20:43 ` 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
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
The function name qcom_swrm_abh_reg_read should say ahb, fix that.
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
drivers/soundwire/qcom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index a1c2a44a3b4d..f38d1fd3679f 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -114,7 +114,7 @@ struct qcom_swrm_ctrl {
#define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus)
-static int qcom_swrm_abh_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
+static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
u32 *val)
{
struct regmap *wcd_regmap = ctrl->regmap;
@@ -754,7 +754,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
return -ENOMEM;
if (dev->parent->bus == &slimbus_bus) {
- ctrl->reg_read = qcom_swrm_abh_reg_read;
+ 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)
--
2.26.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
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-08 20:43 ` Jonathan Marek
2020-06-08 21:31 ` Pierre-Louis Bossart
` (2 more replies)
2020-06-08 20:43 ` [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible Jonathan Marek
` (3 subsequent siblings)
5 siblings, 3 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
Adds support for qcom soundwire devices with memory mapped IO registers.
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;
+ 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);
--
2.26.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible
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-08 20:43 ` [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices Jonathan Marek
@ 2020-06-08 20:43 ` Jonathan Marek
2020-06-09 5:26 ` Vinod Koul
2020-06-08 20:43 ` [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS Jonathan Marek
` (2 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
Add a compatible string for HW version v1.5.1 on sm8250 SoCs.
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
drivers/soundwire/qcom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 628747df1c75..14334442615f 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -880,6 +880,7 @@ static int qcom_swrm_remove(struct platform_device *pdev)
static const struct of_device_id qcom_swrm_of_match[] = {
{ .compatible = "qcom,soundwire-v1.3.0", },
+ { .compatible = "qcom,soundwire-v1.5.1", },
{/* sentinel */},
};
--
2.26.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-08 20:43 [PATCH 0/5] soundwire: qcom: add mmio support Jonathan Marek
` (2 preceding siblings ...)
2020-06-08 20:43 ` [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible Jonathan Marek
@ 2020-06-08 20:43 ` Jonathan Marek
2020-06-08 20:58 ` Jonathan Marek
` (3 more replies)
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
5 siblings, 4 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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
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
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;
--
2.26.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/5] soundwire: qcom: enable CPU interrupts for mmio devices
2020-06-08 20:43 [PATCH 0/5] soundwire: qcom: add mmio support Jonathan Marek
` (3 preceding siblings ...)
2020-06-08 20:43 ` [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS Jonathan Marek
@ 2020-06-08 20:43 ` Jonathan Marek
2020-06-09 9:26 ` [PATCH 0/5] soundwire: qcom: add mmio support Srinivas Kandagatla
5 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:43 UTC (permalink / raw)
To: alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
This allows the CPU to receive interrupts.
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
drivers/soundwire/qcom.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index ac81c64768ea..58ffb46e0d64 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -34,6 +34,7 @@
#define SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED BIT(10)
#define SWRM_INTERRUPT_MASK_ADDR 0x204
#define SWRM_INTERRUPT_CLEAR 0x208
+#define SWRM_INTERRUPT_CPU_EN 0x210
#define SWRM_CMD_FIFO_WR_CMD 0x300
#define SWRM_CMD_FIFO_RD_CMD 0x304
#define SWRM_CMD_FIFO_CMD 0x308
@@ -325,6 +326,12 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR,
SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK |
SWRM_COMP_CFG_ENABLE_MSK);
+
+ /* enable CPU IRQs */
+ if (ctrl->mmio) {
+ ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN,
+ SWRM_INTERRUPT_STATUS_RMSK);
+ }
return 0;
}
--
2.26.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
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
` (2 subsequent siblings)
3 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 20:58 UTC (permalink / raw)
To: alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
On 6/8/20 4:43 PM, 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
> 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
> 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 {
> +
Oops, ended up with a stray whitespace here, will fix for v2.
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> ctrl->reg_read = qcom_swrm_cpu_reg_read;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
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-08-27 18:16 ` Dmitry Baryshkov
3 siblings, 1 reply; 27+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-08 21:20 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Andy Gross, Bjorn Andersson, open list,
open list:ARM/QUALCOMM SUPPORT
On 6/8/20 3:43 PM, 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
> 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
maybe:
if (IS_ENABLED(CONFIG_SLIMBUS) &&
dev->parent->bus == &slimbus_bus)
> 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;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
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:19 ` Srinivas Kandagatla
2 siblings, 0 replies; 27+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-08 21:31 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
open list:ARM/QUALCOMM SUPPORT, open list
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);
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-08 21:20 ` Pierre-Louis Bossart
@ 2020-06-08 21:46 ` Jonathan Marek
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-08 21:46 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Andy Gross, Bjorn Andersson, open list,
open list:ARM/QUALCOMM SUPPORT
On 6/8/20 5:20 PM, Pierre-Louis Bossart wrote:
>
>
> On 6/8/20 3:43 PM, 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
>> 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
>
>
> maybe:
>
> if (IS_ENABLED(CONFIG_SLIMBUS) &&
> dev->parent->bus == &slimbus_bus)
>
>
It won't compile like this, because "slimbus_bus" is not defined when
CONFIG_SLIMBUS is disabled.
>> 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;
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
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 9:19 ` Srinivas Kandagatla
2 siblings, 1 reply; 27+ messages in thread
From: Vinod Koul @ 2020-06-09 4:34 UTC (permalink / raw)
To: Jonathan Marek
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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..?
--
~Vinod
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible
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
0 siblings, 1 reply; 27+ messages in thread
From: Vinod Koul @ 2020-06-09 5:26 UTC (permalink / raw)
To: Jonathan Marek
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
On 08-06-20, 16:43, Jonathan Marek wrote:
> Add a compatible string for HW version v1.5.1 on sm8250 SoCs.
Please document this new compatible
--
~Vinod
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] soundwire: qcom: fix abh/ahb typo
2020-06-08 20:43 ` [PATCH 1/5] soundwire: qcom: fix abh/ahb typo Jonathan Marek
@ 2020-06-09 9:18 ` Srinivas Kandagatla
0 siblings, 0 replies; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-09 9:18 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
On 08/06/2020 21:43, Jonathan Marek wrote:
> The function name qcom_swrm_abh_reg_read should say ahb, fix that.
>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> drivers/soundwire/qcom.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index a1c2a44a3b4d..f38d1fd3679f 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -114,7 +114,7 @@ struct qcom_swrm_ctrl {
>
> #define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus)
>
> -static int qcom_swrm_abh_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
> +static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg,
> u32 *val)
> {
> struct regmap *wcd_regmap = ctrl->regmap;
> @@ -754,7 +754,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> if (dev->parent->bus == &slimbus_bus) {
> - ctrl->reg_read = qcom_swrm_abh_reg_read;
> + 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)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
2020-06-09 4:34 ` Vinod Koul
@ 2020-06-09 9:18 ` Srinivas Kandagatla
2020-06-09 11:20 ` Jonathan Marek
0 siblings, 1 reply; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-09 9:18 UTC (permalink / raw)
To: Vinod Koul, Jonathan Marek
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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.
--srini
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
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:19 ` Srinivas Kandagatla
2020-06-09 11:04 ` Jonathan Marek
2 siblings, 1 reply; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-09 9:19 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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);
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/5] soundwire: qcom: add mmio support
2020-06-08 20:43 [PATCH 0/5] soundwire: qcom: add mmio support Jonathan Marek
` (4 preceding siblings ...)
2020-06-08 20:43 ` [PATCH 5/5] soundwire: qcom: enable CPU interrupts for mmio devices Jonathan Marek
@ 2020-06-09 9:26 ` Srinivas Kandagatla
2020-06-09 11:11 ` Jonathan Marek
5 siblings, 1 reply; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-09 9:26 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Andy Gross, Bjorn Andersson, open list:ARM/QUALCOMM SUPPORT,
open list, Pierre-Louis Bossart, Sanyog Kale, Vinod Koul
Thanks Jonathan for the patches,
On 08/06/2020 21:43, Jonathan Marek wrote:
> This adds initial support for soundwire device on sm8250.
>
One thing off my list!!
> Tested with the "wsa" sdw device, which is simpler than the others.
WSA881x?
did you test both enumeration and streaming?
Are you planing to add any new WSA or WCD codec support for this SoC?
thanks,
srini
>
> Jonathan Marek (5):
> soundwire: qcom: fix abh/ahb typo
> soundwire: qcom: add support for mmio soundwire devices
> soundwire: qcom: add v1.5.1 compatible
> soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
> soundwire: qcom: enable CPU interrupts for mmio devices
>
> drivers/soundwire/Kconfig | 1 -
> drivers/soundwire/qcom.c | 42 +++++++++++++++++++++++++++++++++++----
> 2 files changed, 38 insertions(+), 5 deletions(-)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
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-09 9:52 ` Srinivas Kandagatla
2020-06-09 11:33 ` Jonathan Marek
2020-08-27 18:16 ` Dmitry Baryshkov
3 siblings, 1 reply; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-09 9:52 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
2020-06-09 9:19 ` Srinivas Kandagatla
@ 2020-06-09 11:04 ` Jonathan Marek
2020-06-10 10:55 ` Srinivas Kandagatla
0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Marek @ 2020-06-09 11:04 UTC (permalink / raw)
To: Srinivas Kandagatla, alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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).
>> 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);
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/5] soundwire: qcom: add mmio support
2020-06-09 9:26 ` [PATCH 0/5] soundwire: qcom: add mmio support Srinivas Kandagatla
@ 2020-06-09 11:11 ` Jonathan Marek
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-09 11:11 UTC (permalink / raw)
To: Srinivas Kandagatla, alsa-devel
Cc: Andy Gross, Bjorn Andersson, open list:ARM/QUALCOMM SUPPORT,
open list, Pierre-Louis Bossart, Sanyog Kale, Vinod Koul
On 6/9/20 5:26 AM, Srinivas Kandagatla wrote:
> Thanks Jonathan for the patches,
>
> On 08/06/2020 21:43, Jonathan Marek wrote:
>> This adds initial support for soundwire device on sm8250.
>>
>
> One thing off my list!!
>
>> Tested with the "wsa" sdw device, which is simpler than the others.
>
> WSA881x?
>
> did you test both enumeration and streaming?
>
> Are you planing to add any new WSA or WCD codec support for this SoC?
>
> thanks,
> srini
>
Yes, dual WSA881x. I tested streaming, however for the dual WSA881x case
I need specific amixer settings and an extra hack to make it work (seems
to port config stuff is broken, especially around VISENSE, it tries to
use it in the wrong direction I think). For a single WSA881x, it can
work without any extra hack.
The SoC has at least 4 codec drivers to add ("wsa macro", "rx macro",
"tx macro", "wcd938x"), with what I am doing right now I only need the
"wsa macro" codec which I've ported over from downstream. For now I am
only looking at sending patches for the required ADSP changes, but may
consider doing something with the "wsa macro" driver at some point.
>>
>> Jonathan Marek (5):
>> soundwire: qcom: fix abh/ahb typo
>> soundwire: qcom: add support for mmio soundwire devices
>> soundwire: qcom: add v1.5.1 compatible
>> soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
>> soundwire: qcom: enable CPU interrupts for mmio devices
>>
>> drivers/soundwire/Kconfig | 1 -
>> drivers/soundwire/qcom.c | 42 +++++++++++++++++++++++++++++++++++----
>> 2 files changed, 38 insertions(+), 5 deletions(-)
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible
2020-06-09 5:26 ` Vinod Koul
@ 2020-06-09 11:17 ` Jonathan Marek
2020-06-10 10:40 ` Srinivas Kandagatla
0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Marek @ 2020-06-09 11:17 UTC (permalink / raw)
To: Vinod Koul
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
On 6/9/20 1:26 AM, Vinod Koul wrote:
> On 08-06-20, 16:43, Jonathan Marek wrote:
>> Add a compatible string for HW version v1.5.1 on sm8250 SoCs.
>
> Please document this new compatible
>
Does it really need to be documented? The documentation already says the
compatible should be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>". It gives
"qcom,soundwire-v1.5.0" as an example, which is not actually a supported
compatible, so my understanding is we don't need to update the list of
examples with every possible compatible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
2020-06-09 9:18 ` Srinivas Kandagatla
@ 2020-06-09 11:20 ` Jonathan Marek
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-09 11:20 UTC (permalink / raw)
To: Srinivas Kandagatla, Vinod Koul
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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
>
>
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-09 9:52 ` Srinivas Kandagatla
@ 2020-06-09 11:33 ` Jonathan Marek
2020-06-10 10:36 ` Srinivas Kandagatla
0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Marek @ 2020-06-09 11:33 UTC (permalink / raw)
To: Srinivas Kandagatla, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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;
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-09 11:33 ` Jonathan Marek
@ 2020-06-10 10:36 ` Srinivas Kandagatla
2020-06-10 12:06 ` Jonathan Marek
0 siblings, 1 reply; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-10 10:36 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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;
>>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] soundwire: qcom: add v1.5.1 compatible
2020-06-09 11:17 ` Jonathan Marek
@ 2020-06-10 10:40 ` Srinivas Kandagatla
0 siblings, 0 replies; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-10 10:40 UTC (permalink / raw)
To: Jonathan Marek, Vinod Koul
Cc: alsa-devel, Andy Gross, Bjorn Andersson, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
On 09/06/2020 12:17, Jonathan Marek wrote:
> On 6/9/20 1:26 AM, Vinod Koul wrote:
>> On 08-06-20, 16:43, Jonathan Marek wrote:
>>> Add a compatible string for HW version v1.5.1 on sm8250 SoCs.
>>
>> Please document this new compatible
>>
>
> Does it really need to be documented? The documentation already says the
> compatible should be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>". It gives
> "qcom,soundwire-v1.5.0" as an example, which is not actually a supported
> compatible, so my understanding is we don't need to update the list of
> examples with every possible compatible.
checkpatch should have reported about this, and in future once we
convert to yaml and list the compatible strings then dt_binding_check
would fail too. So there is no harm in adding an additional compatible
string for this new entry.
--srini
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] soundwire: qcom: add support for mmio soundwire devices
2020-06-09 11:04 ` Jonathan Marek
@ 2020-06-10 10:55 ` Srinivas Kandagatla
0 siblings, 0 replies; 27+ messages in thread
From: Srinivas Kandagatla @ 2020-06-10 10:55 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Andy Gross, Bjorn Andersson, Vinod Koul, Sanyog Kale,
Pierre-Louis Bossart, open list:ARM/QUALCOMM SUPPORT, open list
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);
>>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-10 10:36 ` Srinivas Kandagatla
@ 2020-06-10 12:06 ` Jonathan Marek
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Marek @ 2020-06-10 12:06 UTC (permalink / raw)
To: Srinivas Kandagatla, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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;
>>>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS
2020-06-08 20:43 ` [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS Jonathan Marek
` (2 preceding siblings ...)
2020-06-09 9:52 ` Srinivas Kandagatla
@ 2020-08-27 18:16 ` Dmitry Baryshkov
3 siblings, 0 replies; 27+ messages in thread
From: Dmitry Baryshkov @ 2020-08-27 18:16 UTC (permalink / raw)
To: Jonathan Marek, alsa-devel
Cc: Vinod Koul, Sanyog Kale, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, open list, open list:ARM/QUALCOMM SUPPORT
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
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-08-27 18:16 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).