RE: a question about range_map_cfg
> -Original Message- > From: Mark Brown [mailto:broo...@sirena.org.uk] > Sent: Wednesday, May 29, 2013 10:56 PM > To: Bard Liao > Cc: Stephen Warren > Subject: Re: a question about range_map_cfg > > On Wed, May 29, 2013 at 08:46:40PM +0800, Bard Liao wrote: > > > > However, I /think/ that the window_start/len fields in > > > regmap_range_cfg are meant to represent the "virtual" registers > > > numbers of the registers behind the addr/data keyhole, and hence > > > should be 0x100 and something much larger than 1, respectively. Does that > solve the problem? > > > In my opinion, window_start represents the address of indirectly accessed > registers' data. > > Ie. If we want to access the private register of ALC5640, we should > > write the private register's address to 0x6a which is defined in > > .selector_reg. > > Then read/write it's data from reg 0x6c which is defined in .window_start. > > And the .window_len represents the data length of indirectly accessed > register. > > So if I set .window_len = 2 for example, it will write the private > > register's address to 0x6a, and read/write 0x6c and 0x6d to access the > private register's data. > > But I don't know if my thought is right or not. > > That sounds about right, though obviously that's a *very* small window. > The window is the physical registers through which the range can be seen, the > range is the virtual registers where the windowed region is linearised for > upper layers. > > > I look at regmap-debugfs.c and guess the issue is from > regmap_debugfs_get_dump_start function. > > It add debugfs_off_cache list in if (list_empty(&map->debugfs_off_cache)) > condition. > > So if I cat "PR" first, it will add indirectly accessed registers' range in > debugfs_off_cache. > > But after that if I cat "range" or "registers" debugfs_off_cache is not > > empty. > > So it will not enter if (list_empty(&map->debugfs_off_cache)) condition. > > That's why I will only see indirectly accessed registers' data from "range" > and "registers". > > In the other case, if I cat "registers" or "range" first, > > regmap_debugfs_get_dump_start will add all registers'(including directly > and indirectly accessed registers) range in debugfs_off_cache. > > Yes, the cache should be suppressed in the case where we're not looking at the > full map. Try the patch below: > > From c53da2153185cf3f522ce4952e4148aa7287cb89 Mon Sep 17 00:00:00 > 2001 > From: Mark Brown > Date: Wed, 29 May 2013 15:54:54 +0100 > Subject: [PATCH] regmap: debugfs: Suppress cache for partial register files > > The cache is based on the full register map so confuses things if used for a > partial map. Unfortunately, it does not work. Actually, I see from is always 0, so it never enter the "if (from)" condition. > > Reported-by: Bard Liao > Signed-off-by: Mark Brown > --- > drivers/base/regmap/regmap-debugfs.c |4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/base/regmap/regmap-debugfs.c > b/drivers/base/regmap/regmap-debugfs.c > index 98cb94e..5349575 100644 > --- a/drivers/base/regmap/regmap-debugfs.c > +++ b/drivers/base/regmap/regmap-debugfs.c > @@ -84,6 +84,10 @@ static unsigned int > regmap_debugfs_get_dump_start(struct regmap *map, > unsigned int fpos_offset; > unsigned int reg_offset; > > + /* Suppress the cache if we're using a subrange */ > + if (from) > + return from; > + > /* >* If we don't have a cache build one so we don't have to do a >* linear scan each time. > -- > 1.7.10.4 > > > --Please consider the environment before printing this e-mail. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
> -Original Message-
> From: Carlo Caione [mailto:ca...@endlessm.com]
> Sent: Thursday, October 19, 2017 11:55 PM
> To: Pierre-Louis Bossart
> Cc: Carlo Caione; Linux Upstreaming Team; Bard Liao; Oder Chiou; Mark
> Brown; alsa-de...@alsa-project.org; linux-kernel@vger.kernel.org;
> ti...@suse.com; Albert Chen; Edgar Shen
> Subject: Re: [alsa-devel] [PATCH 1/2] ASoC: rt5651: Enable jack detection on
> JD1_1
>
> On Thu, Oct 19, 2017 at 4:47 PM, Pierre-Louis Bossart
> wrote:
> > On 10/19/17 6:03 AM, Carlo Caione wrote:
> >>
> >> From: Carlo Caione
> >>
> >> Enable jack detection or the RT5651 codec on the JD1_1 pin.
> >
> > Nice, but the codec supports a second jack detection on JD1 and has a
> second
> > JD2 pin. I will bet that some devices will have a different routing and I
> > wonder if we could just add support for all options.
>
> I can write support for that but I have no hardware to actually test
> it, that's why I left those cases out.
>
> >> The codec has no means to detect the type of the jack connected so we
> >> assume that the jack is always an headset jack.
> >
> > that's odd, was this confirmed by Realtek?
>
> The Realtek people are in CC :)
> Probably there is way but in the datasheet there is nothing about that
> (or did I miss it?)
Yes, rt5651 has the capability of jack type detection.
Please see the following code for reference.
+static int rt5651_jack_detect(struct snd_soc_codec *codec, int jack_insert)
+{
+ struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
+ int jack_type;
+
+ if (jack_insert) {
+ snd_soc_dapm_force_enable_pin(dapm, "LDO");
+ snd_soc_dapm_sync(dapm);
+
+ snd_soc_update_bits(codec, RT5651_MICBIAS,
+ RT5651_MIC1_OVCD_MASK | RT5651_MIC1_OVTH_MASK |
+ RT5651_PWR_CLK12M_MASK | RT5651_PWR_MB_MASK,
+ RT5651_MIC1_OVCD_EN | RT5651_MIC1_OVTH_600UA |
+ RT5651_PWR_MB_PU | RT5651_PWR_CLK12M_PU);
+ msleep(100);
+ if (snd_soc_read(codec, RT5651_IRQ_CTRL2) & RT5651_MB1_OC_CLR)
+ jack_type = SND_JACK_HEADPHONE;
+ else
+ jack_type = SND_JACK_HEADSET;
+ snd_soc_update_bits(codec, RT5651_IRQ_CTRL2,
+ RT5651_MB1_OC_CLR, 0);
+ } else { /* jack out */
+ jack_type = 0;
+
+ snd_soc_update_bits(codec, RT5651_MICBIAS,
+ RT5651_MIC1_OVCD_MASK, RT5651_MIC1_OVCD_DIS);
+ }
+
+ return jack_type;
+}
>
> >> Signed-off-by: Carlo Caione
> >> ---
> >> include/sound/rt5651.h| 7
> >> sound/soc/codecs/rt5651.c | 91
> >> +--
> >> sound/soc/codecs/rt5651.h | 3 ++
> >> 3 files changed, 99 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
> >> index d35de758dfb5..c563383149c4 100644
> >> --- a/include/sound/rt5651.h
> >> +++ b/include/sound/rt5651.h
> >> @@ -11,11 +11,18 @@
> >> #ifndef __LINUX_SND_RT5651_H
> >> #define __LINUX_SND_RT5651_H
> >> +enum rt5651_jd_src {
> >> + RT5651_JD_NULL,
> >> + RT5651_JD1_1,
> >> +};
> >> +
> >> struct rt5651_platform_data {
> >> /* IN2 can optionally be differential */
> >> bool in2_diff;
> >> bool dmic_en;
> >> +
> >> + enum rt5651_jd_src jd_src;
> >
> >
> > I don't see code that sets this platform data, is there a quirk or
> > of_property missing in this patchset?
>
> Yes, it is supposed to be enabled by a quirk. In general (personal
> taste) I prefer to post the quirk enabling code after the base code
> has been ACKed. If you feel like it I can post also the quirk code
> together with the next respin of this patchset.
>
> Thank you,
>
> --
> Carlo Caione | +39.340.80.30.096 | Endless
>
> --Please consider the environment before printing this e-mail.
RE: [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
> -Original Message-
> From: Carlo Caione [mailto:carlo.cai...@gmail.com] On Behalf Of Carlo
> Caione
> Sent: Thursday, October 19, 2017 7:03 PM
> To: li...@endlessm.com; Bard Liao; Oder Chiou;
> pierre-louis.boss...@linux.intel.com; broo...@kernel.org;
> alsa-de...@alsa-project.org; linux-kernel@vger.kernel.org; ti...@suse.com;
> Albert Chen; Edgar Shen
> Cc: Carlo Caione
> Subject: [PATCH 1/2] ASoC: rt5651: Enable jack detection on JD1_1
>
> From: Carlo Caione
>
> Enable jack detection or the RT5651 codec on the JD1_1 pin.
> The codec has no means to detect the type of the jack connected so we
> assume that the jack is always an headset jack.
>
> Signed-off-by: Carlo Caione
> ---
> include/sound/rt5651.h| 7
> sound/soc/codecs/rt5651.c | 91
> +--
> sound/soc/codecs/rt5651.h | 3 ++
> 3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/include/sound/rt5651.h b/include/sound/rt5651.h
> index d35de758dfb5..c563383149c4 100644
> --- a/include/sound/rt5651.h
> +++ b/include/sound/rt5651.h
> @@ -11,11 +11,18 @@
> #ifndef __LINUX_SND_RT5651_H
> #define __LINUX_SND_RT5651_H
>
> +enum rt5651_jd_src {
> + RT5651_JD_NULL,
> + RT5651_JD1_1,
> +};
> +
> struct rt5651_platform_data {
> /* IN2 can optionally be differential */
> bool in2_diff;
>
> bool dmic_en;
> +
> + enum rt5651_jd_src jd_src;
> };
>
> #endif
> diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
> index 28f7210cec91..9bc7f31af1a4 100644
> --- a/sound/soc/codecs/rt5651.c
> +++ b/sound/soc/codecs/rt5651.c
> @@ -26,6 +26,7 @@
> #include
> #include
> #include
> +#include
>
> #include "rl6231.h"
> #include "rt5651.h"
> @@ -880,6 +881,9 @@ static const struct snd_soc_dapm_widget
> rt5651_dapm_widgets[] = {
> SND_SOC_DAPM_SUPPLY("PLL1", RT5651_PWR_ANLG2,
> RT5651_PWR_PLL_BIT, 0, NULL, 0),
> /* Input Side */
> + SND_SOC_DAPM_SUPPLY("JD Power", RT5651_PWR_ANLG2,
> + RT5651_PWM_JD_M_BIT, 0, NULL, 0),
> +
> /* micbias */
> SND_SOC_DAPM_SUPPLY("LDO", RT5651_PWR_ANLG1,
> RT5651_PWR_LDO_BIT, 0, NULL, 0),
> @@ -1528,6 +1532,8 @@ static int rt5651_set_dai_pll(struct snd_soc_dai *dai,
> int pll_id, int source,
> static int rt5651_set_bias_level(struct snd_soc_codec *codec,
> enum snd_soc_bias_level level)
> {
> + struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> +
> switch (level) {
> case SND_SOC_BIAS_PREPARE:
> if (SND_SOC_BIAS_STANDBY ==
> snd_soc_codec_get_bias_level(codec)) {
> @@ -1556,8 +1562,13 @@ static int rt5651_set_bias_level(struct
> snd_soc_codec *codec,
> snd_soc_write(codec, RT5651_PWR_DIG2, 0x);
> snd_soc_write(codec, RT5651_PWR_VOL, 0x);
> snd_soc_write(codec, RT5651_PWR_MIXER, 0x);
> - snd_soc_write(codec, RT5651_PWR_ANLG1, 0x);
> - snd_soc_write(codec, RT5651_PWR_ANLG2, 0x);
> + if (rt5651->pdata.jd_src) {
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x0204);
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x0002);
> + } else {
> + snd_soc_write(codec, RT5651_PWR_ANLG1, 0x);
> + snd_soc_write(codec, RT5651_PWR_ANLG2, 0x);
> + }
> break;
>
> default:
> @@ -1570,6 +1581,7 @@ static int rt5651_set_bias_level(struct
> snd_soc_codec *codec,
> static int rt5651_probe(struct snd_soc_codec *codec)
> {
> struct rt5651_priv *rt5651 = snd_soc_codec_get_drvdata(codec);
> + struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>
> rt5651->codec = codec;
>
> @@ -1585,6 +1597,13 @@ static int rt5651_probe(struct snd_soc_codec
> *codec)
>
> snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
>
> + if (rt5651->pdata.jd_src) {
> + snd_soc_dapm_force_enable_pin(dapm, "JD Power");
> + snd_soc_dapm_force_enable_pin(dapm, "PLL1");
"PLL1" is not needed here. What we need is
+ regmap_update_bits(rt5651->regmap, RT5651_MICBIAS,
+ 0x38, 0x38);
It is not showing in the datasheet. Sorry about that.
> + snd_soc_dapm_force_enable_pin(dapm, "LDO");
> + snd_soc_dapm_sync(dapm);
> + }
> +
> return 0;
> }
>
> @@ -1728,6 +1747,42 @@ sta
RE: linux-next: build failure after merge of the sound-asoc tree
Dear All,
I will send a patch to fix it immediately.
Thanks.
Bard Liao
Computer Peripherals Business Unit
Realtek Semiconductor Corp.
886-3-578-0211 ext. 3334
bardl...@realtek.com
> -Original Message-
> From: Stephen Rothwell [mailto:s...@canb.auug.org.au]
> Sent: Thursday, April 30, 2015 10:06 AM
> To: Mark Brown; Liam Girdwood
> Cc: linux-n...@vger.kernel.org; linux-kernel@vger.kernel.org; Bard Liao
> Subject: linux-next: build failure after merge of the sound-asoc tree
>
> Hi all,
>
> After merging the sound-asoc tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> sound/soc/codecs/rt5645.c: In function 'rt5645_set_jack_detect':
> sound/soc/codecs/rt5645.c:2555:2: error: implicit declaration of function
> 'rt5645_irq_detection' [-Werror=implicit-function-declaration]
> rt5645_irq_detection(rt5645);
> ^
> sound/soc/codecs/rt5645.c: At top level:
> sound/soc/codecs/rt5645.c:2591:12: error: static declaration of
> 'rt5645_irq_detection' follows non-static declaration static int
> rt5645_irq_detection(struct rt5645_priv *rt5645)
> ^
> sound/soc/codecs/rt5645.c:2555:2: note: previous implicit declaration of
> 'rt5645_irq_detection' was here
> rt5645_irq_detection(rt5645);
> ^
> sound/soc/codecs/rt5645.c:2591:12: warning: 'rt5645_irq_detection'
> defined but not used [-Wunused-function] static int
> rt5645_irq_detection(struct rt5645_priv *rt5645)
> ^
>
> Caused by commit 6e747d5311fc ("ASoC: rt5645: Adds push button
> support for rt5650").
>
> I have used the sound-asoc tree from next-20150429 for today.
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
>
> --Please consider the environment before printing this e-mail.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] soundwire: bus: clock_stop: don't deal with UNATTACHED Slave devices
We don't need to do anything for the slave if it is unattached during
clock stop prepare and exit sequences.
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 2289c2ac8c5a..405335fb790a 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -863,13 +863,13 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
if (!slave->dev_num)
continue;
- /* Identify if Slave(s) are available on Bus */
- is_slave = true;
-
if (slave->status != SDW_SLAVE_ATTACHED &&
slave->status != SDW_SLAVE_ALERT)
continue;
+ /* Identify if Slave(s) are available on Bus */
+ is_slave = true;
+
slave_mode = sdw_get_clk_stop_mode(slave);
slave->curr_clk_stop_mode = slave_mode;
@@ -900,6 +900,10 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
return ret;
}
+ /* Don't need to inform slaves if there is no slave attached */
+ if (!is_slave)
+ return ret;
+
/* Inform slaves that prep is done */
list_for_each_entry(slave, &bus->slaves, node) {
if (!slave->dev_num)
@@ -985,13 +989,13 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
if (!slave->dev_num)
continue;
- /* Identify if Slave(s) are available on Bus */
- is_slave = true;
-
if (slave->status != SDW_SLAVE_ATTACHED &&
slave->status != SDW_SLAVE_ALERT)
continue;
+ /* Identify if Slave(s) are available on Bus */
+ is_slave = true;
+
mode = slave->curr_clk_stop_mode;
if (mode == SDW_CLK_STOP_MODE1) {
@@ -1016,6 +1020,13 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
if (is_slave && !simple_clk_stop)
sdw_bus_wait_for_clk_prep_deprep(bus, SDW_BROADCAST_DEV_NUM);
+ /*
+* Don't need to call slave callback function if there is no slave
+* attached
+*/
+ if (!is_slave)
+ return 0;
+
list_for_each_entry(slave, &bus->slaves, node) {
if (!slave->dev_num)
continue;
--
2.17.1
[PATCH] soundwire: clarify SPDX use of GPL-2.0
From: Pierre-Louis Bossart Change SPDX from GPL-2.0 to GPL-2.0-only for Intel-contributed code. This was explicit before the transition to SPDX and lost in translation. No change to the dual-license parts, the only clarification is to the GPL-2.0 term. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Guennadi Liakhovetski Signed-off-by: Bard Liao --- drivers/soundwire/bus.c | 2 +- drivers/soundwire/bus.h | 2 +- drivers/soundwire/bus_type.c| 2 +- drivers/soundwire/cadence_master.c | 2 +- drivers/soundwire/cadence_master.h | 2 +- drivers/soundwire/intel.c | 2 +- drivers/soundwire/intel.h | 2 +- drivers/soundwire/intel_init.c | 2 +- drivers/soundwire/mipi_disco.c | 2 +- drivers/soundwire/slave.c | 2 +- drivers/soundwire/stream.c | 2 +- include/linux/soundwire/sdw.h | 2 +- include/linux/soundwire/sdw_intel.h | 2 +- include/linux/soundwire/sdw_registers.h | 2 +- include/linux/soundwire/sdw_type.h | 2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 24ba77226376..2289c2ac8c5a 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. #include diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 82484f741168..697a5b371568 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */ /* Copyright(c) 2015-17 Intel Corporation. */ #ifndef __SDW_BUS_H diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index de9a671802b8..95a87d4e26c2 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-only // Copyright(c) 2015-17 Intel Corporation. #include diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 9ea87538b9ef..fe238645ed2c 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. /* diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index b410656f8194..ea606b4fba76 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */ /* Copyright(c) 2015-17 Intel Corporation. */ #include diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 4cfdd074e310..1eda63f488f5 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. /* diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index 38b7c125fb10..7b4af8018e1a 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */ /* Copyright(c) 2015-17 Intel Corporation. */ #ifndef __SDW_INTEL_LOCAL_H diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index d5d42795a48f..947345d5c960 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. /* diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c index 4ae62b452b8c..dbd8fc6f4dad 100644 --- a/drivers/soundwire/mipi_disco.c +++ b/drivers/soundwire/mipi_disco.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. /* diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 0839445ee07b..8e8b62dff51c 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // Copyright(c) 2015-17 Intel Corporation. #include diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index a9a72574b34a..296cea48bdcc 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire
[PATCH v2 2/6] soundwire: intel: clarify drvdata and remove more indirections
From: Pierre-Louis Bossart
The use of drvdata mixes two structures. There was no harm the first
structure is embedded as the first element of the second, but that's
not good. Make sure all drvdata is based on the 'sdw_cdns' structure.
While we are at it, remove indirections for 'dev' and 'cdns' to make
the code more readable.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10e55c708694..ef1196ea93a4 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1077,6 +1077,7 @@ static int intel_master_probe(struct platform_device
*pdev)
struct sdw_cdns_stream_config config;
struct device *dev = &pdev->dev;
struct sdw_intel *sdw;
+ struct sdw_cdns *cdns;
struct sdw_bus *bus;
int ret;
@@ -1084,24 +1085,26 @@ static int intel_master_probe(struct platform_device
*pdev)
if (!sdw)
return -ENOMEM;
- bus = &sdw->cdns.bus;
+ cdns = &sdw->cdns;
+ bus = &cdns->bus;
sdw->instance = pdev->id;
sdw->link_res = dev_get_platdata(dev);
- sdw->cdns.dev = dev;
- sdw->cdns.registers = sdw->link_res->registers;
- sdw->cdns.instance = sdw->instance;
- sdw->cdns.msg_count = 0;
+ cdns->dev = dev;
+ cdns->registers = sdw->link_res->registers;
+ cdns->instance = sdw->instance;
+ cdns->msg_count = 0;
+
bus->link_id = pdev->id;
- sdw_cdns_probe(&sdw->cdns);
+ sdw_cdns_probe(cdns);
/* Set property read ops */
sdw_intel_ops.read_prop = intel_prop_read;
bus->ops = &sdw_intel_ops;
/* set driver data, accessed by snd_soc_dai_get_drvdata() */
- platform_set_drvdata(pdev, sdw);
+ dev_set_drvdata(dev, cdns);
ret = sdw_bus_master_add(bus, dev, dev->fwnode);
if (ret) {
@@ -1123,7 +1126,7 @@ static int intel_master_probe(struct platform_device
*pdev)
/* Read the PDI config and initialize cadence PDI */
intel_pdi_init(sdw, &config);
- ret = sdw_cdns_pdi_init(&sdw->cdns, config);
+ ret = sdw_cdns_pdi_init(cdns, config);
if (ret)
goto err_init;
@@ -1132,20 +1135,20 @@ static int intel_master_probe(struct platform_device
*pdev)
/* Acquire IRQ */
ret = request_threaded_irq(sdw->link_res->irq,
sdw_cdns_irq, sdw_cdns_thread,
- IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
+ IRQF_SHARED, KBUILD_MODNAME, cdns);
if (ret < 0) {
dev_err(dev, "unable to grab IRQ %d, disabling device\n",
sdw->link_res->irq);
goto err_init;
}
- ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
+ ret = sdw_cdns_enable_interrupt(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable interrupts\n");
goto err_init;
}
- ret = sdw_cdns_exit_reset(&sdw->cdns);
+ ret = sdw_cdns_exit_reset(cdns);
if (ret < 0) {
dev_err(dev, "unable to exit bus reset sequence\n");
goto err_interrupt;
@@ -1164,7 +1167,7 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
err_interrupt:
- sdw_cdns_enable_interrupt(&sdw->cdns, false);
+ sdw_cdns_enable_interrupt(cdns, false);
free_irq(sdw->link_res->irq, sdw);
err_init:
sdw_bus_master_delete(bus);
@@ -1174,16 +1177,13 @@ static int intel_master_probe(struct platform_device
*pdev)
static int intel_master_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct sdw_intel *sdw;
- struct sdw_bus *bus;
-
- sdw = platform_get_drvdata(pdev);
-
- bus = &sdw->cdns.bus;
+ struct sdw_cdns *cdns = dev_get_drvdata(dev);
+ struct sdw_intel *sdw = cdns_to_intel(cdns);
+ struct sdw_bus *bus = &cdns->bus;
if (!bus->prop.hw_disabled) {
intel_debugfs_exit(sdw);
- sdw_cdns_enable_interrupt(&sdw->cdns, false);
+ sdw_cdns_enable_interrupt(cdns, false);
free_irq(sdw->link_res->irq, sdw);
snd_soc_unregister_component(dev);
}
--
2.17.1
[PATCH v2 3/6] soundwire: intel_init: remove useless test
From: Pierre-Louis Bossart
No need to test link_mask twice
Suggested-by: Rander Wang
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index de0405359db5..e87f17240547 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -109,7 +109,7 @@ static struct sdw_intel_ctx
/* Create SDW Master devices */
for (i = 0; i < count; i++) {
- if (link_mask && !(link_mask & BIT(i))) {
+ if (!(link_mask & BIT(i))) {
dev_dbg(&adev->dev,
"Link %d masked, will not be enabled\n", i);
link++;
--
2.17.1
[PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization
This series is to split the original "soundwire: intel: transition to 3 steps initialization" patch into different patches for better review. It also address comments from Vinod. Pierre-Louis Bossart (6): soundwire: intel: cleanups for indirections/logs soundwire: intel: clarify drvdata and remove more indirections soundwire: intel_init: remove useless test soundwire: intel_init: use devm_ allocation soundwire: intel_init: pass link information as platform data soundwire: intel: transition to 3 steps initialization drivers/soundwire/intel.c | 138 + drivers/soundwire/intel.h | 13 ++ drivers/soundwire/intel_init.c | 261 ++--- 3 files changed, 295 insertions(+), 117 deletions(-) -- 2.17.1
[PATCH v2 1/6] soundwire: intel: cleanups for indirections/logs
From: Pierre-Louis Bossart
The code can be simplified a bit to have a more consistent use of
'dev' and 'bus', as well as move definitions around. This will help
make the major changes in follow-up patches easier to review.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 68 +-
drivers/soundwire/intel.h | 11 ++
drivers/soundwire/intel_init.c | 1 +
3 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 1eda63f488f5..10e55c708694 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -92,23 +92,12 @@
#define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0)
#define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16)
-#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
-
enum intel_pdi_type {
INTEL_PDI_IN = 0,
INTEL_PDI_OUT = 1,
INTEL_PDI_BD = 2,
};
-struct sdw_intel {
- struct sdw_cdns cdns;
- int instance;
- struct sdw_intel_link_res *link_res;
-#ifdef CONFIG_DEBUG_FS
- struct dentry *debugfs;
-#endif
-};
-
#define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)
/*
@@ -1083,41 +1072,47 @@ static int intel_init(struct sdw_intel *sdw)
/*
* probe and init
*/
-static int intel_probe(struct platform_device *pdev)
+static int intel_master_probe(struct platform_device *pdev)
{
struct sdw_cdns_stream_config config;
+ struct device *dev = &pdev->dev;
struct sdw_intel *sdw;
+ struct sdw_bus *bus;
int ret;
- sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL);
+ sdw = devm_kzalloc(dev, sizeof(*sdw), GFP_KERNEL);
if (!sdw)
return -ENOMEM;
+ bus = &sdw->cdns.bus;
+
sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(&pdev->dev);
- sdw->cdns.dev = &pdev->dev;
+ sdw->link_res = dev_get_platdata(dev);
+ sdw->cdns.dev = dev;
sdw->cdns.registers = sdw->link_res->registers;
sdw->cdns.instance = sdw->instance;
sdw->cdns.msg_count = 0;
- sdw->cdns.bus.link_id = pdev->id;
+ bus->link_id = pdev->id;
sdw_cdns_probe(&sdw->cdns);
/* Set property read ops */
sdw_intel_ops.read_prop = intel_prop_read;
- sdw->cdns.bus.ops = &sdw_intel_ops;
+ bus->ops = &sdw_intel_ops;
+ /* set driver data, accessed by snd_soc_dai_get_drvdata() */
platform_set_drvdata(pdev, sdw);
- ret = sdw_bus_master_add(&sdw->cdns.bus, &pdev->dev, pdev->dev.fwnode);
+ ret = sdw_bus_master_add(bus, dev, dev->fwnode);
if (ret) {
- dev_err(&pdev->dev, "sdw_bus_master_add fail: %d\n", ret);
+ dev_err(dev, "sdw_bus_master_add fail: %d\n", ret);
return ret;
}
- if (sdw->cdns.bus.prop.hw_disabled) {
- dev_info(&pdev->dev, "SoundWire master %d is disabled,
ignoring\n",
-sdw->cdns.bus.link_id);
+ if (bus->prop.hw_disabled) {
+ dev_info(dev,
+"SoundWire master %d is disabled, will be ignored\n",
+bus->link_id);
return 0;
}
@@ -1139,28 +1134,28 @@ static int intel_probe(struct platform_device *pdev)
sdw_cdns_irq, sdw_cdns_thread,
IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
if (ret < 0) {
- dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling
device\n",
+ dev_err(dev, "unable to grab IRQ %d, disabling device\n",
sdw->link_res->irq);
goto err_init;
}
ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
if (ret < 0) {
- dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
+ dev_err(dev, "cannot enable interrupts\n");
goto err_init;
}
ret = sdw_cdns_exit_reset(&sdw->cdns);
if (ret < 0) {
- dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
+ dev_err(dev, "unable to exit bus reset sequence\n");
goto err_interrupt;
}
/* Register DAIs */
ret = intel_register_dai(sdw);
if (ret) {
- dev_err(sdw->cdns.dev, "DAI registration failed: %d\n", ret);
- snd_soc_unregister_component(sdw->cdns.dev);
+ dev_err(dev, "DAI registration failed: %d\n", ret);
+ snd_soc_unregister_component
[PATCH v2 6/6] soundwire: intel: transition to 3 steps initialization
From: Pierre-Louis Bossart
Rather than a plain-vanilla init/exit, this patch provides 3 steps in
the initialization needed for driver selection, machine driver
selection and deal with power rail dependencies.
- ACPI scan: this step is done at a very early stage to detect the
presence of a SoundWire Controller and enabled links at the BIOS
level. This step may be called from the legacy HDaudio driver, which
will abort its probe to let the Sound Open Firmware (SOF) handle the
hardware.
- probe: this step allocates all the required memory and will add a
sdw_bus, which in turn will result in identifying all possible Slaves
listed below the Controller ACPI companion device. All the information
is reported to the parent PCI driver which will select the relevant
machine driver.
- startup: this last step starts the bus reset, which results in Slave
devices reporting as ATTACHED and being enumerated. This step is only
done during the card creation stage, after the DSP is powered to
account for internal power rail dependencies.
These 3 steps are already supported in the Sound Open firmware
drivers and upstream.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 60 +---
drivers/soundwire/intel.h | 2 +
drivers/soundwire/intel_init.c | 242 ++---
3 files changed, 237 insertions(+), 67 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index ef1196ea93a4..f966ac33cd3b 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1074,7 +1074,6 @@ static int intel_init(struct sdw_intel *sdw)
*/
static int intel_master_probe(struct platform_device *pdev)
{
- struct sdw_cdns_stream_config config;
struct device *dev = &pdev->dev;
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
@@ -1112,25 +,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return ret;
}
- if (bus->prop.hw_disabled) {
+ if (bus->prop.hw_disabled)
dev_info(dev,
"SoundWire master %d is disabled, will be ignored\n",
bus->link_id);
- return 0;
- }
-
- /* Initialize shim, controller and Cadence IP */
- ret = intel_init(sdw);
- if (ret)
- goto err_init;
-
- /* Read the PDI config and initialize cadence PDI */
- intel_pdi_init(sdw, &config);
- ret = sdw_cdns_pdi_init(cdns, config);
- if (ret)
- goto err_init;
-
- intel_pdi_ch_update(sdw);
/* Acquire IRQ */
ret = request_threaded_irq(sdw->link_res->irq,
@@ -1142,6 +1126,42 @@ static int intel_master_probe(struct platform_device
*pdev)
goto err_init;
}
+ return 0;
+
+err_init:
+ sdw_bus_master_delete(bus);
+ return ret;
+}
+
+int intel_master_startup(struct platform_device *pdev)
+{
+ struct sdw_cdns_stream_config config;
+ struct device *dev = &pdev->dev;
+ struct sdw_cdns *cdns = dev_get_drvdata(dev);
+ struct sdw_intel *sdw = cdns_to_intel(cdns);
+ struct sdw_bus *bus = &cdns->bus;
+ int ret;
+
+ if (bus->prop.hw_disabled) {
+ dev_info(dev,
+"SoundWire master %d is disabled, ignoring\n",
+sdw->instance);
+ return 0;
+ }
+
+ /* Initialize shim, controller and Cadence IP */
+ ret = intel_init(sdw);
+ if (ret)
+ goto err_init;
+
+ /* Read the PDI config and initialize cadence PDI */
+ intel_pdi_init(sdw, &config);
+ ret = sdw_cdns_pdi_init(cdns, config);
+ if (ret)
+ goto err_init;
+
+ intel_pdi_ch_update(sdw);
+
ret = sdw_cdns_enable_interrupt(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable interrupts\n");
@@ -1168,9 +1188,7 @@ static int intel_master_probe(struct platform_device
*pdev)
err_interrupt:
sdw_cdns_enable_interrupt(cdns, false);
- free_irq(sdw->link_res->irq, sdw);
err_init:
- sdw_bus_master_delete(bus);
return ret;
}
@@ -1196,12 +1214,12 @@ static struct platform_driver sdw_intel_drv = {
.probe = intel_master_probe,
.remove = intel_master_remove,
.driver = {
- .name = "int-sdw",
+ .name = "intel-sdw",
},
};
module_platform_driver(sdw_intel_drv);
MODULE_LICENSE("Dual BSD/GPL");
-MODULE_ALIAS("platform:int-sdw");
+MODULE_ALIAS("platform:intel-sdw");
MODULE_DESCRIPTION("Intel Soundwire Master Driver");
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 001968c362cd..1ff71eb4c219 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
[PATCH v2 5/6] soundwire: intel_init: pass link information as platform data
From: Pierre-Louis Bossart
It's not clear how this code ever worked, the link information is used
in intel.c but never passed as platform_data.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel_init.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index f9d190157c47..ff1017e64abd 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -128,6 +128,8 @@ static struct sdw_intel_ctx
pdevinfo.name = "int-sdw";
pdevinfo.id = i;
pdevinfo.fwnode = acpi_fwnode_handle(adev);
+ pdevinfo.data = link;
+ pdevinfo.size_data = sizeof(*link);
pdev = platform_device_register_full(&pdevinfo);
if (IS_ERR(pdev)) {
--
2.17.1
[PATCH v2 4/6] soundwire: intel_init: use devm_ allocation
From: Pierre-Louis Bossart
Make error handling simpler with devm_ allocation.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel_init.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index e87f17240547..f9d190157c47 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -42,9 +42,6 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
link++;
}
- kfree(ctx->links);
- ctx->links = NULL;
-
return 0;
}
@@ -96,14 +93,15 @@ static struct sdw_intel_ctx
dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
- ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return NULL;
ctx->count = count;
- ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL);
+ ctx->links = devm_kcalloc(&adev->dev, ctx->count,
+ sizeof(*ctx->links), GFP_KERNEL);
if (!ctx->links)
- goto link_err;
+ return NULL;
link = ctx->links;
@@ -146,9 +144,8 @@ static struct sdw_intel_ctx
return ctx;
pdev_err:
+ ctx->count = i;
sdw_intel_cleanup_pdev(ctx);
-link_err:
- kfree(ctx);
return NULL;
}
@@ -216,7 +213,6 @@ void *sdw_intel_init(acpi_handle *parent_handle, struct
sdw_intel_res *res)
void sdw_intel_exit(struct sdw_intel_ctx *ctx)
{
sdw_intel_cleanup_pdev(ctx);
- kfree(ctx);
}
EXPORT_SYMBOL(sdw_intel_exit);
--
2.17.1
[PATCH v2 0/5] soundwire: bus_type: add sdw_master_device support
This series adds sdw master devices support. changes in v2: - Allocate sdw_master_device dynamically - Use unique bus id as master id - Keep checking parent devices - Enable runtime_pm on Master device Bard Liao (2): soundwire: bus: add unique bus id soundwire: master: add runtime pm support Pierre-Louis Bossart (3): soundwire: bus: rename sdw_bus_master_add/delete, add arguments soundwire: bus_type: introduce sdw_slave_type and sdw_master_type soundwire: bus_type: add sdw_master_device support .../driver-api/soundwire/summary.rst | 7 +- drivers/soundwire/Makefile| 2 +- drivers/soundwire/bus.c | 47 -- drivers/soundwire/bus.h | 3 + drivers/soundwire/bus_type.c | 19 ++-- drivers/soundwire/intel.c | 9 +- drivers/soundwire/master.c| 88 +++ drivers/soundwire/qcom.c | 7 +- drivers/soundwire/slave.c | 8 +- include/linux/soundwire/sdw.h | 24 - include/linux/soundwire/sdw_type.h| 9 +- 11 files changed, 191 insertions(+), 32 deletions(-) create mode 100644 drivers/soundwire/master.c -- 2.17.1
[PATCH v2 2/5] soundwire: bus_type: introduce sdw_slave_type and sdw_master_type
From: Pierre-Louis Bossart
this is a preparatory patch before the introduction of the
sdw_master_type. The SoundWire slave support is slightly modified with
the use of a sdw_slave_type, and the uevent handling move to
slave.c (since it's not necessary for the master).
No functionality change other than moving code around.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/bus_type.c | 19 +--
drivers/soundwire/slave.c | 8 +++-
include/linux/soundwire/sdw_type.h | 9 -
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 17f096dd6806..2c1a19caba51 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -33,13 +33,21 @@ sdw_get_device_id(struct sdw_slave *slave, struct
sdw_driver *drv)
static int sdw_bus_match(struct device *dev, struct device_driver *ddrv)
{
- struct sdw_slave *slave = dev_to_sdw_dev(dev);
- struct sdw_driver *drv = drv_to_sdw_driver(ddrv);
+ struct sdw_slave *slave;
+ struct sdw_driver *drv;
+ int ret = 0;
+
+ if (is_sdw_slave(dev)) {
+ slave = dev_to_sdw_dev(dev);
+ drv = drv_to_sdw_driver(ddrv);
- return !!sdw_get_device_id(slave, drv);
+ ret = !!sdw_get_device_id(slave, drv);
+ }
+ return ret;
}
-int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size)
+static int sdw_slave_modalias(const struct sdw_slave *slave, char *buf,
+ size_t size)
{
/* modalias is sdw:mp */
@@ -47,7 +55,7 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char
*buf, size_t size)
slave->id.mfg_id, slave->id.part_id);
}
-static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env)
+int sdw_slave_uevent(struct device *dev, struct kobj_uevent_env *env)
{
struct sdw_slave *slave = dev_to_sdw_dev(dev);
char modalias[32];
@@ -63,7 +71,6 @@ static int sdw_uevent(struct device *dev, struct
kobj_uevent_env *env)
struct bus_type sdw_bus_type = {
.name = "soundwire",
.match = sdw_bus_match,
- .uevent = sdw_uevent,
};
EXPORT_SYMBOL_GPL(sdw_bus_type);
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index aace57fae7f8..ed068a004bd9 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -14,6 +14,12 @@ static void sdw_slave_release(struct device *dev)
kfree(slave);
}
+struct device_type sdw_slave_type = {
+ .name = "sdw_slave",
+ .release = sdw_slave_release,
+ .uevent = sdw_slave_uevent,
+};
+
static int sdw_slave_add(struct sdw_bus *bus,
struct sdw_slave_id *id, struct fwnode_handle *fwnode)
{
@@ -41,9 +47,9 @@ static int sdw_slave_add(struct sdw_bus *bus,
id->class_id, id->unique_id);
}
- slave->dev.release = sdw_slave_release;
slave->dev.bus = &sdw_bus_type;
slave->dev.of_node = of_node_get(to_of_node(fwnode));
+ slave->dev.type = &sdw_slave_type;
slave->bus = bus;
slave->status = SDW_SLAVE_UNATTACHED;
init_completion(&slave->enumeration_complete);
diff --git a/include/linux/soundwire/sdw_type.h
b/include/linux/soundwire/sdw_type.h
index aaa7f4267c14..52eb66cd11bc 100644
--- a/include/linux/soundwire/sdw_type.h
+++ b/include/linux/soundwire/sdw_type.h
@@ -5,6 +5,13 @@
#define __SOUNDWIRE_TYPES_H
extern struct bus_type sdw_bus_type;
+extern struct device_type sdw_slave_type;
+extern struct device_type sdw_master_type;
+
+static inline int is_sdw_slave(const struct device *dev)
+{
+ return dev->type == &sdw_slave_type;
+}
#define drv_to_sdw_driver(_drv) container_of(_drv, struct sdw_driver, driver)
@@ -14,7 +21,7 @@ extern struct bus_type sdw_bus_type;
int __sdw_register_driver(struct sdw_driver *drv, struct module *owner);
void sdw_unregister_driver(struct sdw_driver *drv);
-int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size);
+int sdw_slave_uevent(struct device *dev, struct kobj_uevent_env *env);
/**
* module_sdw_driver() - Helper macro for registering a Soundwire driver
--
2.17.1
[PATCH v2 5/5] soundwire: master: add runtime pm support
We need to enable runtime_pm on master device with generic helpers,
so that a Slave-initiated wake is propagated to the bus parent.
Signed-off-by: Bard Liao
---
drivers/soundwire/master.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c
index 6be0a027def7..5411791e6aff 100644
--- a/drivers/soundwire/master.c
+++ b/drivers/soundwire/master.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include
#include
#include "bus.h"
@@ -14,9 +15,15 @@ static void sdw_master_device_release(struct device *dev)
kfree(md);
}
+static const struct dev_pm_ops master_dev_pm = {
+ SET_RUNTIME_PM_OPS(pm_generic_runtime_suspend,
+ pm_generic_runtime_resume, NULL)
+};
+
struct device_type sdw_master_type = {
.name = "soundwire_master",
.release = sdw_master_device_release,
+ .pm = &master_dev_pm,
};
/**
--
2.17.1
[PATCH v2 4/5] soundwire: bus_type: add sdw_master_device support
From: Pierre-Louis Bossart
In the existing SoundWire code, Master Devices are not explicitly
represented - only SoundWire Slave Devices are exposed (the use of
capital letters follows the SoundWire specification conventions).
With the existing code, the bus is handled without using a proper device,
and bus->dev typically points to a platform device. The right thing to
do as discussed in multiple reviews is use a device for each bus.
The sdw_master_device addition is done with minimal internal plumbing
and not exposed externally. The existing API based on
sdw_bus_master_add() and sdw_bus_master_delete() will deal with the
sdw_master_device life cycle, which minimizes changes to existing
drivers.
Note that the Intel code will be modified in follow-up patches (no
impact on any platform since the connection with ASoC is not supported
upstream so far).
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/Makefile| 2 +-
drivers/soundwire/bus.c | 14 --
drivers/soundwire/bus.h | 3 ++
drivers/soundwire/intel.c | 1 -
drivers/soundwire/master.c| 81 +++
drivers/soundwire/qcom.c | 1 -
include/linux/soundwire/sdw.h | 17 +++-
7 files changed, 112 insertions(+), 7 deletions(-)
create mode 100644 drivers/soundwire/master.c
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
index e2cdff990e9f..7319918e0aec 100644
--- a/drivers/soundwire/Makefile
+++ b/drivers/soundwire/Makefile
@@ -4,7 +4,7 @@
#
#Bus Objs
-soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
+soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o
obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
ifdef CONFIG_DEBUG_FS
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 2d24f183061d..c31a1c2788a9 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -37,14 +37,21 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
struct sdw_master_prop *prop = NULL;
int ret;
- if (!bus->dev) {
- pr_err("SoundWire bus has no device\n");
+ if (!parent) {
+ pr_err("SoundWire parent device is not set\n");
return -ENODEV;
}
ret = sdw_get_id(bus);
if (ret) {
- dev_err(bus->dev, "Failed to get bus id\n");
+ dev_err(parent, "Failed to get bus id\n");
+ return ret;
+ }
+
+ ret = sdw_master_device_add(bus, parent, fwnode);
+ if (ret) {
+ dev_err(parent, "Failed to add master device at link %d\n",
+ bus->link_id);
return ret;
}
@@ -161,6 +168,7 @@ static int sdw_delete_slave(struct device *dev, void *data)
void sdw_bus_master_delete(struct sdw_bus *bus)
{
device_for_each_child(bus->dev, NULL, sdw_delete_slave);
+ sdw_master_device_del(bus);
sdw_bus_debugfs_exit(bus);
ida_free(&sdw_ida, bus->id);
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 204204a26db8..93ab0234a491 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -19,6 +19,9 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus)
int sdw_of_find_slaves(struct sdw_bus *bus);
void sdw_extract_slave_id(struct sdw_bus *bus,
u64 addr, struct sdw_slave_id *id);
+int sdw_master_device_add(struct sdw_bus *bus, struct device *parent,
+ struct fwnode_handle *fwnode);
+int sdw_master_device_del(struct sdw_bus *bus);
#ifdef CONFIG_DEBUG_FS
void sdw_bus_debugfs_init(struct sdw_bus *bus);
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 210459390046..3562f2106e30 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1099,7 +1099,6 @@ static int intel_probe(struct platform_device *pdev)
sdw->cdns.registers = sdw->link_res->registers;
sdw->cdns.instance = sdw->instance;
sdw->cdns.msg_count = 0;
- sdw->cdns.bus.dev = &pdev->dev;
sdw->cdns.bus.link_id = pdev->id;
sdw_cdns_probe(&sdw->cdns);
diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c
new file mode 100644
index ..6be0a027def7
--- /dev/null
+++ b/drivers/soundwire/master.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright(c) 2019-2020 Intel Corporation.
+
+#include
+#include
+#include
+#include
+#include "bus.h"
+
+static void sdw_master_device_release(struct device *dev)
+{
+ struct sdw_master_device *md = dev_to_sdw_master_device(dev);
+
+ kfree(md);
+}
+
+struct device_type sdw_master_type = {
+ .name = "soundwire_master",
+ .release = sdw_master_device_release,
+};
+
+/**
+ * sdw_master_device_
[PATCH v2 1/5] soundwire: bus: rename sdw_bus_master_add/delete, add arguments
From: Pierre-Louis Bossart
In preparation for future extensions, rename functions to use
sdw_bus_master prefix and add a parent and fwnode argument to
sdw_bus_master_add to help with device registration in follow-up
patches.
No functionality change, just renames and additional arguments.
The Intel code is currently unused, the two additional arguments are
only needed for compilation.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
Documentation/driver-api/soundwire/summary.rst | 7 ---
drivers/soundwire/bus.c| 15 +--
drivers/soundwire/intel.c | 8
drivers/soundwire/qcom.c | 6 +++---
include/linux/soundwire/sdw.h | 5 +++--
5 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/Documentation/driver-api/soundwire/summary.rst
b/Documentation/driver-api/soundwire/summary.rst
index 8193125a2bfb..01dcb954f6d7 100644
--- a/Documentation/driver-api/soundwire/summary.rst
+++ b/Documentation/driver-api/soundwire/summary.rst
@@ -101,10 +101,11 @@ Following is the Bus API to register the SoundWire Bus:
.. code-block:: c
- int sdw_add_bus_master(struct sdw_bus *bus)
+ int sdw_bus_master_add(struct sdw_bus *bus,
+ struct device *parent,
+ struct fwnode_handle)
{
- if (!bus->dev)
- return -ENODEV;
+ sdw_master_device_add(bus, parent, fwnode);
mutex_init(&bus->lock);
INIT_LIST_HEAD(&bus->slaves);
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 32de017f08d5..24064dbd74fa 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -10,13 +10,16 @@
#include "bus.h"
/**
- * sdw_add_bus_master() - add a bus Master instance
+ * sdw_bus_master_add() - add a bus Master instance
* @bus: bus instance
+ * @parent: parent device
+ * @fwnode: firmware node handle
*
* Initializes the bus instance, read properties and create child
* devices.
*/
-int sdw_add_bus_master(struct sdw_bus *bus)
+int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
+ struct fwnode_handle *fwnode)
{
struct sdw_master_prop *prop = NULL;
int ret;
@@ -107,7 +110,7 @@ int sdw_add_bus_master(struct sdw_bus *bus)
return 0;
}
-EXPORT_SYMBOL(sdw_add_bus_master);
+EXPORT_SYMBOL(sdw_bus_master_add);
static int sdw_delete_slave(struct device *dev, void *data)
{
@@ -131,18 +134,18 @@ static int sdw_delete_slave(struct device *dev, void
*data)
}
/**
- * sdw_delete_bus_master() - delete the bus master instance
+ * sdw_bus_master_delete() - delete the bus master instance
* @bus: bus to be deleted
*
* Remove the instance, delete the child devices.
*/
-void sdw_delete_bus_master(struct sdw_bus *bus)
+void sdw_bus_master_delete(struct sdw_bus *bus)
{
device_for_each_child(bus->dev, NULL, sdw_delete_slave);
sdw_bus_debugfs_exit(bus);
}
-EXPORT_SYMBOL(sdw_delete_bus_master);
+EXPORT_SYMBOL(sdw_bus_master_delete);
/*
* SDW IO Calls
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 3c83e76c6bf9..210459390046 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1110,9 +1110,9 @@ static int intel_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, sdw);
- ret = sdw_add_bus_master(&sdw->cdns.bus);
+ ret = sdw_bus_master_add(&sdw->cdns.bus, &pdev->dev, pdev->dev.fwnode);
if (ret) {
- dev_err(&pdev->dev, "sdw_add_bus_master fail: %d\n", ret);
+ dev_err(&pdev->dev, "sdw_bus_master_add fail: %d\n", ret);
return ret;
}
@@ -1173,7 +1173,7 @@ static int intel_probe(struct platform_device *pdev)
sdw_cdns_enable_interrupt(&sdw->cdns, false);
free_irq(sdw->link_res->irq, sdw);
err_init:
- sdw_delete_bus_master(&sdw->cdns.bus);
+ sdw_bus_master_delete(&sdw->cdns.bus);
return ret;
}
@@ -1189,7 +1189,7 @@ static int intel_remove(struct platform_device *pdev)
free_irq(sdw->link_res->irq, sdw);
snd_soc_unregister_component(sdw->cdns.dev);
}
- sdw_delete_bus_master(&sdw->cdns.bus);
+ sdw_bus_master_delete(&sdw->cdns.bus);
return 0;
}
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index e08a17c13f92..401811d6627e 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -821,7 +821,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
goto err_clk;
}
- ret = sdw_add_bus_master(&ctrl->bus);
+ ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
if (ret) {
[PATCH v2 3/5] soundwire: bus: add unique bus id
Adding an unique id for each bus.
Suggested-by: Vinod Koul
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 20
include/linux/soundwire/sdw.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 24064dbd74fa..2d24f183061d 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -9,6 +9,19 @@
#include
#include "bus.h"
+static DEFINE_IDA(sdw_ida);
+
+static int sdw_get_id(struct sdw_bus *bus)
+{
+ int rc = ida_alloc(&sdw_ida, GFP_KERNEL);
+
+ if (rc < 0)
+ return rc;
+
+ bus->id = rc;
+ return 0;
+}
+
/**
* sdw_bus_master_add() - add a bus Master instance
* @bus: bus instance
@@ -29,6 +42,12 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
return -ENODEV;
}
+ ret = sdw_get_id(bus);
+ if (ret) {
+ dev_err(bus->dev, "Failed to get bus id\n");
+ return ret;
+ }
+
if (!bus->ops) {
dev_err(bus->dev, "SoundWire Bus ops are not set\n");
return -EINVAL;
@@ -144,6 +163,7 @@ void sdw_bus_master_delete(struct sdw_bus *bus)
device_for_each_child(bus->dev, NULL, sdw_delete_slave);
sdw_bus_debugfs_exit(bus);
+ ida_free(&sdw_ida, bus->id);
}
EXPORT_SYMBOL(sdw_bus_master_delete);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 2003e8c55538..a32cb26f1815 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -789,6 +789,7 @@ struct sdw_master_ops {
* struct sdw_bus - SoundWire bus
* @dev: Master linux device
* @link_id: Link id number, can be 0 to N, unique for each Master
+ * @id: bus system-wide unique id
* @slaves: list of Slaves on this bus
* @assigned: Bitmap for Slave device numbers.
* Bit set implies used number, bit clear implies unused number.
@@ -813,6 +814,7 @@ struct sdw_master_ops {
struct sdw_bus {
struct device *dev;
unsigned int link_id;
+ int id;
struct list_head slaves;
DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
struct mutex bus_lock;
--
2.17.1
[PATCH 0/3] soundwire: add sysfs support
Add soundwire sysfs support. Pierre-Louis Bossart (3): soundwire: disco: s/ch/channels/ soundwire: master: add sysfs support soundwire: add Slave sysfs support .../ABI/testing/sysfs-bus-soundwire-master| 23 ++ .../ABI/testing/sysfs-bus-soundwire-slave | 91 ++ drivers/soundwire/Makefile| 3 +- drivers/soundwire/bus.c | 1 + drivers/soundwire/bus.h | 1 + drivers/soundwire/bus_type.c | 9 +- drivers/soundwire/master.c| 84 + drivers/soundwire/mipi_disco.c| 11 +- drivers/soundwire/sysfs_local.h | 14 + drivers/soundwire/sysfs_slave.c | 215 + drivers/soundwire/sysfs_slave_dpn.c | 300 ++ include/linux/soundwire/sdw.h | 8 +- 12 files changed, 748 insertions(+), 12 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-soundwire-master create mode 100644 Documentation/ABI/testing/sysfs-bus-soundwire-slave create mode 100644 drivers/soundwire/sysfs_local.h create mode 100644 drivers/soundwire/sysfs_slave.c create mode 100644 drivers/soundwire/sysfs_slave_dpn.c -- 2.17.1
[PATCH 2/3] soundwire: master: add sysfs support
From: Pierre-Louis Bossart
Add the master properties as attributes. The description is directly
derived from the MIPI DisCo specification.
Credits: this patch is based on an earlier internal contribution by
Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
.../ABI/testing/sysfs-bus-soundwire-master| 23 +
drivers/soundwire/master.c| 84 +++
2 files changed, 107 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-soundwire-master
diff --git a/Documentation/ABI/testing/sysfs-bus-soundwire-master
b/Documentation/ABI/testing/sysfs-bus-soundwire-master
new file mode 100644
index ..46ef038d8722
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-soundwire-master
@@ -0,0 +1,23 @@
+What: /sys/bus/soundwire/devices/sdw-master-N/revision
+ /sys/bus/soundwire/devices/sdw-master-N/clk_stop_modes
+ /sys/bus/soundwire/devices/sdw-master-N/clk_freq
+ /sys/bus/soundwire/devices/sdw-master-N/clk_gears
+ /sys/bus/soundwire/devices/sdw-master-N/default_col
+ /sys/bus/soundwire/devices/sdw-master-N/default_frame_rate
+ /sys/bus/soundwire/devices/sdw-master-N/default_row
+ /sys/bus/soundwire/devices/sdw-master-N/dynamic_shape
+ /sys/bus/soundwire/devices/sdw-master-N/err_threshold
+ /sys/bus/soundwire/devices/sdw-master-N/max_clk_freq
+
+Date: April 2020
+
+Contact: Pierre-Louis Bossart
+ Bard Liao
+ Vinod Koul
+
+Description: SoundWire Master-N DisCo properties.
+ These properties are defined by MIPI DisCo Specification
+ for SoundWire. They define various properties of the Master
+ and are used by the bus to configure the Master. clk_stop_modes
+ is a bitmask for simplifications and combines the
+ clock-stop-mode0 and clock-stop-mode1 properties.
diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c
index 5411791e6aff..5f0b2189defe 100644
--- a/drivers/soundwire/master.c
+++ b/drivers/soundwire/master.c
@@ -8,6 +8,89 @@
#include
#include "bus.h"
+/*
+ * The sysfs for properties reflects the MIPI description as given
+ * in the MIPI DisCo spec
+ *
+ * Base file is:
+ * sdw-master-N
+ * | revision
+ * | clk_stop_modes
+ * | max_clk_freq
+ * | clk_freq
+ * | clk_gears
+ * | default_row
+ * | default_col
+ * | dynamic_shape
+ * | err_threshold
+ */
+
+#define sdw_master_attr(field, format_string) \
+static ssize_t field##_show(struct device *dev,
\
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct sdw_master_device *md = dev_to_sdw_master_device(dev); \
+ return sprintf(buf, format_string, md->bus->prop.field);\
+} \
+static DEVICE_ATTR_RO(field)
+
+sdw_master_attr(revision, "0x%x\n");
+sdw_master_attr(clk_stop_modes, "0x%x\n");
+sdw_master_attr(max_clk_freq, "%d\n");
+sdw_master_attr(default_row, "%d\n");
+sdw_master_attr(default_col, "%d\n");
+sdw_master_attr(default_frame_rate, "%d\n");
+sdw_master_attr(dynamic_frame, "%d\n");
+sdw_master_attr(err_threshold, "%d\n");
+
+static ssize_t clock_frequencies_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct sdw_master_device *md = dev_to_sdw_master_device(dev);
+ ssize_t size = 0;
+ int i;
+
+ for (i = 0; i < md->bus->prop.num_clk_freq; i++)
+ size += sprintf(buf + size, "%8d ",
+ md->bus->prop.clk_freq[i]);
+ size += sprintf(buf + size, "\n");
+
+ return size;
+}
+static DEVICE_ATTR_RO(clock_frequencies);
+
+static ssize_t clock_gears_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct sdw_master_device *md = dev_to_sdw_master_device(dev);
+ ssize_t size = 0;
+ int i;
+
+ for (i = 0; i < md->bus->prop.num_clk_gears; i++)
+ size += sprintf(buf + size, "%8d ",
+ md->bus->prop.clk_gears[i]);
+ size += sprintf(buf + size, "\n");
+
+ return size;
+}
+static DEVICE_ATTR_RO(clock_gears);
+
+static struct attribute *master_node_attrs[] = {
+ &dev_attr_revision.attr,
+ &dev_at
[PATCH 1/3] soundwire: disco: s/ch/channels/
From: Pierre-Louis Bossart
Use more meaningful member names in preparation for sysfs support.
No functionality change.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/mipi_disco.c | 11 ++-
include/linux/soundwire/sdw.h | 8
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 844e6b22974f..4ae62b452b8c 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -231,16 +231,17 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
nval = fwnode_property_count_u32(node,
"mipi-sdw-channel-number-list");
if (nval > 0) {
- dpn[i].num_ch = nval;
- dpn[i].ch = devm_kcalloc(&slave->dev, dpn[i].num_ch,
-sizeof(*dpn[i].ch),
+ dpn[i].num_channels = nval;
+ dpn[i].channels = devm_kcalloc(&slave->dev,
+ dpn[i].num_channels,
+ sizeof(*dpn[i].channels),
GFP_KERNEL);
- if (!dpn[i].ch)
+ if (!dpn[i].channels)
return -ENOMEM;
fwnode_property_read_u32_array(node,
"mipi-sdw-channel-number-list",
- dpn[i].ch, dpn[i].num_ch);
+ dpn[i].channels, dpn[i].num_channels);
}
nval = fwnode_property_count_u32(node,
"mipi-sdw-channel-combination-list");
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 7658d9698dd5..9c27a32df9bb 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -291,8 +291,8 @@ struct sdw_dpn_audio_mode {
* implementation-defined interrupts
* @max_ch: Maximum channels supported
* @min_ch: Minimum channels supported
- * @num_ch: Number of discrete channels supported
- * @ch: Discrete channels supported
+ * @num_channels: Number of discrete channels supported
+ * @channels: Discrete channels supported
* @num_ch_combinations: Number of channel combinations supported
* @ch_combinations: Channel combinations supported
* @modes: SDW mode supported
@@ -316,8 +316,8 @@ struct sdw_dpn_prop {
u32 imp_def_interrupts;
u32 max_ch;
u32 min_ch;
- u32 num_ch;
- u32 *ch;
+ u32 num_channels;
+ u32 *channels;
u32 num_ch_combinations;
u32 *ch_combinations;
u32 modes;
--
2.17.1
[PATCH 3/3] soundwire: add Slave sysfs support
From: Pierre-Louis Bossart Expose MIPI DisCo Slave properties in sysfs. For Slave properties and Data Port 0, the attributes are managed with simple devm_ support. A Slave Device may have more than one Data Port (DPN), and each Data Port can be sink or source. The attributes are created dynamically using pre-canned macros, but still use devm_ with a name attribute group to avoid creating kobjects - as requested by GregKH. In the _show function, we use container_of() to retrieve port number and direction required to extract the information. Audio modes are not supported for now. Depending on the discussions the SoundWire Device Class, we may add it later as is or follow the new specification. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- .../ABI/testing/sysfs-bus-soundwire-slave | 91 ++ drivers/soundwire/Makefile| 3 +- drivers/soundwire/bus.c | 1 + drivers/soundwire/bus.h | 1 + drivers/soundwire/bus_type.c | 9 +- drivers/soundwire/sysfs_local.h | 14 + drivers/soundwire/sysfs_slave.c | 215 + drivers/soundwire/sysfs_slave_dpn.c | 300 ++ 8 files changed, 631 insertions(+), 3 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-soundwire-slave create mode 100644 drivers/soundwire/sysfs_local.h create mode 100644 drivers/soundwire/sysfs_slave.c create mode 100644 drivers/soundwire/sysfs_slave_dpn.c diff --git a/Documentation/ABI/testing/sysfs-bus-soundwire-slave b/Documentation/ABI/testing/sysfs-bus-soundwire-slave new file mode 100644 index ..db4c9511d1aa --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-soundwire-slave @@ -0,0 +1,91 @@ +What: /sys/bus/soundwire/devices/sdw:.../dev-properties/mipi_revision + /sys/bus/soundwire/devices/sdw:.../dev-properties/wake_capable + /sys/bus/soundwire/devices/sdw:.../dev-properties/test_mode_capable + /sys/bus/soundwire/devices/sdw:.../dev-properties/clk_stop_mode1 + /sys/bus/soundwire/devices/sdw:.../dev-properties/simple_clk_stop_capable + /sys/bus/soundwire/devices/sdw:.../dev-properties/clk_stop_timeout + /sys/bus/soundwire/devices/sdw:.../dev-properties/ch_prep_timeout + /sys/bus/soundwire/devices/sdw:.../dev-properties/reset_behave + /sys/bus/soundwire/devices/sdw:.../dev-properties/high_PHY_capable + /sys/bus/soundwire/devices/sdw:.../dev-properties/paging_support + /sys/bus/soundwire/devices/sdw:.../dev-properties/bank_delay_support + /sys/bus/soundwire/devices/sdw:.../dev-properties/p15_behave + /sys/bus/soundwire/devices/sdw:.../dev-properties/master_count + /sys/bus/soundwire/devices/sdw:.../dev-properties/source_ports + /sys/bus/soundwire/devices/sdw:.../dev-properties/sink_ports + +Date: May 2020 + +Contact: Pierre-Louis Bossart + Bard Liao + Vinod Koul + +Description: SoundWire Slave DisCo properties. + These properties are defined by MIPI DisCo Specification + for SoundWire. They define various properties of the + SoundWire Slave and are used by the bus to configure + the Slave + + +What: /sys/bus/soundwire/devices/sdw:.../dp0/max_word + /sys/bus/soundwire/devices/sdw:.../dp0/min_word + /sys/bus/soundwire/devices/sdw:.../dp0/words + /sys/bus/soundwire/devices/sdw:.../dp0/BRA_flow_controlled + /sys/bus/soundwire/devices/sdw:.../dp0/simple_ch_prep_sm + /sys/bus/soundwire/devices/sdw:.../dp0/imp_def_interrupts + +Date: May 2020 + +Contact: Pierre-Louis Bossart + Bard Liao + Vinod Koul + +Description: SoundWire Slave Data Port-0 DisCo properties. + These properties are defined by MIPI DisCo Specification + for the SoundWire. They define various properties of the + Data port 0 are used by the bus to configure the Data Port 0. + + +What: /sys/bus/soundwire/devices/sdw:.../dpN_src/max_word + /sys/bus/soundwire/devices/sdw:.../dpN_src/min_word + /sys/bus/soundwire/devices/sdw:.../dpN_src/words + /sys/bus/soundwire/devices/sdw:.../dpN_src/type + /sys/bus/soundwire/devices/sdw:.../dpN_src/max_grouping + /sys/bus/soundwire/devices/sdw:.../dpN_src/simple_ch_prep_sm + /sys/bus/soundwire/devices/sdw:.../dpN_src/ch_prep_timeout + /sys/bus/soundwire/devices/sdw:.../dpN_src/imp_def_interrupts + /sys/bus/soundwire/devices/sdw:.../dpN_src/min_ch + /sys/bus/soundwire/devices/sdw:.../dpN_src/max_ch + /sys/bus/soundwire
[PATCH 2/2] soundwire: intel: transition to 3 steps initialization
From: Pierre-Louis Bossart
Rather than a plain-vanilla init/exit, this patch provides 3 steps in
the initialization (ACPI scan, probe, startup) which makes it easier to
detect platform support for SoundWire, allocate required resources as
early as possible, and conversely help make the startup() callback
lighter-weight with only hardware register setup.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 76 +-
drivers/soundwire/intel.h | 15 ++
drivers/soundwire/intel_init.c | 249 ++---
3 files changed, 250 insertions(+), 90 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 4cfdd074e310..48e5712fbdf9 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -92,23 +92,12 @@
#define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0)
#define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16)
-#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1)
-
enum intel_pdi_type {
INTEL_PDI_IN = 0,
INTEL_PDI_OUT = 1,
INTEL_PDI_BD = 2,
};
-struct sdw_intel {
- struct sdw_cdns cdns;
- int instance;
- struct sdw_intel_link_res *link_res;
-#ifdef CONFIG_DEBUG_FS
- struct dentry *debugfs;
-#endif
-};
-
#define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)
/*
@@ -1083,20 +1072,18 @@ static int intel_init(struct sdw_intel *sdw)
/*
* probe and init
*/
-static int intel_probe(struct platform_device *pdev)
+static int intel_master_probe(struct platform_device *pdev)
{
- struct sdw_cdns_stream_config config;
+ struct device *dev = &pdev->dev;
struct sdw_intel *sdw;
int ret;
- sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL);
+ sdw = devm_kzalloc(dev, sizeof(*sdw), GFP_KERNEL);
if (!sdw)
return -ENOMEM;
sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(&pdev->dev);
- sdw->cdns.dev = &pdev->dev;
- sdw->cdns.registers = sdw->link_res->registers;
+ sdw->cdns.dev = dev;
sdw->cdns.instance = sdw->instance;
sdw->cdns.msg_count = 0;
sdw->cdns.bus.link_id = pdev->id;
@@ -1109,15 +1096,36 @@ static int intel_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, sdw);
- ret = sdw_bus_master_add(&sdw->cdns.bus, &pdev->dev, pdev->dev.fwnode);
+ ret = sdw_bus_master_add(&sdw->cdns.bus, dev, dev->fwnode);
if (ret) {
- dev_err(&pdev->dev, "sdw_bus_master_add fail: %d\n", ret);
+ dev_err(dev, "sdw_bus_master_add fail: %d\n", ret);
return ret;
}
- if (sdw->cdns.bus.prop.hw_disabled) {
- dev_info(&pdev->dev, "SoundWire master %d is disabled,
ignoring\n",
+ if (sdw->cdns.bus.prop.hw_disabled)
+ dev_info(dev,
+"SoundWire master %d is disabled, ignoring\n",
sdw->cdns.bus.link_id);
+
+ /* set driver data, accessed by snd_soc_dai_set_drvdata() */
+ dev_set_drvdata(sdw->cdns.bus.dev, &sdw->cdns);
+
+ return 0;
+}
+
+int intel_master_startup(struct platform_device *pdev)
+{
+ struct sdw_cdns_stream_config config;
+ struct device *dev = &pdev->dev;
+ struct sdw_intel *sdw;
+ int ret;
+
+ sdw = platform_get_drvdata(pdev);
+
+ if (sdw->cdns.bus.prop.hw_disabled) {
+ dev_info(dev,
+"SoundWire master %d is disabled, ignoring\n",
+sdw->instance);
return 0;
}
@@ -1134,25 +1142,15 @@ static int intel_probe(struct platform_device *pdev)
intel_pdi_ch_update(sdw);
- /* Acquire IRQ */
- ret = request_threaded_irq(sdw->link_res->irq,
- sdw_cdns_irq, sdw_cdns_thread,
- IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
- if (ret < 0) {
- dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling
device\n",
- sdw->link_res->irq);
- goto err_init;
- }
-
ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
if (ret < 0) {
- dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
+ dev_err(dev, "cannot enable interrupts\n");
goto err_init;
}
ret = sdw_cdns_exit_reset(&sdw->cdns);
if (ret < 0) {
- dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
+ dev_err(dev, "unable to exit bus reset sequence\n");
goto err_interrupt;
[PATCH 1/2] soundwire: intel: use a single module
From: Rander Wang It's not clear why we have two modules for the Intel controller/master support when there is a single Kconfig. This adds complexity for no good reason, the two parts need to work together anyways. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/Makefile | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 7319918e0aec..4f6094767212 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -16,12 +16,9 @@ soundwire-cadence-objs := cadence_master.o obj-$(CONFIG_SOUNDWIRE_CADENCE) += soundwire-cadence.o #Intel driver -soundwire-intel-objs :=intel.o +soundwire-intel-objs :=intel.o intel_init.o obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel.o -soundwire-intel-init-objs := intel_init.o -obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel-init.o - #Qualcomm driver soundwire-qcom-objs := qcom.o obj-$(CONFIG_SOUNDWIRE_QCOM) += soundwire-qcom.o -- 2.17.1
[PATCH] soundwire: intel: fix CONFIG_PM and CONFIG_PM_SLEEP confusion
From: Pierre-Louis Bossart
When CONFIG_PM_SLEEP is not defined, GCC throws compilation warnings:
drivers/soundwire/intel.c:1816:12: warning: ‘intel_resume’ defined but
not used [-Wunused-function]
1816 | static int intel_resume(struct device *dev)
|^~~~
drivers/soundwire/intel.c:1697:12: warning: ‘intel_suspend’ defined
but not used [-Wunused-function]
1697 | static int intel_suspend(struct device *dev)
Fix by adding the missing CONFIG_PM_SLEEP.
Note that we could move code around and use only 2 ifdefs, but this
will generate conflicts so let's do this when all the pm handling is
merged.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index dbcbe2708563..a2f0026cb2c1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1525,7 +1525,7 @@ int intel_master_process_wakeen_event(struct
platform_device *pdev)
* PM calls
*/
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int intel_suspend(struct device *dev)
{
@@ -1562,6 +1562,9 @@ static int intel_suspend(struct device *dev)
return 0;
}
+#endif
+
+#ifdef CONFIG_PM
static int intel_suspend_runtime(struct device *dev)
{
@@ -1624,6 +1627,9 @@ static int intel_suspend_runtime(struct device *dev)
return ret;
}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
static int intel_resume(struct device *dev)
{
@@ -1691,6 +1697,9 @@ static int intel_resume(struct device *dev)
return ret;
}
+#endif
+
+#ifdef CONFIG_PM
static int intel_resume_runtime(struct device *dev)
{
@@ -1797,7 +1806,6 @@ static int intel_resume_runtime(struct device *dev)
return ret;
}
-
#endif
static const struct dev_pm_ops intel_pm = {
--
2.17.1
[PATCH v2 00/12] soundwire: intel: add power management support
This series adds power management support for Intel soundwire links. Changes in v2: - Move "#include " to the first required patch. - Fit debug log in single line. Bard Liao (1): soundwire: intel: reinitialize IP+DSP in .prepare(), but only when resuming Pierre-Louis Bossart (9): soundwire: intel: add pm_runtime support soundwire: intel: reset pm_runtime status during system resume soundwire: intel: fix race condition on system resume soundwire: intel: call helper to reset Slave states on resume soundwire: intel: pm_runtime idle scheduling soundwire: intel: add CLK_STOP_TEARDOWN for pm_runtime suspend soundwire: intel: add CLK_STOP_NOT_ALLOWED support soundwire: intel_init: handle power rail dependencies for clock stop mode soundwire: intel: support clock_stop mode without quirks Rander Wang (2): soundwire: intel: add CLK_STOP_BUS_RESET support soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET drivers/soundwire/cadence_master.h | 4 + drivers/soundwire/intel.c | 363 - drivers/soundwire/intel.h | 2 + drivers/soundwire/intel_init.c | 19 +- 4 files changed, 382 insertions(+), 6 deletions(-) -- 2.17.1
[PATCH v2 02/12] soundwire: intel: reset pm_runtime status during system resume
From: Pierre-Louis Bossart
The system resume does the entire bus re-initialization and brings it
to full-power. If the device was pm_runtime suspended, there is no
need to run the pm_runtime resume sequence after the system runtime.
Follow the documentation from runtime_pm.rst, and conditionally
disable, set_active and re-enable the device on system resume.
Note that pm_runtime_suspended() is used instead of
pm_runtime_status_suspended() so that we can deal with the case where
pm_runtime is disabled.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 85a0bb6af4fe..0e21bae3cd19 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1434,6 +1434,12 @@ static int intel_suspend(struct device *dev)
return 0;
}
+ if (pm_runtime_suspended(dev)) {
+ dev_dbg(dev, "%s: pm_runtime status: suspended\n", __func__);
+
+ return 0;
+ }
+
ret = sdw_cdns_enable_interrupt(cdns, false);
if (ret < 0) {
dev_err(dev, "cannot disable interrupts on suspend\n");
@@ -1494,6 +1500,16 @@ static int intel_resume(struct device *dev)
return 0;
}
+ if (pm_runtime_suspended(dev)) {
+ dev_dbg(dev, "%s: pm_runtime status was suspended, forcing
active\n", __func__);
+
+ /* follow required sequence from runtime_pm.rst */
+ pm_runtime_disable(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_enable(dev);
+ }
+
ret = intel_init(sdw);
if (ret) {
dev_err(dev, "%s failed: %d", __func__, ret);
--
2.17.1
[PATCH v2 08/12] soundwire: intel: add CLK_STOP_BUS_RESET support
From: Rander Wang
Move existing pm_runtime suspend under the CLK_STOP_TEARDOWN case.
In this mode the Master IP will lose all context but in-band wakes are
supported.
On pm_runtime resume a complete re-enumeration will be performed after
a bus reset.
Signed-off-by: Rander Wang
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 44 +++
1 file changed, 44 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 68c1cdfb7999..ad476e9e4d25 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1575,6 +1575,26 @@ static int intel_suspend_runtime(struct device *dev)
intel_shim_wake(sdw, false);
+ } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
+ ret = sdw_cdns_clock_stop(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable clock stop on suspend\n");
+ return ret;
+ }
+
+ ret = sdw_cdns_enable_interrupt(cdns, false);
+ if (ret < 0) {
+ dev_err(dev, "cannot disable interrupts on suspend\n");
+ return ret;
+ }
+
+ ret = intel_link_power_down(sdw);
+ if (ret) {
+ dev_err(dev, "Link power down failed: %d", ret);
+ return ret;
+ }
+
+ intel_shim_wake(sdw, true);
} else {
dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
__func__, clock_stop_quirks);
@@ -1691,6 +1711,30 @@ static int intel_resume_runtime(struct device *dev)
dev_err(dev, "unable to exit bus reset sequence during
resume\n");
return ret;
}
+ } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
+ ret = intel_init(sdw);
+ if (ret) {
+ dev_err(dev, "%s failed: %d", __func__, ret);
+ return ret;
+ }
+
+ /*
+* make sure all Slaves are tagged as UNATTACHED and
+* provide reason for reinitialization
+*/
+ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
+
+ ret = sdw_cdns_enable_interrupt(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable interrupts during
resume\n");
+ return ret;
+ }
+
+ ret = sdw_cdns_clock_restart(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "unable to restart clock during resume\n");
+ return ret;
+ }
} else {
dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
__func__, clock_stop_quirks);
--
2.17.1
[PATCH v2 12/12] soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET
From: Rander Wang
When all the links are suspended, the HDaudio controller may suspend
and the power rails to the SoundWire IP may be disabled, requiring a
complete re-initialization/enumeration on resume. However, if one or
more Masters remained active, the HDaudio controller will remain active
and the power rails will remain enabled. As a result, during the link
resume step we can check if the context was preserved by verifying if
the clock was stopped, and avoid doing a complete bus reset and
re-enumeration.
Signed-off-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 2899445e2649..dbcbe2708563 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1698,6 +1698,8 @@ static int intel_resume_runtime(struct device *dev)
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
u32 clock_stop_quirks;
+ bool clock_stop0;
+ int status;
int ret;
if (bus->prop.hw_disabled) {
@@ -1739,11 +1741,24 @@ static int intel_resume_runtime(struct device *dev)
return ret;
}
+ /*
+* An exception condition occurs for the CLK_STOP_BUS_RESET
+* case if one or more masters remain active. In this condition,
+* all the masters are powered on for they are in the same power
+* domain. Master can preserve its context for clock stop0, so
+* there is no need to clear slave status and reset bus.
+*/
+ clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
+
/*
* make sure all Slaves are tagged as UNATTACHED and
* provide reason for reinitialization
*/
- sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
+ if (!clock_stop0) {
+ status = SDW_UNATTACH_REQUEST_MASTER_RESET;
+ sdw_clear_slave_status(bus, status);
+ }
+
ret = sdw_cdns_enable_interrupt(cdns, true);
if (ret < 0) {
@@ -1751,7 +1766,7 @@ static int intel_resume_runtime(struct device *dev)
return ret;
}
- ret = sdw_cdns_clock_restart(cdns, true);
+ ret = sdw_cdns_clock_restart(cdns, !clock_stop0);
if (ret < 0) {
dev_err(dev, "unable to restart clock during resume\n");
return ret;
--
2.17.1
[PATCH v2 05/12] soundwire: intel: reinitialize IP+DSP in .prepare(), but only when resuming
The .prepare() callback is invoked for normal streaming, underflows or
during the system resume transition. In the latter case, the context
for the ALH PDIs is lost, and the DSP is not initialized properly
either, but the bus parameters don't need to be recomputed.
Conversely, when doing a regular .prepare() during an underflow, the
ALH/SHIM registers shall not be changed as the hardware cannot be
reprogrammed after the DMA started (hardware spec requirement).
This patch adds storage of PDI and hw_params in the DAI dma context,
and the difference between the types of .prepare() usages is handled
via a simple boolean, updated when suspending, and tested for in the
.prepare() case.
Signed-off-by: Bard Liao
Signed-off-by: Pierre-Louis Bossart
---
drivers/soundwire/cadence_master.h | 4 ++
drivers/soundwire/intel.c | 71 +-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/cadence_master.h
b/drivers/soundwire/cadence_master.h
index 7638858397df..fdec62b912d3 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -84,6 +84,8 @@ struct sdw_cdns_stream_config {
* @bus: Bus handle
* @stream_type: Stream type
* @link_id: Master link id
+ * @hw_params: hw_params to be applied in .prepare step
+ * @suspended: status set when suspended, to be used in .prepare
*/
struct sdw_cdns_dma_data {
char *name;
@@ -92,6 +94,8 @@ struct sdw_cdns_dma_data {
struct sdw_bus *bus;
enum sdw_stream_type stream_type;
int link_id;
+ struct snd_pcm_hw_params *hw_params;
+ bool suspended;
};
/**
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10dd0e208ce7..95a1d88a5bfb 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -879,6 +879,10 @@ static int intel_hw_params(struct snd_pcm_substream
*substream,
intel_pdi_alh_configure(sdw, pdi);
sdw_cdns_config_stream(cdns, ch, dir, pdi);
+ /* store pdi and hw_params, may be needed in prepare step */
+ dma->suspended = false;
+ dma->pdi = pdi;
+ dma->hw_params = params;
/* Inform DSP about PDI stream number */
ret = intel_params_stream(sdw, substream, dai, params,
@@ -922,7 +926,11 @@ static int intel_hw_params(struct snd_pcm_substream
*substream,
static int intel_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
+ struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
+ struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_cdns_dma_data *dma;
+ int ch, dir;
+ int ret;
dma = snd_soc_dai_get_dma_data(dai, substream);
if (!dma) {
@@ -931,7 +939,41 @@ static int intel_prepare(struct snd_pcm_substream
*substream,
return -EIO;
}
- return sdw_prepare_stream(dma->stream);
+ if (dma->suspended) {
+ dma->suspended = false;
+
+ /*
+* .prepare() is called after system resume, where we
+* need to reinitialize the SHIM/ALH/Cadence IP.
+* .prepare() is also called to deal with underflows,
+* but in those cases we cannot touch ALH/SHIM
+* registers
+*/
+
+ /* configure stream */
+ ch = params_channels(dma->hw_params);
+ if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+ dir = SDW_DATA_DIR_RX;
+ else
+ dir = SDW_DATA_DIR_TX;
+
+ intel_pdi_shim_configure(sdw, dma->pdi);
+ intel_pdi_alh_configure(sdw, dma->pdi);
+ sdw_cdns_config_stream(cdns, ch, dir, dma->pdi);
+
+ /* Inform DSP about PDI stream number */
+ ret = intel_params_stream(sdw, substream, dai,
+ dma->hw_params,
+ sdw->instance,
+ dma->pdi->intel_alh_id);
+ if (ret)
+ goto err;
+ }
+
+ ret = sdw_prepare_stream(dma->stream);
+
+err:
+ return ret;
}
static int intel_trigger(struct snd_pcm_substream *substream, int cmd,
@@ -1002,6 +1044,9 @@ intel_hw_free(struct snd_pcm_substream *substream, struct
snd_soc_dai *dai)
return ret;
}
+ dma->hw_params = NULL;
+ dma->pdi = NULL;
+
return 0;
}
@@ -1014,6 +1059,29 @@ static void intel_shutdown(struct snd_pcm_substream
*substream,
pm_runtime_put_autosuspend(cdns->dev);
}
+static int intel_component_dais_suspend(struct snd_soc_component *component)
+{
+ struct sdw_cdns_dma_data *dma;
+ struct snd_soc_dai *dai;
+
+ for_each_component_dais(component, dai) {
+ /*
+
[PATCH v2 10/12] soundwire: intel_init: handle power rail dependencies for clock stop mode
From: Pierre-Louis Bossart
When none of the clock stop quirks is specified, the Master IP will
assume the context is preserved and will not reset the Bus and restart
enumeration. Due to power rail dependencies, the HDaudio controller
needs to remain powered and prevented from executing its pm_runtime
suspend routine.
This choice of course has a power impact, and this mode should only be
selected when latency requirements are critical or the parent device
can enter D0ix modes.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel_init.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index dd1050743dca..add46d8fc85c 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -73,6 +73,9 @@ static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx)
pm_runtime_disable(&link->pdev->dev);
platform_device_unregister(link->pdev);
}
+
+ if (!link->clock_stop_quirks)
+ pm_runtime_put_noidle(link->dev);
}
return 0;
@@ -338,6 +341,16 @@ sdw_intel_startup_controller(struct sdw_intel_ctx *ctx)
continue;
intel_master_startup(link->pdev);
+
+ if (!link->clock_stop_quirks) {
+ /*
+* we need to prevent the parent PCI device
+* from entering pm_runtime suspend, so that
+* power rails to the SoundWire IP are not
+* turned off.
+*/
+ pm_runtime_get_noresume(link->dev);
+ }
}
return 0;
--
2.17.1
[PATCH v2 01/12] soundwire: intel: add pm_runtime support
From: Pierre-Louis Bossart
Add basic hooks in DAI .startup and .shutdown callbacks.
The SoundWire IP should be powered between those two calls. The power
dependencies between SoundWire and DSP are handled with the
parent/child relationship, before the SoundWire master device becomes
active the parent device will become active and power-up the shared
rails.
For now the strategy is to rely on complete enumeration when the
device becomes active, so the code is a copy/paste of the sequence for
system suspend/resume. In future patches, the strategy will optionally
be to rely on clock stop if the enumeration time is prohibitive or
when the devices connected to a link can signal a wake.
A module parameter is added to make integration of new Slave devices
easier, to e.g. keep the device active or prevent clock-stop.
Note that we need to we have to disable runtime pm before device
unregister, otherwise we will see "Failed to power up link: -11" error
on module remove test.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 112 +++--
drivers/soundwire/intel_init.c | 5 +-
2 files changed, 112 insertions(+), 5 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 88aeef8b7c0c..85a0bb6af4fe 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -22,6 +22,22 @@
#include "bus.h"
#include "intel.h"
+#define INTEL_MASTER_SUSPEND_DELAY_MS 3000
+
+/*
+ * debug/config flags for the Intel SoundWire Master.
+ *
+ * Since we may have multiple masters active, we can have up to 8
+ * flags reused in each byte, with master0 using the ls-byte, etc.
+ */
+
+#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME BIT(0)
+#define SDW_INTEL_MASTER_DISABLE_CLOCK_STOP BIT(1)
+
+static int md_flags;
+module_param_named(sdw_md_flags, md_flags, int, 0444);
+MODULE_PARM_DESC(sdw_md_flags, "SoundWire Intel Master device flags (0x0 all
off)");
+
/* Intel SHIM Registers Definition */
#define SDW_SHIM_LCAP 0x0
#define SDW_SHIM_LCTL 0x4
@@ -807,10 +823,17 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
static int intel_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
- /*
-* TODO: add pm_runtime support here, the startup callback
-* will make sure the IP is 'active'
-*/
+ struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
+ int ret;
+
+ ret = pm_runtime_get_sync(cdns->dev);
+ if (ret < 0 && ret != -EACCES) {
+ dev_err_ratelimited(cdns->dev,
+ "pm_runtime_get_sync failed in %s, ret
%d\n",
+ __func__, ret);
+ pm_runtime_put_noidle(cdns->dev);
+ return ret;
+ }
return 0;
}
@@ -985,7 +1008,10 @@ intel_hw_free(struct snd_pcm_substream *substream, struct
snd_soc_dai *dai)
static void intel_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
+ struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
+ pm_runtime_mark_last_busy(cdns->dev);
+ pm_runtime_put_autosuspend(cdns->dev);
}
static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
@@ -1270,6 +1296,7 @@ int intel_master_startup(struct platform_device *pdev)
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ int link_flags;
int ret;
if (bus->prop.hw_disabled) {
@@ -1314,6 +1341,18 @@ int intel_master_startup(struct platform_device *pdev)
intel_debugfs_init(sdw);
+ /* Enable runtime PM */
+ link_flags = md_flags >> (bus->link_id * 8);
+ if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) {
+ pm_runtime_set_autosuspend_delay(dev,
+INTEL_MASTER_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_mark_last_busy(dev);
+
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ }
+
return 0;
err_interrupt:
@@ -1412,6 +1451,36 @@ static int intel_suspend(struct device *dev)
return 0;
}
+static int intel_suspend_runtime(struct device *dev)
+{
+ struct sdw_cdns *cdns = dev_get_drvdata(dev);
+ struct sdw_intel *sdw = cdns_to_intel(cdns);
+ struct sdw_bus *bus = &cdns->bus;
+ int ret;
+
+ if (bus->prop.hw_disabled) {
+ dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n",
+ bus->link_id);
+ return 0;
+ }
+
+ ret = sdw_cdns_enable_interrupt(cdns, false);
+ if (re
[PATCH v2 07/12] soundwire: intel: add CLK_STOP_TEARDOWN for pm_runtime suspend
From: Pierre-Louis Bossart
Now that we have options, add support for TEARDOWN mode (same
functionality as existing code)
All other modes will be added in follow-up patches.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 82 +-
drivers/soundwire/intel.h | 2 +
drivers/soundwire/intel_init.c | 1 +
3 files changed, 54 insertions(+), 31 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 3f9015dcb693..68c1cdfb7999 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1548,6 +1548,7 @@ static int intel_suspend_runtime(struct device *dev)
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ u32 clock_stop_quirks;
int ret;
if (bus->prop.hw_disabled) {
@@ -1556,21 +1557,31 @@ static int intel_suspend_runtime(struct device *dev)
return 0;
}
- ret = sdw_cdns_enable_interrupt(cdns, false);
- if (ret < 0) {
- dev_err(dev, "cannot disable interrupts on suspend\n");
- return ret;
- }
+ clock_stop_quirks = sdw->link_res->clock_stop_quirks;
- ret = intel_link_power_down(sdw);
- if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
- return ret;
- }
+ if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
+
+ ret = sdw_cdns_enable_interrupt(cdns, false);
+ if (ret < 0) {
+ dev_err(dev, "cannot disable interrupts on suspend\n");
+ return ret;
+ }
- intel_shim_wake(sdw, false);
+ ret = intel_link_power_down(sdw);
+ if (ret) {
+ dev_err(dev, "Link power down failed: %d", ret);
+ return ret;
+ }
+
+ intel_shim_wake(sdw, false);
+
+ } else {
+ dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
+ __func__, clock_stop_quirks);
+ ret = -EINVAL;
+ }
- return 0;
+ return ret;
}
static int intel_resume(struct device *dev)
@@ -1645,6 +1656,7 @@ static int intel_resume_runtime(struct device *dev)
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ u32 clock_stop_quirks;
int ret;
if (bus->prop.hw_disabled) {
@@ -1653,28 +1665,36 @@ static int intel_resume_runtime(struct device *dev)
return 0;
}
- ret = intel_init(sdw);
- if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
- return ret;
- }
+ clock_stop_quirks = sdw->link_res->clock_stop_quirks;
- /*
-* make sure all Slaves are tagged as UNATTACHED and provide
-* reason for reinitialization
-*/
- sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
+ if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
+ ret = intel_init(sdw);
+ if (ret) {
+ dev_err(dev, "%s failed: %d", __func__, ret);
+ return ret;
+ }
- ret = sdw_cdns_enable_interrupt(cdns, true);
- if (ret < 0) {
- dev_err(dev, "cannot enable interrupts during resume\n");
- return ret;
- }
+ /*
+* make sure all Slaves are tagged as UNATTACHED and provide
+* reason for reinitialization
+*/
+ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
- ret = sdw_cdns_exit_reset(cdns);
- if (ret < 0) {
- dev_err(dev, "unable to exit bus reset sequence during
resume\n");
- return ret;
+ ret = sdw_cdns_enable_interrupt(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable interrupts during
resume\n");
+ return ret;
+ }
+
+ ret = sdw_cdns_exit_reset(cdns);
+ if (ret < 0) {
+ dev_err(dev, "unable to exit bus reset sequence during
resume\n");
+ return ret;
+ }
+ } else {
+ dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
+ __func__, clock_stop_quirks);
+ ret = -EINVAL;
}
return ret;
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 4ea3d262d249..23daab9da329 100644
--- a/drivers/soundwire/in
[PATCH v2 03/12] soundwire: intel: fix race condition on system resume
From: Pierre-Louis Bossart Previous patches took care of the case where the master device is pm_runtime 'suspended' when a system suspend occurs. In the case where the master device was not suspended, e.g. if suspend occurred while streaming audio, Intel validation noticed a race condition: the pm_runtime suspend may conflict with the enumeration started by the system resume. This can be simply fixed by updating the status before exiting system resume. GitHub issue: https://github.com/thesofproject/linux/issues/1482 Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/intel.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 0e21bae3cd19..00c5de1250ec 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1528,6 +1528,18 @@ static int intel_resume(struct device *dev) return ret; } + /* +* after system resume, the pm_runtime suspend() may kick in +* during the enumeration, before any children device force the +* master device to remain active. Using pm_runtime_get() +* routines is not really possible, since it'd prevent the +* master from suspending. +* A reasonable compromise is to update the pm_runtime +* counters and delay the pm_runtime suspend by several +* seconds, by when all enumeration should be complete. +*/ + pm_runtime_mark_last_busy(dev); + return ret; } -- 2.17.1
[PATCH v2 11/12] soundwire: intel: support clock_stop mode without quirks
From: Pierre-Louis Bossart
In this mode, on restart the bus restarts immediately, the Slaves
remain synchronized and all context is kept intact.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 95b14c034ea7..2899445e2649 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1595,7 +1595,8 @@ static int intel_suspend_runtime(struct device *dev)
intel_shim_wake(sdw, false);
- } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
+ } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
+ !clock_stop_quirks) {
ret = sdw_cdns_clock_stop(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable clock stop on suspend\n");
@@ -1755,6 +1756,24 @@ static int intel_resume_runtime(struct device *dev)
dev_err(dev, "unable to restart clock during resume\n");
return ret;
}
+ } else if (!clock_stop_quirks) {
+ ret = intel_init(sdw);
+ if (ret) {
+ dev_err(dev, "%s failed: %d", __func__, ret);
+ return ret;
+ }
+
+ ret = sdw_cdns_enable_interrupt(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable interrupts during
resume\n");
+ return ret;
+ }
+
+ ret = sdw_cdns_clock_restart(cdns, false);
+ if (ret < 0) {
+ dev_err(dev, "unable to resume master during resume\n");
+ return ret;
+ }
} else {
dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
__func__, clock_stop_quirks);
--
2.17.1
[PATCH v2 06/12] soundwire: intel: pm_runtime idle scheduling
From: Pierre-Louis Bossart
Add quirk and pm_runtime idle scheduling to let the Master suspend if
no Slaves become attached. This can happen when a link is not marked
as disabled and has devices exposed in the DSDT, if the power is
controlled by sideband means or the link includes a pluggable
connector.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 26 --
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 95a1d88a5bfb..3f9015dcb693 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -31,8 +31,9 @@
* flags reused in each byte, with master0 using the ls-byte, etc.
*/
-#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME BIT(0)
-#define SDW_INTEL_MASTER_DISABLE_CLOCK_STOP BIT(1)
+#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIMEBIT(0)
+#define SDW_INTEL_MASTER_DISABLE_CLOCK_STOPBIT(1)
+#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE BIT(2)
static int md_flags;
module_param_named(sdw_md_flags, md_flags, int, 0444);
@@ -1422,6 +1423,22 @@ int intel_master_startup(struct platform_device *pdev)
pm_runtime_enable(dev);
}
+ /*
+* The runtime PM status of Slave devices is "Unsupported"
+* until they report as ATTACHED. If they don't, e.g. because
+* there are no Slave devices populated or if the power-on is
+* delayed or dependent on a power switch, the Master will
+* remain active and prevent its parent from suspending.
+*
+* Conditionally force the pm_runtime core to re-evaluate the
+* Master status in the absence of any Slave activity. A quirk
+* is provided to e.g. deal with Slaves that may be powered on
+* with a delay. A more complete solution would require the
+* definition of Master properties.
+*/
+ if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE))
+ pm_runtime_idle(dev);
+
return 0;
err_interrupt:
@@ -1561,6 +1578,7 @@ static int intel_resume(struct device *dev)
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ int link_flags;
int ret;
if (bus->prop.hw_disabled) {
@@ -1577,6 +1595,10 @@ static int intel_resume(struct device *dev)
pm_runtime_set_active(dev);
pm_runtime_mark_last_busy(dev);
pm_runtime_enable(dev);
+
+ link_flags = md_flags >> (bus->link_id * 8);
+ if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE))
+ pm_runtime_idle(dev);
}
ret = intel_init(sdw);
--
2.17.1
[PATCH v2 09/12] soundwire: intel: add CLK_STOP_NOT_ALLOWED support
From: Pierre-Louis Bossart
In case the clock needs to keep running, we need to prevent the Master
from entering pm_runtime suspend.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index ad476e9e4d25..95b14c034ea7 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1367,6 +1367,7 @@ int intel_master_startup(struct platform_device *pdev)
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
int link_flags;
+ u32 clock_stop_quirks;
int ret;
if (bus->prop.hw_disabled) {
@@ -1423,6 +1424,20 @@ int intel_master_startup(struct platform_device *pdev)
pm_runtime_enable(dev);
}
+ clock_stop_quirks = sdw->link_res->clock_stop_quirks;
+ if (clock_stop_quirks & SDW_INTEL_CLK_STOP_NOT_ALLOWED) {
+ /*
+* To keep the clock running we need to prevent
+* pm_runtime suspend from happening by increasing the
+* reference count.
+* This quirk is specified by the parent PCI device in
+* case of specific latency requirements. It will have
+* no effect if pm_runtime is disabled by the user via
+* a module parameter for testing purposes.
+*/
+ pm_runtime_get_noresume(dev);
+ }
+
/*
* The runtime PM status of Slave devices is "Unsupported"
* until they report as ATTACHED. If they don't, e.g. because
@@ -1454,6 +1469,11 @@ static int intel_master_remove(struct platform_device
*pdev)
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ /*
+* Since pm_runtime is already disabled, we don't decrease
+* the refcount when the clock_stop_quirk is
+* SDW_INTEL_CLK_STOP_NOT_ALLOWED
+*/
if (!bus->prop.hw_disabled) {
intel_debugfs_exit(sdw);
sdw_cdns_enable_interrupt(cdns, false);
--
2.17.1
[PATCH v2 04/12] soundwire: intel: call helper to reset Slave states on resume
From: Pierre-Louis Bossart
This helps make sure they are all UNATTACHED and reset the state
machines.
At the moment we perform a bus reset both for system resume and
pm_runtime resume, this will be modified when clock-stop mode is
supported
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 00c5de1250ec..10dd0e208ce7 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1516,6 +1516,12 @@ static int intel_resume(struct device *dev)
return ret;
}
+ /*
+* make sure all Slaves are tagged as UNATTACHED and provide
+* reason for reinitialization
+*/
+ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
+
ret = sdw_cdns_enable_interrupt(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable interrupts during resume\n");
@@ -1562,6 +1568,12 @@ static int intel_resume_runtime(struct device *dev)
return ret;
}
+ /*
+* make sure all Slaves are tagged as UNATTACHED and provide
+* reason for reinitialization
+*/
+ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
+
ret = sdw_cdns_enable_interrupt(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable interrupts during resume\n");
--
2.17.1
[PATCH 1/2] soundwire: add definition for maximum number of ports
From: Pierre-Louis Bossart
A Device may have at most 15 physical ports (DP0, DP1..DP14).
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
include/linux/soundwire/sdw.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 76052f12c9f7..0aa4c6af7554 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -38,7 +38,8 @@ struct sdw_slave;
#define SDW_FRAME_CTRL_BITS48
#define SDW_MAX_DEVICES11
-#define SDW_VALID_PORT_RANGE(n)((n) <= 14 && (n) >= 1)
+#define SDW_MAX_PORTS 15
+#define SDW_VALID_PORT_RANGE(n)((n) < SDW_MAX_PORTS && (n) >=
1)
enum {
SDW_PORT_DIRN_SINK = 0,
--
2.17.1
[PATCH 0/2] soundwire: fix port_ready[] dynamic allocation in
The existing code allocates memory for the total number of ports. This only works if the ports are contiguous, but will break if e.g. a Devices uses port0, 1, and 14. The port_ready[] array would contain 3 elements, which would lead to an out-of-bounds access. Conversely in other cases, the wrong port index would be used leading to timeouts on prepare. This can be fixed by allocating for the worst-case of 15 ports (DP0..DP14). In addition since the number is now fixed, we can use an array instead of a dynamic allocation. Pierre-Louis Bossart (2): soundwire: add definition for maximum number of ports soundwire: fix port_ready[] dynamic allocation in mipi_disco and ASoC codecs drivers/soundwire/mipi_disco.c | 18 +- drivers/soundwire/slave.c | 4 include/linux/soundwire/sdw.h | 5 +++-- sound/soc/codecs/max98373-sdw.c | 15 +-- sound/soc/codecs/rt1308-sdw.c | 14 +- sound/soc/codecs/rt5682-sdw.c | 15 +-- sound/soc/codecs/rt700-sdw.c| 15 +-- sound/soc/codecs/rt711-sdw.c| 15 +-- sound/soc/codecs/rt715-sdw.c| 33 + 9 files changed, 14 insertions(+), 120 deletions(-) -- 2.17.1
[PATCH 2/2] soundwire: fix port_ready[] dynamic allocation in mipi_disco and ASoC codecs
From: Pierre-Louis Bossart
The existing code allocates memory for the total number of ports.
This only works if the ports are contiguous, but will break if e.g. a
Devices uses port0, 1, and 14. The port_ready[] array would contain 3
elements, which would lead to an out-of-bounds access. Conversely in
other cases, the wrong port index would be used leading to timeouts on
prepare.
This can be fixed by allocating for the worst-case of 15
ports (DP0..DP14). In addition since the number is now fixed, we can
use an array instead of a dynamic allocation.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/mipi_disco.c | 18 +-
drivers/soundwire/slave.c | 4
include/linux/soundwire/sdw.h | 2 +-
sound/soc/codecs/max98373-sdw.c | 15 +--
sound/soc/codecs/rt1308-sdw.c | 14 +-
sound/soc/codecs/rt5682-sdw.c | 15 +--
sound/soc/codecs/rt700-sdw.c| 15 +--
sound/soc/codecs/rt711-sdw.c| 15 +--
sound/soc/codecs/rt715-sdw.c| 33 +
9 files changed, 12 insertions(+), 119 deletions(-)
diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 4ae62b452b8c..55a9c51c84c1 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -289,7 +289,7 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
struct sdw_slave_prop *prop = &slave->prop;
struct device *dev = &slave->dev;
struct fwnode_handle *port;
- int num_of_ports, nval, i, dp0 = 0;
+ int nval;
device_property_read_u32(dev, "mipi-sdw-sw-interface-revision",
&prop->mipi_revision);
@@ -352,7 +352,6 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
return -ENOMEM;
sdw_slave_read_dp0(slave, port, prop->dp0_prop);
- dp0 = 1;
}
/*
@@ -383,21 +382,6 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
prop->sink_ports, "sink");
- /* some ports are bidirectional so check total ports by ORing */
- nval = prop->source_ports | prop->sink_ports;
- num_of_ports = hweight32(nval) + dp0; /* add DP0 */
-
- /* Allocate port_ready based on num_of_ports */
- slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
-sizeof(*slave->port_ready),
-GFP_KERNEL);
- if (!slave->port_ready)
- return -ENOMEM;
-
- /* Initialize completion */
- for (i = 0; i < num_of_ports; i++)
- init_completion(&slave->port_ready[i]);
-
return 0;
}
EXPORT_SYMBOL(sdw_slave_read_prop);
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 0839445ee07b..a762ee24e6fa 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -25,6 +25,7 @@ static int sdw_slave_add(struct sdw_bus *bus,
{
struct sdw_slave *slave;
int ret;
+ int i;
slave = kzalloc(sizeof(*slave), GFP_KERNEL);
if (!slave)
@@ -58,6 +59,9 @@ static int sdw_slave_add(struct sdw_bus *bus,
init_completion(&slave->probe_complete);
slave->probed = false;
+ for (i = 0; i < SDW_MAX_PORTS; i++)
+ init_completion(&slave->port_ready[i]);
+
mutex_lock(&bus->bus_lock);
list_add_tail(&slave->node, &bus->slaves);
mutex_unlock(&bus->bus_lock);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 0aa4c6af7554..63e71645fd13 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -619,7 +619,7 @@ struct sdw_slave {
struct dentry *debugfs;
#endif
struct list_head node;
- struct completion *port_ready;
+ struct completion port_ready[SDW_MAX_PORTS];
enum sdw_clk_stop_mode curr_clk_stop_mode;
u16 dev_num;
u16 dev_num_sticky;
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
index 5fe724728e84..a3ec92775ea7 100644
--- a/sound/soc/codecs/max98373-sdw.c
+++ b/sound/soc/codecs/max98373-sdw.c
@@ -282,7 +282,7 @@ static const struct dev_pm_ops max98373_pm = {
static int max98373_read_prop(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
- int nval, i, num_of_ports;
+ int nval, i;
u32 bit;
unsigned long addr;
struct sdw_dpn_prop *dpn;
@@ -295,7 +295,6 @@ static int max98373_read_prop(struct sdw_slave *slave)
prop->clk_stop_timeout = 20;
nval = hweight32(prop->source_ports);
- num_of_
[PATCH] soundwire: bus: fix typo in comment on INTSTAT registers
From: Pierre-Louis Bossart
s/Instat/Intstat/
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index e6e0fb9a81b4..da0201693c24 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1372,7 +1372,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
return ret;
}
- /* Read Instat 1, Instat 2 and Instat 3 registers */
+ /* Read Intstat 1, Intstat 2 and Intstat 3 registers */
ret = sdw_read(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(slave->bus->dev,
--
2.17.1
[PATCH] soundwire: cadence: fix race condition between suspend and Slave device alerts
From: Pierre-Louis Bossart
In system suspend stress cases, the SOF CI reports timeouts. The root
cause is that an alert is generated while the system suspends. The
interrupt handling generates transactions on the bus that will never
be handled because the interrupts are disabled in parallel.
As a result, the transaction never completes and times out on resume.
This error doesn't seem too problematic since it happens in a work
queue, and the system recovers without issues.
Nevertheless, this race condition should not happen. When doing a
system suspend, or when disabling interrupts, we should make sure the
current transaction can complete, and prevent new work from being
queued.
BugLink: https://github.com/thesofproject/linux/issues/2344
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Ranjani Sridharan
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 24 +++-
drivers/soundwire/cadence_master.h | 1 +
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 24eafe0aa1c3..1330ffc47596 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -791,7 +791,16 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
CDNS_MCP_INT_SLAVE_MASK, 0);
int_status &= ~CDNS_MCP_INT_SLAVE_MASK;
- schedule_work(&cdns->work);
+
+ /*
+* Deal with possible race condition between interrupt
+* handling and disabling interrupts on suspend.
+*
+* If the master is in the process of disabling
+* interrupts, don't schedule a workqueue
+*/
+ if (cdns->interrupt_enabled)
+ schedule_work(&cdns->work);
}
cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status);
@@ -924,6 +933,19 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns, bool
state)
slave_state = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave_state);
}
+ cdns->interrupt_enabled = state;
+
+ /*
+* Complete any on-going status updates before updating masks,
+* and cancel queued status updates.
+*
+* There could be a race with a new interrupt thrown before
+* the 3 mask updates below are complete, so in the interrupt
+* we use the 'interrupt_enabled' status to prevent new work
+* from being queued.
+*/
+ if (!state)
+ cancel_work_sync(&cdns->work);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK0, slave_intmask0);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK1, slave_intmask1);
diff --git a/drivers/soundwire/cadence_master.h
b/drivers/soundwire/cadence_master.h
index fdec62b912d3..4d1aab5b5ec2 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -133,6 +133,7 @@ struct sdw_cdns {
bool link_up;
unsigned int msg_count;
+ bool interrupt_enabled;
struct work_struct work;
--
2.17.1
[PATCH 00/11] soundwire: intel: add multi-link support
This series enables multi-link support for Intel platforms. Bard Liao (1): soundwire: intel: Only call sdw stream APIs for the first cpu_dai Pierre-Louis Bossart (10): soundwire: intel: disable shim wake on suspend soundwire: intel: ignore software command retries soundwire: intel: add multi-link support soundwire: intel: add missing support for all clock stop modes soundwire: bus: update multi-link definition with hw sync details soundwire: intel: add multi-link hw_synchronization information soundwire: stream: enable hw_sync as needed by hardware soundwire: intel: add dynamic debug trace for clock-stop invalid configs soundwire: intel: pass link_mask information to each master soundwire: intel: don't manage link power individually drivers/soundwire/intel.c | 309 - drivers/soundwire/intel.h | 2 + drivers/soundwire/intel_init.c | 1 + drivers/soundwire/stream.c | 15 +- include/linux/soundwire/sdw.h | 6 + 5 files changed, 282 insertions(+), 51 deletions(-) -- 2.17.1
[PATCH 01/11] soundwire: intel: disable shim wake on suspend
From: Pierre-Louis Bossart
If we enabled the clock stop mode and suspend, we need to disable the
shim wake. We do so only if the parent is pm_runtime active due to
power rail dependencies.
GitHub issue: https://github.com/thesofproject/linux/issues/1678
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index dbcbe2708563..fe9b92fd48db 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1532,6 +1532,7 @@ static int intel_suspend(struct device *dev)
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
+ u32 clock_stop_quirks;
int ret;
if (bus->prop.hw_disabled) {
@@ -1543,6 +1544,23 @@ static int intel_suspend(struct device *dev)
if (pm_runtime_suspended(dev)) {
dev_dbg(dev, "%s: pm_runtime status: suspended\n", __func__);
+ clock_stop_quirks = sdw->link_res->clock_stop_quirks;
+
+ if ((clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
+!clock_stop_quirks) &&
+ !pm_runtime_suspended(dev->parent)) {
+
+ /*
+* if we've enabled clock stop, and the parent
+* is still active, disable shim wake. The
+* SHIM registers are not accessible if the
+* parent is already pm_runtime suspended so
+* it's too late to change that configuration
+*/
+
+ intel_shim_wake(sdw, false);
+ }
+
return 0;
}
--
2.17.1
[PATCH 09/11] soundwire: intel: add dynamic debug trace for clock-stop invalid configs
From: Pierre-Louis Bossart
Detect cases where the clock is assumed to be stopped but the IP is
not in the relevant state, and add a dynamic debug trace.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 7c63581270fd..b82d02af3c4f 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1964,6 +1964,11 @@ static int intel_resume_runtime(struct device *dev)
}
}
} else if (!clock_stop_quirks) {
+
+ clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns);
+ if (!clock_stop0)
+ dev_err(dev, "%s invalid configuration, clock was not
stopped", __func__);
+
ret = intel_init(sdw);
if (ret) {
dev_err(dev, "%s failed: %d", __func__, ret);
--
2.17.1
[PATCH 07/11] soundwire: intel: Only call sdw stream APIs for the first cpu_dai
We should call these APIs once per stream. So we can only call it
when the dai ops is invoked for the first cpu dai.
Signed-off-by: Bard Liao
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Ranjani Sridharan
---
drivers/soundwire/intel.c | 45 +--
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 89a8ad1f80e8..7c63581270fd 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -941,11 +941,13 @@ static int intel_hw_params(struct snd_pcm_substream
*substream,
static int intel_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *first_cpu_dai = asoc_rtd_to_cpu(rtd, 0);
struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_cdns_dma_data *dma;
int ch, dir;
- int ret;
+ int ret = 0;
dma = snd_soc_dai_get_dma_data(dai, substream);
if (!dma) {
@@ -985,7 +987,13 @@ static int intel_prepare(struct snd_pcm_substream
*substream,
goto err;
}
- ret = sdw_prepare_stream(dma->stream);
+ /*
+* All cpu dais belong to a stream. To ensure sdw_prepare_stream
+* is called once per stream, we should call it only when
+* dai = first_cpu_dai.
+*/
+ if (first_cpu_dai == dai)
+ ret = sdw_prepare_stream(dma->stream);
err:
return ret;
@@ -994,9 +1002,19 @@ static int intel_prepare(struct snd_pcm_substream
*substream,
static int intel_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *dai)
{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *first_cpu_dai = asoc_rtd_to_cpu(rtd, 0);
struct sdw_cdns_dma_data *dma;
int ret;
+ /*
+* All cpu dais belong to a stream. To ensure sdw_enable/disable_stream
+* are called once per stream, we should call them only when
+* dai = first_cpu_dai.
+*/
+ if (first_cpu_dai != dai)
+ return 0;
+
dma = snd_soc_dai_get_dma_data(dai, substream);
if (!dma) {
dev_err(dai->dev, "failed to get dma data in %s", __func__);
@@ -1031,6 +1049,8 @@ static int intel_trigger(struct snd_pcm_substream
*substream, int cmd,
static int
intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *first_cpu_dai = asoc_rtd_to_cpu(rtd, 0);
struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_cdns_dma_data *dma;
@@ -1040,12 +1060,25 @@ intel_hw_free(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
if (!dma)
return -EIO;
- ret = sdw_deprepare_stream(dma->stream);
- if (ret) {
- dev_err(dai->dev, "sdw_deprepare_stream: failed %d", ret);
- return ret;
+ /*
+* All cpu dais belong to a stream. To ensure sdw_deprepare_stream
+* is called once per stream, we should call it only when
+* dai = first_cpu_dai.
+*/
+ if (first_cpu_dai == dai) {
+ ret = sdw_deprepare_stream(dma->stream);
+ if (ret) {
+ dev_err(dai->dev, "sdw_deprepare_stream: failed %d",
ret);
+ return ret;
+ }
}
+ /*
+* The sdw stream state will transition to RELEASED when stream->
+* master_list is empty. So the stream state will transition to
+* DEPREPARED for the first cpu-dai and to RELEASED for the last
+* cpu-dai.
+*/
ret = sdw_stream_remove_master(&cdns->bus, dma->stream);
if (ret < 0) {
dev_err(dai->dev, "remove master from stream %s failed: %d\n",
--
2.17.1
[PATCH 06/11] soundwire: intel: add multi-link hw_synchronization information
From: Pierre-Louis Bossart
set the flags as required by hardware implementation
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index f4441684bd7e..89a8ad1f80e8 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1400,7 +1400,14 @@ int intel_master_startup(struct platform_device *pdev)
dev_dbg(dev, "Multi-link is disabled\n");
bus->multi_link = false;
} else {
+ /*
+* hardware-based synchronization is required regardless
+* of the number of segments used by a stream: SSP-based
+* synchronization is gated by gsync when the multi-master
+* mode is set.
+*/
bus->multi_link = true;
+ bus->hw_sync_min_links = 1;
}
/* Initialize shim, controller */
--
2.17.1
[PATCH 05/11] soundwire: bus: update multi-link definition with hw sync details
From: Pierre-Louis Bossart
Hardware-based synchronization is typically required when the
bus->multi_link flag is set.
On Intel platforms, when the Cadence IP is configured in 'Multi Master
Mode', the hardware synchronization is required even when a stream
only uses a single segment. The existing code only deal with hardware
synchronization when a stream uses more than one segment so to remain
backwards compatible we add a configuration threshold. For Intel cases
this threshold will be set to one, other platforms may be able to use
the SSP-based sync in those cases.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
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 76052f12c9f7..9adbe4fd7980 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -827,6 +827,11 @@ struct sdw_master_ops {
* @multi_link: Store bus property that indicates if multi links
* are supported. This flag is populated by drivers after reading
* appropriate firmware (ACPI/DT).
+ * @hw_sync_min_links: Number of links used by a stream above which
+ * hardware-based synchronization is required. This value is only
+ * meaningful if multi_link is set. If set to 1, hardware-based
+ * synchronization will be used even if a stream only uses a single
+ * SoundWire segment.
*/
struct sdw_bus {
struct device *dev;
@@ -850,6 +855,7 @@ struct sdw_bus {
unsigned int clk_stop_timeout;
u32 bank_switch_timeout;
bool multi_link;
+ int hw_sync_min_links;
};
int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
--
2.17.1
[PATCH 11/11] soundwire: intel: don't manage link power individually
From: Pierre-Louis Bossart
Each link has separate power controls, but experimental results show
we need to use an all-or-none approach to the link power management.
This change has marginal power impacts, the DSP needs to be powered
anyways before SoundWire links can be powered, and even when powered a
link can be in clock-stopped mode.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 70 +--
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index b82d02af3c4f..154aa7c0561a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -63,7 +63,9 @@ MODULE_PARM_DESC(sdw_md_flags, "SoundWire Intel Master device
flags (0x0 all off
#define SDW_SHIM_WAKESTS 0x192
#define SDW_SHIM_LCTL_SPA BIT(0)
+#define SDW_SHIM_LCTL_SPA_MASK GENMASK(3, 0)
#define SDW_SHIM_LCTL_CPA BIT(8)
+#define SDW_SHIM_LCTL_CPA_MASK GENMASK(11, 8)
#define SDW_SHIM_SYNC_SYNCPRD_VAL_24 (24000 / SDW_CADENCE_GSYNC_KHZ - 1)
#define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1)
@@ -295,8 +297,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
u32 *shim_mask = sdw->link_res->shim_mask;
struct sdw_bus *bus = &sdw->cdns.bus;
struct sdw_master_prop *prop = &bus->prop;
- int spa_mask, cpa_mask;
- int link_control;
+ u32 spa_mask, cpa_mask;
+ u32 link_control;
int ret = 0;
u32 syncprd;
u32 sync_reg;
@@ -319,6 +321,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24;
if (!*shim_mask) {
+ dev_dbg(sdw->cdns.dev, "%s: powering up all links\n", __func__);
+
/* we first need to program the SyncPRD/CPU registers */
dev_dbg(sdw->cdns.dev,
"%s: first link up, programming SYNCPRD\n", __func__);
@@ -331,21 +335,24 @@ static int intel_link_power_up(struct sdw_intel *sdw)
/* Set SyncCPU bit */
sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
- }
- /* Link power up sequence */
- link_control = intel_readl(shim, SDW_SHIM_LCTL);
- spa_mask = (SDW_SHIM_LCTL_SPA << link_id);
- cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
- link_control |= spa_mask;
+ /* Link power up sequence */
+ link_control = intel_readl(shim, SDW_SHIM_LCTL);
- ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
- if (ret < 0) {
- dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret);
- goto out;
- }
+ /* only power-up enabled links */
+ spa_mask = sdw->link_res->link_mask <<
+ SDW_REG_SHIFT(SDW_SHIM_LCTL_SPA_MASK);
+ cpa_mask = sdw->link_res->link_mask <<
+ SDW_REG_SHIFT(SDW_SHIM_LCTL_CPA_MASK);
+
+ link_control |= spa_mask;
+
+ ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control,
cpa_mask);
+ if (ret < 0) {
+ dev_err(sdw->cdns.dev, "Failed to power up link: %d\n",
ret);
+ goto out;
+ }
- if (!*shim_mask) {
/* SyncCPU will change once link is active */
ret = intel_wait_bit(shim, SDW_SHIM_SYNC,
SDW_SHIM_SYNC_SYNCCPU, 0);
@@ -483,7 +490,7 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool
wake_enable)
static int intel_link_power_down(struct sdw_intel *sdw)
{
- int link_control, spa_mask, cpa_mask;
+ u32 link_control, spa_mask, cpa_mask;
unsigned int link_id = sdw->instance;
void __iomem *shim = sdw->link_res->shim;
u32 *shim_mask = sdw->link_res->shim_mask;
@@ -493,24 +500,39 @@ static int intel_link_power_down(struct sdw_intel *sdw)
intel_shim_master_ip_to_glue(sdw);
- /* Link power down sequence */
- link_control = intel_readl(shim, SDW_SHIM_LCTL);
- spa_mask = ~(SDW_SHIM_LCTL_SPA << link_id);
- cpa_mask = (SDW_SHIM_LCTL_CPA << link_id);
- link_control &= spa_mask;
-
- ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
-
if (!(*shim_mask & BIT(link_id)))
dev_err(sdw->cdns.dev,
"%s: Unbalanced power-up/down calls\n", __func__);
*shim_mask &= ~BIT(link_id);
+ if (!*shim_mask) {
+
+ dev_dbg(sdw->cdns.dev, "%s: powering down all links\n",
__func__);
+
+ /* Link power down
[PATCH 02/11] soundwire: intel: ignore software command retries
From: Pierre-Louis Bossart with multiple links synchronized in hardware, retrying commands in software is not recommended. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/intel.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index fe9b92fd48db..c9ba706e20c6 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1355,6 +1355,11 @@ static int intel_master_probe(struct platform_device *pdev) dev_info(dev, "SoundWire master %d is disabled, will be ignored\n", bus->link_id); + /* +* Ignore BIOS err_threshold, it's a really bad idea when dealing +* with multiple hardware synchronized links +*/ + bus->prop.err_threshold = 0; return 0; } -- 2.17.1
[PATCH 04/11] soundwire: intel: add missing support for all clock stop modes
From: Pierre-Louis Bossart
Deal with the BUS_RESET case, which is the default. The only change is
to add support for the exit sequence using the syncArm/syncGo mode for
the exit reset sequence.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 49 +++
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index a3aa8ab49285..f4441684bd7e 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1868,11 +1868,6 @@ static int intel_resume_runtime(struct device *dev)
if (!clock_stop0) {
- /*
-* Re-initialize the IP since it was powered-off
-*/
- sdw_cdns_init(&sdw->cdns);
-
/*
* make sure all Slaves are tagged as UNATTACHED and
* provide reason for reinitialization
@@ -1880,13 +1875,31 @@ static int intel_resume_runtime(struct device *dev)
status = SDW_UNATTACH_REQUEST_MASTER_RESET;
sdw_clear_slave_status(bus, status);
- }
+ ret = sdw_cdns_enable_interrupt(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable interrupts during
resume\n");
+ return ret;
+ }
- ret = sdw_cdns_enable_interrupt(cdns, true);
- if (ret < 0) {
- dev_err(dev, "cannot enable interrupts during
resume\n");
- return ret;
+ /*
+* follow recommended programming flows to avoid
+* timeouts when gsync is enabled
+*/
+ if (multi_link)
+ intel_shim_sync_arm(sdw);
+
+ /*
+* Re-initialize the IP since it was powered-off
+*/
+ sdw_cdns_init(&sdw->cdns);
+
+ } else {
+ ret = sdw_cdns_enable_interrupt(cdns, true);
+ if (ret < 0) {
+ dev_err(dev, "cannot enable interrupts during
resume\n");
+ return ret;
+ }
}
ret = sdw_cdns_clock_restart(cdns, !clock_stop0);
@@ -1894,6 +1907,22 @@ static int intel_resume_runtime(struct device *dev)
dev_err(dev, "unable to restart clock during resume\n");
return ret;
}
+
+ if (!clock_stop0) {
+ ret = sdw_cdns_exit_reset(cdns);
+ if (ret < 0) {
+ dev_err(dev, "unable to exit bus reset sequence
during resume\n");
+ return ret;
+ }
+
+ if (multi_link) {
+ ret = intel_shim_sync_go(sdw);
+ if (ret < 0) {
+ dev_err(sdw->cdns.dev, "sync go failed
during resume\n");
+ return ret;
+ }
+ }
+ }
} else if (!clock_stop_quirks) {
ret = intel_init(sdw);
if (ret) {
--
2.17.1
[PATCH 10/11] soundwire: intel: pass link_mask information to each master
From: Pierre-Louis Bossart
While the hardware exposes independent bits to power-up each master,
the recommended sequence is to power all links or none. Idle links can
still use the clock stop mode while the master is powered.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.h | 2 ++
drivers/soundwire/intel_init.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 23daab9da329..76820d0b9deb 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -18,6 +18,7 @@
* @shim_lock: mutex to handle access to shared SHIM registers
* @shim_mask: global pointer to check SHIM register initialization
* @clock_stop_quirks: mask defining requested behavior on pm_suspend
+ * @link_mask: global mask needed for power-up/down sequences
* @cdns: Cadence master descriptor
* @list: used to walk-through all masters exposed by the same controller
*/
@@ -33,6 +34,7 @@ struct sdw_intel_link_res {
struct mutex *shim_lock; /* protect shared registers */
u32 *shim_mask;
u32 clock_stop_quirks;
+ u32 link_mask;
struct sdw_cdns *cdns;
struct list_head list;
};
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index add46d8fc85c..65d9b9bd2106 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -255,6 +255,7 @@ static struct sdw_intel_ctx
link->clock_stop_quirks = res->clock_stop_quirks;
link->shim_lock = &ctx->shim_lock;
link->shim_mask = &ctx->shim_mask;
+ link->link_mask = link_mask;
memset(&pdevinfo, 0, sizeof(pdevinfo));
--
2.17.1
[PATCH 08/11] soundwire: stream: enable hw_sync as needed by hardware
From: Pierre-Louis Bossart
Use platform-specific information to decide when to use hw_sync, not
only a number of links > 1.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 37290a799023..e4cf484f5905 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -689,9 +689,9 @@ static int sdw_bank_switch(struct sdw_bus *bus, int
m_rt_count)
/*
* Set the multi_link flag only when both the hardware supports
-* and there is a stream handled by multiple masters
+* and hardware-based sync is required
*/
- multi_link = bus->multi_link && (m_rt_count > 1);
+ multi_link = bus->multi_link && (m_rt_count >= bus->hw_sync_min_links);
if (multi_link)
ret = sdw_transfer_defer(bus, wr_msg, &bus->defer_msg);
@@ -760,13 +760,16 @@ static int do_bank_switch(struct sdw_stream_runtime
*stream)
const struct sdw_master_ops *ops;
struct sdw_bus *bus;
bool multi_link = false;
+ int m_rt_count;
int ret = 0;
+ m_rt_count = stream->m_rt_count;
+
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
bus = m_rt->bus;
ops = bus->ops;
- if (bus->multi_link) {
+ if (bus->multi_link && m_rt_count >= bus->hw_sync_min_links) {
multi_link = true;
mutex_lock(&bus->msg_lock);
}
@@ -787,7 +790,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
* synchronized across all Masters and happens later as a
* part of post_bank_switch ops.
*/
- ret = sdw_bank_switch(bus, stream->m_rt_count);
+ ret = sdw_bank_switch(bus, m_rt_count);
if (ret < 0) {
dev_err(bus->dev, "Bank switch failed: %d\n", ret);
goto error;
@@ -813,7 +816,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
ret);
goto error;
}
- } else if (bus->multi_link && stream->m_rt_count > 1) {
+ } else if (multi_link) {
dev_err(bus->dev,
"Post bank switch ops not implemented\n");
goto error;
@@ -831,7 +834,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
goto error;
}
- if (bus->multi_link)
+ if (multi_link)
mutex_unlock(&bus->msg_lock);
}
--
2.17.1
[PATCH 03/11] soundwire: intel: add multi-link support
From: Pierre-Louis Bossart
The multi-link support is enabled with a hardware gsync signal
connecting all links. All commands and operations which typically are
handled on an SSP boundary will be deferred further and enabled across
all links with the 'syncGo' sequence.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 120 ++
1 file changed, 110 insertions(+), 10 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index c9ba706e20c6..a3aa8ab49285 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -34,6 +34,7 @@
#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIMEBIT(0)
#define SDW_INTEL_MASTER_DISABLE_CLOCK_STOPBIT(1)
#define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE BIT(2)
+#define SDW_INTEL_MASTER_DISABLE_MULTI_LINKBIT(3)
static int md_flags;
module_param_named(sdw_md_flags, md_flags, int, 0444);
@@ -555,6 +556,19 @@ static int intel_shim_sync_go_unlocked(struct sdw_intel
*sdw)
return ret;
}
+static int intel_shim_sync_go(struct sdw_intel *sdw)
+{
+ int ret;
+
+ mutex_lock(sdw->link_res->shim_lock);
+
+ ret = intel_shim_sync_go_unlocked(sdw);
+
+ mutex_unlock(sdw->link_res->shim_lock);
+
+ return ret;
+}
+
/*
* PDI routines
*/
@@ -1303,10 +1317,7 @@ static int intel_init(struct sdw_intel *sdw)
intel_shim_init(sdw, clock_stop);
- if (clock_stop)
- return 0;
-
- return sdw_cdns_init(&sdw->cdns);
+ return 0;
}
/*
@@ -1372,6 +1383,7 @@ int intel_master_startup(struct platform_device *pdev)
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
int link_flags;
+ bool multi_link;
u32 clock_stop_quirks;
int ret;
@@ -1382,7 +1394,16 @@ int intel_master_startup(struct platform_device *pdev)
return 0;
}
- /* Initialize shim, controller and Cadence IP */
+ link_flags = md_flags >> (bus->link_id * 8);
+ multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
+ if (!multi_link) {
+ dev_dbg(dev, "Multi-link is disabled\n");
+ bus->multi_link = false;
+ } else {
+ bus->multi_link = true;
+ }
+
+ /* Initialize shim, controller */
ret = intel_init(sdw);
if (ret)
goto err_init;
@@ -1401,12 +1422,33 @@ int intel_master_startup(struct platform_device *pdev)
goto err_init;
}
+ /*
+* follow recommended programming flows to avoid timeouts when
+* gsync is enabled
+*/
+ if (multi_link)
+ intel_shim_sync_arm(sdw);
+
+ ret = sdw_cdns_init(cdns);
+ if (ret < 0) {
+ dev_err(dev, "unable to initialize Cadence IP\n");
+ goto err_interrupt;
+ }
+
ret = sdw_cdns_exit_reset(cdns);
if (ret < 0) {
dev_err(dev, "unable to exit bus reset sequence\n");
goto err_interrupt;
}
+ if (multi_link) {
+ ret = intel_shim_sync_go(sdw);
+ if (ret < 0) {
+ dev_err(dev, "sync go failed: %d\n", ret);
+ goto err_interrupt;
+ }
+ }
+
/* Register DAIs */
ret = intel_register_dai(sdw);
if (ret) {
@@ -1418,7 +1460,6 @@ int intel_master_startup(struct platform_device *pdev)
intel_debugfs_init(sdw);
/* Enable runtime PM */
- link_flags = md_flags >> (bus->link_id * 8);
if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) {
pm_runtime_set_autosuspend_delay(dev,
INTEL_MASTER_SUSPEND_DELAY_MS);
@@ -1654,6 +1695,7 @@ static int intel_resume(struct device *dev)
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
int link_flags;
+ bool multi_link;
int ret;
if (bus->prop.hw_disabled) {
@@ -1662,6 +1704,9 @@ static int intel_resume(struct device *dev)
return 0;
}
+ link_flags = md_flags >> (bus->link_id * 8);
+ multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK);
+
if (pm_runtime_suspended(dev)) {
dev_dbg(dev, "%s: pm_runtime status was suspended, forcing
active\n", __func__);
@@ -1672,6 +1717,7 @@ static int intel_resume(struct device *dev)
pm_runtime_enable(dev);
link_flags = md_flags >> (bus->link_id * 8);
+
if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE))
pm_runtime_idle(dev);
}
@@
[PATCH 0/7] soundwire: filter out invalid PARITY errors
Some codecs may report fake PARITY errors in the initial state. This series will filter them out. Pierre-Louis Bossart (7): soundwire: bus: use property to set interrupt masks soundwire: bus: filter-out unwanted interrupt reports soundwire: slave: add first_interrupt_done status soundwire: bus: use quirk to filter out invalid parity errors ASoC: codecs: realtek-soundwire: ignore initial PARITY errors soundwire: bus: export broadcast read/write capability for tests soundwire: cadence: add parity error injection through debugfs drivers/soundwire/bus.c| 93 -- drivers/soundwire/bus.h| 4 ++ drivers/soundwire/cadence_master.c | 86 +++ drivers/soundwire/slave.c | 1 + include/linux/soundwire/sdw.h | 9 +++ sound/soc/codecs/max98373-sdw.c| 3 + sound/soc/codecs/rt1308-sdw.c | 3 + sound/soc/codecs/rt5682-sdw.c | 5 ++ sound/soc/codecs/rt700-sdw.c | 5 ++ sound/soc/codecs/rt711-sdw.c | 5 ++ sound/soc/codecs/rt715-sdw.c | 5 ++ sound/soc/codecs/wsa881x.c | 1 + 12 files changed, 202 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH 2/7] soundwire: bus: filter-out unwanted interrupt reports
From: Pierre-Louis Bossart
Unlike the traditional usage, in the SoundWire specification the
interrupt masks only gate the propagation of an interrupt condition to
the PING frame status. They do not gate the changes of the INT_STAT
registers, which will happen regardless of the mask settings. See
Figure 116 of the SoundWire 1.2 specification for an in-depth
description of the interrupt model.
When the bus driver reads the SCP_INT1_STAT register, it will retrieve
all the interrupt status, including for the mask fields that were not
explicitly set. For example, even if the PARITY mask is not set, the
PARITY error status will be reported if an implementation-defined
interrupt for jack detection is enabled and occurs.
Filtering undesired interrupt reports and handling has to be
implemented in software. This patch enables this filtering for the
INT1_IMPL_DEF, PARITY and BUS_CLASH interrupt sources.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 3b6a87bc254e..9e5bcd0dd115 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1394,12 +1394,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
* interrupt
*/
if (buf & SDW_SCP_INT1_PARITY) {
- dev_err(&slave->dev, "Parity error detected\n");
+ if (slave->prop.scp_int1_mask & SDW_SCP_INT1_PARITY)
+ dev_err(&slave->dev, "Parity error detected\n");
clear |= SDW_SCP_INT1_PARITY;
}
if (buf & SDW_SCP_INT1_BUS_CLASH) {
- dev_err(&slave->dev, "Bus clash error detected\n");
+ if (slave->prop.scp_int1_mask & SDW_SCP_INT1_BUS_CLASH)
+ dev_err(&slave->dev, "Bus clash detected\n");
clear |= SDW_SCP_INT1_BUS_CLASH;
}
@@ -1411,9 +1413,11 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
*/
if (buf & SDW_SCP_INT1_IMPL_DEF) {
- dev_dbg(&slave->dev, "Slave impl defined interrupt\n");
+ if (slave->prop.scp_int1_mask & SDW_SCP_INT1_IMPL_DEF) {
+ dev_dbg(&slave->dev, "Slave impl defined
interrupt\n");
+ slave_notify = true;
+ }
clear |= SDW_SCP_INT1_IMPL_DEF;
- slave_notify = true;
}
/* Check port 0 - 3 interrupts */
--
2.17.1
[PATCH 4/7] soundwire: bus: use quirk to filter out invalid parity errors
From: Pierre-Louis Bossart
If a Slave device reports with a quirk that its initial parity check
may be incorrect, filter it but keep the parity checks active in
steady state.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 8 +++-
include/linux/soundwire/sdw.h | 4
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index cddf39e3adfc..869290a8db40 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1362,6 +1362,8 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
unsigned long port;
bool slave_notify = false;
u8 buf, buf2[2], _buf, _buf2[2];
+ bool parity_check;
+ bool parity_quirk;
sdw_modify_slave_status(slave, SDW_SLAVE_ALERT);
@@ -1394,7 +1396,11 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
* interrupt
*/
if (buf & SDW_SCP_INT1_PARITY) {
- if (slave->prop.scp_int1_mask & SDW_SCP_INT1_PARITY)
+ parity_check = slave->prop.scp_int1_mask &
SDW_SCP_INT1_PARITY;
+ parity_quirk = !slave->first_interrupt_done &&
+ (slave->prop.quirks &
SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY);
+
+ if (parity_check && !parity_quirk)
dev_err(&slave->dev, "Parity error detected\n");
clear |= SDW_SCP_INT1_PARITY;
}
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 3550ab530c43..19bec6e4bbb6 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -356,6 +356,7 @@ struct sdw_dpn_prop {
* @src_dpn_prop: Source Data Port N properties
* @sink_dpn_prop: Sink Data Port N properties
* @scp_int1_mask: SCP_INT1_MASK desired settings
+ * @quirks: bitmask identifying deltas from the MIPI specification
*/
struct sdw_slave_prop {
u32 mipi_revision;
@@ -378,8 +379,11 @@ struct sdw_slave_prop {
struct sdw_dpn_prop *src_dpn_prop;
struct sdw_dpn_prop *sink_dpn_prop;
u8 scp_int1_mask;
+ u32 quirks;
};
+#define SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITYBIT(0)
+
/**
* struct sdw_master_prop - Master properties
* @revision: MIPI spec version of the implementation
--
2.17.1
[PATCH 6/7] soundwire: bus: export broadcast read/write capability for tests
From: Pierre-Louis Bossart
Provide prototype and export symbol to enable tests. The bus lock is
handled externally to avoid conflicts e.g. between kernel-generated
traffic and test traffic.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 56 +++--
drivers/soundwire/bus.h | 4 +++
2 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 869290a8db40..2e08e8f8b9aa 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -255,6 +255,21 @@ static int sdw_reset_page(struct sdw_bus *bus, u16 dev_num)
return ret;
}
+static int sdw_transfer_unlocked(struct sdw_bus *bus, struct sdw_msg *msg)
+{
+ int ret;
+
+ ret = do_transfer(bus, msg);
+ if (ret != 0 && ret != -ENODATA)
+ dev_err(bus->dev, "trf on Slave %d failed:%d\n",
+ msg->dev_num, ret);
+
+ if (msg->page)
+ sdw_reset_page(bus, msg->dev_num);
+
+ return ret;
+}
+
/**
* sdw_transfer() - Synchronous transfer message to a SDW Slave device
* @bus: SDW bus
@@ -266,13 +281,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
mutex_lock(&bus->msg_lock);
- ret = do_transfer(bus, msg);
- if (ret != 0 && ret != -ENODATA)
- dev_err(bus->dev, "trf on Slave %d failed:%d\n",
- msg->dev_num, ret);
-
- if (msg->page)
- sdw_reset_page(bus, msg->dev_num);
+ ret = sdw_transfer_unlocked(bus, msg);
mutex_unlock(&bus->msg_lock);
@@ -428,6 +437,39 @@ sdw_bwrite_no_pm(struct sdw_bus *bus, u16 dev_num, u32
addr, u8 value)
return sdw_transfer(bus, &msg);
}
+int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr)
+{
+ struct sdw_msg msg;
+ u8 buf;
+ int ret;
+
+ ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
+ SDW_MSG_FLAG_READ, &buf);
+ if (ret)
+ return ret;
+
+ ret = sdw_transfer_unlocked(bus, &msg);
+ if (ret < 0)
+ return ret;
+
+ return buf;
+}
+EXPORT_SYMBOL(sdw_bread_no_pm_unlocked);
+
+int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8
value)
+{
+ struct sdw_msg msg;
+ int ret;
+
+ ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
+ SDW_MSG_FLAG_WRITE, &value);
+ if (ret)
+ return ret;
+
+ return sdw_transfer_unlocked(bus, &msg);
+}
+EXPORT_SYMBOL(sdw_bwrite_no_pm_unlocked);
+
static int
sdw_read_no_pm(struct sdw_slave *slave, u32 addr)
{
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 82484f741168..c53345fbc4c7 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -168,6 +168,10 @@ sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8
val)
return sdw_write(slave, addr, tmp);
}
+/* broadcast read/write for tests */
+int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr);
+int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8
value);
+
/*
* At the moment we only track Master-initiated hw_reset.
* Additional fields can be added as needed
--
2.17.1
[PATCH 3/7] soundwire: slave: add first_interrupt_done status
From: Pierre-Louis Bossart
Some Slaves report incorrect information in their interrupt status
registers after a master/bus reset, track the initial interrupt
handling so that quirks can be introduced to filter out incorrect
information while keeping interrupts enabled in steady state.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 7 ++-
drivers/soundwire/slave.c | 1 +
include/linux/soundwire/sdw.h | 3 +++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 9e5bcd0dd115..cddf39e3adfc 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1472,6 +1472,9 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
goto io_err;
}
+ /* at this point all initial interrupt sources were handled */
+ slave->first_interrupt_done = true;
+
/*
* Read status again to ensure no new interrupts arrived
* while servicing interrupts.
@@ -1674,8 +1677,10 @@ void sdw_clear_slave_status(struct sdw_bus *bus, u32
request)
if (!slave)
continue;
- if (slave->status != SDW_SLAVE_UNATTACHED)
+ if (slave->status != SDW_SLAVE_UNATTACHED) {
sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED);
+ slave->first_interrupt_done = false;
+ }
/* keep track of request, used in pm_runtime resume */
slave->unattach_request = request;
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 0839445ee07b..755d43eba63b 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -57,6 +57,7 @@ static int sdw_slave_add(struct sdw_bus *bus,
slave->dev_num = 0;
init_completion(&slave->probe_complete);
slave->probed = false;
+ slave->first_interrupt_done = false;
mutex_lock(&bus->bus_lock);
list_add_tail(&slave->node, &bus->slaves);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 6d91f2ca20b2..3550ab530c43 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -608,6 +608,8 @@ struct sdw_slave_ops {
* between the Master suspending and the codec resuming, and make sure that
* when the Master triggered a reset the Slave is properly enumerated and
* initialized
+ * @first_interrupt_done: status flag tracking if the interrupt handling
+ * for a Slave happens for the first time after enumeration
*/
struct sdw_slave {
struct sdw_slave_id id;
@@ -629,6 +631,7 @@ struct sdw_slave {
struct completion enumeration_complete;
struct completion initialization_complete;
u32 unattach_request;
+ bool first_interrupt_done;
};
#define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev)
--
2.17.1
[PATCH 1/7] soundwire: bus: use property to set interrupt masks
From: Pierre-Louis Bossart
Add a slave-level property and program the SCP_INT1_MASK as desired by
the codec driver. Since there is no DisCo property this has to be an
implementation-specific firmware property or hard-coded in the driver.
The only functionality change is that implementation-defined
interrupts are no longer set for amplifiers - those interrupts are
typically for jack detection or acoustic event detection/hotwording.
Tested-by: Srinivas Kandagatla
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 12 ++--
include/linux/soundwire/sdw.h | 2 ++
sound/soc/codecs/max98373-sdw.c | 3 +++
sound/soc/codecs/rt1308-sdw.c | 2 ++
sound/soc/codecs/rt5682-sdw.c | 4
sound/soc/codecs/rt700-sdw.c| 4
sound/soc/codecs/rt711-sdw.c| 4
sound/soc/codecs/rt715-sdw.c| 4
sound/soc/codecs/wsa881x.c | 1 +
9 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index e6e0fb9a81b4..3b6a87bc254e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1184,13 +1184,13 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
return ret;
/*
-* Set bus clash, parity and SCP implementation
-* defined interrupt mask
-* TODO: Read implementation defined interrupt mask
-* from Slave property
+* Set SCP_INT1_MASK register, typically bus clash and
+* implementation-defined interrupt mask. The Parity detection
+* may not always be correct on startup so its use is
+* device-dependent, it might e.g. only be enabled in
+* steady-state after a couple of frames.
*/
- val = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH |
- SDW_SCP_INT1_PARITY;
+ val = slave->prop.scp_int1_mask;
/* Enable SCP interrupts */
ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 76052f12c9f7..6d91f2ca20b2 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -355,6 +355,7 @@ struct sdw_dpn_prop {
* @dp0_prop: Data Port 0 properties
* @src_dpn_prop: Source Data Port N properties
* @sink_dpn_prop: Sink Data Port N properties
+ * @scp_int1_mask: SCP_INT1_MASK desired settings
*/
struct sdw_slave_prop {
u32 mipi_revision;
@@ -376,6 +377,7 @@ struct sdw_slave_prop {
struct sdw_dp0_prop *dp0_prop;
struct sdw_dpn_prop *src_dpn_prop;
struct sdw_dpn_prop *sink_dpn_prop;
+ u8 scp_int1_mask;
};
/**
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
index 5fe724728e84..17fd1989e873 100644
--- a/sound/soc/codecs/max98373-sdw.c
+++ b/sound/soc/codecs/max98373-sdw.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "max98373.h"
#include "max98373-sdw.h"
@@ -287,6 +288,8 @@ static int max98373_read_prop(struct sdw_slave *slave)
unsigned long addr;
struct sdw_dpn_prop *dpn;
+ prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
+
/* BITMAP: 1000 Dataport 3 is active */
prop->source_ports = BIT(3);
/* BITMAP: 0010 Dataport 1 is active */
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index b0ba0d2acbdd..5cf10fd447eb 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -123,6 +123,8 @@ static int rt1308_read_prop(struct sdw_slave *slave)
unsigned long addr;
struct sdw_dpn_prop *dpn;
+ prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
+
prop->paging_support = true;
/* first we need to allocate memory for set bits in port lists */
diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c
index 94bf6bee78e6..544073975020 100644
--- a/sound/soc/codecs/rt5682-sdw.c
+++ b/sound/soc/codecs/rt5682-sdw.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -542,6 +543,9 @@ static int rt5682_read_prop(struct sdw_slave *slave)
unsigned long addr;
struct sdw_dpn_prop *dpn;
+ prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH |
+ SDW_SCP_INT1_PARITY;
+
prop->paging_support = false;
/* first we need to allocate memory for set bits in port lists */
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
index 4d14048d1197..a46b957a3f1b 100644
--- a/sound/soc/codecs/rt700-sdw.c
+++ b/sound/soc/codecs/rt700-sdw.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -338,6 +339,9 @@ static int rt700_read_prop(struct sdw_slave *slave)
[PATCH 5/7] ASoC: codecs: realtek-soundwire: ignore initial PARITY errors
From: Pierre-Louis Bossart The parity calculation is not reset on a Severe Reset, which leads to misleading/harmless errors reported on startup. The addition of a quirk helps filter out such errors while leaving the error checks on in steady-state. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Signed-off-by: Bard Liao --- sound/soc/codecs/rt1308-sdw.c | 1 + sound/soc/codecs/rt5682-sdw.c | 1 + sound/soc/codecs/rt700-sdw.c | 1 + sound/soc/codecs/rt711-sdw.c | 1 + sound/soc/codecs/rt715-sdw.c | 1 + 5 files changed, 5 insertions(+) diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index 5cf10fd447eb..81e3e2caeb2f 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -124,6 +124,7 @@ static int rt1308_read_prop(struct sdw_slave *slave) struct sdw_dpn_prop *dpn; prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; prop->paging_support = true; diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 544073975020..148d4cff267b 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -545,6 +545,7 @@ static int rt5682_read_prop(struct sdw_slave *slave) prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; prop->paging_support = false; diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index a46b957a3f1b..2d475405b20d 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -341,6 +341,7 @@ static int rt700_read_prop(struct sdw_slave *slave) prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; prop->paging_support = false; diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index a877e366fec5..7a1ae7442e75 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -345,6 +345,7 @@ static int rt711_read_prop(struct sdw_slave *slave) prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; prop->paging_support = false; diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index 0eb8943ed6ff..761d4663e813 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -439,6 +439,7 @@ static int rt715_read_prop(struct sdw_slave *slave) prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; prop->paging_support = false; -- 2.17.1
[PATCH 7/7] soundwire: cadence: add parity error injection through debugfs
From: Pierre-Louis Bossart
The Cadence IP can inject errors, let's make use of this capability to
test Slave parity error checks.
See e.g. example log where both the master and slave detect the parity
error injected on a dummy read command.
cd /sys/kernel/debug/soundwire/master-1/intel-sdw/
echo 1 > cdns-parity-error-injection
[ 44.756249] intel-master sdw-master-1: Parity error
[ 44.756313] intel-master sdw-master-1: Msg NACK received
[ 44.756366] intel-master sdw-master-1: Msg NACKed for Slave 15
[ 44.756375] intel-master sdw-master-1: trf on Slave 15 failed:-5
[ 44.756382] intel-master sdw-master-1: parity error injection, read: -5
[ 44.756649] rt1308 sdw:1:25d:1308:0: Parity error detected
The code makes sure the Master device is resumed, hence the clock
restarted, before sending a parity error.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 86 ++
1 file changed, 86 insertions(+)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 24eafe0aa1c3..807d70b82455 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -50,6 +51,9 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
#define CDNS_MCP_CONTROL_BLOCK_WAKEUP BIT(0)
#define CDNS_MCP_CMDCTRL 0x8
+
+#define CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR BIT(2)
+
#define CDNS_MCP_SSPSTAT 0xC
#define CDNS_MCP_FRAME_SHAPE 0x10
#define CDNS_MCP_FRAME_SHAPE_INIT 0x14
@@ -367,6 +371,85 @@ static int cdns_hw_reset(void *data, u64 value)
DEFINE_DEBUGFS_ATTRIBUTE(cdns_hw_reset_fops, NULL, cdns_hw_reset, "%llu\n");
+static int cdns_parity_error_injection(void *data, u64 value)
+{
+ struct sdw_cdns *cdns = data;
+ struct sdw_bus *bus;
+ int ret;
+
+ if (value != 1)
+ return -EINVAL;
+
+ bus = &cdns->bus;
+
+ /*
+* Resume Master device. If this results in a bus reset, the
+* Slave devices will re-attach and be re-enumerated.
+*/
+ ret = pm_runtime_get_sync(bus->dev);
+ if (ret < 0 && ret != -EACCES) {
+ dev_err_ratelimited(cdns->dev,
+ "pm_runtime_get_sync failed in %s, ret
%d\n",
+ __func__, ret);
+ pm_runtime_put_noidle(bus->dev);
+ return ret;
+ }
+
+ /*
+* wait long enough for Slave(s) to be in steady state. This
+* does not need to be super precise.
+*/
+ msleep(200);
+
+ /*
+* Take the bus lock here to make sure that any bus transactions
+* will be queued while we inject a parity error on a dummy read
+*/
+ mutex_lock(&bus->bus_lock);
+
+ /* program hardware to inject parity error */
+ cdns_updatel(cdns, CDNS_MCP_CMDCTRL,
+CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR,
+CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR);
+
+ /* commit changes */
+ cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE,
+CDNS_MCP_CONFIG_UPDATE_BIT,
+CDNS_MCP_CONFIG_UPDATE_BIT);
+
+ /* do a broadcast dummy read to avoid bus clashes */
+ ret = sdw_bread_no_pm_unlocked(&cdns->bus, 0xf, SDW_SCP_DEVID_0);
+ dev_info(cdns->dev, "parity error injection, read: %d\n", ret);
+
+ /* program hardware to disable parity error */
+ cdns_updatel(cdns, CDNS_MCP_CMDCTRL,
+CDNS_MCP_CMDCTRL_INSERT_PARITY_ERR,
+0);
+
+ /* commit changes */
+ cdns_updatel(cdns, CDNS_MCP_CONFIG_UPDATE,
+CDNS_MCP_CONFIG_UPDATE_BIT,
+CDNS_MCP_CONFIG_UPDATE_BIT);
+
+ /* Continue bus operation with parity error injection disabled */
+ mutex_unlock(&bus->bus_lock);
+
+ /* Userspace changed the hardware state behind the kernel's back */
+ add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+
+ /*
+* allow Master device to enter pm_runtime suspend. This may
+* also result in Slave devices suspending.
+*/
+ pm_runtime_mark_last_busy(bus->dev);
+ pm_runtime_put_autosuspend(bus->dev);
+
+ return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(cdns_parity_error_fops, NULL,
+cdns_parity_error_injection, "%llu\n");
+
/**
* sdw_cdns_debugfs_init() - Cadence debugfs init
* @cdns: Cadence instance
@@ -378,6 +461,9 @@ void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct
dentry *root)
debugfs_create_file("cdns-hw-reset", 020
[PATCH] soundwire: intel: don't return error when clock stop failed
dev->power.runtime_error will be set to the return value of the runtime
suspend callback function, and runtime resume function will return
-EINVAL if dev->power.runtime_error is not 0.
Somehow the codec rarely doesn't return an ACK to the clock prepare
command. If we stop the runtime suspend process and return error, we
will not be able to resume again. Likewise, if the codec lost sync and
did not rejoin, the resume operation will also fail. As a result, the
SoundWire bus can not be used anymore.
This patch suggests to finish the runtime suspend process even if we fail
to stop sdw bus clock. In the case where we do a hardware reset, the codecs
will be reconfigured completely. In the case where we use the regular clock
stop, the codecs keep their state and worst case will fall off the bus and
reattach.
The only drawback is that the power consumption may be higher and
device-initiated interrupts may be lost, but at least audio function can
still work after resume.
Signed-off-by: Bard Liao
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Reviewed-by: Pierre-Louis Bossart
---
drivers/soundwire/intel.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10b60b7b17bb..a2d5cdaa9998 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1673,10 +1673,12 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET ||
!clock_stop_quirks) {
+ bool wake_enable = true;
+
ret = sdw_cdns_clock_stop(cdns, true);
if (ret < 0) {
dev_err(dev, "cannot enable clock stop on suspend\n");
- return ret;
+ wake_enable = false;
}
ret = sdw_cdns_enable_interrupt(cdns, false);
@@ -1691,7 +1693,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
return ret;
}
- intel_shim_wake(sdw, true);
+ intel_shim_wake(sdw, wake_enable);
} else {
dev_err(dev, "%s clock_stop_quirks %x unsupported\n",
__func__, clock_stop_quirks);
--
2.17.1
[PATCH 2/5] soundwire: cadence: add status in dev_dbg 'State change' log
From: Pierre-Louis Bossart
The existing debug log only mentions a state change, without providing
any details. For integration and stress-tests, it's helpful to see in
the dmesg log the reason for the state change.
The value is intended for power users and isn't converted as
human-readable values. But for the record each device has a 4-bit
status:
BIT(0): Unattached
BIT(1): Attached
BIT(2): Alert
BIT(3): Reserved (should not happen)
Example:
[ 121.891288] intel-sdw intel-sdw.0: Slave status change: 0x2
<< this shows a Device0 Attached
[ 121.891295] soundwire sdw-master-0: Slave attached, programming device number
[ 121.891629] soundwire sdw-master-0: SDW Slave Addr: 30025d071101
[ 121.891632] soundwire sdw-master-0: SDW Slave class_id 1, part_id 711,
mfg_id 25d, unique_id 0, version 3
[ 121.892011] intel-sdw intel-sdw.0: Msg ignored for Slave 0
[ 121.892013] soundwire sdw-master-0: No more devices to enumerate
[ 121.892200] intel-sdw intel-sdw.0: Slave status change: 0x21
<< this shows the device now Attached as Device1 and Unattached as
Device0, i.e. a successful enumeration.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 9fa55164354a..801e1fef59d8 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -734,21 +734,18 @@ static void cdns_read_response(struct sdw_cdns *cdns)
}
static int cdns_update_slave_status(struct sdw_cdns *cdns,
- u32 slave0, u32 slave1)
+ u64 slave_intstat)
{
enum sdw_slave_status status[SDW_MAX_DEVICES + 1];
bool is_slave = false;
- u64 slave;
u32 mask;
int i, set_status;
- /* combine the two status */
- slave = ((u64)slave1 << 32) | slave0;
memset(status, 0, sizeof(status));
for (i = 0; i <= SDW_MAX_DEVICES; i++) {
- mask = (slave >> (i * CDNS_MCP_SLAVE_STATUS_NUM)) &
- CDNS_MCP_SLAVE_STATUS_BITS;
+ mask = (slave_intstat >> (i * CDNS_MCP_SLAVE_STATUS_NUM)) &
+ CDNS_MCP_SLAVE_STATUS_BITS;
if (!mask)
continue;
@@ -918,13 +915,17 @@ static void cdns_update_slave_status_work(struct
work_struct *work)
struct sdw_cdns *cdns =
container_of(work, struct sdw_cdns, work);
u32 slave0, slave1;
-
- dev_dbg_ratelimited(cdns->dev, "Slave status change\n");
+ u64 slave_intstat;
slave0 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT0);
slave1 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
- cdns_update_slave_status(cdns, slave0, slave1);
+ /* combine the two status */
+ slave_intstat = ((u64)slave1 << 32) | slave0;
+
+ dev_dbg_ratelimited(cdns->dev, "Slave status change: 0x%llx\n",
slave_intstat);
+
+ cdns_update_slave_status(cdns, slave_intstat);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0);
cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1);
--
2.17.1
[PATCH 1/5] soundwire: use consistent format for Slave devID logs
From: Pierre-Louis Bossart
We mix decimal and hexadecimal values, this leads to confusions in
dmesg logs and bug reports. Let's add a 0x prefix for all hexadecimal
values and a format when more than 4 bits are used.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 5 ++---
drivers/soundwire/slave.c | 10 --
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d1e8c3a54976..3cc006bfae71 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -679,9 +679,8 @@ void sdw_extract_slave_id(struct sdw_bus *bus,
id->class_id = SDW_CLASS_ID(addr);
dev_dbg(bus->dev,
- "SDW Slave class_id %x, part_id %x, mfg_id %x, unique_id %x,
version %x\n",
- id->class_id, id->part_id, id->mfg_id,
- id->unique_id, id->sdw_version);
+ "SDW Slave class_id 0x%02x, mfg_id 0x%04x, part_id 0x%04x,
unique_id 0x%x, version 0x%x\n",
+ id->class_id, id->mfg_id, id->part_id, id->unique_id,
id->sdw_version);
}
static int sdw_program_device_num(struct sdw_bus *bus)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index a08f4081c1c4..180f38bd003b 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -163,15 +163,13 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)
if (id.unique_id != id2.unique_id) {
dev_dbg(bus->dev,
- "Valid unique IDs %x %x for Slave mfg
%x part %d\n",
- id.unique_id, id2.unique_id,
- id.mfg_id, id.part_id);
+ "Valid unique IDs 0x%x 0x%x for Slave
mfg_id 0x%04x, part_id 0x%04x\n",
+ id.unique_id, id2.unique_id, id.mfg_id,
id.part_id);
ignore_unique_id = false;
} else {
dev_err(bus->dev,
- "Invalid unique IDs %x %x for Slave mfg
%x part %d\n",
- id.unique_id, id2.unique_id,
- id.mfg_id, id.part_id);
+ "Invalid unique IDs 0x%x 0x%x for Slave
mfg_id 0x%04x, part_id 0x%04x\n",
+ id.unique_id, id2.unique_id, id.mfg_id,
id.part_id);
return -ENODEV;
}
}
--
2.17.1
[PATCH 0/5] soundwire: fix ACK/NAK handling and improve log
The existing code reports a NAK only when ACK=0 This is not aligned with the SoundWire 1.x specifications. Table 32 in the SoundWire 1.2 specification shows that a Device shall not set NAK=1 if ACK=1. But Table 33 shows the Combined Response may very well be NAK=1/ACK=1, e.g. if another Device than the one addressed reports a parity error. NAK=1 signals a 'Command_Aborted', regardless of the ACK bit value. Move the tests for NAK so that the NAK=1/ACK=1 combination is properly detected according to the specification. Also, improve the demesg log to get more information for debugging. Bard Liao (1): soundwire: bus: add more details to track failed transfers Pierre-Louis Bossart (4): soundwire: use consistent format for Slave devID logs soundwire: cadence: add status in dev_dbg 'State change' log soundwire: cadence: fix ACK/NAK handling soundwire: cadence: adjust verbosity in response handling drivers/soundwire/bus.c| 11 ++- drivers/soundwire/cadence_master.c | 29 +++-- drivers/soundwire/slave.c | 10 -- 3 files changed, 25 insertions(+), 25 deletions(-) -- 2.17.1
[PATCH 5/5] soundwire: cadence: adjust verbosity in response handling
From: Pierre-Louis Bossart
There are too many logs on startup, e.g.
[ 8811.851497] cdns_fill_msg_resp: 2 callbacks suppressed
[ 8811.851497] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851498] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851499] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851499] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851500] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851500] intel-sdw intel-sdw.0: Msg Ack not received
[ 8811.851502] intel-sdw intel-sdw.0: Msg ignored for Slave 0
[ 8811.851503] soundwire sdw-master-0: No more devices to enumerate
We can skip the 'Msg Ack not received' since it's typical of the
enumeration end, and conversely add the information on which command
fails.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d3c9cf920cbd..8d7166ffd4ad 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -483,11 +483,11 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns,
for (i = 0; i < count; i++) {
if (!(cdns->response_buf[i] & CDNS_MCP_RESP_ACK)) {
no_ack = 1;
- dev_dbg_ratelimited(cdns->dev, "Msg Ack not
received\n");
+ dev_vdbg(cdns->dev, "Msg Ack not received, cmd %d\n",
i);
}
if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
nack = 1;
- dev_err_ratelimited(cdns->dev, "Msg NACK received\n");
+ dev_err_ratelimited(cdns->dev, "Msg NACK received, cmd
%d\n", i);
}
}
--
2.17.1
[PATCH 4/5] soundwire: cadence: fix ACK/NAK handling
From: Pierre-Louis Bossart
The existing code reports a NAK only when ACK=0
This is not aligned with the SoundWire 1.x specifications.
Table 32 in the SoundWire 1.2 specification shows that a Device shall
not set NAK=1 if ACK=1. But Table 33 shows the Combined Response
may very well be NAK=1/ACK=1, e.g. if another Device than the one
addressed reports a parity error.
NAK=1 signals a 'Command_Aborted', regardless of the ACK bit value.
Move the tests for NAK so that the NAK=1/ACK=1 combination is properly
detected according to the specification.
Fixes: 956baa1992f9a ('soundwire: cdns: Add sdw_master_ops and IO transfer
support')
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index 801e1fef59d8..d3c9cf920cbd 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -484,10 +484,10 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns,
if (!(cdns->response_buf[i] & CDNS_MCP_RESP_ACK)) {
no_ack = 1;
dev_dbg_ratelimited(cdns->dev, "Msg Ack not
received\n");
- if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
- nack = 1;
- dev_err_ratelimited(cdns->dev, "Msg NACK
received\n");
- }
+ }
+ if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
+ nack = 1;
+ dev_err_ratelimited(cdns->dev, "Msg NACK received\n");
}
}
--
2.17.1
[PATCH 3/5] soundwire: bus: add more details to track failed transfers
The current error log does not provide details on the type of transfers and which address/count was requested. All this information can help locate in which parts of the configuration process an error occurred. Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/bus.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 3cc006bfae71..6e1c988f3845 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -267,8 +267,10 @@ static int sdw_transfer_unlocked(struct sdw_bus *bus, struct sdw_msg *msg) ret = do_transfer(bus, msg); if (ret != 0 && ret != -ENODATA) - dev_err(bus->dev, "trf on Slave %d failed:%d\n", - msg->dev_num, ret); + dev_err(bus->dev, "trf on Slave %d failed:%d %s addr %x count %d\n", + msg->dev_num, ret, + (msg->flags & SDW_MSG_FLAG_WRITE) ? "write" : "read", + msg->addr, msg->len); if (msg->page) sdw_reset_page(bus, msg->dev_num); -- 2.17.1
[PATCH 0/2] ASoC/SoundWire: fix timeout values
The timeout for an individual transaction w/ the Cadence IP is the same as the entire resume operation for codecs. This doesn't make sense, we need to have at least one order of magnitude between individual transactions and the entire resume operation. Set the timeout on the Cadence side to 500ms and 5s for the codec resume. Both ASoC and SoundWire trees are fine for this series. Pierre-Louis Bossart (2): ASoC: codecs: soundwire: increase resume timeout soundwire: cadence: reduce timeout on transactions drivers/soundwire/cadence_master.c | 2 +- sound/soc/codecs/max98373-sdw.c| 4 +++- sound/soc/codecs/rt1308-sdw.c | 2 +- sound/soc/codecs/rt5682.h | 2 +- sound/soc/codecs/rt700-sdw.c | 2 +- sound/soc/codecs/rt711-sdw.c | 2 +- sound/soc/codecs/rt715-sdw.c | 2 +- 7 files changed, 9 insertions(+), 7 deletions(-) -- 2.17.1
[PATCH 1/2] ASoC: codecs: soundwire: increase resume timeout
From: Pierre-Louis Bossart
The resume operation relies on multiple transactions to synchronize
the regmap state, make sure the timeout is one order of magnitude
larger than an individual transaction, so that timeouts of failed
transactions are detected first.
Signed-off-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
sound/soc/codecs/max98373-sdw.c | 4 +++-
sound/soc/codecs/rt1308-sdw.c | 2 +-
sound/soc/codecs/rt5682.h | 2 +-
sound/soc/codecs/rt700-sdw.c| 2 +-
sound/soc/codecs/rt711-sdw.c| 2 +-
sound/soc/codecs/rt715-sdw.c| 2 +-
6 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
index ec2e79c57357..ad2d5d6a2fe4 100644
--- a/sound/soc/codecs/max98373-sdw.c
+++ b/sound/soc/codecs/max98373-sdw.c
@@ -251,6 +251,8 @@ static __maybe_unused int max98373_suspend(struct device
*dev)
return 0;
}
+#define MAX98373_PROBE_TIMEOUT 5000
+
static __maybe_unused int max98373_resume(struct device *dev)
{
struct sdw_slave *slave = dev_to_sdw_dev(dev);
@@ -264,7 +266,7 @@ static __maybe_unused int max98373_resume(struct device
*dev)
goto regmap_sync;
time = wait_for_completion_timeout(&slave->initialization_complete,
- msecs_to_jiffies(2000));
+
msecs_to_jiffies(MAX98373_PROBE_TIMEOUT));
if (!time) {
dev_err(dev, "Initialization not complete, timed out\n");
return -ETIMEDOUT;
diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
index ec5564f780e8..afd2c3b687cc 100644
--- a/sound/soc/codecs/rt1308-sdw.c
+++ b/sound/soc/codecs/rt1308-sdw.c
@@ -701,7 +701,7 @@ static int __maybe_unused rt1308_dev_suspend(struct device
*dev)
return 0;
}
-#define RT1308_PROBE_TIMEOUT 2000
+#define RT1308_PROBE_TIMEOUT 5000
static int __maybe_unused rt1308_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt5682.h b/sound/soc/codecs/rt5682.h
index 99b85cfe6248..1f9c51a5b9bf 100644
--- a/sound/soc/codecs/rt5682.h
+++ b/sound/soc/codecs/rt5682.h
@@ -1356,7 +1356,7 @@
#define RT5682_SAR_SOUR_TYPE (0x0)
/* soundwire timeout */
-#define RT5682_PROBE_TIMEOUT 2000
+#define RT5682_PROBE_TIMEOUT 5000
#define RT5682_STEREO_RATES SNDRV_PCM_RATE_8000_192000
diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c
index fb77e77a4ebd..ce9255b881d4 100644
--- a/sound/soc/codecs/rt700-sdw.c
+++ b/sound/soc/codecs/rt700-sdw.c
@@ -490,7 +490,7 @@ static int __maybe_unused rt700_dev_suspend(struct device
*dev)
return 0;
}
-#define RT700_PROBE_TIMEOUT 2000
+#define RT700_PROBE_TIMEOUT 5000
static int __maybe_unused rt700_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c
index fc7df79c3b91..756c0ada3b31 100644
--- a/sound/soc/codecs/rt711-sdw.c
+++ b/sound/soc/codecs/rt711-sdw.c
@@ -493,7 +493,7 @@ static int __maybe_unused rt711_dev_suspend(struct device
*dev)
return 0;
}
-#define RT711_PROBE_TIMEOUT 2000
+#define RT711_PROBE_TIMEOUT 5000
static int __maybe_unused rt711_dev_resume(struct device *dev)
{
diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c
index 8f0aa1e8a273..71dd3b97a459 100644
--- a/sound/soc/codecs/rt715-sdw.c
+++ b/sound/soc/codecs/rt715-sdw.c
@@ -533,7 +533,7 @@ static int __maybe_unused rt715_dev_suspend(struct device
*dev)
return 0;
}
-#define RT715_PROBE_TIMEOUT 2000
+#define RT715_PROBE_TIMEOUT 5000
static int __maybe_unused rt715_dev_resume(struct device *dev)
{
--
2.17.1
[PATCH 2/2] soundwire: cadence: reduce timeout on transactions
From: Pierre-Louis Bossart Currently the timeout for SoundWire individual transactions is 2s. This is too large in comparison with the enumeration and completion timeouts used in codec drivers. A command will typically be handled in less than 100us, so 500ms for the command completion is more than generous. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 9fa55164354a..f0b0ec173f8b 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -188,7 +188,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_PDI_CONFIG_PORT GENMASK(4, 0) /* Driver defaults */ -#define CDNS_TX_TIMEOUT2000 +#define CDNS_TX_TIMEOUT500 #define CDNS_SCP_RX_FIFOLEVEL 0x2 -- 2.17.1
[PATCH] soundwire: intel: move to auxiliary bus
From: Pierre-Louis Bossart
Now that the auxiliary_bus exists, there's no reason to use platform
devices as children of a PCI device any longer.
This patch refactors the code by extending a basic auxiliary device
with Intel link-specific structures that need to be passed between
controller and link levels. This refactoring is much cleaner with no
need for cross-pointers between device and link structures.
Note that the auxiliary bus API has separate init and add steps, which
requires more attention in the error unwinding paths. The main loop
needs to deal with kfree() and auxiliary_device_uninit() for the
current iteration before jumping to the common label which releases
everything allocated in prior iterations.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Ranjani Sridharan
Signed-off-by: Bard Liao
---
drivers/soundwire/Kconfig | 1 +
drivers/soundwire/intel.c | 52
drivers/soundwire/intel.h | 14 +-
drivers/soundwire/intel_init.c | 190 +++-
include/linux/soundwire/sdw_intel.h | 6 +-
5 files changed, 175 insertions(+), 88 deletions(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 016e74230bb7..2b7795233282 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
tristate "Intel SoundWire Master driver"
select SOUNDWIRE_CADENCE
select SOUNDWIRE_GENERIC_ALLOCATION
+ select AUXILIARY_BUS
depends on ACPI && SND_SOC
help
SoundWire Intel Master driver.
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d2254ee2fee2..039a101982c9 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1331,9 +1331,10 @@ static int intel_init(struct sdw_intel *sdw)
/*
* probe and init
*/
-static int intel_master_probe(struct platform_device *pdev)
+static int intel_link_probe(struct auxiliary_device *auxdev, const struct
auxiliary_device_id *id)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
+ struct sdw_intel_link_dev *ldev =
auxiliary_dev_to_sdw_intel_link_dev(auxdev);
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
struct sdw_bus *bus;
@@ -1346,14 +1347,14 @@ static int intel_master_probe(struct platform_device
*pdev)
cdns = &sdw->cdns;
bus = &cdns->bus;
- sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(dev);
+ sdw->instance = auxdev->id;
+ sdw->link_res = &ldev->link_res;
cdns->dev = dev;
cdns->registers = sdw->link_res->registers;
cdns->instance = sdw->instance;
cdns->msg_count = 0;
- bus->link_id = pdev->id;
+ bus->link_id = auxdev->id;
sdw_cdns_probe(cdns);
@@ -1386,10 +1387,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
}
-int intel_master_startup(struct platform_device *pdev)
+int intel_link_startup(struct auxiliary_device *auxdev)
{
struct sdw_cdns_stream_config config;
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
@@ -1526,9 +1527,9 @@ int intel_master_startup(struct platform_device *pdev)
return ret;
}
-static int intel_master_remove(struct platform_device *pdev)
+static void intel_link_remove(struct auxiliary_device *auxdev)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
@@ -1544,19 +1545,17 @@ static int intel_master_remove(struct platform_device
*pdev)
snd_soc_unregister_component(dev);
}
sdw_bus_master_delete(bus);
-
- return 0;
}
-int intel_master_process_wakeen_event(struct platform_device *pdev)
+int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_intel *sdw;
struct sdw_bus *bus;
void __iomem *shim;
u16 wake_sts;
- sdw = platform_get_drvdata(pdev);
+ sdw = dev_get_drvdata(dev);
bus = &sdw->cdns.bus;
if (bus->prop.hw_disabled) {
@@ -1978,17 +1977,22 @@ static const struct dev_pm_ops intel_pm = {
SET_RUNTIME_PM_OPS(intel_suspend_runtime, intel_resume_runtime, NULL)
};
-static st
[PATCH 0/5] soundwire: add missing \n in dev_err()
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps adding new ones... Pierre-Louis Bossart (5): soundwire: intel: add missing \n in dev_err() soundwire: bandwidth_allocation: add missing \n in dev_err() soundwire: cadence: add missing \n in dev_err() soundwire: stream: add missing \n in dev_err() soundwire: qcom: add missing \n in dev_err() drivers/soundwire/cadence_master.c | 2 +- .../soundwire/generic_bandwidth_allocation.c | 4 ++-- drivers/soundwire/intel.c | 18 +- drivers/soundwire/qcom.c | 2 +- drivers/soundwire/stream.c | 10 +- 5 files changed, 18 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH 2/5] soundwire: bandwidth_allocation: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/generic_bandwidth_allocation.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/generic_bandwidth_allocation.c
b/drivers/soundwire/generic_bandwidth_allocation.c
index 0bdef38c9a30..a9abb9722fde 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -406,14 +406,14 @@ int sdw_compute_params(struct sdw_bus *bus)
/* Computes clock frequency, frame shape and frame frequency */
ret = sdw_compute_bus_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute bus params failed: %d", ret);
+ dev_err(bus->dev, "Compute bus params failed: %d\n", ret);
return ret;
}
/* Compute transport and port params */
ret = sdw_compute_port_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute transport params failed: %d", ret);
+ dev_err(bus->dev, "Compute transport params failed: %d\n", ret);
return ret;
}
--
2.17.1
[PATCH 3/5] soundwire: cadence: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d05442e646a3..1b50cf7abe66 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1462,7 +1462,7 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool
block_wake)
*/
ret = sdw_bus_clk_stop(&cdns->bus);
if (ret < 0 && slave_present && ret != -ENODATA) {
- dev_err(cdns->dev, "bus clock stop failed %d", ret);
+ dev_err(cdns->dev, "bus clock stop failed %d\n", ret);
return ret;
}
--
2.17.1
[PATCH 1/5] soundwire: intel: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index d2254ee2fee2..e2e95115832a 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -997,7 +997,7 @@ static int intel_prepare(struct snd_pcm_substream
*substream,
dma = snd_soc_dai_get_dma_data(dai, substream);
if (!dma) {
- dev_err(dai->dev, "failed to get dma data in %s",
+ dev_err(dai->dev, "failed to get dma data in %s\n",
__func__);
return -EIO;
}
@@ -1061,7 +1061,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct
snd_soc_dai *dai)
ret = intel_free_stream(sdw, substream, dai, sdw->instance);
if (ret < 0) {
- dev_err(dai->dev, "intel_free_stream: failed %d", ret);
+ dev_err(dai->dev, "intel_free_stream: failed %d\n", ret);
return ret;
}
@@ -1634,7 +1634,7 @@ static int __maybe_unused intel_suspend(struct device
*dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1669,7 +1669,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1693,7 +1693,7 @@ static int __maybe_unused intel_suspend_runtime(struct
device *dev)
ret = intel_link_power_down(sdw);
if (ret) {
- dev_err(dev, "Link power down failed: %d", ret);
+ dev_err(dev, "Link power down failed: %d\n", ret);
return ret;
}
@@ -1742,7 +1742,7 @@ static int __maybe_unused intel_resume(struct device *dev)
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1826,7 +1826,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) {
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1871,7 +1871,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
@@ -1949,7 +1949,7 @@ static int __maybe_unused intel_resume_runtime(struct
device *dev)
ret = intel_init(sdw);
if (ret) {
- dev_err(dev, "%s failed: %d", __func__, ret);
+ dev_err(dev, "%s failed: %d\n", __func__, ret);
return ret;
}
--
2.17.1
[PATCH 4/5] soundwire: stream: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 1099b5d1262b..4915676c4ac2 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1513,7 +1513,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime
*stream,
if (bus->compute_params) {
ret = bus->compute_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute params failed: %d",
+ dev_err(bus->dev, "Compute params failed: %d\n",
ret);
return ret;
}
@@ -1791,7 +1791,7 @@ static int _sdw_deprepare_stream(struct
sdw_stream_runtime *stream)
if (bus->compute_params) {
ret = bus->compute_params(bus);
if (ret < 0) {
- dev_err(bus->dev, "Compute params failed: %d",
+ dev_err(bus->dev, "Compute params failed: %d\n",
ret);
return ret;
}
@@ -1855,7 +1855,7 @@ static int set_stream(struct snd_pcm_substream *substream,
for_each_rtd_dais(rtd, i, dai) {
ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream,
substream->stream);
if (ret < 0) {
- dev_err(rtd->dev, "failed to set stream pointer on dai
%s", dai->name);
+ dev_err(rtd->dev, "failed to set stream pointer on dai
%s\n", dai->name);
break;
}
}
@@ -1888,7 +1888,7 @@ int sdw_startup_stream(void *sdw_substream)
sdw_stream = sdw_alloc_stream(name);
if (!sdw_stream) {
- dev_err(rtd->dev, "alloc stream failed for substream DAI %s",
substream->name);
+ dev_err(rtd->dev, "alloc stream failed for substream DAI %s\n",
substream->name);
ret = -ENOMEM;
goto error;
}
@@ -1927,7 +1927,7 @@ void sdw_shutdown_stream(void *sdw_substream)
sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
if (IS_ERR(sdw_stream)) {
- dev_err(rtd->dev, "no stream found for DAI %s", dai->name);
+ dev_err(rtd->dev, "no stream found for DAI %s\n", dai->name);
return;
}
--
2.17.1
[PATCH 5/5] soundwire: qcom: add missing \n in dev_err()
From: Pierre-Louis Bossart
We fixed a lot of warnings in 2019 but the magic of copy-paste keeps
adding new ones...
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6d22df01f354..9cce09cba068 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -652,7 +652,7 @@ static int qcom_swrm_startup(struct snd_pcm_substream
*substream,
ret = snd_soc_dai_set_sdw_stream(codec_dai, sruntime,
substream->stream);
if (ret < 0 && ret != -ENOTSUPP) {
- dev_err(dai->dev, "Failed to set sdw stream on %s",
+ dev_err(dai->dev, "Failed to set sdw stream on %s\n",
codec_dai->name);
sdw_release_stream(sruntime);
return ret;
--
2.17.1
[PATCH] soundwire: cadence: only prepare attached devices on clock stop
From: Pierre-Louis Bossart
We sometimes see COMMAND_IGNORED responses during the clock stop
sequence. It turns out we already have information if devices are
present on a link, so we should only prepare those when they
are attached.
In addition, even when COMMAND_IGNORED are received, we should still
proceed with the clock stop. The device will not be prepared but
that's not a problem.
The only case where the clock stop will fail is if the Cadence IP
reports an error (including a timeout), or if the devices throw a
COMMAND_FAILED response.
BugLink: https://github.com/thesofproject/linux/issues/2621
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/cadence_master.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/cadence_master.c
b/drivers/soundwire/cadence_master.c
index d05442e646a3..57c59a33ce61 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1450,10 +1450,12 @@ int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool
block_wake)
}
/* Prepare slaves for clock stop */
- ret = sdw_bus_prep_clk_stop(&cdns->bus);
- if (ret < 0) {
- dev_err(cdns->dev, "prepare clock stop failed %d", ret);
- return ret;
+ if (slave_present) {
+ ret = sdw_bus_prep_clk_stop(&cdns->bus);
+ if (ret < 0 && ret != -ENODATA) {
+ dev_err(cdns->dev, "prepare clock stop failed %d\n",
ret);
+ return ret;
+ }
}
/*
--
2.17.1
[PATCH] soundwire: add slave device to linked list after device_register()
From: Pierre-Louis Bossart
We currently add the slave device to a linked list before
device_register(), and then remove it if device_register() fails.
It's not clear why this sequence was necessary, this patch moves the
linked list management to after the device_register().
Suggested-by: Keyon Jie
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/slave.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index 112b21967c7a..0c92db2e1ddc 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -65,9 +65,6 @@ int sdw_slave_add(struct sdw_bus *bus,
for (i = 0; i < SDW_MAX_PORTS; i++)
init_completion(&slave->port_ready[i]);
- mutex_lock(&bus->bus_lock);
- list_add_tail(&slave->node, &bus->slaves);
- mutex_unlock(&bus->bus_lock);
ret = device_register(&slave->dev);
if (ret) {
@@ -77,13 +74,15 @@ int sdw_slave_add(struct sdw_bus *bus,
* On err, don't free but drop ref as this will be freed
* when release method is invoked.
*/
- mutex_lock(&bus->bus_lock);
- list_del(&slave->node);
- mutex_unlock(&bus->bus_lock);
put_device(&slave->dev);
return ret;
}
+
+ mutex_lock(&bus->bus_lock);
+ list_add_tail(&slave->node, &bus->slaves);
+ mutex_unlock(&bus->bus_lock);
+
sdw_slave_debugfs_init(slave);
return ret;
--
2.17.1
[PATCH] soundwire: stream: fix memory leak in stream config error path
From: Rander Wang
When stream config is failed, master runtime will release all
slave runtime in the slave_rt_list, but slave runtime is not
added to the list at this time. This patch frees slave runtime
in the config error path to fix the memory leak.
Fixes: bbe7379d8040a ("soundwire: Add support for SoundWire stream management")
Signed-off-by: Rander Wang
Reviewed-by: Keyon Jie
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 9c064b672745..1eaedaaba094 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1375,8 +1375,16 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
}
ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
- if (ret)
+ if (ret) {
+ /*
+* sdw_release_master_stream will release s_rt in slave_rt_list
in
+* stream_error case, but s_rt is only added to slave_rt_list
+* when sdw_config_stream is successful, so free s_rt explicitly
+* when sdw_config_stream is failed.
+*/
+ kfree(s_rt);
goto stream_error;
+ }
list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
--
2.17.1
[PATCH] soundwire: intel_init: test link->cdns
intel_link_probe() could return error and dev_get_drvdata() will return
null in such case. So we have to test link->cdns after
link->cdns = dev_get_drvdata(&ldev->auxdev.dev);
Otherwise, we will meet the "kernel NULL pointer dereference" error.
Signed-off-by: Bard Liao
Reviewed-by: Ranjani Sridharan
Reviewed-by: Rander Wang
Reviewed-by: Pierre-Louis Bossart
---
drivers/soundwire/intel_init.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 5b32a2ffd376..5ef5bd0defab 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -250,6 +250,15 @@ static struct sdw_intel_ctx
link = &ldev->link_res;
link->cdns = dev_get_drvdata(&ldev->auxdev.dev);
+ if (!link->cdns) {
+ dev_err(&adev->dev, "failed to get link->cdns\n");
+ /*
+* 1 will be subtracted from i in the err label, but we
need to call
+* intel_link_dev_unregister for this ldev, so plus 1
now
+*/
+ i++;
+ goto err;
+ }
list_add_tail(&link->list, &ctx->link_list);
bus = &link->cdns->bus;
/* Calculate number of slaves */
--
2.17.1
[PATCH 0/2] soundwire: bus: handle errors in clock stop/start sequences
If a device lost sync and can no longer ACK a command, it may not be able to enter a lower-power state but it will still be able to resync when the clock restarts. In those cases, we want to continue with the clock stop sequence. This patch modifies the behavior when -ENODATA is received, with the error level demoted to a dev_dbg() since it's a recoverable issue. For consistency the log messages are also modified to be unique and self-explanatory, and missing logs are also added on clock stop exit. Pierre-Louis Bossart (2): soundwire: add macro to selectively change error levels soundwire: bus: handle errors in clock stop/start sequences drivers/soundwire/bus.c | 70 +++-- drivers/soundwire/bus.h | 8 + 2 files changed, 40 insertions(+), 38 deletions(-) -- 2.17.1
[PATCH 2/2] soundwire: bus: handle errors in clock stop/start sequences
From: Pierre-Louis Bossart
If a device lost sync and can no longer ACK a command, it may not be
able to enter a lower-power state but it will still be able to resync
when the clock restarts. In those cases, we want to continue with the
clock stop sequence.
This patch modifies the behavior when -ENODATA is received, with the
error level demoted to a dev_dbg() since it's a recoverable issue.
For consistency the log messages are also modified to be unique and
self-explanatory, and missing logs are also added on clock stop exit.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 70 +++--
1 file changed, 32 insertions(+), 38 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 9bd83c91a873..ea54a1f02252 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -848,8 +848,9 @@ static int sdw_slave_clk_stop_callback(struct sdw_slave
*slave,
if (slave->ops && slave->ops->clk_stop) {
ret = slave->ops->clk_stop(slave, mode, type);
if (ret < 0) {
- dev_err(&slave->dev,
- "Clk Stop type =%d failed: %d\n", type, ret);
+ sdw_dev_dbg_or_err(&slave->dev, ret != -ENODATA,
+ "Clk Stop mode %d type =%d failed:
%d\n",
+ mode, type, ret);
return ret;
}
}
@@ -878,7 +879,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
} else {
ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
if (ret < 0) {
- dev_err(&slave->dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
+ sdw_dev_dbg_or_err(&slave->dev, ret != -ENODATA,
+ "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
return ret;
}
val = ret;
@@ -888,8 +890,8 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
if (ret < 0)
- dev_err(&slave->dev,
- "Clock Stop prepare failed for slave: %d", ret);
+ sdw_dev_dbg_or_err(&slave->dev, ret != -ENODATA,
+ "SDW_SCP_SYSTEMCTRL write ignored:%d\n",
ret);
return ret;
}
@@ -907,7 +909,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
}
val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
- dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d",
+ dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d\n",
dev_num);
return 0;
}
@@ -916,7 +918,7 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
retry--;
} while (retry);
- dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d",
+ dev_err(bus->dev, "clock stop prep/de-prep failed slave:%d\n",
dev_num);
return -ETIMEDOUT;
@@ -956,19 +958,18 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
slave_mode = sdw_get_clk_stop_mode(slave);
slave->curr_clk_stop_mode = slave_mode;
- ret = sdw_slave_clk_stop_callback(slave, slave_mode,
- SDW_CLK_PRE_PREPARE);
+ ret = sdw_slave_clk_stop_callback(slave, slave_mode,
SDW_CLK_PRE_PREPARE);
if (ret < 0) {
- dev_err(&slave->dev,
- "pre-prepare failed:%d", ret);
+ sdw_dev_dbg_or_err(&slave->dev, ret != -ENODATA,
+ "clock stop pre prepare cb
failed:%d\n", ret);
return ret;
}
ret = sdw_slave_clk_stop_prepare(slave,
slave_mode, true);
if (ret < 0) {
- dev_err(&slave->dev,
- "pre-prepare failed:%d", ret);
+ sdw_dev_dbg_or_err(&slave->dev, ret != -ENODATA,
+ "clock stop prepare failed:%d\n",
ret);
return ret;
}
@@ -999,13 +1000,11 @@ int sdw_bus_prep_clk_s
[PATCH 1/2] soundwire: add macro to selectively change error levels
From: Pierre-Louis Bossart
We sometimes discard -ENODATA when reporting errors and lose all
traces of issues in the console log, add a macro to add use dev_dbg()
in such cases.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Rander Wang
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.h | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 40354469860a..8370216f95d4 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -227,4 +227,12 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
void sdw_clear_slave_status(struct sdw_bus *bus, u32 request);
int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size);
+#define sdw_dev_dbg_or_err(dev, is_err, fmt, ...) \
+ do {\
+ if (is_err) \
+ dev_err(dev, fmt, __VA_ARGS__); \
+ else\
+ dev_dbg(dev, fmt, __VA_ARGS__); \
+ } while (0)
+
#endif /* __SDW_BUS_H */
--
2.17.1
[PATCH v2] soundwire: intel: move to auxiliary bus
From: Pierre-Louis Bossart
Now that the auxiliary_bus exists, there's no reason to use platform
devices as children of a PCI device any longer.
This patch refactors the code by extending a basic auxiliary device
with Intel link-specific structures that need to be passed between
controller and link levels. This refactoring is much cleaner with no
need for cross-pointers between device and link structures.
Note that the auxiliary bus API has separate init and add steps, which
requires more attention in the error unwinding paths. The main loop
needs to deal with kfree() and auxiliary_device_uninit() for the
current iteration before jumping to the common label which releases
everything allocated in prior iterations.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Ranjani Sridharan
Signed-off-by: Bard Liao
---
v2:
- add link_dev_register for all kzalloc, device_init, and device_add.
---
drivers/soundwire/Kconfig | 1 +
drivers/soundwire/intel.c | 56 ---
drivers/soundwire/intel.h | 14 +-
drivers/soundwire/intel_init.c | 228 +++-
include/linux/soundwire/sdw_intel.h | 6 +-
5 files changed, 200 insertions(+), 105 deletions(-)
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 016e74230bb7..2b7795233282 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
tristate "Intel SoundWire Master driver"
select SOUNDWIRE_CADENCE
select SOUNDWIRE_GENERIC_ALLOCATION
+ select AUXILIARY_BUS
depends on ACPI && SND_SOC
help
SoundWire Intel Master driver.
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index fd95f94630b1..c11e3d8cd308 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1327,11 +1327,14 @@ static int intel_init(struct sdw_intel *sdw)
}
/*
- * probe and init
+ * probe and init (aux_dev_id argument is required by function prototype but
not used)
*/
-static int intel_master_probe(struct platform_device *pdev)
+static int intel_link_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *aux_dev_id)
+
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
+ struct sdw_intel_link_dev *ldev =
auxiliary_dev_to_sdw_intel_link_dev(auxdev);
struct sdw_intel *sdw;
struct sdw_cdns *cdns;
struct sdw_bus *bus;
@@ -1344,14 +1347,14 @@ static int intel_master_probe(struct platform_device
*pdev)
cdns = &sdw->cdns;
bus = &cdns->bus;
- sdw->instance = pdev->id;
- sdw->link_res = dev_get_platdata(dev);
+ sdw->instance = auxdev->id;
+ sdw->link_res = &ldev->link_res;
cdns->dev = dev;
cdns->registers = sdw->link_res->registers;
cdns->instance = sdw->instance;
cdns->msg_count = 0;
- bus->link_id = pdev->id;
+ bus->link_id = auxdev->id;
sdw_cdns_probe(cdns);
@@ -1384,10 +1387,10 @@ static int intel_master_probe(struct platform_device
*pdev)
return 0;
}
-int intel_master_startup(struct platform_device *pdev)
+int intel_link_startup(struct auxiliary_device *auxdev)
{
struct sdw_cdns_stream_config config;
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
@@ -1524,9 +1527,9 @@ int intel_master_startup(struct platform_device *pdev)
return ret;
}
-static int intel_master_remove(struct platform_device *pdev)
+static void intel_link_remove(struct auxiliary_device *auxdev)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_cdns *cdns = dev_get_drvdata(dev);
struct sdw_intel *sdw = cdns_to_intel(cdns);
struct sdw_bus *bus = &cdns->bus;
@@ -1542,19 +1545,17 @@ static int intel_master_remove(struct platform_device
*pdev)
snd_soc_unregister_component(dev);
}
sdw_bus_master_delete(bus);
-
- return 0;
}
-int intel_master_process_wakeen_event(struct platform_device *pdev)
+int intel_link_process_wakeen_event(struct auxiliary_device *auxdev)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &auxdev->dev;
struct sdw_intel *sdw;
struct sdw_bus *bus;
void __iomem *shim;
u16 wake_sts;
- sdw = platform_get_drvdata(pdev);
+ sdw = dev_get_drvdata(dev);
bus = &sdw->cdns.bus;
i
[RESEND PATCH 00/11] soundwire: some cleanup patches
To make soundwire driver more decent and less Cppcheck complaint. Pierre-Louis Bossart (11): soundwire: bus: use correct driver name in error messages soundwire: bus: test read status soundwire: bus: use consistent tests for return values soundwire: bus: demote clock stop prepare log to dev_dbg() soundwire: bus: uniquify dev_err() for SCP_INT access soundwire: bus: remove useless initialization soundwire: generic_bandwidth_allocation: remove useless init soundwire: intel: remove useless readl soundwire: qcom: check of_property_read status soundwire: stream: remove useless initialization soundwire: stream: remove useless bus initializations drivers/soundwire/bus.c | 54 +++ drivers/soundwire/bus_type.c | 15 -- .../soundwire/generic_bandwidth_allocation.c | 4 +- drivers/soundwire/intel.c | 2 - drivers/soundwire/qcom.c | 3 ++ drivers/soundwire/stream.c| 8 +-- 6 files changed, 52 insertions(+), 34 deletions(-) -- 2.17.1
[RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages
From: Pierre-Louis Bossart
None of the existing codec drivers set the sdw_driver.name, but
instead set sdw_driver.driver.name.
This leads to error messages such as
[ 23.935355] rt700 sdw:2:25d:700:0: Probe of (null) failed: -19
We could remove this sdw_driver.name if it doesn't have any
purpose. This patch only suggests using the proper indirection.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/bus_type.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 575b9bad99d5..893296f3fe39 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -82,6 +82,7 @@ static int sdw_drv_probe(struct device *dev)
struct sdw_slave *slave = dev_to_sdw_dev(dev);
struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
const struct sdw_device_id *id;
+ const char *name;
int ret;
/*
@@ -108,7 +109,10 @@ static int sdw_drv_probe(struct device *dev)
ret = drv->probe(slave, id);
if (ret) {
- dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
+ name = drv->name;
+ if (!name)
+ name = drv->driver.name;
+ dev_err(dev, "Probe of %s failed: %d\n", name, ret);
dev_pm_domain_detach(dev, false);
return ret;
}
@@ -174,11 +178,16 @@ static void sdw_drv_shutdown(struct device *dev)
*/
int __sdw_register_driver(struct sdw_driver *drv, struct module *owner)
{
+ const char *name;
+
drv->driver.bus = &sdw_bus_type;
if (!drv->probe) {
- pr_err("driver %s didn't provide SDW probe routine\n",
- drv->name);
+ name = drv->name;
+ if (!name)
+ name = drv->driver.name;
+
+ pr_err("driver %s didn't provide SDW probe routine\n", name);
return -EINVAL;
}
--
2.17.1
[RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values
From: Pierre-Louis Bossart
We use different styles to check the return values of IO related
routines. The majority of the cases use 'if (ret < 0)', align the
remaining cases for consistency.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 1c01cc192cbd..d39e5baa3e64 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -44,13 +44,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
}
ret = sdw_get_id(bus);
- if (ret) {
+ if (ret < 0) {
dev_err(parent, "Failed to get bus id\n");
return ret;
}
ret = sdw_master_device_add(bus, parent, fwnode);
- if (ret) {
+ if (ret < 0) {
dev_err(parent, "Failed to add master device at link %d\n",
bus->link_id);
return ret;
@@ -121,7 +121,7 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device
*parent,
else
ret = -ENOTSUPP; /* No ACPI/DT so error out */
- if (ret) {
+ if (ret < 0) {
dev_err(bus->dev, "Finding slaves failed:%d\n", ret);
return ret;
}
@@ -422,7 +422,7 @@ sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_READ, &buf);
- if (ret)
+ if (ret < 0)
return ret;
ret = sdw_transfer(bus, &msg);
@@ -440,7 +440,7 @@ sdw_bwrite_no_pm(struct sdw_bus *bus, u16 dev_num, u32
addr, u8 value)
ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_WRITE, &value);
- if (ret)
+ if (ret < 0)
return ret;
return sdw_transfer(bus, &msg);
@@ -454,7 +454,7 @@ int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr)
ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_READ, &buf);
- if (ret)
+ if (ret < 0)
return ret;
ret = sdw_transfer_unlocked(bus, &msg);
@@ -472,7 +472,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16
dev_num, u32 addr, u8 val
ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
SDW_MSG_FLAG_WRITE, &value);
- if (ret)
+ if (ret < 0)
return ret;
return sdw_transfer_unlocked(bus, &msg);
@@ -749,7 +749,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
* dev_num
*/
ret = sdw_assign_device_num(slave);
- if (ret) {
+ if (ret < 0) {
dev_err(bus->dev,
"Assign dev_num failed:%d\n",
ret);
@@ -886,7 +886,7 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
- if (ret != 0)
+ if (ret < 0)
dev_err(&slave->dev,
"Clock Stop prepare failed for slave: %d", ret);
@@ -1748,7 +1748,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
if (status[0] == SDW_SLAVE_ATTACHED) {
dev_dbg(bus->dev, "Slave attached, programming device
number\n");
ret = sdw_program_device_num(bus);
- if (ret)
+ if (ret < 0)
dev_err(bus->dev, "Slave attach failed: %d\n", ret);
/*
* programming a device number will have side effects,
@@ -1782,7 +1782,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
case SDW_SLAVE_ALERT:
ret = sdw_handle_slave_alerts(slave);
- if (ret)
+ if (ret < 0)
dev_err(&slave->dev,
"Slave %d alert handling failed: %d\n",
i, ret);
@@ -1801,7 +1801,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
attached_initializing = true;
ret = sdw_initialize_slave(slave);
- if (ret)
+ if (ret < 0)
dev_err(&slave->dev,
"Slave %d initializatio
[RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access
From: Pierre-Louis Bossart
We have multiple cases where we read/write SCP_INT registers, but the
same error message in all cases. Add a distinct error message for each
case to help debug.
Reported-by: Guennadi Liakhovetski
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 8b6d8fe934ae..a38b017f7a54 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1636,7 +1636,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(&slave->dev,
- "SDW_SCP_INT1 read failed:%d\n", ret);
+ "SDW_SCP_INT1 recheck read failed:%d\n", ret);
goto io_err;
}
_buf = ret;
@@ -1644,7 +1644,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, _buf2);
if (ret < 0) {
dev_err(&slave->dev,
- "SDW_SCP_INT2/3 read failed:%d\n", ret);
+ "SDW_SCP_INT2/3 recheck read failed:%d\n", ret);
goto io_err;
}
@@ -1652,7 +1652,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave
*slave)
ret = sdw_read_no_pm(slave, SDW_DP0_INT);
if (ret < 0) {
dev_err(&slave->dev,
- "SDW_DP0_INT read failed:%d\n", ret);
+ "SDW_DP0_INT recheck read failed:%d\n",
ret);
goto io_err;
}
sdca_cascade = ret & SDW_DP0_SDCA_CASCADE;
--
2.17.1
[RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg()
From: Pierre-Louis Bossart
There is no real reason to provide this information except for debug
sessions, hence dev_dbg() is a better fit.
Reported-by: Guennadi Liakhovetski
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d39e5baa3e64..8b6d8fe934ae 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -906,8 +906,8 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
}
val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
- dev_info(bus->dev, "clock stop prep/de-prep done
slave:%d",
-dev_num);
+ dev_dbg(bus->dev, "clock stop prep/de-prep done
slave:%d",
+ dev_num);
return 0;
}
--
2.17.1
[RESEND PATCH 06/11] soundwire: bus: remove useless initialization
From: Pierre-Louis Bossart
Cppcheck complains about a possible null pointer dereference, but it's
more like there is an unnecessary initialization before walking
through a list.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index a38b017f7a54..1a9e307e6a4c 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -593,7 +593,7 @@ EXPORT_SYMBOL(sdw_write);
/* called with bus_lock held */
static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
{
- struct sdw_slave *slave = NULL;
+ struct sdw_slave *slave;
list_for_each_entry(slave, &bus->slaves, node) {
if (slave->dev_num == i)
--
2.17.1
[RESEND PATCH 02/11] soundwire: bus: test read status
From: Pierre-Louis Bossart
In the existing code we may read a negative error value but still mask
it and write it back.
Make sure all reads are tested and errors not propagated further.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Guennadi Liakhovetski
Signed-off-by: Bard Liao
---
drivers/soundwire/bus.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 04eb879de145..1c01cc192cbd 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -875,8 +875,12 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave
*slave,
if (wake_en)
val |= SDW_SCP_SYSTEMCTRL_WAKE_UP_EN;
} else {
- val = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
-
+ ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
+ if (ret < 0) {
+ dev_err(&slave->dev, "SDW_SCP_SYSTEMCTRL read
failed:%d\n", ret);
+ return ret;
+ }
+ val = ret;
val &= ~(SDW_SCP_SYSTEMCTRL_CLK_STP_PREP);
}
@@ -895,8 +899,12 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus
*bus, u16 dev_num)
int val;
do {
- val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT) &
- SDW_SCP_STAT_CLK_STP_NF;
+ val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT);
+ if (val < 0) {
+ dev_err(bus->dev, "SDW_SCP_STAT bread failed:%d\n",
val);
+ return val;
+ }
+ val &= SDW_SCP_STAT_CLK_STP_NF;
if (!val) {
dev_info(bus->dev, "clock stop prep/de-prep done
slave:%d",
dev_num);
--
2.17.1
[RESEND PATCH 09/11] soundwire: qcom: check of_property_read status
From: Pierre-Louis Bossart
Cppcheck complains:
drivers/soundwire/qcom.c:773:6: style: Variable 'ret' is assigned a
value that is never used. [unreadVariable]
ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
^
The return value is checked for all other cases, not sure why it was
missed here.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/qcom.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 9cce09cba068..277f711e374d 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -772,6 +772,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl
*ctrl)
ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
bp_mode, nports);
+ if (ret)
+ return ret;
+
for (i = 0; i < nports; i++) {
ctrl->pconfig[i].si = si[i];
ctrl->pconfig[i].off1 = off1[i];
--
2.17.1
[RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations
From: Pierre-Louis Bossart
There is no need to assign a pointer to NULL if it's only used in a
loop and assigned within that loop.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 6a682179cd05..9c064b672745 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1449,7 +1449,7 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct
sdw_slave *slave,
static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
{
struct sdw_master_runtime *m_rt;
- struct sdw_bus *bus = NULL;
+ struct sdw_bus *bus;
/* Iterate for all Master(s) in Master list */
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
@@ -1471,7 +1471,7 @@ static void sdw_acquire_bus_lock(struct
sdw_stream_runtime *stream)
static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
{
struct sdw_master_runtime *m_rt;
- struct sdw_bus *bus = NULL;
+ struct sdw_bus *bus;
/* Iterate for all Master(s) in Master list */
list_for_each_entry_reverse(m_rt, &stream->master_list, stream_node) {
--
2.17.1
[RESEND PATCH 10/11] soundwire: stream: remove useless initialization
From: Pierre-Louis Bossart
Cppcheck complains about possible null pointer dereferences, but it's
more like there are unnecessary initializations before walking
through a list.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/stream.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 4915676c4ac2..6a682179cd05 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -261,7 +261,7 @@ static int sdw_program_master_port_params(struct sdw_bus
*bus,
*/
static int sdw_program_port_params(struct sdw_master_runtime *m_rt)
{
- struct sdw_slave_runtime *s_rt = NULL;
+ struct sdw_slave_runtime *s_rt;
struct sdw_bus *bus = m_rt->bus;
struct sdw_port_runtime *p_rt;
int ret = 0;
@@ -1470,7 +1470,7 @@ static void sdw_acquire_bus_lock(struct
sdw_stream_runtime *stream)
*/
static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
{
- struct sdw_master_runtime *m_rt = NULL;
+ struct sdw_master_runtime *m_rt;
struct sdw_bus *bus = NULL;
/* Iterate for all Master(s) in Master list */
--
2.17.1
[RESEND PATCH 08/11] soundwire: intel: remove useless readl
From: Pierre-Louis Bossart
Cppcheck complains:
drivers/soundwire/intel.c:564:15: style: Variable 'link_control' is
assigned a value that is never used. [unreadVariable]
link_control = intel_readl(shim, SDW_SHIM_LCTL);
This looks like a leftover from a previous version, remove.
Signed-off-by: Pierre-Louis Bossart
Reviewed-by: Guennadi Liakhovetski
Reviewed-by: Rander Wang
Reviewed-by: Kai Vehmanen
Signed-off-by: Bard Liao
---
drivers/soundwire/intel.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index e2e95115832a..fd95f94630b1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -561,8 +561,6 @@ static int intel_link_power_down(struct sdw_intel *sdw)
ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control,
cpa_mask);
}
- link_control = intel_readl(shim, SDW_SHIM_LCTL);
-
mutex_unlock(sdw->link_res->shim_lock);
if (ret < 0) {
--
2.17.1
