March 2021 - Alsa-devel - mailman.alsa-project.org

[PATCH V2] soundwire: qcom: use signed variable for error return
by Vinod Koul 31 Mar '21

31 Mar '21

We get warning of using a unsigned variable being compared to less than zero. The comparison is correct as it checks for errors from previous call to qcom_swrm_get_alert_slave_dev_num(), so we should use a signed variable here. While at it, drop the superfluous initialization as well drivers/soundwire/qcom.c: qcom_swrm_irq_handler() warn: impossible condition '(devnum < 0) => (0-255 < 0)' Reported-by: kernel test robot <lkp(a)intel.com> Signed-off-by: Vinod Koul <vkoul(a)kernel.org> --- 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 b08ecb9b418c..ec86c4e53fdb 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -428,7 +428,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id) struct qcom_swrm_ctrl *swrm = dev_id; u32 value, intr_sts, intr_sts_masked, slave_status; u32 i; - u8 devnum = 0; + int devnum; int ret = IRQ_HANDLED; swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts); -- 2.26.3

3 2

[PATCH] soundwire: qcom: use signed variable for error return
by Vinod Koul 31 Mar '21

31 Mar '21

We get warning for using a unsigned variable being compared to less than zero. The comparison is correct as it checks for errors from previous call to qcom_swrm_get_alert_slave_dev_num(), so we should use a signed variable instead. drivers/soundwire/qcom.c: qcom_swrm_irq_handler() warn: impossible condition '(devnum < 0) => (0-255 < 0)' Reported-by: kernel test robot <lkp(a)intel.com> Signed-off-by: Vinod Koul <vkoul(a)kernel.org> --- 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 b08ecb9b418c..55ed133c6704 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -428,7 +428,7 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id) struct qcom_swrm_ctrl *swrm = dev_id; u32 value, intr_sts, intr_sts_masked, slave_status; u32 i; - u8 devnum = 0; + s8 devnum = 0; int ret = IRQ_HANDLED; swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts); -- 2.26.3

3 3

Re: [PATCH] ASoC: sunxi: sun4i-codec: fill ASoC card owner
by Mark Brown 31 Mar '21

31 Mar '21

On Wed, Mar 31, 2021 at 04:59:15PM +0200, bage(a)linutronix.de wrote: > From: Bastian Germann <bage(a)linutronix.de> > > card->owner is a required property and since commit 81033c6b584b ("ALSA: > core: Warn on empty module") a warning is issued if it is empty. Add it. > This fixes following warning observed on Lamobo R1: > > WARNING: CPU: 1 PID: 190 at sound/core/init.c:207 snd_card_new+0x430/0x480 [snd] > Modules linked in: sun4i_codec(E+) sun4i_backend(E+) snd_soc_core(E) ... > CPU: 1 PID: 190 Comm: systemd-udevd Tainted: G C E 5.10.0-1-armmp #1 Debian 5.10.4-1 > Hardware name: Allwinner sun7i (A20) Family > [<c0ce8544>] (dump_backtrace) from [<c0ce88f0>] (show_stack+0x20/0x24) > [<c0ce88d0>] (show_stack) from [<c0ceda20>] (dump_stack+0xc8/0xdc) > [<c0ced958>] (dump_stack) from [<c034d49c>] (__warn+0xfc/0x158) > [<c034d3a0>] (__warn) from [<c0ce966c>] (warn_slowpath_fmt+0x70/0xe4) Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative (it often is for search engines if nothing else) then it's usually better to pull out the relevant sections.

1 0

Re: [PATCH -next] ASoC: cs35l35: Remove unused including <linux/version.h>
by Mark Brown 31 Mar '21

31 Mar '21

On Fri, Mar 26, 2021 at 02:13:31PM +0800, Zheng Yongjun wrote: > Remove including <linux/version.h> that don't need it. This doesn't apply against current code, please check and resend: Applying: ASoC: cs35l35: Remove unused including <linux/version.h> Using index info to reconstruct a base tree... error: patch failed: sound/soc/codecs/cs35l35.c:9 error: sound/soc/codecs/cs35l35.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. Patch failed at 0006 ASoC: cs35l35: Remove unused including <linux/version.h>

1 0

Re: [PATCH -next] ASoC: cs42l42: Remove unused including <linux/version.h>
by Mark Brown 31 Mar '21

