linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org, tiwai@suse.de, broonie@kernel.org,
	vkoul@kernel.org, gregkh@linuxfoundation.org, jank@cadence.com,
	srinivas.kandagatla@linaro.org, slawomir.blauciak@intel.com,
	Bard liao <yung-chuan.liao@linux.intel.com>,
	Rander Wang <rander.wang@linux.intel.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Sanyog Kale <sanyog.r.kale@intel.com>
Subject: [PATCH v5 01/11] soundwire: sdw_slave: add probe_complete structure and new fields
Date: Wed, 11 Dec 2019 19:44:57 -0600	[thread overview]
Message-ID: <20191212014507.28050-2-pierre-louis.bossart@linux.intel.com> (raw)
In-Reply-To: <20191212014507.28050-1-pierre-louis.bossart@linux.intel.com>

When a Slave device becomes synchronized with the bus, it may report
its presence in PING frames, as well as optionally asserting an
in-band PREQ signal.

The bus driver will detect a new Device0, start the enumeration
process and assign it a non-zero device number. The SoundWire
enumeration provides an arbitration to deal with multiple Slaves
reporting ATTACHED at the same time. The bus driver will also invoke
the driver .probe() callback associated with this device. The probe()
depends on the Linux device core, which handles the match operations
and may result in modules being loaded.

Once the non-zero device number is programmed, the Slave will report
its new status in PING frames and the Master hardware will typically
report this status change with an interrupt. At this point, the
.update_status() callback of the codec driver will be invoked (usually
from an interrupt thread or workqueue scheduled from the interrupt
thread).

The first race condition which can happen is between the .probe(),
which allocates the resources, and .update_status() where
initializations are typically handled. The .probe() is only called
once during the initial boot, while .update_status() will be called
for every bus hardware reset and if the Slave device loses
synchronization (an unlikely event but with non-zero probability).

The time difference between the end of the enumeration process and a
change of status reported by the hardware may be as small as one
SoundWire PING frame. The scheduling of the interrupt thread, which
invokes .update_status() is not deterministic, but can be small enough
to create a race condition. With a 48 kHz frame rate and ideal
scheduling cases, the .probe() may be pre-empted within double-digit
microseconds.

Since there is no guarantee that the .probe() completes by the time
.update_status() is invoked as a result of an interrupt, it's not
unusual for the .update_status() to rely on data structures that have
not been allocated yet, leading to kernel oopses.

This patch adds a probe_complete utility, which is used in the
sdw_update_slave_status() routine. The codec driver does not need to
do anything and can safely assume all resources are allocated in its
update_status() callback.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/linux/soundwire/sdw.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 28745b9ba279..cb1db4a7475d 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -547,6 +547,10 @@ struct sdw_slave_ops {
  * @node: node for bus list
  * @port_ready: Port ready completion flag for each Slave port
  * @dev_num: 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
+ * Slave state changes/implementation-defined interrupts
  */
 struct sdw_slave {
 	struct sdw_slave_id id;
@@ -561,6 +565,8 @@ struct sdw_slave {
 	struct list_head node;
 	struct completion *port_ready;
 	u16 dev_num;
+	bool probed;
+	struct completion probe_complete;
 };
 
 #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev)
-- 
2.20.1


  reply	other threads:[~2019-12-12  1:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-12  1:44 [PATCH v5 00/11] soundwire: update ASoC interfaces Pierre-Louis Bossart
2019-12-12  1:44 ` Pierre-Louis Bossart [this message]
2019-12-12  1:44 ` [PATCH v5 02/11] soundwire: sdw_slave: add enumeration_complete structure Pierre-Louis Bossart
2019-12-12  1:44 ` [PATCH v5 03/11] soundwire: sdw_slave: add initialization_complete definition Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 04/11] soundwire: sdw_slave: track unattach_request to handle all init sequences Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 05/11] soundwire: intel: update interfaces between ASoC and SoundWire Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 06/11] soundwire: intel: update stream callbacks for hwparams/free stream operations Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 07/11] soundwire: intel: update headers for interrupts Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 08/11] soundwire: intel: add link_list to handle interrupts with a single thread Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 09/11] soundwire: intel: add prototype for WAKEEN interrupt processing Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 10/11] soundwire: intel: add mutex for shared SHIM register access Pierre-Louis Bossart
2019-12-12  1:45 ` [PATCH v5 11/11] soundwire: intel: add clock stop quirks Pierre-Louis Bossart
2019-12-12  3:49 ` [PATCH v5 00/11] soundwire: update ASoC interfaces Vinod Koul
2019-12-12 14:37   ` [alsa-devel] " Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191212014507.28050-2-pierre-louis.bossart@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jank@cadence.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rander.wang@linux.intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=sanyog.r.kale@intel.com \
    --cc=slawomir.blauciak@intel.com \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.org \
    --cc=yung-chuan.liao@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).