This is a generic cleanup series.
v2:
- Remove the "soundwire: stream: uniquify dev_err() logs" patch
Pierre-Louis Bossart (4):
soundwire: stream: use consistent pattern for freeing buffers
soundwire: bus: remove sdw_defer argument in sdw_transfer_defer()
soundwire: cadence: use directly bus sdw_defer structure
soundwire: cadence: further simplify low-level xfer_msg_defer()
callback
drivers/soundwire/bus.c | 12 +++++-------
drivers/soundwire/bus.h | 3 +--
drivers/soundwire/cadence_master.c | 20 ++++++++++----------
drivers/soundwire/cadence_master.h | 5 +----
drivers/soundwire/stream.c | 8 ++++----
include/linux/soundwire/sdw.h | 6 +++---
6 files changed, 24 insertions(+), 30 deletions(-)
--
2.25.1
From: Pierre-Louis Bossart <[email protected]>
Copying the bus sdw_defer structure into the Cadence internals leads
to using stale pointers and kernel oopses on errors. It's just simpler
and safer to use the bus sdw_defer structure directly.
Link: https://github.com/thesofproject/linux/issues/4056
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/bus.c | 2 +-
drivers/soundwire/cadence_master.c | 17 ++++++++---------
drivers/soundwire/cadence_master.h | 5 +----
include/linux/soundwire/sdw.h | 3 +--
4 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 572cc9a9af41..fc192d0c61e7 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -237,7 +237,7 @@ static inline int do_transfer_defer(struct sdw_bus *bus,
init_completion(&defer->complete);
for (i = 0; i <= retry; i++) {
- resp = bus->ops->xfer_msg_defer(bus, msg, defer);
+ resp = bus->ops->xfer_msg_defer(bus, msg);
ret = find_response_code(resp);
/* if cmd is ok or ignored return */
if (ret == 0 || ret == -ENODATA)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 521387322145..2365395cb181 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -750,7 +750,7 @@ EXPORT_SYMBOL(cdns_xfer_msg);
enum sdw_command_response
cdns_xfer_msg_defer(struct sdw_bus *bus,
- struct sdw_msg *msg, struct sdw_defer *defer)
+ struct sdw_msg *msg)
{
struct sdw_cdns *cdns = bus_to_cdns(bus);
int cmd = 0, ret;
@@ -763,9 +763,6 @@ cdns_xfer_msg_defer(struct sdw_bus *bus,
if (ret)
return SDW_CMD_FAIL_OTHER;
- cdns->defer = defer;
- cdns->defer->length = msg->len;
-
return _cdns_xfer_msg(cdns, msg, cmd, 0, msg->len, true);
}
EXPORT_SYMBOL(cdns_xfer_msg_defer);
@@ -893,13 +890,15 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
return IRQ_NONE;
if (int_status & CDNS_MCP_INT_RX_WL) {
+ struct sdw_bus *bus = &cdns->bus;
+ struct sdw_defer *defer = &bus->defer_msg;
+
cdns_read_response(cdns);
- if (cdns->defer) {
- cdns_fill_msg_resp(cdns, cdns->defer->msg,
- cdns->defer->length, 0);
- complete(&cdns->defer->complete);
- cdns->defer = NULL;
+ if (defer && defer->msg) {
+ cdns_fill_msg_resp(cdns, defer->msg,
+ defer->length, 0);
+ complete(&defer->complete);
} else {
complete(&cdns->tx_complete);
}
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index fa9dc38264a4..53029d22822d 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -109,7 +109,6 @@ struct sdw_cdns_dai_runtime {
* @instance: instance number
* @response_buf: SoundWire response buffer
* @tx_complete: Tx completion
- * @defer: Defer pointer
* @ports: Data ports
* @num_ports: Total number of data ports
* @pcm: PCM streams
@@ -130,7 +129,6 @@ struct sdw_cdns {
u32 response_buf[CDNS_MCP_IP_MAX_CMD_LEN + 2];
struct completion tx_complete;
- struct sdw_defer *defer;
struct sdw_cdns_port *ports;
int num_ports;
@@ -189,8 +187,7 @@ enum sdw_command_response
cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response
-cdns_xfer_msg_defer(struct sdw_bus *bus,
- struct sdw_msg *msg, struct sdw_defer *defer);
+cdns_xfer_msg_defer(struct sdw_bus *bus, struct sdw_msg *msg);
u32 cdns_read_ping_status(struct sdw_bus *bus);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 3cd2a761911f..fb2bd1524a26 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -852,8 +852,7 @@ struct sdw_master_ops {
enum sdw_command_response (*xfer_msg)
(struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response (*xfer_msg_defer)
- (struct sdw_bus *bus, struct sdw_msg *msg,
- struct sdw_defer *defer);
+ (struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response (*reset_page_addr)
(struct sdw_bus *bus, unsigned int dev_num);
int (*set_bus_conf)(struct sdw_bus *bus,
--
2.25.1
From: Pierre-Louis Bossart <[email protected]>
The code should free the message buffer used for data, the message
structure used for control and assign the latter to NULL. The last
part is missing for multi-link cases, and the order is inconsistent
for single-link cases.
Link: https://github.com/thesofproject/linux/issues/4056
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/stream.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index df3b36670df4..9c13dbd2b26e 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -723,8 +723,8 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
}
if (!multi_link) {
- kfree(wr_msg);
kfree(wbuf);
+ kfree(wr_msg);
bus->defer_msg.msg = NULL;
bus->params.curr_bank = !bus->params.curr_bank;
bus->params.next_bank = !bus->params.next_bank;
@@ -769,6 +769,7 @@ static int sdw_ml_sync_bank_switch(struct sdw_bus *bus)
if (bus->defer_msg.msg) {
kfree(bus->defer_msg.msg->buf);
kfree(bus->defer_msg.msg);
+ bus->defer_msg.msg = NULL;
}
return 0;
@@ -867,6 +868,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
if (bus->defer_msg.msg) {
kfree(bus->defer_msg.msg->buf);
kfree(bus->defer_msg.msg);
+ bus->defer_msg.msg = NULL;
}
}
--
2.25.1
From: Pierre-Louis Bossart <[email protected]>
The message pointer is already stored in the bus->defer structure, not
need to pass it as an argument.
Suggested-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/bus.c | 2 +-
drivers/soundwire/cadence_master.c | 5 +++--
drivers/soundwire/cadence_master.h | 2 +-
include/linux/soundwire/sdw.h | 5 +++--
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index fc192d0c61e7..ed94c92bc575 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -237,7 +237,7 @@ static inline int do_transfer_defer(struct sdw_bus *bus,
init_completion(&defer->complete);
for (i = 0; i <= retry; i++) {
- resp = bus->ops->xfer_msg_defer(bus, msg);
+ resp = bus->ops->xfer_msg_defer(bus);
ret = find_response_code(resp);
/* if cmd is ok or ignored return */
if (ret == 0 || ret == -ENODATA)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 2365395cb181..ece0ad89746f 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -749,10 +749,11 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg)
EXPORT_SYMBOL(cdns_xfer_msg);
enum sdw_command_response
-cdns_xfer_msg_defer(struct sdw_bus *bus,
- struct sdw_msg *msg)
+cdns_xfer_msg_defer(struct sdw_bus *bus)
{
struct sdw_cdns *cdns = bus_to_cdns(bus);
+ struct sdw_defer *defer = &bus->defer_msg;
+ struct sdw_msg *msg = defer->msg;
int cmd = 0, ret;
/* for defer only 1 message is supported */
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index 53029d22822d..63c58b9b1f59 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -187,7 +187,7 @@ enum sdw_command_response
cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response
-cdns_xfer_msg_defer(struct sdw_bus *bus, struct sdw_msg *msg);
+cdns_xfer_msg_defer(struct sdw_bus *bus);
u32 cdns_read_ping_status(struct sdw_bus *bus);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index fb2bd1524a26..86e320cf27b1 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -837,7 +837,8 @@ struct sdw_defer {
* @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
+ * @xfer_msg_defer: Defer version of transfer message callback. The message is handled with the
+ * bus struct @sdw_defer
* @reset_page_addr: Reset the SCP page address registers
* @set_bus_conf: Set the bus configuration
* @pre_bank_switch: Callback for pre bank switch
@@ -852,7 +853,7 @@ struct sdw_master_ops {
enum sdw_command_response (*xfer_msg)
(struct sdw_bus *bus, struct sdw_msg *msg);
enum sdw_command_response (*xfer_msg_defer)
- (struct sdw_bus *bus, struct sdw_msg *msg);
+ (struct sdw_bus *bus);
enum sdw_command_response (*reset_page_addr)
(struct sdw_bus *bus, unsigned int dev_num);
int (*set_bus_conf)(struct sdw_bus *bus,
--
2.25.1
From: Pierre-Louis Bossart <[email protected]>
There's no point in passing an argument that is a pointer to a bus
member. We can directly get the member and do an indirection when
needed.
This is a first step before simplifying the hardware-specific
callbacks further.
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/bus.c | 10 ++++------
drivers/soundwire/bus.h | 3 +--
drivers/soundwire/stream.c | 4 +---
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 633d411b64f3..572cc9a9af41 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -225,9 +225,9 @@ static inline int do_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
}
static inline int do_transfer_defer(struct sdw_bus *bus,
- struct sdw_msg *msg,
- struct sdw_defer *defer)
+ struct sdw_msg *msg)
{
+ struct sdw_defer *defer = &bus->defer_msg;
int retry = bus->prop.err_threshold;
enum sdw_command_response resp;
int ret = 0, i;
@@ -335,19 +335,17 @@ EXPORT_SYMBOL(sdw_show_ping_status);
* sdw_transfer_defer() - Asynchronously transfer message to a SDW Slave device
* @bus: SDW bus
* @msg: SDW message to be xfered
- * @defer: Defer block for signal completion
*
* Caller needs to hold the msg_lock lock while calling this
*/
-int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
- struct sdw_defer *defer)
+int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg)
{
int ret;
if (!bus->ops->xfer_msg_defer)
return -ENOTSUPP;
- ret = do_transfer_defer(bus, msg, defer);
+ ret = do_transfer_defer(bus, msg);
if (ret != 0 && ret != -ENODATA)
dev_err(bus->dev, "Defer trf on Slave %d failed:%d\n",
msg->dev_num, ret);
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 7631ef5e71fb..96927a143796 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -151,8 +151,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave, int port,
bool enable, int mask);
int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg);
-int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
- struct sdw_defer *defer);
+int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg);
#define SDW_READ_INTR_CLEAR_RETRY 10
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 9c13dbd2b26e..2e39587ed1de 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -684,8 +684,6 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
if (!wr_msg)
return -ENOMEM;
- bus->defer_msg.msg = wr_msg;
-
wbuf = kzalloc(sizeof(*wbuf), GFP_KERNEL);
if (!wbuf) {
ret = -ENOMEM;
@@ -713,7 +711,7 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
multi_link = bus->multi_link && (m_rt_count >= bus->hw_sync_min_links);
if (multi_link)
- ret = sdw_transfer_defer(bus, wr_msg, &bus->defer_msg);
+ ret = sdw_transfer_defer(bus, wr_msg);
else
ret = sdw_transfer(bus, wr_msg);
--
2.25.1
On 19-01-23, 15:32, Bard Liao wrote:
> This is a generic cleanup series.
Applied, thanks
--
~Vinod