31 Mar '21

On Fri, Mar 26, 2021 at 02:13:35PM +0800, Zheng Yongjun wrote: > Remove including <linux/version.h> that don't need it. This doesn't apply against current code, please check and resend: Applying: ASoC: cs42l42: Remove unused including <linux/version.h> Using index info to reconstruct a base tree... error: patch failed: sound/soc/codecs/cs42l42.c:11 error: sound/soc/codecs/cs42l42.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. Patch failed at 0006 ASoC: cs42l42: Remove unused including <linux/version.h>

1 0

Bus error with alsaloop
by GitHub issues - opened 31 Mar '21

31 Mar '21

alsa-project/alsa-utils issue #85 was opened from philipp-fischer: ## Behaviour alsaloop crashes with a bus error (SIGBUS) immediately ``` pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 Bus error ``` ## Analysis and debugging Very verbose output: ``` pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 -vvvvvvvvvvvvvvvvv !!!Scheduler set to Round Robin with priority 99 FAILED! playback hw:3,0: buffer_size=1920 playback hw:3,0: period_size=240 capture hw:2,0: buffer_size=1920 capture hw:2,0: period_size=240 playback hw:3,0: avail_min2=1560 capture hw:2,0: avail_min2=120 Hardware PCM card 3 'ICUSBAUDIO7D' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 1920 period_size : 240 period_time : 5000 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 1560 period_event : 0 start_threshold : 2147483647 stop_threshold : 1920 silence_threshold: 0 silence_size : 0 boundary : 2013265920 appl_ptr : 0 hw_ptr : 0 Hardware PCM card 2 'ICUSBAUDIO7D' device 0 subdevice 0 Its setup is: stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 1920 period_size : 240 period_time : 5000 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 240 period_event : 0 start_threshold : 2147483647 stop_threshold : 1920 silence_threshold: 0 silence_size : 0 boundary : 2013265920 appl_ptr : 0 hw_ptr : 0 Latency 480 frames, 10000.000us, 10.000000ms (100.0000Hz) playback hw:3,0/capture hw:2,0 sync type: SAMPLERATE (SRC_SINC_FASTEST) playback hw:3,0/capture hw:2,0: capt->buffer_size = 3840, play->buffer_size = 3840 playback hw:3,0/capture hw:2,0: Samplerate src_ratio update1: 1.00000000 New pitch for playback hw:3,0/capture hw:2,0: 1.00000000 (min/max samples = 0/0) playback hw:3,0/capture hw:2,0: silence queued 480 samples pool took 3074us playback hw:3,0/capture hw:2,0: pollfds handle playback hw:3,0: delay 480 / 3840 / 0 capture hw:2,0: delay 0 / 3840 / 0 playback hw:3,0/capture hw:2,0: prevents = 0x4, crevents = 0x0 playback hw:3,0/capture hw:2,0: queued 480/0 samples playback hw:3,0: end delay 480 / 3840 / 0 capture hw:2,0: end delay 0 / 3840 / 0 playback hw:3,0/capture hw:2,0: processing time 59us pool took 13us [...] playback hw:3,0/capture hw:2,0: pollfds handle playback hw:3,0: delay 323 / 3840 / 0 capture hw:2,0: delay 0 / 3840 / 0 playback hw:3,0/capture hw:2,0: prevents = 0x4, crevents = 0x0 playback hw:3,0/capture hw:2,0: queued 323/0 samples playback hw:3,0: end delay 323 / 3840 / 0 capture hw:2,0: end delay 0 / 3840 / 0 playback hw:3,0/capture hw:2,0: processing time 212us pool took 5us playback hw:3,0/capture hw:2,0: pollfds handle playback hw:3,0: delay 323 / 3840 / 0 capture hw:2,0: delay 48 / 3840 / 0 playback hw:3,0/capture hw:2,0: prevents = 0x4, crevents = 0x0 Bus error ``` Debugging with backtrace (binary without dbg symbols): ``` [...] Program received signal SIGBUS, Bus error. 0xb6dbd394 in src_short_to_float_array () from /lib/arm-linux-gnueabihf/libsamplerate.so.0 (gdb) bt #0 0xb6dbd394 in src_short_to_float_array () from /lib/arm-linux-gnueabihf/libsamplerate.so.0 #1 0x00016a20 in ?? () Backtrace stopped: previous frame identical to this frame (corrupt stack?) (gdb) ``` So the error seems to happen in `libsamplerate.so` Switching the sync method to "SIMPLE" prevents the error from happening as alsaloop doesn't seem to make use of libsamplerate in this case. The following modes run without failing but don't give the desired results: ``` pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 -S 0 # SYNC_TYPE_NONE pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 -S 1 # SYNC_TYPE_SIMPLE pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 -S 2 # SYNC_TYPE_CAPTRATESHIFT pi@pi-snapserver:~ $ alsaloop -C hw:2,0 -P hw:3,0 -S 3 # SYNC_TYPE_PLAYRATESHIFT ``` Only `SYNC_TYPE_SAMPLERATE` and `SYNC_TYPE_AUTO` (which selects SYNC_TYPE_SAMPLERATE) fail. The modes `SYNC_TYPE_NONE` and `SYNC_TYPE_SIMPLE` don't crash for me, but they don't work and result in increasing latency until the fifos reset. (Which is another issue I guess) Issue URL : https://github.com/alsa-project/alsa-utils/issues/85 Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[PATCH 1/2] ASoC: ak5558: Add support for ak5552
by Shengjiu Wang 31 Mar '21

