From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
<vkoul@kernel.org>, <yung-chuan.liao@linux.intel.com>,
<lgirdwood@gmail.com>, <peter.ujfalusi@linux.intel.com>,
<ranjani.sridharan@linux.intel.com>,
<kai.vehmanen@linux.intel.com>, <daniel.baluta@nxp.com>,
<sanyog.r.kale@intel.com>, <broonie@kernel.org>
Cc: <patches@opensource.cirrus.com>, <alsa-devel@alsa-project.org>,
<linux-kernel@vger.kernel.org>,
<sound-open-firmware@alsa-project.org>
Subject: Re: [PATCH 5/7] soundwire: intel: Don't disable interrupt until children are removed
Date: Mon, 12 Sep 2022 16:36:09 +0100 [thread overview]
Message-ID: <9de21235-c77d-f53f-3426-4a5927f484c9@opensource.cirrus.com> (raw)
In-Reply-To: <c83fb25e-ef59-63e1-4223-648dab9885ea@linux.intel.com>
On 12/09/2022 11:53, Pierre-Louis Bossart wrote:
>
>
> On 9/7/22 12:14, Richard Fitzgerald wrote:
>> The cadence_master code needs the interrupt to complete message transfers.
>> When the bus driver is being removed child drivers are removed, and their
>> remove actions might need bus transactions.
>>
>> Use the sdw_master_ops.remove callback to disable the interrupt handling
>> only after the child drivers have been removed.
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
>> drivers/soundwire/intel.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
>> index 01be62fa6c83..d5e723a9c80b 100644
>> --- a/drivers/soundwire/intel.c
>> +++ b/drivers/soundwire/intel.c
>> @@ -1255,6 +1255,13 @@ static int intel_prop_read(struct sdw_bus *bus)
>> return 0;
>> }
>>
>> +static void intel_bus_remove(struct sdw_bus *bus)
>> +{
>> + struct sdw_cdns *cdns = bus_to_cdns(bus);
>> +
>> + sdw_cdns_enable_interrupt(cdns, false);
>
> don't you need to check for any on-going transactions on the bus?
>
As all the child drivers have removed, I think the only other place that
can generate bus transactions is the PING handler but
sdw_cdns_enable_interrupt(false) calls cancel_work_sync() to
cancel the cdns->work and it sets a flag so that it will not be
re-queued.
> I wonder if there could be a corner case where there are no child
> devices but still a device physically attached to the bus. I am not sure
> if the 'no devices left' is a good-enough indication of no activity on
> the bus.
>
As above - yes there could, but sdw_cdns_enable_interrupt(false) will
cancel the work and stop it being re-queued.
>> +}
>> +
>> static struct sdw_master_ops sdw_intel_ops = {
>> .read_prop = sdw_master_read_prop,
>> .override_adr = sdw_dmi_override_adr,
>> @@ -1264,6 +1271,7 @@ static struct sdw_master_ops sdw_intel_ops = {
>> .set_bus_conf = cdns_bus_conf,
>> .pre_bank_switch = intel_pre_bank_switch,
>> .post_bank_switch = intel_post_bank_switch,
>> + .remove = intel_bus_remove,
>> };
>>
>> static int intel_init(struct sdw_intel *sdw)
>> @@ -1502,7 +1510,6 @@ static void intel_link_remove(struct auxiliary_device *auxdev)
>> */
>> if (!bus->prop.hw_disabled) {
>> intel_debugfs_exit(sdw);
>> - sdw_cdns_enable_interrupt(cdns, false);
>> snd_soc_unregister_component(dev);
>> }
>> sdw_bus_master_delete(bus);
next prev parent reply other threads:[~2022-09-12 15:36 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-07 10:13 [PATCH 0/7] soundwire: Fix driver removal Richard Fitzgerald
2022-09-07 10:13 ` [PATCH 1/7] soundwire: bus: Do not forcibly disable child pm_runtime Richard Fitzgerald
2022-09-12 10:43 ` Pierre-Louis Bossart
2022-09-07 10:13 ` [PATCH 2/7] soundwire: intel_init: Separate shutdown and cleanup Richard Fitzgerald
2022-09-07 10:13 ` [PATCH 3/7] ASoC: SOF: Intel: Don't disable Soundwire interrupt before the bus has shut down Richard Fitzgerald
2022-09-07 11:26 ` Mark Brown
2022-09-07 10:13 ` [PATCH 4/7] soundwire: bus: Add remove callback to struct sdw_master_ops Richard Fitzgerald
2022-09-07 10:14 ` [PATCH 5/7] soundwire: intel: Don't disable interrupt until children are removed Richard Fitzgerald
2022-09-12 10:53 ` Pierre-Louis Bossart
2022-09-12 15:36 ` Richard Fitzgerald [this message]
2022-09-12 17:12 ` Pierre-Louis Bossart
2022-09-13 9:29 ` Richard Fitzgerald
2022-09-07 10:14 ` [PATCH 6/7] soundwire: intel: Don't disable pm_runtime " Richard Fitzgerald
2022-09-07 10:14 ` [PATCH 7/7] soundwire: bus: Fix premature removal of sdw_slave objects Richard Fitzgerald
2022-09-12 10:57 ` 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=9de21235-c77d-f53f-3426-4a5927f484c9@opensource.cirrus.com \
--to=rf@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=daniel.baluta@nxp.com \
--cc=kai.vehmanen@linux.intel.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=sound-open-firmware@alsa-project.org \
--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).