* [PATCH] soundwire: bus: fix device number leak on errors
@ 2020-01-13 22:56 Pierre-Louis Bossart
2020-01-14 6:37 ` Vinod Koul
0 siblings, 1 reply; 3+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-13 22:56 UTC (permalink / raw)
To: alsa-devel
Cc: linux-kernel, tiwai, broonie, vkoul, gregkh, jank,
srinivas.kandagatla, slawomir.blauciak, Bard liao, Rander Wang,
Ranjani Sridharan, Pierre-Louis Bossart, Sanyog Kale
If the programming of the dev_number fails due to an IO error, a new
device_number will be assigned, resulting in a leak.
Make sure we only assign a device_number once per Slave device.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
drivers/soundwire/bus.c | 37 ++++++++++++++++++++++-------------
include/linux/soundwire/sdw.h | 4 +++-
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index be5d437058ed..8f973e70e6f7 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -456,26 +456,35 @@ static int sdw_get_device_num(struct sdw_slave *slave)
static int sdw_assign_device_num(struct sdw_slave *slave)
{
int ret, dev_num;
+ bool new_device = false;
/* check first if device number is assigned, if so reuse that */
if (!slave->dev_num) {
- mutex_lock(&slave->bus->bus_lock);
- dev_num = sdw_get_device_num(slave);
- mutex_unlock(&slave->bus->bus_lock);
- if (dev_num < 0) {
- dev_err(slave->bus->dev, "Get dev_num failed: %d\n",
- dev_num);
- return dev_num;
+ if (!slave->dev_num_sticky) {
+ mutex_lock(&slave->bus->bus_lock);
+ dev_num = sdw_get_device_num(slave);
+ mutex_unlock(&slave->bus->bus_lock);
+ if (dev_num < 0) {
+ dev_err(slave->bus->dev, "Get dev_num failed: %d\n",
+ dev_num);
+ return dev_num;
+ }
+ slave->dev_num = dev_num;
+ slave->dev_num_sticky = dev_num;
+ new_device = true;
+ } else {
+ slave->dev_num = slave->dev_num_sticky;
}
- } else {
+ }
+
+ if (!new_device)
dev_info(slave->bus->dev,
- "Slave already registered dev_num:%d\n",
+ "Slave already registered, reusing dev_num:%d\n",
slave->dev_num);
- /* Clear the slave->dev_num to transfer message on device 0 */
- dev_num = slave->dev_num;
- slave->dev_num = 0;
- }
+ /* 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);
if (ret < 0) {
@@ -485,7 +494,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
}
/* After xfer of msg, restore dev_num */
- slave->dev_num = dev_num;
+ slave->dev_num = slave->dev_num_sticky;
return 0;
}
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index b7c9eca4332a..b451bb622335 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -546,7 +546,8 @@ struct sdw_slave_ops {
* @debugfs: Slave debugfs
* @node: node for bus list
* @port_ready: Port ready completion flag for each Slave port
- * @dev_num: Device Number assigned by Bus
+ * @dev_num: Current Device Number, values can be 0 or dev_num_sticky
+ * @dev_num_sticky: one-time static Device Number assigned by Bus
* @probed: boolean tracking driver state
* @probe_complete: completion utility to control potential races
* on startup between driver probe/initialization and SoundWire
@@ -575,6 +576,7 @@ struct sdw_slave {
struct list_head node;
struct completion *port_ready;
u16 dev_num;
+ u16 dev_num_sticky;
bool probed;
struct completion probe_complete;
struct completion enumeration_complete;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] soundwire: bus: fix device number leak on errors
2020-01-13 22:56 [PATCH] soundwire: bus: fix device number leak on errors Pierre-Louis Bossart
@ 2020-01-14 6:37 ` Vinod Koul
2020-01-14 16:10 ` [alsa-devel] " Pierre-Louis Bossart
0 siblings, 1 reply; 3+ messages in thread
From: Vinod Koul @ 2020-01-14 6:37 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: alsa-devel, linux-kernel, tiwai, broonie, gregkh, jank,
srinivas.kandagatla, slawomir.blauciak, Bard liao, Rander Wang,
Ranjani Sridharan, Sanyog Kale
On 13-01-20, 16:56, Pierre-Louis Bossart wrote:
> If the programming of the dev_number fails due to an IO error, a new
> device_number will be assigned, resulting in a leak.
>
> Make sure we only assign a device_number once per Slave device.
Although I am not sure if this would be a leak, we assign a new num and
old number should have gotten recycled as they would be unattached
status.
Anyway this is good improvement as it helps to debug having same
dev_num, so Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] [PATCH] soundwire: bus: fix device number leak on errors
2020-01-14 6:37 ` Vinod Koul
@ 2020-01-14 16:10 ` Pierre-Louis Bossart
0 siblings, 0 replies; 3+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-14 16:10 UTC (permalink / raw)
To: Vinod Koul
Cc: alsa-devel, tiwai, gregkh, linux-kernel, Ranjani Sridharan,
broonie, srinivas.kandagatla, jank, slawomir.blauciak,
Sanyog Kale, Bard liao, Rander Wang
On 1/14/20 12:37 AM, Vinod Koul wrote:
> On 13-01-20, 16:56, Pierre-Louis Bossart wrote:
>> If the programming of the dev_number fails due to an IO error, a new
>> device_number will be assigned, resulting in a leak.
>>
>> Make sure we only assign a device_number once per Slave device.
>
> Although I am not sure if this would be a leak, we assign a new num and
> old number should have gotten recycled as they would be unattached
> status.
When you program the device number and it fails, there is still a
Device0 reporting as attached, so you will loop and try to assign a new
device number. In this case there is never a transition to UNATTACHED,
the Slave remains ATTACHED as Device0 until the enumeration succeed with
a successful non-zero device number.
This only happened to us w/ early prototypes where the PCB routing was
questionable and the speed too high, but still it's useful to keep this
device number constant
> Anyway this is good improvement as it helps to debug having same
> dev_num, so Applied, thanks
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-14 17:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13 22:56 [PATCH] soundwire: bus: fix device number leak on errors Pierre-Louis Bossart
2020-01-14 6:37 ` Vinod Koul
2020-01-14 16:10 ` [alsa-devel] " Pierre-Louis Bossart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).