31 Mar '21

From: Viorel Suman <viorel.suman(a)nxp.com> AK5552 is a 32-bit 2ch ADC and has the same register map as AK5558. Signed-off-by: Viorel Suman <viorel.suman(a)nxp.com> Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- sound/soc/codecs/ak5558.c | 93 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c index 85bdd0534180..af12afa83b4f 100644 --- a/sound/soc/codecs/ak5558.c +++ b/sound/soc/codecs/ak5558.c @@ -9,6 +9,7 @@ #include <linux/gpio/consumer.h> #include <linux/i2c.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> @@ -23,6 +24,11 @@ #include "ak5558.h" +enum ak555x_type { + AK5558, + AK5552, +}; + #define AK5558_NUM_SUPPLIES 2 static const char *ak5558_supply_names[AK5558_NUM_SUPPLIES] = { "DVDD", @@ -59,6 +65,15 @@ static const struct soc_enum ak5558_mono_enum[] = { ARRAY_SIZE(mono_texts), mono_texts), }; +static const char * const mono_5552_texts[] = { + "2 Slot", "1 Slot (Fixed)", "2 Slot", "1 Slot (Optimal)", +}; + +static const struct soc_enum ak5552_mono_enum[] = { + SOC_ENUM_SINGLE(AK5558_01_POWER_MANAGEMENT2, 1, + ARRAY_SIZE(mono_5552_texts), mono_5552_texts), +}; + static const char * const digfil_texts[] = { "Sharp Roll-Off", "Show Roll-Off", "Short Delay Sharp Roll-Off", "Short Delay Show Roll-Off", @@ -74,6 +89,11 @@ static const struct snd_kcontrol_new ak5558_snd_controls[] = { SOC_ENUM("AK5558 Digital Filter", ak5558_adcset_enum[0]), }; +static const struct snd_kcontrol_new ak5552_snd_controls[] = { + SOC_ENUM("AK5552 Monaural Mode", ak5552_mono_enum[0]), + SOC_ENUM("AK5552 Digital Filter", ak5558_adcset_enum[0]), +}; + static const struct snd_soc_dapm_widget ak5558_dapm_widgets[] = { /* Analog Input */ SND_SOC_DAPM_INPUT("AIN1"), @@ -97,6 +117,17 @@ static const struct snd_soc_dapm_widget ak5558_dapm_widgets[] = { SND_SOC_DAPM_AIF_OUT("SDTO", "Capture", 0, SND_SOC_NOPM, 0, 0), }; +static const struct snd_soc_dapm_widget ak5552_dapm_widgets[] = { + /* Analog Input */ + SND_SOC_DAPM_INPUT("AIN1"), + SND_SOC_DAPM_INPUT("AIN2"), + + SND_SOC_DAPM_ADC("ADC Ch1", NULL, AK5558_00_POWER_MANAGEMENT1, 0, 0), + SND_SOC_DAPM_ADC("ADC Ch2", NULL, AK5558_00_POWER_MANAGEMENT1, 1, 0), + + SND_SOC_DAPM_AIF_OUT("SDTO", "Capture", 0, SND_SOC_NOPM, 0, 0), +}; + static const struct snd_soc_dapm_route ak5558_intercon[] = { {"ADC Ch1", NULL, "AIN1"}, {"SDTO", NULL, "ADC Ch1"}, @@ -123,6 +154,14 @@ static const struct snd_soc_dapm_route ak5558_intercon[] = { {"SDTO", NULL, "ADC Ch8"}, }; +static const struct snd_soc_dapm_route ak5552_intercon[] = { + {"ADC Ch1", NULL, "AIN1"}, + {"SDTO", NULL, "ADC Ch1"}, + + {"ADC Ch2", NULL, "AIN2"}, + {"SDTO", NULL, "ADC Ch2"}, +}; + static int ak5558_set_mcki(struct snd_soc_component *component) { return snd_soc_component_update_bits(component, AK5558_02_CONTROL1, AK5558_CKS, @@ -267,6 +306,18 @@ static struct snd_soc_dai_driver ak5558_dai = { .ops = &ak5558_dai_ops, }; +static struct snd_soc_dai_driver ak5552_dai = { + .name = "ak5558-aif", + .capture = { + .stream_name = "Capture", + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_KNOT, + .formats = AK5558_FORMATS, + }, + .ops = &ak5558_dai_ops, +}; + static void ak5558_power_off(struct ak5558_priv *ak5558) { if (!ak5558->reset_gpiod) @@ -354,6 +405,21 @@ static const struct snd_soc_component_driver soc_codec_dev_ak5558 = { .non_legacy_dai_naming = 1, }; +static const struct snd_soc_component_driver soc_codec_dev_ak5552 = { + .probe = ak5558_probe, + .remove = ak5558_remove, + .controls = ak5552_snd_controls, + .num_controls = ARRAY_SIZE(ak5552_snd_controls), + .dapm_widgets = ak5552_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(ak5552_dapm_widgets), + .dapm_routes = ak5552_intercon, + .num_dapm_routes = ARRAY_SIZE(ak5552_intercon), + .idle_bias_on = 1, + .use_pmdown_time = 1, + .endianness = 1, + .non_legacy_dai_naming = 1, +}; + static const struct regmap_config ak5558_regmap = { .reg_bits = 8, .val_bits = 8, @@ -368,6 +434,7 @@ static int ak5558_i2c_probe(struct i2c_client *i2c) { struct ak5558_priv *ak5558; int ret = 0; + int dev_id; int i; ak5558 = devm_kzalloc(&i2c->dev, sizeof(*ak5558), GFP_KERNEL); @@ -396,11 +463,26 @@ static int ak5558_i2c_probe(struct i2c_client *i2c) return ret; } - ret = devm_snd_soc_register_component(&i2c->dev, - &soc_codec_dev_ak5558, - &ak5558_dai, 1); - if (ret) + dev_id = (uintptr_t)of_device_get_match_data(&i2c->dev); + switch (dev_id) { + case AK5552: + ret = devm_snd_soc_register_component(&i2c->dev, + &soc_codec_dev_ak5552, + &ak5552_dai, 1); + break; + case AK5558: + ret = devm_snd_soc_register_component(&i2c->dev, + &soc_codec_dev_ak5558, + &ak5558_dai, 1); + break; + default: + dev_err(&i2c->dev, "unexpected device type\n"); + return -EINVAL; + } + if (ret < 0) { + dev_err(&i2c->dev, "failed to register component: %d\n", ret); return ret; + } pm_runtime_enable(&i2c->dev); regcache_cache_only(ak5558->regmap, true); @@ -416,7 +498,8 @@ static int ak5558_i2c_remove(struct i2c_client *i2c) } static const struct of_device_id ak5558_i2c_dt_ids[] __maybe_unused = { - { .compatible = "asahi-kasei,ak5558"}, + { .compatible = "asahi-kasei,ak5558", .data = (void *) AK5558 }, + { .compatible = "asahi-kasei,ak5552", .data = (void *) AK5552 }, { } }; MODULE_DEVICE_TABLE(of, ak5558_i2c_dt_ids); -- 2.27.0

