[alsa-devel] [PATCH 00/14] soundwire: more code cleanups
After applying cleanup from Pierre, I realized few more things can be cleaned up as well, so fixing these instance in the subsystem.
Looks like bool is no longer encouraged, but I have skipped those and few alignment as they caused code to look worse.
Greg, if you are okay I would like to add this as well (i know it is late, but they are cosmetic changes and no logic ones, let me know and I can send a updated PR)
Vinod Koul (14): soundwire: fix kconfig help format soundwire: fix SPDX license for header files soundwire: intel: fix SPDX license for header file soundwire: remove empty line before/after braces soundwire: cadence: remove empty line after braces soundwire: intel: remove empty line after braces soundwire: add argument to function definition soundwire: more alignment fixes soundwire: intel: more alignment fixes soundwire: avoid multiple assignments soundwire: fix more typos soundwire: wrap macro argument in parenthesis soundwire: add a blank line between functions soundwire: remove multiple blank lines
drivers/soundwire/Kconfig | 2 +- drivers/soundwire/bus.c | 17 +++++------- drivers/soundwire/cadence_master.c | 1 - drivers/soundwire/intel.c | 36 ++++++++++++------------- drivers/soundwire/intel_init.c | 1 - drivers/soundwire/mipi_disco.c | 6 ----- drivers/soundwire/stream.c | 28 +++++++++---------- include/linux/soundwire/sdw.h | 16 +++++------ include/linux/soundwire/sdw_intel.h | 6 ++--- include/linux/soundwire/sdw_registers.h | 5 ++-- include/linux/soundwire/sdw_type.h | 6 ++--- 11 files changed, 54 insertions(+), 70 deletions(-)
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC - ---help--- + help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
On Thu, May 02, 2019 at 04:29:17PM +0530, Vinod Koul wrote:
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC
- ---help---
- help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
-- 2.20.1
Huh?
Pierre-Louis sent this patch before you did. Why did you just rewrite the changelog text a bit and then ignore his authorship and signed-off-by?
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
greg k-h
On 02-05-19, 17:07, Greg KH wrote:
On Thu, May 02, 2019 at 04:29:17PM +0530, Vinod Koul wrote:
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC
- ---help---
- help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
-- 2.20.1
Huh?
Pierre-Louis sent this patch before you did. Why did you just rewrite the changelog text a bit and then ignore his authorship and signed-off-by?
This is not *that* instance! The one Pierre changed was for symbol "SOUNDWIRE" and is already applied [1] this is for SOUNDWIRE_INTEL which is another instance. If you have looked or cared, even the log mentions "makes file consistent with other instance"
This series is on *top* on the "whole" series from Pierre
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
Honestly, this is a terrible accusation, seems everyone is quick to jump and yell at others. I asking for an apology.
On Thu, May 02, 2019 at 08:45:02PM +0530, Vinod Koul wrote:
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
Honestly, this is a terrible accusation, seems everyone is quick to jump and yell at others. I asking for an apology.
Our emails crossed, I already sent it, sorry about that.
On Thu, May 02, 2019 at 05:07:54PM +0200, Greg KH wrote:
On Thu, May 02, 2019 at 04:29:17PM +0530, Vinod Koul wrote:
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC
- ---help---
- help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
-- 2.20.1
Huh?
Pierre-Louis sent this patch before you did. Why did you just rewrite the changelog text a bit and then ignore his authorship and signed-off-by?
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
Ok, my apologies, that was incorrect. Your patch was against a different file than his.
Way to go and make this a total mess to try to figure out, let me see what I can do...
greg k-h
On 02-05-19, 17:15, Greg KH wrote:
On Thu, May 02, 2019 at 05:07:54PM +0200, Greg KH wrote:
On Thu, May 02, 2019 at 04:29:17PM +0530, Vinod Koul wrote:
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC
- ---help---
- help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
-- 2.20.1
Huh?
Pierre-Louis sent this patch before you did. Why did you just rewrite the changelog text a bit and then ignore his authorship and signed-off-by?
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
Ok, my apologies, that was incorrect. Your patch was against a different file than his.
Yes and our emails crossed, apology accepted.
Way to go and make this a total mess to try to figure out, let me see what I can do...
If you can hold off for a bit longer, I have patches in next (pierre's full series and couple of then split and ofcourse authorship retain (https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/log/?h=n...) and mine on top (https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/log/?h=c...)
I can send you a signed PR for both and you can merged them. otherwise the style changes will conflict and defeat the whole purpose..
Let me know if that is okay, thanks
On Thu, May 02, 2019 at 08:51:46PM +0530, Vinod Koul wrote:
On 02-05-19, 17:15, Greg KH wrote:
On Thu, May 02, 2019 at 05:07:54PM +0200, Greg KH wrote:
On Thu, May 02, 2019 at 04:29:17PM +0530, Vinod Koul wrote:
Move to help format instead of --help-- as that is not recommended and this makes file consistent with other instance
Signed-off-by: Vinod Koul vkoul@kernel.org
drivers/soundwire/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index 84876a74874f..53b55b79c4af 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -28,7 +28,7 @@ config SOUNDWIRE_INTEL select SOUNDWIRE_CADENCE select SOUNDWIRE_BUS depends on X86 && ACPI && SND_SOC
- ---help---
- help SoundWire Intel Master driver. If you have an Intel platform which has a SoundWire Master then enable this config option to get the SoundWire support for that
-- 2.20.1
Huh?
Pierre-Louis sent this patch before you did. Why did you just rewrite the changelog text a bit and then ignore his authorship and signed-off-by?
That's a really really shitty thing to do, I'm going to go take his version of these patches instead, let me see how they line up...
Ok, my apologies, that was incorrect. Your patch was against a different file than his.
Yes and our emails crossed, apology accepted.
Way to go and make this a total mess to try to figure out, let me see what I can do...
If you can hold off for a bit longer, I have patches in next (pierre's full series and couple of then split and ofcourse authorship retain (https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/log/?h=n...) and mine on top (https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/log/?h=c...)
I can send you a signed PR for both and you can merged them. otherwise the style changes will conflict and defeat the whole purpose..
Let me know if that is okay, thanks
Send me the pull request and let's see how bad things are messed up :)
thanks,
greg k-h
Some more headers had C++ style SDPX line, fix that and change copyright so that it is consistent with rest of the code in subsystem
Signed-off-by: Vinod Koul vkoul@kernel.org --- include/linux/soundwire/sdw.h | 4 ++-- include/linux/soundwire/sdw_registers.h | 4 ++-- include/linux/soundwire/sdw_type.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index df313913e856..d9102af5e26c 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) -// Copyright(c) 2015-17 Intel Corporation. +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* Copyright(c) 2015-17 Intel Corporation. */
#ifndef __SOUNDWIRE_H #define __SOUNDWIRE_H diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h index df472b1ab410..e75b2e3cf93d 100644 --- a/include/linux/soundwire/sdw_registers.h +++ b/include/linux/soundwire/sdw_registers.h @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) -// Copyright(c) 2015-17 Intel Corporation. +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* Copyright(c) 2015-17 Intel Corporation. */
#ifndef __SDW_REGISTERS_H #define __SDW_REGISTERS_H diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index 9fd553e553e9..b7e198a035c9 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: GPL-2.0 -// Copyright(c) 2015-17 Intel Corporation. +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright(c) 2015-17 Intel Corporation. */
#ifndef __SOUNDWIRE_TYPES_H #define __SOUNDWIRE_TYPES_H
Some more headers had C++ style SDPX line, fix that and change copyright so that it is consistent with rest of the code in subsystem
Signed-off-by: Vinod Koul vkoul@kernel.org --- include/linux/soundwire/sdw_intel.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 2b9573b8aedd..0848f9d38bcb 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -1,5 +1,5 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) -// Copyright(c) 2015-17 Intel Corporation. +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* Copyright(c) 2015-17 Intel Corporation. */
#ifndef __SDW_INTEL_H #define __SDW_INTEL_H
Linux code style doesn't expect empty lines before or after braces and gives warning:
CHECK: Blank lines aren't necessary after an open brace '{' CHECK: Blank lines aren't necessary before a close brace '}'
Fix these instances in soundwire core
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/bus.c | 8 -------- drivers/soundwire/mipi_disco.c | 6 ------ drivers/soundwire/stream.c | 8 -------- 3 files changed, 22 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index fa86957cb615..e2ee305905a6 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -391,7 +391,6 @@ EXPORT_SYMBOL(sdw_read); int sdw_write(struct sdw_slave *slave, u32 addr, u8 value) { return sdw_nwrite(slave, addr, 1, &value); - } EXPORT_SYMBOL(sdw_write);
@@ -414,7 +413,6 @@ static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
static int sdw_compare_devid(struct sdw_slave *slave, struct sdw_slave_id id) { - if (slave->id.unique_id != id.unique_id || slave->id.mfg_id != id.mfg_id || slave->id.part_id != id.part_id || @@ -467,7 +465,6 @@ static int sdw_assign_device_num(struct sdw_slave *slave) /* Clear the slave->dev_num to transfer message on device 0 */ dev_num = slave->dev_num; slave->dev_num = 0; - }
ret = sdw_write(slave, SDW_SCP_DEVNUMBER, dev_num); @@ -508,7 +505,6 @@ void sdw_extract_slave_id(struct sdw_bus *bus, "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); - }
static int sdw_program_device_num(struct sdw_bus *bus) @@ -677,7 +673,6 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status) }
do { - if (status & SDW_DP0_INT_TEST_FAIL) { dev_err(&slave->dev, "Test fail for port 0\n"); clear |= SDW_DP0_INT_TEST_FAIL; @@ -754,7 +749,6 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave, }
do { - if (status & SDW_DPN_INT_TEST_FAIL) { dev_err(&slave->dev, "Test fail for port:%d\n", port); clear |= SDW_DPN_INT_TEST_FAIL; @@ -867,7 +861,6 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) for_each_set_bit(bit, &port, 8) { sdw_handle_port_interrupt(slave, bit, &port_status[bit]); - }
/* Check if cascade 2 interrupt is present */ @@ -1035,7 +1028,6 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (ret) dev_err(slave->bus->dev, "Update Slave status failed:%d\n", ret); - }
return ret; diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c index 2bf4046e68b6..c1f51d6a23d2 100644 --- a/drivers/soundwire/mipi_disco.c +++ b/drivers/soundwire/mipi_disco.c @@ -63,7 +63,6 @@ int sdw_master_read_prop(struct sdw_bus *bus) nval = fwnode_property_read_u32_array(link, "mipi-sdw-clock-frequencies-supported", NULL, 0); if (nval > 0) { - prop->num_freq = nval; prop->freq = devm_kcalloc(bus->dev, prop->num_freq, sizeof(*prop->freq), GFP_KERNEL); @@ -90,7 +89,6 @@ int sdw_master_read_prop(struct sdw_bus *bus) nval = fwnode_property_read_u32_array(link, "mipi-sdw-supported-clock-gears", NULL, 0); if (nval > 0) { - prop->num_clk_gears = nval; prop->clk_gears = devm_kcalloc(bus->dev, prop->num_clk_gears, sizeof(*prop->clk_gears), @@ -197,7 +195,6 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave, nval = fwnode_property_read_u32_array(node, "mipi-sdw-port-wordlength-configs", NULL, 0); if (nval > 0) { - dpn[i].num_words = nval; dpn[i].words = devm_kcalloc(&slave->dev, dpn[i].num_words, @@ -238,7 +235,6 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave, nval = fwnode_property_read_u32_array(node, "mipi-sdw-channel-number-list", NULL, 0); if (nval > 0) { - dpn[i].num_ch = nval; dpn[i].ch = devm_kcalloc(&slave->dev, dpn[i].num_ch, sizeof(*dpn[i].ch), @@ -254,7 +250,6 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave, nval = fwnode_property_read_u32_array(node, "mipi-sdw-channel-combination-list", NULL, 0); if (nval > 0) { - dpn[i].num_ch_combinations = nval; dpn[i].ch_combinations = devm_kcalloc(&slave->dev, dpn[i].num_ch_combinations, @@ -354,7 +349,6 @@ int sdw_slave_read_prop(struct sdw_slave *slave) if (!port) { dev_dbg(dev, "DP0 node not found!!\n"); } else { - prop->dp0_prop = devm_kzalloc(&slave->dev, sizeof(*prop->dp0_prop), GFP_KERNEL); diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 166b0c16003f..032a326a96d7 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -771,7 +771,6 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) if (ret < 0) { dev_err(bus->dev, "Bank switch failed: %d\n", ret); goto error; - } }
@@ -818,7 +817,6 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
error: list_for_each_entry(m_rt, &stream->master_list, stream_node) { - bus = m_rt->bus;
kfree(bus->defer_msg.msg->buf); @@ -985,13 +983,11 @@ static void sdw_slave_port_release(struct sdw_bus *bus,
list_for_each_entry(m_rt, &stream->master_list, stream_node) { list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { - if (s_rt->slave != slave) continue;
list_for_each_entry_safe(p_rt, _p_rt, &s_rt->port_list, port_node) { - list_del(&p_rt->port_node); kfree(p_rt); } @@ -1017,7 +1013,6 @@ static void sdw_release_slave_stream(struct sdw_slave *slave, /* Retrieve Slave runtime handle */ list_for_each_entry_safe(s_rt, _s_rt, &m_rt->slave_rt_list, m_rt_node) { - if (s_rt->slave == slave) { list_del(&s_rt->m_rt_node); kfree(s_rt); @@ -1070,7 +1065,6 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
list_for_each_entry_safe(m_rt, _m_rt, &stream->master_list, stream_node) { - if (m_rt->bus != bus) continue;
@@ -1492,7 +1486,6 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) dev_err(bus->dev, "Program params failed: %d\n", ret); goto restore_params; } - }
ret = do_bank_switch(stream); @@ -1693,7 +1686,6 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream) dev_err(bus->dev, "Program params failed: %d\n", ret); return ret; } - }
stream->state = SDW_STREAM_DEPREPARED;
Linux code style doesn't expect empty lines after braces and gives warning:
CHECK: Blank lines aren't necessary after an open brace '{'
Remove the empty line in cadence lib
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/cadence_master.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 0fdc17b995fc..682789bb8ab3 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -574,7 +574,6 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id) }
if (int_status & CDNS_MCP_INT_CTRL_CLASH) { - /* Slave is driving bit slot during control word */ dev_err_ratelimited(cdns->dev, "Bus clash for control word\n"); int_status |= CDNS_MCP_INT_CTRL_CLASH;
Linux code style doesn't expect empty lines after braces and gives warning:
CHECK: Blank lines aren't necessary after an open brace '{'
Remove the empty line in intel module
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/intel_init.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 9ad6045720c4..d3d6b54c5791 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -106,7 +106,6 @@ static struct sdw_intel_ctx
/* Create SDW Master devices */ for (i = 0; i < count; i++) { - link->res.irq = res->irq; link->res.registers = res->mmio_base + SDW_LINK_BASE + (SDW_LINK_SIZE * i);
Checkpatch warns that function definition of __sdw_register_driver misses argument, so add it
WARNING: function definition argument 'struct module *' should also have an identifier name
Signed-off-by: Vinod Koul vkoul@kernel.org --- include/linux/soundwire/sdw_type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index b7e198a035c9..9c756b5a0dfe 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -11,7 +11,7 @@ extern struct bus_type sdw_bus_type; #define sdw_register_driver(drv) \ __sdw_register_driver(drv, THIS_MODULE)
-int __sdw_register_driver(struct sdw_driver *drv, struct module *); +int __sdw_register_driver(struct sdw_driver *drv, struct module *owner); void sdw_unregister_driver(struct sdw_driver *drv);
int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size);
Found few more issues reported checkpatch on code alignment so fix those as well in the soundwire core.
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/bus.c | 3 ++- drivers/soundwire/stream.c | 15 ++++++++++----- include/linux/soundwire/sdw.h | 10 +++++----- 3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index e2ee305905a6..16b2a3b2662d 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -43,7 +43,8 @@ int sdw_add_bus_master(struct sdw_bus *bus) if (bus->ops->read_prop) { ret = bus->ops->read_prop(bus); if (ret < 0) { - dev_err(bus->dev, "Bus read properties failed:%d\n", ret); + dev_err(bus->dev, + "Bus read properties failed:%d\n", ret); return ret; } } diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 032a326a96d7..bffa535294ed 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -400,7 +400,8 @@ static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt, ret = ops->port_prep(s_rt->slave, &prep_ch, cmd); if (ret < 0) { dev_err(&s_rt->slave->dev, - "Slave Port Prep cmd %d failed: %d\n", cmd, ret); + "Slave Port Prep cmd %d failed: %d\n", + cmd, ret); return ret; } } @@ -614,7 +615,8 @@ static int sdw_program_params(struct sdw_bus *bus)
ret = sdw_notify_config(m_rt); if (ret < 0) { - dev_err(bus->dev, "Notify bus config failed: %d\n", ret); + dev_err(bus->dev, + "Notify bus config failed: %d\n", ret); return ret; }
@@ -789,7 +791,8 @@ static int do_bank_switch(struct sdw_stream_runtime *stream) ret = ops->post_bank_switch(bus); if (ret < 0) { dev_err(bus->dev, - "Post bank switch op failed: %d\n", ret); + "Post bank switch op failed: %d\n", + ret); goto error; } } else if (bus->multi_link && stream->m_rt_count > 1) { @@ -1562,7 +1565,8 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream) /* Enable port(s) */ ret = sdw_enable_disable_ports(m_rt, true); if (ret < 0) { - dev_err(bus->dev, "Enable port(s) failed ret: %d\n", ret); + dev_err(bus->dev, + "Enable port(s) failed ret: %d\n", ret); return ret; } } @@ -1672,7 +1676,8 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream) /* De-prepare port(s) */ ret = sdw_prep_deprep_ports(m_rt, false); if (ret < 0) { - dev_err(bus->dev, "De-prepare port(s) failed: %d\n", ret); + dev_err(bus->dev, + "De-prepare port(s) failed: %d\n", ret); return ret; }
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index d9102af5e26c..41c49631288a 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -470,14 +470,14 @@ struct sdw_bus_params { struct sdw_slave_ops { int (*read_prop)(struct sdw_slave *sdw); int (*interrupt_callback)(struct sdw_slave *slave, - struct sdw_slave_intr_status *status); + struct sdw_slave_intr_status *status); int (*update_status)(struct sdw_slave *slave, - enum sdw_slave_status status); + enum sdw_slave_status status); int (*bus_config)(struct sdw_slave *slave, - struct sdw_bus_params *params); + struct sdw_bus_params *params); int (*port_prep)(struct sdw_slave *slave, - struct sdw_prepare_ch *prepare_ch, - enum sdw_port_prep_ops pre_ops); + struct sdw_prepare_ch *prepare_ch, + enum sdw_port_prep_ops pre_ops); };
/**
Found few more issues reported checkpatch on code alignment so fix those as well in the intel module.
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/intel.c | 36 ++++++++++++++--------------- include/linux/soundwire/sdw_intel.h | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 9774dc1e4029..31336b0271b0 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -691,9 +691,9 @@ static int intel_create_dai(struct sdw_cdns *cdns, return -ENOMEM;
if (type == INTEL_PDI_BD || type == INTEL_PDI_OUT) { - dais[i].playback.stream_name = kasprintf(GFP_KERNEL, - "SDW%d Tx%d", - cdns->instance, i); + dais[i].playback.stream_name = + kasprintf(GFP_KERNEL, "SDW%d Tx%d", + cdns->instance, i); if (!dais[i].playback.stream_name) { kfree(dais[i].name); return -ENOMEM; @@ -706,9 +706,9 @@ static int intel_create_dai(struct sdw_cdns *cdns, }
if (type == INTEL_PDI_BD || type == INTEL_PDI_IN) { - dais[i].capture.stream_name = kasprintf(GFP_KERNEL, - "SDW%d Rx%d", - cdns->instance, i); + dais[i].capture.stream_name = + kasprintf(GFP_KERNEL, "SDW%d Rx%d", + cdns->instance, i); if (!dais[i].capture.stream_name) { kfree(dais[i].name); kfree(dais[i].playback.stream_name); @@ -749,40 +749,40 @@ static int intel_register_dai(struct sdw_intel *sdw) /* Create PCM DAIs */ stream = &cdns->pcm;
- ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, - stream->num_in, off, stream->num_ch_in, true); + ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, stream->num_in, + off, stream->num_ch_in, true); if (ret) return ret;
off += cdns->pcm.num_in; - ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, - cdns->pcm.num_out, off, stream->num_ch_out, true); + ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns->pcm.num_out, + off, stream->num_ch_out, true); if (ret) return ret;
off += cdns->pcm.num_out; - ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, - cdns->pcm.num_bd, off, stream->num_ch_bd, true); + ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pcm.num_bd, + off, stream->num_ch_bd, true); if (ret) return ret;
/* Create PDM DAIs */ stream = &cdns->pdm; off += cdns->pcm.num_bd; - ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, - cdns->pdm.num_in, off, stream->num_ch_in, false); + ret = intel_create_dai(cdns, dais, INTEL_PDI_IN, cdns->pdm.num_in, + off, stream->num_ch_in, false); if (ret) return ret;
off += cdns->pdm.num_in; - ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, - cdns->pdm.num_out, off, stream->num_ch_out, false); + ret = intel_create_dai(cdns, dais, INTEL_PDI_OUT, cdns->pdm.num_out, + off, stream->num_ch_out, false); if (ret) return ret;
off += cdns->pdm.num_bd; - ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, - cdns->pdm.num_bd, off, stream->num_ch_bd, false); + ret = intel_create_dai(cdns, dais, INTEL_PDI_BD, cdns->pdm.num_bd, + off, stream->num_ch_bd, false); if (ret) return ret;
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 0848f9d38bcb..4d70da45363d 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -11,7 +11,7 @@ */ struct sdw_intel_ops { int (*config_stream)(void *arg, void *substream, - void *dai, void *hw_params, int stream_num); + void *dai, void *hw_params, int stream_num); };
/**
Modify the code to avoid multiple assignments by assigning to variable after error checks in soundwire bus.
CHECK: multiple assignments should be avoided
Signed-off-by: Vinod Koul vkoul@kernel.org --- 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 16b2a3b2662d..aac35fc3cf22 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -812,12 +812,13 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) sdw_modify_slave_status(slave, SDW_SLAVE_ALERT);
/* Read Instat 1, Instat 2 and Instat 3 registers */ - buf = ret = sdw_read(slave, SDW_SCP_INT1); + ret = sdw_read(slave, SDW_SCP_INT1); if (ret < 0) { dev_err(slave->bus->dev, "SDW_SCP_INT1 read failed:%d\n", ret); return ret; } + buf = ret;
ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2); if (ret < 0) { @@ -910,12 +911,13 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) * Read status again to ensure no new interrupts arrived * while servicing interrupts. */ - _buf = ret = sdw_read(slave, SDW_SCP_INT1); + ret = sdw_read(slave, SDW_SCP_INT1); if (ret < 0) { dev_err(slave->bus->dev, "SDW_SCP_INT1 read failed:%d\n", ret); return ret; } + _buf = ret;
ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, _buf2); if (ret < 0) {
Found few more typos in the code, fix them
CHECK: 'and and' may be misspelled - perhaps 'and'? CHECK: 'smaple' may be misspelled - perhaps 'sample'?
Signed-off-by: Vinod Koul vkoul@kernel.org --- 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 bffa535294ed..debb67882df4 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -89,7 +89,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
/* * Data ports are FULL, SIMPLE and REDUCED. This function handles - * FULL and REDUCED only and and beyond this point only FULL is + * FULL and REDUCED only and beyond this point only FULL is * handled, so bail out if we are not FULL data port type */ if (type != SDW_DPN_FULL) @@ -233,7 +233,7 @@ static int sdw_program_master_port_params(struct sdw_bus *bus,
/* * we need to set transport and port parameters for the port. - * Transport parameters refers to the smaple interval, offsets and + * Transport parameters refers to the sample interval, offsets and * hstart/stop etc of the data. Port parameters refers to word * length, flow mode etc of the port */
macro argument should be inside a parenthesis to avoid precedence issues
checkpatch complains: CHECK: Macro argument 'n' may be better as '(n)' to avoid precedence issues
Signed-off-by: Vinod Koul vkoul@kernel.org --- include/linux/soundwire/sdw.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 41c49631288a..35662d9c2c62 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -36,7 +36,7 @@ struct sdw_slave; #define SDW_FRAME_CTRL_BITS 48 #define SDW_MAX_DEVICES 11
-#define SDW_VALID_PORT_RANGE(n) (n <= 14 && n >= 1) +#define SDW_VALID_PORT_RANGE(n) ((n) <= 14 && (n) >= 1)
#define SDW_DAI_ID_RANGE_START 100 #define SDW_DAI_ID_RANGE_END 200
For improving code readability it helps to have a blank line between function so add when missing.
Checkpatch complains: CHECK: Please use a blank line after function/struct/union/enum declarations
Signed-off-by: Vinod Koul vkoul@kernel.org --- drivers/soundwire/stream.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index debb67882df4..d01060dbee96 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -52,6 +52,7 @@ static int sdw_find_row_index(int row) pr_warn("Requested row not found, selecting lowest row no: 48\n"); return 0; } + static int _sdw_program_slave_port_params(struct sdw_bus *bus, struct sdw_slave *slave, struct sdw_transport_params *t_params,
Multi-blank lines do not help readability so remove them
Checkpatch complains: CHECK: Please don't use multiple blank lines
Signed-off-by: Vinod Koul vkoul@kernel.org --- include/linux/soundwire/sdw_registers.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h index e75b2e3cf93d..a686f7988156 100644 --- a/include/linux/soundwire/sdw_registers.h +++ b/include/linux/soundwire/sdw_registers.h @@ -73,7 +73,6 @@ #define SDW_SCP_INTSTAT2_SCP3_CASCADE BIT(7) #define SDW_SCP_INTSTAT2_PORT4_10 GENMASK(6, 0)
- #define SDW_SCP_INTSTAT3 0x43 #define SDW_SCP_INTSTAT3_PORT11_14 GENMASK(3, 0)
On Thu, May 02, 2019 at 04:29:16PM +0530, Vinod Koul wrote:
After applying cleanup from Pierre, I realized few more things can be cleaned up as well, so fixing these instance in the subsystem.
I do not see any of Pierre's patches in here, only yours :(
I'll go queue his up first, and then see what is remaining in this series...
greg k-h
On Thu, May 02, 2019 at 04:29:16PM +0530, Vinod Koul wrote:
After applying cleanup from Pierre, I realized few more things can be cleaned up as well, so fixing these instance in the subsystem.
Looks like bool is no longer encouraged, but I have skipped those and few alignment as they caused code to look worse.
Greg, if you are okay I would like to add this as well (i know it is late, but they are cosmetic changes and no logic ones, let me know and I can send a updated PR)
This made no sense, as it was on top of Pierre's patches.
I've applied both his, and your, patch series to the tree now, hopefully this type of mess doesn't happen again in the future.
Please be a lot more specific as to what you expect me to do with a patch series, and what exactly it is for/against/after, as this was not obvious at all.
ugh, someone owes me a drink...
greg k-h
On 5/2/19 10:19 AM, Greg KH wrote:
On Thu, May 02, 2019 at 04:29:16PM +0530, Vinod Koul wrote:
After applying cleanup from Pierre, I realized few more things can be cleaned up as well, so fixing these instance in the subsystem.
Looks like bool is no longer encouraged, but I have skipped those and few alignment as they caused code to look worse.
Greg, if you are okay I would like to add this as well (i know it is late, but they are cosmetic changes and no logic ones, let me know and I can send a updated PR)
This made no sense, as it was on top of Pierre's patches.
I've applied both his, and your, patch series to the tree now, hopefully this type of mess doesn't happen again in the future.
Please be a lot more specific as to what you expect me to do with a patch series, and what exactly it is for/against/after, as this was not obvious at all.
ugh, someone owes me a drink...
greg k-h
I'll be glad to buy a round at the next ELC.
I also boiled initially when I saw the Kconfig patch until I realized it was a different patch. Same for the SPDX files. It wasn't obvious from a quick review and I could have been the one sending a nasty email.
There are additional cleanups from Vinod that I saw but didn't fix as I wasn't sure they made the code readable (e.g. dev_dbg logs exceeding 80 chars and newlines after loops), but I also missed others that are very much needed, so please add my Rvb tag for this 'more code cleanups' series.
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Thank you Vinod for the additional work. I am almost done with the sysfs/debugfs cleanups and should post that next week.
participants (3)
-
Greg KH -
Pierre-Louis Bossart -
Vinod Koul