From bd84256e86ecfb117d80c52870f4ece744610c97 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 27 Jul 2020 05:59:45 +0800 Subject: soundwire: master: enable pm runtime The hierarchy of soundwire devices is platform device -> M device -> S device. A S device is physically attached on the platform device. So the platform device should be resumed when a S device is resumed. As the bridge of platform device and S device, we have to implement runtime pm on M driver. We have set runtime pm ops in M driver already, but still need to enable runtime pm. Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200726215945.3119-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/master.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c index 5f0b2189defe..3488bb824e84 100644 --- a/drivers/soundwire/master.c +++ b/drivers/soundwire/master.c @@ -154,6 +154,7 @@ int sdw_master_device_add(struct sdw_bus *bus, struct device *parent, bus->dev = &md->dev; bus->md = md; + pm_runtime_enable(&bus->md->dev); device_register_err: return ret; } @@ -166,6 +167,7 @@ device_register_err: */ int sdw_master_device_del(struct sdw_bus *bus) { + pm_runtime_disable(&bus->md->dev); device_unregister(bus->dev); return 0; -- cgit v1.2.3 From 9b3b4b3f2f2af863d2f6dd65afd295a5a673afa2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 22 Jul 2020 04:37:11 +0800 Subject: soundwire: intel: Add basic power management support Implement suspend/resume capabilities (not runtime_pm for now) The resume part is essentially a full-blown re-enumeration. When S0ix is supported, we will select clock stop mode when the ACPI target state is S0, and tear down the link for S3. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200721203723.18305-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 81 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a283670659a9..88aeef8b7c0c 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -463,7 +463,7 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable) mutex_unlock(sdw->link_res->shim_lock); } -static int __maybe_unused intel_link_power_down(struct sdw_intel *sdw) +static int intel_link_power_down(struct sdw_intel *sdw) { int link_control, spa_mask, cpa_mask; unsigned int link_id = sdw->instance; @@ -1376,12 +1376,89 @@ int intel_master_process_wakeen_event(struct platform_device *pdev) return 0; } +/* + * PM calls + */ + +#ifdef CONFIG_PM + +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; + 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 (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, false); + + return 0; +} + +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 ret; + + if (bus->prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + bus->link_id); + return 0; + } + + 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_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "unable to exit bus reset sequence during resume\n"); + return ret; + } + + return ret; +} + +#endif + +static const struct dev_pm_ops intel_pm = { + SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume) +}; + static struct platform_driver sdw_intel_drv = { .probe = intel_master_probe, .remove = intel_master_remove, .driver = { .name = "intel-sdw", - }, + .pm = &intel_pm, + } }; module_platform_driver(sdw_intel_drv); -- cgit v1.2.3 From ebf878eddbb449091078d2ed282aed42a4fcef34 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:12 +0800 Subject: soundwire: intel: add pm_runtime support 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 Link: https://lore.kernel.org/r/20200817152923.3259-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 @@ unlock: 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 (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, false); + + return 0; +} + static int intel_resume(struct device *dev) { struct sdw_cdns *cdns = dev_get_drvdata(dev); @@ -1446,10 +1515,45 @@ static int intel_resume(struct device *dev) return ret; } +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; + int ret; + + if (bus->prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + bus->link_id); + return 0; + } + + 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_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "unable to exit bus reset sequence during resume\n"); + return ret; + } + + return ret; +} + #endif static const struct dev_pm_ops intel_pm = { SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume) + SET_RUNTIME_PM_OPS(intel_suspend_runtime, intel_resume_runtime, NULL) }; static struct platform_driver sdw_intel_drv = { diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 047252a91c9e..a1f210853545 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "cadence_master.h" #include "intel.h" @@ -68,8 +69,10 @@ static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) if (!(link_mask & BIT(i))) continue; - if (link->pdev) + if (link->pdev) { + pm_runtime_disable(&link->pdev->dev); platform_device_unregister(link->pdev); + } } return 0; -- cgit v1.2.3 From b61b8b37888ab13d4bf1b8f406713d51f01568cf Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:13 +0800 Subject: soundwire: intel: reset pm_runtime status during system resume 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 Link: https://lore.kernel.org/r/20200817152923.3259-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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); -- cgit v1.2.3 From cb1e6d59e89ce272a67e6dc22e9ec72df622fb1b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:14 +0800 Subject: soundwire: intel: fix race condition on system resume 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 Link: https://lore.kernel.org/r/20200817152923.3259-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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; } -- cgit v1.2.3 From 99b6a30f9f9995be3698f59df9882490d604f5d1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:15 +0800 Subject: soundwire: intel: call helper to reset Slave states on resume 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 Link: https://lore.kernel.org/r/20200817152923.3259-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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"); -- cgit v1.2.3 From a5a0239c27fe1125826c5cad4dec9cd1fd960d4a Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 17 Aug 2020 23:29:16 +0800 Subject: 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 Link: https://lore.kernel.org/r/20200817152923.3259-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 @@ error: 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) { + /* + * we don't have a .suspend dai_ops, and we don't have access + * to the substream, so let's mark both capture and playback + * DMA contexts as suspended + */ + dma = dai->playback_dma_data; + if (dma) + dma->suspended = true; + + dma = dai->capture_dma_data; + if (dma) + dma->suspended = true; + } + + return 0; +} + static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction) { @@ -1066,6 +1134,7 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { static const struct snd_soc_component_driver dai_component = { .name = "soundwire", + .suspend = intel_component_dais_suspend }; static int intel_create_dai(struct sdw_cdns *cdns, -- cgit v1.2.3 From a2d9c161db24929a4b71399bdbe097d5f61b7d8d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:17 +0800 Subject: soundwire: intel: pm_runtime idle scheduling 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 Link: https://lore.kernel.org/r/20200817152923.3259-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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_RUNTIME BIT(0) +#define SDW_INTEL_MASTER_DISABLE_CLOCK_STOP BIT(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); -- cgit v1.2.3 From a320f41eac7b250c27db1c8071a984e584109ce1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:18 +0800 Subject: soundwire: intel: add CLK_STOP_TEARDOWN for pm_runtime suspend 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 Link: https://lore.kernel.org/r/20200817152923.3259-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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) { - intel_shim_wake(sdw, false); + ret = sdw_cdns_enable_interrupt(cdns, false); + if (ret < 0) { + dev_err(dev, "cannot disable interrupts on suspend\n"); + return ret; + } - return 0; + 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 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/intel.h +++ b/drivers/soundwire/intel.h @@ -17,6 +17,7 @@ * @dev: device implementing hw_params and free callbacks * @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 * @cdns: Cadence master descriptor * @list: used to walk-through all masters exposed by the same controller */ @@ -31,6 +32,7 @@ struct sdw_intel_link_res { struct device *dev; struct mutex *shim_lock; /* protect shared registers */ u32 *shim_mask; + u32 clock_stop_quirks; struct sdw_cdns *cdns; struct list_head list; }; diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index a1f210853545..dd1050743dca 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -249,6 +249,7 @@ static struct sdw_intel_ctx 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; -- cgit v1.2.3 From 6626a616aab5e5c0d4fceffd861a10582272fce9 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 17 Aug 2020 23:29:19 +0800 Subject: soundwire: intel: add CLK_STOP_BUS_RESET support 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 Link: https://lore.kernel.org/r/20200817152923.3259-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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); -- cgit v1.2.3 From caf688192bc452c0fdd04920c3ed2405a57ac10b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:20 +0800 Subject: soundwire: intel: add CLK_STOP_NOT_ALLOWED support 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 Link: https://lore.kernel.org/r/20200817152923.3259-10-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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); -- cgit v1.2.3 From ab996b2971d76e1602064b0ae4eb9af5def085d8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:21 +0800 Subject: soundwire: intel_init: handle power rail dependencies for clock stop mode 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 Link: https://lore.kernel.org/r/20200817152923.3259-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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; -- cgit v1.2.3 From 61fb830bf9ca02f040fdfe65b714bea492a44986 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 17 Aug 2020 23:29:22 +0800 Subject: soundwire: intel: support clock_stop mode without quirks 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 Link: https://lore.kernel.org/r/20200817152923.3259-12-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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); -- cgit v1.2.3 From 08abad9f45f105e947d91822bdbc47650fbd7773 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 17 Aug 2020 23:29:23 +0800 Subject: soundwire: intel: refine runtime pm for SDW_INTEL_CLK_STOP_BUS_RESET 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 Link: https://lore.kernel.org/r/20200817152923.3259-13-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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; -- cgit v1.2.3 From f046b2334083c6e2bab96a8d7eba76684cf4b03e Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 24 Aug 2020 21:32:34 +0800 Subject: soundwire: intel: fix intel_suspend/resume defined but not used warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When CONFIG_PM_SLEEP is not defined, GCC throws compilation warnings: drivers/soundwire/intel.c:1799:12: warning: ‘intel_resume’ defined but not used [-Wunused-function] static int intel_resume(struct device *dev) ^~~~~~~~~~~~ drivers/soundwire/intel.c:1683:12: warning: ‘intel_suspend’ defined but not used [-Wunused-function] static int intel_suspend(struct device *dev) ^~~~~~~~~~~~~ Fix by using __maybe_unused macro. Suggested-by: Vinod Koul Signed-off-by: Bard Liao Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200824133234.28115-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index dbcbe2708563..ebca8ced59ec 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1527,7 +1527,7 @@ int intel_master_process_wakeen_event(struct platform_device *pdev) #ifdef CONFIG_PM -static int intel_suspend(struct device *dev) +static int __maybe_unused intel_suspend(struct device *dev) { struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); @@ -1625,7 +1625,7 @@ static int intel_suspend_runtime(struct device *dev) return ret; } -static int intel_resume(struct device *dev) +static int __maybe_unused intel_resume(struct device *dev) { struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); -- cgit v1.2.3 From d0bbcb4e836f0ffc6c0aeba6341ce227384805e3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 31 Aug 2020 21:43:16 +0800 Subject: ASoC: codecs: soundwire: remove port_ready[] usage from codecs. port_ready will be changed to a fixed array in sdw.h and all initialization work will be done at soundwire side in the following patch. So remove them from codec drivers. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Reviewed-by: Rander Wang Reviewed-by: Guennadi Liakhovetski Acked-by: Mark Brown Link: https://lore.kernel.org/r/20200831134318.11443-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 +-------------------------------- 6 files changed, 6 insertions(+), 101 deletions(-) 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_ports = nval; prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); @@ -315,7 +314,6 @@ static int max98373_read_prop(struct sdw_slave *slave) /* do this again for sink now */ nval = hweight32(prop->sink_ports); - num_of_ports += nval; prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); @@ -333,17 +331,6 @@ static int max98373_read_prop(struct sdw_slave *slave) i++; } - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index b0ba0d2acbdd..09c69dbab12a 100644 --- a/sound/soc/codecs/rt1308-sdw.c +++ b/sound/soc/codecs/rt1308-sdw.c @@ -118,7 +118,7 @@ static int rt1308_clock_config(struct device *dev) static int rt1308_read_prop(struct sdw_slave *slave) { struct sdw_slave_prop *prop = &slave->prop; - int nval, i, num_of_ports = 1; + int nval, i; u32 bit; unsigned long addr; struct sdw_dpn_prop *dpn; @@ -131,7 +131,6 @@ static int rt1308_read_prop(struct sdw_slave *slave) /* for sink */ nval = hweight32(prop->sink_ports); - num_of_ports += nval; prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); @@ -149,17 +148,6 @@ static int rt1308_read_prop(struct sdw_slave *slave) i++; } - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 94bf6bee78e6..b7c97aba7f17 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -537,7 +537,7 @@ static int rt5682_update_status(struct sdw_slave *slave, static int rt5682_read_prop(struct sdw_slave *slave) { struct sdw_slave_prop *prop = &slave->prop; - int nval, i, num_of_ports = 1; + int nval, i; u32 bit; unsigned long addr; struct sdw_dpn_prop *dpn; @@ -549,7 +549,6 @@ static int rt5682_read_prop(struct sdw_slave *slave) prop->sink_ports = 0x2; /* BITMAP: 00000010 */ nval = hweight32(prop->source_ports); - num_of_ports += nval; prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); @@ -569,7 +568,6 @@ static int rt5682_read_prop(struct sdw_slave *slave) /* do this again for sink now */ nval = hweight32(prop->sink_ports); - num_of_ports += nval; prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); @@ -587,17 +585,6 @@ static int rt5682_read_prop(struct sdw_slave *slave) i++; } - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index 4d14048d1197..b19fbcc12c69 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -333,7 +333,7 @@ static int rt700_update_status(struct sdw_slave *slave, static int rt700_read_prop(struct sdw_slave *slave) { struct sdw_slave_prop *prop = &slave->prop; - int nval, i, num_of_ports = 1; + int nval, i; u32 bit; unsigned long addr; struct sdw_dpn_prop *dpn; @@ -345,7 +345,6 @@ static int rt700_read_prop(struct sdw_slave *slave) prop->sink_ports = 0xA; /* BITMAP: 00001010 */ nval = hweight32(prop->source_ports); - num_of_ports += nval; prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); @@ -365,7 +364,6 @@ static int rt700_read_prop(struct sdw_slave *slave) /* do this again for sink now */ nval = hweight32(prop->sink_ports); - num_of_ports += nval; prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); @@ -383,17 +381,6 @@ static int rt700_read_prop(struct sdw_slave *slave) i++; } - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 45b928954b58..dc4a2b482462 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -337,7 +337,7 @@ static int rt711_update_status(struct sdw_slave *slave, static int rt711_read_prop(struct sdw_slave *slave) { struct sdw_slave_prop *prop = &slave->prop; - int nval, i, num_of_ports = 1; + int nval, i; u32 bit; unsigned long addr; struct sdw_dpn_prop *dpn; @@ -349,7 +349,6 @@ static int rt711_read_prop(struct sdw_slave *slave) prop->sink_ports = 0x8; /* BITMAP: 00001000 */ nval = hweight32(prop->source_ports); - num_of_ports += nval; prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); @@ -369,7 +368,6 @@ static int rt711_read_prop(struct sdw_slave *slave) /* do this again for sink now */ nval = hweight32(prop->sink_ports); - num_of_ports += nval; prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->sink_dpn_prop), GFP_KERNEL); @@ -387,17 +385,6 @@ static int rt711_read_prop(struct sdw_slave *slave) i++; } - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; diff --git a/sound/soc/codecs/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index d11b23d6b240..d8ed07305ffc 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -431,7 +431,7 @@ static int rt715_update_status(struct sdw_slave *slave, static int rt715_read_prop(struct sdw_slave *slave) { struct sdw_slave_prop *prop = &slave->prop; - int nval, i, num_of_ports = 1; + int nval, i; u32 bit; unsigned long addr; struct sdw_dpn_prop *dpn; @@ -443,7 +443,6 @@ static int rt715_read_prop(struct sdw_slave *slave) prop->sink_ports = 0x0; /* BITMAP: 00000000 */ nval = hweight32(prop->source_ports); - num_of_ports += nval; prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval, sizeof(*prop->src_dpn_prop), GFP_KERNEL); @@ -460,36 +459,6 @@ static int rt715_read_prop(struct sdw_slave *slave) i++; } - /* do this again for sink now */ - nval = hweight32(prop->sink_ports); - num_of_ports += nval; - prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval, - sizeof(*prop->sink_dpn_prop), - GFP_KERNEL); - if (!prop->sink_dpn_prop) - return -ENOMEM; - - dpn = prop->sink_dpn_prop; - i = 0; - addr = prop->sink_ports; - for_each_set_bit(bit, &addr, 32) { - dpn[i].num = bit; - dpn[i].simple_ch_prep_sm = true; - dpn[i].ch_prep_timeout = 10; - i++; - } - - /* 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]); - /* set the timeout values */ prop->clk_stop_timeout = 20; -- cgit v1.2.3 From 63642595a78da42f841fabcc3f309f4c1362dc40 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 31 Aug 2020 21:43:17 +0800 Subject: soundwire: add definition for maximum number of ports A Device may have at most 15 physical ports (DP0, DP1..DP14). Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Reviewed-by: Rander Wang Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200831134318.11443-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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_BITS 48 #define SDW_MAX_DEVICES 11 -#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, -- cgit v1.2.3 From 6073755886a463a7a7aecdd0abb32a1d38bdb7e6 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 31 Aug 2020 21:43:18 +0800 Subject: soundwire: fix port_ready[] dynamic allocation in mipi_disco 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 Signed-off-by: Bard Liao Reviewed-by: Rander Wang Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200831134318.11443-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/mipi_disco.c | 18 +----------------- drivers/soundwire/slave.c | 4 ++++ include/linux/soundwire/sdw.h | 2 +- 3 files changed, 6 insertions(+), 18 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; -- cgit v1.2.3 From e4be9facb969b3cbb76ba7cce7dcd64fef463346 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:47 +0800 Subject: soundwire: intel: disable shim wake on suspend 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 Link: https://lore.kernel.org/r/20200901150556.19432-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index ebca8ced59ec..aa8484366c95 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1532,6 +1532,7 @@ static int __maybe_unused 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 __maybe_unused 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; } -- cgit v1.2.3 From 0ef2986e19c922e0e22206e02b3b1e52d11d6f38 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:48 +0800 Subject: soundwire: intel: ignore software command retries with multiple links synchronized in hardware, retrying commands in software is not recommended. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200901150556.19432-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index aa8484366c95..94a659e65f86 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; } -- cgit v1.2.3 From 857a7c429e33993df77602ebd682e034e73fe2d2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:49 +0800 Subject: soundwire: intel: add multi-link support 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 Link: https://lore.kernel.org/r/20200901150556.19432-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 94a659e65f86..259e3da98e42 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -34,6 +34,7 @@ #define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME BIT(0) #define SDW_INTEL_MASTER_DISABLE_CLOCK_STOP BIT(1) #define SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE BIT(2) +#define SDW_INTEL_MASTER_DISABLE_MULTI_LINK BIT(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 __maybe_unused 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 __maybe_unused 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 __maybe_unused 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); } @@ -1694,12 +1740,33 @@ static int __maybe_unused intel_resume(struct device *dev) return ret; } + /* + * follow recommended programming flows to avoid timeouts when + * gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(&sdw->cdns); + if (ret < 0) { + dev_err(dev, "unable to initialize Cadence IP 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; } + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(dev, "sync go failed during resume\n"); + return ret; + } + } + /* * after system resume, the pm_runtime suspend() may kick in * during the enumeration, before any children device force the @@ -1722,6 +1789,8 @@ static int intel_resume_runtime(struct device *dev) struct sdw_bus *bus = &cdns->bus; u32 clock_stop_quirks; bool clock_stop0; + int link_flags; + bool multi_link; int status; int ret; @@ -1731,6 +1800,9 @@ static int intel_resume_runtime(struct device *dev) return 0; } + link_flags = md_flags >> (bus->link_id * 8); + multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); + clock_stop_quirks = sdw->link_res->clock_stop_quirks; if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) { @@ -1752,11 +1824,32 @@ static int intel_resume_runtime(struct device *dev) return ret; } + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(&sdw->cdns); + if (ret < 0) { + dev_err(dev, "unable to initialize Cadence IP 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; } + + if (multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(dev, "sync go failed during resume\n"); + return ret; + } + } } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) { ret = intel_init(sdw); if (ret) { @@ -1773,11 +1866,18 @@ static int intel_resume_runtime(struct device *dev) */ clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); - /* - * make sure all Slaves are tagged as UNATTACHED and - * provide reason for reinitialization - */ 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 + */ + status = SDW_UNATTACH_REQUEST_MASTER_RESET; sdw_clear_slave_status(bus, status); } -- cgit v1.2.3 From d78071b4e1c3252fdceeb1402eb2541649a97206 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:50 +0800 Subject: soundwire: intel: add missing support for all clock stop modes 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 Link: https://lore.kernel.org/r/20200901150556.19432-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 259e3da98e42..8634d33c388c 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; + } + + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (multi_link) + intel_shim_sync_arm(sdw); - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; + /* + * 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) { -- cgit v1.2.3 From 88d7c71ea5b29b322d9c72103a196234cb5040db Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:51 +0800 Subject: soundwire: bus: update multi-link definition with hw sync details 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 Link: https://lore.kernel.org/r/20200901150556.19432-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 63e71645fd13..78f52cdeb2c9 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -828,6 +828,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; @@ -851,6 +856,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, -- cgit v1.2.3 From 94eed66107ffabdf9c07a922bfcd7e396fe83053 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:52 +0800 Subject: soundwire: intel: add multi-link hw_synchronization information set the flags as required by hardware implementation Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200901150556.19432-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 8634d33c388c..272826973426 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 */ -- cgit v1.2.3 From 063ff4e568cec51e110471836645bd5f94316cac Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:53 +0800 Subject: soundwire: stream: enable hw_sync as needed by hardware 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 Link: https://lore.kernel.org/r/20200901150556.19432-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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); } -- cgit v1.2.3 From f748f34ef96848fc576002084d7ef9773011ed6a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:54 +0800 Subject: soundwire: intel: add error log for clock-stop invalid configs Detect cases where the clock is assumed to be stopped but the IP is not in the relevant state. There is no real way to recover here, but adding an error log can help detect bad programming sequences or race conditions. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200901150556.19432-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 272826973426..97c8cfc54ddd 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1931,6 +1931,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); -- cgit v1.2.3 From de763fa888736f2ac2cb5bf7eac965e017310a7b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:55 +0800 Subject: soundwire: intel: pass link_mask information to each master 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 Link: https://lore.kernel.org/r/20200901150556.19432-10-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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)); -- cgit v1.2.3 From 5ee74eb280d0aa1f27fbdba5dcbde31e2df369e2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 1 Sep 2020 23:05:56 +0800 Subject: soundwire: intel: don't manage link power individually 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 Link: https://lore.kernel.org/r/20200901150556.19432-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 97c8cfc54ddd..710f5eba936b 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 sequence */ + link_control = intel_readl(shim, SDW_SHIM_LCTL); + + /* only power-down 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_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) + if (ret < 0) { + dev_err(sdw->cdns.dev, "%s: could not power down link\n", __func__); + return ret; + } sdw->cdns.link_up = false; return 0; -- cgit v1.2.3 From 25e804926da39f1de7ae486920bfe65b099195f1 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:14:56 +0530 Subject: soundwire: define and use addr bit masks Soundwire addr is a 52bit value encoding link, version, unique id, mfg id, part id and class id. Define bit masks for these and use FIELD_GET() to extract these fields. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-2-vkoul@kernel.org Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 78f52cdeb2c9..1e9010c139f0 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -5,6 +5,7 @@ #define __SOUNDWIRE_H #include +#include struct sdw_bus; struct sdw_slave; @@ -456,13 +457,19 @@ struct sdw_slave_id { * * The MIPI DisCo for SoundWire defines in addition the link_id as bits 51:48 */ - -#define SDW_DISCO_LINK_ID(adr) (((adr) >> 48) & GENMASK(3, 0)) -#define SDW_VERSION(adr) (((adr) >> 44) & GENMASK(3, 0)) -#define SDW_UNIQUE_ID(adr) (((adr) >> 40) & GENMASK(3, 0)) -#define SDW_MFG_ID(adr) (((adr) >> 24) & GENMASK(15, 0)) -#define SDW_PART_ID(adr) (((adr) >> 8) & GENMASK(15, 0)) -#define SDW_CLASS_ID(adr) ((adr) & GENMASK(7, 0)) +#define SDW_DISCO_LINK_ID_MASK GENMASK_ULL(51, 48) +#define SDW_VERSION_MASK GENMASK_ULL(47, 44) +#define SDW_UNIQUE_ID_MASK GENMASK_ULL(43, 40) +#define SDW_MFG_ID_MASK GENMASK_ULL(39, 24) +#define SDW_PART_ID_MASK GENMASK_ULL(23, 8) +#define SDW_CLASS_ID_MASK GENMASK_ULL(7, 0) + +#define SDW_DISCO_LINK_ID(addr) FIELD_GET(SDW_DISCO_LINK_ID_MASK, addr) +#define SDW_VERSION(addr) FIELD_GET(SDW_VERSION_MASK, addr) +#define SDW_UNIQUE_ID(addr) FIELD_GET(SDW_UNIQUE_ID_MASK, addr) +#define SDW_MFG_ID(addr) FIELD_GET(SDW_MFG_ID_MASK, addr) +#define SDW_PART_ID(addr) FIELD_GET(SDW_PART_ID_MASK, addr) +#define SDW_CLASS_ID(addr) FIELD_GET(SDW_CLASS_ID_MASK, addr) /** * struct sdw_slave_intr_status - Slave interrupt status -- cgit v1.2.3 From d5826a4bdbc88004279df1f24db61e8e48071f78 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:14:57 +0530 Subject: soundwire: bus: use FIELD_GET() use FIELD_GET() in bus code to extract field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-3-vkoul@kernel.org Signed-off-by: Vinod Koul --- 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 e6e0fb9a81b4..d808f0256ba0 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -347,8 +347,8 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave, return -EINVAL; } - msg->addr_page1 = (addr >> SDW_REG_SHIFT(SDW_SCP_ADDRPAGE1_MASK)); - msg->addr_page2 = (addr >> SDW_REG_SHIFT(SDW_SCP_ADDRPAGE2_MASK)); + msg->addr_page1 = FIELD_GET(SDW_SCP_ADDRPAGE1_MASK, addr); + msg->addr_page2 = FIELD_GET(SDW_SCP_ADDRPAGE2_MASK, addr); msg->addr |= BIT(15); msg->page = true; @@ -1420,7 +1420,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) port = buf & SDW_SCP_INT1_PORT0_3; /* To get port number corresponding to bits, shift it */ - port = port >> SDW_REG_SHIFT(SDW_SCP_INT1_PORT0_3); + port = FIELD_GET(SDW_SCP_INT1_PORT0_3, port); for_each_set_bit(bit, &port, 8) { sdw_handle_port_interrupt(slave, bit, &port_status[bit]); -- cgit v1.2.3 From bd6a024f21ceb37025dfd584f59cfeba850c8e02 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:14:58 +0530 Subject: soundwire: slave: use SDW_DISCO_LINK_ID() use SDW_DISCO_LINK_ID() in slave code to extract field values instead of open coding masks and shift operations to extract link_id Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-4-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/slave.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index a762ee24e6fa..2191dd6e7aa4 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -106,7 +106,7 @@ static bool find_slave(struct sdw_bus *bus, } /* Extract link id from ADR, Bit 51 to 48 (included) */ - link_id = (addr >> 48) & GENMASK(3, 0); + link_id = SDW_DISCO_LINK_ID(addr); /* Check for link_id match */ if (link_id != bus->link_id) -- cgit v1.2.3 From 41ff91741c25d4987bf0405fa219b9eb339f24ee Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:14:59 +0530 Subject: soundwire: stream: use FIELD_{GET|PREP} use FIELD_{GET|PREP} in stream code to get/set field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-5-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/stream.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index e4cf484f5905..afb95febb0d3 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -100,9 +100,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus, return ret; /* Program DPN_SampleCtrl2 register */ - wbuf = (t_params->sample_interval - 1); - wbuf &= SDW_DPN_SAMPLECTRL_HIGH; - wbuf >>= SDW_REG_SHIFT(SDW_DPN_SAMPLECTRL_HIGH); + wbuf = FIELD_GET(SDW_DPN_SAMPLECTRL_HIGH, t_params->sample_interval - 1); ret = sdw_write(slave, addr3, wbuf); if (ret < 0) { @@ -111,9 +109,8 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus, } /* Program DPN_HCtrl register */ - wbuf = t_params->hstart; - wbuf <<= SDW_REG_SHIFT(SDW_DPN_HCTRL_HSTART); - wbuf |= t_params->hstop; + wbuf = FIELD_PREP(SDW_DPN_HCTRL_HSTART, t_params->hstart); + wbuf |= FIELD_PREP(SDW_DPN_HCTRL_HSTOP, t_params->hstop); ret = sdw_write(slave, addr4, wbuf); if (ret < 0) @@ -157,8 +154,8 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus, } /* Program DPN_PortCtrl register */ - wbuf = p_params->data_mode << SDW_REG_SHIFT(SDW_DPN_PORTCTRL_DATAMODE); - wbuf |= p_params->flow_mode; + wbuf = FIELD_PREP(SDW_DPN_PORTCTRL_DATAMODE, p_params->data_mode); + wbuf |= FIELD_PREP(SDW_DPN_PORTCTRL_FLOWMODE, p_params->flow_mode); ret = sdw_update(s_rt->slave, addr1, 0xF, wbuf); if (ret < 0) { -- cgit v1.2.3 From 9972b90ae8fd9bcd6ecbaef33cd5e240b6dbc636 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:15:00 +0530 Subject: soundwire: qcom : use FIELD_{GET|PREP} use FIELD_{GET|PREP} in qcom driver to get/set field values instead of open coding masks and shift operations. Also, remove now unused register shift defines Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-6-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 915c2cf0c274..dafa3f3dd1ab 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -43,13 +43,10 @@ #define SWRM_CMD_FIFO_RD_FIFO_ADDR 0x318 #define SWRM_ENUMERATOR_CFG_ADDR 0x500 #define SWRM_MCP_FRAME_CTRL_BANK_ADDR(m) (0x101C + 0x40 * (m)) -#define SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT 3 #define SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK GENMASK(2, 0) #define SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK GENMASK(7, 3) -#define SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT 0 #define SWRM_MCP_CFG_ADDR 0x1048 #define SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK GENMASK(21, 17) -#define SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_SHFT 0x11 #define SWRM_DEF_CMD_NO_PINGS 0x1f #define SWRM_MCP_STATUS 0x104C #define SWRM_MCP_STATUS_BANK_NUM_MASK BIT(0) @@ -284,8 +281,8 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) u32 val; /* Clear Rows and Cols */ - val = (SWRM_MAX_ROW_VAL << SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT | - SWRM_MIN_COL_VAL << SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT); + val = FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK, SWRM_MAX_ROW_VAL); + val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK, SWRM_MIN_COL_VAL); ctrl->reg_write(ctrl, SWRM_MCP_FRAME_CTRL_BANK_ADDR(0), val); @@ -298,9 +295,7 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) /* Configure No pings */ ctrl->reg_read(ctrl, SWRM_MCP_CFG_ADDR, &val); - val &= ~SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK; - val |= (SWRM_DEF_CMD_NO_PINGS << - SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_SHFT); + val |= FIELD_PREP(SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK, SWRM_DEF_CMD_NO_PINGS); ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val); /* Configure number of retries of a read/write cmd */ @@ -355,11 +350,8 @@ static int qcom_swrm_pre_bank_switch(struct sdw_bus *bus) ctrl->reg_read(ctrl, reg, &val); - val &= ~SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK; - val &= ~SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK; - - val |= (SWRM_MAX_ROW_VAL << SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT | - SWRM_MAX_COL_VAL << SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT); + val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK, SWRM_MAX_COL_VAL); + val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK, SWRM_MAX_ROW_VAL); return ctrl->reg_write(ctrl, reg, val); } @@ -693,8 +685,8 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl) ctrl->reg_read(ctrl, SWRM_COMP_PARAMS, &val); - ctrl->num_dout_ports = val & SWRM_COMP_PARAMS_DOUT_PORTS_MASK; - ctrl->num_din_ports = (val & SWRM_COMP_PARAMS_DIN_PORTS_MASK) >> 5; + ctrl->num_dout_ports = FIELD_GET(SWRM_COMP_PARAMS_DOUT_PORTS_MASK, val); + ctrl->num_din_ports = FIELD_GET(SWRM_COMP_PARAMS_DIN_PORTS_MASK, val); ret = of_property_read_u32(np, "qcom,din-ports", &val); if (ret) -- cgit v1.2.3 From 3cf25d63b1b9871188c6838e630b337363287e17 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:15:01 +0530 Subject: soundwire: cadence: use FIELD_{GET|PREP} use FIELD_{GET|PREP} in cadence driver to get/set field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-7-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 61 ++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 24eafe0aa1c3..ecf503fb23e1 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -54,7 +54,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_MCP_FRAME_SHAPE 0x10 #define CDNS_MCP_FRAME_SHAPE_INIT 0x14 #define CDNS_MCP_FRAME_SHAPE_COL_MASK GENMASK(2, 0) -#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET 3 +#define CDNS_MCP_FRAME_SHAPE_ROW_MASK GENMASK(7, 3) #define CDNS_MCP_CONFIG_UPDATE 0x18 #define CDNS_MCP_CONFIG_UPDATE_BIT BIT(0) @@ -129,8 +129,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_MCP_CMD_SSP_TAG BIT(31) #define CDNS_MCP_CMD_COMMAND GENMASK(30, 28) #define CDNS_MCP_CMD_DEV_ADDR GENMASK(27, 24) -#define CDNS_MCP_CMD_REG_ADDR_H GENMASK(23, 16) -#define CDNS_MCP_CMD_REG_ADDR_L GENMASK(15, 8) +#define CDNS_MCP_CMD_REG_ADDR GENMASK(23, 8) #define CDNS_MCP_CMD_REG_DATA GENMASK(7, 0) #define CDNS_MCP_CMD_READ 2 @@ -417,8 +416,7 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns, /* fill response */ for (i = 0; i < count; i++) - msg->buf[i + offset] = cdns->response_buf[i] >> - SDW_REG_SHIFT(CDNS_MCP_RESP_RDATA); + msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, cdns->response_buf[i]); return SDW_CMD_OK; } @@ -441,14 +439,15 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd, addr = msg->addr; for (i = 0; i < count; i++) { - data = msg->dev_num << SDW_REG_SHIFT(CDNS_MCP_CMD_DEV_ADDR); - data |= cmd << SDW_REG_SHIFT(CDNS_MCP_CMD_COMMAND); - data |= addr++ << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L); + data = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num); + data |= FIELD_PREP(CDNS_MCP_CMD_COMMAND, cmd); + data |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR, addr); + addr++; if (msg->flags == SDW_MSG_FLAG_WRITE) data |= msg->buf[i + offset]; - data |= msg->ssp_sync << SDW_REG_SHIFT(CDNS_MCP_CMD_SSP_TAG); + data |= FIELD_PREP(CDNS_MCP_CMD_SSP_TAG, msg->ssp_sync); cdns_writel(cdns, base, data); base += CDNS_MCP_CMD_WORD_LEN; } @@ -483,12 +482,12 @@ cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg) cdns->msg_count = CDNS_SCP_RX_FIFOLEVEL; } - data[0] = msg->dev_num << SDW_REG_SHIFT(CDNS_MCP_CMD_DEV_ADDR); - data[0] |= 0x3 << SDW_REG_SHIFT(CDNS_MCP_CMD_COMMAND); + data[0] = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num); + data[0] |= FIELD_PREP(CDNS_MCP_CMD_COMMAND, 0x3); data[1] = data[0]; - data[0] |= SDW_SCP_ADDRPAGE1 << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L); - data[1] |= SDW_SCP_ADDRPAGE2 << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L); + data[0] |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR, SDW_SCP_ADDRPAGE1); + data[1] |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR, SDW_SCP_ADDRPAGE2); data[0] |= msg->addr_page1; data[1] |= msg->addr_page2; @@ -1041,9 +1040,10 @@ static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols) int r; r = sdw_find_row_index(n_rows); - c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK; + c = sdw_find_col_index(n_cols); - val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c; + val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_MASK, r); + val |= FIELD_PREP(CDNS_MCP_FRAME_SHAPE_COL_MASK, c); return val; } @@ -1170,12 +1170,9 @@ static int cdns_port_params(struct sdw_bus *bus, dpn_config = cdns_readl(cdns, dpn_config_off); - dpn_config |= ((p_params->bps - 1) << - SDW_REG_SHIFT(CDNS_DPN_CONFIG_WL)); - dpn_config |= (p_params->flow_mode << - SDW_REG_SHIFT(CDNS_DPN_CONFIG_PORT_FLOW)); - dpn_config |= (p_params->data_mode << - SDW_REG_SHIFT(CDNS_DPN_CONFIG_PORT_DAT)); + dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_WL, (p_params->bps - 1)); + dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_FLOW, p_params->flow_mode); + dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_DAT, p_params->data_mode); cdns_writel(cdns, dpn_config_off, dpn_config); @@ -1212,23 +1209,17 @@ static int cdns_transport_params(struct sdw_bus *bus, dpn_config = cdns_readl(cdns, dpn_config_off); - dpn_config |= (t_params->blk_grp_ctrl << - SDW_REG_SHIFT(CDNS_DPN_CONFIG_BGC)); - dpn_config |= (t_params->blk_pkg_mode << - SDW_REG_SHIFT(CDNS_DPN_CONFIG_BPM)); + dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BGC, t_params->blk_grp_ctrl); + dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BPM, t_params->blk_pkg_mode); cdns_writel(cdns, dpn_config_off, dpn_config); - dpn_offsetctrl |= (t_params->offset1 << - SDW_REG_SHIFT(CDNS_DPN_OFFSET_CTRL_1)); - dpn_offsetctrl |= (t_params->offset2 << - SDW_REG_SHIFT(CDNS_DPN_OFFSET_CTRL_2)); + dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_1, t_params->offset1); + dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_2, t_params->offset2); cdns_writel(cdns, dpn_offsetctrl_off, dpn_offsetctrl); - dpn_hctrl |= (t_params->hstart << - SDW_REG_SHIFT(CDNS_DPN_HCTRL_HSTART)); - dpn_hctrl |= (t_params->hstop << SDW_REG_SHIFT(CDNS_DPN_HCTRL_HSTOP)); - dpn_hctrl |= (t_params->lane_ctrl << - SDW_REG_SHIFT(CDNS_DPN_HCTRL_LCTRL)); + dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTART, t_params->hstart); + dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTOP, t_params->hstop); + dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_LCTRL, t_params->lane_ctrl); cdns_writel(cdns, dpn_hctrl_off, dpn_hctrl); cdns_writel(cdns, dpn_samplectrl_off, (t_params->sample_interval - 1)); @@ -1534,7 +1525,7 @@ void sdw_cdns_config_stream(struct sdw_cdns *cdns, val = pdi->num; val |= CDNS_PDI_CONFIG_SOFT_RESET; - val |= ((1 << ch) - 1) << SDW_REG_SHIFT(CDNS_PDI_CONFIG_CHANNEL); + val |= FIELD_PREP(CDNS_PDI_CONFIG_CHANNEL, (1 << ch) - 1); cdns_writel(cdns, CDNS_PDI_CONFIG(pdi->num), val); } EXPORT_SYMBOL(sdw_cdns_config_stream); -- cgit v1.2.3 From 3b4979cabd4b49b84f794d8dd1625bbdb1213b05 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:15:02 +0530 Subject: soundwire: intel: use FIELD_{GET|PREP} use FIELD_{GET|PREP} in intel driver to get/set field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-8-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 52 ++++++++++++++++------------------------------- 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 710f5eba936b..8a958b6cc0fe 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -329,8 +329,7 @@ static int intel_link_power_up(struct sdw_intel *sdw) /* set SyncPRD period */ sync_reg = intel_readl(shim, SDW_SHIM_SYNC); - sync_reg |= (syncprd << - SDW_REG_SHIFT(SDW_SHIM_SYNC_SYNCPRD)); + sync_reg |= FIELD_PREP(SDW_SHIM_SYNC_SYNCPRD, syncprd); /* Set SyncCPU bit */ sync_reg |= SDW_SHIM_SYNC_SYNCCPU; @@ -340,10 +339,8 @@ static int intel_link_power_up(struct sdw_intel *sdw) link_control = intel_readl(shim, SDW_SHIM_LCTL); /* 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); + spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, sdw->link_res->link_mask); + cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, sdw->link_res->link_mask); link_control |= spa_mask; @@ -451,7 +448,7 @@ static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop) intel_shim_glue_to_master_ip(sdw); - act |= 0x1 << SDW_REG_SHIFT(SDW_SHIM_CTMCTL_DOAIS); + act |= FIELD_PREP(SDW_SHIM_CTMCTL_DOAIS, 0x1); act |= SDW_SHIM_CTMCTL_DACTQE; act |= SDW_SHIM_CTMCTL_DODS; intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act); @@ -514,10 +511,8 @@ static int intel_link_power_down(struct sdw_intel *sdw) link_control = intel_readl(shim, SDW_SHIM_LCTL); /* only power-down 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); + spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, ~sdw->link_res->link_mask); + cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, sdw->link_res->link_mask); link_control &= spa_mask; @@ -604,12 +599,9 @@ static void intel_pdi_init(struct sdw_intel *sdw, /* PCM Stream Capability */ pcm_cap = intel_readw(shim, SDW_SHIM_PCMSCAP(link_id)); - config->pcm_bd = (pcm_cap & SDW_SHIM_PCMSCAP_BSS) >> - SDW_REG_SHIFT(SDW_SHIM_PCMSCAP_BSS); - config->pcm_in = (pcm_cap & SDW_SHIM_PCMSCAP_ISS) >> - SDW_REG_SHIFT(SDW_SHIM_PCMSCAP_ISS); - config->pcm_out = (pcm_cap & SDW_SHIM_PCMSCAP_OSS) >> - SDW_REG_SHIFT(SDW_SHIM_PCMSCAP_OSS); + config->pcm_bd = FIELD_GET(SDW_SHIM_PCMSCAP_BSS, pcm_cap); + config->pcm_in = FIELD_GET(SDW_SHIM_PCMSCAP_ISS, pcm_cap); + config->pcm_out = FIELD_GET(SDW_SHIM_PCMSCAP_OSS, pcm_cap); dev_dbg(sdw->cdns.dev, "PCM cap bd:%d in:%d out:%d\n", config->pcm_bd, config->pcm_in, config->pcm_out); @@ -617,12 +609,9 @@ static void intel_pdi_init(struct sdw_intel *sdw, /* PDM Stream Capability */ pdm_cap = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id)); - config->pdm_bd = (pdm_cap & SDW_SHIM_PDMSCAP_BSS) >> - SDW_REG_SHIFT(SDW_SHIM_PDMSCAP_BSS); - config->pdm_in = (pdm_cap & SDW_SHIM_PDMSCAP_ISS) >> - SDW_REG_SHIFT(SDW_SHIM_PDMSCAP_ISS); - config->pdm_out = (pdm_cap & SDW_SHIM_PDMSCAP_OSS) >> - SDW_REG_SHIFT(SDW_SHIM_PDMSCAP_OSS); + config->pdm_bd = FIELD_GET(SDW_SHIM_PDMSCAP_BSS, pdm_cap); + config->pdm_in = FIELD_GET(SDW_SHIM_PDMSCAP_ISS, pdm_cap); + config->pdm_out = FIELD_GET(SDW_SHIM_PDMSCAP_OSS, pdm_cap); dev_dbg(sdw->cdns.dev, "PDM cap bd:%d in:%d out:%d\n", config->pdm_bd, config->pdm_in, config->pdm_out); @@ -649,8 +638,7 @@ intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm) } else { count = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id)); - count = ((count & SDW_SHIM_PDMSCAP_CPSS) >> - SDW_REG_SHIFT(SDW_SHIM_PDMSCAP_CPSS)); + count = FIELD_GET(SDW_SHIM_PDMSCAP_CPSS, count); } /* zero based values for channel count in register */ @@ -724,10 +712,9 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) else pdi_conf &= ~(SDW_SHIM_PCMSYCM_DIR); - pdi_conf |= (pdi->intel_alh_id << - SDW_REG_SHIFT(SDW_SHIM_PCMSYCM_STREAM)); - pdi_conf |= (pdi->l_ch_num << SDW_REG_SHIFT(SDW_SHIM_PCMSYCM_LCHN)); - pdi_conf |= (pdi->h_ch_num << SDW_REG_SHIFT(SDW_SHIM_PCMSYCM_HCHN)); + pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_STREAM, pdi->intel_alh_id); + pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_LCHN, pdi->l_ch_num); + pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_HCHN, pdi->h_ch_num); intel_writew(shim, SDW_SHIM_PCMSYCHM(link_id, pdi->num), pdi_conf); } @@ -747,11 +734,8 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) /* Program Stream config ALH register */ conf = intel_readl(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id)); - conf |= (SDW_ALH_STRMZCFG_DMAT_VAL << - SDW_REG_SHIFT(SDW_ALH_STRMZCFG_DMAT)); - - conf |= ((pdi->ch_count - 1) << - SDW_REG_SHIFT(SDW_ALH_STRMZCFG_CHN)); + conf |= FIELD_PREP(SDW_ALH_STRMZCFG_DMAT, SDW_ALH_STRMZCFG_DMAT_VAL); + conf |= FIELD_PREP(SDW_ALH_STRMZCFG_CHN, pdi->ch_count - 1); intel_writel(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id), conf); } -- cgit v1.2.3 From c30f92984117658d614e405ea476c3e8ebbd10aa Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:15:03 +0530 Subject: soundwire: intel_init: use FIELD_{GET|PREP} use FIELD_{GET|PREP} in intel_init driver to get/set field values instead of open coding masks and shift operations. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-9-vkoul@kernel.org Signed-off-by: Vinod Koul --- 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 65d9b9bd2106..cabdadb09a1b 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -383,7 +383,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, * Name(_ADR, 0x40000000), with bits 31..28 representing the * SoundWire link so filter accordingly */ - if ((adr & GENMASK(31, 28)) >> 28 != SDW_LINK_TYPE) + if (FIELD_GET(GENMASK(31, 28), adr) != SDW_LINK_TYPE) return AE_OK; /* keep going */ /* device found, stop namespace walk */ -- cgit v1.2.3 From 8be2f84acf2317068c93fea291d823ac30d172d6 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 3 Sep 2020 17:15:04 +0530 Subject: soundwire: remove SDW_REG_SHIFT() soundwire had defined SDW_REG_SHIFT to calculate shift values for bitmasks, but now that we have better things in bitfield.h, remove this. Signed-off-by: Vinod Koul Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200903114504.1202143-10-vkoul@kernel.org Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_registers.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/include/linux/soundwire/sdw_registers.h b/include/linux/soundwire/sdw_registers.h index 5d3c271af7d1..f420e8059779 100644 --- a/include/linux/soundwire/sdw_registers.h +++ b/include/linux/soundwire/sdw_registers.h @@ -4,13 +4,6 @@ #ifndef __SDW_REGISTERS_H #define __SDW_REGISTERS_H -/* - * typically we define register and shifts but if one observes carefully, - * the shift can be generated from MASKS using few bit primitaives like ffs - * etc, so we use that and avoid defining shifts - */ -#define SDW_REG_SHIFT(n) (ffs(n) - 1) - /* * SDW registers as defined by MIPI 1.2 Spec */ -- cgit v1.2.3 From 3471d2a192bace106e4da7bcdcdd5ebcca4267d2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 4 Sep 2020 04:47:36 +0800 Subject: soundwire: stream: fix NULL/IS_ERR confusion snd_soc_dai_get_sdw_stream() can only return -ENOTSUPP or the stream, NULL is not a possible value. Fixes: 4550569bd779f ('soundwire: stream: add helper to startup/shutdown streams') Reported-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200903204739.31206-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index afb95febb0d3..b8b1973e3ee2 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1911,7 +1911,7 @@ void sdw_shutdown_stream(void *sdw_substream) sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream); - if (!sdw_stream) { + if (IS_ERR(sdw_stream)) { dev_err(rtd->dev, "no stream found for DAI %s", dai->name); return; } -- cgit v1.2.3 From 06dcb4e443643db93b286f861133785ca46f5a19 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 4 Sep 2020 04:47:37 +0800 Subject: soundwire: intel: fix NULL/ERR_PTR confusion snd_soc_dai_get_sdw_stream() can only return the pointer to stream or an ERR_PTR value, NULL is not a possible value. Fixes: 09553140c8d7b ('soundwire: intel: implement get_sdw_stream() operations') Reported-by: Bard Liao Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200903204739.31206-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 8a958b6cc0fe..669c8af13f55 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1126,7 +1126,7 @@ static void *intel_get_sdw_stream(struct snd_soc_dai *dai, dma = dai->capture_dma_data; if (!dma) - return NULL; + return ERR_PTR(-EINVAL); return dma->stream; } -- cgit v1.2.3 From e1c3a7f027546c47dc4e8d2422666eea84d675fb Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 4 Sep 2020 04:47:38 +0800 Subject: soundwire: intel: remove .trigger operation Now that the stream trigger is handled at the dai-link level, there is no need for a dai-level trigger any longer. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200903204739.31206-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 669c8af13f55..05de8c44449f 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -997,43 +997,6 @@ err: return ret; } -static int intel_trigger(struct snd_pcm_substream *substream, int cmd, - struct snd_soc_dai *dai) -{ - struct sdw_cdns_dma_data *dma; - int ret; - - dma = snd_soc_dai_get_dma_data(dai, substream); - if (!dma) { - dev_err(dai->dev, "failed to get dma data in %s", __func__); - return -EIO; - } - - switch (cmd) { - case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - case SNDRV_PCM_TRIGGER_RESUME: - ret = sdw_enable_stream(dma->stream); - break; - - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_STOP: - ret = sdw_disable_stream(dma->stream); - break; - - default: - ret = -EINVAL; - break; - } - - if (ret) - dev_err(dai->dev, - "%s trigger %d failed: %d", - __func__, cmd, ret); - return ret; -} - static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -1135,7 +1098,6 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, - .trigger = intel_trigger, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pcm_set_sdw_stream, @@ -1146,7 +1108,6 @@ static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .startup = intel_startup, .hw_params = intel_hw_params, .prepare = intel_prepare, - .trigger = intel_trigger, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pdm_set_sdw_stream, -- cgit v1.2.3 From 244eb888f9ab86854fc2c1d726cebfd7efbe780c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 4 Sep 2020 04:47:39 +0800 Subject: soundwire: intel: remove stream handling from .prepare and .hw_free Now that the stream is handled at the dai-link level (in the machine driver), we can remove the stream handling at the dai level. We still need these callbacks to perform dai-level resource handling (i.e. addition/removal of a master). Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200903204739.31206-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 05de8c44449f..e047910d73f5 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -951,7 +951,7 @@ static int intel_prepare(struct snd_pcm_substream *substream, 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) { @@ -987,13 +987,8 @@ static int intel_prepare(struct snd_pcm_substream *substream, dma->hw_params, sdw->instance, dma->pdi->intel_alh_id); - if (ret) - goto err; } - ret = sdw_prepare_stream(dma->stream); - -err: return ret; } @@ -1009,12 +1004,12 @@ 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; - } - + /* + * 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", -- cgit v1.2.3 From d1df23fe688b58e0577cf9a7df793a9559b53baa Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sat, 5 Sep 2020 13:39:02 -0400 Subject: soundwire: qcom: fix abh/ahb typo The function name qcom_swrm_abh_reg_read should say ahb, fix that. Signed-off-by: Jonathan Marek Tested-by: Srinivas Kandagatla Reviewed-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200905173905.16541-2-jonathan@marek.ca Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index dafa3f3dd1ab..d7e141b7bf78 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -111,7 +111,7 @@ struct qcom_swrm_ctrl { #define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus) -static int qcom_swrm_abh_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, +static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, u32 *val) { struct regmap *wcd_regmap = ctrl->regmap; @@ -746,7 +746,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) return -ENOMEM; if (dev->parent->bus == &slimbus_bus) { - ctrl->reg_read = qcom_swrm_abh_reg_read; + ctrl->reg_read = qcom_swrm_ahb_reg_read; ctrl->reg_write = qcom_swrm_ahb_reg_write; ctrl->regmap = dev_get_regmap(dev->parent, NULL); if (!ctrl->regmap) -- cgit v1.2.3 From 5bd773242f75da3b25750a2ccee88d97599aa757 Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sat, 5 Sep 2020 13:39:03 -0400 Subject: soundwire: qcom: avoid dependency on CONFIG_SLIMBUS The driver may be used without slimbus, so don't depend on slimbus. Signed-off-by: Jonathan Marek Tested-by: Srinivas Kandagatla Reviewed-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200905173905.16541-3-jonathan@marek.ca Signed-off-by: Vinod Koul --- drivers/soundwire/Kconfig | 2 +- drivers/soundwire/qcom.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index fa2b4ab92ed9..f83d02c9c60a 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -33,7 +33,7 @@ config SOUNDWIRE_INTEL config SOUNDWIRE_QCOM tristate "Qualcomm SoundWire Master driver" - depends on SLIMBUS + imply SLIMBUS depends on SND_SOC help SoundWire Qualcomm Master driver. diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index d7e141b7bf78..35517d1a7765 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -745,7 +745,11 @@ static int qcom_swrm_probe(struct platform_device *pdev) if (!ctrl) return -ENOMEM; +#if IS_ENABLED(CONFIG_SLIBMUS) if (dev->parent->bus == &slimbus_bus) { +#else + if (false) { +#endif ctrl->reg_read = qcom_swrm_ahb_reg_read; ctrl->reg_write = qcom_swrm_ahb_reg_write; ctrl->regmap = dev_get_regmap(dev->parent, NULL); -- cgit v1.2.3 From 82f5c70c26511ba9521418a94811eb4b8d96f0bd Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sat, 5 Sep 2020 13:39:04 -0400 Subject: soundwire: qcom: add support for mmio soundwire master devices Adds support for qcom soundwire devices with memory mapped IO registers. Signed-off-by: Jonathan Marek Tested-by: Srinivas Kandagatla Reviewed-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200905173905.16541-4-jonathan@marek.ca Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 35517d1a7765..4e66239c4417 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -34,6 +34,7 @@ #define SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED BIT(10) #define SWRM_INTERRUPT_MASK_ADDR 0x204 #define SWRM_INTERRUPT_CLEAR 0x208 +#define SWRM_INTERRUPT_CPU_EN 0x210 #define SWRM_CMD_FIFO_WR_CMD 0x300 #define SWRM_CMD_FIFO_RD_CMD 0x304 #define SWRM_CMD_FIFO_CMD 0x308 @@ -87,6 +88,7 @@ struct qcom_swrm_ctrl { struct sdw_bus bus; struct device *dev; struct regmap *regmap; + void __iomem *mmio; struct completion *comp; struct work_struct slave_work; /* read/write lock */ @@ -151,6 +153,20 @@ static int qcom_swrm_ahb_reg_write(struct qcom_swrm_ctrl *ctrl, return SDW_CMD_OK; } +static int qcom_swrm_cpu_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, + u32 *val) +{ + *val = readl(ctrl->mmio + reg); + return SDW_CMD_OK; +} + +static int qcom_swrm_cpu_reg_write(struct qcom_swrm_ctrl *ctrl, int reg, + int val) +{ + writel(val, ctrl->mmio + reg); + return SDW_CMD_OK; +} + static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data, u8 dev_addr, u16 reg_addr) { @@ -305,6 +321,12 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR, SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK | SWRM_COMP_CFG_ENABLE_MSK); + + /* enable CPU IRQs */ + if (ctrl->mmio) { + ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, + SWRM_INTERRUPT_STATUS_RMSK); + } return 0; } @@ -756,8 +778,11 @@ static int qcom_swrm_probe(struct platform_device *pdev) if (!ctrl->regmap) return -EINVAL; } else { - /* Only WCD based SoundWire controller is supported */ - return -ENOTSUPP; + ctrl->reg_read = qcom_swrm_cpu_reg_read; + ctrl->reg_write = qcom_swrm_cpu_reg_write; + ctrl->mmio = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(ctrl->mmio)) + return PTR_ERR(ctrl->mmio); } ctrl->irq = of_irq_get(dev->of_node, 0); -- cgit v1.2.3 From 8564551eec8afdc7aaf68853fbcae559426c9fe7 Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Sat, 5 Sep 2020 13:39:05 -0400 Subject: soundwire: qcom: add v1.5.1 compatible Add a compatible string for HW version v1.5.1 on sm8250 SoCs. Signed-off-by: Jonathan Marek Tested-by: Srinivas Kandagatla Reviewed-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200905173905.16541-5-jonathan@marek.ca Signed-off-by: Vinod Koul --- Documentation/devicetree/bindings/soundwire/qcom,sdw.txt | 1 + drivers/soundwire/qcom.c | 1 + 2 files changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt index 436547f3b155..b104be131235 100644 --- a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt +++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt @@ -11,6 +11,7 @@ board specific bus parameters. Example: "qcom,soundwire-v1.3.0" "qcom,soundwire-v1.5.0" + "qcom,soundwire-v1.5.1" "qcom,soundwire-v1.6.0" - reg: Usage: required diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 4e66239c4417..77bc58b4cca2 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -880,6 +880,7 @@ static int qcom_swrm_remove(struct platform_device *pdev) static const struct of_device_id qcom_swrm_of_match[] = { { .compatible = "qcom,soundwire-v1.3.0", }, + { .compatible = "qcom,soundwire-v1.5.1", }, {/* sentinel */}, }; -- cgit v1.2.3 From d2068da5c85697b5880483dd7beaba98e0b62e02 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 18 Aug 2020 06:23:40 +0800 Subject: soundwire: cadence: fix race condition between suspend and Slave device alerts 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 Acked-by: Jaroslav Kysela Link: https://lore.kernel.org/r/20200817222340.18042-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 ecf503fb23e1..8c354f946879 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -790,7 +790,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); @@ -923,6 +932,19 @@ update_masks: 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; -- cgit v1.2.3 From 09309093d5e8f8774e4a3a0d42b73cf47e9421cf Mon Sep 17 00:00:00 2001 From: Jonathan Marek Date: Tue, 8 Sep 2020 10:08:17 -0400 Subject: soundwire: qcom: fix SLIBMUS/SLIMBUS typo Fix slimbus case being broken thanks to a typo. Fixes: 5bd773242f75 ("soundwire: qcom: avoid dependency on CONFIG_SLIMBUS") Signed-off-by: Jonathan Marek Reviewed-by: Bjorn Andersson Link: https://lore.kernel.org/r/20200908140818.28373-1-jonathan@marek.ca Signed-off-by: Vinod Koul --- 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 77bc58b4cca2..d7aabdaffee3 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -767,7 +767,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) if (!ctrl) return -ENOMEM; -#if IS_ENABLED(CONFIG_SLIBMUS) +#if IS_ENABLED(CONFIG_SLIMBUS) if (dev->parent->bus == &slimbus_bus) { #else if (false) { -- cgit v1.2.3 From 2acd30b9f6032c6cbefc5e255c17ebbb0718e56a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:15 +0800 Subject: ASoC/soundwire: bus: use property to set interrupt masks 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. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao Tested-by: Srinivas Kandagatla Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Acked-by: Mark Brown Link: https://lore.kernel.org/r/20200908134521.6781-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 d808f0256ba0..9f4cc24ccea3 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 1e9010c139f0..9d94cdf6346f 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -357,6 +357,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; @@ -378,6 +379,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 a3ec92775ea7..76ddde509a08 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: 00001000 Dataport 3 is active */ prop->source_ports = BIT(3); /* BITMAP: 00000010 Dataport 1 is active */ diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c index 09c69dbab12a..e02b325240df 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 b7c97aba7f17..8d4ea46bc6b5 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 b19fbcc12c69..8d9678d1f3c7 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) 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/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index dc4a2b482462..d4b3d7716cac 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -342,6 +343,9 @@ static int rt711_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/rt715-sdw.c b/sound/soc/codecs/rt715-sdw.c index d8ed07305ffc..0e5c75f85926 100644 --- a/sound/soc/codecs/rt715-sdw.c +++ b/sound/soc/codecs/rt715-sdw.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -436,6 +437,9 @@ static int rt715_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/wsa881x.c b/sound/soc/codecs/wsa881x.c index d39d479e2378..68e774e69c85 100644 --- a/sound/soc/codecs/wsa881x.c +++ b/sound/soc/codecs/wsa881x.c @@ -1112,6 +1112,7 @@ static int wsa881x_probe(struct sdw_slave *pdev, wsa881x->sconfig.type = SDW_STREAM_PDM; pdev->prop.sink_ports = GENMASK(WSA881X_MAX_SWR_PORTS, 0); pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop; + pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; gpiod_direction_output(wsa881x->sd_n, 1); wsa881x->regmap = devm_regmap_init_sdw(pdev, &wsa881x_regmap_config); -- cgit v1.2.3 From 310f6dc6dc5d7372e878e3e401ae087b63d545de Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:16 +0800 Subject: soundwire: bus: filter-out unwanted interrupt reports 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200908134521.6781-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 9f4cc24ccea3..029818b1f568 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 */ -- cgit v1.2.3 From c2819e196b3cc1901a4612f72e66da4821966a5e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:17 +0800 Subject: soundwire: slave: add first_interrupt_done status 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200908134521.6781-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 029818b1f568..30b0bed16630 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 2191dd6e7aa4..4a250d33de5d 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -58,6 +58,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; for (i = 0; i < SDW_MAX_PORTS; i++) init_completion(&slave->port_ready[i]); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 9d94cdf6346f..2b93a8ef7fad 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -616,6 +616,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; @@ -637,6 +639,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) -- cgit v1.2.3 From 4724f12c1315efa79a0cbf74dfb0c9b98b1a4bff Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:18 +0800 Subject: soundwire: bus: use quirk to filter out invalid parity errors 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200908134521.6781-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 30b0bed16630..09185e5cfd70 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 2b93a8ef7fad..790823d2d33b 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -358,6 +358,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; @@ -380,8 +381,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_PARITY BIT(0) + /** * struct sdw_master_prop - Master properties * @revision: MIPI spec version of the implementation -- cgit v1.2.3 From 38edbfae6c7f4ad402dd0464c1887eaf068468b8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:19 +0800 Subject: ASoC: codecs: realtek-soundwire: ignore initial PARITY errors 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Acked-by: Mark Brown Link: https://lore.kernel.org/r/20200908134521.6781-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 e02b325240df..c74685b016af 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 8d4ea46bc6b5..feb2db95829e 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 8d9678d1f3c7..c6fd22058b62 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 d4b3d7716cac..10435d97f9ab 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 0e5c75f85926..9fb14a0d7334 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; -- cgit v1.2.3 From a350aff45b4d33355cdc59b7b76950e39639e32f Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:20 +0800 Subject: soundwire: bus: export broadcast read/write capability for tests 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200908134521.6781-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 09185e5cfd70..02574b4bb179 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 -- cgit v1.2.3 From 32d2a8935bf8faf201ff1a859eeb43ef6e5e438d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Sep 2020 21:45:21 +0800 Subject: soundwire: cadence: add parity error injection through debugfs 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 Signed-off-by: Bard Liao Reviewed-by: Kai Vehmanen Reviewed-by: Guennadi Liakhovetski Link: https://lore.kernel.org/r/20200908134521.6781-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- 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 8c354f946879..f1efe5beead6 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 @@ -366,6 +370,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 @@ -377,6 +460,9 @@ void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root) debugfs_create_file("cdns-hw-reset", 0200, root, cdns, &cdns_hw_reset_fops); + + debugfs_create_file("cdns-parity-error-injection", 0200, root, cdns, + &cdns_parity_error_fops); } EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init); -- cgit v1.2.3 From 9026118f20e28f202dab34f219bbb831ffb8c4dc Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 8 Sep 2020 21:15:20 +0800 Subject: soundwire: Add generic bandwidth allocation algorithm This algorithm computes bus parameters like clock frequency, frame shape and port transport parameters based on active stream(s) running on the bus. Developers can also implement their own .compute_params() callback for specific resource management algorithm, and set if before calling sdw_add_bus_master() Credits: this patch is based on an earlier internal contribution by Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah. All hard-coded values were removed from the initial contribution to use BIOS information instead. Signed-off-by: Bard Liao Acked-by: Jaroslav Kysela Link: https://lore.kernel.org/r/20200908131520.5712-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/Kconfig | 5 + drivers/soundwire/Makefile | 3 + drivers/soundwire/bus.c | 6 + drivers/soundwire/bus.h | 46 ++- drivers/soundwire/generic_bandwidth_allocation.c | 427 +++++++++++++++++++++++ drivers/soundwire/intel.c | 3 + drivers/soundwire/stream.c | 12 + include/linux/soundwire/sdw.h | 3 + 8 files changed, 503 insertions(+), 2 deletions(-) create mode 100644 drivers/soundwire/generic_bandwidth_allocation.c diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index f83d02c9c60a..016e74230bb7 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -24,6 +24,7 @@ config SOUNDWIRE_CADENCE config SOUNDWIRE_INTEL tristate "Intel SoundWire Master driver" select SOUNDWIRE_CADENCE + select SOUNDWIRE_GENERIC_ALLOCATION depends on ACPI && SND_SOC help SoundWire Intel Master driver. @@ -40,4 +41,8 @@ config SOUNDWIRE_QCOM If you have an Qualcomm platform which has a SoundWire Master then enable this config option to get the SoundWire support for that device + +config SOUNDWIRE_GENERIC_ALLOCATION + tristate + endif diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 7c53ffae9f50..bf1e250d50dd 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -8,6 +8,9 @@ soundwire-bus-y := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o \ sysfs_slave.o sysfs_slave_dpn.o obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o +soundwire-generic-allocation-objs := generic_bandwidth_allocation.o +obj-$(CONFIG_SOUNDWIRE_GENERIC_ALLOCATION) += soundwire-generic-allocation.o + ifdef CONFIG_DEBUG_FS soundwire-bus-y += debugfs.o endif diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 02574b4bb179..340fd1e23e5c 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -61,6 +61,12 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, return -EINVAL; } + if (!bus->compute_params) { + dev_err(bus->dev, + "Bandwidth allocation not configured, compute_params no set\n"); + return -EINVAL; + } + mutex_init(&bus->msg_lock); mutex_init(&bus->bus_lock); INIT_LIST_HEAD(&bus->slaves); diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index c53345fbc4c7..e0703991aad9 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -69,6 +69,7 @@ struct sdw_msg { }; #define SDW_DOUBLE_RATE_FACTOR 2 +#define SDW_STRM_RATE_GROUPING 1 extern int sdw_rows[SDW_FRAME_ROWS]; extern int sdw_cols[SDW_FRAME_COLS]; @@ -154,9 +155,50 @@ int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg, int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave, u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf); +/* Retrieve and return channel count from channel mask */ +static inline int sdw_ch_mask_to_ch(int ch_mask) +{ + int c = 0; + + for (c = 0; ch_mask; ch_mask >>= 1) + c += ch_mask & 1; + + return c; +} + +/* Fill transport parameter data structure */ +static inline void sdw_fill_xport_params(struct sdw_transport_params *params, + int port_num, bool grp_ctrl_valid, + int grp_ctrl, int sample_int, + int off1, int off2, + int hstart, int hstop, + int pack_mode, int lane_ctrl) +{ + params->port_num = port_num; + params->blk_grp_ctrl_valid = grp_ctrl_valid; + params->blk_grp_ctrl = grp_ctrl; + params->sample_interval = sample_int; + params->offset1 = off1; + params->offset2 = off2; + params->hstart = hstart; + params->hstop = hstop; + params->blk_pkg_mode = pack_mode; + params->lane_ctrl = lane_ctrl; +} + +/* Fill port parameter data structure */ +static inline void sdw_fill_port_params(struct sdw_port_params *params, + int port_num, int bps, + int flow_mode, int data_mode) +{ + params->num = port_num; + params->bps = bps; + params->flow_mode = flow_mode; + params->data_mode = data_mode; +} + /* Read-Modify-Write Slave register */ -static inline int -sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) +static inline int sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) { int tmp; diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c new file mode 100644 index 000000000000..6088775b67a5 --- /dev/null +++ b/drivers/soundwire/generic_bandwidth_allocation.c @@ -0,0 +1,427 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) +// Copyright(c) 2015-2020 Intel Corporation. + +/* + * Bandwidth management algorithm based on 2^n gears + * + */ + +#include +#include +#include +#include +#include +#include "bus.h" + +#define SDW_STRM_RATE_GROUPING 1 + +struct sdw_group_params { + unsigned int rate; + int full_bw; + int payload_bw; + int hwidth; +}; + +struct sdw_group { + unsigned int count; + unsigned int max_size; + unsigned int *rates; +}; + +struct sdw_transport_data { + int hstart; + int hstop; + int block_offset; + int sub_block_offset; +}; + +static void sdw_compute_slave_ports(struct sdw_master_runtime *m_rt, + struct sdw_transport_data *t_data) +{ + struct sdw_slave_runtime *s_rt = NULL; + struct sdw_port_runtime *p_rt; + int port_bo, sample_int; + unsigned int rate, bps, ch = 0; + unsigned int slave_total_ch; + + port_bo = t_data->block_offset; + + list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { + rate = m_rt->stream->params.rate; + bps = m_rt->stream->params.bps; + sample_int = (m_rt->bus->params.curr_dr_freq / rate); + slave_total_ch = 0; + + list_for_each_entry(p_rt, &s_rt->port_list, port_node) { + ch = sdw_ch_mask_to_ch(p_rt->ch_mask); + + sdw_fill_xport_params(&p_rt->transport_params, + p_rt->num, false, + SDW_BLK_GRP_CNT_1, + sample_int, port_bo, port_bo >> 8, + t_data->hstart, + t_data->hstop, + (SDW_BLK_GRP_CNT_1 * ch), 0x0); + + sdw_fill_port_params(&p_rt->port_params, + p_rt->num, bps, + SDW_PORT_FLOW_MODE_ISOCH, + SDW_PORT_DATA_MODE_NORMAL); + + port_bo += bps * ch; + slave_total_ch += ch; + } + + if (m_rt->direction == SDW_DATA_DIR_TX && + m_rt->ch_count == slave_total_ch) { + /* + * Slave devices were configured to access all channels + * of the stream, which indicates that they operate in + * 'mirror mode'. Make sure we reset the port offset for + * the next device in the list + */ + port_bo = t_data->block_offset; + } + } +} + +static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt, + struct sdw_group_params *params, + int port_bo, int hstop) +{ + struct sdw_transport_data t_data = {0}; + struct sdw_port_runtime *p_rt; + struct sdw_bus *bus = m_rt->bus; + int sample_int, hstart = 0; + unsigned int rate, bps, ch, no_ch; + + rate = m_rt->stream->params.rate; + bps = m_rt->stream->params.bps; + ch = m_rt->ch_count; + sample_int = (bus->params.curr_dr_freq / rate); + + if (rate != params->rate) + return; + + t_data.hstop = hstop; + hstart = hstop - params->hwidth + 1; + t_data.hstart = hstart; + + list_for_each_entry(p_rt, &m_rt->port_list, port_node) { + no_ch = sdw_ch_mask_to_ch(p_rt->ch_mask); + + sdw_fill_xport_params(&p_rt->transport_params, p_rt->num, + false, SDW_BLK_GRP_CNT_1, sample_int, + port_bo, port_bo >> 8, hstart, hstop, + (SDW_BLK_GRP_CNT_1 * no_ch), 0x0); + + sdw_fill_port_params(&p_rt->port_params, + p_rt->num, bps, + SDW_PORT_FLOW_MODE_ISOCH, + SDW_PORT_DATA_MODE_NORMAL); + + /* Check for first entry */ + if (!(p_rt == list_first_entry(&m_rt->port_list, + struct sdw_port_runtime, + port_node))) { + port_bo += bps * ch; + continue; + } + + t_data.hstart = hstart; + t_data.hstop = hstop; + t_data.block_offset = port_bo; + t_data.sub_block_offset = 0; + port_bo += bps * ch; + } + + sdw_compute_slave_ports(m_rt, &t_data); +} + +static void _sdw_compute_port_params(struct sdw_bus *bus, + struct sdw_group_params *params, int count) +{ + struct sdw_master_runtime *m_rt = NULL; + int hstop = bus->params.col - 1; + int block_offset, port_bo, i; + + /* Run loop for all groups to compute transport parameters */ + for (i = 0; i < count; i++) { + port_bo = 1; + block_offset = 1; + + list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { + sdw_compute_master_ports(m_rt, ¶ms[i], + port_bo, hstop); + + block_offset += m_rt->ch_count * + m_rt->stream->params.bps; + port_bo = block_offset; + } + + hstop = hstop - params[i].hwidth; + } +} + +static int sdw_compute_group_params(struct sdw_bus *bus, + struct sdw_group_params *params, + int *rates, int count) +{ + struct sdw_master_runtime *m_rt = NULL; + int sel_col = bus->params.col; + unsigned int rate, bps, ch; + int i, column_needed = 0; + + /* Calculate bandwidth per group */ + for (i = 0; i < count; i++) { + params[i].rate = rates[i]; + params[i].full_bw = bus->params.curr_dr_freq / params[i].rate; + } + + list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { + rate = m_rt->stream->params.rate; + bps = m_rt->stream->params.bps; + ch = m_rt->ch_count; + + for (i = 0; i < count; i++) { + if (rate == params[i].rate) + params[i].payload_bw += bps * ch; + } + } + + for (i = 0; i < count; i++) { + params[i].hwidth = (sel_col * + params[i].payload_bw + params[i].full_bw - 1) / + params[i].full_bw; + + column_needed += params[i].hwidth; + } + + if (column_needed > sel_col - 1) + return -EINVAL; + + return 0; +} + +static int sdw_add_element_group_count(struct sdw_group *group, + unsigned int rate) +{ + int num = group->count; + int i; + + for (i = 0; i <= num; i++) { + if (rate == group->rates[i]) + break; + + if (i != num) + continue; + + if (group->count >= group->max_size) { + unsigned int *rates; + + group->max_size += 1; + rates = krealloc(group->rates, + (sizeof(int) * group->max_size), + GFP_KERNEL); + if (!rates) + return -ENOMEM; + group->rates = rates; + } + + group->rates[group->count++] = rate; + } + + return 0; +} + +static int sdw_get_group_count(struct sdw_bus *bus, + struct sdw_group *group) +{ + struct sdw_master_runtime *m_rt; + unsigned int rate; + int ret = 0; + + group->count = 0; + group->max_size = SDW_STRM_RATE_GROUPING; + group->rates = kcalloc(group->max_size, sizeof(int), GFP_KERNEL); + if (!group->rates) + return -ENOMEM; + + list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { + rate = m_rt->stream->params.rate; + if (m_rt == list_first_entry(&bus->m_rt_list, + struct sdw_master_runtime, + bus_node)) { + group->rates[group->count++] = rate; + + } else { + ret = sdw_add_element_group_count(group, rate); + if (ret < 0) { + kfree(group->rates); + return ret; + } + } + } + + return ret; +} + +/** + * sdw_compute_port_params: Compute transport and port parameters + * + * @bus: SDW Bus instance + */ +static int sdw_compute_port_params(struct sdw_bus *bus) +{ + struct sdw_group_params *params = NULL; + struct sdw_group group; + int ret; + + ret = sdw_get_group_count(bus, &group); + if (ret < 0) + return ret; + + if (group.count == 0) + goto out; + + params = kcalloc(group.count, sizeof(*params), GFP_KERNEL); + if (!params) { + ret = -ENOMEM; + goto out; + } + + /* Compute transport parameters for grouped streams */ + ret = sdw_compute_group_params(bus, params, + &group.rates[0], group.count); + if (ret < 0) + goto free_params; + + _sdw_compute_port_params(bus, params, group.count); + +free_params: + kfree(params); +out: + kfree(group.rates); + + return ret; +} + +static int sdw_select_row_col(struct sdw_bus *bus, int clk_freq) +{ + struct sdw_master_prop *prop = &bus->prop; + int frame_int, frame_freq; + int r, c; + + for (c = 0; c < SDW_FRAME_COLS; c++) { + for (r = 0; r < SDW_FRAME_ROWS; r++) { + if (sdw_rows[r] != prop->default_row || + sdw_cols[c] != prop->default_col) + continue; + + frame_int = sdw_rows[r] * sdw_cols[c]; + frame_freq = clk_freq / frame_int; + + if ((clk_freq - (frame_freq * SDW_FRAME_CTRL_BITS)) < + bus->params.bandwidth) + continue; + + bus->params.row = sdw_rows[r]; + bus->params.col = sdw_cols[c]; + return 0; + } + } + + return -EINVAL; +} + +/** + * sdw_compute_bus_params: Compute bus parameters + * + * @bus: SDW Bus instance + */ +static int sdw_compute_bus_params(struct sdw_bus *bus) +{ + unsigned int max_dr_freq, curr_dr_freq = 0; + struct sdw_master_prop *mstr_prop = NULL; + int i, clk_values, ret; + bool is_gear = false; + u32 *clk_buf; + + mstr_prop = &bus->prop; + if (!mstr_prop) + return -EINVAL; + + if (mstr_prop->num_clk_gears) { + clk_values = mstr_prop->num_clk_gears; + clk_buf = mstr_prop->clk_gears; + is_gear = true; + } else if (mstr_prop->num_clk_freq) { + clk_values = mstr_prop->num_clk_freq; + clk_buf = mstr_prop->clk_freq; + } else { + clk_values = 1; + clk_buf = NULL; + } + + max_dr_freq = mstr_prop->max_clk_freq * SDW_DOUBLE_RATE_FACTOR; + + for (i = 0; i < clk_values; i++) { + if (!clk_buf) + curr_dr_freq = max_dr_freq; + else + curr_dr_freq = (is_gear) ? + (max_dr_freq >> clk_buf[i]) : + clk_buf[i] * SDW_DOUBLE_RATE_FACTOR; + + if (curr_dr_freq <= bus->params.bandwidth) + continue; + + break; + + /* + * TODO: Check all the Slave(s) port(s) audio modes and find + * whether given clock rate is supported with glitchless + * transition. + */ + } + + if (i == clk_values) + return -EINVAL; + + ret = sdw_select_row_col(bus, curr_dr_freq); + if (ret < 0) + return -EINVAL; + + bus->params.curr_dr_freq = curr_dr_freq; + return 0; +} + +/** + * sdw_compute_params: Compute bus, transport and port parameters + * + * @bus: SDW Bus instance + */ +int sdw_compute_params(struct sdw_bus *bus) +{ + int ret; + + /* 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); + 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); + return ret; + } + + return 0; +} +EXPORT_SYMBOL(sdw_compute_params); + +MODULE_LICENSE("Dual BSD/GPL"); +MODULE_DESCRIPTION("SoundWire Generic Bandwidth Allocation"); diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e047910d73f5..1211d114ff59 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1318,6 +1318,9 @@ static int intel_master_probe(struct platform_device *pdev) /* set driver data, accessed by snd_soc_dai_get_drvdata() */ dev_set_drvdata(dev, cdns); + /* use generic bandwidth allocation algorithm */ + sdw->cdns.bus.compute_params = sdw_compute_params; + ret = sdw_bus_master_add(bus, dev, dev->fwnode); if (ret) { dev_err(dev, "sdw_bus_master_add fail: %d\n", ret); diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index b8b1973e3ee2..f3219d1fa63b 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -25,8 +25,10 @@ int sdw_rows[SDW_FRAME_ROWS] = {48, 50, 60, 64, 75, 80, 125, 147, 96, 100, 120, 128, 150, 160, 250, 0, 192, 200, 240, 256, 72, 144, 90, 180}; +EXPORT_SYMBOL(sdw_rows); int sdw_cols[SDW_FRAME_COLS] = {2, 4, 6, 8, 10, 12, 14, 16}; +EXPORT_SYMBOL(sdw_cols); int sdw_find_col_index(int col) { @@ -1782,6 +1784,16 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream) bus->params.bandwidth -= m_rt->stream->params.rate * m_rt->ch_count * m_rt->stream->params.bps; + /* Compute params */ + if (bus->compute_params) { + ret = bus->compute_params(bus); + if (ret < 0) { + dev_err(bus->dev, "Compute params failed: %d", + ret); + return ret; + } + } + /* Program params */ ret = sdw_program_params(bus, false); if (ret < 0) { diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 790823d2d33b..de9ea2ce2d35 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -964,6 +964,9 @@ struct sdw_stream_runtime { struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name); void sdw_release_stream(struct sdw_stream_runtime *stream); + +int sdw_compute_params(struct sdw_bus *bus); + int sdw_stream_add_master(struct sdw_bus *bus, struct sdw_stream_config *stream_config, struct sdw_port_config *port_config, -- cgit v1.2.3 From 578ddced239f554baf6ba935a53077f6a80cb584 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Thu, 17 Sep 2020 13:01:36 +0100 Subject: soundwire: qcom: clear BIT FIELDs before value set. According to usage (bitfields.h) of REG_FIELDS, Modify is: reg &= ~REG_FIELD_C; reg |= FIELD_PREP(REG_FIELD_C, c); Patch ("soundwire: qcom : use FIELD_{GET|PREP}") seems to have accidentally removed clearing bit field while modifying the register. Fix this by using u32p_replace_bits() to clear and set the values. Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200917120138.11313-2-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index d7aabdaffee3..c1bb35884182 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -311,7 +311,7 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) /* Configure No pings */ ctrl->reg_read(ctrl, SWRM_MCP_CFG_ADDR, &val); - val |= FIELD_PREP(SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK, SWRM_DEF_CMD_NO_PINGS); + u32p_replace_bits(&val, SWRM_DEF_CMD_NO_PINGS, SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK); ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val); /* Configure number of retries of a read/write cmd */ @@ -372,8 +372,8 @@ static int qcom_swrm_pre_bank_switch(struct sdw_bus *bus) ctrl->reg_read(ctrl, reg, &val); - val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK, SWRM_MAX_COL_VAL); - val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK, SWRM_MAX_ROW_VAL); + u32p_replace_bits(&val, SWRM_MAX_COL_VAL, SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK); + u32p_replace_bits(&val, SWRM_MAX_ROW_VAL, SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK); return ctrl->reg_write(ctrl, reg, val); } -- cgit v1.2.3 From 5ffba1fb6d555556c554d373643b0eafbd3e6a4f Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Thu, 17 Sep 2020 13:01:37 +0100 Subject: soundwire: qcom: add support to block packing mode This patch adds support to block pack mode, which is required on Qcom soundwire controllers v1.5.x on few ports! Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200917120138.11313-3-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index c1bb35884182..16023b5bcbd5 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -54,6 +54,7 @@ #define SWRM_MCP_SLV_STATUS 0x1090 #define SWRM_MCP_SLV_STATUS_MASK GENMASK(1, 0) #define SWRM_DP_PORT_CTRL_BANK(n, m) (0x1124 + 0x100 * (n - 1) + 0x40 * m) +#define SWRM_DP_BLOCK_CTRL3_BANK(n, m) (0x1138 + 0x100 * (n - 1) + 0x40 * m) #define SWRM_DP_PORT_CTRL_EN_CHAN_SHFT 0x18 #define SWRM_DP_PORT_CTRL_OFFSET2_SHFT 0x10 #define SWRM_DP_PORT_CTRL_OFFSET1_SHFT 0x08 @@ -82,6 +83,7 @@ struct qcom_swrm_port_config { u8 si; u8 off1; u8 off2; + u8 bp_mode; }; struct qcom_swrm_ctrl { @@ -392,14 +394,22 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus, { struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); u32 value; + int reg = SWRM_DP_PORT_CTRL_BANK((params->port_num), bank); + int ret; value = params->offset1 << SWRM_DP_PORT_CTRL_OFFSET1_SHFT; value |= params->offset2 << SWRM_DP_PORT_CTRL_OFFSET2_SHFT; value |= params->sample_interval - 1; - return ctrl->reg_write(ctrl, - SWRM_DP_PORT_CTRL_BANK((params->port_num), bank), - value); + ret = ctrl->reg_write(ctrl, reg, value); + + if (!ret && params->blk_pkg_mode) { + reg = SWRM_DP_BLOCK_CTRL3_BANK(params->port_num, bank); + + ret = ctrl->reg_write(ctrl, reg, 1); + } + + return ret; } static int qcom_swrm_port_enable(struct sdw_bus *bus, @@ -447,6 +457,7 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus) p_rt->transport_params.sample_interval = pcfg->si + 1; p_rt->transport_params.offset1 = pcfg->off1; p_rt->transport_params.offset2 = pcfg->off2; + p_rt->transport_params.blk_pkg_mode = pcfg->bp_mode; } list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { @@ -457,6 +468,7 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus) pcfg->si + 1; p_rt->transport_params.offset1 = pcfg->off1; p_rt->transport_params.offset2 = pcfg->off2; + p_rt->transport_params.blk_pkg_mode = pcfg->bp_mode; i++; } } @@ -703,6 +715,7 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl) u8 off1[QCOM_SDW_MAX_PORTS]; u8 off2[QCOM_SDW_MAX_PORTS]; u8 si[QCOM_SDW_MAX_PORTS]; + u8 bp_mode[QCOM_SDW_MAX_PORTS] = { 0, }; int i, ret, nports, val; ctrl->reg_read(ctrl, SWRM_COMP_PARAMS, &val); @@ -745,10 +758,13 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl) if (ret) return ret; + ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode", + bp_mode, nports); for (i = 0; i < nports; i++) { ctrl->pconfig[i].si = si[i]; ctrl->pconfig[i].off1 = off1[i]; ctrl->pconfig[i].off2 = off2[i]; + ctrl->pconfig[i].bp_mode = bp_mode[i]; } return 0; -- cgit v1.2.3 From 8cb3b4e74cd810b86e18bcf832c2a4a877737981 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Thu, 17 Sep 2020 13:01:38 +0100 Subject: soundwire: qcom: get max rows and cols info from compatible currently the max rows and cols values are hardcoded. In reality these values depend on the IP version. So get these based on device tree compatible strings. Signed-off-by: Srinivas Kandagatla Link: https://lore.kernel.org/r/20200917120138.11313-4-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 46 +++++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 16023b5bcbd5..fbca4ebf63e9 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -66,11 +66,6 @@ #define SWRM_REG_VAL_PACK(data, dev, id, reg) \ ((reg) | ((id) << 16) | ((dev) << 20) | ((data) << 24)) -#define SWRM_MAX_ROW_VAL 0 /* Rows = 48 */ -#define SWRM_DEFAULT_ROWS 48 -#define SWRM_MIN_COL_VAL 0 /* Cols = 2 */ -#define SWRM_DEFAULT_COL 16 -#define SWRM_MAX_COL_VAL 7 #define SWRM_SPECIAL_CMD_ID 0xF #define MAX_FREQ_NUM 1 #define TIMEOUT_MS (2 * HZ) @@ -104,6 +99,8 @@ struct qcom_swrm_ctrl { unsigned int version; int num_din_ports; int num_dout_ports; + int cols_index; + int rows_index; unsigned long dout_port_mask; unsigned long din_port_mask; struct qcom_swrm_port_config pconfig[QCOM_SDW_MAX_PORTS]; @@ -113,6 +110,21 @@ struct qcom_swrm_ctrl { int (*reg_write)(struct qcom_swrm_ctrl *ctrl, int reg, int val); }; +struct qcom_swrm_data { + u32 default_cols; + u32 default_rows; +}; + +static struct qcom_swrm_data swrm_v1_3_data = { + .default_rows = 48, + .default_cols = 16, +}; + +static struct qcom_swrm_data swrm_v1_5_data = { + .default_rows = 50, + .default_cols = 16, +}; + #define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus) static int qcom_swrm_ahb_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, @@ -299,8 +311,8 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) u32 val; /* Clear Rows and Cols */ - val = FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK, SWRM_MAX_ROW_VAL); - val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK, SWRM_MIN_COL_VAL); + val = FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK, ctrl->rows_index); + val |= FIELD_PREP(SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK, ctrl->cols_index); ctrl->reg_write(ctrl, SWRM_MCP_FRAME_CTRL_BANK_ADDR(0), val); @@ -374,8 +386,8 @@ static int qcom_swrm_pre_bank_switch(struct sdw_bus *bus) ctrl->reg_read(ctrl, reg, &val); - u32p_replace_bits(&val, SWRM_MAX_COL_VAL, SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK); - u32p_replace_bits(&val, SWRM_MAX_ROW_VAL, SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK); + u32p_replace_bits(&val, ctrl->cols_index, SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK); + u32p_replace_bits(&val, ctrl->rows_index, SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK); return ctrl->reg_write(ctrl, reg, val); } @@ -776,6 +788,7 @@ static int qcom_swrm_probe(struct platform_device *pdev) struct sdw_master_prop *prop; struct sdw_bus_params *params; struct qcom_swrm_ctrl *ctrl; + const struct qcom_swrm_data *data; int ret; u32 val; @@ -783,6 +796,9 @@ static int qcom_swrm_probe(struct platform_device *pdev) if (!ctrl) return -ENOMEM; + data = of_device_get_match_data(dev); + ctrl->rows_index = sdw_find_row_index(data->default_rows); + ctrl->cols_index = sdw_find_col_index(data->default_cols); #if IS_ENABLED(CONFIG_SLIMBUS) if (dev->parent->bus == &slimbus_bus) { #else @@ -832,8 +848,8 @@ static int qcom_swrm_probe(struct platform_device *pdev) params = &ctrl->bus.params; params->max_dr_freq = DEFAULT_CLK_FREQ; params->curr_dr_freq = DEFAULT_CLK_FREQ; - params->col = SWRM_DEFAULT_COL; - params->row = SWRM_DEFAULT_ROWS; + params->col = data->default_cols; + params->row = data->default_rows; ctrl->reg_read(ctrl, SWRM_MCP_STATUS, &val); params->curr_bank = val & SWRM_MCP_STATUS_BANK_NUM_MASK; params->next_bank = !params->curr_bank; @@ -843,8 +859,8 @@ static int qcom_swrm_probe(struct platform_device *pdev) prop->num_clk_gears = 0; prop->num_clk_freq = MAX_FREQ_NUM; prop->clk_freq = &qcom_swrm_freq_tbl[0]; - prop->default_col = SWRM_DEFAULT_COL; - prop->default_row = SWRM_DEFAULT_ROWS; + prop->default_col = data->default_cols; + prop->default_row = data->default_rows; ctrl->reg_read(ctrl, SWRM_COMP_HW_VERSION, &ctrl->version); @@ -895,8 +911,8 @@ static int qcom_swrm_remove(struct platform_device *pdev) } static const struct of_device_id qcom_swrm_of_match[] = { - { .compatible = "qcom,soundwire-v1.3.0", }, - { .compatible = "qcom,soundwire-v1.5.1", }, + { .compatible = "qcom,soundwire-v1.3.0", .data = &swrm_v1_3_data }, + { .compatible = "qcom,soundwire-v1.5.1", .data = &swrm_v1_5_data }, {/* sentinel */}, }; -- cgit v1.2.3 From 714db045cf30f7897dded6417cf70f9426c6b87b Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 17 Sep 2020 17:31:45 +0530 Subject: soundwire: cadence: use u32p_replace_bits FIELD_PREP() does not replace the bits so it is not apt in case where we modify a register. Use u32p_replace_bits() instead. Fixes: 3cf25d63b1b9 ("soundwire: cadence: use FIELD_{GET|PREP}") Tested-by: Bard Liao Link: https://lore.kernel.org/r/20200917120146.1780323-2-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index f1efe5beead6..19d445ef6764 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -1278,9 +1278,9 @@ static int cdns_port_params(struct sdw_bus *bus, dpn_config = cdns_readl(cdns, dpn_config_off); - dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_WL, (p_params->bps - 1)); - dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_FLOW, p_params->flow_mode); - dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_DAT, p_params->data_mode); + u32p_replace_bits(&dpn_config, (p_params->bps - 1), CDNS_DPN_CONFIG_WL); + u32p_replace_bits(&dpn_config, p_params->flow_mode, CDNS_DPN_CONFIG_PORT_FLOW); + u32p_replace_bits(&dpn_config, p_params->data_mode, CDNS_DPN_CONFIG_PORT_DAT); cdns_writel(cdns, dpn_config_off, dpn_config); @@ -1316,18 +1316,17 @@ static int cdns_transport_params(struct sdw_bus *bus, } dpn_config = cdns_readl(cdns, dpn_config_off); - - dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BGC, t_params->blk_grp_ctrl); - dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BPM, t_params->blk_pkg_mode); + u32p_replace_bits(&dpn_config, t_params->blk_grp_ctrl, CDNS_DPN_CONFIG_BGC); + u32p_replace_bits(&dpn_config, t_params->blk_pkg_mode, CDNS_DPN_CONFIG_BPM); cdns_writel(cdns, dpn_config_off, dpn_config); - dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_1, t_params->offset1); - dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_2, t_params->offset2); + u32p_replace_bits(&dpn_offsetctrl, t_params->offset1, CDNS_DPN_OFFSET_CTRL_1); + u32p_replace_bits(&dpn_offsetctrl, t_params->offset2, CDNS_DPN_OFFSET_CTRL_2); cdns_writel(cdns, dpn_offsetctrl_off, dpn_offsetctrl); - dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTART, t_params->hstart); - dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTOP, t_params->hstop); - dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_LCTRL, t_params->lane_ctrl); + u32p_replace_bits(&dpn_hctrl, t_params->hstart, CDNS_DPN_HCTRL_HSTART); + u32p_replace_bits(&dpn_hctrl, t_params->hstop, CDNS_DPN_HCTRL_HSTOP); + u32p_replace_bits(&dpn_hctrl, t_params->lane_ctrl, CDNS_DPN_HCTRL_LCTRL); cdns_writel(cdns, dpn_hctrl_off, dpn_hctrl); cdns_writel(cdns, dpn_samplectrl_off, (t_params->sample_interval - 1)); -- cgit v1.2.3 From f067c9251797ab0ff13cb4c6d5f5a0e6dc2c65d3 Mon Sep 17 00:00:00 2001 From: Vinod Koul Date: Thu, 17 Sep 2020 17:31:46 +0530 Subject: soundwire: intel: use {u32|u16}p_replace_bits FIELD_PREP() does not replace the bits so it is not apt in case where we modify a register. Use u32_replace_bits() or u16_replace_bits() instead. Fixes: 3b4979cabd4b ("soundwire: intel: use FIELD_{GET|PREP}") Tested-by: Bard Liao Signed-off-by: Vinod Koul Link: https://lore.kernel.org/r/20200917120146.1780323-3-vkoul@kernel.org Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 1211d114ff59..7dc6569ac431 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -329,7 +329,7 @@ static int intel_link_power_up(struct sdw_intel *sdw) /* set SyncPRD period */ sync_reg = intel_readl(shim, SDW_SHIM_SYNC); - sync_reg |= FIELD_PREP(SDW_SHIM_SYNC_SYNCPRD, syncprd); + u32p_replace_bits(&sync_reg, syncprd, SDW_SHIM_SYNC_SYNCPRD); /* Set SyncCPU bit */ sync_reg |= SDW_SHIM_SYNC_SYNCCPU; @@ -448,7 +448,7 @@ static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop) intel_shim_glue_to_master_ip(sdw); - act |= FIELD_PREP(SDW_SHIM_CTMCTL_DOAIS, 0x1); + u16p_replace_bits(&act, 0x1, SDW_SHIM_CTMCTL_DOAIS); act |= SDW_SHIM_CTMCTL_DACTQE; act |= SDW_SHIM_CTMCTL_DODS; intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act); @@ -712,9 +712,9 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) else pdi_conf &= ~(SDW_SHIM_PCMSYCM_DIR); - pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_STREAM, pdi->intel_alh_id); - pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_LCHN, pdi->l_ch_num); - pdi_conf |= FIELD_PREP(SDW_SHIM_PCMSYCM_HCHN, pdi->h_ch_num); + u32p_replace_bits(&pdi_conf, pdi->intel_alh_id, SDW_SHIM_PCMSYCM_STREAM); + u32p_replace_bits(&pdi_conf, pdi->l_ch_num, SDW_SHIM_PCMSYCM_LCHN); + u32p_replace_bits(&pdi_conf, pdi->h_ch_num, SDW_SHIM_PCMSYCM_HCHN); intel_writew(shim, SDW_SHIM_PCMSYCHM(link_id, pdi->num), pdi_conf); } @@ -734,8 +734,8 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) /* Program Stream config ALH register */ conf = intel_readl(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id)); - conf |= FIELD_PREP(SDW_ALH_STRMZCFG_DMAT, SDW_ALH_STRMZCFG_DMAT_VAL); - conf |= FIELD_PREP(SDW_ALH_STRMZCFG_CHN, pdi->ch_count - 1); + u32p_replace_bits(&conf, SDW_ALH_STRMZCFG_DMAT_VAL, SDW_ALH_STRMZCFG_DMAT); + u32p_replace_bits(&conf, pdi->ch_count - 1, SDW_ALH_STRMZCFG_CHN); intel_writel(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id), conf); } -- cgit v1.2.3 From dd87a72ae968f70852cd7a2d939acd9693e76095 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 21 Sep 2020 03:32:05 +0800 Subject: soundwire: enable Data Port test modes Test modes are required for all SoundWire IP, and help debug integration issues. In theory each port can be configured with a different mode but to simplify this patch only offers separate configurations for the Master and Slave ports - this covers 99% of the intended cases during platform integration. The test mode value is set via platform-specific ways. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Guennadi Liakhovetski Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200920193207.31241-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 6 ++++++ drivers/soundwire/generic_bandwidth_allocation.c | 6 ++++-- drivers/soundwire/stream.c | 3 ++- include/linux/soundwire/sdw.h | 6 ++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 340fd1e23e5c..e3fe53f38708 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1099,6 +1099,12 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave, int ret; u8 val = 0; + if (slave->bus->params.s_data_mode != SDW_PORT_DATA_MODE_NORMAL) { + dev_dbg(&slave->dev, "TEST FAIL interrupt %s\n", + enable ? "on" : "off"); + mask |= SDW_DPN_INT_TEST_FAIL; + } + addr = SDW_DPN_INTMASK(port); /* Set/Clear port ready interrupt mask */ diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c index 6088775b67a5..058cb9cd8547 100644 --- a/drivers/soundwire/generic_bandwidth_allocation.c +++ b/drivers/soundwire/generic_bandwidth_allocation.c @@ -43,6 +43,7 @@ static void sdw_compute_slave_ports(struct sdw_master_runtime *m_rt, int port_bo, sample_int; unsigned int rate, bps, ch = 0; unsigned int slave_total_ch; + struct sdw_bus_params *b_params = &m_rt->bus->params; port_bo = t_data->block_offset; @@ -66,7 +67,7 @@ static void sdw_compute_slave_ports(struct sdw_master_runtime *m_rt, sdw_fill_port_params(&p_rt->port_params, p_rt->num, bps, SDW_PORT_FLOW_MODE_ISOCH, - SDW_PORT_DATA_MODE_NORMAL); + b_params->s_data_mode); port_bo += bps * ch; slave_total_ch += ch; @@ -92,6 +93,7 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt, struct sdw_transport_data t_data = {0}; struct sdw_port_runtime *p_rt; struct sdw_bus *bus = m_rt->bus; + struct sdw_bus_params *b_params = &bus->params; int sample_int, hstart = 0; unsigned int rate, bps, ch, no_ch; @@ -118,7 +120,7 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt, sdw_fill_port_params(&p_rt->port_params, p_rt->num, bps, SDW_PORT_FLOW_MODE_ISOCH, - SDW_PORT_DATA_MODE_NORMAL); + b_params->m_data_mode); /* Check for first entry */ if (!(p_rt == list_first_entry(&m_rt->port_list, diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index f3219d1fa63b..8608b093abcc 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -443,7 +443,8 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus, prep_ch.bank = bus->params.next_bank; - if (dpn_prop->imp_def_interrupts || !dpn_prop->simple_ch_prep_sm) + if (dpn_prop->imp_def_interrupts || !dpn_prop->simple_ch_prep_sm || + bus->params.s_data_mode != SDW_PORT_DATA_MODE_NORMAL) intr = true; /* diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index de9ea2ce2d35..41cc1192f9aa 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -554,6 +554,10 @@ enum sdw_port_prep_ops { * @bandwidth: Current bandwidth * @col: Active columns * @row: Active rows + * @s_data_mode: NORMAL, STATIC or PRBS mode for all Slave ports + * @m_data_mode: NORMAL, STATIC or PRBS mode for all Master ports. The value + * should be the same to detect transmission issues, but can be different to + * test the interrupt reports */ struct sdw_bus_params { enum sdw_reg_bank curr_bank; @@ -563,6 +567,8 @@ struct sdw_bus_params { unsigned int bandwidth; unsigned int col; unsigned int row; + int s_data_mode; + int m_data_mode; }; /** -- cgit v1.2.3 From 0f9138e75753511572f67b1bf943224d504ca170 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 21 Sep 2020 03:32:06 +0800 Subject: soundwire: intel: enable test modes This patch adds debugfs support to override the Master and Slave data modes. The settings only take effect prior to a new stream being prepared/enabled, or on resume. The test mode can be set to verify data integrity and detect bus clashes, but can only be used to test capture paths. In this case the input generated by a Slave source port is replaced by a fixed or cyclical patterns. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Guennadi Liakhovetski Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200920193207.31241-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 7dc6569ac431..6a1e862b16c3 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -262,6 +262,42 @@ static int intel_reg_show(struct seq_file *s_file, void *data) } DEFINE_SHOW_ATTRIBUTE(intel_reg); +static int intel_set_m_datamode(void *data, u64 value) +{ + struct sdw_intel *sdw = data; + struct sdw_bus *bus = &sdw->cdns.bus; + + if (value > SDW_PORT_DATA_MODE_STATIC_1) + return -EINVAL; + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + bus->params.m_data_mode = value; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(intel_set_m_datamode_fops, NULL, + intel_set_m_datamode, "%llu\n"); + +static int intel_set_s_datamode(void *data, u64 value) +{ + struct sdw_intel *sdw = data; + struct sdw_bus *bus = &sdw->cdns.bus; + + if (value > SDW_PORT_DATA_MODE_STATIC_1) + return -EINVAL; + + /* Userspace changed the hardware state behind the kernel's back */ + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + + bus->params.s_data_mode = value; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(intel_set_s_datamode_fops, NULL, + intel_set_s_datamode, "%llu\n"); + static void intel_debugfs_init(struct sdw_intel *sdw) { struct dentry *root = sdw->cdns.bus.debugfs; @@ -274,6 +310,12 @@ static void intel_debugfs_init(struct sdw_intel *sdw) debugfs_create_file("intel-registers", 0400, sdw->debugfs, sdw, &intel_reg_fops); + debugfs_create_file("intel-m-datamode", 0200, sdw->debugfs, sdw, + &intel_set_m_datamode_fops); + + debugfs_create_file("intel-s-datamode", 0200, sdw->debugfs, sdw, + &intel_set_s_datamode_fops); + sdw_cdns_debugfs_init(&sdw->cdns, sdw->debugfs); } -- cgit v1.2.3 From 9e4e6019e68cc1749bf5aba0ee5e91576de242bf Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 21 Sep 2020 03:32:07 +0800 Subject: soundwire: cadence: add data port test fail interrupt The Master ports can report errors in test data modes, enable the interrupt and just log a message. This capability is useful for Master sink ports only (Master source ports generate data). Signed-off-by: Pierre-Louis Bossart Reviewed-by: Guennadi Liakhovetski Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20200920193207.31241-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 19d445ef6764..9fa55164354a 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -175,6 +175,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask"); #define CDNS_DPN_HCTRL_LCTRL GENMASK(10, 8) #define CDNS_PORTCTRL 0x130 +#define CDNS_PORTCTRL_TEST_FAILED BIT(1) #define CDNS_PORTCTRL_DIRN BIT(7) #define CDNS_PORTCTRL_BANK_INVERT BIT(8) @@ -870,6 +871,19 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id) dev_err_ratelimited(cdns->dev, "Bus clash for data word\n"); } + if (cdns->bus.params.m_data_mode != SDW_PORT_DATA_MODE_NORMAL && + int_status & CDNS_MCP_INT_DPINT) { + u32 port_intstat; + + /* just log which ports report an error */ + port_intstat = cdns_readl(cdns, CDNS_MCP_PORT_INTSTAT); + dev_err_ratelimited(cdns->dev, "DP interrupt: PortIntStat %8x\n", + port_intstat); + + /* clear status w/ write1 */ + cdns_writel(cdns, CDNS_MCP_PORT_INTSTAT, port_intstat); + } + if (int_status & CDNS_MCP_INT_SLAVE_MASK) { /* Mask the Slave interrupt and wake thread */ cdns_updatel(cdns, CDNS_MCP_INTMASK, @@ -994,7 +1008,9 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns, bool state) mask |= CDNS_MCP_INT_CTRL_CLASH | CDNS_MCP_INT_DATA_CLASH | CDNS_MCP_INT_PARITY; - /* no detection of port interrupts for now */ + /* port interrupt limited to test modes for now */ + if (cdns->bus.params.m_data_mode != SDW_PORT_DATA_MODE_NORMAL) + mask |= CDNS_MCP_INT_DPINT; /* enable detection of RX fifo level */ mask |= CDNS_MCP_INT_RX_WL; @@ -1624,11 +1640,16 @@ void sdw_cdns_config_stream(struct sdw_cdns *cdns, { u32 offset, val = 0; - if (dir == SDW_DATA_DIR_RX) + if (dir == SDW_DATA_DIR_RX) { val = CDNS_PORTCTRL_DIRN; + if (cdns->bus.params.m_data_mode != SDW_PORT_DATA_MODE_NORMAL) + val |= CDNS_PORTCTRL_TEST_FAILED; + } offset = CDNS_PORTCTRL + pdi->num * CDNS_PORT_OFFSET; - cdns_updatel(cdns, offset, CDNS_PORTCTRL_DIRN, val); + cdns_updatel(cdns, offset, + CDNS_PORTCTRL_DIRN | CDNS_PORTCTRL_TEST_FAILED, + val); val = pdi->num; val |= CDNS_PDI_CONFIG_SOFT_RESET; -- cgit v1.2.3 From 5ec3215e56af8f7ce30c3d9d39029a85616713c0 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 23 Sep 2020 11:32:35 +0300 Subject: soundwire: remove an unnecessary NULL check The "bus" pointer isn't NULL so the address to a non-zero offset in middle of "bus" cannot be NULL. Delete the NULL check. Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/20200923083235.GB1454948@mwanda Signed-off-by: Vinod Koul --- drivers/soundwire/generic_bandwidth_allocation.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c index 058cb9cd8547..0bdef38c9a30 100644 --- a/drivers/soundwire/generic_bandwidth_allocation.c +++ b/drivers/soundwire/generic_bandwidth_allocation.c @@ -344,15 +344,11 @@ static int sdw_select_row_col(struct sdw_bus *bus, int clk_freq) static int sdw_compute_bus_params(struct sdw_bus *bus) { unsigned int max_dr_freq, curr_dr_freq = 0; - struct sdw_master_prop *mstr_prop = NULL; + struct sdw_master_prop *mstr_prop = &bus->prop; int i, clk_values, ret; bool is_gear = false; u32 *clk_buf; - mstr_prop = &bus->prop; - if (!mstr_prop) - return -EINVAL; - if (mstr_prop->num_clk_gears) { clk_values = mstr_prop->num_clk_gears; clk_buf = mstr_prop->clk_gears; -- cgit v1.2.3 From fcb9d730be1d3010e0c8fdc5aaff62836d7a942c Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Thu, 24 Sep 2020 14:44:29 -0500 Subject: soundwire: bus: add enumerated Slave device to device list Currently Slave devices are only added on startup, either from Device Tree or ACPI entries. However Slave devices that are physically present on the bus, but not described in platform firmware, will never be added to the device list. The user/integrator can only know the list of devices by looking a dynamic debug logs. This patch suggests adding a Slave device even if there is no matching DT or ACPI entry, so that we can see this in sysfs entry. Initial code from Srinivas. Comments, fixes for ACPI probe and edits of commit message by Pierre. Signed-off-by: Srinivas Kandagatla Co-developed-by: Pierre-Louis Bossart Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200924194430.121058-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 9 +++++++++ drivers/soundwire/bus.h | 2 ++ drivers/soundwire/bus_type.c | 9 +++++++++ drivers/soundwire/slave.c | 4 ++-- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index e3fe53f38708..f4c7887c3234 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -747,6 +747,15 @@ static int sdw_program_device_num(struct sdw_bus *bus) if (!found) { /* TODO: Park this device in Group 13 */ + + /* + * add Slave device even if there is no platform + * firmware description. There will be no driver probe + * but the user/integration will be able to see the + * device, enumeration status and device number in sysfs + */ + sdw_slave_add(bus, &id, NULL); + dev_err(bus->dev, "Slave Entry not found\n"); } diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index e0703991aad9..2e049d39c6e5 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -19,6 +19,8 @@ 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_slave_add(struct sdw_bus *bus, struct sdw_slave_id *id, + struct fwnode_handle *fwnode); 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); diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 6fba55898cf0..575b9bad99d5 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -84,6 +84,15 @@ static int sdw_drv_probe(struct device *dev) const struct sdw_device_id *id; int ret; + /* + * fw description is mandatory to bind + */ + if (!dev->fwnode) + return -ENODEV; + + if (!IS_ENABLED(CONFIG_ACPI) && !dev->of_node) + return -ENODEV; + id = sdw_get_device_id(slave, drv); if (!id) return -ENODEV; diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 4a250d33de5d..19b012310c29 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -20,8 +20,8 @@ struct device_type sdw_slave_type = { .uevent = sdw_slave_uevent, }; -static int sdw_slave_add(struct sdw_bus *bus, - struct sdw_slave_id *id, struct fwnode_handle *fwnode) +int sdw_slave_add(struct sdw_bus *bus, + struct sdw_slave_id *id, struct fwnode_handle *fwnode) { struct sdw_slave *slave; int ret; -- cgit v1.2.3 From 0173f525b2c1b02a51784e2119d434593235aed1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 24 Sep 2020 14:44:30 -0500 Subject: soundwire: sysfs: add slave status and device number before probe The MIPI DisCo device properties that are read by the driver from platform firmware, or hard-coded in the driver, should only be provided as sysfs entries when a driver probes successfully. However the device status and device number is updated even when there is no driver present, and hence can be updated when a Slave device is detected on the bus without being described in platform firmware and without any driver registered/probed. As suggested by GregKH, the attribute group for Slave status and device number is is added by default upon device registration. Credits to Vinod Koul for the status_show() function, shared in a separate patch and used as is here. The status table was modified to remove an unnecessary enum and status_show() is handled in a different group attribute than what was suggested by Vinod. Tested-by: Srinivas Kandgatla Signed-off-by: Pierre-Louis Bossart Co-developed-by: Vinod Koul Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20200924194430.121058-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Vinod Koul --- .../ABI/testing/sysfs-bus-soundwire-slave | 18 +++++++ drivers/soundwire/slave.c | 2 + drivers/soundwire/sysfs_local.h | 4 ++ drivers/soundwire/sysfs_slave.c | 58 +++++++++++++++++++++- 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/Documentation/ABI/testing/sysfs-bus-soundwire-slave b/Documentation/ABI/testing/sysfs-bus-soundwire-slave index db4c9511d1aa..d324aa0b678f 100644 --- a/Documentation/ABI/testing/sysfs-bus-soundwire-slave +++ b/Documentation/ABI/testing/sysfs-bus-soundwire-slave @@ -1,3 +1,21 @@ +What: /sys/bus/soundwire/devices/sdw:.../status + /sys/bus/soundwire/devices/sdw:.../device_number + +Date: September 2020 + +Contact: Pierre-Louis Bossart + Bard Liao + Vinod Koul + +Description: SoundWire Slave status + + These properties report the Slave status, e.g. if it + is UNATTACHED or not, and in the latter case show the + device_number. This status information is useful to + detect devices exposed by platform firmware but not + physically present on the bus, and conversely devices + not exposed in platform firmware but enumerated. + 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 diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 19b012310c29..a08f4081c1c4 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -6,6 +6,7 @@ #include #include #include "bus.h" +#include "sysfs_local.h" static void sdw_slave_release(struct device *dev) { @@ -51,6 +52,7 @@ int sdw_slave_add(struct sdw_bus *bus, slave->dev.bus = &sdw_bus_type; slave->dev.of_node = of_node_get(to_of_node(fwnode)); slave->dev.type = &sdw_slave_type; + slave->dev.groups = sdw_slave_status_attr_groups; slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; init_completion(&slave->enumeration_complete); diff --git a/drivers/soundwire/sysfs_local.h b/drivers/soundwire/sysfs_local.h index ff60adee3c41..7268bc24c538 100644 --- a/drivers/soundwire/sysfs_local.h +++ b/drivers/soundwire/sysfs_local.h @@ -8,6 +8,10 @@ * SDW sysfs APIs - */ +/* basic attributes to report status of Slave (attachment, dev_num) */ +extern const struct attribute_group *sdw_slave_status_attr_groups[]; + +/* additional device-managed properties reported after driver probe */ int sdw_slave_sysfs_init(struct sdw_slave *slave); int sdw_slave_sysfs_dpn_init(struct sdw_slave *slave); diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c index f510071b0add..b48b6617a396 100644 --- a/drivers/soundwire/sysfs_slave.c +++ b/drivers/soundwire/sysfs_slave.c @@ -16,9 +16,13 @@ /* * The sysfs for Slave reflects the MIPI description as given - * in the MIPI DisCo spec + * in the MIPI DisCo spec. + * status and device_number come directly from the MIPI SoundWire + * 1.x specification. * * Base file is device + * |---- status + * |---- device_number * |---- modalias * |---- dev-properties * |---- mipi_revision @@ -212,3 +216,55 @@ int sdw_slave_sysfs_init(struct sdw_slave *slave) return 0; } + +/* + * the status is shown in capital letters for UNATTACHED and RESERVED + * on purpose, to highligh users to the fact that these status values + * are not expected. + */ +static const char *const slave_status[] = { + [SDW_SLAVE_UNATTACHED] = "UNATTACHED", + [SDW_SLAVE_ATTACHED] = "Attached", + [SDW_SLAVE_ALERT] = "Alert", + [SDW_SLAVE_RESERVED] = "RESERVED", +}; + +static ssize_t status_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct sdw_slave *slave = dev_to_sdw_dev(dev); + + return sprintf(buf, "%s\n", slave_status[slave->status]); +} +static DEVICE_ATTR_RO(status); + +static ssize_t device_number_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct sdw_slave *slave = dev_to_sdw_dev(dev); + + if (slave->status == SDW_SLAVE_UNATTACHED) + return sprintf(buf, "%s", "N/A"); + else + return sprintf(buf, "%d", slave->dev_num); +} +static DEVICE_ATTR_RO(device_number); + +static struct attribute *slave_status_attrs[] = { + &dev_attr_status.attr, + &dev_attr_device_number.attr, + NULL, +}; + +/* + * we don't use ATTRIBUTES_GROUP here since the group is used in a + * separate file and can't be handled as a static. + */ +static const struct attribute_group sdw_slave_status_attr_group = { + .attrs = slave_status_attrs, +}; + +const struct attribute_group *sdw_slave_status_attr_groups[] = { + &sdw_slave_status_attr_group, + NULL +}; -- cgit v1.2.3