4 4

[PATCH 0/2] clean up dt_binding_check and dtbs_check warnings of renesas, rsnd.yaml
by Zhen Lei 31 Mar '21

31 Mar '21

When I do dt_binding_check for all YAML files, below warning is reported: /root/mainline/Documentation/devicetree/bindings/sound/renesas,rsnd.example.dt.yaml: sound@ec500000: 'dais' is a required property From schema: /root/mainline/Documentation/devicetree/bindings/sound/renesas,rsnd.yaml make dt_binding_check -j24 2>err.txt Before: cat err.txt | wc -l 2 After: cat err.txt | wc -l 0 make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/sound/renesas,rsnd.yaml -j24 2>err.txt Before: cat err.txt | wc -l 130 After: cat err.txt | wc -l 0 Zhen Lei (2): ASoC: dt-bindings: renesas, rsnd: Clear warning 'dais' is a required property ASoC: dt-bindings: renesas, rsnd: Clear warning 'ports' does not match any of the regexes Documentation/devicetree/bindings/sound/renesas,rsnd.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 1.8.3

3 4

[PATCH v2] soundwire: intel: move to auxiliary bus
by Bard Liao 31 Mar '21

31 Mar '21

From: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> 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 <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> --- 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 <linux/module.h> #include <linux/interrupt.h> #include <linux/io.h> -#include <linux/platform_device.h> +#include <linux/auxiliary_bus.h> #include <sound/pcm_params.h> #include <linux/pm_runtime.h> #include <sound/soc.h> @@ -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; if (bus->prop.hw_disabled) { @@ -1976,17 +1977,22 @@ static const struct dev_pm_ops intel_pm = { SET_RUNTIME_PM_OPS(intel_suspend_runtime, intel_resume_runtime, NULL) }; -static struct platform_driver sdw_intel_drv = { - .probe = intel_master_probe, - .remove = intel_master_remove, +static const struct auxiliary_device_id intel_link_id_table[] = { + { .name = "soundwire_intel.link" }, + {}, +}; +MODULE_DEVICE_TABLE(auxiliary, intel_link_id_table); + +static struct auxiliary_driver sdw_intel_drv = { + .probe = intel_link_probe, + .remove = intel_link_remove, .driver = { - .name = "intel-sdw", + /* auxiliary_driver_register() sets .name to be the modname */ .pm = &intel_pm, - } + }, + .id_table = intel_link_id_table }; - -module_platform_driver(sdw_intel_drv); +module_auxiliary_driver(sdw_intel_drv); MODULE_LICENSE("Dual BSD/GPL"); -MODULE_ALIAS("platform:intel-sdw"); -MODULE_DESCRIPTION("Intel Soundwire Master Driver"); +MODULE_DESCRIPTION("Intel Soundwire Link Driver"); diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index 06bac8ba14e9..0b47b148da3f 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -7,7 +7,6 @@ /** * struct sdw_intel_link_res - Soundwire Intel link resource structure, * typically populated by the controller driver. - * @pdev: platform_device * @mmio_base: mmio base of SoundWire registers * @registers: Link IO registers base * @shim: Audio shim pointer @@ -23,7 +22,6 @@ * @list: used to walk-through all masters exposed by the same controller */ struct sdw_intel_link_res { - struct platform_device *pdev; void __iomem *mmio_base; /* not strictly needed, useful for debug */ void __iomem *registers; void __iomem *shim; @@ -48,7 +46,15 @@ struct sdw_intel { #endif }; -int intel_master_startup(struct platform_device *pdev); -int intel_master_process_wakeen_event(struct platform_device *pdev); +int intel_link_startup(struct auxiliary_device *auxdev); +int intel_link_process_wakeen_event(struct auxiliary_device *auxdev); + +struct sdw_intel_link_dev { + struct auxiliary_device auxdev; + struct sdw_intel_link_res link_res; +}; + +#define auxiliary_dev_to_sdw_intel_link_dev(auxiliary_dev) \ + container_of(auxiliary_dev, struct sdw_intel_link_dev, auxdev) #endif /* __SDW_INTEL_LOCAL_H */ diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 05b726cdfebc..5b32a2ffd376 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -12,7 +12,7 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h> -#include <linux/platform_device.h> +#include <linux/auxiliary_bus.h> #include <linux/pm_runtime.h> #include <linux/soundwire/sdw_intel.h> #include "cadence_master.h" @@ -24,28 +24,108 @@ #define SDW_LINK_BASE 0x30000 #define SDW_LINK_SIZE 0x10000 +static void intel_link_dev_release(struct device *dev) +{ + struct auxiliary_device *auxdev = to_auxiliary_dev(dev); + struct sdw_intel_link_dev *ldev = auxiliary_dev_to_sdw_intel_link_dev(auxdev); + + kfree(ldev); +} + +/* alloc, init and add link devices */ +static struct sdw_intel_link_dev *intel_link_dev_register(struct sdw_intel_res *res, + struct sdw_intel_ctx *ctx, + struct fwnode_handle *fwnode, + const char *name, + int link_id) +{ + struct sdw_intel_link_dev *ldev; + struct sdw_intel_link_res *link; + struct auxiliary_device *auxdev; + int ret; + + ldev = kzalloc(sizeof(*ldev), GFP_KERNEL); + if (!ldev) + return ERR_PTR(-ENOMEM); + + auxdev = &ldev->auxdev; + auxdev->name = name; + auxdev->dev.parent = res->parent; + auxdev->dev.fwnode = fwnode; + auxdev->dev.release = intel_link_dev_release; + + /* we don't use an IDA since we already have a link ID */ + auxdev->id = link_id; + + /* + * keep a handle on the allocated memory, to be used in all other functions. + * Since the same pattern is used to skip links that are not enabled, there is + * no need to check if ctx->ldev[i] is NULL later on. + */ + ctx->ldev[link_id] = ldev; + + /* Add link information used in the driver probe */ + link = &ldev->link_res; + link->mmio_base = res->mmio_base; + link->registers = res->mmio_base + SDW_LINK_BASE + + (SDW_LINK_SIZE * link_id); + link->shim = res->mmio_base + SDW_SHIM_BASE; + link->alh = res->mmio_base + SDW_ALH_BASE; + + link->ops = res->ops; + link->dev = res->dev; + + link->clock_stop_quirks = res->clock_stop_quirks; + link->shim_lock = &ctx->shim_lock; + link->shim_mask = &ctx->shim_mask; + link->link_mask = ctx->link_mask; + + /* now follow the two-step init/add sequence */ + ret = auxiliary_device_init(auxdev); + if (ret < 0) { + dev_err(res->parent, "failed to initialize link dev %s link_id %d\n", + name, link_id); + kfree(ldev); + return ERR_PTR(ret); + } + + ret = auxiliary_device_add(&ldev->auxdev); + if (ret < 0) { + dev_err(res->parent, "failed to add link dev %s link_id %d\n", + ldev->auxdev.name, link_id); + /* ldev will be freed with the put_device() and .release sequence */ + auxiliary_device_uninit(&ldev->auxdev); + return ERR_PTR(ret); + } + + return ldev; +} + +static void intel_link_dev_unregister(struct sdw_intel_link_dev *ldev) +{ + auxiliary_device_delete(&ldev->auxdev); + auxiliary_device_uninit(&ldev->auxdev); +} + static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) { - struct sdw_intel_link_res *link = ctx->links; + struct sdw_intel_link_dev *ldev; u32 link_mask; int i; - if (!link) - return 0; - link_mask = ctx->link_mask; - for (i = 0; i < ctx->count; i++, link++) { + for (i = 0; i < ctx->count; i++) { if (!(link_mask & BIT(i))) continue; - if (link->pdev) { - pm_runtime_disable(&link->pdev->dev); - platform_device_unregister(link->pdev); - } + ldev = ctx->ldev[i]; - if (!link->clock_stop_quirks) - pm_runtime_put_noidle(link->dev); + pm_runtime_disable(&ldev->auxdev.dev); + intel_link_dev_unregister(ldev); + + if (!ldev->link_res.clock_stop_quirks) + pm_runtime_put_noidle(ldev->link_res.dev); } return 0; @@ -91,9 +171,8 @@ EXPORT_SYMBOL_NS(sdw_intel_thread, SOUNDWIRE_INTEL_INIT); static struct sdw_intel_ctx *sdw_intel_probe_controller(struct sdw_intel_res *res) { - struct platform_device_info pdevinfo; - struct platform_device *pdev; struct sdw_intel_link_res *link; + struct sdw_intel_link_dev *ldev; struct sdw_intel_ctx *ctx; struct acpi_device *adev; struct sdw_slave *slave; @@ -116,67 +195,60 @@ static struct sdw_intel_ctx count = res->count; dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count); - ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL); + /* + * we need to alloc/free memory manually and can't use devm: + * this routine may be called from a workqueue, and not from + * the parent .probe. + * If devm_ was used, the memory might never be freed on errors. + */ + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return NULL; ctx->count = count; - ctx->links = devm_kcalloc(&adev->dev, ctx->count, - sizeof(*ctx->links), GFP_KERNEL); - if (!ctx->links) + + /* + * allocate the array of pointers. The link-specific data is allocated + * as part of the first loop below and released with the auxiliary_device_uninit(). + * If some links are disabled, the link pointer will remain NULL. Given that the + * number of links is small, this is simpler than using a list to keep track of links. + */ + ctx->ldev = kcalloc(ctx->count, sizeof(*ctx->ldev), GFP_KERNEL); + if (!ctx->ldev) { + kfree(ctx); return NULL; + } - ctx->count = count; ctx->mmio_base = res->mmio_base; ctx->link_mask = res->link_mask; ctx->handle = res->handle; mutex_init(&ctx->shim_lock); - link = ctx->links; link_mask = ctx->link_mask; INIT_LIST_HEAD(&ctx->link_list); - /* Create SDW Master devices */ - for (i = 0; i < count; i++, link++) { - if (!(link_mask & BIT(i))) { - dev_dbg(&adev->dev, - "Link %d masked, will not be enabled\n", i); + for (i = 0; i < count; i++) { + if (!(link_mask & BIT(i))) continue; - } - link->mmio_base = res->mmio_base; - link->registers = res->mmio_base + SDW_LINK_BASE - + (SDW_LINK_SIZE * i); - link->shim = res->mmio_base + SDW_SHIM_BASE; - link->alh = res->mmio_base + SDW_ALH_BASE; - - link->ops = res->ops; - link->dev = res->dev; - - 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)); - - pdevinfo.parent = res->parent; - pdevinfo.name = "intel-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)) { - dev_err(&adev->dev, - "platform device creation failed: %ld\n", - PTR_ERR(pdev)); + /* + * init and add a device for each link + * + * The name of the device will be soundwire_intel.link.[i], + * with the "soundwire_intel" module prefix automatically added + * by the auxiliary bus core. + */ + ldev = intel_link_dev_register(res, + ctx, + acpi_fwnode_handle(adev), + "link", + i); + if (IS_ERR(ldev)) goto err; - } - link->pdev = pdev; - link->cdns = platform_get_drvdata(pdev); + + link = &ldev->link_res; + link->cdns = dev_get_drvdata(&ldev->auxdev.dev); list_add_tail(&link->list, &ctx->link_list); bus = &link->cdns->bus; @@ -185,8 +257,7 @@ static struct sdw_intel_ctx num_slaves++; } - ctx->ids = devm_kcalloc(&adev->dev, num_slaves, - sizeof(*ctx->ids), GFP_KERNEL); + ctx->ids = kcalloc(num_slaves, sizeof(*ctx->ids), GFP_KERNEL); if (!ctx->ids) goto err; @@ -204,8 +275,14 @@ static struct sdw_intel_ctx return ctx; err: - ctx->count = i; - sdw_intel_cleanup(ctx); + while (i--) { + if (!(link_mask & BIT(i))) + continue; + ldev = ctx->ldev[i]; + intel_link_dev_unregister(ldev); + } + kfree(ctx->ldev); + kfree(ctx); return NULL; } @@ -213,7 +290,7 @@ static int sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) { struct acpi_device *adev; - struct sdw_intel_link_res *link; + struct sdw_intel_link_dev *ldev; u32 caps; u32 link_mask; int i; @@ -232,27 +309,28 @@ sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) return -EINVAL; } - if (!ctx->links) + if (!ctx->ldev) return -EINVAL; - link = ctx->links; link_mask = ctx->link_mask; /* Startup SDW Master devices */ - for (i = 0; i < ctx->count; i++, link++) { + for (i = 0; i < ctx->count; i++) { if (!(link_mask & BIT(i))) continue; - intel_master_startup(link->pdev); + ldev = ctx->ldev[i]; - if (!link->clock_stop_quirks) { + intel_link_startup(&ldev->auxdev); + + if (!ldev->link_res.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); + pm_runtime_get_noresume(ldev->link_res.dev); } } @@ -297,27 +375,31 @@ EXPORT_SYMBOL_NS(sdw_intel_startup, SOUNDWIRE_INTEL_INIT); void sdw_intel_exit(struct sdw_intel_ctx *ctx) { sdw_intel_cleanup(ctx); + kfree(ctx->ids); + kfree(ctx->ldev); + kfree(ctx); } EXPORT_SYMBOL_NS(sdw_intel_exit, SOUNDWIRE_INTEL_INIT); void sdw_intel_process_wakeen_event(struct sdw_intel_ctx *ctx) { - struct sdw_intel_link_res *link; + struct sdw_intel_link_dev *ldev; u32 link_mask; int i; - if (!ctx->links) + if (!ctx->ldev) return; - link = ctx->links; link_mask = ctx->link_mask; /* Startup SDW Master devices */ - for (i = 0; i < ctx->count; i++, link++) { + for (i = 0; i < ctx->count; i++) { if (!(link_mask & BIT(i))) continue; - intel_master_process_wakeen_event(link->pdev); + ldev = ctx->ldev[i]; + + intel_link_process_wakeen_event(&ldev->auxdev); } } EXPORT_SYMBOL_NS(sdw_intel_process_wakeen_event, SOUNDWIRE_INTEL_INIT); diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 3a5446ac014a..1ebea7764011 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -58,7 +58,7 @@ struct sdw_intel_acpi_info { u32 link_mask; }; -struct sdw_intel_link_res; +struct sdw_intel_link_dev; /* Intel clock-stop/pm_runtime quirk definitions */ @@ -109,7 +109,7 @@ struct sdw_intel_slave_id { * Controller * @num_slaves: total number of devices exposed across all enabled links * @handle: ACPI parent handle - * @links: information for each link (controller-specific and kept + * @ldev: information for each link (controller-specific and kept * opaque here) * @ids: array of slave_id, representing Slaves exposed across all enabled * links @@ -123,7 +123,7 @@ struct sdw_intel_ctx { u32 link_mask; int num_slaves; acpi_handle handle; - struct sdw_intel_link_res *links; + struct sdw_intel_link_dev **ldev; struct sdw_intel_slave_id *ids; struct list_head link_list; struct mutex shim_lock; /* lock for access to shared SHIM registers */ -- 2.17.1

