On 1/13/23 04:22, Amadeusz Sławiński wrote:
On 1/13/2023 10:35 AM, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
There are a couple of duplicate logs which makes harder than needed to follow the error flows. Add __func__ or make the log unique.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
drivers/soundwire/stream.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index df3b36670df4..e0eae0b98267 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1389,7 +1389,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream, ret = do_bank_switch(stream); if (ret < 0) { - dev_err(bus->dev, "Bank switch failed: %d\n", ret); + dev_err(bus->dev, "do_bank_switch failed: %d\n", ret); goto restore_params; }
This one seems bit unrelated to the change and makes error message inconsistent with: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/tree/dri... and https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/tree/dri... which actually brings me to another suggestion, can this error message perhaps be just moved into do_bank_switch() function itself, instead of being duplicated multiple times or alternatively just also prefix all of them with function name?
well, as you correctly pointed out, there are multiple users of 'do_bank_switch' so we don't want to put the message in the function itself.
We could indeed use __func__ instead, that'd be fine.
Looking at the code, there are also inconsistencies with the use of pr_err and dev_err. dev_err(bus->dev is wrong actually, this would use the bus variable assigned in the previous loop, this makes no sense for multi-segment topologies.
Let's drop this patch and revisit all this, hope Vinod can deal with patch 1..4 otherwise we'll resend the set.
Thanks Amadeusz for the feedback.