public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Richard Fitzgerald <rf@opensource.cirrus.com>,
	vkoul@kernel.org, yung-chuan.liao@linux.intel.com,
	sanyog.r.kale@intel.com
Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
	patches@opensource.cirrus.com, jack.yu@realtek.com
Subject: Re: [PATCH 2/3] soundwire: bus: Don't lose unattach notifications
Date: Thu, 25 Aug 2022 14:39:45 +0200	[thread overview]
Message-ID: <718d6503-f284-2cd4-c0a0-b8e90547bee6@linux.intel.com> (raw)
In-Reply-To: <20220825122241.273090-3-rf@opensource.cirrus.com>



On 8/25/22 14:22, Richard Fitzgerald wrote:
> Ensure that if sdw_handle_slave_status() sees a peripheral
> has dropped off the bus it reports it to the client driver.
> 
> If there are any devices reporting on address 0 it bails out
> after programming the device IDs. So it never reaches the second
> loop that calls sdw_update_slave_status().
> 
> If the missing device is one that is now showing as unenumerated
> it has been given a device ID so will report as attached next
> time sdw_handle_slave_status() runs.
> 
> With the previous code the client driver would only see another
> ATTACHED notification because the UNATTACHED state was lost when
> sdw_handle_slave_status() bailed out after programming the
> device ID.
> 
> This shows up most when the peripheral has to be reset after
> downloading updated firmware and there are multiple of these
> peripherals on the bus. They will all return to unenumerated state
> after the reset, and then there is a mix of unattached, attached
> and unenumerated PING states from the peripherals, as each is reset
> and they reboot.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---
>  drivers/soundwire/bus.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index 704f75c0bae2..bb8ce26c68b3 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -1756,6 +1756,11 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
>  			dev_warn(&slave->dev, "Slave %d state check1: UNATTACHED, status was %d\n",
>  				 i, slave->status);
>  			sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED);
> +
> +			/* Ensure driver knows that peripheral unattached */
> +			ret = sdw_update_slave_status(slave, status[i]);
> +			if (ret < 0)
> +				dev_warn(&slave->dev, "Update Slave status failed:%d\n", ret);

This is indeed a good fix, this will make sure the driver will
re-initialize the device when it reports as ATTACHED again.

The codec driver needs to keep track on the UNATTACHED change though.

This is the case in all codec drivers except rt715 and rt715-sdca.
Something to change in a follow-up patch (cc: Jack Yu).

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

>  		}
>  	}
>  

  reply	other threads:[~2022-08-25 14:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-25 12:22 [PATCH 0/3] soundwire: Fixes for spurious and missing UNATTACH Richard Fitzgerald
2022-08-25 12:22 ` [PATCH 1/3] soundwire: cadence: fix updating slave status when a bus has multiple peripherals Richard Fitzgerald
2022-08-25 12:57   ` Pierre-Louis Bossart
2022-08-25 12:22 ` [PATCH 2/3] soundwire: bus: Don't lose unattach notifications Richard Fitzgerald
2022-08-25 12:39   ` Pierre-Louis Bossart [this message]
2022-08-25 12:22 ` [PATCH 3/3] soundwire: bus: Fix lost UNATTACH when re-enumerating Richard Fitzgerald
2022-08-25 14:24   ` Pierre-Louis Bossart
2022-08-25 15:25     ` Richard Fitzgerald
2022-08-26  8:06       ` Pierre-Louis Bossart
2022-08-29  9:50         ` Richard Fitzgerald
2022-08-26 10:38       ` Richard Fitzgerald
2022-08-30  9:00   ` Richard Fitzgerald

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=718d6503-f284-2cd4-c0a0-b8e90547bee6@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=jack.yu@realtek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=rf@opensource.cirrus.com \
    --cc=sanyog.r.kale@intel.com \
    --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