1 0

[PATCH v6 0/9] soundwire: qcom: various improvements
by Srinivas Kandagatla 30 Mar '21

30 Mar '21

Thanks for reviewing v5 of this patchset! During testing SoundWire controller on SM8250 MTP, we found few issues like all the interrupts are not handled, all transport parameters are not read from device tree. Patch to add Auto Enumeration supported by the controller is also part of this series. Other major issue was register read/writes which was interrupt based was an overhead and puts lot of limitation on context it can be used from. With previous approach number of interrupts generated after enumeration are around 130: $ cat /proc/interrupts | grep soundwire 21: 130 0 0 0 0 0 0 0 GICv3 234 Edge soundwire after this patch they are just 3 interrupts $ cat /proc/interrupts | grep soundwire 21: 3 0 0 0 0 0 0 0 GICv3 234 Edge soundwire So this patchset add various improvements to the existing driver to address above issues. Tested it on SM8250 MTP with 2x WSA881x speakers, HeadPhones on WCD938x via lpass-rx-macro and Analog MICs via lpass-tx-macro. Also tested on DragonBoard DB845c with 2xWSA881x speakers. Changes since v5: - Use BIT macro in interrupt routine - add comment for fifo write Srinivas Kandagatla (9): dt-bindings: soundwire: qcom: clarify data port bus parameters soundwire: qcom: add support to missing transport params soundwire: qcom: set continue execution flag for ignored commands soundwire: qcom: start the clock during initialization soundwire: qcom: update register read/write routine soundwire: qcom: add support to new interrupts soundwire: export sdw_compare_devid, sdw_extract_slave_id and sdw_slave_add soundwire: qcom: add auto enumeration support soundwire: qcom: wait for enumeration to be complete in probe .../bindings/soundwire/qcom,sdw.txt | 20 + drivers/soundwire/bus.c | 4 +- drivers/soundwire/qcom.c | 529 ++++++++++++++---- drivers/soundwire/slave.c | 1 + include/linux/soundwire/sdw.h | 2 + 5 files changed, 443 insertions(+), 113 deletions(-) -- 2.21.0

2 10