On 6/18/21 9:47 AM, Richard Fitzgerald wrote:
> In sdw_prep_deprep_slave_ports(), after the wait_for_completion()
> the DP prepare status register is read. If this indicates that the
> port is now prepared, the code should continue with the port setup.
> It is irrelevant whether the wait_for_completion() timed out if the
> port is now ready.
>
> The previous implementation would always fail if the
> wait_for_completion() timed out, even if the port was reporting
> successful prepare.
>
> This patch also fixes a minor bug where the return from sdw_read()
> was not checked for error - any error code with LSBits clear could
> be misinterpreted as a successful port prepare.
>
> Fixes: 79df15b7d37c ("soundwire: Add helpers for ports operations")
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
Looks good to me
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> ---
> drivers/soundwire/stream.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 1eaedaaba094..1a18308f4ef4 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -422,7 +422,6 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
> struct completion *port_ready;
> struct sdw_dpn_prop *dpn_prop;
> struct sdw_prepare_ch prep_ch;
> - unsigned int time_left;
> bool intr = false;
> int ret = 0, val;
> u32 addr;
> @@ -479,15 +478,15 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
>
> /* Wait for completion on port ready */
> port_ready = &s_rt->slave->port_ready[prep_ch.num];
> - time_left = wait_for_completion_timeout(port_ready,
> - msecs_to_jiffies(dpn_prop->ch_prep_timeout));
> + wait_for_completion_timeout(port_ready,
> + msecs_to_jiffies(dpn_prop->ch_prep_timeout));
>
> val = sdw_read(s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
> - val &= p_rt->ch_mask;
> - if (!time_left || val) {
> + if ((val < 0) || (val & p_rt->ch_mask)) {
> + ret = (val < 0) ? val : -ETIMEDOUT;
> dev_err(&s_rt->slave->dev,
> - "Chn prep failed for port:%d\n", prep_ch.num);
> - return -ETIMEDOUT;
> + "Chn prep failed for port %d: %d\n", prep_ch.num, ret);
> + return ret;
> }
> }
>
>
