| Message ID | 20221020015624.1703950-1-yung-chuan.liao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soundwire: intel: Initialize clock stop timeout | expand |
On 10/19/22 20:56, Bard Liao wrote: > From: Sjoerd Simons <sjoerd@collabora.com> > > The bus->clk_stop_timeout member is only initialized to a non-zero value > during the codec driver probe. This can lead to corner cases where this > value remains pegged at zero when the bus suspends, which results in an > endless loop in sdw_bus_wait_for_clk_prep_deprep(). > > Corner cases include configurations with no codecs described in the > firmware, or delays in probing codec drivers. > > Initializing the default timeout to the smallest non-zero value avoid this > problem and allows for the existing logic to be preserved: the > bus->clk_stop_timeout is set as the maximum required by all codecs > connected on the bus. > > Fixes: 1f2dcf3a154ac ("soundwire: intel: set dev_num_ida_min") > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Reviewed-by: Chao Song <chao.song@intel.com> > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com> this patch should be sent to GregKH/Linus as a 6.1-rcx fix, it does seem to make the life of Arch/Debian users less miserable - for some reason very large delays on driver probe seem to trigger this corner case and make things even worse. see https://github.com/thesofproject/linux/issues/3777 for details. Thanks Vinod. > --- > drivers/soundwire/intel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c > index 25ec9c272239..78d35bb4852c 100644 > --- a/drivers/soundwire/intel.c > +++ b/drivers/soundwire/intel.c > @@ -1311,6 +1311,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev, > > bus->link_id = auxdev->id; > bus->dev_num_ida_min = INTEL_DEV_NUM_IDA_MIN; > + bus->clk_stop_timeout = 1; > > sdw_cdns_probe(cdns); >
On 20-10-22, 09:56, Bard Liao wrote: > From: Sjoerd Simons <sjoerd@collabora.com> > > The bus->clk_stop_timeout member is only initialized to a non-zero value > during the codec driver probe. This can lead to corner cases where this > value remains pegged at zero when the bus suspends, which results in an > endless loop in sdw_bus_wait_for_clk_prep_deprep(). > > Corner cases include configurations with no codecs described in the > firmware, or delays in probing codec drivers. > > Initializing the default timeout to the smallest non-zero value avoid this > problem and allows for the existing logic to be preserved: the > bus->clk_stop_timeout is set as the maximum required by all codecs > connected on the bus. Applied to fixes, thanks
On 10/28/22 06:28, Vinod Koul wrote: > On 20-10-22, 09:56, Bard Liao wrote: >> From: Sjoerd Simons <sjoerd@collabora.com> >> >> The bus->clk_stop_timeout member is only initialized to a non-zero value >> during the codec driver probe. This can lead to corner cases where this >> value remains pegged at zero when the bus suspends, which results in an >> endless loop in sdw_bus_wait_for_clk_prep_deprep(). >> >> Corner cases include configurations with no codecs described in the >> firmware, or delays in probing codec drivers. >> >> Initializing the default timeout to the smallest non-zero value avoid this >> problem and allows for the existing logic to be preserved: the >> bus->clk_stop_timeout is set as the maximum required by all codecs >> connected on the bus. > > Applied to fixes, thanks Thanks Vinod, was this sent to Greg/Linus? the last pull request I see was for 6.1-rc1. Arch Linux cherry-picked this patch but other distros did not, so quite a few users are left with no audio card.
On 09-11-22, 10:05, Pierre-Louis Bossart wrote: > > > On 10/28/22 06:28, Vinod Koul wrote: > > On 20-10-22, 09:56, Bard Liao wrote: > >> From: Sjoerd Simons <sjoerd@collabora.com> > >> > >> The bus->clk_stop_timeout member is only initialized to a non-zero value > >> during the codec driver probe. This can lead to corner cases where this > >> value remains pegged at zero when the bus suspends, which results in an > >> endless loop in sdw_bus_wait_for_clk_prep_deprep(). > >> > >> Corner cases include configurations with no codecs described in the > >> firmware, or delays in probing codec drivers. > >> > >> Initializing the default timeout to the smallest non-zero value avoid this > >> problem and allows for the existing logic to be preserved: the > >> bus->clk_stop_timeout is set as the maximum required by all codecs > >> connected on the bus. > > > > Applied to fixes, thanks > > Thanks Vinod, was this sent to Greg/Linus? the last pull request I see > was for 6.1-rc1. > Arch Linux cherry-picked this patch but other distros did not, so quite > a few users are left with no audio card. https://git.kernel.org/torvalds/c/f014699cca9a9a28fbdc06a9225b54562154fc20
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 25ec9c272239..78d35bb4852c 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1311,6 +1311,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev, bus->link_id = auxdev->id; bus->dev_num_ida_min = INTEL_DEV_NUM_IDA_MIN; + bus->clk_stop_timeout = 1; sdw_cdns_probe(cdns);