[PATCH 4/4] soundwire: bus: add missing \n in dynamic debug
From: Pierre-Louis Bossart
They were missed in previous contributions.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 100d904bf700..85bcf60f9697 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -886,7 +886,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
}
val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
- dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d",
+ dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d\n",
dev_num);
return 0;
}
@@ -895,7 +895,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
retry--;
} while (retry);
- dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d",
+ dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d\n",
dev_num);
return -ETIMEDOUT;
--
2.17.1
[PATCH 3/4] soundwire: bus: handle -ENODATA errors in clock stop/start sequences
From: Pierre-Louis Bossart
If a device lost sync and can no longer ACK a command, it may not be
able to enter a lower-power state but it will still be able to resync
when the clock restarts. In those cases, we want to continue with the
clock stop sequence.
This patch modifies the behavior during clock stop sequences to only
log errors unrelated to -ENODATA/Command_Ignored. The flow is also
modified so that loops continue to prepare/deprepare other devices
even when one seems to have lost sync.
When resuming the clocks, all issues are logged with a dev_warn(),
previously only some of them were checked. This is the only part that
now differs between the clock stop entry and clock stop exit
sequences: while we don't want to stop the suspend flow, we do want
information on potential issues while resuming, as they may have
ripple effects.
For consistency the log messages are also modified to be unique and
self-explanatory. Errors in sdw_slave_clk_stop_callback() were
removed, they are now handled in the caller.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 69 +
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index dc4033b6f2e9..100d904bf700 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -829,11 +829,8 @@ static int sdw_slave_clk_stop_callback(struct sdw_slave
*slave,
if (slave->ops && slave->ops->clk_stop) {
ret = slave->ops->clk_stop(slave, mode, type);
- if (ret < 0) {
- dev_err(>dev,
- "Clk Stop type =%d failed: %d\n", type, ret);
+ if (ret < 0)
return ret;
- }
}
return 0;
@@ -860,7 +857,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
} else {
ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
if (ret < 0) {
- dev_err(>dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
+ if (ret != -ENODATA)
+ dev_err(>dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
return ret;
}
val = ret;
@@ -869,9 +867,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
- if (ret < 0)
- dev_err(>dev,
- "Clock Stop prepare failed for slave: %d", ret);
+ if (ret < 0 && ret != -ENODATA)
+ dev_err(>dev, "SDW_SCP_SYSTEMCTRL write failed:%d\n",
ret);
return ret;
}
@@ -922,6 +919,9 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
* In order to save on transition time, prepare
* each Slave and then wait for all Slave(s) to be
* prepared for clock stop.
+* If one of the Slave devices has lost sync and
+* replies with Command Ignored/-ENODATA, we continue
+* the loop
*/
list_for_each_entry(slave, >slaves, node) {
if (!slave->dev_num)
@@ -937,9 +937,8 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
ret = sdw_slave_clk_stop_callback(slave,
SDW_CLK_STOP_MODE0,
SDW_CLK_PRE_PREPARE);
- if (ret < 0) {
- dev_err(>dev,
- "pre-prepare failed:%d", ret);
+ if (ret < 0 && ret != -ENODATA) {
+ dev_err(>dev, "clock stop pre-prepare cb
failed:%d\n", ret);
return ret;
}
@@ -950,9 +949,8 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
ret = sdw_slave_clk_stop_prepare(slave,
SDW_CLK_STOP_MODE0,
true);
- if (ret < 0) {
- dev_err(>dev,
- "pre-prepare failed:%d", ret);
+ if (ret < 0 && ret != -ENODATA) {
+ dev_err(>dev, "clock stop prepare
failed:%d\n", ret);
return ret;
}
}
@@ -960,7 +958,7 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
/* Skip remaining clock stop preparation if no Slave is attached */
if (!is_slave)
- return ret;
+ return 0;
/*
* Don't wait for all Slaves
[PATCH 2/4] soundwire: add missing kernel-doc description
From: Pierre-Louis Bossart
For some reason we never added a description for the clk_stop
callback.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
include/linux/soundwire/sdw.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 5d93d9949653..8ca736e92d5a 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -612,6 +612,7 @@ struct sdw_bus_params {
* @update_status: Update Slave status
* @bus_config: Update the bus config for Slave
* @port_prep: Prepare the port with parameters
+ * @clk_stop: handle imp-def sequences before and after prepare and de-prepare
*/
struct sdw_slave_ops {
int (*read_prop)(struct sdw_slave *sdw);
--
2.17.1
[PATCH 1/4] soundwire: bus: only use CLOCK_STOP_MODE0 and fix confusions
From: Pierre-Louis Bossart
Existing devices and implementations only support the required
CLOCK_STOP_MODE0. All the code related to CLOCK_STOP_MODE1 has not
been tested and is highly questionable, with a clear confusion between
CLOCK_STOP_MODE1 and the simple clock stop state machine.
This patch removes all usages of CLOCK_STOP_MODE1 - which has no
impact on any solution - and fixes the use of the simple clock stop
state machine. The resulting code should be a lot more symmetrical and
easier to maintain.
Note that CLOCK_STOP_MODE1 is not supported in the SoundWire Device
Class specification so it's rather unlikely that we need to re-add
this mode later.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 100 ++
include/linux/soundwire/sdw.h | 2 -
2 files changed, 40 insertions(+), 62 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index a9e0aa72654d..dc4033b6f2e9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -821,26 +821,6 @@ static void sdw_modify_slave_status(struct sdw_slave
*slave,
mutex_unlock(>bus_lock);
}
-static enum sdw_clk_stop_mode sdw_get_clk_stop_mode(struct sdw_slave *slave)
-{
- enum sdw_clk_stop_mode mode;
-
- /*
-* Query for clock stop mode if Slave implements
-* ops->get_clk_stop_mode, else read from property.
-*/
- if (slave->ops && slave->ops->get_clk_stop_mode) {
- mode = slave->ops->get_clk_stop_mode(slave);
- } else {
- if (slave->prop.clk_stop_mode1)
- mode = SDW_CLK_STOP_MODE1;
- else
- mode = SDW_CLK_STOP_MODE0;
- }
-
- return mode;
-}
-
static int sdw_slave_clk_stop_callback(struct sdw_slave *slave,
enum sdw_clk_stop_mode mode,
enum sdw_clk_stop_type type)
@@ -933,7 +913,6 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
*/
int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
{
- enum sdw_clk_stop_mode slave_mode;
bool simple_clk_stop = true;
struct sdw_slave *slave;
bool is_slave = false;
@@ -955,10 +934,8 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
/* Identify if Slave(s) are available on Bus */
is_slave = true;
- slave_mode = sdw_get_clk_stop_mode(slave);
- slave->curr_clk_stop_mode = slave_mode;
-
- ret = sdw_slave_clk_stop_callback(slave, slave_mode,
+ ret = sdw_slave_clk_stop_callback(slave,
+ SDW_CLK_STOP_MODE0,
SDW_CLK_PRE_PREPARE);
if (ret < 0) {
dev_err(>dev,
@@ -966,22 +943,29 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
return ret;
}
- ret = sdw_slave_clk_stop_prepare(slave,
-slave_mode, true);
- if (ret < 0) {
- dev_err(>dev,
- "pre-prepare failed:%d", ret);
- return ret;
- }
-
- if (slave_mode == SDW_CLK_STOP_MODE1)
+ /* Only prepare a Slave device if needed */
+ if (!slave->prop.simple_clk_stop_capable) {
simple_clk_stop = false;
+
+ ret = sdw_slave_clk_stop_prepare(slave,
+SDW_CLK_STOP_MODE0,
+true);
+ if (ret < 0) {
+ dev_err(>dev,
+ "pre-prepare failed:%d", ret);
+ return ret;
+ }
+ }
}
/* Skip remaining clock stop preparation if no Slave is attached */
if (!is_slave)
return ret;
+ /*
+* Don't wait for all Slaves to be ready if they follow the simple
+* state machine
+*/
if (!simple_clk_stop) {
ret = sdw_bus_wait_for_clk_prep_deprep(bus,
SDW_BROADCAST_DEV_NUM);
@@ -998,17 +982,13 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
slave->status != SDW_SLAVE_ALERT)
continue;
- slave_mode = slave->curr_clk_stop_mode;
+ ret = sdw_slave_clk_stop_callback(slave,
+ SDW_CLK_STOP_MODE0,
+
[PATCH 0/4] soundwire: only use CLOCK_STOP_MODE0 and handle -ENODATA
Existing devices and implementations only support the required CLOCK_STOP_MODE0. All the code related to CLOCK_STOP_MODE1 has not been tested and is highly questionable, with a clear confusion between CLOCK_STOP_MODE1 and the simple clock stop state machine. This patch removes all usages of CLOCK_STOP_MODE1 - which has no impact on any solution - and fixes the use of the simple clock stop state machine. The resulting code should be a lot more symmetrical and easier to maintain. Note that CLOCK_STOP_MODE1 is not supported in the SoundWire Device Class specification so it's rather unlikely that we need to re-add this mode later. If a device lost sync and can no longer ACK a command, it may not be able to enter a lower-power state but it will still be able to resync when the clock restarts. In those cases, we want to continue with the clock stop sequence. This patch modifies the behavior during clock stop sequences to only log errors unrelated to -ENODATA/Command_Ignored. The flow is also modified so that loops continue to prepare/deprepare other devices even when one seems to have lost sync. When resuming the clocks, all issues are logged with a dev_warn(), previously only some of them were checked. This is the only part that now differs between the clock stop entry and clock stop exit sequences: while we don't want to stop the suspend flow, we do want information on potential issues while resuming, as they may have ripple effects. For consistency the log messages are also modified to be unique and self-explanatory. Errors in sdw_slave_clk_stop_callback() were removed, they are now handled in the caller. Pierre-Louis Bossart (4): soundwire: bus: only use CLOCK_STOP_MODE0 and fix confusions soundwire: add missing kernel-doc description soundwire: bus: handle -ENODATA errors in clock stop/start sequences soundwire: bus: add missing \n in dynamic debug drivers/soundwire/bus.c | 155 +++--- include/linux/soundwire/sdw.h | 3 +- 2 files changed, 70 insertions(+), 88 deletions(-) -- 2.17.1
[PATCH] soundwire: cadence_master: always set CMD_ACCEPT
From: Pierre-Louis Bossart
The Cadence IP can be configured in two different ways to deal with
CMD_IGNORED replies to broadcast commands. The CMD_ACCEPT bitfield
controls whether the command is discarded or if the IP proceeds with
the change (typically a bank switch or clock stop command).
The existing code seems to be inconsistent:
a) For some historical reason, we set this CMD_ACCEPT bitfield during
the initialization, but we don't during a resume from a clock-stoppped
state.
b) In addition, the loop used in the clock-stop sequence is quite
racy, it's possible that a device has lost sync but it's still tagged
as ATTACHED.
c) If somehow a Device loses sync and is unable to ack a broadcast
command, we do not have an error handling mechanism anyways. The IP
should go ahead and let the Device regain sync at a later time.
Make sure the CMD_ACCEPT bit is always set.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 192dac10f0c2..25950422b085 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1428,20 +1428,6 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool
block_wake)
}
}
- /*
-* This CMD_ACCEPT should be used when there are no devices
-* attached on the link when entering clock stop mode. If this is
-* not set and there is a broadcast write then the command ignored
-* will be treated as a failure
-*/
- if (!slave_present)
- cdns_updatel(cdns, CDNS_MCP_CONTROL,
-CDNS_MCP_CONTROL_CMD_ACCEPT,
-CDNS_MCP_CONTROL_CMD_ACCEPT);
- else
- cdns_updatel(cdns, CDNS_MCP_CONTROL,
-CDNS_MCP_CONTROL_CMD_ACCEPT, 0);
-
/* commit changes */
ret = cdns_config_update(cdns);
if (ret < 0) {
@@ -1508,11 +1494,8 @@ int sdw_cdns_clock_restart(struct sdw_cdns *cdns, bool
bus_reset)
cdns_updatel(cdns, CDNS_MCP_CONTROL,
CDNS_MCP_CONTROL_BLOCK_WAKEUP, 0);
- /*
-* clear CMD_ACCEPT so that the command ignored
-* will be treated as a failure during a broadcast write
-*/
- cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT, 0);
+ cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT,
+CDNS_MCP_CONTROL_CMD_ACCEPT);
if (!bus_reset) {
--
2.17.1
[PATCH v3] soundwire: intel: move to auxiliary bus
From: Pierre-Louis Bossart
Now that the auxiliary_bus exists, there's no reason to use platform
devices as children of a PCI device any longer.
This patch refactors the code by extending a basic auxiliary device
with Intel link-specific structures that need to be passed between
controller and link levels. This refactoring is much cleaner with no
need for cross-pointers between device and link structures.
Note that the auxiliary bus API has separate init and add steps, which
requires more attention in the error unwinding paths. The main loop
needs to deal with kfree() and auxiliary_device_uninit() for the
current iteration before jumping to the common label which releases
everything allocated in prior iterations.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Ranjani Sridharan
Signed-off-by: Bard Liao
---
v2:
- add link_dev_register for all kzalloc, device_init, and device_add.
v3:
- Modify the function description of sdw_intel_probe() which was
missing in previous version.
---
drivers/soundwire/Kconfig | 1 +
drivers/soundwire/intel.c | 56 ---
drivers/soundwire/intel.h | 14 +-
drivers/soundwire/intel_init.c | 232 +++-
include/linux/soundwire/sdw_intel.h | 6 +-
5 files changed, 202 insertions(+), 107 deletions(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 016e74230bb7..2b7795233282 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
tristate "Intel SoundWire Master driver"
select SOUNDWIRE_CADENCE
select SOUNDWIRE_GENERIC_ALLOCATION
+ select AUXILIARY_BUS
depends on ACPI && SND_SOC
help
SoundWire Intel Master driver.
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index fd95f94630b1..c11e3d8cd308 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1327,11 +1327,14 @@ static int intel_init(struct sdw_intel *sdw)
}
/*
- * probe and init
+ * probe and init (aux_dev_id argument is required by function prototype but
not used)
*/
-static int intel_master_probe(struct platform_device *pdev)
+static int intel_link_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *aux_dev_id)
+
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct sdw_intel_link_dev *ldev =
auxiliary_dev_to_sdw_intel_link_dev(auxdev);
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
struct sdw_bus *bus;
@@ -1344,14 +1347,14 @@ static int intel_master_probe(struct platform_device
*pdev)
cdns = >cdns;
bus = >bus;
- sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(dev);
+ sdw->instance = auxdev->id;
+ sdw->link_res = >link_res;
cdns->dev = dev;
cdns->registers = sdw->link_res->registers;
cdns->instance = sdw->instance;
cdns->msg_count = 0;
- bus->link_id = pdev->id;
+ bus->link_id = auxdev->id;
sdw_cdns_probe(cdns);
@@ -1384,10 +1387,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
}
-int intel_master_startup(struct platform_device *pdev)
+int intel_link_startup(struct auxiliary_device *auxdev)
{
struct sdw_cdns_stream_config config;
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1524,9 +1527,9 @@ int intel_master_startup(struct platform_device *pdev)
return ret;
}
-static int intel_master_remove(struct platform_device *pdev)
+static void intel_link_remove(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1542,19 +1545,17 @@ static int intel_master_remove(struct platform_device
*pdev)
snd_soc_unregister_component(dev);
}
sdw_bus_master_delete(bus);
-
- return 0;
}
-int intel_master_process_wakeen_event(struct platform_device *pdev)
+int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_intel *sdw;
struct sdw_bus *bus;
void __iomem *shim;
u16 wake_sts;
- sdw = platform_get_drvdata(pdev);
+ sdw = dev_get_drvdata(dev);
bus = >cdns.bus;
if (bus->prop.hw_disabled) {
@@
[PATCH] soundwire: dmi-quirks: remove duplicate initialization
From: Pierre-Louis Bossart
cppcheck warning:
drivers/soundwire/dmi-quirks.c:85:12: style: Redundant initialization
for 'map'. The initialized value is overwritten before it is
read. [redundantInitialization]
for (map = dmi_id->driver_data; map->adr; map++) {
^
drivers/soundwire/dmi-quirks.c:83:25: note: map is initialized
struct adr_remap *map = dmi_id->driver_data;
^
drivers/soundwire/dmi-quirks.c:85:12: note: map is overwritten
for (map = dmi_id->driver_data; map->adr; map++) {
^
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Daniel Baluta
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/dmi-quirks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c
index 82061c1d9835..5db0a2443a1d 100644
--- a/drivers/soundwire/dmi-quirks.c
+++ b/drivers/soundwire/dmi-quirks.c
@@ -80,7 +80,7 @@ u64 sdw_dmi_override_adr(struct sdw_bus *bus, u64 addr)
/* check if any address remap quirk applies */
dmi_id = dmi_first_match(adr_remap_quirk_table);
if (dmi_id) {
- struct adr_remap *map = dmi_id->driver_data;
+ struct adr_remap *map;
for (map = dmi_id->driver_data; map->adr; map++) {
if (map->adr == addr) {
--
2.17.1
[PATCH v2] soundwire: intel_init: test link->cdns
intel_link_probe() could return error and dev_get_drvdata() will return
null in such case. So we have to test link->cdns after
link->cdns = dev_get_drvdata(>auxdev.dev);
Otherwise, we will meet the "kernel NULL pointer dereference" error.
Signed-off-by: Bard Liao
Reviewed-by: Ranjani Sridharan
Reviewed-by: Rander Wang
Reviewed-by: Pierre-Louis Bossart
---
v2:
- Rebase to latest code base.
---
drivers/soundwire/intel_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 05b726cdfebc..30ce95ec2d70 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -178,6 +178,15 @@ static struct sdw_intel_ctx
link->pdev = pdev;
link->cdns = platform_get_drvdata(pdev);
+ if (!link->cdns) {
+ dev_err(>dev, "failed to get link->cdns\n");
+ /*
+* 1 will be subtracted from i in the err label, but we
need to call
+* intel_link_dev_unregister for this ldev, so plus 1
now
+*/
+ i++;
+ goto err;
+ }
list_add_tail(>list, >link_list);
bus = >cdns->bus;
/* Calculate number of slaves */
--
2.17.1
[PATCH v2] soundwire: intel: move to auxiliary bus
From: Pierre-Louis Bossart
Now that the auxiliary_bus exists, there's no reason to use platform
devices as children of a PCI device any longer.
This patch refactors the code by extending a basic auxiliary device
with Intel link-specific structures that need to be passed between
controller and link levels. This refactoring is much cleaner with no
need for cross-pointers between device and link structures.
Note that the auxiliary bus API has separate init and add steps, which
requires more attention in the error unwinding paths. The main loop
needs to deal with kfree() and auxiliary_device_uninit() for the
current iteration before jumping to the common label which releases
everything allocated in prior iterations.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Ranjani Sridharan
Signed-off-by: Bard Liao
---
v2:
- add link_dev_register for all kzalloc, device_init, and device_add.
---
drivers/soundwire/Kconfig | 1 +
drivers/soundwire/intel.c | 56 ---
drivers/soundwire/intel.h | 14 +-
drivers/soundwire/intel_init.c | 228 +++-
include/linux/soundwire/sdw_intel.h | 6 +-
5 files changed, 200 insertions(+), 105 deletions(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 016e74230bb7..2b7795233282 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
tristate "Intel SoundWire Master driver"
select SOUNDWIRE_CADENCE
select SOUNDWIRE_GENERIC_ALLOCATION
+ select AUXILIARY_BUS
depends on ACPI && SND_SOC
help
SoundWire Intel Master driver.
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index fd95f94630b1..c11e3d8cd308 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1327,11 +1327,14 @@ static int intel_init(struct sdw_intel *sdw)
}
/*
- * probe and init
+ * probe and init (aux_dev_id argument is required by function prototype but
not used)
*/
-static int intel_master_probe(struct platform_device *pdev)
+static int intel_link_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *aux_dev_id)
+
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct sdw_intel_link_dev *ldev =
auxiliary_dev_to_sdw_intel_link_dev(auxdev);
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
struct sdw_bus *bus;
@@ -1344,14 +1347,14 @@ static int intel_master_probe(struct platform_device
*pdev)
cdns = >cdns;
bus = >bus;
- sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(dev);
+ sdw->instance = auxdev->id;
+ sdw->link_res = >link_res;
cdns->dev = dev;
cdns->registers = sdw->link_res->registers;
cdns->instance = sdw->instance;
cdns->msg_count = 0;
- bus->link_id = pdev->id;
+ bus->link_id = auxdev->id;
sdw_cdns_probe(cdns);
@@ -1384,10 +1387,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
}
-int intel_master_startup(struct platform_device *pdev)
+int intel_link_startup(struct auxiliary_device *auxdev)
{
struct sdw_cdns_stream_config config;
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1524,9 +1527,9 @@ int intel_master_startup(struct platform_device *pdev)
return ret;
}
-static int intel_master_remove(struct platform_device *pdev)
+static void intel_link_remove(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1542,19 +1545,17 @@ static int intel_master_remove(struct platform_device
*pdev)
snd_soc_unregister_component(dev);
}
sdw_bus_master_delete(bus);
-
- return 0;
}
-int intel_master_process_wakeen_event(struct platform_device *pdev)
+int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_intel *sdw;
struct sdw_bus *bus;
void __iomem *shim;
u16 wake_sts;
- sdw = platform_get_drvdata(pdev);
+ sdw = dev_get_drvdata(dev);
bus = >cdns.bus;
if (bus->prop.hw_disabled) {
@@ -1976,17 +1977,22 @@ static const struct dev_pm_ops intel_pm = {
SET_RUNTIME_PM_OPS(intel_
[PATCH 2/2] soundwire: bus: handle errors in clock stop/start sequences
From: Pierre-Louis Bossart
If a device lost sync and can no longer ACK a command, it may not be
able to enter a lower-power state but it will still be able to resync
when the clock restarts. In those cases, we want to continue with the
clock stop sequence.
This patch modifies the behavior when -ENODATA is received, with the
error level demoted to a dev_dbg() since it's a recoverable issue.
For consistency the log messages are also modified to be unique and
self-explanatory, and missing logs are also added on clock stop exit.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 70 +++--
1 file changed, 32 insertions(+), 38 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 9bd83c91a873..ea54a1f02252 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -848,8 +848,9 @@ static int sdw_slave_clk_stop_callback(struct sdw_slave
*slave,
if (slave->ops && slave->ops->clk_stop) {
ret = slave->ops->clk_stop(slave, mode, type);
if (ret < 0) {
- dev_err(>dev,
- "Clk Stop type =%d failed: %d\n", type, ret);
+ sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
+ "Clk Stop mode %d type =%d failed:
%d\n",
+ mode, type, ret);
return ret;
}
}
@@ -878,7 +879,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
} else {
ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
if (ret < 0) {
- dev_err(>dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
+ sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
+ "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
return ret;
}
val = ret;
@@ -888,8 +890,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
if (ret < 0)
- dev_err(>dev,
- "Clock Stop prepare failed for slave: %d", ret);
+ sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
+ "SDW_SCP_SYSTEMCTRL write ignored:%d\n",
ret);
return ret;
}
@@ -907,7 +909,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
}
val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
- dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d",
+ dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d\n",
dev_num);
return 0;
}
@@ -916,7 +918,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
retry--;
} while (retry);
- dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d",
+ dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d\n",
dev_num);
return -ETIMEDOUT;
@@ -956,19 +958,18 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
slave_mode = sdw_get_clk_stop_mode(slave);
slave->curr_clk_stop_mode = slave_mode;
- ret = sdw_slave_clk_stop_callback(slave, slave_mode,
- SDW_CLK_PRE_PREPARE);
+ ret = sdw_slave_clk_stop_callback(slave, slave_mode,
SDW_CLK_PRE_PREPARE);
if (ret < 0) {
- dev_err(>dev,
- "pre-prepare failed:%d", ret);
+ sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
+ "clock stop pre prepare cb
failed:%d\n", ret);
return ret;
}
ret = sdw_slave_clk_stop_prepare(slave,
slave_mode, true);
if (ret < 0) {
- dev_err(>dev,
- "pre-prepare failed:%d", ret);
+ sdw_dev_dbg_or_err(>dev, ret != -ENODATA,
+ "clock stop prepare failed:%d\n",
ret);
return ret;
}
@@ -999,13 +1000,11 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
slave_mode = slave->curr_clk_stop_mode;
if (slave_mode == SDW_CLK_STOP_MODE1) {
-
[PATCH 1/2] soundwire: add macro to selectively change error levels
From: Pierre-Louis Bossart
We sometimes discard -ENODATA when reporting errors and lose all
traces of issues in the console log, add a macro to add use dev_dbg()
in such cases.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.h | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 40354469860a..8370216f95d4 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -227,4 +227,12 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
void sdw_clear_slave_status(struct sdw_bus *bus, u32 request);
int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size);
+#define sdw_dev_dbg_or_err(dev, is_err, fmt, ...) \
+ do {\
+ if (is_err) \
+ dev_err(dev, fmt, __VA_ARGS__); \
+ else\
+ dev_dbg(dev, fmt, __VA_ARGS__); \
+ } while (0)
+
#endif /* __SDW_BUS_H */
--
2.17.1
[PATCH 0/2] soundwire: bus: handle errors in clock stop/start sequences
If a device lost sync and can no longer ACK a command, it may not be able to enter a lower-power state but it will still be able to resync when the clock restarts. In those cases, we want to continue with the clock stop sequence. This patch modifies the behavior when -ENODATA is received, with the error level demoted to a dev_dbg() since it's a recoverable issue. For consistency the log messages are also modified to be unique and self-explanatory, and missing logs are also added on clock stop exit. Pierre-Louis Bossart (2): soundwire: add macro to selectively change error levels soundwire: bus: handle errors in clock stop/start sequences drivers/soundwire/bus.c | 70 +++-- drivers/soundwire/bus.h | 8 + 2 files changed, 40 insertions(+), 38 deletions(-) -- 2.17.1
[PATCH] soundwire: intel_init: test link->cdns
intel_link_probe() could return error and dev_get_drvdata() will return
null in such case. So we have to test link->cdns after
link->cdns = dev_get_drvdata(>auxdev.dev);
Otherwise, we will meet the "kernel NULL pointer dereference" error.
Signed-off-by: Bard Liao
Reviewed-by: Ranjani Sridharan
Reviewed-by: Rander Wang
Reviewed-by: Pierre-Louis Bossart
---
drivers/soundwire/intel_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 5b32a2ffd376..5ef5bd0defab 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -250,6 +250,15 @@ static struct sdw_intel_ctx
link = >link_res;
link->cdns = dev_get_drvdata(>auxdev.dev);
+ if (!link->cdns) {
+ dev_err(>dev, "failed to get link->cdns\n");
+ /*
+* 1 will be subtracted from i in the err label, but we
need to call
+* intel_link_dev_unregister for this ldev, so plus 1
now
+*/
+ i++;
+ goto err;
+ }
list_add_tail(>list, >link_list);
bus = >cdns->bus;
/* Calculate number of slaves */
--
2.17.1
[PATCH] soundwire: stream: fix memory leak in stream config error path
From: Rander Wang
When stream config is failed, master runtime will release all
slave runtime in the slave_rt_list, but slave runtime is not
added to the list at this time. This patch frees slave runtime
in the config error path to fix the memory leak.
Fixes: bbe7379d8040a ("soundwire: Add support for SoundWire stream management")
Signed-off-by: Rander Wang
Reviewed-by: Keyon Jie
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 9c064b672745..1eaedaaba094 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1375,8 +1375,16 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
}
ret = sdw_config_stream(>dev, stream, stream_config, true);
- if (ret)
+ if (ret) {
+ /*
+* sdw_release_master_stream will release s_rt in slave_rt_list
in
+* stream_error case, but s_rt is only added to slave_rt_list
+* when sdw_config_stream is successful, so free s_rt explicitly
+* when sdw_config_stream is failed.
+*/
+ kfree(s_rt);
goto stream_error;
+ }
list_add_tail(_rt->m_rt_node, _rt->slave_rt_list);
--
2.17.1
[RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations
From: Pierre-Louis Bossart
There is no need to assign a pointer to NULL if it's only used in a
loop and assigned within that loop.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 6a682179cd05..9c064b672745 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1449,7 +1449,7 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct
sdw_slave *slave,
static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
{
struct sdw_master_runtime *m_rt;
- struct sdw_bus *bus = NULL;
+ struct sdw_bus *bus;
/* Iterate for all Master(s) in Master list */
list_for_each_entry(m_rt, >master_list, stream_node) {
@@ -1471,7 +1471,7 @@ static void sdw_acquire_bus_lock(struct
sdw_stream_runtime *stream)
static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
{
struct sdw_master_runtime *m_rt;
- struct sdw_bus *bus = NULL;
+ struct sdw_bus *bus;
/* Iterate for all Master(s) in Master list */
list_for_each_entry_reverse(m_rt, >master_list, stream_node) {
--
2.17.1
[RESEND PATCH 10/11] soundwire: stream: remove useless initialization
From: Pierre-Louis Bossart
Cppcheck complains about possible null pointer dereferences, but it's
more like there are unnecessary initializations before walking
through a list.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 4915676c4ac2..6a682179cd05 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -261,7 +261,7 @@ static int sdw_program_master_port_params(struct sdw_bus
*bus,
*/
static int sdw_program_port_params(struct sdw_master_runtime *m_rt)
{
- struct sdw_slave_runtime *s_rt = NULL;
+ struct sdw_slave_runtime *s_rt;
struct sdw_bus *bus = m_rt->bus;
struct sdw_port_runtime *p_rt;
int ret = 0;
@@ -1470,7 +1470,7 @@ static void sdw_acquire_bus_lock(struct
sdw_stream_runtime *stream)
*/
static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
{
- struct sdw_master_runtime *m_rt = NULL;
+ struct sdw_master_runtime *m_rt;
struct sdw_bus *bus = NULL;
/* Iterate for all Master(s) in Master list */
--
2.17.1
[RESEND PATCH 08/11] soundwire: intel: remove useless readl
From: Pierre-Louis Bossart
Cppcheck complains:
drivers/soundwire/intel.c:564:15: style: Variable 'link_control' is
assigned a value that is never used. [unreadVariable]
link_control = intel_readl(shim, SDW_SHIM_LCTL);
This looks like a leftover from a previous version, remove.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index e2e95115832a..fd95f94630b1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -561,8 +561,6 @@ static int intel_link_power_down(struct sdw_intel *sdw)
ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control,
cpa_mask);
}
- link_control = intel_readl(shim, SDW_SHIM_LCTL);
-
mutex_unlock(sdw->link_res->shim_lock);
if (ret < 0) {
--
2.17.1
[RESEND PATCH 07/11] soundwire: generic_bandwidth_allocation: remove useless init
From: Pierre-Louis Bossart
Cppcheck complains about two possible null pointer dereferences, but
it's more like there are unnecessary initializations before walking
through a list.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/generic_bandwidth_allocation.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c
b/drivers/soundwire/generic_bandwidth_allocation.c
index a9abb9722fde..805f5b6ebe86 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -143,7 +143,7 @@ static void sdw_compute_master_ports(struct
sdw_master_runtime *m_rt,
static void _sdw_compute_port_params(struct sdw_bus *bus,
struct sdw_group_params *params, int count)
{
- struct sdw_master_runtime *m_rt = NULL;
+ struct sdw_master_runtime *m_rt;
int hstop = bus->params.col - 1;
int block_offset, port_bo, i;
@@ -169,7 +169,7 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
struct sdw_group_params *params,
int *rates, int count)
{
- struct sdw_master_runtime *m_rt = NULL;
+ struct sdw_master_runtime *m_rt;
int sel_col = bus->params.col;
unsigned int rate, bps, ch;
int i, column_needed = 0;
--
2.17.1
[RESEND PATCH 09/11] soundwire: qcom: check of_property_read status
From: Pierre-Louis Bossart
Cppcheck complains:
drivers/soundwire/qcom.c:773:6: style: Variable 'ret' is assigned a
value that is never used. [unreadVariable]
ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
^
The return value is checked for all other cases, not sure why it was
missed here.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/qcom.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 9cce09cba068..277f711e374d 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -772,6 +772,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl
*ctrl)
ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
bp_mode, nports);
+ if (ret)
+ return ret;
+
for (i = 0; i < nports; i++) {
ctrl->pconfig[i].si = si[i];
ctrl->pconfig[i].off1 = off1[i];
--
2.17.1
[RESEND PATCH 06/11] soundwire: bus: remove useless initialization
From: Pierre-Louis Bossart
Cppcheck complains about a possible null pointer dereference, but it's
more like there is an unnecessary initialization before walking
through a list.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index a38b017f7a54..1a9e307e6a4c 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -593,7 +593,7 @@ EXPORT_SYMBOL(sdw_write);
/* called with bus_lock held */
static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
{
- struct sdw_slave *slave = NULL;
+ struct sdw_slave *slave;
list_for_each_entry(slave, >slaves, node) {
if (slave->dev_num == i)
--
2.17.1
[RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values
From: Pierre-Louis Bossart
We use different styles to check the return values of IO related
routines. The majority of the cases use 'if (ret < 0)', align the
remaining cases for consistency.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 1c01cc192cbd..d39e5baa3e64 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -44,13 +44,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
}
ret = sdw_get_id(bus);
- if (ret) {
+ if (ret < 0) {
dev_err(parent, "Failed to get bus id\n");
return ret;
}
ret = sdw_master_device_add(bus, parent, fwnode);
- if (ret) {
+ if (ret < 0) {
dev_err(parent, "Failed to add master device at link %d\n",
bus->link_id);
return ret;
@@ -121,7 +121,7 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
else
ret = -ENOTSUPP; /* No ACPI/DT so error out */
- if (ret) {
+ if (ret < 0) {
dev_err(bus->dev, "Finding slaves failed:%d\n", ret);
return ret;
}
@@ -422,7 +422,7 @@ sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
ret = sdw_fill_msg(, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_READ, );
- if (ret)
+ if (ret < 0)
return ret;
ret = sdw_transfer(bus, );
@@ -440,7 +440,7 @@ sdw_bwrite_no_pm(struct sdw_bus *bus, u16 dev_num, u32
addr, u8 value)
ret = sdw_fill_msg(, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_WRITE, );
- if (ret)
+ if (ret < 0)
return ret;
return sdw_transfer(bus, );
@@ -454,7 +454,7 @@ int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr)
ret = sdw_fill_msg(, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_READ, );
- if (ret)
+ if (ret < 0)
return ret;
ret = sdw_transfer_unlocked(bus, );
@@ -472,7 +472,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
ret = sdw_fill_msg(, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_WRITE, );
- if (ret)
+ if (ret < 0)
return ret;
return sdw_transfer_unlocked(bus, );
@@ -749,7 +749,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
* dev_num
*/
ret = sdw_assign_device_num(slave);
- if (ret) {
+ if (ret < 0) {
dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
@@ -886,7 +886,7 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
- if (ret != 0)
+ if (ret < 0)
dev_err(>dev,
"Clock Stop prepare failed for slave: %d", ret);
@@ -1748,7 +1748,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
if (status[0] == SDW_SLAVE_ATTACHED) {
dev_dbg(bus->dev, "Slave attached, programming device
number\n");
ret = sdw_program_device_num(bus);
- if (ret)
+ if (ret < 0)
dev_err(bus->dev, "Slave attach failed: %d\n", ret);
/*
* programming a device number will have side effects,
@@ -1782,7 +1782,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
case SDW_SLAVE_ALERT:
ret = sdw_handle_slave_alerts(slave);
- if (ret)
+ if (ret < 0)
dev_err(>dev,
"Slave %d alert handling failed: %d\n",
i, ret);
@@ -1801,7 +1801,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
attached_initializing = true;
ret = sdw_initialize_slave(slave);
- if (ret)
+ if (ret < 0)
dev_err(>dev,
"Slave %d initialization failed: %d\n",
i, ret);
@@ -1815,7 +1815,7 @@ int sdw_handle_slave_status(struc
[RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access
From: Pierre-Louis Bossart
We have multiple cases where we read/write SCP_INT registers, but the
same error message in all cases. Add a distinct error message for each
case to help debug.
Reported-by: Guennadi Liakhovetski
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 8b6d8fe934ae..a38b017f7a54 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1636,7 +1636,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(>dev,
- "SDW_SCP_INT1 read failed:%d\n", ret);
+ "SDW_SCP_INT1 recheck read failed:%d\n", ret);
goto io_err;
}
_buf = ret;
@@ -1644,7 +1644,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, _buf2);
if (ret < 0) {
dev_err(>dev,
- "SDW_SCP_INT2/3 read failed:%d\n", ret);
+ "SDW_SCP_INT2/3 recheck read failed:%d\n", ret);
goto io_err;
}
@@ -1652,7 +1652,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_read_no_pm(slave, SDW_DP0_INT);
if (ret < 0) {
dev_err(>dev,
- "SDW_DP0_INT read failed:%d\n", ret);
+ "SDW_DP0_INT recheck read failed:%d\n",
ret);
goto io_err;
}
sdca_cascade = ret & SDW_DP0_SDCA_CASCADE;
--
2.17.1
[RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg()
From: Pierre-Louis Bossart
There is no real reason to provide this information except for debug
sessions, hence dev_dbg() is a better fit.
Reported-by: Guennadi Liakhovetski
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d39e5baa3e64..8b6d8fe934ae 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -906,8 +906,8 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
}
val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
- dev_info(bus->dev, "clock stop prep/de-prep done
slave:%d",
-dev_num);
+ dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d",
+ dev_num);
return 0;
}
--
2.17.1
[RESEND PATCH 02/11] soundwire: bus: test read status
From: Pierre-Louis Bossart
In the existing code we may read a negative error value but still mask
it and write it back.
Make sure all reads are tested and errors not propagated further.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 04eb879de145..1c01cc192cbd 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -875,8 +875,12 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
if (wake_en)
val |= SDW_SCP_SYSTEMCTRL_WAKE_UP_EN;
} else {
- val = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
-
+ ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
+ if (ret < 0) {
+ dev_err(>dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
+ return ret;
+ }
+ val = ret;
val &= ~(SDW_SCP_SYSTEMCTRL_CLK_STP_PREP);
}
@@ -895,8 +899,12 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
int val;
do {
- val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT) &
- SDW_SCP_STAT_CLK_STP_NF;
+ val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT);
+ if (val < 0) {
+ dev_err(bus->dev, "SDW_SCP_STAT bread failed:%d\n",
val);
+ return val;
+ }
+ val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
dev_info(bus->dev, "clock stop prep/de-prep done
slave:%d",
dev_num);
--
2.17.1
[RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages
From: Pierre-Louis Bossart
None of the existing codec drivers set the sdw_driver.name, but
instead set sdw_driver.driver.name.
This leads to error messages such as
[ 23.935355] rt700 sdw:2:25d:700:0: Probe of (null) failed: -19
We could remove this sdw_driver.name if it doesn't have any
purpose. This patch only suggests using the proper indirection.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/bus_type.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 575b9bad99d5..893296f3fe39 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -82,6 +82,7 @@ static int sdw_drv_probe(struct device *dev)
struct sdw_slave *slave = dev_to_sdw_dev(dev);
struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
const struct sdw_device_id *id;
+ const char *name;
int ret;
/*
@@ -108,7 +109,10 @@ static int sdw_drv_probe(struct device *dev)
ret = drv->probe(slave, id);
if (ret) {
- dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
+ name = drv->name;
+ if (!name)
+ name = drv->driver.name;
+ dev_err(dev, "Probe of %s failed: %d\n", name, ret);
dev_pm_domain_detach(dev, false);
return ret;
}
@@ -174,11 +178,16 @@ static void sdw_drv_shutdown(struct device *dev)
*/
int __sdw_register_driver(struct sdw_driver *drv, struct module *owner)
{
+ const char *name;
+
drv->driver.bus = _bus_type;
if (!drv->probe) {
- pr_err("driver %s didn't provide SDW probe routine\n",
- drv->name);
+ name = drv->name;
+ if (!name)
+ name = drv->driver.name;
+
+ pr_err("driver %s didn't provide SDW probe routine\n", name);
return -EINVAL;
}
--
2.17.1
[RESEND PATCH 00/11] soundwire: some cleanup patches
To make soundwire driver more decent and less Cppcheck complaint. Pierre-Louis Bossart (11): soundwire: bus: use correct driver name in error messages soundwire: bus: test read status soundwire: bus: use consistent tests for return values soundwire: bus: demote clock stop prepare log to dev_dbg() soundwire: bus: uniquify dev_err() for SCP_INT access soundwire: bus: remove useless initialization soundwire: generic_bandwidth_allocation: remove useless init soundwire: intel: remove useless readl soundwire: qcom: check of_property_read status soundwire: stream: remove useless initialization soundwire: stream: remove useless bus initializations drivers/soundwire/bus.c | 54 +++ drivers/soundwire/bus_type.c | 15 -- .../soundwire/generic_bandwidth_allocation.c | 4 +- drivers/soundwire/intel.c | 2 - drivers/soundwire/qcom.c | 3 ++ drivers/soundwire/stream.c| 8 +-- 6 files changed, 52 insertions(+), 34 deletions(-) -- 2.17.1
[PATCH] soundwire: add slave device to linked list after device_register()
From: Pierre-Louis Bossart
We currently add the slave device to a linked list before
device_register(), and then remove it if device_register() fails.
It's not clear why this sequence was necessary, this patch moves the
linked list management to after the device_register().
Suggested-by: Keyon Jie
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/slave.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 112b21967c7a..0c92db2e1ddc 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -65,9 +65,6 @@ int sdw_slave_add(struct sdw_bus *bus,
for (i = 0; i < SDW_MAX_PORTS; i++)
init_completion(>port_ready[i]);
- mutex_lock(>bus_lock);
- list_add_tail(>node, >slaves);
- mutex_unlock(>bus_lock);
ret = device_register(>dev);
if (ret) {
@@ -77,13 +74,15 @@ int sdw_slave_add(struct sdw_bus *bus,
* On err, don't free but drop ref as this will be freed
* when release method is invoked.
*/
- mutex_lock(>bus_lock);
- list_del(>node);
- mutex_unlock(>bus_lock);
put_device(>dev);
return ret;
}
+
+ mutex_lock(>bus_lock);
+ list_add_tail(>node, >slaves);
+ mutex_unlock(>bus_lock);
+
sdw_slave_debugfs_init(slave);
return ret;
--
2.17.1
[PATCH] soundwire: cadence: only prepare attached devices on clock stop
From: Pierre-Louis Bossart
We sometimes see COMMAND_IGNORED responses during the clock stop
sequence. It turns out we already have information if devices are
present on a link, so we should only prepare those when they
are attached.
In addition, even when COMMAND_IGNORED are received, we should still
proceed with the clock stop. The device will not be prepared but
that's not a problem.
The only case where the clock stop will fail is if the Cadence IP
reports an error (including a timeout), or if the devices throw a
COMMAND_FAILED response.
BugLink: https://github.com/thesofproject/linux/issues/2621
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d05442e646a3..57c59a33ce61 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1450,10 +1450,12 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool
block_wake)
}
/* Prepare slaves for clock stop */
- ret = sdw_bus_prep_clk_stop(>bus);
- if (ret < 0) {
- dev_err(cdns->dev, "prepare clock stop failed %d", ret);
- return ret;
+ if (slave_present) {
+ ret = sdw_bus_prep_clk_stop(>bus);
+ if (ret < 0 && ret != -ENODATA) {
+ dev_err(cdns->dev, "prepare clock stop failed %d\n",
ret);
+ return ret;
+ }
}
/*
--
2.17.1
[PATCH 5/5] soundwire: qcom: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6d22df01f354..9cce09cba068 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -652,7 +652,7 @@ static int qcom_swrm_startup(struct snd_pcm_substream
*substream,
ret = snd_soc_dai_set_sdw_stream(codec_dai, sruntime,
substream->stream);
if (ret < 0 && ret != -ENOTSUPP) {
- dev_err(dai->dev, "Failed to set sdw stream on %s",
+ dev_err(dai->dev, "Failed to set sdw stream on %s\n",
codec_dai->name);
sdw_release_stream(sruntime);
return ret;
--
2.17.1
[PATCH 4/5] soundwire: stream: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 1099b5d1262b..4915676c4ac2 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1513,7 +1513,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime
*stream,
if (bus->compute_params) {
ret = bus->compute_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute params failed: %d",
+ dev_err(bus->dev, "Compute params failed: %d\n",
ret);
return ret;
}
@@ -1791,7 +1791,7 @@ static int _sdw_deprepare_stream(struct
sdw_stream_runtime *stream)
if (bus->compute_params) {
ret = bus->compute_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute params failed: %d",
+ dev_err(bus->dev, "Compute params failed: %d\n",
ret);
return ret;
}
@@ -1855,7 +1855,7 @@ static int set_stream(struct snd_pcm_substream *substream,
for_each_rtd_dais(rtd, i, dai) {
ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream,
substream->stream);
if (ret < 0) {
- dev_err(rtd->dev, "failed to set stream pointer on dai
%s", dai->name);
+ dev_err(rtd->dev, "failed to set stream pointer on dai
%s\n", dai->name);
break;
}
}
@@ -1888,7 +1888,7 @@ int sdw_startup_stream(void *sdw_substream)
sdw_stream = sdw_alloc_stream(name);
if (!sdw_stream) {
- dev_err(rtd->dev, "alloc stream failed for substream DAI %s",
substream->name);
+ dev_err(rtd->dev, "alloc stream failed for substream DAI %s\n",
substream->name);
ret = -ENOMEM;
goto error;
}
@@ -1927,7 +1927,7 @@ void sdw_shutdown_stream(void *sdw_substream)
sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
if (IS_ERR(sdw_stream)) {
- dev_err(rtd->dev, "no stream found for DAI %s", dai->name);
+ dev_err(rtd->dev, "no stream found for DAI %s\n", dai->name);
return;
}
--
2.17.1
[PATCH 3/5] soundwire: cadence: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d05442e646a3..1b50cf7abe66 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1462,7 +1462,7 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool
block_wake)
*/
ret = sdw_bus_clk_stop(>bus);
if (ret < 0 && slave_present && ret != -ENODATA) {
- dev_err(cdns->dev, "bus clock stop failed %d", ret);
+ dev_err(cdns->dev, "bus clock stop failed %d\n", ret);
return ret;
}
--
2.17.1
[PATCH 0/5] soundwire: add missing \n in dev_err()
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps adding new ones... Pierre-Louis Bossart (5): soundwire: intel: add missing \n in dev_err() soundwire: bandwidth_allocation: add missing \n in dev_err() soundwire: cadence: add missing \n in dev_err() soundwire: stream: add missing \n in dev_err() soundwire: qcom: add missing \n in dev_err() drivers/soundwire/cadence_master.c | 2 +- .../soundwire/generic_bandwidth_allocation.c | 4 ++-- drivers/soundwire/intel.c | 18 +- drivers/soundwire/qcom.c | 2 +- drivers/soundwire/stream.c | 10 +- 5 files changed, 18 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH 2/5] soundwire: bandwidth_allocation: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/generic_bandwidth_allocation.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c
b/drivers/soundwire/generic_bandwidth_allocation.c
index 0bdef38c9a30..a9abb9722fde 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -406,14 +406,14 @@ int sdw_compute_params(struct sdw_bus *bus)
/* Computes clock frequency, frame shape and frame frequency */
ret = sdw_compute_bus_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute bus params failed: %d", ret);
+ dev_err(bus->dev, "Compute bus params failed: %d\n", ret);
return ret;
}
/* Compute transport and port params */
ret = sdw_compute_port_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute transport params failed: %d", ret);
+ dev_err(bus->dev, "Compute transport params failed: %d\n", ret);
return ret;
}
--
2.17.1
[PATCH 1/5] soundwire: intel: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d2254ee2fee2..e2e95115832a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -997,7 +997,7 @@ static int intel_prepare(struct snd_pcm_substream
*substream,
dma = snd_soc_dai_get_dma_data(dai, substream);
if (!dma) {
- dev_err(dai->dev, "failed to get dma data in %s",
+ dev_err(dai->dev, "failed to get dma data in %s\n",
__func__);
return -EIO;
}
@@ -1061,7 +1061,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct
snd_soc_dai *dai)
ret = intel_free_stream(sdw, substream, dai, sdw->instance);
if (ret < 0) {
- dev_err(dai->dev, "intel_free_stream: failed %d", ret);
+ dev_err(dai->dev, "intel_free_stream: failed %d\n", ret);
return ret;
}
@@ -1634,7 +1634,7 @@ static int __maybe_unused intel_suspend(struct device
*dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1669,7 +1669,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1693,7 +1693,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1742,7 +1742,7 @@ static int __maybe_unused intel_resume(struct device *dev)
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1826,7 +1826,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1871,7 +1871,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1949,7 +1949,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
--
2.17.1
[PATCH] soundwire: intel: move to auxiliary bus
From: Pierre-Louis Bossart
Now that the auxiliary_bus exists, there's no reason to use platform
devices as children of a PCI device any longer.
This patch refactors the code by extending a basic auxiliary device
with Intel link-specific structures that need to be passed between
controller and link levels. This refactoring is much cleaner with no
need for cross-pointers between device and link structures.
Note that the auxiliary bus API has separate init and add steps, which
requires more attention in the error unwinding paths. The main loop
needs to deal with kfree() and auxiliary_device_uninit() for the
current iteration before jumping to the common label which releases
everything allocated in prior iterations.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Ranjani Sridharan
Signed-off-by: Bard Liao
---
drivers/soundwire/Kconfig | 1 +
drivers/soundwire/intel.c | 52
drivers/soundwire/intel.h | 14 +-
drivers/soundwire/intel_init.c | 190 +++-
include/linux/soundwire/sdw_intel.h | 6 +-
5 files changed, 175 insertions(+), 88 deletions(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 016e74230bb7..2b7795233282 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
tristate "Intel SoundWire Master driver"
select SOUNDWIRE_CADENCE
select SOUNDWIRE_GENERIC_ALLOCATION
+ select AUXILIARY_BUS
depends on ACPI && SND_SOC
help
SoundWire Intel Master driver.
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d2254ee2fee2..039a101982c9 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1331,9 +1331,10 @@ static int intel_init(struct sdw_intel *sdw)
/*
* probe and init
*/
-static int intel_master_probe(struct platform_device *pdev)
+static int intel_link_probe(struct auxiliary_device *auxdev, const struct
auxiliary_device_id *id)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
+ struct sdw_intel_link_dev *ldev =
auxiliary_dev_to_sdw_intel_link_dev(auxdev);
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
struct sdw_bus *bus;
@@ -1346,14 +1347,14 @@ static int intel_master_probe(struct platform_device
*pdev)
cdns = >cdns;
bus = >bus;
- sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(dev);
+ sdw->instance = auxdev->id;
+ sdw->link_res = >link_res;
cdns->dev = dev;
cdns->registers = sdw->link_res->registers;
cdns->instance = sdw->instance;
cdns->msg_count = 0;
- bus->link_id = pdev->id;
+ bus->link_id = auxdev->id;
sdw_cdns_probe(cdns);
@@ -1386,10 +1387,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
}
-int intel_master_startup(struct platform_device *pdev)
+int intel_link_startup(struct auxiliary_device *auxdev)
{
struct sdw_cdns_stream_config config;
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1526,9 +1527,9 @@ int intel_master_startup(struct platform_device *pdev)
return ret;
}
-static int intel_master_remove(struct platform_device *pdev)
+static void intel_link_remove(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = >bus;
@@ -1544,19 +1545,17 @@ static int intel_master_remove(struct platform_device
*pdev)
snd_soc_unregister_component(dev);
}
sdw_bus_master_delete(bus);
-
- return 0;
}
-int intel_master_process_wakeen_event(struct platform_device *pdev)
+int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
{
- struct device *dev = >dev;
+ struct device *dev = >dev;
struct sdw_intel *sdw;
struct sdw_bus *bus;
void __iomem *shim;
u16 wake_sts;
- sdw = platform_get_drvdata(pdev);
+ sdw = dev_get_drvdata(dev);
bus = >cdns.bus;
if (bus->prop.hw_disabled) {
@@ -1978,17 +1977,22 @@ static const struct dev_pm_ops intel_pm = {
SET_RUNTIME_PM_OPS(intel_suspend_runtime, intel_resume_runtime, NULL)
};
-static struct platform_driver sdw_intel_drv = {
- .probe = intel_master_probe,
- .remove = intel_master_remove,
+static const struct auxiliary_device_id in
[PATCH v2 1/3] soundwire: add master quirks for bus clash and parity
Currently quirks are only allowed for Slave devices. This patch
describes the need for two quirks at the Master level.
a) bus clash
The SoundWire specification allows a Slave device to report a bus clash
with the in-band interrupt mechanism when it detects a conflict while
driving a bitSlot it owns. This can be a symptom of an electrical conflict
or a programming error, and it's vital to detect reliably.
Unfortunately, on some platforms, bus clashes are randomly reported by
Slave devices after a bus reset, with an interrupt status set even before
the bus clash interrupt is enabled. These initial spurious interrupts are
not relevant and should optionally be filtered out, while leaving the
interrupt mechanism enabled to detect 'true' issues.
This patch suggests the addition of a Master level quirk to discard such
interrupts. The quirk should in theory have been added at the Slave level,
but since the problem was detected with different generations of Slave
devices it's hard to point to a specific IP. The problem might also be
board-dependent and hence dealing with a Master quirk is simpler.
b) parity
Additional tests on a new platform with the Maxim 98373 amplifier
showed a rare case where the parity interrupt is also thrown on
startup, at the same time as bus clashes. This issue only seems to
happen infrequently and was only observed during suspend-resume stress
tests while audio is streaming. We could make the problem go away by
adding a Slave-level quirk, but there is no evidence that the issue is
actually a Slave problem: the parity is provided by the Master, which
could also set an invalid parity in corner cases.
BugLink: https://github.com/thesofproject/linux/issues/2578
BugLink: https://github.com/thesofproject/linux/issues/2533
Co-developed-by: Pierre-Louis Bossart
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
Reviewed-by: Guennadi Liakhovetski
---
include/linux/soundwire/sdw.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index d08039d65825..25f2a14e2e83 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -405,6 +405,7 @@ struct sdw_slave_prop {
* command
* @mclk_freq: clock reference passed to SoundWire Master, in Hz.
* @hw_disabled: if true, the Master is not functional, typically due to
pin-mux
+ * @quirks: bitmask identifying optional behavior beyond the scope of the MIPI
specification
*/
struct sdw_master_prop {
u32 revision;
@@ -421,8 +422,29 @@ struct sdw_master_prop {
u32 err_threshold;
u32 mclk_freq;
bool hw_disabled;
+ u64 quirks;
};
+/* Definitions for Master quirks */
+
+/*
+ * In a number of platforms bus clashes are reported after a hardware
+ * reset but without any explanations or evidence of a real problem.
+ * The following quirk will discard all initial bus clash interrupts
+ * but will leave the detection on should real bus clashes happen
+ */
+#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH BIT(0)
+
+/*
+ * Some Slave devices have known issues with incorrect parity errors
+ * reported after a hardware reset. However during integration unexplained
+ * parity errors can be reported by Slave devices, possibly due to electrical
+ * issues at the Master level.
+ * The following quirk will discard all initial parity errors but will leave
+ * the detection on should real parity errors happen.
+ */
+#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY BIT(1)
+
int sdw_master_read_prop(struct sdw_bus *bus);
int sdw_slave_read_prop(struct sdw_slave *slave);
--
2.17.1
[PATCH v2 2/3] soundwire: bus: handle master quirks for bus clash and parity
Add optional interrupt status read/clear if the master quirks are set.
In the case of the parity, the master quirk is only applied if the
Slave doesn't already have a parity-related quirk.
Co-developed-by: Pierre-Louis Bossart
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
Reviewed-by: Guennadi Liakhovetski
---
drivers/soundwire/bus.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 46885429928a..04eb879de145 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1253,6 +1253,7 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
static int sdw_initialize_slave(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = >prop;
+ int status;
int ret;
u8 val;
@@ -1260,6 +1261,44 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
if (ret < 0)
return ret;
+ if (slave->bus->prop.quirks & SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH) {
+ /* Clear bus clash interrupt before enabling interrupt mask */
+ status = sdw_read_no_pm(slave, SDW_SCP_INT1);
+ if (status < 0) {
+ dev_err(>dev,
+ "SDW_SCP_INT1 (BUS_CLASH) read failed:%d\n",
status);
+ return status;
+ }
+ if (status & SDW_SCP_INT1_BUS_CLASH) {
+ dev_warn(>dev, "Bus clash detected before INT
mask is enabled\n");
+ ret = sdw_write_no_pm(slave, SDW_SCP_INT1,
SDW_SCP_INT1_BUS_CLASH);
+ if (ret < 0) {
+ dev_err(>dev,
+ "SDW_SCP_INT1 (BUS_CLASH) write
failed:%d\n", ret);
+ return ret;
+ }
+ }
+ }
+ if ((slave->bus->prop.quirks & SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY)
&&
+ !(slave->prop.quirks & SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY)) {
+ /* Clear parity interrupt before enabling interrupt mask */
+ status = sdw_read_no_pm(slave, SDW_SCP_INT1);
+ if (status < 0) {
+ dev_err(>dev,
+ "SDW_SCP_INT1 (PARITY) read failed:%d\n",
status);
+ return status;
+ }
+ if (status & SDW_SCP_INT1_PARITY) {
+ dev_warn(>dev, "PARITY error detected before INT
mask is enabled\n");
+ ret = sdw_write_no_pm(slave, SDW_SCP_INT1,
SDW_SCP_INT1_PARITY);
+ if (ret < 0) {
+ dev_err(>dev,
+ "SDW_SCP_INT1 (PARITY) write
failed:%d\n", ret);
+ return ret;
+ }
+ }
+ }
+
/*
* Set SCP_INT1_MASK register, typically bus clash and
* implementation-defined interrupt mask. The Parity detection
--
2.17.1
[PATCH v2 3/3] soundwire: intel: add master quirks for bus clash and parity
Now that we have declarations and bus support, add quirks for Intel platforms. Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Reviewed-by: Guennadi Liakhovetski --- drivers/soundwire/intel.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a2d5cdaa9998..c1fdc85d0a74 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1286,6 +1286,9 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus) if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE) prop->hw_disabled = true; + prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH | + SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY; + return 0; } -- 2.17.1
[PATCH v2 0/3] soundwire: clear bus clash/parity interrupt before the mask is enabled
The SoundWire specification allows a Slave device to report a bus clash or parity error with the in-band interrupt mechanism. Unfortunately, on some platforms, these errors are randomly reported and don't seem to be valid. This series suggests the addition of a Master level quirk to discard such interrupts. The quirk should in theory have been added at the Slave level, but since the problem was detected with different generations of Slave devices it's hard to point to a specific IP. The problem might also be board-dependent and hence dealing with a Master quirk is simpler. v2: - Reorder the patches sequence. - Add comments about each quirk. - Use u64 quirks. Bard Liao (3): soundwire: add master quirks for bus clash and parity soundwire: bus: handle master quirks for bus clash and parity soundwire: intel: add master quirks for bus clash and parity drivers/soundwire/bus.c | 39 +++ drivers/soundwire/intel.c | 3 +++ include/linux/soundwire/sdw.h | 22 3 files changed, 64 insertions(+) -- 2.17.1
[PATCH v2 3/3] soundwire: Intel: add DMI quirk for Dell SKU 0A3E
From: Pierre-Louis Bossart
We've been handling ACPI issues on early versions of the product with
a local ACPI initrd override but now that we have the possibility of a
kernel quirk let's get rid of the initrd override. This helps make
sure that the kernel will support all versions of the BIOS, with or
without updates.
Co-developed-by: Bard Liao
Signed-off-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
---
drivers/soundwire/dmi-quirks.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c
index 249e476e49ea..82061c1d9835 100644
--- a/drivers/soundwire/dmi-quirks.c
+++ b/drivers/soundwire/dmi-quirks.c
@@ -32,6 +32,29 @@ static const struct adr_remap hp_spectre_360[] = {
{}
};
+/*
+ * The initial version of the Dell SKU 0A3E did not expose the devices
+ * on the correct links.
+ */
+static const struct adr_remap dell_sku_0A3E[] = {
+ /* rt715 on link0 */
+ {
+ 0x00020025d071100,
+ 0x00021025d071500
+ },
+ /* rt711 on link1 */
+ {
+ 0x000120025d130800,
+ 0x000120025d071100,
+ },
+ /* rt1308 on link2 */
+ {
+ 0x000220025d071500,
+ 0x000220025d130800
+ },
+ {}
+};
+
static const struct dmi_system_id adr_remap_quirk_table[] = {
{
.matches = {
@@ -40,6 +63,13 @@ static const struct dmi_system_id adr_remap_quirk_table[] = {
},
.driver_data = (void *)hp_spectre_360,
},
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "0A3E")
+ },
+ .driver_data = (void *)dell_sku_0A3E,
+ },
{}
};
--
2.17.1
[PATCH v2 2/3] soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible
From: Pierre-Louis Bossart
HP Spectre x360 Convertible devices expose invalid _ADR fields in the
DSDT, which prevents codec drivers from probing. A possible solution
is to override the DSDT, but that's just too painful for users.
This patch suggests a simple DMI-based quirk to remap the existing
invalid ADR information into valid ones.
BugLink: https://github.com/thesofproject/linux/issues/2700
Co-developed-by: Bard Liao
Signed-off-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
---
drivers/soundwire/Makefile | 2 +-
drivers/soundwire/bus.h| 2 ++
drivers/soundwire/dmi-quirks.c | 66 ++
drivers/soundwire/intel.c | 1 +
4 files changed, 70 insertions(+), 1 deletion(-)
create mode 100644 drivers/soundwire/dmi-quirks.c
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
index bf1e250d50dd..986776787b9e 100644
--- a/drivers/soundwire/Makefile
+++ b/drivers/soundwire/Makefile
@@ -20,7 +20,7 @@ soundwire-cadence-y := cadence_master.o
obj-$(CONFIG_SOUNDWIRE_CADENCE) += soundwire-cadence.o
#Intel driver
-soundwire-intel-y := intel.o intel_init.o
+soundwire-intel-y := intel.o intel_init.o dmi-quirks.o
obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel.o
#Qualcomm driver
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 2e049d39c6e5..40354469860a 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -7,6 +7,8 @@
#define DEFAULT_BANK_SWITCH_TIMEOUT 3000
#define DEFAULT_PROBE_TIMEOUT 2000
+u64 sdw_dmi_override_adr(struct sdw_bus *bus, u64 addr);
+
#if IS_ENABLED(CONFIG_ACPI)
int sdw_acpi_find_slaves(struct sdw_bus *bus);
#else
diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c
new file mode 100644
index ..249e476e49ea
--- /dev/null
+++ b/drivers/soundwire/dmi-quirks.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2021 Intel Corporation.
+
+/*
+ * Soundwire DMI quirks
+ */
+
+#include
+#include
+#include
+#include "bus.h"
+
+struct adr_remap {
+ u64 adr;
+ u64 remapped_adr;
+};
+
+/*
+ * HP Spectre 360 Convertible devices do not expose the correct _ADR
+ * in the DSDT.
+ * Remap the bad _ADR values to the ones reported by hardware
+ */
+static const struct adr_remap hp_spectre_360[] = {
+ {
+ 0x10025D070100,
+ 0x20025D071100
+ },
+ {
+ 0x000110025d070100,
+ 0x000120025D130800
+ },
+ {}
+};
+
+static const struct dmi_system_id adr_remap_quirk_table[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Spectre x360
Convertible"),
+ },
+ .driver_data = (void *)hp_spectre_360,
+ },
+ {}
+};
+
+u64 sdw_dmi_override_adr(struct sdw_bus *bus, u64 addr)
+{
+ const struct dmi_system_id *dmi_id;
+
+ /* check if any address remap quirk applies */
+ dmi_id = dmi_first_match(adr_remap_quirk_table);
+ if (dmi_id) {
+ struct adr_remap *map = dmi_id->driver_data;
+
+ for (map = dmi_id->driver_data; map->adr; map++) {
+ if (map->adr == addr) {
+ dev_dbg(bus->dev, "remapped _ADR 0x%llx as
0x%llx\n",
+ addr, map->remapped_adr);
+ addr = map->remapped_adr;
+ break;
+ }
+ }
+ }
+
+ return addr;
+}
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index a2d5cdaa9998..a401e270a47f 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1302,6 +1302,7 @@ static int intel_prop_read(struct sdw_bus *bus)
static struct sdw_master_ops sdw_intel_ops = {
.read_prop = sdw_master_read_prop,
+ .override_adr = sdw_dmi_override_adr,
.xfer_msg = cdns_xfer_msg,
.xfer_msg_defer = cdns_xfer_msg_defer,
.reset_page_addr = cdns_reset_page_addr,
--
2.17.1
[PATCH v2 1/3] soundwire: add override addr ops
From: Vinod Koul
Platform firmware may have incorrect _ADR values causing the driver
probes to fail. Add the override_ops, which when configured will allow
for quirks based on DMI etc to override the addr values.
Co-developed-by: Bard Liao
Signed-off-by: Bard Liao
Signed-off-by: Vinod Koul
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
---
drivers/soundwire/slave.c | 8 +++-
include/linux/soundwire/sdw.h | 4 +++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 180f38bd003b..112b21967c7a 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -95,7 +95,7 @@ static bool find_slave(struct sdw_bus *bus,
struct acpi_device *adev,
struct sdw_slave_id *id)
{
- unsigned long long addr;
+ u64 addr;
unsigned int link_id;
acpi_status status;
@@ -108,6 +108,12 @@ static bool find_slave(struct sdw_bus *bus,
return false;
}
+ if (bus->ops->override_adr)
+ addr = bus->ops->override_adr(bus, addr);
+
+ if (!addr)
+ return false;
+
/* Extract link id from ADR, Bit 51 to 48 (included) */
link_id = SDW_DISCO_LINK_ID(addr);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index d08039d65825..f0a3895e8faf 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -804,6 +804,7 @@ struct sdw_defer {
/**
* struct sdw_master_ops - Master driver ops
* @read_prop: Read Master properties
+ * @override_adr: Override value read from firmware (quirk for buggy firmware)
* @xfer_msg: Transfer message callback
* @xfer_msg_defer: Defer version of transfer message callback
* @reset_page_addr: Reset the SCP page address registers
@@ -813,7 +814,8 @@ struct sdw_defer {
*/
struct sdw_master_ops {
int (*read_prop)(struct sdw_bus *bus);
-
+ u64 (*override_adr)
+ (struct sdw_bus *bus, u64 addr);
enum sdw_command_response (*xfer_msg)
(struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response (*xfer_msg_defer)
--
2.17.1
[PATCH v2 0/3] soundwire: add DMI quirks for overridind addr
Platform firmware may have incorrect _ADR values causing the driver probes to fail. Adding the override_ops allows people to override the addr values. v2: - Add the override_adr ops - Move DMI quirks to a new file Pierre-Louis Bossart (2): soundwire: Intel: introduce DMI quirks for HP Spectre x360 Convertible soundwire: Intel: add DMI quirk for Dell SKU 0A3E Vinod Koul (1): soundwire: add override addr ops drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus.h| 2 + drivers/soundwire/dmi-quirks.c | 96 ++ drivers/soundwire/intel.c | 1 + drivers/soundwire/slave.c | 8 ++- include/linux/soundwire/sdw.h | 4 +- 6 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 drivers/soundwire/dmi-quirks.c -- 2.17.1
[PATCH] soundwire: bus: add better dev_dbg to track complete() calls
From: Pierre-Louis Bossart
Add a dev_dbg() log for both enumeration and initialization completion
to better track suspend-resume issues.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Chao Song
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 6e1c988f3845..82df088c9333 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -784,7 +784,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
if (status == SDW_SLAVE_UNATTACHED) {
dev_dbg(>dev,
- "%s: initializing completion for Slave %d\n",
+ "%s: initializing enumeration and init completion for
Slave %d\n",
__func__, slave->dev_num);
init_completion(>enumeration_complete);
@@ -793,7 +793,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
} else if ((status == SDW_SLAVE_ATTACHED) &&
(slave->status == SDW_SLAVE_UNATTACHED)) {
dev_dbg(>dev,
- "%s: signaling completion for Slave %d\n",
+ "%s: signaling enumeration completion for Slave %d\n",
__func__, slave->dev_num);
complete(>enumeration_complete);
@@ -1758,8 +1758,13 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
if (ret)
dev_err(slave->bus->dev,
"Update Slave status failed:%d\n", ret);
- if (attached_initializing)
+ if (attached_initializing) {
+ dev_dbg(>dev,
+ "%s: signaling initialization completion for
Slave %d\n",
+ __func__, slave->dev_num);
+
complete(>initialization_complete);
+ }
}
return ret;
--
2.17.1
[PATCH] soundwire: return earlier if no slave is attached
From: Chao Song
If there is no slave attached to soundwire bus, we
can return earlier from sdw_bus_prep_clk_stop() and
sdw_bus_exit_clk_stop(), this saves a redundant value
check.
Signed-off-by: Chao Song
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 82df088c9333..d9deafdcf495 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -951,17 +951,17 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
simple_clk_stop = false;
}
- if (is_slave && !simple_clk_stop) {
+ /* Skip remaining clock stop preparation if no Slave is attached */
+ if (!is_slave)
+ return ret;
+
+ if (!simple_clk_stop) {
ret = sdw_bus_wait_for_clk_prep_deprep(bus,
SDW_BROADCAST_DEV_NUM);
if (ret < 0)
return ret;
}
- /* Don't need to inform slaves if there is no slave attached */
- if (!is_slave)
- return ret;
-
/* Inform slaves that prep is done */
list_for_each_entry(slave, >slaves, node) {
if (!slave->dev_num)
@@ -1075,16 +1075,13 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
"clk stop deprep failed:%d", ret);
}
- if (is_slave && !simple_clk_stop)
+ /* Skip remaining clock stop de-preparation if no Slave is attached */
+ if (!is_slave)
+ return 0;
+
+ if (!simple_clk_stop)
sdw_bus_wait_for_clk_prep_deprep(bus, SDW_BROADCAST_DEV_NUM);
- /*
-* Don't need to call slave callback function if there is no slave
-* attached
-*/
- if (!is_slave)
- return 0;
-
list_for_each_entry(slave, >slaves, node) {
if (!slave->dev_num)
continue;
--
2.17.1
[PATCH 3/3] soundwire: bus: clear parity interrupt before the mask is enabled
From: Pierre-Louis Bossart
We recently added the ability to discard bus clash interrupts reported
on startup. These bus clash interrupts can happen randomly on some
platforms and don't seem to be valid. A master-level quirk helped
squelch those spurious errors.
Additional tests on a new platform with the Maxim 98373 amplifier
showed a rare case where the parity interrupt is also thrown on
startup, at the same time as bus clashes. This issue only seems to
happen infrequently and was only observed during suspend-resume stress
tests while audio is streaming. We could make the problem go away by
adding a Slave-level quirk, but there is no evidence that the issue is
actually a Slave problem: the parity is provided by the Master, which
could also set an invalid parity in corner cases.
This patch suggests an additional bus-level quirk for parity, which is
only applied when the codec device is not known to have an issue with
parity. The initial parity error will be ignored, but a trace will be
logged to help identify potential root causes (likely a combination of
issues on both master and slave sides influenced by board-specific
electrical parameters).
BugLink: https://github.com/thesofproject/linux/issues/2533
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 9 +
drivers/soundwire/intel.c | 3 ++-
include/linux/soundwire/sdw.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d394905936e4..57581fdb2ea9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1256,6 +1256,15 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
sdw_write_no_pm(slave, SDW_SCP_INT1,
SDW_SCP_INT1_BUS_CLASH);
}
}
+ if ((slave->bus->prop.quirks & SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY)
&&
+ !(slave->prop.quirks & SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY)) {
+ /* Clear parity interrupt before enabling interrupt mask */
+ status = sdw_read_no_pm(slave, SDW_SCP_INT1);
+ if (status & SDW_SCP_INT1_PARITY) {
+ dev_warn(>dev, "PARITY error detected before INT
mask is enabled\n");
+ sdw_write_no_pm(slave, SDW_SCP_INT1,
SDW_SCP_INT1_PARITY);
+ }
+ }
/*
* Set SCP_INT1_MASK register, typically bus clash and
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index f7ba1a77a1df..c1fdc85d0a74 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1286,7 +1286,8 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
prop->hw_disabled = true;
- prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH;
+ prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH |
+ SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY;
return 0;
}
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index a2766c3b603d..30415354d419 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -426,6 +426,7 @@ struct sdw_master_prop {
};
#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH BIT(0)
+#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY BIT(1)
int sdw_master_read_prop(struct sdw_bus *bus);
int sdw_slave_read_prop(struct sdw_slave *slave);
--
2.17.1
[PATCH 0/3] soundwire: clear bus clash/parity interrupt before the mask is enabled
The SoundWire specification allows a Slave device to report a bus clash or parity error with the in-band interrupt mechanism. Unfortunately, on some platforms, these errors are randomly reported and don't seem to be valid. This series suggests the addition of a Master level quirk to discard such interrupts. The quirk should in theory have been added at the Slave level, but since the problem was detected with different generations of Slave devices it's hard to point to a specific IP. The problem might also be board-dependent and hence dealing with a Master quirk is simpler. Bard Liao (2): soundwire: bus: clear bus clash interrupt before the mask is enabled soundwire: intel: add SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH quirk Pierre-Louis Bossart (1): soundwire: bus: clear parity interrupt before the mask is enabled drivers/soundwire/bus.c | 19 +++ drivers/soundwire/intel.c | 3 +++ include/linux/soundwire/sdw.h | 5 + 3 files changed, 27 insertions(+) -- 2.17.1
[PATCH 2/3] soundwire: intel: add SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH quirk
There is nothing we can do to handle the bus clash interrupt before interrupt mask is enabled. Signed-off-by: Bard Liao Reviewed-by: Rander Wang Reviewed-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a2d5cdaa9998..f7ba1a77a1df 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1286,6 +1286,8 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus) if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE) prop->hw_disabled = true; + prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH; + return 0; } -- 2.17.1
[PATCH 1/3] soundwire: bus: clear bus clash interrupt before the mask is enabled
The SoundWire specification allows a Slave device to report a bus clash
with the in-band interrupt mechanism when it detects a conflict while
driving a bitSlot it owns. This can be a symptom of an electrical conflict
or a programming error, and it's vital to detect reliably.
Unfortunately, on some platforms, bus clashes are randomly reported by
Slave devices after a bus reset, with an interrupt status set even before
the bus clash interrupt is enabled. These initial spurious interrupts are
not relevant and should optionally be filtered out, while leaving the
interrupt mechanism enabled to detect 'true' issues.
This patch suggests the addition of a Master level quirk to discard such
interrupts. The quirk should in theory have been added at the Slave level,
but since the problem was detected with different generations of Slave
devices it's hard to point to a specific IP. The problem might also be
board-dependent and hence dealing with a Master quirk is simpler.
Signed-off-by: Bard Liao
Reviewed-by: Rander Wang
Reviewed-by: Pierre-Louis Bossart
---
drivers/soundwire/bus.c | 10 ++
include/linux/soundwire/sdw.h | 4
2 files changed, 14 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 6e1c988f3845..d394905936e4 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1240,6 +1240,7 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
static int sdw_initialize_slave(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = >prop;
+ int status;
int ret;
u8 val;
@@ -1247,6 +1248,15 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
if (ret < 0)
return ret;
+ if (slave->bus->prop.quirks & SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH) {
+ /* Clear bus clash interrupt before enabling interrupt mask */
+ status = sdw_read_no_pm(slave, SDW_SCP_INT1);
+ if (status & SDW_SCP_INT1_BUS_CLASH) {
+ dev_warn(>dev, "Bus clash detected before INT
mask is enabled\n");
+ sdw_write_no_pm(slave, SDW_SCP_INT1,
SDW_SCP_INT1_BUS_CLASH);
+ }
+ }
+
/*
* Set SCP_INT1_MASK register, typically bus clash and
* implementation-defined interrupt mask. The Parity detection
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f0b01b728640..a2766c3b603d 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -405,6 +405,7 @@ struct sdw_slave_prop {
* command
* @mclk_freq: clock reference passed to SoundWire Master, in Hz.
* @hw_disabled: if true, the Master is not functional, typically due to
pin-mux
+ * @quirks: bitmask identifying optional behavior beyond the scope of the MIPI
specification
*/
struct sdw_master_prop {
u32 revision;
@@ -421,8 +422,11 @@ struct sdw_master_prop {
u32 err_threshold;
u32 mclk_freq;
bool hw_disabled;
+ u32 quirks;
};
+#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH BIT(0)
+
int sdw_master_read_prop(struct sdw_bus *bus);
int sdw_slave_read_prop(struct sdw_slave *slave);
--
2.17.1
[RESEND PATCH v2 5/9] regmap: sdw: use _no_pm functions in regmap_read/write
sdw_update_slave_status will be invoked when a codec is attached,
and the codec driver will initialize the codec with regmap functions
while the codec device is pm_runtime suspended.
regmap routines currently rely on regular SoundWire IO functions,
which will call pm_runtime_get_sync()/put_autosuspend.
This causes a deadlock where the resume routine waits for an
initialization complete signal that while the initialization complete
can only be reached when the resume completes.
The only solution if we allow regmap functions to be used in resume
operations as well as during codec initialization is to use _no_pm
routines. The duty of making sure the bus is operational needs to be
handled above the regmap level.
Fixes: 7c22ce6e21840 ('regmap: Add SoundWire bus support')
Signed-off-by: Bard Liao
---
drivers/base/regmap/regmap-sdw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c
index c92d614b4943..4b8d2d010cab 100644
--- a/drivers/base/regmap/regmap-sdw.c
+++ b/drivers/base/regmap/regmap-sdw.c
@@ -11,7 +11,7 @@ static int regmap_sdw_write(void *context, unsigned int reg,
unsigned int val)
struct device *dev = context;
struct sdw_slave *slave = dev_to_sdw_dev(dev);
- return sdw_write(slave, reg, val);
+ return sdw_write_no_pm(slave, reg, val);
}
static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val)
@@ -20,7 +20,7 @@ static int regmap_sdw_read(void *context, unsigned int reg,
unsigned int *val)
struct sdw_slave *slave = dev_to_sdw_dev(dev);
int read;
- read = sdw_read(slave, reg);
+ read = sdw_read_no_pm(slave, reg);
if (read < 0)
return read;
--
2.17.1
[RESEND PATCH v2 8/9] soundwire: bus: fix confusion on device used by pm_runtime
From: Pierre-Louis Bossart
Intel stress-tests routinely report IO timeouts and invalid power
management transitions. Upon further analysis, we seem to be using the
wrong devices in pm_runtime calls.
Before reading and writing registers, we first need to make sure the
Slave is fully resumed. The existing code attempts to do such that,
however because of a confusion dating from 2017 and copy/paste, we
end-up resuming the parent only instead of resuming the codec device.
This can lead to accesses to the Slave registers while the bus is
still being configured and the Slave not enumerated, and as a result
IO errors occur.
This is a classic problem, similar confusions happened for HDaudio
between bus and codec device, leading to power management issues.
Fix by using the relevant device for all uses of pm_runtime functions.
Fixes: 60ee9be255712 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Fixes: aa79293517b39 ('soundwire: bus: fix io error when processing alert
event')
Fixes: 9d715fa005ebc ('soundwire: Add IO transfer')
Reported-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 4eaeeb4090f0..7c4717dd9a34 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -513,16 +513,16 @@ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nread_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -539,16 +539,16 @@ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nwrite_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -1453,7 +1453,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
dev_err(>dev, "Failed to resume device: %d\n", ret);
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
--
2.17.1
[RESEND PATCH v2 9/9] soundwire: bus: clarify dev_err/dbg device references
From: Pierre-Louis Bossart
The SoundWire bus code confuses bus and Slave device levels for
dev_err/dbg logs. That's not impacting functionality but the accuracy
of kernel logs.
We should only use bus->dev for bus-level operations and handling of
Device0. For all other logs where the device number is not zero, we
should use >dev to provide more precisions to the
user/integrator.
Reported-by: Rander Wang
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 63 +
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 7c4717dd9a34..39edf87cf832 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -636,6 +636,7 @@ static int sdw_get_device_num(struct sdw_slave *slave)
static int sdw_assign_device_num(struct sdw_slave *slave)
{
+ struct sdw_bus *bus = slave->bus;
int ret, dev_num;
bool new_device = false;
@@ -646,7 +647,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
dev_num = sdw_get_device_num(slave);
mutex_unlock(>bus->bus_lock);
if (dev_num < 0) {
- dev_err(slave->bus->dev, "Get dev_num failed:
%d\n",
+ dev_err(bus->dev, "Get dev_num failed: %d\n",
dev_num);
return dev_num;
}
@@ -659,7 +660,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
}
if (!new_device)
- dev_dbg(slave->bus->dev,
+ dev_dbg(bus->dev,
"Slave already registered, reusing dev_num:%d\n",
slave->dev_num);
@@ -669,7 +670,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num);
if (ret < 0) {
- dev_err(>dev, "Program device_num %d failed: %d\n",
+ dev_err(bus->dev, "Program device_num %d failed: %d\n",
dev_num, ret);
return ret;
}
@@ -748,7 +749,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
*/
ret = sdw_assign_device_num(slave);
if (ret) {
- dev_err(slave->bus->dev,
+ dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
return ret;
@@ -788,9 +789,11 @@ static int sdw_program_device_num(struct sdw_bus *bus)
static void sdw_modify_slave_status(struct sdw_slave *slave,
enum sdw_slave_status status)
{
- mutex_lock(>bus->bus_lock);
+ struct sdw_bus *bus = slave->bus;
- dev_vdbg(>dev,
+ mutex_lock(>bus_lock);
+
+ dev_vdbg(bus->dev,
"%s: changing status slave %d status %d new status %d\n",
__func__, slave->dev_num, slave->status, status);
@@ -811,7 +814,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
complete(>enumeration_complete);
}
slave->status = status;
- mutex_unlock(>bus->bus_lock);
+ mutex_unlock(>bus_lock);
}
static enum sdw_clk_stop_mode sdw_get_clk_stop_mode(struct sdw_slave *slave)
@@ -1140,7 +1143,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DPN_INTMASK write failed:%d\n", val);
return ret;
@@ -1271,7 +1274,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
/* Enable SCP interrupts */
ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
return ret;
}
@@ -1286,7 +1289,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
return ret;
}
@@ -1298,7 +1301,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave,
[RESEND PATCH v2 7/9] regmap: sdw-mbq: use MODULE_LICENSE("GPL")
"GPL v2" is the same as "GPL". It exists for historic reasons.
See Documentation/process/license-rules.rst
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/base/regmap/regmap-sdw-mbq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regmap-sdw-mbq.c
b/drivers/base/regmap/regmap-sdw-mbq.c
index 6675c3a4b829..fe3ac26b66ad 100644
--- a/drivers/base/regmap/regmap-sdw-mbq.c
+++ b/drivers/base/regmap/regmap-sdw-mbq.c
@@ -98,4 +98,4 @@ struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave
*sdw,
EXPORT_SYMBOL_GPL(__devm_regmap_init_sdw_mbq);
MODULE_DESCRIPTION("Regmap SoundWire MBQ Module");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
--
2.17.1
[RESEND PATCH v2 6/9] regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
Use no_pm versions for write and read. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang --- drivers/base/regmap/regmap-sdw-mbq.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/regmap/regmap-sdw-mbq.c b/drivers/base/regmap/regmap-sdw-mbq.c index 8ce30650b97c..6675c3a4b829 100644 --- a/drivers/base/regmap/regmap-sdw-mbq.c +++ b/drivers/base/regmap/regmap-sdw-mbq.c @@ -15,11 +15,11 @@ static int regmap_sdw_mbq_write(void *context, unsigned int reg, unsigned int va struct sdw_slave *slave = dev_to_sdw_dev(dev); int ret; - ret = sdw_write(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff); + ret = sdw_write_no_pm(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff); if (ret < 0) return ret; - return sdw_write(slave, reg, val & 0xff); + return sdw_write_no_pm(slave, reg, val & 0xff); } static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *val) @@ -29,11 +29,11 @@ static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *va int read0; int read1; - read0 = sdw_read(slave, reg); + read0 = sdw_read_no_pm(slave, reg); if (read0 < 0) return read0; - read1 = sdw_read(slave, SDW_SDCA_MBQ_CTL(reg)); + read1 = sdw_read_no_pm(slave, SDW_SDCA_MBQ_CTL(reg)); if (read1 < 0) return read1; -- 2.17.1
[RESEND PATCH v2 4/9] soundwire: export sdw_write/read_no_pm functions
sdw_write_no_pm and sdw_read_no_pm are useful when we want to do IO
without touching PM.
Fixes: 0231453bc08f ('soundwire: bus: add clock stop helpers')
Fixes: 60ee9be25571 ('soundwire: bus: add PM/no-PM versions of
read/write functions')
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 7 ---
include/linux/soundwire/sdw.h | 2 ++
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 86c339d77a39..4eaeeb4090f0 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -405,10 +405,11 @@ sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr,
size_t count, u8 *val)
return sdw_transfer(slave->bus, );
}
-static int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
{
return sdw_nwrite_no_pm(slave, addr, 1, );
}
+EXPORT_SYMBOL(sdw_write_no_pm);
static int
sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
@@ -476,8 +477,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
}
EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked);
-static int
-sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
+int sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
{
u8 buf;
int ret;
@@ -488,6 +488,7 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
else
return buf;
}
+EXPORT_SYMBOL(sdw_read_no_pm);
static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
{
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f0b01b728640..d08039d65825 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1005,6 +1005,8 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus);
int sdw_read(struct sdw_slave *slave, u32 addr);
int sdw_write(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_read_no_pm(struct sdw_slave *slave, u32 addr);
int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
--
2.17.1
[RESEND PATCH v2 3/9] soundwire: bus: use no_pm IO routines for all interrupt handling
From: Pierre-Louis Bossart
There is no need to play with pm_runtime reference counts, if needed
the codec drivers are already explicitly resumed.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index b1830032b052..86c339d77a39 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1295,7 +1295,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
u8 clear, impl_int_mask;
int status, status2, ret, count = 0;
- status = sdw_read(slave, SDW_DP0_INT);
+ status = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status);
@@ -1334,7 +1334,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* clear the interrupts but don't touch reserved and
SDCA_CASCADE fields */
- ret = sdw_write(slave, SDW_DP0_INT, clear);
+ ret = sdw_write_no_pm(slave, SDW_DP0_INT, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT write failed:%d\n", ret);
@@ -1342,7 +1342,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* Read DP0 interrupt again */
- status2 = sdw_read(slave, SDW_DP0_INT);
+ status2 = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status2);
@@ -1373,7 +1373,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
return sdw_handle_dp0_interrupt(slave, slave_status);
addr = SDW_DPN_INT(port);
- status = sdw_read(slave, addr);
+ status = sdw_read_no_pm(slave, addr);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status);
@@ -1407,7 +1407,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* clear the interrupt but don't touch reserved fields */
- ret = sdw_write(slave, addr, clear);
+ ret = sdw_write_no_pm(slave, addr, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT write failed:%d\n", ret);
@@ -1415,7 +1415,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* Read DPN interrupt again */
- status2 = sdw_read(slave, addr);
+ status2 = sdw_read_no_pm(slave, addr);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status2);
@@ -1457,7 +1457,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Read Intstat 1, Intstat 2 and Intstat 3 registers */
- ret = sdw_read(slave, SDW_SCP_INT1);
+ ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 read failed:%d\n", ret);
@@ -1465,7 +1465,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
buf = ret;
- ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2);
+ ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, buf2);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT2/3 read failed:%d\n", ret);
@@ -1473,7 +1473,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
if (slave->prop.is_sdca) {
- ret = sdw_read(slave, SDW_DP0_INT);
+ ret = sdw_read_no_pm(slave, SDW_DP0_INT);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", ret);
@@ -1570,7 +1570,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Ack interrupt */
- ret = sdw_write(slave, SDW_SCP_INT1, clear);
+ ret = sdw_write_no_pm(slave, SDW_SCP_INT1, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 write failed:%d\n", ret);
@@ -1584,7 +1584,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
*
[RESEND PATCH v2 1/9] soundwire: bus: use sdw_update_no_pm when initializing a device
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
Fixes: 60ee9be25571 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d1e8c3a54976..60c42508c6c6 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -489,6 +489,18 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
return buf;
}
+static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
+{
+ int tmp;
+
+ tmp = sdw_read_no_pm(slave, addr);
+ if (tmp < 0)
+ return tmp;
+
+ tmp = (tmp & ~mask) | val;
+ return sdw_write_no_pm(slave, addr, tmp);
+}
+
/**
* sdw_nread() - Read "n" contiguous SDW Slave registers
* @slave: SDW Slave
@@ -1256,7 +1268,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = slave->prop.scp_int1_mask;
/* Enable SCP interrupts */
- ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
+ ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
@@ -1271,7 +1283,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = prop->dp0_prop->imp_def_interrupts;
val |= SDW_DP0_INT_PORT_READY | SDW_DP0_INT_BRA_FAILURE;
- ret = sdw_update(slave, SDW_DP0_INTMASK, val, val);
+ ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
dev_err(slave->bus->dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
--
2.17.1
[RESEND PATCH v2 2/9] soundwire: bus: use sdw_write_no_pm when setting the bus scale registers
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
This patch fixes the same problem as the previous one, but is split to
make the life of linux-stable maintainers less painful.
Fixes: 29d158f90690 ('soundwire: bus: initialize bus clock base and scale
registers')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 60c42508c6c6..b1830032b052 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1222,7 +1222,7 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
scale_index++;
- ret = sdw_write(slave, SDW_SCP_BUS_CLOCK_BASE, base);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUS_CLOCK_BASE, base);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUS_CLOCK_BASE write failed:%d\n", ret);
@@ -1230,13 +1230,13 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
/* initialize scale for both banks */
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B0 write failed:%d\n", ret);
return ret;
}
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
if (ret < 0)
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B1 write failed:%d\n", ret);
--
2.17.1
[RESEND PATCH v2 0/9] soundwire/regmap: use _no_pm routines
When a Slave device is resumed, it may resume the bus and restart the
enumeration. And Slave drivers will wait for initialization_complete
complete in their resume function, however initialization_complete will
complete after sdw_update_slave_status function is finished and codec
driver usually call some IO functions in the update_status callback
function.
It will become a deadlock if we use regular read/write routines during
the resuming process.
This series touches both soundwire and regmap trees.
commit fb5103f9d6ce ("regmap/SoundWire: sdw: add support for SoundWire 1.2 MBQ")
is needed for soundwire tree to complie.
On the other hands,
commit 6e06a85556f9 ("soundwire: bus: add comments to explain interrupt loop
filter")
to
commit 47b8520997a8 ("soundwire: bus: only clear valid DPN interrupts")
are needed for regmap tree.
v2:
- Separate commits according to maintainer's comments.
Bard Liao (4):
soundwire: export sdw_write/read_no_pm functions
regmap: sdw: use _no_pm functions in regmap_read/write
regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
regmap: sdw-mbq: use MODULE_LICENSE("GPL")
Pierre-Louis Bossart (5):
soundwire: bus: use sdw_update_no_pm when initializing a device
soundwire: bus: use sdw_write_no_pm when setting the bus scale
registers
soundwire: bus: use no_pm IO routines for all interrupt handling
soundwire: bus: fix confusion on device used by pm_runtime
soundwire: bus: clarify dev_err/dbg device references
drivers/base/regmap/regmap-sdw-mbq.c | 10 +-
drivers/base/regmap/regmap-sdw.c | 4 +-
drivers/soundwire/bus.c | 136 +++
include/linux/soundwire/sdw.h| 2 +
4 files changed, 85 insertions(+), 67 deletions(-)
--
2.17.1
[PATCH 1/2] ASoC: codecs: soundwire: increase resume timeout
From: Pierre-Louis Bossart
The resume operation relies on multiple transactions to synchronize
the regmap state, make sure the timeout is one order of magnitude
larger than an individual transaction, so that timeouts of failed
transactions are detected first.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
sound/soc/codecs/max98373-sdw.c | 4 +++-
sound/soc/codecs/rt1308-sdw.c | 2 +-
sound/soc/codecs/rt5682.h | 2 +-
sound/soc/codecs/rt700-sdw.c| 2 +-
sound/soc/codecs/rt711-sdw.c| 2 +-
sound/soc/codecs/rt715-sdw.c| 2 +-
6 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
index ec2e79c57357..ad2d5d6a2fe4 100644
--- a/sound/soc/codecs/max98373-sdw.c
+++ b/sound/soc/codecs/max98373-sdw.c
@@ -251,6 +251,8 @@ static __maybe_unused int max98373_suspend(struct device
*dev)
return 0;
}
+#define MAX98373_PROBE_TIMEOUT 5000
+
static __maybe_unused int max98373_resume(struct device *dev)
{
struct sdw_slave *slave = dev_to_sdw_dev(dev);
@@ -264,7 +266,7 @@ static __maybe_unused int max98373_resume(struct device
*dev)
goto regmap_sync;
time = wait_for_completion_timeout(>initialization_complete,
- msecs_to_jiffies(2000));
+
msecs_to_jiffies(MAX98373_PROBE_TIMEOUT));
if (!time) {
dev_err(dev, "Initialization not complete, timed out\n");
return -ETIMEDOUT;
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index ec5564f780e8..afd2c3b687cc 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -701,7 +701,7 @@ static int __maybe_unused rt1308_dev_suspend(struct device
*dev)
return 0;
}
-#define RT1308_PROBE_TIMEOUT 2000
+#define RT1308_PROBE_TIMEOUT 5000
static int __maybe_unused rt1308_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt5682.h b/sound/soc/codecs/rt5682.h
index 99b85cfe6248..1f9c51a5b9bf 100644
--- a/sound/soc/codecs/rt5682.h
+++ b/sound/soc/codecs/rt5682.h
@@ -1356,7 +1356,7 @@
#define RT5682_SAR_SOUR_TYPE (0x0)
/* soundwire timeout */
-#define RT5682_PROBE_TIMEOUT 2000
+#define RT5682_PROBE_TIMEOUT 5000
#define RT5682_STEREO_RATES SNDRV_PCM_RATE_8000_192000
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
index fb77e77a4ebd..ce9255b881d4 100644
--- a/sound/soc/codecs/rt700-sdw.c
+++ b/sound/soc/codecs/rt700-sdw.c
@@ -490,7 +490,7 @@ static int __maybe_unused rt700_dev_suspend(struct device
*dev)
return 0;
}
-#define RT700_PROBE_TIMEOUT 2000
+#define RT700_PROBE_TIMEOUT 5000
static int __maybe_unused rt700_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c
index fc7df79c3b91..756c0ada3b31 100644
--- a/sound/soc/codecs/rt711-sdw.c
+++ b/sound/soc/codecs/rt711-sdw.c
@@ -493,7 +493,7 @@ static int __maybe_unused rt711_dev_suspend(struct device
*dev)
return 0;
}
-#define RT711_PROBE_TIMEOUT 2000
+#define RT711_PROBE_TIMEOUT 5000
static int __maybe_unused rt711_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c
index 8f0aa1e8a273..71dd3b97a459 100644
--- a/sound/soc/codecs/rt715-sdw.c
+++ b/sound/soc/codecs/rt715-sdw.c
@@ -533,7 +533,7 @@ static int __maybe_unused rt715_dev_suspend(struct device
*dev)
return 0;
}
-#define RT715_PROBE_TIMEOUT 2000
+#define RT715_PROBE_TIMEOUT 5000
static int __maybe_unused rt715_dev_resume(struct device *dev)
{
--
2.17.1
[PATCH 2/2] soundwire: cadence: reduce timeout on transactions
From: Pierre-Louis Bossart Currently the timeout for SoundWire individual transactions is 2s. This is too large in comparison with the enumeration and completion timeouts used in codec drivers. A command will typically be handled in less than 100us, so 500ms for the command completion is more than generous. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 9fa55164354a..f0b0ec173f8b 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -188,7 +188,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_PDI_CONFIG_PORT GENMASK(4, 0) /* Driver defaults */ -#define CDNS_TX_TIMEOUT2000 +#define CDNS_TX_TIMEOUT500 #define CDNS_SCP_RX_FIFOLEVEL 0x2 -- 2.17.1
[PATCH 0/2] ASoC/SoundWire: fix timeout values
The timeout for an individual transaction w/ the Cadence IP is the same as the entire resume operation for codecs. This doesn't make sense, we need to have at least one order of magnitude between individual transactions and the entire resume operation. Set the timeout on the Cadence side to 500ms and 5s for the codec resume. Both ASoC and SoundWire trees are fine for this series. Pierre-Louis Bossart (2): ASoC: codecs: soundwire: increase resume timeout soundwire: cadence: reduce timeout on transactions drivers/soundwire/cadence_master.c | 2 +- sound/soc/codecs/max98373-sdw.c| 4 +++- sound/soc/codecs/rt1308-sdw.c | 2 +- sound/soc/codecs/rt5682.h | 2 +- sound/soc/codecs/rt700-sdw.c | 2 +- sound/soc/codecs/rt711-sdw.c | 2 +- sound/soc/codecs/rt715-sdw.c | 2 +- 7 files changed, 9 insertions(+), 7 deletions(-) -- 2.17.1
[PATCH 5/5] soundwire: cadence: adjust verbosity in response handling
From: Pierre-Louis Bossart
There are too many logs on startup, e.g.
[ 8811.851497] cdns_fill_msg_resp: 2 callbacks suppressed
[ 8811.851497] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851498] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851499] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851499] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851500] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851500] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851502] intel-sdw intel-sdw.0: Msg ignored for Slave 0
[ 8811.851503] soundwire sdw-master-0: No more devices to enumerate
We can skip the 'Msg Ack not received' since it's typical of the
enumeration end, and conversely add the information on which command
fails.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d3c9cf920cbd..8d7166ffd4ad 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -483,11 +483,11 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns,
for (i = 0; i < count; i++) {
if (!(cdns->response_buf[i] & CDNS_MCP_RESP_ACK)) {
no_ack = 1;
- dev_dbg_ratelimited(cdns->dev, "Msg Ack not
received\n");
+ dev_vdbg(cdns->dev, "Msg Ack not received, cmd %d\n",
i);
}
if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
nack = 1;
- dev_err_ratelimited(cdns->dev, "Msg NACK received\n");
+ dev_err_ratelimited(cdns->dev, "Msg NACK received, cmd
%d\n", i);
}
}
--
2.17.1
[PATCH 4/5] soundwire: cadence: fix ACK/NAK handling
From: Pierre-Louis Bossart
The existing code reports a NAK only when ACK=0
This is not aligned with the SoundWire 1.x specifications.
Table 32 in the SoundWire 1.2 specification shows that a Device shall
not set NAK=1 if ACK=1. But Table 33 shows the Combined Response
may very well be NAK=1/ACK=1, e.g. if another Device than the one
addressed reports a parity error.
NAK=1 signals a 'Command_Aborted', regardless of the ACK bit value.
Move the tests for NAK so that the NAK=1/ACK=1 combination is properly
detected according to the specification.
Fixes: 956baa1992f9a ('soundwire: cdns: Add sdw_master_ops and IO transfer
support')
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 801e1fef59d8..d3c9cf920cbd 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -484,10 +484,10 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns,
if (!(cdns->response_buf[i] & CDNS_MCP_RESP_ACK)) {
no_ack = 1;
dev_dbg_ratelimited(cdns->dev, "Msg Ack not
received\n");
- if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
- nack = 1;
- dev_err_ratelimited(cdns->dev, "Msg NACK
received\n");
- }
+ }
+ if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
+ nack = 1;
+ dev_err_ratelimited(cdns->dev, "Msg NACK received\n");
}
}
--
2.17.1
[PATCH 3/5] soundwire: bus: add more details to track failed transfers
The current error log does not provide details on the type of transfers and which address/count was requested. All this information can help locate in which parts of the configuration process an error occurred. Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/bus.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 3cc006bfae71..6e1c988f3845 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -267,8 +267,10 @@ static int sdw_transfer_unlocked(struct sdw_bus *bus, struct sdw_msg *msg) ret = do_transfer(bus, msg); if (ret != 0 && ret != -ENODATA) - dev_err(bus->dev, "trf on Slave %d failed:%d\n", - msg->dev_num, ret); + dev_err(bus->dev, "trf on Slave %d failed:%d %s addr %x count %d\n", + msg->dev_num, ret, + (msg->flags & SDW_MSG_FLAG_WRITE) ? "write" : "read", + msg->addr, msg->len); if (msg->page) sdw_reset_page(bus, msg->dev_num); -- 2.17.1
[PATCH 2/5] soundwire: cadence: add status in dev_dbg 'State change' log
From: Pierre-Louis Bossart
The existing debug log only mentions a state change, without providing
any details. For integration and stress-tests, it's helpful to see in
the dmesg log the reason for the state change.
The value is intended for power users and isn't converted as
human-readable values. But for the record each device has a 4-bit
status:
BIT(0): Unattached
BIT(1): Attached
BIT(2): Alert
BIT(3): Reserved (should not happen)
Example:
[ 121.891288] intel-sdw intel-sdw.0: Slave status change: 0x2
<< this shows a Device0 Attached
[ 121.891295] soundwire sdw-master-0: Slave attached, programming device number
[ 121.891629] soundwire sdw-master-0: SDW Slave Addr: 30025d071101
[ 121.891632] soundwire sdw-master-0: SDW Slave class_id 1, part_id 711,
mfg_id 25d, unique_id 0, version 3
[ 121.892011] intel-sdw intel-sdw.0: Msg ignored for Slave 0
[ 121.892013] soundwire sdw-master-0: No more devices to enumerate
[ 121.892200] intel-sdw intel-sdw.0: Slave status change: 0x21
<< this shows the device now Attached as Device1 and Unattached as
Device0, i.e. a successful enumeration.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 9fa55164354a..801e1fef59d8 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -734,21 +734,18 @@ static void cdns_read_response(struct sdw_cdns *cdns)
}
static int cdns_update_slave_status(struct sdw_cdns *cdns,
- u32 slave0, u32 slave1)
+ u64 slave_intstat)
{
enum sdw_slave_status status[SDW_MAX_DEVICES + 1];
bool is_slave = false;
- u64 slave;
u32 mask;
int i, set_status;
- /* combine the two status */
- slave = ((u64)slave1 << 32) | slave0;
memset(status, 0, sizeof(status));
for (i = 0; i <= SDW_MAX_DEVICES; i++) {
- mask = (slave >> (i * CDNS_MCP_SLAVE_STATUS_NUM)) &
- CDNS_MCP_SLAVE_STATUS_BITS;
+ mask = (slave_intstat >> (i * CDNS_MCP_SLAVE_STATUS_NUM)) &
+ CDNS_MCP_SLAVE_STATUS_BITS;
if (!mask)
continue;
@@ -918,13 +915,17 @@ static void cdns_update_slave_status_work(struct
work_struct *work)
struct sdw_cdns *cdns =
container_of(work, struct sdw_cdns, work);
u32 slave0, slave1;
-
- dev_dbg_ratelimited(cdns->dev, "Slave status change\n");
+ u64 slave_intstat;
slave0 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT0);
slave1 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
- cdns_update_slave_status(cdns, slave0, slave1);
+ /* combine the two status */
+ slave_intstat = ((u64)slave1 << 32) | slave0;
+
+ dev_dbg_ratelimited(cdns->dev, "Slave status change: 0x%llx\n",
slave_intstat);
+
+ cdns_update_slave_status(cdns, slave_intstat);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1);
--
2.17.1
[PATCH 1/5] soundwire: use consistent format for Slave devID logs
From: Pierre-Louis Bossart
We mix decimal and hexadecimal values, this leads to confusions in
dmesg logs and bug reports. Let's add a 0x prefix for all hexadecimal
values and a format when more than 4 bits are used.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 5 ++---
drivers/soundwire/slave.c | 10 --
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d1e8c3a54976..3cc006bfae71 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -679,9 +679,8 @@ void sdw_extract_slave_id(struct sdw_bus *bus,
id->class_id = SDW_CLASS_ID(addr);
dev_dbg(bus->dev,
- "SDW Slave class_id %x, part_id %x, mfg_id %x, unique_id %x,
version %x\n",
- id->class_id, id->part_id, id->mfg_id,
- id->unique_id, id->sdw_version);
+ "SDW Slave class_id 0x%02x, mfg_id 0x%04x, part_id 0x%04x,
unique_id 0x%x, version 0x%x\n",
+ id->class_id, id->mfg_id, id->part_id, id->unique_id,
id->sdw_version);
}
static int sdw_program_device_num(struct sdw_bus *bus)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index a08f4081c1c4..180f38bd003b 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -163,15 +163,13 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)
if (id.unique_id != id2.unique_id) {
dev_dbg(bus->dev,
- "Valid unique IDs %x %x for Slave mfg
%x part %d\n",
- id.unique_id, id2.unique_id,
- id.mfg_id, id.part_id);
+ "Valid unique IDs 0x%x 0x%x for Slave
mfg_id 0x%04x, part_id 0x%04x\n",
+ id.unique_id, id2.unique_id, id.mfg_id,
id.part_id);
ignore_unique_id = false;
} else {
dev_err(bus->dev,
- "Invalid unique IDs %x %x for Slave mfg
%x part %d\n",
- id.unique_id, id2.unique_id,
- id.mfg_id, id.part_id);
+ "Invalid unique IDs 0x%x 0x%x for Slave
mfg_id 0x%04x, part_id 0x%04x\n",
+ id.unique_id, id2.unique_id, id.mfg_id,
id.part_id);
return -ENODEV;
}
}
--
2.17.1
[PATCH 0/5] soundwire: fix ACK/NAK handling and improve log
The existing code reports a NAK only when ACK=0 This is not aligned with the SoundWire 1.x specifications. Table 32 in the SoundWire 1.2 specification shows that a Device shall not set NAK=1 if ACK=1. But Table 33 shows the Combined Response may very well be NAK=1/ACK=1, e.g. if another Device than the one addressed reports a parity error. NAK=1 signals a 'Command_Aborted', regardless of the ACK bit value. Move the tests for NAK so that the NAK=1/ACK=1 combination is properly detected according to the specification. Also, improve the demesg log to get more information for debugging. Bard Liao (1): soundwire: bus: add more details to track failed transfers Pierre-Louis Bossart (4): soundwire: use consistent format for Slave devID logs soundwire: cadence: add status in dev_dbg 'State change' log soundwire: cadence: fix ACK/NAK handling soundwire: cadence: adjust verbosity in response handling drivers/soundwire/bus.c| 11 ++- drivers/soundwire/cadence_master.c | 29 +++-- drivers/soundwire/slave.c | 10 -- 3 files changed, 25 insertions(+), 25 deletions(-) -- 2.17.1
[PATCH] soundwire: intel: don't return error when clock stop failed
dev->power.runtime_error will be set to the return value of the runtime
suspend callback function, and runtime resume function will return
-EINVAL if dev->power.runtime_error is not 0.
Somehow the codec rarely doesn't return an ACK to the clock prepare
command. If we stop the runtime suspend process and return error, we
will not be able to resume again. Likewise, if the codec lost sync and
did not rejoin, the resume operation will also fail. As a result, the
SoundWire bus can not be used anymore.
This patch suggests to finish the runtime suspend process even if we fail
to stop sdw bus clock. In the case where we do a hardware reset, the codecs
will be reconfigured completely. In the case where we use the regular clock
stop, the codecs keep their state and worst case will fall off the bus and
reattach.
The only drawback is that the power consumption may be higher and
device-initiated interrupts may be lost, but at least audio function can
still work after resume.
Signed-off-by: Bard Liao
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Reviewed-by: Pierre-Louis Bossart
---
drivers/soundwire/intel.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10b60b7b17bb..a2d5cdaa9998 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1673,10 +1673,12 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
!clock_stop_quirks) {
+ bool wake_enable = true;
+
ret = sdw_cdns_clock_stop(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable clock stop on suspend\n");
- return ret;
+ wake_enable = false;
}
ret = sdw_cdns_enable_interrupt(cdns, false);
@@ -1691,7 +1693,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
return ret;
}
- intel_shim_wake(sdw, true);
+ intel_shim_wake(sdw, wake_enable);
} else {
dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
__func__, clock_stop_quirks);
--
2.17.1
[PATCH 2/2] device property: add description of fwnode cases
There are only four valid fwnode cases which are
- primary --> secondary --> -ENODEV
- primary --> NULL
- secondary --> -ENODEV
- NULL
dev->fwnode should be converted between the 4 cases above no matter
how/when set_primary_fwnode() and set_secondary_fwnode() are called.
Describe it in the code so people will keep it in mind.
Signed-off-by: Bard Liao
---
drivers/base/core.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 51b9545a050b..17eb14607074 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct
fwnode_handle *fwnode)
*
* Set the device's firmware node pointer to @fwnode, but if a secondary
* firmware node of the device is present, preserve it.
+ *
+ * Valid fwnode cases are:
+ * - primary --> secondary --> -ENODEV
+ * - primary --> NULL
+ * - secondary --> -ENODEV
+ * - NULL
*/
void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
{
@@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct
fwnode_handle *fwnode)
} else {
if (fwnode_is_primary(fn)) {
dev->fwnode = fn->secondary;
+ /* Set fn->secondary = NULL to keep fn as a primary
fwnode */
if (!(parent && fn == parent->fwnode))
fn->secondary = NULL;
} else {
--
2.17.1
[PATCH 0/2] Revert "device property: Keep secondary firmware node secondary by type"
While the commit d5dcce0 ("device property: Keep secondary firmware
node secondary by type")
describes everything correct in its commit message the change it made does
the opposite and original commit c15e1bd ("device property: Fix the
secondary firmware node handling in set_primary_fwnode()") was fully
correct. Thus, revert the former one here and improve documentation.
Bard Liao (2):
Revert "device property: Keep secondary firmware node secondary by
type"
device property: add description of fwnode cases
drivers/base/core.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.17.1
[PATCH 1/2] Revert "device property: Keep secondary firmware node secondary by type"
While the commit d5dcce0c414f ("device property: Keep secondary firmware
node secondary by type")
describes everything correct in its commit message the change it made does
the opposite and original commit c15e1bdda436 ("device property: Fix the
secondary firmware node handling in set_primary_fwnode()") was fully
correct. Thus, revert the former one here and improve documentation in
the next patch.
Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary
by type")
Signed-off-by: Bard Liao
---
drivers/base/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 25e08e5f40bd..51b9545a050b 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct
fwnode_handle *fwnode)
if (fwnode_is_primary(fn)) {
dev->fwnode = fn->secondary;
if (!(parent && fn == parent->fwnode))
- fn->secondary = ERR_PTR(-ENODEV);
+ fn->secondary = NULL;
} else {
dev->fwnode = NULL;
}
--
2.17.1
[PATCH v2 9/9] soundwire: bus: clarify dev_err/dbg device references
From: Pierre-Louis Bossart
The SoundWire bus code confuses bus and Slave device levels for
dev_err/dbg logs. That's not impacting functionality but the accuracy
of kernel logs.
We should only use bus->dev for bus-level operations and handling of
Device0. For all other logs where the device number is not zero, we
should use >dev to provide more precisions to the
user/integrator.
Reported-by: Rander Wang
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 63 +
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 7c4717dd9a34..39edf87cf832 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -636,6 +636,7 @@ static int sdw_get_device_num(struct sdw_slave *slave)
static int sdw_assign_device_num(struct sdw_slave *slave)
{
+ struct sdw_bus *bus = slave->bus;
int ret, dev_num;
bool new_device = false;
@@ -646,7 +647,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
dev_num = sdw_get_device_num(slave);
mutex_unlock(>bus->bus_lock);
if (dev_num < 0) {
- dev_err(slave->bus->dev, "Get dev_num failed:
%d\n",
+ dev_err(bus->dev, "Get dev_num failed: %d\n",
dev_num);
return dev_num;
}
@@ -659,7 +660,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
}
if (!new_device)
- dev_dbg(slave->bus->dev,
+ dev_dbg(bus->dev,
"Slave already registered, reusing dev_num:%d\n",
slave->dev_num);
@@ -669,7 +670,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num);
if (ret < 0) {
- dev_err(>dev, "Program device_num %d failed: %d\n",
+ dev_err(bus->dev, "Program device_num %d failed: %d\n",
dev_num, ret);
return ret;
}
@@ -748,7 +749,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
*/
ret = sdw_assign_device_num(slave);
if (ret) {
- dev_err(slave->bus->dev,
+ dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
return ret;
@@ -788,9 +789,11 @@ static int sdw_program_device_num(struct sdw_bus *bus)
static void sdw_modify_slave_status(struct sdw_slave *slave,
enum sdw_slave_status status)
{
- mutex_lock(>bus->bus_lock);
+ struct sdw_bus *bus = slave->bus;
- dev_vdbg(>dev,
+ mutex_lock(>bus_lock);
+
+ dev_vdbg(bus->dev,
"%s: changing status slave %d status %d new status %d\n",
__func__, slave->dev_num, slave->status, status);
@@ -811,7 +814,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
complete(>enumeration_complete);
}
slave->status = status;
- mutex_unlock(>bus->bus_lock);
+ mutex_unlock(>bus_lock);
}
static enum sdw_clk_stop_mode sdw_get_clk_stop_mode(struct sdw_slave *slave)
@@ -1140,7 +1143,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DPN_INTMASK write failed:%d\n", val);
return ret;
@@ -1271,7 +1274,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
/* Enable SCP interrupts */
ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
return ret;
}
@@ -1286,7 +1289,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
return ret;
}
@@ -1298,7 +1301,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave,
[PATCH v2 7/9] regmap: sdw-mbq: use MODULE_LICENSE("GPL")
"GPL v2" is the same as "GPL". It exists for historic reasons.
See Documentation/process/license-rules.rst
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/base/regmap/regmap-sdw-mbq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regmap-sdw-mbq.c
b/drivers/base/regmap/regmap-sdw-mbq.c
index 6675c3a4b829..fe3ac26b66ad 100644
--- a/drivers/base/regmap/regmap-sdw-mbq.c
+++ b/drivers/base/regmap/regmap-sdw-mbq.c
@@ -98,4 +98,4 @@ struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave
*sdw,
EXPORT_SYMBOL_GPL(__devm_regmap_init_sdw_mbq);
MODULE_DESCRIPTION("Regmap SoundWire MBQ Module");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
--
2.17.1
[PATCH v2 8/9] soundwire: bus: fix confusion on device used by pm_runtime
From: Pierre-Louis Bossart
Intel stress-tests routinely report IO timeouts and invalid power
management transitions. Upon further analysis, we seem to be using the
wrong devices in pm_runtime calls.
Before reading and writing registers, we first need to make sure the
Slave is fully resumed. The existing code attempts to do such that,
however because of a confusion dating from 2017 and copy/paste, we
end-up resuming the parent only instead of resuming the codec device.
This can lead to accesses to the Slave registers while the bus is
still being configured and the Slave not enumerated, and as a result
IO errors occur.
This is a classic problem, similar confusions happened for HDaudio
between bus and codec device, leading to power management issues.
Fix by using the relevant device for all uses of pm_runtime functions.
Fixes: 60ee9be255712 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Fixes: aa79293517b39 ('soundwire: bus: fix io error when processing alert
event')
Fixes: 9d715fa005ebc ('soundwire: Add IO transfer')
Reported-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 4eaeeb4090f0..7c4717dd9a34 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -513,16 +513,16 @@ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nread_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -539,16 +539,16 @@ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nwrite_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -1453,7 +1453,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
dev_err(>dev, "Failed to resume device: %d\n", ret);
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
--
2.17.1
[PATCH v2 5/9] regmap: sdw: use _no_pm functions in regmap_read/write
sdw_update_slave_status will be invoked when a codec is attached,
and the codec driver will initialize the codec with regmap functions
while the codec device is pm_runtime suspended.
regmap routines currently rely on regular SoundWire IO functions,
which will call pm_runtime_get_sync()/put_autosuspend.
This causes a deadlock where the resume routine waits for an
initialization complete signal that while the initialization complete
can only be reached when the resume completes.
The only solution if we allow regmap functions to be used in resume
operations as well as during codec initialization is to use _no_pm
routines. The duty of making sure the bus is operational needs to be
handled above the regmap level.
Fixes: 7c22ce6e21840 ('regmap: Add SoundWire bus support')
Signed-off-by: Bard Liao
---
drivers/base/regmap/regmap-sdw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c
index c92d614b4943..4b8d2d010cab 100644
--- a/drivers/base/regmap/regmap-sdw.c
+++ b/drivers/base/regmap/regmap-sdw.c
@@ -11,7 +11,7 @@ static int regmap_sdw_write(void *context, unsigned int reg,
unsigned int val)
struct device *dev = context;
struct sdw_slave *slave = dev_to_sdw_dev(dev);
- return sdw_write(slave, reg, val);
+ return sdw_write_no_pm(slave, reg, val);
}
static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val)
@@ -20,7 +20,7 @@ static int regmap_sdw_read(void *context, unsigned int reg,
unsigned int *val)
struct sdw_slave *slave = dev_to_sdw_dev(dev);
int read;
- read = sdw_read(slave, reg);
+ read = sdw_read_no_pm(slave, reg);
if (read < 0)
return read;
--
2.17.1
[PATCH v2 6/9] regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
Use no_pm versions for write and read. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang --- drivers/base/regmap/regmap-sdw-mbq.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/regmap/regmap-sdw-mbq.c b/drivers/base/regmap/regmap-sdw-mbq.c index 8ce30650b97c..6675c3a4b829 100644 --- a/drivers/base/regmap/regmap-sdw-mbq.c +++ b/drivers/base/regmap/regmap-sdw-mbq.c @@ -15,11 +15,11 @@ static int regmap_sdw_mbq_write(void *context, unsigned int reg, unsigned int va struct sdw_slave *slave = dev_to_sdw_dev(dev); int ret; - ret = sdw_write(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff); + ret = sdw_write_no_pm(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff); if (ret < 0) return ret; - return sdw_write(slave, reg, val & 0xff); + return sdw_write_no_pm(slave, reg, val & 0xff); } static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *val) @@ -29,11 +29,11 @@ static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *va int read0; int read1; - read0 = sdw_read(slave, reg); + read0 = sdw_read_no_pm(slave, reg); if (read0 < 0) return read0; - read1 = sdw_read(slave, SDW_SDCA_MBQ_CTL(reg)); + read1 = sdw_read_no_pm(slave, SDW_SDCA_MBQ_CTL(reg)); if (read1 < 0) return read1; -- 2.17.1
[PATCH v2 3/9] soundwire: bus: use no_pm IO routines for all interrupt handling
From: Pierre-Louis Bossart
There is no need to play with pm_runtime reference counts, if needed
the codec drivers are already explicitly resumed.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index b1830032b052..86c339d77a39 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1295,7 +1295,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
u8 clear, impl_int_mask;
int status, status2, ret, count = 0;
- status = sdw_read(slave, SDW_DP0_INT);
+ status = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status);
@@ -1334,7 +1334,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* clear the interrupts but don't touch reserved and
SDCA_CASCADE fields */
- ret = sdw_write(slave, SDW_DP0_INT, clear);
+ ret = sdw_write_no_pm(slave, SDW_DP0_INT, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT write failed:%d\n", ret);
@@ -1342,7 +1342,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* Read DP0 interrupt again */
- status2 = sdw_read(slave, SDW_DP0_INT);
+ status2 = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status2);
@@ -1373,7 +1373,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
return sdw_handle_dp0_interrupt(slave, slave_status);
addr = SDW_DPN_INT(port);
- status = sdw_read(slave, addr);
+ status = sdw_read_no_pm(slave, addr);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status);
@@ -1407,7 +1407,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* clear the interrupt but don't touch reserved fields */
- ret = sdw_write(slave, addr, clear);
+ ret = sdw_write_no_pm(slave, addr, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT write failed:%d\n", ret);
@@ -1415,7 +1415,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* Read DPN interrupt again */
- status2 = sdw_read(slave, addr);
+ status2 = sdw_read_no_pm(slave, addr);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status2);
@@ -1457,7 +1457,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Read Intstat 1, Intstat 2 and Intstat 3 registers */
- ret = sdw_read(slave, SDW_SCP_INT1);
+ ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 read failed:%d\n", ret);
@@ -1465,7 +1465,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
buf = ret;
- ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2);
+ ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, buf2);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT2/3 read failed:%d\n", ret);
@@ -1473,7 +1473,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
if (slave->prop.is_sdca) {
- ret = sdw_read(slave, SDW_DP0_INT);
+ ret = sdw_read_no_pm(slave, SDW_DP0_INT);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", ret);
@@ -1570,7 +1570,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Ack interrupt */
- ret = sdw_write(slave, SDW_SCP_INT1, clear);
+ ret = sdw_write_no_pm(slave, SDW_SCP_INT1, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 write failed:%d\n", ret);
@@ -1584,7 +1584,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
*
[PATCH v2 4/9] soundwire: export sdw_write/read_no_pm functions
sdw_write_no_pm and sdw_read_no_pm are useful when we want to do IO
without touching PM.
Fixes: 0231453bc08f ('soundwire: bus: add clock stop helpers')
Fixes: 60ee9be25571 ('soundwire: bus: add PM/no-PM versions of
read/write functions')
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 7 ---
include/linux/soundwire/sdw.h | 2 ++
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 86c339d77a39..4eaeeb4090f0 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -405,10 +405,11 @@ sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr,
size_t count, u8 *val)
return sdw_transfer(slave->bus, );
}
-static int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
{
return sdw_nwrite_no_pm(slave, addr, 1, );
}
+EXPORT_SYMBOL(sdw_write_no_pm);
static int
sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
@@ -476,8 +477,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
}
EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked);
-static int
-sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
+int sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
{
u8 buf;
int ret;
@@ -488,6 +488,7 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
else
return buf;
}
+EXPORT_SYMBOL(sdw_read_no_pm);
static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
{
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f0b01b728640..d08039d65825 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1005,6 +1005,8 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus);
int sdw_read(struct sdw_slave *slave, u32 addr);
int sdw_write(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_read_no_pm(struct sdw_slave *slave, u32 addr);
int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
--
2.17.1
[PATCH v2 2/9] soundwire: bus: use sdw_write_no_pm when setting the bus scale registers
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
This patch fixes the same problem as the previous one, but is split to
make the life of linux-stable maintainers less painful.
Fixes: 29d158f90690 ('soundwire: bus: initialize bus clock base and scale
registers')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 60c42508c6c6..b1830032b052 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1222,7 +1222,7 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
scale_index++;
- ret = sdw_write(slave, SDW_SCP_BUS_CLOCK_BASE, base);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUS_CLOCK_BASE, base);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUS_CLOCK_BASE write failed:%d\n", ret);
@@ -1230,13 +1230,13 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
/* initialize scale for both banks */
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B0 write failed:%d\n", ret);
return ret;
}
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
if (ret < 0)
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B1 write failed:%d\n", ret);
--
2.17.1
[PATCH v2 1/9] soundwire: bus: use sdw_update_no_pm when initializing a device
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
Fixes: 60ee9be25571 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d1e8c3a54976..60c42508c6c6 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -489,6 +489,18 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
return buf;
}
+static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
+{
+ int tmp;
+
+ tmp = sdw_read_no_pm(slave, addr);
+ if (tmp < 0)
+ return tmp;
+
+ tmp = (tmp & ~mask) | val;
+ return sdw_write_no_pm(slave, addr, tmp);
+}
+
/**
* sdw_nread() - Read "n" contiguous SDW Slave registers
* @slave: SDW Slave
@@ -1256,7 +1268,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = slave->prop.scp_int1_mask;
/* Enable SCP interrupts */
- ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
+ ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
@@ -1271,7 +1283,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = prop->dp0_prop->imp_def_interrupts;
val |= SDW_DP0_INT_PORT_READY | SDW_DP0_INT_BRA_FAILURE;
- ret = sdw_update(slave, SDW_DP0_INTMASK, val, val);
+ ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
dev_err(slave->bus->dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
--
2.17.1
[PATCH v2 0/9] soundwire/regmap: use _no_pm routines
When a Slave device is resumed, it may resume the bus and restart the
enumeration. And Slave drivers will wait for initialization_complete
complete in their resume function, however initialization_complete will
complete after sdw_update_slave_status function is finished and codec
driver usually call some IO functions in the update_status callback
function.
It will become a deadlock if we use regular read/write routines during
the resuming process.
This series touches both soundwire and regmap trees.
commit fb5103f9d6ce ("regmap/SoundWire: sdw: add support for SoundWire 1.2 MBQ")
is needed for soundwire tree to complie.
On the other hands,
commit 6e06a85556f9 ("soundwire: bus: add comments to explain interrupt loop
filter")
to
commit 47b8520997a8 ("soundwire: bus: only clear valid DPN interrupts")
are needed for regmap tree.
v2:
- Separate commits according to maintainer's comments.
Bard Liao (4):
soundwire: export sdw_write/read_no_pm functions
regmap: sdw: use _no_pm functions in regmap_read/write
regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
regmap: sdw-mbq: use MODULE_LICENSE("GPL")
Pierre-Louis Bossart (5):
soundwire: bus: use sdw_update_no_pm when initializing a device
soundwire: bus: use sdw_write_no_pm when setting the bus scale
registers
soundwire: bus: use no_pm IO routines for all interrupt handling
soundwire: bus: fix confusion on device used by pm_runtime
soundwire: bus: clarify dev_err/dbg device references
drivers/base/regmap/regmap-sdw-mbq.c | 10 +-
drivers/base/regmap/regmap-sdw.c | 4 +-
drivers/soundwire/bus.c | 136 +++
include/linux/soundwire/sdw.h| 2 +
4 files changed, 85 insertions(+), 67 deletions(-)
--
2.17.1
[PATCH 6/7] soundwire: bus: fix confusion on device used by pm_runtime
From: Pierre-Louis Bossart
Intel stress-tests routinely report IO timeouts and invalid power
management transitions. Upon further analysis, we seem to be using the
wrong devices in pm_runtime calls.
Before reading and writing registers, we first need to make sure the
Slave is fully resumed. The existing code attempts to do such that,
however because of a confusion dating from 2017 and copy/paste, we
end-up resuming the parent only instead of resuming the codec device.
This can lead to accesses to the Slave registers while the bus is
still being configured and the Slave not enumerated, and as a result
IO errors occur.
This is a classic problem, similar confusions happened for HDaudio
between bus and codec device, leading to power management issues.
Fix by using the relevant device for all uses of pm_runtime functions.
Fixes: 60ee9be255712 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Fixes: aa79293517b39 ('soundwire: bus: fix io error when processing alert
event')
Fixes: 9d715fa005ebc ('soundwire: Add IO transfer')
Reported-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index c5ea59673dee..c317f41eba4e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -514,16 +514,16 @@ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nread_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -540,16 +540,16 @@ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t
count, u8 *val)
{
int ret;
- ret = pm_runtime_get_sync(slave->bus->dev);
+ ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
ret = sdw_nwrite_no_pm(slave, addr, count, val);
- pm_runtime_mark_last_busy(slave->bus->dev);
- pm_runtime_put(slave->bus->dev);
+ pm_runtime_mark_last_busy(>dev);
+ pm_runtime_put(>dev);
return ret;
}
@@ -1454,7 +1454,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = pm_runtime_get_sync(>dev);
if (ret < 0 && ret != -EACCES) {
dev_err(>dev, "Failed to resume device: %d\n", ret);
- pm_runtime_put_noidle(slave->bus->dev);
+ pm_runtime_put_noidle(>dev);
return ret;
}
--
2.17.1
[PATCH 7/7] soundwire: bus: clarify dev_err/dbg device references
From: Pierre-Louis Bossart
The SoundWire bus code confuses bus and Slave device levels for
dev_err/dbg logs. That's not impacting functionality but the accuracy
of kernel logs.
We should only use bus->dev for bus-level operations and handling of
Device0. For all other logs where the device number is not zero, we
should use >dev to provide more precisions to the
user/integrator.
Reported-by: Rander Wang
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 63 +
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index c317f41eba4e..7d07cacef740 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -637,6 +637,7 @@ static int sdw_get_device_num(struct sdw_slave *slave)
static int sdw_assign_device_num(struct sdw_slave *slave)
{
+ struct sdw_bus *bus = slave->bus;
int ret, dev_num;
bool new_device = false;
@@ -647,7 +648,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
dev_num = sdw_get_device_num(slave);
mutex_unlock(>bus->bus_lock);
if (dev_num < 0) {
- dev_err(slave->bus->dev, "Get dev_num failed:
%d\n",
+ dev_err(bus->dev, "Get dev_num failed: %d\n",
dev_num);
return dev_num;
}
@@ -660,7 +661,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
}
if (!new_device)
- dev_dbg(slave->bus->dev,
+ dev_dbg(bus->dev,
"Slave already registered, reusing dev_num:%d\n",
slave->dev_num);
@@ -670,7 +671,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num);
if (ret < 0) {
- dev_err(>dev, "Program device_num %d failed: %d\n",
+ dev_err(bus->dev, "Program device_num %d failed: %d\n",
dev_num, ret);
return ret;
}
@@ -749,7 +750,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
*/
ret = sdw_assign_device_num(slave);
if (ret) {
- dev_err(slave->bus->dev,
+ dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
return ret;
@@ -789,9 +790,11 @@ static int sdw_program_device_num(struct sdw_bus *bus)
static void sdw_modify_slave_status(struct sdw_slave *slave,
enum sdw_slave_status status)
{
- mutex_lock(>bus->bus_lock);
+ struct sdw_bus *bus = slave->bus;
- dev_vdbg(>dev,
+ mutex_lock(>bus_lock);
+
+ dev_vdbg(bus->dev,
"%s: changing status slave %d status %d new status %d\n",
__func__, slave->dev_num, slave->status, status);
@@ -812,7 +815,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
complete(>enumeration_complete);
}
slave->status = status;
- mutex_unlock(>bus->bus_lock);
+ mutex_unlock(>bus_lock);
}
static enum sdw_clk_stop_mode sdw_get_clk_stop_mode(struct sdw_slave *slave)
@@ -1141,7 +1144,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DPN_INTMASK write failed:%d\n", val);
return ret;
@@ -1272,7 +1275,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
/* Enable SCP interrupts */
ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
return ret;
}
@@ -1287,7 +1290,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
- dev_err(slave->bus->dev,
+ dev_err(>dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
return ret;
}
@@ -1299,7 +1302,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave,
[PATCH 5/7] regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
Use no_pm versions for write and read.
Signed-off-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
---
drivers/base/regmap/regmap-sdw-mbq.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw-mbq.c
b/drivers/base/regmap/regmap-sdw-mbq.c
index 8ce30650b97c..fe3ac26b66ad 100644
--- a/drivers/base/regmap/regmap-sdw-mbq.c
+++ b/drivers/base/regmap/regmap-sdw-mbq.c
@@ -15,11 +15,11 @@ static int regmap_sdw_mbq_write(void *context, unsigned int
reg, unsigned int va
struct sdw_slave *slave = dev_to_sdw_dev(dev);
int ret;
- ret = sdw_write(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff);
+ ret = sdw_write_no_pm(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff);
if (ret < 0)
return ret;
- return sdw_write(slave, reg, val & 0xff);
+ return sdw_write_no_pm(slave, reg, val & 0xff);
}
static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int
*val)
@@ -29,11 +29,11 @@ static int regmap_sdw_mbq_read(void *context, unsigned int
reg, unsigned int *va
int read0;
int read1;
- read0 = sdw_read(slave, reg);
+ read0 = sdw_read_no_pm(slave, reg);
if (read0 < 0)
return read0;
- read1 = sdw_read(slave, SDW_SDCA_MBQ_CTL(reg));
+ read1 = sdw_read_no_pm(slave, SDW_SDCA_MBQ_CTL(reg));
if (read1 < 0)
return read1;
@@ -98,4 +98,4 @@ struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave
*sdw,
EXPORT_SYMBOL_GPL(__devm_regmap_init_sdw_mbq);
MODULE_DESCRIPTION("Regmap SoundWire MBQ Module");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
--
2.17.1
[PATCH 4/7] soundwire/regmap: use _no_pm functions in regmap_read/write
sdw_update_slave_status will be invoked when a codec is attached,
and the codec driver will initialize the codec with regmap functions
while the codec device is pm_runtime suspended.
regmap routines currently rely on regular SoundWire IO functions,
which will call pm_runtime_get_sync()/put_autosuspend.
This causes a deadlock where the resume routine waits for an
initialization complete signal that while the initialization complete
can only be reached when the resume completes.
The only solution if we allow regmap functions to be used in resume
operations as well as during codec initialization is to use _no_pm
routines. The duty of making sure the bus is operational needs to be
handled above the regmap level.
Fixes: 7c22ce6e21840 ('regmap: Add SoundWire bus support')
Signed-off-by: Bard Liao
Reviewed-by: Rander Wang
---
drivers/base/regmap/regmap-sdw.c | 4 ++--
drivers/soundwire/bus.c | 6 --
include/linux/soundwire/sdw.h| 2 ++
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c
index c92d614b4943..4b8d2d010cab 100644
--- a/drivers/base/regmap/regmap-sdw.c
+++ b/drivers/base/regmap/regmap-sdw.c
@@ -11,7 +11,7 @@ static int regmap_sdw_write(void *context, unsigned int reg,
unsigned int val)
struct device *dev = context;
struct sdw_slave *slave = dev_to_sdw_dev(dev);
- return sdw_write(slave, reg, val);
+ return sdw_write_no_pm(slave, reg, val);
}
static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val)
@@ -20,7 +20,7 @@ static int regmap_sdw_read(void *context, unsigned int reg,
unsigned int *val)
struct sdw_slave *slave = dev_to_sdw_dev(dev);
int read;
- read = sdw_read(slave, reg);
+ read = sdw_read_no_pm(slave, reg);
if (read < 0)
return read;
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 86c339d77a39..c5ea59673dee 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -405,10 +405,11 @@ sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr,
size_t count, u8 *val)
return sdw_transfer(slave->bus, );
}
-static int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value)
{
return sdw_nwrite_no_pm(slave, addr, 1, );
}
+EXPORT_SYMBOL(sdw_write_no_pm);
static int
sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
@@ -476,7 +477,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
}
EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked);
-static int
+int
sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
{
u8 buf;
@@ -488,6 +489,7 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
else
return buf;
}
+EXPORT_SYMBOL(sdw_read_no_pm);
static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
{
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f0b01b728640..d08039d65825 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -1005,6 +1005,8 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus);
int sdw_read(struct sdw_slave *slave, u32 addr);
int sdw_write(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value);
+int sdw_read_no_pm(struct sdw_slave *slave, u32 addr);
int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val);
--
2.17.1
[PATCH 3/7] soundwire: bus: use no_pm IO routines for all interrupt handling
From: Pierre-Louis Bossart
There is no need to play with pm_runtime reference counts, if needed
the codec drivers are already explicitly resumed.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index b1830032b052..86c339d77a39 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1295,7 +1295,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
u8 clear, impl_int_mask;
int status, status2, ret, count = 0;
- status = sdw_read(slave, SDW_DP0_INT);
+ status = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status);
@@ -1334,7 +1334,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* clear the interrupts but don't touch reserved and
SDCA_CASCADE fields */
- ret = sdw_write(slave, SDW_DP0_INT, clear);
+ ret = sdw_write_no_pm(slave, SDW_DP0_INT, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT write failed:%d\n", ret);
@@ -1342,7 +1342,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
/* Read DP0 interrupt again */
- status2 = sdw_read(slave, SDW_DP0_INT);
+ status2 = sdw_read_no_pm(slave, SDW_DP0_INT);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", status2);
@@ -1373,7 +1373,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
return sdw_handle_dp0_interrupt(slave, slave_status);
addr = SDW_DPN_INT(port);
- status = sdw_read(slave, addr);
+ status = sdw_read_no_pm(slave, addr);
if (status < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status);
@@ -1407,7 +1407,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* clear the interrupt but don't touch reserved fields */
- ret = sdw_write(slave, addr, clear);
+ ret = sdw_write_no_pm(slave, addr, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT write failed:%d\n", ret);
@@ -1415,7 +1415,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave
*slave,
}
/* Read DPN interrupt again */
- status2 = sdw_read(slave, addr);
+ status2 = sdw_read_no_pm(slave, addr);
if (status2 < 0) {
dev_err(slave->bus->dev,
"SDW_DPN_INT read failed:%d\n", status2);
@@ -1457,7 +1457,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Read Intstat 1, Intstat 2 and Intstat 3 registers */
- ret = sdw_read(slave, SDW_SCP_INT1);
+ ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 read failed:%d\n", ret);
@@ -1465,7 +1465,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
buf = ret;
- ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2);
+ ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, buf2);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT2/3 read failed:%d\n", ret);
@@ -1473,7 +1473,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
if (slave->prop.is_sdca) {
- ret = sdw_read(slave, SDW_DP0_INT);
+ ret = sdw_read_no_pm(slave, SDW_DP0_INT);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_DP0_INT read failed:%d\n", ret);
@@ -1570,7 +1570,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
}
/* Ack interrupt */
- ret = sdw_write(slave, SDW_SCP_INT1, clear);
+ ret = sdw_write_no_pm(slave, SDW_SCP_INT1, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INT1 write failed:%d\n", ret);
@@ -1584,7 +1584,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
*
[PATCH 0/7] soundwire/regmap: use _no_pm routines
When a Slave device is resumed, it may resume the bus and restart the
enumeration. And Slave drivers will wait for initialization_complete
complete in their resume function, however initialization_complete will
complete after sdw_update_slave_status function is finished and codec
driver usually call some IO functions in the update_status callback
function.
It will become a deadlock if we use regular read/write routines during
the resuming process.
This series touches both soundwire and regmap trees.
commit fb5103f9d6ce ("regmap/SoundWire: sdw: add support for SoundWire 1.2 MBQ")
is needed for soundwire tree to complie.
On the other hands,
commit 6e06a85556f9 ("soundwire: bus: add comments to explain interrupt loop
filter")
to
commit 47b8520997a8 ("soundwire: bus: only clear valid DPN interrupts")
are needed for regmap tree.
Bard Liao (2):
soundwire/regmap: use _no_pm functions in regmap_read/write
regmap: sdw: use no_pm routines for SoundWire 1.2 MBQ
Pierre-Louis Bossart (5):
soundwire: bus: use sdw_update_no_pm when initializing a device
soundwire: bus: use sdw_write_no_pm when setting the bus scale
registers
soundwire: bus: use no_pm IO routines for all interrupt handling
soundwire: bus: fix confusion on device used by pm_runtime
soundwire: bus: clarify dev_err/dbg device references
drivers/base/regmap/regmap-sdw-mbq.c | 10 +-
drivers/base/regmap/regmap-sdw.c | 4 +-
drivers/soundwire/bus.c | 135 +++
include/linux/soundwire/sdw.h| 2 +
4 files changed, 85 insertions(+), 66 deletions(-)
--
2.17.1
[PATCH 2/7] soundwire: bus: use sdw_write_no_pm when setting the bus scale registers
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
This patch fixes the same problem as the previous one, but is split to
make the life of linux-stable maintainers less painful.
Fixes: 29d158f90690 ('soundwire: bus: initialize bus clock base and scale
registers')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 60c42508c6c6..b1830032b052 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1222,7 +1222,7 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
scale_index++;
- ret = sdw_write(slave, SDW_SCP_BUS_CLOCK_BASE, base);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUS_CLOCK_BASE, base);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUS_CLOCK_BASE write failed:%d\n", ret);
@@ -1230,13 +1230,13 @@ static int sdw_slave_set_frequency(struct sdw_slave
*slave)
}
/* initialize scale for both banks */
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B0, scale_index);
if (ret < 0) {
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B0 write failed:%d\n", ret);
return ret;
}
- ret = sdw_write(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
+ ret = sdw_write_no_pm(slave, SDW_SCP_BUSCLOCK_SCALE_B1, scale_index);
if (ret < 0)
dev_err(>dev,
"SDW_SCP_BUSCLOCK_SCALE_B1 write failed:%d\n", ret);
--
2.17.1
[PATCH 1/7] soundwire: bus: use sdw_update_no_pm when initializing a device
From: Pierre-Louis Bossart
When a Slave device is resumed, it may resume the bus and restart the
enumeration. During that process, we absolutely don't want to call
regular read/write routines which will wait for the resume to
complete, otherwise a deadlock occurs.
Fixes: 60ee9be25571 ('soundwire: bus: add PM/no-PM versions of read/write
functions')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d1e8c3a54976..60c42508c6c6 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -489,6 +489,18 @@ sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
return buf;
}
+static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val)
+{
+ int tmp;
+
+ tmp = sdw_read_no_pm(slave, addr);
+ if (tmp < 0)
+ return tmp;
+
+ tmp = (tmp & ~mask) | val;
+ return sdw_write_no_pm(slave, addr, tmp);
+}
+
/**
* sdw_nread() - Read "n" contiguous SDW Slave registers
* @slave: SDW Slave
@@ -1256,7 +1268,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = slave->prop.scp_int1_mask;
/* Enable SCP interrupts */
- ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
+ ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, val, val);
if (ret < 0) {
dev_err(slave->bus->dev,
"SDW_SCP_INTMASK1 write failed:%d\n", ret);
@@ -1271,7 +1283,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
val = prop->dp0_prop->imp_def_interrupts;
val |= SDW_DP0_INT_PORT_READY | SDW_DP0_INT_BRA_FAILURE;
- ret = sdw_update(slave, SDW_DP0_INTMASK, val, val);
+ ret = sdw_update_no_pm(slave, SDW_DP0_INTMASK, val, val);
if (ret < 0)
dev_err(slave->bus->dev,
"SDW_DP0_INTMASK read failed:%d\n", ret);
--
2.17.1
[PATCH v2 5/5] ASoC/SoundWire: rt711-sdca: Add RT711 SDCA vendor-specific driver
From: Shuming Fan
This is the initial codec driver for rt711 SDCA version.
Signed-off-by: Shuming Fan
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
sound/soc/codecs/Kconfig |7 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/rt711-sdca-sdw.c | 424 +
sound/soc/codecs/rt711-sdca-sdw.h | 101 ++
sound/soc/codecs/rt711-sdca.c | 1482 +
sound/soc/codecs/rt711-sdca.h | 246 +
6 files changed, 2262 insertions(+)
create mode 100644 sound/soc/codecs/rt711-sdca-sdw.c
create mode 100644 sound/soc/codecs/rt711-sdca-sdw.h
create mode 100644 sound/soc/codecs/rt711-sdca.c
create mode 100644 sound/soc/codecs/rt711-sdca.h
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index ebc124142f90..a9064f279d33 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -176,6 +176,7 @@ config SND_SOC_ALL_CODECS
imply SND_SOC_RT5682_SDW
imply SND_SOC_RT700_SDW
imply SND_SOC_RT711_SDW
+ imply SND_SOC_RT711_SDCA_SDW
imply SND_SOC_RT715_SDW
imply SND_SOC_RT715_SDCA_SDW
imply SND_SOC_RT1308_SDW
@@ -1214,6 +1215,12 @@ config SND_SOC_RT711_SDW
select SND_SOC_RT711
select REGMAP_SOUNDWIRE
+config SND_SOC_RT711_SDCA_SDW
+ tristate "Realtek RT711 SDCA Codec - SDW"
+ depends on SOUNDWIRE
+ select REGMAP_SOUNDWIRE
+ select REGMAP_SOUNDWIRE_MBQ
+
config SND_SOC_RT715
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 601bbb8b46e7..8bdd587bbd3b 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -188,6 +188,7 @@ snd-soc-rt5682-sdw-objs := rt5682-sdw.o
snd-soc-rt5682-i2c-objs := rt5682-i2c.o
snd-soc-rt700-objs := rt700.o rt700-sdw.o
snd-soc-rt711-objs := rt711.o rt711-sdw.o
+snd-soc-rt711-sdca-objs := rt711-sdca.o rt711-sdca-sdw.o
snd-soc-rt715-objs := rt715.o rt715-sdw.o
snd-soc-rt715-sdca-objs := rt715-sdca.o rt715-sdca-sdw.o
snd-soc-sgtl5000-objs := sgtl5000.o
@@ -500,6 +501,7 @@ obj-$(CONFIG_SND_SOC_RT5682_I2C)+= snd-soc-rt5682-i2c.o
obj-$(CONFIG_SND_SOC_RT5682_SDW) += snd-soc-rt5682-sdw.o
obj-$(CONFIG_SND_SOC_RT700) += snd-soc-rt700.o
obj-$(CONFIG_SND_SOC_RT711) += snd-soc-rt711.o
+obj-$(CONFIG_SND_SOC_RT711_SDCA_SDW) += snd-soc-rt711-sdca.o
obj-$(CONFIG_SND_SOC_RT715) += snd-soc-rt715.o
obj-$(CONFIG_SND_SOC_RT715_SDCA_SDW) += snd-soc-rt715-sdca.o
obj-$(CONFIG_SND_SOC_SGTL5000) += snd-soc-sgtl5000.o
diff --git a/sound/soc/codecs/rt711-sdca-sdw.c
b/sound/soc/codecs/rt711-sdca-sdw.c
new file mode 100644
index ..6aaf9e09c118
--- /dev/null
+++ b/sound/soc/codecs/rt711-sdca-sdw.c
@@ -0,0 +1,424 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt711-sdw-sdca.c -- rt711 SDCA ALSA SoC audio driver
+//
+// Copyright(c) 2020 Realtek Semiconductor Corp.
+//
+//
+
+#include
+#include
+#include
+#include
+#include
+
+#include "rt711-sdca.h"
+#include "rt711-sdca-sdw.h"
+
+static bool rt711_sdca_readable_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case SDW_DP0_INT:
+ case SDW_SCP_SDCA_INT1 ... SDW_SCP_SDCA_INTMASK4:
+ case 0x201a ... 0x2027:
+ case 0x2029 ... 0x202a:
+ case 0x202d ... 0x2034:
+ case 0x2200 ... 0x2204:
+ case 0x2206 ... 0x2212:
+ case 0x2220 ... 0x2223:
+ case 0x2230 ... 0x2239:
+ case 0x2f01 ... 0x2f0f:
+ case 0x2f30 ... 0x2f36:
+ case 0x2f50 ... 0x2f5a:
+ case 0x2f60:
+ case 0x3200 ... 0x3212:
+ case SDW_SDCA_CTL(FUN_JACK_CODEC, RT711_SDCA_ENT_GE49,
RT711_SDCA_CTL_SELECTED_MODE, 0):
+ case SDW_SDCA_CTL(FUN_JACK_CODEC, RT711_SDCA_ENT_GE49,
RT711_SDCA_CTL_DETECTED_MODE, 0):
+ case SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_CURRENT_OWNER, 0) ...
+ SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_MESSAGE_LENGTH, 0):
+ case RT711_BUF_ADDR_HID1 ... RT711_BUF_ADDR_HID2:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool rt711_sdca_volatile_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case SDW_DP0_INT:
+ case SDW_SCP_SDCA_INT1 ... SDW_SCP_SDCA_INT4:
+ case 0x201b:
+ case 0x201c:
+ case 0x201d:
+ case 0x201f:
+ case 0x2021:
+ case 0x2023:
+ case 0x2230:
+ case 0x202d ... 0x202f: /* BRA */
+ case 0x2200 ... 0x2212: /* i2c debug */
+ case RT711_RC_CAL_STATUS:
+ case SDW_SDCA_CTL(FUN_JACK_CODEC, RT711_SDCA_ENT_GE49,
RT711_SDCA_CTL_DETECTED_MODE, 0):
+ case SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_CURRENT_OWNER, 0) ...
+ SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_MESSAGE_LENGTH, 0):
+ case RT711_BUF_ADDR_HID1 ... RT711_BUF_ADDR_HID
[PATCH v2 4/5] ASoC/SoundWire: rt1316: Add RT1316 SDCA vendor-specific driver
From: Shuming Fan
This is the initial amplifier driver for rt1316 SDCA version.
Signed-off-by: Shuming Fan
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
sound/soc/codecs/Kconfig | 6 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/rt1316-sdw.c | 756 ++
sound/soc/codecs/rt1316-sdw.h | 115 ++
4 files changed, 879 insertions(+)
create mode 100644 sound/soc/codecs/rt1316-sdw.c
create mode 100644 sound/soc/codecs/rt1316-sdw.h
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index e7797f08e057..ebc124142f90 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -179,6 +179,7 @@ config SND_SOC_ALL_CODECS
imply SND_SOC_RT715_SDW
imply SND_SOC_RT715_SDCA_SDW
imply SND_SOC_RT1308_SDW
+ imply SND_SOC_RT1316_SDW
imply SND_SOC_SGTL5000
imply SND_SOC_SI476X
imply SND_SOC_SIMPLE_AMPLIFIER
@@ -1110,6 +,11 @@ config SND_SOC_RT1308_SDW
depends on I2C && SOUNDWIRE
select REGMAP_SOUNDWIRE
+config SND_SOC_RT1316_SDW
+ tristate "Realtek RT1316 Codec - SDW"
+ depends on SOUNDWIRE
+ select REGMAP_SOUNDWIRE
+
config SND_SOC_RT5514
tristate
depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index b1683403afb3..601bbb8b46e7 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -164,6 +164,7 @@ snd-soc-rt1015p-objs := rt1015p.o
snd-soc-rt1305-objs := rt1305.o
snd-soc-rt1308-objs := rt1308.o
snd-soc-rt1308-sdw-objs := rt1308-sdw.o
+snd-soc-rt1316-sdw-objs := rt1316-sdw.o
snd-soc-rt274-objs := rt274.o
snd-soc-rt286-objs := rt286.o
snd-soc-rt298-objs := rt298.o
@@ -474,6 +475,7 @@ obj-$(CONFIG_SND_SOC_RT1015P) += snd-soc-rt1015p.o
obj-$(CONFIG_SND_SOC_RT1305) += snd-soc-rt1305.o
obj-$(CONFIG_SND_SOC_RT1308) += snd-soc-rt1308.o
obj-$(CONFIG_SND_SOC_RT1308_SDW) += snd-soc-rt1308-sdw.o
+obj-$(CONFIG_SND_SOC_RT1316_SDW) += snd-soc-rt1316-sdw.o
obj-$(CONFIG_SND_SOC_RT274)+= snd-soc-rt274.o
obj-$(CONFIG_SND_SOC_RT286)+= snd-soc-rt286.o
obj-$(CONFIG_SND_SOC_RT298)+= snd-soc-rt298.o
diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c
new file mode 100644
index ..145ffb8cd1ca
--- /dev/null
+++ b/sound/soc/codecs/rt1316-sdw.c
@@ -0,0 +1,756 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt1316-sdw.c -- rt1316 SDCA ALSA SoC amplifier audio driver
+//
+// Copyright(c) 2020 Realtek Semiconductor Corp.
+//
+//
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "rt1316-sdw.h"
+
+static bool rt1316_readable_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case 0x2f0a:
+ case 0x2f36:
+ case 0x3203 ... 0x320e:
+ case 0xc000 ... 0xc7b4:
+ case 0xcf00 ... 0xcf03:
+ case 0xd101 ... 0xd103:
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_UDMPU21,
RT1316_SDCA_CTL_UDMPU_CLUSTER, 0):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_FU21,
RT1316_SDCA_CTL_FU_MUTE, CH_L):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_FU21,
RT1316_SDCA_CTL_FU_MUTE, CH_R):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_PDE23,
RT1316_SDCA_CTL_REQ_POWER_STATE, 0):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_PDE27,
RT1316_SDCA_CTL_REQ_POWER_STATE, 0):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_PDE22,
RT1316_SDCA_CTL_REQ_POWER_STATE, 0):
+ case SDW_SDCA_CTL(FUN_SMART_AMP, RT1316_SDCA_ENT_PDE24,
RT1316_SDCA_CTL_REQ_POWER_STATE, 0):
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool rt1316_volatile_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case 0xc000:
+ case 0xc093:
+ case 0xc09d:
+ case 0xc0a3:
+ case 0xc201:
+ case 0xc427 ... 0xc428:
+ case 0xd102:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct regmap_config rt1316_sdw_regmap = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .readable_reg = rt1316_readable_register,
+ .volatile_reg = rt1316_volatile_register,
+ .max_register = 0x4108,
+ .reg_defaults = rt1316_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(rt1316_reg_defaults),
+ .cache_type = REGCACHE_RBTREE,
+ .use_single_read = true,
+ .use_single_write = true,
+};
+
+static int rt1316_read_prop(struct sdw_slave *slave)
+{
+ struct sdw_slave_prop *prop = >prop;
+ int nval;
+ int i, j;
+ u32 bit;
+ unsigned long addr;
+ struct sdw_dpn_prop *dpn;
+
+ prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
+ prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY;
+
[PATCH v2 3/5] ASoC/SoundWire: rt715-sdca: First version of rt715 sdw sdca codec driver
From: Jack Yu
First version of rt715 sdw sdca codec driver.
Signed-off-by: Jack Yu
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
sound/soc/codecs/Kconfig | 7 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/rt715-sdca-sdw.c | 278 +
sound/soc/codecs/rt715-sdca-sdw.h | 170 ++
sound/soc/codecs/rt715-sdca.c | 947 ++
sound/soc/codecs/rt715-sdca.h | 124
6 files changed, 1528 insertions(+)
create mode 100644 sound/soc/codecs/rt715-sdca-sdw.c
create mode 100644 sound/soc/codecs/rt715-sdca-sdw.h
create mode 100644 sound/soc/codecs/rt715-sdca.c
create mode 100644 sound/soc/codecs/rt715-sdca.h
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 34c6dd04b85a..e7797f08e057 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -177,6 +177,7 @@ config SND_SOC_ALL_CODECS
imply SND_SOC_RT700_SDW
imply SND_SOC_RT711_SDW
imply SND_SOC_RT715_SDW
+ imply SND_SOC_RT715_SDCA_SDW
imply SND_SOC_RT1308_SDW
imply SND_SOC_SGTL5000
imply SND_SOC_SI476X
@@ -1216,6 +1217,12 @@ config SND_SOC_RT715_SDW
select SND_SOC_RT715
select REGMAP_SOUNDWIRE
+config SND_SOC_RT715_SDCA_SDW
+ tristate "Realtek RT715 SDCA Codec - SDW"
+ depends on SOUNDWIRE
+ select REGMAP_SOUNDWIRE
+ select REGMAP_SOUNDWIRE_MBQ
+
#Freescale sgtl5000 codec
config SND_SOC_SGTL5000
tristate "Freescale SGTL5000 CODEC"
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 11ce98c25d6c..b1683403afb3 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -188,6 +188,7 @@ snd-soc-rt5682-i2c-objs := rt5682-i2c.o
snd-soc-rt700-objs := rt700.o rt700-sdw.o
snd-soc-rt711-objs := rt711.o rt711-sdw.o
snd-soc-rt715-objs := rt715.o rt715-sdw.o
+snd-soc-rt715-sdca-objs := rt715-sdca.o rt715-sdca-sdw.o
snd-soc-sgtl5000-objs := sgtl5000.o
snd-soc-alc5623-objs := alc5623.o
snd-soc-alc5632-objs := alc5632.o
@@ -498,6 +499,7 @@ obj-$(CONFIG_SND_SOC_RT5682_SDW)+= snd-soc-rt5682-sdw.o
obj-$(CONFIG_SND_SOC_RT700) += snd-soc-rt700.o
obj-$(CONFIG_SND_SOC_RT711) += snd-soc-rt711.o
obj-$(CONFIG_SND_SOC_RT715) += snd-soc-rt715.o
+obj-$(CONFIG_SND_SOC_RT715_SDCA_SDW) += snd-soc-rt715-sdca.o
obj-$(CONFIG_SND_SOC_SGTL5000) += snd-soc-sgtl5000.o
obj-$(CONFIG_SND_SOC_SIGMADSP) += snd-soc-sigmadsp.o
obj-$(CONFIG_SND_SOC_SIGMADSP_I2C) += snd-soc-sigmadsp-i2c.o
diff --git a/sound/soc/codecs/rt715-sdca-sdw.c
b/sound/soc/codecs/rt715-sdca-sdw.c
new file mode 100644
index ..e73a826ee8e3
--- /dev/null
+++ b/sound/soc/codecs/rt715-sdca-sdw.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt715-sdca-sdw.c -- rt715 ALSA SoC audio driver
+//
+// Copyright(c) 2020 Realtek Semiconductor Corp.
+//
+//
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "rt715-sdca.h"
+#include "rt715-sdca-sdw.h"
+
+static bool rt715_sdca_readable_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case 0x201a ... 0x2027:
+ case 0x2029 ... 0x202a:
+ case 0x202d ... 0x2034:
+ case 0x2200 ... 0x2204:
+ case 0x2206 ... 0x2212:
+ case 0x2230 ... 0x2239:
+ case 0x2f5b:
+ case SDW_SDCA_CTL(FUN_MIC_ARRAY, RT715_SDCA_SMPU_TRIG_ST_EN,
+ RT715_SDCA_SMPU_TRIG_ST_CTRL, CH_00):
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool rt715_sdca_volatile_register(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case 0x201b:
+ case 0x201c:
+ case 0x201d:
+ case 0x201f:
+ case 0x2021:
+ case 0x2023:
+ case 0x2230:
+ case 0x202d ... 0x202f: /* BRA */
+ case 0x2200 ... 0x2212: /* i2c debug */
+ case 0x2f07:
+ case 0x2f1b ... 0x2f1e:
+ case 0x2f30 ... 0x2f34:
+ case 0x2f50 ... 0x2f51:
+ case 0x2f53 ... 0x2f59:
+ case 0x2f5c ... 0x2f5f:
+ case SDW_SDCA_CTL(FUN_MIC_ARRAY, RT715_SDCA_SMPU_TRIG_ST_EN,
+ RT715_SDCA_SMPU_TRIG_ST_CTRL, CH_00): /* VAD Searching status */
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool rt715_sdca_mbq_readable_register(struct device *dev, unsigned int
reg)
+{
+ switch (reg) {
+ case 0x200:
+ case 0x22b:
+ case 0x236:
+ case 0x237:
+ case 0x239:
+ case 0x610:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool rt715_sdca_mbq_volatile_register(struct device *dev, unsigned int
reg)
+{
+ switch (reg) {
+ case 0x200:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static cons
[PATCH v2 1/5] soundwire: SDCA: add helper macro to access controls
From: Pierre-Louis Bossart The upcoming SDCA (SoundWire Device Class Audio) specification defines a hierarchical encoding to interface with Class-defined capabilities. The specification is not yet accessible to the general public but this information is released with explicit permission from the MIPI Board to avoid delays with SDCA support on Linux platforms. A block of 64 MBytes of register addresses are allocated to SDCA controls, starting at address 0x4000. The 26 LSBs which identify individual controls are set based on the following variables: - Function Number. An SCDA device can be split in up to 8 independent Functions. Each of these Functions is described in the SDCA specification, e.g. Smart Amplifier, Smart Microphone, Simple Microphone, Jack codec, HID, etc. - Entity Number. Within each Function, an Entity is an identifiable block. Up to 127 Entities are connected in a pre-defined graph (similar to USB), with Entity0 reserved for Function-level configurations. In contrast to USB, the SDCA spec pre-defines Function Types, topologies, and allowed options, i.e. the degree of freedom is not unlimited to limit the possibility of errors in descriptors leading to software quirks. - Control Selector. Within each Entity, the SDCA specification defines 48 controls such as Mute, Gain, AGC, etc, and 16 implementation defined ones. Some Control Selectors might be used for low-level platform setup, and other exposed to applications and users. Note that the same Control Selector capability, e.g. Latency control, might be located at different offsets in different entities, the Control Selector mapping is Entity-specific. - Control Number. Some Control Selectors allow channel-specific values to be set, with up to 64 channels allowed. This is mostly used for volume control. - Current/Next values. Some Control Selectors are 'Dual-Ranked'. Software may either update the Current value directly for immediate effect. Alternatively, software may write into the 'Next' values and update the SoundWire 1.2 'Commit Groups' register to copy 'Next' values into 'Current' ones in a synchronized manner. This is different from bank switching which is typically used to change the bus configuration only. - MBQ. the Multi-Byte Quantity bit is used to provide atomic updates when accessing more that one byte, for example a 16-bit volume control would be updated consistently, the intermediate values mixing old MSB with new LSB are not applied. These 6 parameters are used to build a 32-bit address to access the desired Controls. Because of address range, paging is required, but the most often used parameter values are placed in the lower 16 bits of the address. This helps to keep the paging registers constant while updating Controls for a specific Device/Function. Reviewed-by: Rander Wang Reviewed-by: Guennadi Liakhovetski Reviewed-by: Kai Vehmanen Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- include/linux/soundwire/sdw_registers.h | 32 + 1 file changed, 32 insertions(+) diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h index 0cb1a22685b8..138bec908c40 100644 --- a/include/linux/soundwire/sdw_registers.h +++ b/include/linux/soundwire/sdw_registers.h @@ -309,4 +309,36 @@ #define SDW_CASC_PORT_MASK_INTSTAT31 #define SDW_CASC_PORT_REG_OFFSET_INTSTAT3 2 +/* + * v1.2 device - SDCA address mapping + * + * Spec definition + * BitsContents + * 31 0 (required by addressing range) + * 30:26 0b1 (Control Prefix) + * 25 0 (Reserved) + * 24:22 Function Number [2:0] + * 21 Entity[6] + * 20:19 Control Selector[5:4] + * 18 0 (Reserved) + * 17:15 Control Number[5:3] + * 14 Next + * 13 MBQ + * 12:7Entity[5:0] + * 6:3 Control Selector[3:0] + * 2:0 Control Number[2:0] + */ + +#define SDW_SDCA_CTL(fun, ent, ctl, ch)(BIT(30) | \ +(((fun) & 0x7) << 22) | \ +(((ent) & 0x40) << 15) | \ +(((ent) & 0x3f) << 7) | \ +(((ctl) & 0x30) << 15) | \ +(((ctl) & 0x0f) << 3) | \ +(((ch) & 0x38) << 12) | \ +((ch) & 0x07)) + +#define SDW_SDCA_MBQ_CTL(reg) ((reg) | BIT(13)) +#define SDW_SDCA_NEXT_CTL(reg) ((reg) | BIT(14)) + #endif /* __SDW_REGISTERS_H */ -- 2.17.1
[PATCH v2 2/5] regmap/SoundWire: sdw: add support for SoundWire 1.2 MBQ
From: Pierre-Louis Bossart
The SoundWire 1.1 specification only allowed for reads and writes of
bytes. The SoundWire 1.2 specification adds a new capability to
transfer "Multi-Byte Quantities" (MBQ) across the bus. The transfers
still happens one-byte-at-a-time, but the update is atomic.
For example when writing a 16-bit volume, the first byte transferred
is only taken into account when the second byte is successfully
transferred.
The mechanism is symmetrical for read and writes:
- On a read, the address of the last byte to be read is modified by
setting the MBQ bit
- On a write, the address of all but the last byte to be written are
modified by setting the MBQ bit. The address for the last byte relies
on the MBQ bit being cleared.
The current definitions for MBQ-based controls in the SDCA draft
standard are limited to 16 bits for volumes, so for now this is the
only supported format. An update will be provided if and when support
for 24-bit and 32-bit values is specified by the SDCA standard.
One possible objection is that this code could have been handled with
regmap-sdw.c. However this is a new spec addition not handled by every
SoundWire 1.1 and non-SDCA device, so there's no reason to load code
that will never be used.
Also in practice it's extremely unlikely that CONFIG_REGMAP would not
be selected with CONFIG_REGMAP_MBQ selected. However there's no
functional dependency between the two modules so they can be selected
separately.
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Kai Vehmanen
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/base/regmap/Kconfig | 6 +-
drivers/base/regmap/Makefile | 1 +
drivers/base/regmap/regmap-sdw-mbq.c | 101 +++
include/linux/regmap.h | 35 ++
4 files changed, 142 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/regmap/regmap-sdw-mbq.c
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index bcb90d8c3960..50b1e2d06a25 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -4,7 +4,7 @@
# subsystems should select the appropriate symbols.
config REGMAP
- default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 ||
REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE || REGMAP_SCCB ||
REGMAP_I3C || REGMAP_SPI_AVMM)
+ default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 ||
REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE ||
REGMAP_SOUNDWIRE_MBQ || REGMAP_SCCB || REGMAP_I3C || REGMAP_SPI_AVMM)
select IRQ_DOMAIN if REGMAP_IRQ
bool
@@ -46,6 +46,10 @@ config REGMAP_SOUNDWIRE
tristate
depends on SOUNDWIRE
+config REGMAP_SOUNDWIRE_MBQ
+ tristate
+ depends on SOUNDWIRE
+
config REGMAP_SCCB
tristate
depends on I2C
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index ac1b69ee4051..33f63adb5b3d 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
+obj-$(CONFIG_REGMAP_SOUNDWIRE_MBQ) += regmap-sdw-mbq.o
obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
diff --git a/drivers/base/regmap/regmap-sdw-mbq.c
b/drivers/base/regmap/regmap-sdw-mbq.c
new file mode 100644
index ..8ce30650b97c
--- /dev/null
+++ b/drivers/base/regmap/regmap-sdw-mbq.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright(c) 2020 Intel Corporation.
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include "internal.h"
+
+static int regmap_sdw_mbq_write(void *context, unsigned int reg, unsigned int
val)
+{
+ struct device *dev = context;
+ struct sdw_slave *slave = dev_to_sdw_dev(dev);
+ int ret;
+
+ ret = sdw_write(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff);
+ if (ret < 0)
+ return ret;
+
+ return sdw_write(slave, reg, val & 0xff);
+}
+
+static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int
*val)
+{
+ struct device *dev = context;
+ struct sdw_slave *slave = dev_to_sdw_dev(dev);
+ int read0;
+ int read1;
+
+ read0 = sdw_read(slave, reg);
+ if (read0 < 0)
+ return read0;
+
+ read1 = sdw_read(slave, SDW_SDCA_MBQ_CTL(reg));
+ if (read1 < 0)
+ return read1;
+
+ *val = (read1 << 8) | read0;
+
+ return 0;
+}
+
+static struct regmap_bus regmap_sdw_mbq = {
+ .reg_read = regmap_sdw_mbq_read,
+ .reg_write = regmap_sdw_mbq_write,
+ .reg_format_endian_default = REGMAP_ENDIAN_LITTLE,
+ .val_format_endian_
[PATCH v2 0/5] regmap/SoundWire/ASoC: Add SoundWire SDCA support
The MIPI SoundWire Device Class standard will define audio functionality beyond the scope of the existing SoundWire 1.2 standard, which is limited to the bus and interface. The description is inspired by the USB Audio Class, with "functions", "entities", "control selectors", "audio clusters". The main difference with the USB Audio class is that the devices are typically on a motherboard and descriptors stored in platform firmware instead of being retrieved from the device. The current set of devices managed in this patchset are conformant with the SDCA 0.6 specification and require dedicated drivers since the descriptors and platform firmware specification is not complete at this time. They do however rely on the hierarchical addressing required by the SDCA standard. Future devices conformant with SDCA 1.0 should rely on a class driver. This series adds support for the hierarchical SDCA addressing and extends regmap. It then provides 3 codecs for RT711-sdca headset codec, RT1316 amplifier and RT715-scda microphone codec. Note that the release of this code before the formal adoption of the SDCA 1.0 specification was formally endorsed by the MIPI Board to make sure there is no delay for Linux-based support of this specification. v2: - rt715-sdca: Use rt715_sdca prefix to avoid compiling issue. - rt715-sdca: Merge multiple mute/volume operation into single mute/volume operation - rt711-sdca: Initial ret = 0 as it could be used uninitialized. Jack Yu (1): ASoC/SoundWire: rt715-sdca: First version of rt715 sdw sdca codec driver Pierre-Louis Bossart (2): soundwire: SDCA: add helper macro to access controls regmap/SoundWire: sdw: add support for SoundWire 1.2 MBQ Shuming Fan (2): ASoC/SoundWire: rt1316: Add RT1316 SDCA vendor-specific driver ASoC/SoundWire: rt711-sdca: Add RT711 SDCA vendor-specific driver drivers/base/regmap/Kconfig |6 +- drivers/base/regmap/Makefile|1 + drivers/base/regmap/regmap-sdw-mbq.c| 101 ++ include/linux/regmap.h | 35 + include/linux/soundwire/sdw_registers.h | 32 + sound/soc/codecs/Kconfig| 20 + sound/soc/codecs/Makefile |6 + sound/soc/codecs/rt1316-sdw.c | 756 sound/soc/codecs/rt1316-sdw.h | 115 ++ sound/soc/codecs/rt711-sdca-sdw.c | 424 +++ sound/soc/codecs/rt711-sdca-sdw.h | 101 ++ sound/soc/codecs/rt711-sdca.c | 1482 +++ sound/soc/codecs/rt711-sdca.h | 246 sound/soc/codecs/rt715-sdca-sdw.c | 278 + sound/soc/codecs/rt715-sdca-sdw.h | 170 +++ sound/soc/codecs/rt715-sdca.c | 947 +++ sound/soc/codecs/rt715-sdca.h | 124 ++ 17 files changed, 4843 insertions(+), 1 deletion(-) create mode 100644 drivers/base/regmap/regmap-sdw-mbq.c create mode 100644 sound/soc/codecs/rt1316-sdw.c create mode 100644 sound/soc/codecs/rt1316-sdw.h create mode 100644 sound/soc/codecs/rt711-sdca-sdw.c create mode 100644 sound/soc/codecs/rt711-sdca-sdw.h create mode 100644 sound/soc/codecs/rt711-sdca.c create mode 100644 sound/soc/codecs/rt711-sdca.h create mode 100644 sound/soc/codecs/rt715-sdca-sdw.c create mode 100644 sound/soc/codecs/rt715-sdca-sdw.h create mode 100644 sound/soc/codecs/rt715-sdca.c create mode 100644 sound/soc/codecs/rt715-sdca.h -- 2.17.1
[PATCH] regmap: sdw: add required header files
From: Pierre-Louis Bossart Explicitly add header files used by regmap SoundWire support. Suggested-by: Guennadi Liakhovetski Reviewed-by: Rander Wang Reviewed-by: Guennadi Liakhovetski Reviewed-by: Kai Vehmanen Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/base/regmap/regmap-sdw.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c index c92d614b4943..c83be26434e7 100644 --- a/drivers/base/regmap/regmap-sdw.c +++ b/drivers/base/regmap/regmap-sdw.c @@ -2,7 +2,9 @@ // Copyright(c) 2015-17 Intel Corporation. #include +#include #include +#include #include #include "internal.h" -- 2.17.1
[PATCH] soundwire: master: use pm_runtime_set_active() on add
From: Pierre-Louis Bossart
The 'master' device acts as a glue layer used during bus
initialization only, and it needs to be 'transparent' for pm_runtime
management. Its behavior should be that it becomes active when one of
its children becomes active, and suspends when all of its children are
suspended.
In our tests on Intel platforms, we routinely see these sort of
warnings on the initial boot:
[ 21.447345] rt715 sdw:3:25d:715:0: runtime PM trying to activate
child device sdw:3:25d:715:0 but parent (sdw-master-3) is not active
This is root-caused to a missing setup to make the device 'active' on
probe. Since we don't want the device to remain active forever after
the probe, the autosuspend configuration is also enabled at the end of
the probe - the device will actually autosuspend only in the case
where there are no devices physically attached. In practice, the
master device will suspend when all its children are no longer active.
Fixes: bd84256e86ecf ('soundwire: master: enable pm runtime')
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/master.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c
index 3488bb824e84..9b05c9e25ebe 100644
--- a/drivers/soundwire/master.c
+++ b/drivers/soundwire/master.c
@@ -8,6 +8,15 @@
#include
#include "bus.h"
+/*
+ * The 3s value for autosuspend will only be used if there are no
+ * devices physically attached on a bus segment. In practice enabling
+ * the bus operation will result in children devices become active and
+ * the master device will only suspend when all its children are no
+ * longer active.
+ */
+#define SDW_MASTER_SUSPEND_DELAY_MS 3000
+
/*
* The sysfs for properties reflects the MIPI description as given
* in the MIPI DisCo spec
@@ -154,7 +163,12 @@ int sdw_master_device_add(struct sdw_bus *bus, struct
device *parent,
bus->dev = >dev;
bus->md = md;
+ pm_runtime_set_autosuspend_delay(>md->dev,
SDW_MASTER_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(>md->dev);
+ pm_runtime_mark_last_busy(>md->dev);
+ pm_runtime_set_active(>md->dev);
pm_runtime_enable(>md->dev);
+ pm_runtime_idle(>md->dev);
device_register_err:
return ret;
}
--
2.17.1
[PATCH 4/5] soundwire: bus: only clear valid DP0 interrupts
From: Pierre-Louis Bossart
We should only access the fields that are relevant for DP0, and never
write to reserved or read-only SDCA_CASCADE fields.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d6e646521819..f66a804f9b74 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1280,7 +1280,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
{
- u8 clear = 0, impl_int_mask;
+ u8 clear, impl_int_mask;
int status, status2, ret, count = 0;
status = sdw_read(slave, SDW_DP0_INT);
@@ -1291,6 +1291,8 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
}
do {
+ clear = status & ~SDW_DP0_INTERRUPTS;
+
if (status & SDW_DP0_INT_TEST_FAIL) {
dev_err(>dev, "Test fail for port 0\n");
clear |= SDW_DP0_INT_TEST_FAIL;
@@ -1319,7 +1321,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
*slave_status = clear;
}
- /* clear the interrupt */
+ /* clear the interrupts but don't touch reserved and
SDCA_CASCADE fields */
ret = sdw_write(slave, SDW_DP0_INT, clear);
if (ret < 0) {
dev_err(slave->bus->dev,
@@ -1340,7 +1342,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave
*slave, u8 *slave_status)
count++;
/* we can get alerts while processing so keep retrying */
- } while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY);
+ } while ((status & SDW_DP0_INTERRUPTS) && (count <
SDW_READ_INTR_CLEAR_RETRY));
if (count == SDW_READ_INTR_CLEAR_RETRY)
dev_warn(slave->bus->dev, "Reached MAX_RETRY on DP0 read\n");
--
2.17.1
