From e8f1b03b3540d7ee990674cf1c7b8ee29a127aad Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 9 Oct 2019 11:34:58 -0500 Subject: [PATCH 01/59] soundwire: sdw_slave: add probe_complete structure and new fields When a Slave device becomes synchronized with the bus, it may report its presence in PING frames, as well as optionally asserting an in-band PREQ signal. The bus driver will detect a new Device0, start the enumeration process and assign it a non-zero device number. The SoundWire enumeration provides an arbitration to deal with multiple Slaves reporting ATTACHED at the same time. The bus driver will also invoke the driver .probe() callback associated with this device. The probe() depends on the Linux device core, which handles the match operations and may result in modules being loaded. Once the non-zero device number is programmed, the Slave will report its new status in PING frames and the Master hardware will typically report this status change with an interrupt. At this point, the .update_status() callback of the codec driver will be invoked (usually from an interrupt thread or workqueue scheduled from the interrupt thread). The first race condition which can happen is between the .probe(), which allocates the resources, and .update_status() where initializations are typically handled. The .probe() is only called once during the initial boot, while .update_status() will be called for every bus hardware reset and if the Slave device loses synchronization (an unlikely event but with non-zero probability). The time difference between the end of the enumeration process and a change of status reported by the hardware may be as small as one SoundWire PING frame. The scheduling of the interrupt thread, which invokes .update_status() is not deterministic, but can be small enough to create a race condition. With a 48 kHz frame rate and ideal scheduling cases, the .probe() may be pre-empted within double-digit microseconds. Since there is no guarantee that the .probe() completes by the time .update_status() is invoked as a result of an interrupt, it's not unusual for the .update_status() to rely on data structures that have not been allocated yet, leading to kernel oopses. This patch adds a probe_complete utility, which is used in the sdw_update_slave_status() routine. The codec driver does not need to do anything and can safely assume all resources are allocated in its update_status() callback. Signed-off-by: Pierre-Louis Bossart --- 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 28745b9ba27954..cb1db4a7475d5b 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -547,6 +547,10 @@ struct sdw_slave_ops { * @node: node for bus list * @port_ready: Port ready completion flag for each Slave port * @dev_num: Device Number assigned by Bus + * @probed: boolean tracking driver state + * @probe_complete: completion utility to control potential races + * on startup between driver probe/initialization and SoundWire + * Slave state changes/implementation-defined interrupts */ struct sdw_slave { struct sdw_slave_id id; @@ -561,6 +565,8 @@ struct sdw_slave { struct list_head node; struct completion *port_ready; u16 dev_num; + bool probed; + struct completion probe_complete; }; #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) From 91ee5979e715122a2271f0a5ff89289e033870f3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 18:42:47 -0500 Subject: [PATCH 02/59] soundwire: sdw_slave: add enumeration_complete structure When the Master starts the bus (be it during the initial boot or system resume), it usually performs a HardReset to make sure electrical levels are correct, then enables the control channel. While the PM framework guarantees that the Slave devices will only become 'active' once the Master completes the bus initialization, there is still a risk of a race condition: the Slave enumeration is handled in a separate interrupt thread triggered by hardware status changes, so the Slave device may not be ready to accept commands when the Slave driver tries to access the registers and restore settings in its resume or pm_runtime_resume callbacks. In those cases, any read/write commands from/to the Slave device will result in a timeout. This patch adds an enumeration_complete structure. When the bus is goes through a HardReset sequence and restarted, the Slave will be marked as UNATTACHED, which will result in a call to init_completion(). When the Slave reports its presence during PING frames as a non-zero Device, the Master hardware will issue an interrupt and the bus driver will invoke complete(). The order between init_completion()/complete() is predictable since this is a Master-initiated transition. The Slave driver may use wait_for_completion() in its resume callback. When regmap is used, the Slave driver will typically set its regmap in cache-only mode on suspend, then on resume block on wait_for_completion(&enumeration_complete) to guarantee it is safe to start read/write transactions. It may then exit the cache-only mode and use a regmap_sync to restore settings. All these steps are optional, their use completely depends on the Slave device capabilities and how the Slave driver is implemented. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index cb1db4a7475d5b..3fa8d875b16baa 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -551,6 +551,9 @@ struct sdw_slave_ops { * @probe_complete: completion utility to control potential races * on startup between driver probe/initialization and SoundWire * Slave state changes/implementation-defined interrupts + * @enumeration_complete: completion utility to control potential races + * on startup between device enumeration and read/write access to the + * Slave device */ struct sdw_slave { struct sdw_slave_id id; @@ -567,6 +570,7 @@ struct sdw_slave { u16 dev_num; bool probed; struct completion probe_complete; + struct completion enumeration_complete; }; #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) From 409e3cc226eeed7a1969149fb787f088f213300e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 4 Nov 2019 12:44:46 -0600 Subject: [PATCH 03/59] soundwire: sdw_slave: add initialization_complete definition Slave drivers may have different ways of handling their settings, with or without regmap. During the integration of codec drivers, done in partnership between Intel and Realtek, it became desirable to implement a predictable order between low-level initializations performed in .update_status() (invoked by an interrupt thread) and the settings restored in the resume steps (invoked by the PM core). This patch builds on the previous solution to wait for the Slave device to be fully enumerated. The complete() in this case is signaled not before the .update_status() is called, but after .update_status() returns. Without this patch, the settings were not properly restored, leading to timing-dependent 'no sound after resume' or 'no headset detected after resume' bug reports. Depending on how initialization is handled, a Slave device driver may wait for enumeration_complete, or for initialization_complete, both are valid synchronization points. They are initialized at the same time, they only differ on when complete() is invoked. Signed-off-by: Pierre-Louis Bossart --- include/linux/soundwire/sdw.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 3fa8d875b16baa..ed42cd79eab7ef 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -554,6 +554,8 @@ struct sdw_slave_ops { * @enumeration_complete: completion utility to control potential races * on startup between device enumeration and read/write access to the * Slave device + * @initialization_complete: completion utility to control potential races + * on startup between device enumeration and settings being restored */ struct sdw_slave { struct sdw_slave_id id; @@ -571,6 +573,7 @@ struct sdw_slave { bool probed; struct completion probe_complete; struct completion enumeration_complete; + struct completion initialization_complete; }; #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) From b8441b6af999c99ccf4256d8b7142f8010bed624 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 11 Nov 2019 10:48:31 -0600 Subject: [PATCH 04/59] soundwire: sdw_slave: track unattach_request to handle all init sequences The Slave device initialization can be split in 4 different cases: 1. Master-initiated hardware reset, system suspend-resume and pm_runtime based on clock-stop mode1. To avoid timeouts and a bad audio experience, the Slave device resume operations need to wait for the Slave device to be re-enumerated and its settings restored. 2. Exit from clock-stop mode0. In this case, the Slave device is required to remain enumerated and its context preserved while the clock is stopped, so no re-initialization or wait_for_completion() is necessary. 3. Slave-initiated pm_runtime D3 transition. With the parent child relationship, it is possible that a Slave device becomes 'suspended' while its parent is still 'active' with the bus clock still toggling. In this case, during the pm_runtime resume operation, there is no need to wait for any settings to be restored. 4. Slave reset (sync loss or implementation-defined). In that case the bus remains operational and the Slave device will be re-initialized when it becomes ATTACHED again. In previous patches, we suggested the use of wait_for_completion() to deal with the case #1, but case #2 and #3 do not need any wait. To account for those differences, this patch adds an unattach_request field. The field is explicitly set by the Master for the case #1, and if non-zero the Slave device shall wait on resume. In all other cases, the Slave resume operations can proceed without wait. The only request tracked so far is Master HardReset, but the request is declared as a bit mask for future extensions (if needed). The definition for this value is added in bus.h and does not need to be exposed in sdw.h Signed-off-by: Pierre-Louis Bossart --- 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 ed42cd79eab7ef..b7c9eca4332a3a 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -556,6 +556,11 @@ struct sdw_slave_ops { * Slave device * @initialization_complete: completion utility to control potential races * on startup between device enumeration and settings being restored + * @unattach_request: mask field to keep track why the Slave re-attached and + * was re-initialized. This is useful to deal with potential race conditions + * 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 */ struct sdw_slave { struct sdw_slave_id id; @@ -574,6 +579,7 @@ struct sdw_slave { struct completion probe_complete; struct completion enumeration_complete; struct completion initialization_complete; + u32 unattach_request; }; #define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) From a3e3e575624c71f455078e60ce3b4b10f730ac71 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 23 Oct 2019 12:10:43 -0500 Subject: [PATCH 05/59] soundwire: intel: update interfaces between ASoC and SoundWire The current interfaces between ASoC and SoundWire are limited by the platform_device infrastructure to an init() and exit() (mapped to the platform driver.probe and .remove) To help with the platform detection, machine driver selection and management of power dependencies between DSP and SoundWire IP, the ASoC side requires: a) an ACPI scan helper, to report if any devices are exposed in the DSDT tables, and if any links are disabled by the BIOS. b) a probe helper that allocates the resources without actually starting the bus. c) a startup helper which does start the bus when all power dependencies are settled. d) an exit helper to free all resources e) an interrupt_enable/disable helper, typically invoked after the startup helper but also used in suspend routines. This patch moves all required interfaces to sdw_intel.h, mainly to allow SoundWire and ASoC parts to be merged separately once the header files are shared between trees. To avoid compilation issues, the conflicts in intel_init.c are blindly removed. This would in theory prevent the code from working, but since there are no users of the Intel Soundwire driver this has no impact. Functionality will be restored when the removal of platform devices is complete. Support for SoundWire + SOF builds will only be provided once all the required pieces are upstream. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.h | 9 ++-- drivers/soundwire/intel_init.c | 30 +++-------- include/linux/soundwire/sdw_intel.h | 77 +++++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index d923b626233026..e4cc1d3804ff32 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -5,17 +5,20 @@ #define __SDW_INTEL_LOCAL_H /** - * struct sdw_intel_link_res - Soundwire link resources + * struct sdw_intel_link_res - Soundwire Intel link resource structure, + * typically populated by the controller driver. + * @pdev: platform_device + * @mmio_base: mmio base of SoundWire registers * @registers: Link IO registers base * @shim: Audio shim pointer * @alh: ALH (Audio Link Hub) pointer * @irq: Interrupt line * @ops: Shim callback ops * @arg: Shim callback ops argument - * - * This is set as pdata for each link instance. */ struct sdw_intel_link_res { + struct platform_device *pdev; + void __iomem *mmio_base; /* not strictly needed, useful for debug */ void __iomem *registers; void __iomem *shim; void __iomem *alh; diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 2a2b4d8df46222..f8ae199094d39f 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -27,19 +27,9 @@ static int link_mask; module_param_named(sdw_link_mask, link_mask, int, 0444); MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)"); -struct sdw_link_data { - struct sdw_intel_link_res res; - struct platform_device *pdev; -}; - -struct sdw_intel_ctx { - int count; - struct sdw_link_data *links; -}; - static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx) { - struct sdw_link_data *link = ctx->links; + struct sdw_intel_link_res *link = ctx->links; int i; if (!link) @@ -62,7 +52,7 @@ static struct sdw_intel_ctx { struct platform_device_info pdevinfo; struct platform_device *pdev; - struct sdw_link_data *link; + struct sdw_intel_link_res *link; struct sdw_intel_ctx *ctx; struct acpi_device *adev; int ret, i; @@ -123,14 +113,12 @@ static struct sdw_intel_ctx continue; } - link->res.irq = res->irq; - link->res.registers = res->mmio_base + SDW_LINK_BASE + link->registers = res->mmio_base + SDW_LINK_BASE + (SDW_LINK_SIZE * i); - link->res.shim = res->mmio_base + SDW_SHIM_BASE; - link->res.alh = res->mmio_base + SDW_ALH_BASE; + link->shim = res->mmio_base + SDW_SHIM_BASE; + link->alh = res->mmio_base + SDW_ALH_BASE; - link->res.ops = res->ops; - link->res.arg = res->arg; + link->ops = res->ops; memset(&pdevinfo, 0, sizeof(pdevinfo)); @@ -138,8 +126,6 @@ static struct sdw_intel_ctx pdevinfo.name = "int-sdw"; pdevinfo.id = i; pdevinfo.fwnode = acpi_fwnode_handle(adev); - pdevinfo.data = &link->res; - pdevinfo.size_data = sizeof(link->res); pdev = platform_device_register_full(&pdevinfo); if (IS_ERR(pdev)) { @@ -224,10 +210,8 @@ EXPORT_SYMBOL(sdw_intel_init); * * Delete the controller instances created and cleanup */ -void sdw_intel_exit(void *arg) +void sdw_intel_exit(struct sdw_intel_ctx *ctx) { - struct sdw_intel_ctx *ctx = arg; - sdw_intel_cleanup_pdev(ctx); kfree(ctx); } diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index c9427cb6020b3e..034eca8df74808 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -16,24 +16,91 @@ struct sdw_intel_ops { }; /** - * struct sdw_intel_res - Soundwire Intel resource structure + * struct sdw_intel_acpi_info - Soundwire Intel information found in ACPI tables + * @handle: ACPI controller handle + * @count: link count found with "sdw-master-count" property + * @link_mask: bit-wise mask listing links enabled by BIOS menu + * + * this structure could be expanded to e.g. provide all the _ADR + * information in case the link_mask is not sufficient to identify + * platform capabilities. + */ +struct sdw_intel_acpi_info { + acpi_handle handle; + int count; + u32 link_mask; +}; + +struct sdw_intel_link_res; + +/** + * struct sdw_intel_ctx - context allocated by the controller + * driver probe + * @count: link count + * @mmio_base: mmio base of SoundWire registers, only used to check + * hardware capabilities after all power dependencies are settled. + * @link_mask: bit-wise mask listing SoundWire links reported by the + * Controller + * @handle: ACPI parent handle + * @links: information for each link (controller-specific and kept + * opaque here) + */ +struct sdw_intel_ctx { + int count; + void __iomem *mmio_base; + u32 link_mask; + acpi_handle handle; + struct sdw_intel_link_res *links; +}; + +/** + * struct sdw_intel_res - Soundwire Intel global resource structure, + * typically populated by the DSP driver + * + * @count: link count * @mmio_base: mmio base of SoundWire registers * @irq: interrupt number * @handle: ACPI parent handle * @parent: parent device * @ops: callback ops - * @arg: callback arg + * @dev: device implementing hwparams and free callbacks + * @link_mask: bit-wise mask listing links selected by the DSP driver + * This mask may be a subset of the one reported by the controller since + * machine-specific quirks are handled in the DSP driver. */ struct sdw_intel_res { + int count; void __iomem *mmio_base; int irq; acpi_handle handle; struct device *parent; const struct sdw_intel_ops *ops; - void *arg; + struct device *dev; + u32 link_mask; }; -void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res); -void sdw_intel_exit(void *arg); +/* + * On Intel platforms, the SoundWire IP has dependencies on power + * rails shared with the DSP, and the initialization steps are split + * in three. First an ACPI scan to check what the firmware describes + * in DSDT tables, then an allocation step (with no hardware + * configuration but with all the relevant devices created) and last + * the actual hardware configuration. The final stage is a global + * interrupt enable which is controlled by the DSP driver. Splitting + * these phases helps simplify the boot flow and make early decisions + * on e.g. which machine driver to select (I2S mode, HDaudio or + * SoundWire). + */ +int sdw_intel_acpi_scan(acpi_handle *parent_handle, + struct sdw_intel_acpi_info *info); + +struct sdw_intel_ctx * +sdw_intel_probe(struct sdw_intel_res *res); + +int sdw_intel_startup(struct sdw_intel_ctx *ctx); + +void sdw_intel_exit(struct sdw_intel_ctx *ctx); + +void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable); #endif From c7b0f96203a7febddf5a460a960e934e5d2438a4 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 19 Aug 2019 13:32:09 -0500 Subject: [PATCH 06/59] soundwire: intel: update stream callbacks for hwparams/free stream operations The SoundWire DAIs for Intel platform are created in drivers/soundwire/intel.c, while the communication with the Intel DSP is all controlled in soc/sof/intel When the DAI status changes, a callback is used to bridge the gap between the two subsystems. The naming of the existing 'config_stream' callback does not map well with any of ALSA/ASoC concepts. This patch renames it as 'params_stream' to be more self-explanatory. A new 'free_stream' callback is added in case any resources allocated in the 'params_stream' stage need to be released. In the SOF implementation, this is used in the hw_free case to release the DMA channels over IPC. These two callbacks now rely on structures which expose the link_id and alh_stream_id (required by the firmware IPC), instead of a list of parameters. The 'void *' definitions are changed to use explicit types, as suggested on alsa-devel during earlier reviews. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 20 ++++++++++++------ drivers/soundwire/intel.h | 4 ++-- drivers/soundwire/intel_init.c | 1 + include/linux/soundwire/sdw_intel.h | 32 +++++++++++++++++++++++++---- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 99dc610212113f..0371d3d5501a97 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -529,17 +529,24 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) intel_writel(alh, SDW_ALH_STRMZCFG(pdi->intel_alh_id), conf); } -static int intel_config_stream(struct sdw_intel *sdw, +static int intel_params_stream(struct sdw_intel *sdw, struct snd_pcm_substream *substream, struct snd_soc_dai *dai, - struct snd_pcm_hw_params *hw_params, int link_id) + struct snd_pcm_hw_params *hw_params, + int link_id, int alh_stream_id) { struct sdw_intel_link_res *res = sdw->res; + struct sdw_intel_stream_params_data params_data; - if (res->ops && res->ops->config_stream && res->arg) - return res->ops->config_stream(res->arg, - substream, dai, hw_params, link_id); + params_data.substream = substream; + params_data.dai = dai; + params_data.hw_params = hw_params; + params_data.link_id = link_id; + params_data.alh_stream_id = alh_stream_id; + if (res->ops && res->ops->params_stream && res->dev) + return res->ops->params_stream(res->dev, + ¶ms_data); return -EIO; } @@ -654,7 +661,8 @@ static int intel_hw_params(struct snd_pcm_substream *substream, /* Inform DSP about PDI stream number */ - ret = intel_config_stream(sdw, substream, dai, params, + ret = intel_params_stream(sdw, substream, dai, params, + sdw->instance, pdi->intel_alh_id); if (ret) goto error; diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index e4cc1d3804ff32..38b7c125fb1052 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -14,7 +14,7 @@ * @alh: ALH (Audio Link Hub) pointer * @irq: Interrupt line * @ops: Shim callback ops - * @arg: Shim callback ops argument + * @dev: device implementing hw_params and free callbacks */ struct sdw_intel_link_res { struct platform_device *pdev; @@ -24,7 +24,7 @@ struct sdw_intel_link_res { void __iomem *alh; int irq; const struct sdw_intel_ops *ops; - void *arg; + struct device *dev; }; #endif /* __SDW_INTEL_LOCAL_H */ diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index f8ae199094d39f..6bc167c83b475e 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -119,6 +119,7 @@ static struct sdw_intel_ctx link->alh = res->mmio_base + SDW_ALH_BASE; link->ops = res->ops; + link->dev = res->dev; memset(&pdevinfo, 0, sizeof(pdevinfo)); diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 034eca8df74808..3ccb38d48eef15 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -4,15 +4,39 @@ #ifndef __SDW_INTEL_H #define __SDW_INTEL_H +/** + * struct sdw_intel_stream_params_data: configuration passed during + * the @params_stream callback, e.g. for interaction with DSP + * firmware. + */ +struct sdw_intel_stream_params_data { + struct snd_pcm_substream *substream; + struct snd_soc_dai *dai; + struct snd_pcm_hw_params *hw_params; + int link_id; + int alh_stream_id; +}; + +/** + * struct sdw_intel_stream_free_data: configuration passed during + * the @free_stream callback, e.g. for interaction with DSP + * firmware. + */ +struct sdw_intel_stream_free_data { + struct snd_pcm_substream *substream; + struct snd_soc_dai *dai; + int link_id; +}; + /** * struct sdw_intel_ops: Intel audio driver callback ops * - * @config_stream: configure the stream with the hw_params - * the first argument containing the context is mandatory */ struct sdw_intel_ops { - int (*config_stream)(void *arg, void *substream, - void *dai, void *hw_params, int stream_num); + int (*params_stream)(struct device *dev, + struct sdw_intel_stream_params_data *params_data); + int (*free_stream)(struct device *dev, + struct sdw_intel_stream_free_data *free_data); }; /** From 77099d2dc3d69ba0cd0c3b40e9d09a4c4813b816 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 18:46:08 -0500 Subject: [PATCH 07/59] soundwire: renames to prepare support for master drivers/devices Add clearer references to sdw_slave_driver for internal macros No change for sdw_driver and module_sdw_driver to avoid compatibility issues with existing codec devices No functionality change. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus_type.c | 21 +++++++++++---------- include/linux/soundwire/sdw_type.h | 18 ++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 4a465f55039f4b..370b9475266228 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -34,7 +34,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv) static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(ddrv); + struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv); return !!sdw_get_device_id(slave, drv); } @@ -70,7 +70,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type); static int sdw_drv_probe(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret; @@ -116,7 +116,7 @@ static int sdw_drv_probe(struct device *dev) static int sdw_drv_remove(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); int ret = 0; if (drv->remove) @@ -130,20 +130,21 @@ static int sdw_drv_remove(struct device *dev) static void sdw_drv_shutdown(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); - struct sdw_driver *drv = drv_to_sdw_driver(dev->driver); + struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); if (drv->shutdown) drv->shutdown(slave); } /** - * __sdw_register_driver() - register a SoundWire Slave driver + * __sdw_register_slave_driver() - register a SoundWire Slave driver * @drv: driver to register * @owner: owning module/driver * * Return: zero on success, else a negative error code. */ -int __sdw_register_driver(struct sdw_driver *drv, struct module *owner) +int __sdw_register_slave_driver(struct sdw_driver *drv, + struct module *owner) { drv->driver.bus = &sdw_bus_type; @@ -164,17 +165,17 @@ int __sdw_register_driver(struct sdw_driver *drv, struct module *owner) return driver_register(&drv->driver); } -EXPORT_SYMBOL_GPL(__sdw_register_driver); +EXPORT_SYMBOL_GPL(__sdw_register_slave_driver); /** - * sdw_unregister_driver() - unregisters the SoundWire Slave driver + * sdw_unregister_slave_driver() - unregisters the SoundWire Slave driver * @drv: driver to unregister */ -void sdw_unregister_driver(struct sdw_driver *drv) +void sdw_unregister_slave_driver(struct sdw_driver *drv) { driver_unregister(&drv->driver); } -EXPORT_SYMBOL_GPL(sdw_unregister_driver); +EXPORT_SYMBOL_GPL(sdw_unregister_slave_driver); static int __init sdw_bus_init(void) { diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index aaa7f4267c149a..abaa2127815254 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,13 +6,15 @@ extern struct bus_type sdw_bus_type; -#define drv_to_sdw_driver(_drv) container_of(_drv, struct sdw_driver, driver) +#define drv_to_sdw_slave_driver(_drv) \ + container_of(_drv, struct sdw_driver, driver) -#define sdw_register_driver(drv) \ - __sdw_register_driver(drv, THIS_MODULE) +#define sdw_register_slave_driver(drv) \ + __sdw_register_slave_driver(drv, THIS_MODULE) -int __sdw_register_driver(struct sdw_driver *drv, struct module *owner); -void sdw_unregister_driver(struct sdw_driver *drv); +int __sdw_register_slave_driver(struct sdw_driver *drv, + struct module *owner); +void sdw_unregister_slave_driver(struct sdw_driver *drv); int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size); @@ -24,7 +26,7 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size); * module init/exit. This eliminates a lot of boilerplate. Each module may only * use this macro once, and calling it replaces module_init() and module_exit() */ -#define module_sdw_driver(__sdw_driver) \ - module_driver(__sdw_driver, sdw_register_driver, \ - sdw_unregister_driver) +#define module_sdw_driver(__sdw_slave_driver) \ + module_driver(__sdw_slave_driver, sdw_register_slave_driver, \ + sdw_unregister_slave_driver) #endif /* __SOUNDWIRE_TYPES_H */ From a10f8d2437e7b1b892bbd1196c84ebfe9980e061 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 19:58:17 -0500 Subject: [PATCH 08/59] soundwire: rename dev_to_sdw_dev macro Since we want to introduce master devices, rename macro so that we have consistency between slave and master device access, following the Grey Bus example. Signed-off-by: Pierre-Louis Bossart --- drivers/base/regmap/regmap-sdw.c | 4 ++-- drivers/soundwire/bus.c | 2 +- drivers/soundwire/bus_type.c | 11 ++++++----- drivers/soundwire/slave.c | 2 +- include/linux/soundwire/sdw.h | 3 ++- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/base/regmap/regmap-sdw.c b/drivers/base/regmap/regmap-sdw.c index 50a66382d87d09..d1fc0c22180a17 100644 --- a/drivers/base/regmap/regmap-sdw.c +++ b/drivers/base/regmap/regmap-sdw.c @@ -10,7 +10,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) { struct device *dev = context; - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); return sdw_write(slave, reg, val); } @@ -18,7 +18,7 @@ static int regmap_sdw_write(void *context, unsigned int reg, unsigned int val) static int regmap_sdw_read(void *context, unsigned int reg, unsigned int *val) { struct device *dev = context; - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); int read; read = sdw_read(slave, reg); diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index be5d437058ed19..4b22ee996a6566 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -110,7 +110,7 @@ EXPORT_SYMBOL(sdw_add_bus_master); static int sdw_delete_slave(struct device *dev, void *data) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_bus *bus = slave->bus; sdw_slave_debugfs_exit(slave); diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 370b9475266228..c0585bcc8a41b8 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -33,7 +33,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv) static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv); return !!sdw_get_device_id(slave, drv); @@ -49,7 +49,7 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size) static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); char modalias[32]; sdw_slave_modalias(slave, modalias, sizeof(modalias)); @@ -69,7 +69,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type); static int sdw_drv_probe(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret; @@ -115,8 +115,9 @@ static int sdw_drv_probe(struct device *dev) static int sdw_drv_remove(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + int ret = 0; if (drv->remove) @@ -129,7 +130,7 @@ static int sdw_drv_remove(struct device *dev) static void sdw_drv_shutdown(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); if (drv->shutdown) diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 19919975bb6da4..48a513680db665 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -9,7 +9,7 @@ static void sdw_slave_release(struct device *dev) { - struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct sdw_slave *slave = to_sdw_slave_device(dev); kfree(slave); } diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index b7c9eca4332a3a..5b1180f1e6b558 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -582,7 +582,8 @@ struct sdw_slave { u32 unattach_request; }; -#define dev_to_sdw_dev(_dev) container_of(_dev, struct sdw_slave, dev) +#define to_sdw_slave_device(d) \ + container_of(d, struct sdw_slave, dev) struct sdw_driver { const char *name; From c57f5ecf4cc334888e4d604585c16be1d9103063 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 20:27:41 -0500 Subject: [PATCH 09/59] soundwire: rename drv_to_sdw_slave_driver macro Align with previous renames and shorten macro No functionality change Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus_type.c | 9 ++++----- include/linux/soundwire/sdw_type.h | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index c0585bcc8a41b8..2b2830b622faef 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -34,7 +34,7 @@ sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv) static int sdw_bus_match(struct device *dev, struct device_driver *ddrv) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(ddrv); + struct sdw_driver *drv = to_sdw_slave_driver(ddrv); return !!sdw_get_device_id(slave, drv); } @@ -70,7 +70,7 @@ EXPORT_SYMBOL_GPL(sdw_bus_type); static int sdw_drv_probe(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); const struct sdw_device_id *id; int ret; @@ -116,8 +116,7 @@ static int sdw_drv_probe(struct device *dev) static int sdw_drv_remove(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); - + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); int ret = 0; if (drv->remove) @@ -131,7 +130,7 @@ static int sdw_drv_remove(struct device *dev) static void sdw_drv_shutdown(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); - struct sdw_driver *drv = drv_to_sdw_slave_driver(dev->driver); + struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); if (drv->shutdown) drv->shutdown(slave); diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index abaa2127815254..7d4bc6a979bf6b 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,7 +6,7 @@ extern struct bus_type sdw_bus_type; -#define drv_to_sdw_slave_driver(_drv) \ +#define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver) #define sdw_register_slave_driver(drv) \ @@ -29,4 +29,5 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size); #define module_sdw_driver(__sdw_slave_driver) \ module_driver(__sdw_slave_driver, sdw_register_slave_driver, \ sdw_unregister_slave_driver) + #endif /* __SOUNDWIRE_TYPES_H */ From d7e35bdd66b4725d3b286f1adaf18c4201065a98 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 21:53:10 -0500 Subject: [PATCH 10/59] soundwire: bus_type: rename sdw_drv_ to sdw_slave_drv Before we add master driver support, make sure there is no ambiguity and no occurrences of sdw_drv_ functions. No functionality change. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus_type.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 2b2830b622faef..9a0fd3ee1014a7 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -67,7 +67,7 @@ struct bus_type sdw_bus_type = { }; EXPORT_SYMBOL_GPL(sdw_bus_type); -static int sdw_drv_probe(struct device *dev) +static int sdw_slave_drv_probe(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -113,7 +113,7 @@ static int sdw_drv_probe(struct device *dev) return 0; } -static int sdw_drv_remove(struct device *dev) +static int sdw_slave_drv_remove(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -127,7 +127,7 @@ static int sdw_drv_remove(struct device *dev) return ret; } -static void sdw_drv_shutdown(struct device *dev) +static void sdw_slave_drv_shutdown(struct device *dev) { struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_driver *drv = to_sdw_slave_driver(dev->driver); @@ -155,13 +155,13 @@ int __sdw_register_slave_driver(struct sdw_driver *drv, } drv->driver.owner = owner; - drv->driver.probe = sdw_drv_probe; + drv->driver.probe = sdw_slave_drv_probe; if (drv->remove) - drv->driver.remove = sdw_drv_remove; + drv->driver.remove = sdw_slave_drv_remove; if (drv->shutdown) - drv->driver.shutdown = sdw_drv_shutdown; + drv->driver.shutdown = sdw_slave_drv_shutdown; return driver_register(&drv->driver); } From 0d9bbdd7a490b79cf37afeb302e5067f1c5a8590 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 11 Sep 2019 14:26:53 -0500 Subject: [PATCH 11/59] soundwire: intel: rename res field as link_res There are too many fields called 'res' so add prefix to make it easier to track what the structures are. Pure rename, no functionality change Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 0371d3d5501a97..64f97bb1a13573 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -103,7 +103,7 @@ enum intel_pdi_type { struct sdw_intel { struct sdw_cdns cdns; int instance; - struct sdw_intel_link_res *res; + struct sdw_intel_link_res *link_res; #ifdef CONFIG_DEBUG_FS struct dentry *debugfs; #endif @@ -193,8 +193,8 @@ static ssize_t intel_sprintf(void __iomem *mem, bool l, static int intel_reg_show(struct seq_file *s_file, void *data) { struct sdw_intel *sdw = s_file->private; - void __iomem *s = sdw->res->shim; - void __iomem *a = sdw->res->alh; + void __iomem *s = sdw->link_res->shim; + void __iomem *a = sdw->link_res->alh; char *buf; ssize_t ret; int i, j; @@ -289,7 +289,7 @@ static void intel_debugfs_exit(struct sdw_intel *sdw) {} static int intel_link_power_up(struct sdw_intel *sdw) { unsigned int link_id = sdw->instance; - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int spa_mask, cpa_mask; int link_control, ret; @@ -309,7 +309,7 @@ static int intel_link_power_up(struct sdw_intel *sdw) static int intel_shim_init(struct sdw_intel *sdw) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int sync_reg, ret; u16 ioctl = 0, act = 0; @@ -370,7 +370,7 @@ static int intel_shim_init(struct sdw_intel *sdw) static void intel_pdi_init(struct sdw_intel *sdw, struct sdw_cdns_stream_config *config) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int pcm_cap, pdm_cap; @@ -404,7 +404,7 @@ static void intel_pdi_init(struct sdw_intel *sdw, static int intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int count; @@ -476,7 +476,7 @@ static int intel_pdi_ch_update(struct sdw_intel *sdw) static void intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) { - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; int pdi_conf = 0; @@ -508,7 +508,7 @@ intel_pdi_shim_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) static void intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi) { - void __iomem *alh = sdw->res->alh; + void __iomem *alh = sdw->link_res->alh; unsigned int link_id = sdw->instance; unsigned int conf; @@ -535,7 +535,7 @@ static int intel_params_stream(struct sdw_intel *sdw, struct snd_pcm_hw_params *hw_params, int link_id, int alh_stream_id) { - struct sdw_intel_link_res *res = sdw->res; + struct sdw_intel_link_res *res = sdw->link_res; struct sdw_intel_stream_params_data params_data; params_data.substream = substream; @@ -558,7 +558,7 @@ static int intel_pre_bank_switch(struct sdw_bus *bus) { struct sdw_cdns *cdns = bus_to_cdns(bus); struct sdw_intel *sdw = cdns_to_intel(cdns); - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int sync_reg; /* Write to register only for multi-link */ @@ -577,7 +577,7 @@ static int intel_post_bank_switch(struct sdw_bus *bus) { struct sdw_cdns *cdns = bus_to_cdns(bus); struct sdw_intel *sdw = cdns_to_intel(cdns); - void __iomem *shim = sdw->res->shim; + void __iomem *shim = sdw->link_res->shim; int sync_reg, ret; /* Write to register only for multi-link */ @@ -934,9 +934,9 @@ static int intel_probe(struct platform_device *pdev) return -ENOMEM; sdw->instance = pdev->id; - sdw->res = dev_get_platdata(&pdev->dev); + sdw->link_res = dev_get_platdata(&pdev->dev); sdw->cdns.dev = &pdev->dev; - sdw->cdns.registers = sdw->res->registers; + sdw->cdns.registers = sdw->link_res->registers; sdw->cdns.instance = sdw->instance; sdw->cdns.msg_count = 0; sdw->cdns.bus.dev = &pdev->dev; @@ -976,11 +976,12 @@ static int intel_probe(struct platform_device *pdev) intel_pdi_ch_update(sdw); /* Acquire IRQ */ - ret = request_threaded_irq(sdw->res->irq, sdw_cdns_irq, sdw_cdns_thread, + ret = request_threaded_irq(sdw->link_res->irq, + sdw_cdns_irq, sdw_cdns_thread, IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); if (ret < 0) { dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", - sdw->res->irq); + sdw->link_res->irq); goto err_init; } @@ -1010,7 +1011,7 @@ static int intel_probe(struct platform_device *pdev) err_interrupt: sdw_cdns_enable_interrupt(&sdw->cdns, false); - free_irq(sdw->res->irq, sdw); + free_irq(sdw->link_res->irq, sdw); err_init: sdw_delete_bus_master(&sdw->cdns.bus); return ret; @@ -1025,7 +1026,7 @@ static int intel_remove(struct platform_device *pdev) if (!sdw->cdns.bus.prop.hw_disabled) { intel_debugfs_exit(sdw); sdw_cdns_enable_interrupt(&sdw->cdns, false); - free_irq(sdw->res->irq, sdw); + free_irq(sdw->link_res->irq, sdw); snd_soc_unregister_component(sdw->cdns.dev); } sdw_delete_bus_master(&sdw->cdns.bus); From 2f1d819be128bc0fc01d1b985612ae00b522fd04 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 19:11:58 -0500 Subject: [PATCH 12/59] soundwire: add support for sdw_slave_type Currently the bus does not have any explicit support for master devices. First add explicit support for sdw_slave_type and error checks if this type is not set. In follow-up patches we can add support for the sdw_md_type (md==Master Device), following the Grey Bus example. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus_type.c | 16 ++++++++++++---- drivers/soundwire/slave.c | 7 ++++++- include/linux/soundwire/sdw_type.h | 6 ++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 9a0fd3ee1014a7..bbdedce5eb261a 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -49,13 +49,21 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size) static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) { - struct sdw_slave *slave = to_sdw_slave_device(dev); + struct sdw_slave *slave; char modalias[32]; - sdw_slave_modalias(slave, modalias, sizeof(modalias)); + if (is_sdw_slave(dev)) { + slave = to_sdw_slave_device(dev); + + sdw_slave_modalias(slave, modalias, sizeof(modalias)); - if (add_uevent_var(env, "MODALIAS=%s", modalias)) - return -ENOMEM; + if (add_uevent_var(env, "MODALIAS=%s", modalias)) + return -ENOMEM; + } else { + /* only Slave device type supported */ + dev_warn(dev, "uevent for unknown Soundwire type\n"); + return -EINVAL; + } return 0; } diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 48a513680db665..c87267f12a3b97 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -14,6 +14,11 @@ static void sdw_slave_release(struct device *dev) kfree(slave); } +struct device_type sdw_slave_type = { + .name = "sdw_slave", + .release = sdw_slave_release, +}; + static int sdw_slave_add(struct sdw_bus *bus, struct sdw_slave_id *id, struct fwnode_handle *fwnode) { @@ -41,9 +46,9 @@ static int sdw_slave_add(struct sdw_bus *bus, id->class_id, id->unique_id); } - slave->dev.release = sdw_slave_release; slave->dev.bus = &sdw_bus_type; slave->dev.of_node = of_node_get(to_of_node(fwnode)); + slave->dev.type = &sdw_slave_type; slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; slave->dev_num = 0; diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index 7d4bc6a979bf6b..c681b3426478e7 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -5,6 +5,12 @@ #define __SOUNDWIRE_TYPES_H extern struct bus_type sdw_bus_type; +extern struct device_type sdw_slave_type; + +static inline int is_sdw_slave(const struct device *dev) +{ + return dev->type == &sdw_slave_type; +} #define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver) From 24bb987cfb92ca2864baf2af6dfc9d72e864d858 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 14 Nov 2019 10:37:04 -0600 Subject: [PATCH 13/59] soundwire: slave: move uevent handling to slave Currently the code deals with uevents at the bus level, but we only care for Slave events Suggested-by: Vinod Koul Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.h | 2 ++ drivers/soundwire/bus_type.c | 3 +-- drivers/soundwire/slave.c | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index cb482da914da94..be01a5f3d00bae 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -6,6 +6,8 @@ #define DEFAULT_BANK_SWITCH_TIMEOUT 3000 +int sdw_uevent(struct device *dev, struct kobj_uevent_env *env); + #if IS_ENABLED(CONFIG_ACPI) int sdw_acpi_find_slaves(struct sdw_bus *bus); #else diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index bbdedce5eb261a..5c18c21545b585 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -47,7 +47,7 @@ int sdw_slave_modalias(const struct sdw_slave *slave, char *buf, size_t size) slave->id.mfg_id, slave->id.part_id); } -static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) +int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) { struct sdw_slave *slave; char modalias[32]; @@ -71,7 +71,6 @@ static int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) struct bus_type sdw_bus_type = { .name = "soundwire", .match = sdw_bus_match, - .uevent = sdw_uevent, }; EXPORT_SYMBOL_GPL(sdw_bus_type); diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index c87267f12a3b97..014c3ece1f1768 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -17,6 +17,7 @@ static void sdw_slave_release(struct device *dev) struct device_type sdw_slave_type = { .name = "sdw_slave", .release = sdw_slave_release, + .uevent = sdw_uevent, }; static int sdw_slave_add(struct sdw_bus *bus, From f8ba66ad58ea57da34dee78421946fcf0c76c6a2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 10 Sep 2019 19:23:22 -0500 Subject: [PATCH 14/59] soundwire: add initial definitions for sdw_master_device Since we want an explicit support for the SoundWire Master device, add the definitions, following the Grey Bus example. Unlike for the Slave device, we do not set a variable when dealing with the master uevent. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/Makefile | 2 +- drivers/soundwire/bus_type.c | 7 +++- drivers/soundwire/master.c | 62 ++++++++++++++++++++++++++++++ include/linux/soundwire/sdw.h | 35 +++++++++++++++++ include/linux/soundwire/sdw_type.h | 9 +++++ 5 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 drivers/soundwire/master.c diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 563894e5ecaf50..89b29819dd3ad8 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -4,7 +4,7 @@ # #Bus Objs -soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o +soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o ifdef CONFIG_DEBUG_FS diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index 5c18c21545b585..df1271f6db6142 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -59,9 +59,12 @@ int sdw_uevent(struct device *dev, struct kobj_uevent_env *env) if (add_uevent_var(env, "MODALIAS=%s", modalias)) return -ENOMEM; + } else if (is_sdw_md(dev)) { + /* this should not happen but throw an error */ + dev_warn(dev, "uevent for Master device, unsupported\n"); + return -EINVAL; } else { - /* only Slave device type supported */ - dev_warn(dev, "uevent for unknown Soundwire type\n"); + dev_warn(dev, "uevent for unknown device\n"); return -EINVAL; } diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c new file mode 100644 index 00000000000000..6210098c892bad --- /dev/null +++ b/drivers/soundwire/master.c @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright(c) 2019 Intel Corporation. + +#include +#include +#include +#include +#include "bus.h" + +static void sdw_md_release(struct device *dev) +{ + struct sdw_master_device *md = to_sdw_master_device(dev); + + kfree(md); +} + +struct device_type sdw_md_type = { + .name = "soundwire_master", + .release = sdw_md_release, +}; + +struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, + struct device *parent, + struct fwnode_handle *fwnode, + int link_id) +{ + struct sdw_master_device *md; + int ret; + + if (!driver->probe) { + dev_err(parent, "mandatory probe callback missing\n"); + return ERR_PTR(-EINVAL); + } + + md = kzalloc(sizeof(*md), GFP_KERNEL); + if (!md) + return ERR_PTR(-ENOMEM); + + md->link_id = link_id; + + md->driver = driver; + + md->dev.parent = parent; + md->dev.fwnode = fwnode; + md->dev.bus = &sdw_bus_type; + md->dev.type = &sdw_md_type; + md->dev.dma_mask = md->dev.parent->dma_mask; + dev_set_name(&md->dev, "sdw-master-%d", md->link_id); + + ret = device_register(&md->dev); + if (ret) { + dev_err(parent, "Failed to add master: ret %d\n", ret); + /* + * On err, don't free but drop ref as this will be freed + * when release method is invoked. + */ + put_device(&md->dev); + } + + return md; +} +EXPORT_SYMBOL(sdw_md_add); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 5b1180f1e6b558..af0a72e7afdf4a 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -585,6 +585,16 @@ struct sdw_slave { #define to_sdw_slave_device(d) \ container_of(d, struct sdw_slave, dev) +struct sdw_master_device { + struct device dev; + int link_id; + struct sdw_md_driver *driver; + void *pdata; /* core does not touch */ +}; + +#define to_sdw_master_device(d) \ + container_of(d, struct sdw_master_device, dev) + struct sdw_driver { const char *name; @@ -599,6 +609,26 @@ struct sdw_driver { struct device_driver driver; }; +struct sdw_md_driver { + /* initializations and allocations */ + int (*probe)(struct sdw_master_device *md, void *link_ctx); + /* hardware enablement, all clock/power dependencies are available */ + int (*startup)(struct sdw_master_device *md); + /* hardware disabled */ + int (*shutdown)(struct sdw_master_device *md); + /* free all resources */ + int (*remove)(struct sdw_master_device *md); + /* + * enable/disable driver control while in clock-stop mode, + * typically in always-on/D0ix modes. When the driver yields + * control, another entity in the system (typically firmware + * running on an always-on microprocessor) is responsible to + * tracking Slave-initiated wakes + */ + int (*autonomous_clock_stop_enable)(struct sdw_master_device *md, + bool state); +}; + #define SDW_SLAVE_ENTRY(_mfg_id, _part_id, _drv_data) \ { .mfg_id = (_mfg_id), .part_id = (_part_id), \ .driver_data = (unsigned long)(_drv_data) } @@ -788,6 +818,11 @@ struct sdw_bus { int sdw_add_bus_master(struct sdw_bus *bus); void sdw_delete_bus_master(struct sdw_bus *bus); +struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, + struct device *parent, + struct fwnode_handle *fwnode, + int link_id); + /** * sdw_port_config: Master or Slave Port configuration * diff --git a/include/linux/soundwire/sdw_type.h b/include/linux/soundwire/sdw_type.h index c681b3426478e7..463d6d018d568d 100644 --- a/include/linux/soundwire/sdw_type.h +++ b/include/linux/soundwire/sdw_type.h @@ -6,15 +6,24 @@ extern struct bus_type sdw_bus_type; extern struct device_type sdw_slave_type; +extern struct device_type sdw_md_type; static inline int is_sdw_slave(const struct device *dev) { return dev->type == &sdw_slave_type; } +static inline int is_sdw_md(const struct device *dev) +{ + return dev->type == &sdw_md_type; +} + #define to_sdw_slave_driver(_drv) \ container_of(_drv, struct sdw_driver, driver) +#define to_sdw_md_driver(_drv) \ + container_of(_drv, struct sdw_md_driver, driver) + #define sdw_register_slave_driver(drv) \ __sdw_register_slave_driver(drv, THIS_MODULE) From e72aeeaeb6e3f233571d7bc5311757163c58dfbb Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 23 Oct 2019 12:12:51 -0500 Subject: [PATCH 15/59] soundwire: intel: remove platform devices and provide new interface Use sdw_master_device and driver instead of platform devices To quote GregKH: "Don't mess with a platform device unless you really have no other possible choice. And even then, don't do it and try to do something else. Platform devices are really abused, don't perpetuate it " In addition, rather than a plain-vanilla init/exit, this patch provides 3 steps in the initialization (ACPI scan, probe, startup) which make it easier to verify support and allocate required resources as early as possible, and conversely help make the startup lighter-weight with only hardware register setup. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 92 ++++++----- drivers/soundwire/intel.h | 8 +- drivers/soundwire/intel_init.c | 276 ++++++++++++++++++++++++--------- 3 files changed, 268 insertions(+), 108 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 64f97bb1a13573..ba3bc410d816fc 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -92,8 +92,6 @@ #define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0) #define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16) -#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) - enum intel_pdi_type { INTEL_PDI_IN = 0, INTEL_PDI_OUT = 1, @@ -923,24 +921,23 @@ static int intel_init(struct sdw_intel *sdw) /* * probe and init */ -static int intel_probe(struct platform_device *pdev) +static int intel_master_probe(struct sdw_master_device *md, void *link_ctx) { - struct sdw_cdns_stream_config config; struct sdw_intel *sdw; int ret; - sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL); + sdw = devm_kzalloc(&md->dev, sizeof(*sdw), GFP_KERNEL); if (!sdw) return -ENOMEM; - sdw->instance = pdev->id; - sdw->link_res = dev_get_platdata(&pdev->dev); - sdw->cdns.dev = &pdev->dev; + sdw->instance = md->link_id; + sdw->link_res = link_ctx; + sdw->cdns.dev = &md->dev; sdw->cdns.registers = sdw->link_res->registers; - sdw->cdns.instance = sdw->instance; + sdw->cdns.instance = md->link_id; sdw->cdns.msg_count = 0; - sdw->cdns.bus.dev = &pdev->dev; - sdw->cdns.bus.link_id = pdev->id; + sdw->cdns.bus.dev = &md->dev; + sdw->cdns.bus.link_id = md->link_id; sdw_cdns_probe(&sdw->cdns); @@ -948,16 +945,50 @@ static int intel_probe(struct platform_device *pdev) sdw_intel_ops.read_prop = intel_prop_read; sdw->cdns.bus.ops = &sdw_intel_ops; - platform_set_drvdata(pdev, sdw); + md->pdata = sdw; + + /* set driver data, accessed by snd_soc_dai_set_drvdata() */ + dev_set_drvdata(&md->dev, &sdw->cdns); ret = sdw_add_bus_master(&sdw->cdns.bus); if (ret) { - dev_err(&pdev->dev, "sdw_add_bus_master fail: %d\n", ret); + dev_err(&md->dev, "sdw_add_bus_master fail: %d\n", ret); return ret; } if (sdw->cdns.bus.prop.hw_disabled) { - dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n", + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", + sdw->cdns.bus.link_id); + return 0; + } + + /* Acquire IRQ */ + ret = request_threaded_irq(sdw->link_res->irq, + sdw_cdns_irq, sdw_cdns_thread, + IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); + if (ret < 0) { + dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", + sdw->link_res->irq); + goto err_init; + } + + return 0; + +err_init: + sdw_delete_bus_master(&sdw->cdns.bus); + return ret; +} + +static int intel_master_startup(struct sdw_master_device *md) +{ + struct sdw_cdns_stream_config config; + struct sdw_intel *sdw; + int ret; + + sdw = md->pdata; + + if (sdw->cdns.bus.prop.hw_disabled) { + dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", sdw->cdns.bus.link_id); return 0; } @@ -975,16 +1006,6 @@ static int intel_probe(struct platform_device *pdev) intel_pdi_ch_update(sdw); - /* Acquire IRQ */ - ret = request_threaded_irq(sdw->link_res->irq, - sdw_cdns_irq, sdw_cdns_thread, - IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); - if (ret < 0) { - dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", - sdw->link_res->irq); - goto err_init; - } - ret = sdw_cdns_enable_interrupt(&sdw->cdns, true); if (ret < 0) { dev_err(sdw->cdns.dev, "cannot enable interrupts\n"); @@ -1011,17 +1032,17 @@ static int intel_probe(struct platform_device *pdev) err_interrupt: sdw_cdns_enable_interrupt(&sdw->cdns, false); - free_irq(sdw->link_res->irq, sdw); err_init: + free_irq(sdw->link_res->irq, sdw); sdw_delete_bus_master(&sdw->cdns.bus); return ret; } -static int intel_remove(struct platform_device *pdev) +static int intel_master_remove(struct sdw_master_device *md) { struct sdw_intel *sdw; - sdw = platform_get_drvdata(pdev); + sdw = md->pdata; if (!sdw->cdns.bus.prop.hw_disabled) { intel_debugfs_exit(sdw); @@ -1031,19 +1052,18 @@ static int intel_remove(struct platform_device *pdev) } sdw_delete_bus_master(&sdw->cdns.bus); + md->dev.driver = NULL; + device_unregister(&md->dev); + return 0; } -static struct platform_driver sdw_intel_drv = { - .probe = intel_probe, - .remove = intel_remove, - .driver = { - .name = "int-sdw", - - }, +struct sdw_md_driver intel_sdw_driver = { + .probe = intel_master_probe, + .startup = intel_master_startup, + .remove = intel_master_remove, }; - -module_platform_driver(sdw_intel_drv); +EXPORT_SYMBOL(intel_sdw_driver); MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("platform:int-sdw"); diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index 38b7c125fb1052..cfab2f00214d25 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -7,7 +7,7 @@ /** * struct sdw_intel_link_res - Soundwire Intel link resource structure, * typically populated by the controller driver. - * @pdev: platform_device + * @md: master device * @mmio_base: mmio base of SoundWire registers * @registers: Link IO registers base * @shim: Audio shim pointer @@ -17,7 +17,7 @@ * @dev: device implementing hw_params and free callbacks */ struct sdw_intel_link_res { - struct platform_device *pdev; + struct sdw_master_device *md; void __iomem *mmio_base; /* not strictly needed, useful for debug */ void __iomem *registers; void __iomem *shim; @@ -27,4 +27,8 @@ struct sdw_intel_link_res { struct device *dev; }; +#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) + +extern struct sdw_md_driver intel_sdw_driver; + #endif /* __SDW_INTEL_LOCAL_H */ diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 6bc167c83b475e..42f7ae034beabf 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include "intel.h" @@ -23,22 +23,47 @@ #define SDW_LINK_BASE 0x30000 #define SDW_LINK_SIZE 0x10000 -static int link_mask; -module_param_named(sdw_link_mask, link_mask, int, 0444); +static int ctrl_link_mask; +module_param_named(sdw_link_mask, ctrl_link_mask, int, 0444); MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)"); -static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx) +static bool is_link_enabled(struct fwnode_handle *fw_node, int i) +{ + struct fwnode_handle *link; + char name[32]; + u32 quirk_mask = 0; + + /* Find master handle */ + snprintf(name, sizeof(name), + "mipi-sdw-link-%d-subproperties", i); + + link = fwnode_get_named_child_node(fw_node, name); + if (!link) + return false; + + fwnode_property_read_u32(link, + "intel-quirk-mask", + &quirk_mask); + + if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE) + return false; + + return true; +} + +static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx) { struct sdw_intel_link_res *link = ctx->links; + struct sdw_master_device *md; int i; if (!link) return 0; - for (i = 0; i < ctx->count; i++) { - if (link->pdev) - platform_device_unregister(link->pdev); - link++; + for (i = 0; i < ctx->count; i++, link++) { + md = link->md; + if (md) + md->driver->remove(md); } kfree(ctx->links); @@ -47,112 +72,194 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx) return 0; } -static struct sdw_intel_ctx -*sdw_intel_add_controller(struct sdw_intel_res *res) +static int +sdw_intel_scan_controller(struct sdw_intel_acpi_info *info) { - struct platform_device_info pdevinfo; - struct platform_device *pdev; - struct sdw_intel_link_res *link; - struct sdw_intel_ctx *ctx; struct acpi_device *adev; int ret, i; u8 count; - u32 caps; - if (acpi_bus_get_device(res->handle, &adev)) - return NULL; + if (acpi_bus_get_device(info->handle, &adev)) + return -EINVAL; /* Found controller, find links supported */ count = 0; ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev), "mipi-sdw-master-count", &count, 1); - /* Don't fail on error, continue and use hw value */ + /* + * In theory we could check the number of links supported in + * hardware, but in that step we cannot assume SoundWire IP is + * powered. + * + * In addition, if the BIOS doesn't even provide this + * 'master-count' property then all the inits based on link + * masks will fail as well. + * + * We will check the hardware capabilities in the startup() step + */ + if (ret) { dev_err(&adev->dev, "Failed to read mipi-sdw-master-count: %d\n", ret); - count = SDW_MAX_LINKS; + return -EINVAL; } - /* Check SNDWLCAP.LCOUNT */ - caps = ioread32(res->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP); - caps &= GENMASK(2, 0); - - /* Check HW supported vs property value and use min of two */ - count = min_t(u8, caps, count); - /* Check count is within bounds */ if (count > SDW_MAX_LINKS) { dev_err(&adev->dev, "Link count %d exceeds max %d\n", count, SDW_MAX_LINKS); - return NULL; + return -EINVAL; } else if (!count) { dev_warn(&adev->dev, "No SoundWire links detected\n"); - return NULL; + return -EINVAL; } + dev_dbg(&adev->dev, "ACPI reports %d SDW Link devices\n", count); + + info->count = count; + for (i = 0; i < count; i++) { + if (ctrl_link_mask && !(ctrl_link_mask & BIT(i))) { + dev_dbg(&adev->dev, + "Link %d masked, will not be enabled\n", i); + continue; + } + + if (!is_link_enabled(acpi_fwnode_handle(adev), i)) { + dev_dbg(&adev->dev, + "Link %d not selected in firmware\n", i); + continue; + } + + info->link_mask |= BIT(i); + } + + return 0; +} + +static struct sdw_intel_ctx +*sdw_intel_probe_controller(struct sdw_intel_res *res) +{ + struct sdw_intel_link_res *link; + struct sdw_intel_ctx *ctx; + struct acpi_device *adev; + struct sdw_master_device *md; + u32 link_mask; + int count; + int i; + + if (!res) + return NULL; + + if (acpi_bus_get_device(res->handle, &adev)) + return NULL; + + if (!res->count) + return NULL; + + count = res->count; dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count); ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return NULL; - ctx->count = count; - ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL); + ctx->links = kcalloc(count, sizeof(*ctx->links), GFP_KERNEL); if (!ctx->links) goto link_err; + ctx->count = count; + ctx->mmio_base = res->mmio_base; + ctx->link_mask = res->link_mask; + ctx->handle = res->handle; + link = ctx->links; + link_mask = ctx->link_mask; /* Create SDW Master devices */ - for (i = 0; i < count; i++) { - if (link_mask && !(link_mask & BIT(i))) { - dev_dbg(&adev->dev, - "Link %d masked, will not be enabled\n", i); - link++; + for (i = 0; i < count; i++, link++) { + if (link_mask && !(link_mask & BIT(i))) continue; - } + md = sdw_md_add(&intel_sdw_driver, + res->parent, + acpi_fwnode_handle(adev), + i); + + if (IS_ERR(md)) { + dev_err(&adev->dev, "Could not create link %d\n", i); + goto err; + } + link->md = md; + link->mmio_base = res->mmio_base; link->registers = res->mmio_base + SDW_LINK_BASE - + (SDW_LINK_SIZE * i); + + (SDW_LINK_SIZE * i); link->shim = res->mmio_base + SDW_SHIM_BASE; link->alh = res->mmio_base + SDW_ALH_BASE; - + link->irq = res->irq; link->ops = res->ops; link->dev = res->dev; - memset(&pdevinfo, 0, sizeof(pdevinfo)); - - pdevinfo.parent = res->parent; - pdevinfo.name = "int-sdw"; - pdevinfo.id = i; - pdevinfo.fwnode = acpi_fwnode_handle(adev); - - pdev = platform_device_register_full(&pdevinfo); - if (IS_ERR(pdev)) { - dev_err(&adev->dev, - "platform device creation failed: %ld\n", - PTR_ERR(pdev)); - goto pdev_err; - } - - link->pdev = pdev; - link++; + /* let the SoundWire master driver to its probe */ + md->driver->probe(md, link); } return ctx; -pdev_err: - sdw_intel_cleanup_pdev(ctx); +err: + sdw_intel_cleanup(ctx); link_err: kfree(ctx); return NULL; } +static int +sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) +{ + struct acpi_device *adev; + struct sdw_intel_link_res *link; + struct sdw_master_device *md; + u32 caps; + u32 link_mask; + int i; + + if (acpi_bus_get_device(ctx->handle, &adev)) + return -EINVAL; + + /* Check SNDWLCAP.LCOUNT */ + caps = ioread32(ctx->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP); + caps &= GENMASK(2, 0); + + /* Check HW supported vs property value */ + if (caps < ctx->count) { + dev_err(&adev->dev, + "BIOS master count is larger than hardware capabilities\n"); + return -EINVAL; + } + + if (!ctx->links) + return -EINVAL; + + link = ctx->links; + link_mask = ctx->link_mask; + + /* Create SDW Master devices */ + for (i = 0; i < ctx->count; i++, link++) { + if (link_mask && !(link_mask & BIT(i))) + continue; + + md = link->md; + + md->driver->startup(md); + } + + return 0; +} + static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, void *cdata, void **return_value) { - struct sdw_intel_res *res = cdata; + struct sdw_intel_acpi_info *info = cdata; struct acpi_device *adev; acpi_status status; u64 adr; @@ -166,7 +273,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, return AE_NOT_FOUND; } - res->handle = handle; + info->handle = handle; /* * On some Intel platforms, multiple children of the HDAS @@ -183,37 +290,66 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, } /** - * sdw_intel_init() - SoundWire Intel init routine + * sdw_intel_acpi_scan() - SoundWire Intel init routine * @parent_handle: ACPI parent handle - * @res: resource data + * @info: description of what firmware/DSDT tables expose * - * This scans the namespace and creates SoundWire link controller devices - * based on the info queried. + * This scans the namespace and queries firmware to figure out which + * links to enable. A follow-up use of sdw_intel_probe() and + * sdw_intel_startup() is required for creation of devices and bus + * startup */ -void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res) +int sdw_intel_acpi_scan(acpi_handle *parent_handle, + struct sdw_intel_acpi_info *info) { acpi_status status; status = acpi_walk_namespace(ACPI_TYPE_DEVICE, parent_handle, 1, sdw_intel_acpi_cb, - NULL, res, NULL); + NULL, info, NULL); if (ACPI_FAILURE(status)) - return NULL; + return -ENODEV; - return sdw_intel_add_controller(res); + return sdw_intel_scan_controller(info); } -EXPORT_SYMBOL(sdw_intel_init); +EXPORT_SYMBOL(sdw_intel_acpi_scan); +/** + * sdw_intel_probe() - SoundWire Intel probe routine + * @parent_handle: ACPI parent handle + * @res: resource data + * + * This creates SoundWire Master and Slave devices below the controller. + * All the information necessary is stored in the context, and the res + * argument pointer can be freed after this step. + */ +struct sdw_intel_ctx +*sdw_intel_probe(struct sdw_intel_res *res) +{ + return sdw_intel_probe_controller(res); +} +EXPORT_SYMBOL(sdw_intel_probe); + +/** + * sdw_intel_startup() - SoundWire Intel startup + * @ctx: SoundWire context allocated in the probe + * + */ +int sdw_intel_startup(struct sdw_intel_ctx *ctx) +{ + return sdw_intel_startup_controller(ctx); +} +EXPORT_SYMBOL(sdw_intel_startup); /** * sdw_intel_exit() - SoundWire Intel exit - * @arg: callback context + * @ctx: SoundWire context allocated in the probe * * Delete the controller instances created and cleanup */ void sdw_intel_exit(struct sdw_intel_ctx *ctx) { - sdw_intel_cleanup_pdev(ctx); + sdw_intel_cleanup(ctx); kfree(ctx); } EXPORT_SYMBOL(sdw_intel_exit); From 5de3bff19dd7df06d045cad5adf122d6a6cc4a6b Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 12 Sep 2019 17:35:48 +0800 Subject: [PATCH 16/59] soundwire: add device driver to sdw_md_driver Setting an device driver is necessary for ASoC to register DAI components. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 4 ++++ drivers/soundwire/master.c | 2 ++ include/linux/soundwire/sdw.h | 1 + 3 files changed, 7 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index ba3bc410d816fc..492684ba08c324 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1059,6 +1059,10 @@ static int intel_master_remove(struct sdw_master_device *md) } struct sdw_md_driver intel_sdw_driver = { + .driver = { + .name = "intel-sdw", + .owner = THIS_MODULE, + }, .probe = intel_master_probe, .startup = intel_master_startup, .remove = intel_master_remove, diff --git a/drivers/soundwire/master.c b/drivers/soundwire/master.c index 6210098c892bad..d23111d43fd6f3 100644 --- a/drivers/soundwire/master.c +++ b/drivers/soundwire/master.c @@ -57,6 +57,8 @@ struct sdw_master_device *sdw_md_add(struct sdw_md_driver *driver, put_device(&md->dev); } + md->dev.driver = &driver->driver; + return md; } EXPORT_SYMBOL(sdw_md_add); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index af0a72e7afdf4a..d22950b1a5d929 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -627,6 +627,7 @@ struct sdw_md_driver { */ int (*autonomous_clock_stop_enable)(struct sdw_master_device *md, bool state); + struct device_driver driver; }; #define SDW_SLAVE_ENTRY(_mfg_id, _part_id, _drv_data) \ From fa6e999d748e246823b21c7ea5bb3fab461c0e3d Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 19 Aug 2019 16:43:06 +0800 Subject: [PATCH 17/59] soundwire: intel: add prepare support in sdw dai driver The existing code does not expose a prepare operation, which is very much needed to deal with underflow and resume operations. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 492684ba08c324..c50aebda7f4d52 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -697,6 +697,21 @@ static int intel_hw_params(struct snd_pcm_substream *substream, return ret; } +static int intel_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct sdw_cdns_dma_data *dma; + + 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; + } + + return sdw_prepare_stream(dma->stream); +} + static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -743,6 +758,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai, static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .hw_params = intel_hw_params, + .prepare = intel_prepare, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pcm_set_sdw_stream, @@ -750,6 +766,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .hw_params = intel_hw_params, + .prepare = intel_prepare, .hw_free = intel_hw_free, .shutdown = intel_shutdown, .set_sdw_stream = intel_pdm_set_sdw_stream, From 78bbe93f85e4444c5f433e08f86ba7c156eec9d9 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 19 Aug 2019 16:48:46 +0800 Subject: [PATCH 18/59] soundwire: intel: add trigger support in sdw dai driver The existing code does not expose a trigger callback, which is very much required for streaming. The SoundWire stream is enabled and disabled in trigger function. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index c50aebda7f4d52..862dde9a9b869e 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -712,6 +712,43 @@ static int intel_prepare(struct snd_pcm_substream *substream, return sdw_prepare_stream(dma->stream); } +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) { @@ -759,6 +796,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai, static const struct snd_soc_dai_ops intel_pcm_dai_ops = { .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, @@ -767,6 +805,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { static const struct snd_soc_dai_ops intel_pdm_dai_ops = { .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, From 19ae232c20886a2247b9005defa9baa3ee58e1d1 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Mon, 19 Aug 2019 16:55:01 +0800 Subject: [PATCH 19/59] soundwire: intel: add sdw_stream_setup helper for .startup callback The sdw stream is allocated and stored in dai to share the sdw runtime information. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 65 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 862dde9a9b869e..af24fa048addca 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -615,6 +615,69 @@ static int intel_post_bank_switch(struct sdw_bus *bus) * DAI routines */ +static int sdw_stream_setup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct sdw_stream_runtime *sdw_stream = NULL; + char *name; + int i, ret; + + name = kzalloc(32, GFP_KERNEL); + if (!name) + return -ENOMEM; + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + snprintf(name, 32, "%s-Playback", dai->name); + else + snprintf(name, 32, "%s-Capture", dai->name); + + sdw_stream = sdw_alloc_stream(name); + if (!sdw_stream) { + dev_err(dai->dev, "alloc stream failed for DAI %s", dai->name); + ret = -ENOMEM; + goto error; + } + + /* Set stream pointer on CPU DAI */ + ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream); + if (ret < 0) { + dev_err(dai->dev, "failed to set stream pointer on cpu dai %s", + dai->name); + goto release_stream; + } + + /* Set stream pointer on all CODEC DAIs */ + for (i = 0; i < rtd->num_codecs; i++) { + ret = snd_soc_dai_set_sdw_stream(rtd->codec_dais[i], sdw_stream, + substream->stream); + if (ret < 0) { + dev_err(dai->dev, "failed to set stream pointer on codec dai %s", + rtd->codec_dais[i]->name); + goto release_stream; + } + } + + return 0; + +release_stream: + sdw_release_stream(sdw_stream); +error: + kfree(name); + return ret; +} + +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' + */ + + return sdw_stream_setup(substream, dai); +} + static int intel_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -794,6 +857,7 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai, } 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, @@ -803,6 +867,7 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = { }; 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, From d060b910bb83438b0009e15e402ab1779fdffab8 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 23 Oct 2019 14:05:33 -0500 Subject: [PATCH 20/59] soundwire: intel: free all resources on hw_free() Make sure all calls to the SoundWire stream API are done and involve callback Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 40 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index af24fa048addca..cad1c0b64ee3ed 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -548,6 +548,25 @@ static int intel_params_stream(struct sdw_intel *sdw, return -EIO; } +static int intel_free_stream(struct sdw_intel *sdw, + struct snd_pcm_substream *substream, + struct snd_soc_dai *dai, + int link_id) +{ + struct sdw_intel_link_res *res = sdw->link_res; + struct sdw_intel_stream_free_data free_data; + + free_data.substream = substream; + free_data.dai = dai; + free_data.link_id = link_id; + + if (res->ops && res->ops->free_stream && res->dev) + return res->ops->free_stream(res->dev, + &free_data); + + return 0; +} + /* * bank switch routines */ @@ -816,6 +835,7 @@ static int intel_hw_free(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 ret; @@ -823,12 +843,28 @@ 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; + } + ret = sdw_stream_remove_master(&cdns->bus, dma->stream); - if (ret < 0) + if (ret < 0) { dev_err(dai->dev, "remove master from stream %s failed: %d\n", dma->stream->name, ret); + return ret; + } - return ret; + ret = intel_free_stream(sdw, substream, dai, sdw->instance); + if (ret < 0) { + dev_err(dai->dev, "intel_free_stream: failed %d", ret); + return ret; + } + + sdw_release_stream(dma->stream); + + return 0; } static void intel_shutdown(struct snd_pcm_substream *substream, From 1bf477d3d520d3eda1f8c0c62ff794a8a2c83ad0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 23 Oct 2019 14:38:00 -0500 Subject: [PATCH 21/59] soundwire: intel_init: add implementation of sdw_intel_enable_irq() This function is required to enable all interrupts across all links. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel_init.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 42f7ae034beabf..14ffe9ce29292c 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -137,6 +137,30 @@ sdw_intel_scan_controller(struct sdw_intel_acpi_info *info) return 0; } +#define HDA_DSP_REG_ADSPIC2 (0x10) +#define HDA_DSP_REG_ADSPIS2 (0x14) +#define HDA_DSP_REG_ADSPIC2_SNDW BIT(5) + +/** + * sdw_intel_enable_irq() - enable/disable Intel SoundWire IRQ + * @mmio_base: The mmio base of the control register + * @enable: true if enable + */ +void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable) +{ + u32 val; + + val = readl(mmio_base + HDA_DSP_REG_ADSPIC2); + + if (enable) + val |= HDA_DSP_REG_ADSPIC2_SNDW; + else + val &= ~HDA_DSP_REG_ADSPIC2_SNDW; + + writel(val, mmio_base + HDA_DSP_REG_ADSPIC2); +} +EXPORT_SYMBOL(sdw_intel_enable_irq); + static struct sdw_intel_ctx *sdw_intel_probe_controller(struct sdw_intel_res *res) { From 6e8ff74f8e91fe7de455e3dd6ef44a62cb235b98 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 24 Sep 2019 16:16:46 +0800 Subject: [PATCH 22/59] soundwire: intel/cadence: fix timeouts in MSI mode The existing code uses one pair of interrupt handler/thread per link but at the hardware level the interrupt is shared. This works fine for legacy PCI interrupts, but leads to timeouts in MSI (Message-Signaled Interrupt) mode, likely due to edges being lost. This patch unifies interrupt handling for all links with a single threaded IRQ handler. The handler is simplified to the bare minimum of detecting a SoundWire interrupt, and the thread takes care of dealing with interrupt sources. This partition follows the model used for the SOF IPC on HDaudio platforms, where similar timeout issues were noticed and doing all the interrupt handling/clearing in the thread improved reliability/stability. Validation results with 4 links active in parallel show a night-and-day improvement with no timeouts noticed even during stress tests. Latency and quality of service are not affected by the change - mostly because events on a SoundWire link are throttled by the bus frame rate (typically 8..48kHz). Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 17 ++++++----- drivers/soundwire/cadence_master.h | 4 +++ drivers/soundwire/intel.c | 27 ++++------------- drivers/soundwire/intel.h | 2 ++ drivers/soundwire/intel_init.c | 45 ++++++++++++++++++++++++++++- include/linux/soundwire/sdw_intel.h | 4 +++ 6 files changed, 69 insertions(+), 30 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index fed21e2b22774d..362fb6e49bfe09 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "bus.h" #include "cadence_master.h" @@ -745,7 +746,7 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id) CDNS_MCP_INT_SLAVE_MASK, 0); int_status &= ~CDNS_MCP_INT_SLAVE_MASK; - ret = IRQ_WAKE_THREAD; + schedule_work(&cdns->work); } cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status); @@ -754,13 +755,14 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id) EXPORT_SYMBOL(sdw_cdns_irq); /** - * sdw_cdns_thread() - Cadence irq thread handler - * @irq: irq number - * @dev_id: irq context + * To update slave status in a work since we will need to handle + * other interrupts eg. CDNS_MCP_INT_RX_WL during the update slave + * process. */ -irqreturn_t sdw_cdns_thread(int irq, void *dev_id) +static void cdns_update_slave_status_work(struct work_struct *work) { - struct sdw_cdns *cdns = dev_id; + struct sdw_cdns *cdns = + container_of(work, struct sdw_cdns, work); u32 slave0, slave1; dev_dbg_ratelimited(cdns->dev, "Slave status change\n"); @@ -777,9 +779,7 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id) cdns_updatel(cdns, CDNS_MCP_INTMASK, CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK); - return IRQ_HANDLED; } -EXPORT_SYMBOL(sdw_cdns_thread); /* * init routines @@ -1187,6 +1187,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns) init_completion(&cdns->tx_complete); cdns->bus.port_ops = &cdns_port_ops; + INIT_WORK(&cdns->work, cdns_update_slave_status_work); return 0; } EXPORT_SYMBOL(sdw_cdns_probe); diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 001457cbe5ad2e..0f108fd31fd943 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -126,6 +126,10 @@ struct sdw_cdns { bool link_up; unsigned int msg_count; + + struct work_struct work; + + struct list_head list; }; #define bus_to_cdns(_bus) container_of(_bus, struct sdw_cdns, bus) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index cad1c0b64ee3ed..23c5c3f9d30dce 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1095,6 +1095,7 @@ static int intel_master_probe(struct sdw_master_device *md, void *link_ctx) sdw->cdns.msg_count = 0; sdw->cdns.bus.dev = &md->dev; sdw->cdns.bus.link_id = md->link_id; + sdw->link_res->cdns = &sdw->cdns; sdw_cdns_probe(&sdw->cdns); @@ -1113,27 +1114,12 @@ static int intel_master_probe(struct sdw_master_device *md, void *link_ctx) return ret; } - if (sdw->cdns.bus.prop.hw_disabled) { - dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", + if (sdw->cdns.bus.prop.hw_disabled) + dev_info(&md->dev, + "SoundWire master %d is disabled, will be ignored\n", sdw->cdns.bus.link_id); - return 0; - } - - /* Acquire IRQ */ - ret = request_threaded_irq(sdw->link_res->irq, - sdw_cdns_irq, sdw_cdns_thread, - IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns); - if (ret < 0) { - dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n", - sdw->link_res->irq); - goto err_init; - } return 0; - -err_init: - sdw_delete_bus_master(&sdw->cdns.bus); - return ret; } static int intel_master_startup(struct sdw_master_device *md) @@ -1145,7 +1131,8 @@ static int intel_master_startup(struct sdw_master_device *md) sdw = md->pdata; if (sdw->cdns.bus.prop.hw_disabled) { - dev_info(&md->dev, "SoundWire master %d is disabled, ignoring\n", + dev_info(&md->dev, + "SoundWire master %d is disabled, ignoring\n", sdw->cdns.bus.link_id); return 0; } @@ -1190,7 +1177,6 @@ static int intel_master_startup(struct sdw_master_device *md) err_interrupt: sdw_cdns_enable_interrupt(&sdw->cdns, false); err_init: - free_irq(sdw->link_res->irq, sdw); sdw_delete_bus_master(&sdw->cdns.bus); return ret; } @@ -1204,7 +1190,6 @@ static int intel_master_remove(struct sdw_master_device *md) if (!sdw->cdns.bus.prop.hw_disabled) { intel_debugfs_exit(sdw); sdw_cdns_enable_interrupt(&sdw->cdns, false); - free_irq(sdw->link_res->irq, sdw); snd_soc_unregister_component(sdw->cdns.dev); } sdw_delete_bus_master(&sdw->cdns.bus); diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index cfab2f00214d25..2ae4fa7486861d 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -25,6 +25,8 @@ struct sdw_intel_link_res { int irq; const struct sdw_intel_ops *ops; struct device *dev; + struct sdw_cdns *cdns; + struct list_head list; }; #define SDW_INTEL_QUIRK_MASK_BUS_DISABLE BIT(1) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 14ffe9ce29292c..ed2a83989c72ff 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -11,8 +11,10 @@ #include #include #include +#include #include #include +#include "cadence_master.h" #include "intel.h" #define SDW_LINK_TYPE 4 /* from Intel ACPI documentation */ @@ -161,6 +163,32 @@ void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable) } EXPORT_SYMBOL(sdw_intel_enable_irq); +static irqreturn_t sdw_intel_irq(int irq, void *dev_id) +{ + struct sdw_intel_ctx *ctx = dev_id; + u32 int_status; + + int_status = readl(ctx->mmio_base + HDA_DSP_REG_ADSPIS2); + if (int_status & HDA_DSP_REG_ADSPIC2_SNDW) { + sdw_intel_enable_irq(ctx->mmio_base, false); + return IRQ_WAKE_THREAD; + } + + return IRQ_NONE; +} + +static irqreturn_t sdw_intel_thread(int irq, void *dev_id) +{ + struct sdw_intel_ctx *ctx = dev_id; + struct sdw_intel_link_res *link; + + list_for_each_entry(link, &ctx->link_list, list) + sdw_cdns_irq(irq, link->cdns); + + sdw_intel_enable_irq(ctx->mmio_base, true); + return IRQ_HANDLED; +} + static struct sdw_intel_ctx *sdw_intel_probe_controller(struct sdw_intel_res *res) { @@ -171,6 +199,7 @@ static struct sdw_intel_ctx u32 link_mask; int count; int i; + int ret; if (!res) return NULL; @@ -196,10 +225,13 @@ static struct sdw_intel_ctx ctx->mmio_base = res->mmio_base; ctx->link_mask = res->link_mask; ctx->handle = res->handle; + ctx->irq = res->irq; link = ctx->links; link_mask = ctx->link_mask; + INIT_LIST_HEAD(&ctx->link_list); + /* Create SDW Master devices */ for (i = 0; i < count; i++, link++) { if (link_mask && !(link_mask & BIT(i))) @@ -220,12 +252,22 @@ static struct sdw_intel_ctx + (SDW_LINK_SIZE * i); link->shim = res->mmio_base + SDW_SHIM_BASE; link->alh = res->mmio_base + SDW_ALH_BASE; - link->irq = res->irq; link->ops = res->ops; link->dev = res->dev; /* let the SoundWire master driver to its probe */ md->driver->probe(md, link); + + list_add_tail(&link->list, &ctx->link_list); + } + + ret = request_threaded_irq(ctx->irq, + sdw_intel_irq, sdw_intel_thread, + IRQF_SHARED, KBUILD_MODNAME, ctx); + if (ret < 0) { + dev_err(&adev->dev, "unable to grab IRQ %d, disabling device\n", + res->irq); + goto err; } return ctx; @@ -373,6 +415,7 @@ EXPORT_SYMBOL(sdw_intel_startup); */ void sdw_intel_exit(struct sdw_intel_ctx *ctx) { + free_irq(ctx->irq, ctx); sdw_intel_cleanup(ctx); kfree(ctx); } diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 3ccb38d48eef15..9a9ce9fd1d71b2 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -68,6 +68,8 @@ struct sdw_intel_link_res; * @handle: ACPI parent handle * @links: information for each link (controller-specific and kept * opaque here) + * @irq: shared interrupts for all links + * @link_list: list to handle interrupts across all links */ struct sdw_intel_ctx { int count; @@ -75,6 +77,8 @@ struct sdw_intel_ctx { u32 link_mask; acpi_handle handle; struct sdw_intel_link_res *links; + int irq; + struct list_head link_list; }; /** From 33ec84430782b0f775b36585ed34bdb39bab6908 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 1 Aug 2019 19:53:15 -0500 Subject: [PATCH 23/59] soundwire: bus: fix race condition with probe_complete signaling The driver probe takes care of basic initialization and is invoked when a Slave becomes attached, after a match between the Slave DevID registers and ACPI/DT entries. The update_status callback is invoked when a Slave state changes, e.g. when it is assigned a non-zero Device Number and it reports with an ATTACHED/ALERT state. The state change detection is usually hardware-based and based on the SoundWire frame rate (e.g. double-digit microseconds) while the probe is a pure software operation, which may involve a kernel module load. In corner cases, it's possible that the state changes before the probe completes. This patch suggests the use of wait_for_completion to avoid races on startup, so that the update_status callback does not rely on invalid pointers/data structures. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 25 ++++++++++++++++++++++--- drivers/soundwire/bus.h | 1 + drivers/soundwire/bus_type.c | 5 +++++ drivers/soundwire/slave.c | 2 ++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 4b22ee996a6566..d99acbc614c68b 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -961,10 +961,29 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) static int sdw_update_slave_status(struct sdw_slave *slave, enum sdw_slave_status status) { - if (slave->ops && slave->ops->update_status) - return slave->ops->update_status(slave, status); + unsigned long time; - return 0; + if (!slave->probed) { + /* + * the slave status update is typically handled in an + * interrupt thread, which can race with the driver + * probe, e.g. when a module needs to be loaded. + * + * make sure the probe is complete before updating + * status. + */ + time = wait_for_completion_timeout(&slave->probe_complete, + msecs_to_jiffies(DEFAULT_PROBE_TIMEOUT)); + if (!time) { + dev_err(&slave->dev, "Probe not complete, timed out\n"); + return -ETIMEDOUT; + } + } + + if (!slave->ops || !slave->ops->update_status) + return 0; + + return slave->ops->update_status(slave, status); } /** diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index be01a5f3d00bae..93e7bbab0938fd 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -5,6 +5,7 @@ #define __SDW_BUS_H #define DEFAULT_BANK_SWITCH_TIMEOUT 3000 +#define DEFAULT_PROBE_TIMEOUT 2000 int sdw_uevent(struct device *dev, struct kobj_uevent_env *env); diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c index df1271f6db6142..c9dbc98ac028db 100644 --- a/drivers/soundwire/bus_type.c +++ b/drivers/soundwire/bus_type.c @@ -120,6 +120,11 @@ static int sdw_slave_drv_probe(struct device *dev) slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout, slave->prop.clk_stop_timeout); + slave->probed = true; + complete(&slave->probe_complete); + + dev_dbg(dev, "probe complete\n"); + return 0; } diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 014c3ece1f1768..15ac89ecae0109 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -53,6 +53,8 @@ static int sdw_slave_add(struct sdw_bus *bus, slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; slave->dev_num = 0; + init_completion(&slave->probe_complete); + slave->probed = false; mutex_lock(&bus->bus_lock); list_add_tail(&slave->node, &bus->slaves); From 150f71103cf980127fc5fea84b8da6e8b0a09ed9 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 10 Jun 2019 19:09:14 -0500 Subject: [PATCH 24/59] soundwire: bus: add PM/no-PM versions of read/write functions Add support for pm_runtime with the appropriate error checks for sdw_write/read functions, e.g. when pm_runtime is not supported. Also expose internal functions without pm_runtime support, which are required to perform any sort of suspend/resume operation, as well as any enumeration tasks. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 68 +++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index d99acbc614c68b..a397d0a772b3d9 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -317,6 +317,46 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave, return 0; } +/* + * Read/Write IO functions. + * no_pm versions can only be called by the bus, e.g. while enumerating or + * handling suspend-resume sequences. + * all clients need to use the pm versions + */ + +static int +sdw_nread_no_pm(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) +{ + struct sdw_msg msg; + int ret; + + ret = sdw_fill_msg(&msg, slave, addr, count, + slave->dev_num, SDW_MSG_FLAG_READ, val); + if (ret < 0) + return ret; + + return sdw_transfer(slave->bus, &msg); +} + +static int +sdw_nwrite_no_pm(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) +{ + struct sdw_msg msg; + int ret; + + ret = sdw_fill_msg(&msg, slave, addr, count, + slave->dev_num, SDW_MSG_FLAG_WRITE, val); + if (ret < 0) + return ret; + + return sdw_transfer(slave->bus, &msg); +} + +int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value) +{ + return sdw_nwrite_no_pm(slave, addr, 1, &value); +} + /** * sdw_nread() - Read "n" contiguous SDW Slave registers * @slave: SDW Slave @@ -326,19 +366,17 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave, */ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) { - struct sdw_msg msg; int ret; - ret = sdw_fill_msg(&msg, slave, addr, count, - slave->dev_num, SDW_MSG_FLAG_READ, val); - if (ret < 0) - return ret; - ret = pm_runtime_get_sync(slave->bus->dev); - if (ret < 0) + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_noidle(slave->bus->dev); return ret; + } + + ret = sdw_nread_no_pm(slave, addr, count, val); - ret = sdw_transfer(slave->bus, &msg); + pm_runtime_mark_last_busy(slave->bus->dev); pm_runtime_put(slave->bus->dev); return ret; @@ -354,19 +392,17 @@ EXPORT_SYMBOL(sdw_nread); */ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val) { - struct sdw_msg msg; int ret; - ret = sdw_fill_msg(&msg, slave, addr, count, - slave->dev_num, SDW_MSG_FLAG_WRITE, val); - if (ret < 0) - return ret; - ret = pm_runtime_get_sync(slave->bus->dev); - if (ret < 0) + if (ret < 0 && ret != -EACCES) { + pm_runtime_put_noidle(slave->bus->dev); return ret; + } + + ret = sdw_nwrite_no_pm(slave, addr, count, val); - ret = sdw_transfer(slave->bus, &msg); + pm_runtime_mark_last_busy(slave->bus->dev); pm_runtime_put(slave->bus->dev); return ret; From a3b360e35fdb91de0ce8f4c880ee28c706e3c712 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 16:19:00 -0500 Subject: [PATCH 25/59] soundwire: bus: write Slave Device Number without runtime_pm While handling the Device0, we can safely use sdw_write_no_pm. This move will also helps us track that all other usages of sdw_write() happen when the Slave is already enumerated. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index a397d0a772b3d9..d0461bb2fb86ea 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -513,7 +513,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave) slave->dev_num = 0; } - ret = sdw_write(slave, SDW_SCP_DEVNUMBER, dev_num); + ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num); if (ret < 0) { dev_err(&slave->dev, "Program device_num %d failed: %d\n", dev_num, ret); From cd930c4bd74f48e2d3e21875e8f7ff90d80983e6 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 20 Sep 2019 14:08:57 -0500 Subject: [PATCH 26/59] soundwire: intel: add helpers for link power down and shim wake These routines are required for power management Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 23c5c3f9d30dce..00fa5c3a6c1f48 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -362,6 +362,59 @@ static int intel_shim_init(struct sdw_intel *sdw) return ret; } +static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable) +{ + void __iomem *shim = sdw->link_res->shim; + unsigned int link_id = sdw->instance; + u16 wake_en, wake_sts; + + if (wake_enable) { + /* Enable the wakeup */ + intel_writew(shim, SDW_SHIM_WAKEEN, + (SDW_SHIM_WAKEEN_ENABLE << link_id)); + } else { + /* Disable the wake up interrupt */ + wake_en = intel_readw(shim, SDW_SHIM_WAKEEN); + wake_en &= ~(SDW_SHIM_WAKEEN_ENABLE << link_id); + intel_writew(shim, SDW_SHIM_WAKEEN, wake_en); + + /* Clear wake status */ + wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS); + wake_sts |= (SDW_SHIM_WAKEEN_ENABLE << link_id); + intel_writew(shim, SDW_SHIM_WAKESTS_STATUS, wake_sts); + } +} + +static int intel_link_power_down(struct sdw_intel *sdw) +{ + int link_control, spa_mask, cpa_mask, ret; + unsigned int link_id = sdw->instance; + void __iomem *shim = sdw->link_res->shim; + u16 ioctl; + + /* Glue logic */ + ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id)); + ioctl |= SDW_SHIM_IOCTL_BKE; + ioctl |= SDW_SHIM_IOCTL_COE; + intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl); + + ioctl &= ~(SDW_SHIM_IOCTL_MIF); + intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl); + + /* 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 (ret < 0) + return ret; + + sdw->cdns.link_up = false; + return 0; +} + /* * PDI routines */ From 064544939186d76c0e7d74ad99f108a553d0f339 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 20 Sep 2019 14:21:18 -0500 Subject: [PATCH 27/59] 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 --- drivers/soundwire/intel.c | 75 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 00fa5c3a6c1f48..c15596c011dee7 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1253,10 +1253,85 @@ static int intel_master_remove(struct sdw_master_device *md) 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); + int ret; + + if (cdns->bus.prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + cdns->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); + int ret; + + if (cdns->bus.prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + cdns->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) +}; + struct sdw_md_driver intel_sdw_driver = { .driver = { .name = "intel-sdw", .owner = THIS_MODULE, + .pm = &intel_pm, }, .probe = intel_master_probe, .startup = intel_master_startup, From 79057ead9ae77ece4f79152ee7ad36414a8f1dd3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 20 Sep 2019 14:51:39 -0500 Subject: [PATCH 28/59] 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. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 108 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index c15596c011dee7..1ce2a3c5900c68 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -21,6 +22,20 @@ #include "bus.h" #include "intel.h" +/* + * 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 @@ -742,10 +757,16 @@ static int sdw_stream_setup(struct snd_pcm_substream *substream, 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 sdw_stream_setup(substream, dai); } @@ -924,6 +945,7 @@ static void intel_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct sdw_cdns_dma_data *dma; + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) @@ -931,6 +953,9 @@ static void intel_shutdown(struct snd_pcm_substream *substream, snd_soc_dai_set_dma_data(dai, substream, NULL); kfree(dma); + + 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, @@ -1179,6 +1204,7 @@ static int intel_master_startup(struct sdw_master_device *md) { struct sdw_cdns_stream_config config; struct sdw_intel *sdw; + int link_flags; int ret; sdw = md->pdata; @@ -1225,6 +1251,17 @@ static int intel_master_startup(struct sdw_master_device *md) intel_debugfs_init(sdw); + /* Enable runtime PM */ + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); + if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) { + pm_runtime_set_autosuspend_delay(&md->dev, 3000); + pm_runtime_use_autosuspend(&md->dev); + pm_runtime_mark_last_busy(&md->dev); + + pm_runtime_set_active(&md->dev); + pm_runtime_enable(&md->dev); + } + return 0; err_interrupt: @@ -1288,6 +1325,35 @@ 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); + int ret; + + if (cdns->bus.prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + cdns->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); @@ -1321,10 +1387,44 @@ 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); + int ret; + + if (cdns->bus.prop.hw_disabled) { + dev_dbg(dev, "SoundWire master %d is disabled, ignoring\n", + cdns->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) }; struct sdw_md_driver intel_sdw_driver = { From 3bbbbe4e7199931b6ad40a906dddcc811a370fee Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 11 Oct 2019 17:15:41 -0500 Subject: [PATCH 29/59] 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. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 30 ++++++++++++++++++++++++++++++ include/linux/soundwire/sdw.h | 1 + 2 files changed, 31 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 1ce2a3c5900c68..b2bc9997024594 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1298,6 +1298,7 @@ static int intel_master_remove(struct sdw_master_device *md) static int intel_suspend(struct device *dev) { + struct sdw_master_device *md = to_sdw_master_device(dev); struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); int ret; @@ -1308,6 +1309,20 @@ static int intel_suspend(struct device *dev) return 0; } + if (pm_runtime_status_suspended(dev)) { + dev_dbg(dev, + "%s: pm_runtime status: suspended\n", + __func__); + + /* + * keep track of the state for the system resume, where + * we will need to reset the pm_runtime status to active + */ + md->pm_runtime_suspended = true; + + return 0; + } + ret = sdw_cdns_enable_interrupt(cdns, false); if (ret < 0) { dev_err(dev, "cannot disable interrupts on suspend\n"); @@ -1356,6 +1371,7 @@ static int intel_suspend_runtime(struct device *dev) static int intel_resume(struct device *dev) { + struct sdw_master_device *md = to_sdw_master_device(dev); struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); int ret; @@ -1366,6 +1382,20 @@ static int intel_resume(struct device *dev) return 0; } + if (md->pm_runtime_suspended) { + 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); + + md->pm_runtime_suspended = false; + } + ret = intel_init(sdw); if (ret) { dev_err(dev, "%s failed: %d", __func__, ret); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index d22950b1a5d929..72b2e0438abbae 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -589,6 +589,7 @@ struct sdw_master_device { struct device dev; int link_id; struct sdw_md_driver *driver; + bool pm_runtime_suspended; void *pdata; /* core does not touch */ }; From 28115c8bdee07ee17c7ec225584a5a6e375b3a2a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 18:26:11 -0500 Subject: [PATCH 30/59] soundwire: bus: add helper to reset Slave status to UNATTACHED When resuming, we need to re-enumerate and restart from UNATTACHED. This will help implement a more robust state machine avoiding race conditions on resume. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 24 ++++++++++++++++++++++++ drivers/soundwire/bus.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index d0461bb2fb86ea..b050a45d1745c7 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1108,3 +1108,27 @@ int sdw_handle_slave_status(struct sdw_bus *bus, return ret; } EXPORT_SYMBOL(sdw_handle_slave_status); + +void sdw_clear_slave_status(struct sdw_bus *bus) +{ + struct sdw_slave *slave; + int i; + + /* Check all non-zero devices */ + for (i = 1; i <= SDW_MAX_DEVICES; i++) { + mutex_lock(&bus->bus_lock); + if (test_bit(i, bus->assigned) == false) { + mutex_unlock(&bus->bus_lock); + continue; + } + mutex_unlock(&bus->bus_lock); + + slave = sdw_get_slave(bus, i); + if (!slave) + continue; + + if (slave->status != SDW_SLAVE_UNATTACHED) + sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); + } +} +EXPORT_SYMBOL(sdw_clear_slave_status); diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 93e7bbab0938fd..4e6fb5d2f5ccd9 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -167,4 +167,6 @@ sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) return sdw_write(slave, addr, tmp); } +void sdw_clear_slave_status(struct sdw_bus *bus); + #endif /* __SDW_BUS_H */ From 7d133dd932c67f11a584cf061830816aee72a20e Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 18:32:52 -0500 Subject: [PATCH 31/59] 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 resume and pm_resume, this will be modified when clock-stop mode is supported Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index b2bc9997024594..e3741c3afe1c5b 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1402,6 +1402,9 @@ static int intel_resume(struct device *dev) return ret; } + /* make sure all Slaves are tagged as UNATTACHED */ + sdw_clear_slave_status(&sdw->cdns.bus); + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { dev_err(dev, "cannot enable interrupts during resume\n"); @@ -1435,6 +1438,9 @@ static int intel_resume_runtime(struct device *dev) return ret; } + /* make sure all Slaves are tagged as UNATTACHED */ + sdw_clear_slave_status(&sdw->cdns.bus); + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { dev_err(dev, "cannot enable interrupts during resume\n"); From 6397b09a4c45f712ac34fed742e51c46130d1606 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 18:37:00 -0500 Subject: [PATCH 32/59] soundwire: bus: check first if Slaves become UNATTACHED Before checking for the presence of Device0, we first need to clean-up the internal state of Slaves that are no longer attached. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index b050a45d1745c7..9f03a915a09ae4 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1034,6 +1034,24 @@ int sdw_handle_slave_status(struct sdw_bus *bus, struct sdw_slave *slave; int i, ret = 0; + /* first check if any Slaves fell off the bus */ + for (i = 1; i <= SDW_MAX_DEVICES; i++) { + mutex_lock(&bus->bus_lock); + if (test_bit(i, bus->assigned) == false) { + mutex_unlock(&bus->bus_lock); + continue; + } + mutex_unlock(&bus->bus_lock); + + slave = sdw_get_slave(bus, i); + if (!slave) + continue; + + if (status[i] == SDW_SLAVE_UNATTACHED && + slave->status != SDW_SLAVE_UNATTACHED) + sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); + } + if (status[0] == SDW_SLAVE_ATTACHED) { dev_dbg(bus->dev, "Slave attached, programming device number\n"); ret = sdw_program_device_num(bus); From a27514d1bb2c86e18eb4c20f15e37a3bb78e72ae Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 26 Sep 2019 18:47:35 -0500 Subject: [PATCH 33/59] soundwire: bus: fix race condition with enumeration_complete signaling This patch adds the signaling needed for Slave drivers to wait until the enumeration completes so that race conditions when issuing read/write commands are avoided. The calls for wait_for_completion() will be added in codec drivers in follow-up patches. The order between init_completion() and complete() is deterministic, the Slave is marked as UNATTACHED either during a Master-initiated HardReset, or when the hardware detects the Slave no longer reports as ATTACHED. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 19 +++++++++++++++++++ drivers/soundwire/slave.c | 1 + 2 files changed, 20 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 9f03a915a09ae4..e34b5ed534b332 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -637,6 +637,25 @@ static void sdw_modify_slave_status(struct sdw_slave *slave, enum sdw_slave_status status) { mutex_lock(&slave->bus->bus_lock); + + dev_vdbg(&slave->dev, + "%s: changing status slave %d status %d new status %d\n", + __func__, slave->dev_num, slave->status, status); + + if (status == SDW_SLAVE_UNATTACHED) { + dev_dbg(&slave->dev, + "%s: initializing completion for Slave %d\n", + __func__, slave->dev_num); + + init_completion(&slave->enumeration_complete); + + } else if (status == SDW_SLAVE_ATTACHED) { + dev_dbg(&slave->dev, + "%s: signaling completion for Slave %d\n", + __func__, slave->dev_num); + + complete(&slave->enumeration_complete); + } slave->status = status; mutex_unlock(&slave->bus->bus_lock); } diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 15ac89ecae0109..76fdfbd8b50d5d 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -52,6 +52,7 @@ static int sdw_slave_add(struct sdw_bus *bus, slave->dev.type = &sdw_slave_type; slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; + init_completion(&slave->enumeration_complete); slave->dev_num = 0; init_completion(&slave->probe_complete); slave->probed = false; From 0808d6fe5cea8d1ee1bac0551ec66881458c1aa4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 4 Nov 2019 12:47:01 -0600 Subject: [PATCH 34/59] soundwire: bus: fix race condition with initialization_complete signaling Waiting for the enumeration to be complete may not be enough for a Slave driver, there is a possible race condition between resume operations and initializations handled in an interrupt thread, which can results in settings not being fully restored after system or pm_runtime resume. This patch builds on the changes added for enumeration_complete, init_completion() is called when the Slave device becomes UNATTACHED, as done with enumeration_complete. The difference with the enumeration_complete case is that complete() is signaled after the Slave device is fully initialized after the .update_status() callback is called. A Slave device driver can decide to wait on either of the two complete() cases, depending on its initialization code and requirements. Signed-off-by: Rander Wang Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 8 ++++++++ drivers/soundwire/slave.c | 1 + 2 files changed, 9 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index e34b5ed534b332..329f35a406491e 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -648,6 +648,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave, __func__, slave->dev_num); init_completion(&slave->enumeration_complete); + init_completion(&slave->initialization_complete); } else if (status == SDW_SLAVE_ATTACHED) { dev_dbg(&slave->dev, @@ -1051,6 +1052,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus, { enum sdw_slave_status prev_status; struct sdw_slave *slave; + bool attached_initializing; int i, ret = 0; /* first check if any Slaves fell off the bus */ @@ -1096,6 +1098,8 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (!slave) continue; + attached_initializing = false; + switch (status[i]) { case SDW_SLAVE_UNATTACHED: if (slave->status == SDW_SLAVE_UNATTACHED) @@ -1122,6 +1126,8 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (prev_status == SDW_SLAVE_ALERT) break; + attached_initializing = true; + ret = sdw_initialize_slave(slave); if (ret) dev_err(bus->dev, @@ -1140,6 +1146,8 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (ret) dev_err(slave->bus->dev, "Update Slave status failed:%d\n", ret); + if (attached_initializing) + complete(&slave->initialization_complete); } return ret; diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 76fdfbd8b50d5d..d2a952d9bd4755 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -53,6 +53,7 @@ static int sdw_slave_add(struct sdw_bus *bus, slave->bus = bus; slave->status = SDW_SLAVE_UNATTACHED; init_completion(&slave->enumeration_complete); + init_completion(&slave->initialization_complete); slave->dev_num = 0; init_completion(&slave->probe_complete); slave->probed = false; From 0283e2745001750dc7da144ef88fcc263f330722 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 8 Nov 2019 17:13:22 -0600 Subject: [PATCH 35/59] soundwire: bus: fix race condition by tracking UNATTACHED transition In previous patches, we added enumeration_ and initialization_complete fields to avoid race conditions. When streaming restarts, the Master first resumes, then requests all Slaves to renumerate/reinitialized, and then the Slave devices resume. Intel validation exposed a corner case where the Slave device may transition to D3 when streaming stops, but streaming restarts before the Master transitions to D3. In that case, the Slave status was not cleared as UNATTACHED, and the wait_for_completion will time out. The proposed solution is that when the Master clears the Slave(s) status, the reason for the Slave(s) becoming unattached is memorized. When the slave resumes, it can check if a Master-initiated re-enumeration and initialization takes place and skip the wait_for_completion() if there is no reason to wait. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 5 ++++- drivers/soundwire/bus.h | 8 +++++++- drivers/soundwire/intel.c | 16 ++++++++++++---- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 329f35a406491e..ddc3042e15e087 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1154,7 +1154,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus, } EXPORT_SYMBOL(sdw_handle_slave_status); -void sdw_clear_slave_status(struct sdw_bus *bus) +void sdw_clear_slave_status(struct sdw_bus *bus, u32 request) { struct sdw_slave *slave; int i; @@ -1174,6 +1174,9 @@ void sdw_clear_slave_status(struct sdw_bus *bus) if (slave->status != SDW_SLAVE_UNATTACHED) sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); + + /* keep track of request, used in pm_runtime resume */ + slave->unattach_request = request; } } EXPORT_SYMBOL(sdw_clear_slave_status); diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 4e6fb5d2f5ccd9..ee362472003a93 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -167,6 +167,12 @@ sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) return sdw_write(slave, addr, tmp); } -void sdw_clear_slave_status(struct sdw_bus *bus); +/* + * At the moment we only track Master-initiated hw_reset. + * Additional fields can be added as needed + */ +#define SDW_UNATTACH_REQUEST_MASTER_RESET BIT(0) + +void sdw_clear_slave_status(struct sdw_bus *bus, u32 request); #endif /* __SDW_BUS_H */ diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e3741c3afe1c5b..69b0c8f65ad85b 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1402,8 +1402,12 @@ static int intel_resume(struct device *dev) return ret; } - /* make sure all Slaves are tagged as UNATTACHED */ - sdw_clear_slave_status(&sdw->cdns.bus); + /* + * make sure all Slaves are tagged as UNATTACHED and provide + * reason for reinitialization + */ + sdw_clear_slave_status(&sdw->cdns.bus, + SDW_UNATTACH_REQUEST_MASTER_RESET); ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { @@ -1438,8 +1442,12 @@ static int intel_resume_runtime(struct device *dev) return ret; } - /* make sure all Slaves are tagged as UNATTACHED */ - sdw_clear_slave_status(&sdw->cdns.bus); + /* + * make sure all Slaves are tagged as UNATTACHED and provide + * reason for reinitialization + */ + sdw_clear_slave_status(&sdw->cdns.bus, + SDW_UNATTACH_REQUEST_MASTER_RESET); ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { From de1fe3d74d6f488a0f2664c36eda34a473e7ceef Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Oct 2019 21:05:13 -0500 Subject: [PATCH 36/59] soundwire: intel: disable pm_runtime when removing a master Prevent race conditions between remove and resume by disabling pm_runtime. Note that this only takes care of pm_runtime at the Master level, the same precautions are needed when removing a Slave device. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 69b0c8f65ad85b..460e2d8cf80d31 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1275,6 +1275,8 @@ static int intel_master_remove(struct sdw_master_device *md) { struct sdw_intel *sdw; + pm_runtime_disable(&md->dev); + sdw = md->pdata; if (!sdw->cdns.bus.prop.hw_disabled) { From 837bd6a58d93d4b8b2d9b103749d9ae9f7eadbc1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 8 Oct 2019 21:02:08 -0500 Subject: [PATCH 37/59] soundwire: bus: disable pm_runtime in sdw_slave_delete Before removing the slave device, disable pm_runtime to prevent any race condition with the resume being executed after the bus and slave devices are removed. Since this pm_runtime_disable() is handled in common routines, implementations of Slave drivers do not need to call it in their .remove() routine. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index ddc3042e15e087..23bdcb9a468c81 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -113,6 +113,8 @@ static int sdw_delete_slave(struct device *dev, void *data) struct sdw_slave *slave = to_sdw_slave_device(dev); struct sdw_bus *bus = slave->bus; + pm_runtime_disable(dev); + sdw_slave_debugfs_exit(slave); mutex_lock(&bus->bus_lock); From 9e2418c4af6b130748c7a9d081e01642d020dbd2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 14 Nov 2019 11:14:55 -0600 Subject: [PATCH 38/59] soundwire: stream: remove redundant pr_err traces Only keep pr_err to flag critical configuration errors that will typically only happen during system integration. For errors on prepare/deprepare/enable/disable, the caller can do a much better job with more information on the DAI and device that caused the issue. Suggested-by: Cezary Rojewski Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index e69f94a8c3a865..178ae92b8cc1cc 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1554,8 +1554,6 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); ret = _sdw_prepare_stream(stream); - if (ret < 0) - pr_err("Prepare for stream:%s failed: %d\n", stream->name, ret); sdw_release_bus_lock(stream); return ret; @@ -1622,8 +1620,6 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); ret = _sdw_enable_stream(stream); - if (ret < 0) - pr_err("Enable for stream:%s failed: %d\n", stream->name, ret); sdw_release_bus_lock(stream); return ret; @@ -1698,8 +1694,6 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); ret = _sdw_disable_stream(stream); - if (ret < 0) - pr_err("Disable for stream:%s failed: %d\n", stream->name, ret); sdw_release_bus_lock(stream); return ret; @@ -1756,8 +1750,6 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); ret = _sdw_deprepare_stream(stream); - if (ret < 0) - pr_err("De-prepare for stream:%d failed: %d\n", ret, ret); sdw_release_bus_lock(stream); return ret; From 7a5f80af1064ddac7a4defd5810153c4bdc941c7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Sat, 12 Oct 2019 14:38:06 -0500 Subject: [PATCH 39/59] soundwire: stream: update state machine and add state checks The state machine and notes don't accurately explain or allow transitions from STREAM_DEPREPARED and STREAM_DISABLED. Add more explanations and allow for more transitions as a result of a trigger_stop(), trigger_suspend() and prepare(), depending on the ALSA/ASoC layer behavior defined by the INFO_RESUME and INFO_PAUSE flags. Also add basic checks to help debug inconsistent states and illegal state machine transitions. Signed-off-by: Pierre-Louis Bossart --- Documentation/driver-api/soundwire/stream.rst | 63 +++++++++++++------ drivers/soundwire/stream.c | 37 +++++++++++ 2 files changed, 82 insertions(+), 18 deletions(-) diff --git a/Documentation/driver-api/soundwire/stream.rst b/Documentation/driver-api/soundwire/stream.rst index 5351bd2f34a870..9b7418ff8d59aa 100644 --- a/Documentation/driver-api/soundwire/stream.rst +++ b/Documentation/driver-api/soundwire/stream.rst @@ -156,22 +156,27 @@ Below shows the SoundWire stream states and state transition diagram. :: +-----------+ +------------+ +----------+ +----------+ | ALLOCATED +---->| CONFIGURED +---->| PREPARED +---->| ENABLED | | STATE | | STATE | | STATE | | STATE | - +-----------+ +------------+ +----------+ +----+-----+ - ^ - | - | - v - +----------+ +------------+ +----+-----+ + +-----------+ +------------+ +---+--+---+ +----+-----+ + ^ ^ ^ + | | | + __| |___________ | + | | | + v | v + +----------+ +-----+------+ +-+--+-----+ | RELEASED |<----------+ DEPREPARED |<-------+ DISABLED | | STATE | | STATE | | STATE | +----------+ +------------+ +----------+ -NOTE: State transition between prepare and deprepare is supported in Spec -but not in the software (subsystem) +NOTE: State transitions between ``SDW_STREAM_ENABLED`` and +``SDW_STREAM_DISABLED`` are only relevant when then INFO_PAUSE flag is +supported at the ALSA/ASoC level. Likewise the transition between +``SDW_DISABLED_STATE`` and ``SDW_PREPARED_STATE`` depends on the +INFO_RESUME flag. -NOTE2: Stream state transition checks need to be handled by caller -framework, for example ALSA/ASoC. No checks for stream transition exist in -SoundWire subsystem. +NOTE2: The framework implements basic state transition checks, but +does not e.g. check if a transition from DISABLED to ENABLED is valid +on a specific platform. Such tests need to be added at the ALSA/ASoC +level. Stream State Operations ----------------------- @@ -246,6 +251,9 @@ SDW_STREAM_PREPARED Prepare state of stream. Operations performed before entering in this state: + (0) Steps 1 and 2 are omitted in the case of a resume operation, + where the bus bandwidth is known. + (1) Bus parameters such as bandwidth, frame shape, clock frequency, are computed based on current stream as well as already active stream(s) on Bus. Re-computation is required to accommodate current @@ -270,13 +278,15 @@ Prepare state of stream. Operations performed before entering in this state: After all above operations are successful, stream state is set to ``SDW_STREAM_PREPARED``. -Bus implements below API for PREPARE state which needs to be called once per -stream. From ASoC DPCM framework, this stream state is linked to -.prepare() operation. +Bus implements below API for PREPARE state which needs to be called +once per stream. From ASoC DPCM framework, this stream state is linked +to .prepare() operation. Since the .trigger() operations may not +follow the .prepare(), a direct transitions from +``SDW_STREAM_PREPARED`` to ``SDW_STREAM_DEPREPARED`` is allowed. .. code-block:: c - int sdw_prepare_stream(struct sdw_stream_runtime * stream); + int sdw_prepare_stream(struct sdw_stream_runtime * stream, bool resume); SDW_STREAM_ENABLED @@ -332,6 +342,14 @@ Bus implements below API for DISABLED state which needs to be called once per stream. From ASoC DPCM framework, this stream state is linked to .trigger() stop operation. +When the INFO_PAUSE flag is supported, a direct transition to +``SDW_STREAM_ENABLED`` is allowed. + +For resume operations where ASoC will use the .prepare() callback, the +stream can transition from ``SDW_STREAM_DISABLED`` to +``SDW_STREAM_PREPARED``, with all required settings restored but +without updating the bandwidth and bit allocation. + .. code-block:: c int sdw_disable_stream(struct sdw_stream_runtime * stream); @@ -353,9 +371,18 @@ state: After all above operations are successful, stream state is set to ``SDW_STREAM_DEPREPARED``. -Bus implements below API for DEPREPARED state which needs to be called once -per stream. From ASoC DPCM framework, this stream state is linked to -.trigger() stop operation. +Bus implements below API for DEPREPARED state which needs to be called +once per stream. ALSA/ASoC do not have a concept of 'deprepare', and +the mapping from this stream state to ALSA/ASoC operation may be +implementation specific. + +When the INFO_PAUSE flag is supported, the stream state is linked to +the .hw_free() operation - the stream is not deprepared on a +TRIGGER_STOP. + +Other implementations may transition to the ``SDW_STREAM_DEPREPARED`` +state on TRIGGER_STOP, should they require a transition through the +``SDW_STREAM_PREPARED`` state. .. code-block:: c diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 178ae92b8cc1cc..6aa0b5d370c039 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1553,8 +1553,18 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state != SDW_STREAM_CONFIGURED && + stream->state != SDW_STREAM_DEPREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_prepare_stream(stream); +state_err: sdw_release_bus_lock(stream); return ret; } @@ -1619,8 +1629,17 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state != SDW_STREAM_PREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_enable_stream(stream); +state_err: sdw_release_bus_lock(stream); return ret; } @@ -1693,8 +1712,16 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state != SDW_STREAM_ENABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_disable_stream(stream); +state_err: sdw_release_bus_lock(stream); return ret; } @@ -1749,8 +1776,18 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) } sdw_acquire_bus_lock(stream); + + if (stream->state != SDW_STREAM_PREPARED && + stream->state != SDW_STREAM_DISABLED) { + pr_err("%s: %s: inconsistent state state %d\n", + __func__, stream->name, stream->state); + ret = -EINVAL; + goto state_err; + } + ret = _sdw_deprepare_stream(stream); +state_err: sdw_release_bus_lock(stream); return ret; } From 8281dccb6874e4fabf12827f214d9b68c2f559b1 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 20 Sep 2019 14:48:26 +0800 Subject: [PATCH 40/59] soundwire: stream: only prepare stream when it is configured. We don't need to prepare the stream again if the stream is already prepared. sdw_prepare_stream() could be called multiple times without calling sdw_deprepare_stream(). We call sdw_prepare_stream() in the prepare dai ops and sdw_deprepare_stream() in the hw_free dai ops. If an xrun happens, sdw_prepare_stream() will be called but sdw_deprepare_stream() will not, which results in an imbalance and an invalid total bandwidth. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 6aa0b5d370c039..bd0bddf738302a 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1544,7 +1544,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) */ int sdw_prepare_stream(struct sdw_stream_runtime *stream) { - int ret = 0; + int ret; if (!stream) { pr_err("SoundWire: Handle not found for stream\n"); @@ -1553,6 +1553,11 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) sdw_acquire_bus_lock(stream); + if (stream->state == SDW_STREAM_PREPARED) { + ret = 0; + goto state_err; + } + if (stream->state != SDW_STREAM_CONFIGURED && stream->state != SDW_STREAM_DEPREPARED && stream->state != SDW_STREAM_DISABLED) { From cc31e2951dca4462a04b40ea27826d3961815e3a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 15 Oct 2019 09:43:47 -0500 Subject: [PATCH 41/59] soundwire: stream: do not update parameters during DISABLED-PREPARED transition After a system suspend, the ALSA/ASoC core will invoke the .prepare() callback and a TRIGGER_START when INFO_RESUME is not supported. Likewise, when an underflow occurs, the .prepare callback will be invoked. In both cases, the stream can be in DISABLED mode, and will transition into the PREPARED mode. We however don't want the bus bandwidth to be recomputed. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/stream.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index bd0bddf738302a..c28ce7f0d7424f 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1460,7 +1460,8 @@ static void sdw_release_bus_lock(struct sdw_stream_runtime *stream) } } -static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) +static int _sdw_prepare_stream(struct sdw_stream_runtime *stream, + bool update_params) { struct sdw_master_runtime *m_rt; struct sdw_bus *bus = NULL; @@ -1480,6 +1481,9 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) return -EINVAL; } + if (!update_params) + goto program_params; + /* Increment cumulative bus bandwidth */ /* TODO: Update this during Device-Device support */ bus->params.bandwidth += m_rt->stream->params.rate * @@ -1495,6 +1499,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) } } +program_params: /* Program params */ ret = sdw_program_params(bus); if (ret < 0) { @@ -1544,6 +1549,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) */ int sdw_prepare_stream(struct sdw_stream_runtime *stream) { + bool update_params = true; int ret; if (!stream) { @@ -1567,7 +1573,16 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) goto state_err; } - ret = _sdw_prepare_stream(stream); + /* + * when the stream is DISABLED, this means sdw_prepare_stream() + * is called as a result of an underflow or a resume operation. + * In this case, the bus parameters shall not be recomputed, but + * still need to be re-applied + */ + if (stream->state == SDW_STREAM_DISABLED) + update_params = false; + + ret = _sdw_prepare_stream(stream, update_params); state_err: sdw_release_bus_lock(stream); From 95d8d9266094759372fc1781062be9be1e14876d Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 14 Oct 2019 17:35:52 +0800 Subject: [PATCH 42/59] soundwire: intel: reinitialize IP+DSP in .prepare(), but only when resuming The .prepare() callback is invoked for normal streaming, underflows or during the system resume transition. In the latter case, the context for the ALH PDIs is lost, and the DSP is not initialized properly either, but the bus parameters don't need to be recomputed. Conversely, when doing a regular .prepare() during an underflow, the ALH/SHIM registers shall not be changed as the hardware cannot be reprogrammed after the DMA started (hardware spec requirement). This patch adds storage of PDI and hw_params in the DAI dma context, and the difference between the types of .prepare() usages is handled via a simple boolean, updated when suspending, and tested for in the .prepare() case. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.h | 4 ++ drivers/soundwire/intel.c | 69 +++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h index 0f108fd31fd943..cd4feef0b9001b 100644 --- a/drivers/soundwire/cadence_master.h +++ b/drivers/soundwire/cadence_master.h @@ -81,6 +81,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; @@ -89,6 +91,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 460e2d8cf80d31..b114e9a72fdc37 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -813,6 +813,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, @@ -856,7 +860,11 @@ static int intel_hw_params(struct snd_pcm_substream *substream, static int intel_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { + struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_cdns_dma_data *dma; + int ch, dir; + int ret; dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) { @@ -865,7 +873,42 @@ 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, @@ -936,6 +979,8 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) return ret; } + dma->hw_params = NULL; + dma->pdi = NULL; sdw_release_stream(dma->stream); return 0; @@ -958,6 +1003,26 @@ static void intel_shutdown(struct snd_pcm_substream *substream, pm_runtime_put_autosuspend(cdns->dev); } +static int intel_dai_suspend(struct snd_soc_dai *dai) +{ + struct sdw_cdns_dma_data *dma; + + /* + * 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) { @@ -1029,6 +1094,8 @@ static int intel_create_dai(struct sdw_cdns *cdns, dais[i].ops = &intel_pcm_dai_ops; else dais[i].ops = &intel_pdm_dai_ops; + + dais[i].suspend = intel_dai_suspend; } return 0; From 642b49e5d9e78eea36bef1ad1aae5f431eab8fdc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 24 Oct 2019 13:43:14 -0500 Subject: [PATCH 43/59] 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: Rander Wang Signed-off-by: Pierre-Louis Bossart --- 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 b114e9a72fdc37..f0077f58c4bf21 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -29,8 +29,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); @@ -1329,6 +1330,22 @@ static int intel_master_startup(struct sdw_master_device *md) pm_runtime_enable(&md->dev); } + /* + * Slave devices have an pm_runtime of '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(&md->dev); + return 0; err_interrupt: @@ -1443,6 +1460,7 @@ static int intel_resume(struct device *dev) struct sdw_master_device *md = to_sdw_master_device(dev); struct sdw_cdns *cdns = dev_get_drvdata(dev); struct sdw_intel *sdw = cdns_to_intel(cdns); + int link_flags; int ret; if (cdns->bus.prop.hw_disabled) { @@ -1463,6 +1481,10 @@ static int intel_resume(struct device *dev) pm_runtime_enable(dev); md->pm_runtime_suspended = false; + + link_flags = md_flags >> (sdw->cdns.bus.link_id * 8); + if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME_IDLE)) + pm_runtime_idle(&md->dev); } ret = intel_init(sdw); From a3789cce1659f41c7e91ff5d072257b28ef08bf6 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Sat, 12 Oct 2019 12:32:57 -0500 Subject: [PATCH 44/59] [HACK] add traces to debug aplay suspend/resume issue Signed-off-by: Pierre-Louis Bossart Signed-off-by: Bard Liao --- drivers/soundwire/intel.c | 52 +++++++++++++++++++++++++++++++++++--- drivers/soundwire/stream.c | 11 ++++++++ sound/soc/Makefile | 2 ++ sound/soc/sof/pcm.c | 7 +++++ sound/soc/sof/pm.c | 18 +++++++++++++ 5 files changed, 87 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index f0077f58c4bf21..12369950420e7a 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -761,6 +761,8 @@ static int intel_startup(struct snd_pcm_substream *substream, struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + ret = pm_runtime_get_sync(cdns->dev); if (ret < 0 && ret != -EACCES) { dev_err_ratelimited(cdns->dev, @@ -769,7 +771,11 @@ static int intel_startup(struct snd_pcm_substream *substream, pm_runtime_put_noidle(cdns->dev); } - return sdw_stream_setup(substream, dai); + ret = sdw_stream_setup(substream, dai); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + + return ret; } static int intel_hw_params(struct snd_pcm_substream *substream, @@ -786,6 +792,8 @@ static int intel_hw_params(struct snd_pcm_substream *substream, int ret; bool pcm = true; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return -EIO; @@ -855,6 +863,8 @@ static int intel_hw_params(struct snd_pcm_substream *substream, kfree(pconfig); error: + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); return ret; } @@ -867,6 +877,8 @@ static int intel_prepare(struct snd_pcm_substream *substream, int ch, dir; int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) { dev_err(dai->dev, "failed to get dma data in %s", @@ -908,6 +920,7 @@ static int intel_prepare(struct snd_pcm_substream *substream, ret = sdw_prepare_stream(dma->stream); + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); err: return ret; } @@ -918,6 +931,8 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, struct sdw_cdns_dma_data *dma; int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) { dev_err(dai->dev, "failed to get dma data in %s", __func__); @@ -925,14 +940,18 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, } switch (cmd) { + case SNDRV_PCM_TRIGGER_RESUME: + dev_err(dai->dev, "%s: %s: resume\n", __func__, dai->name); + /* fallthrough */ 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: + dev_err(dai->dev, "%s: %s: suspend\n", __func__, dai->name); + /* fallthrough */ + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_STOP: ret = sdw_disable_stream(dma->stream); break; @@ -946,6 +965,9 @@ static int intel_trigger(struct snd_pcm_substream *substream, int cmd, dev_err(dai->dev, "%s trigger %d failed: %d", __func__, cmd, ret); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + return ret; } @@ -957,6 +979,8 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) struct sdw_cdns_dma_data *dma; int ret; + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return -EIO; @@ -984,6 +1008,8 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) dma->pdi = NULL; sdw_release_stream(dma->stream); + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); + return 0; } @@ -993,6 +1019,8 @@ static void intel_shutdown(struct snd_pcm_substream *substream, struct sdw_cdns_dma_data *dma; struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai); + dev_err(dai->dev, "%s: %s: start\n", __func__, dai->name); + dma = snd_soc_dai_get_dma_data(dai, substream); if (!dma) return; @@ -1002,6 +1030,8 @@ static void intel_shutdown(struct snd_pcm_substream *substream, pm_runtime_mark_last_busy(cdns->dev); pm_runtime_put_autosuspend(cdns->dev); + + dev_err(dai->dev, "%s: %s: done\n", __func__, dai->name); } static int intel_dai_suspend(struct snd_soc_dai *dai) @@ -1395,6 +1425,8 @@ static int intel_suspend(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + if (pm_runtime_status_suspended(dev)) { dev_dbg(dev, "%s: pm_runtime status: suspended\n", @@ -1423,6 +1455,8 @@ static int intel_suspend(struct device *dev) intel_shim_wake(sdw, false); + dev_err(dev, "%s done\n", __func__); + return 0; } @@ -1438,6 +1472,8 @@ static int intel_suspend_runtime(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + ret = sdw_cdns_enable_interrupt(cdns, false); if (ret < 0) { dev_err(dev, "cannot disable interrupts on suspend\n"); @@ -1452,6 +1488,8 @@ static int intel_suspend_runtime(struct device *dev) intel_shim_wake(sdw, false); + dev_err(dev, "%s done\n", __func__); + return 0; } @@ -1469,6 +1507,8 @@ static int intel_resume(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + if (md->pm_runtime_suspended) { dev_dbg(dev, "%s: pm_runtime status was suspended, forcing active\n", @@ -1512,6 +1552,8 @@ static int intel_resume(struct device *dev) return ret; } + dev_err(dev, "%s done\n", __func__); + return ret; } @@ -1527,6 +1569,8 @@ static int intel_resume_runtime(struct device *dev) return 0; } + dev_err(dev, "%s start\n", __func__); + ret = intel_init(sdw); if (ret) { dev_err(dev, "%s failed: %d", __func__, ret); @@ -1552,6 +1596,8 @@ static int intel_resume_runtime(struct device *dev) return ret; } + dev_err(dev, "%s done\n", __func__); + return ret; } diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index c28ce7f0d7424f..3e8ff16fed052b 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1586,6 +1586,9 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_prepare_stream); @@ -1661,6 +1664,8 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); return ret; } EXPORT_SYMBOL(sdw_enable_stream); @@ -1743,6 +1748,9 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_disable_stream); @@ -1809,6 +1817,9 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) state_err: sdw_release_bus_lock(stream); + + pr_err("%s: %s: done\n", __func__, stream->name); + return ret; } EXPORT_SYMBOL(sdw_deprepare_stream); diff --git a/sound/soc/Makefile b/sound/soc/Makefile index 861a21b7948443..429add4afe3827 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,3 +1,5 @@ +ccflags-y += -DDEBUG + # SPDX-License-Identifier: GPL-2.0 snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-utils.o soc-dai.o soc-component.o snd-soc-core-objs += soc-pcm.o soc-io.o soc-devres.o soc-ops.o diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 549238a98b2ab5..cb73e0f317ee00 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -356,6 +356,9 @@ static int sof_pcm_trigger(struct snd_soc_component *component, return 0; } + dev_dbg(sdev->dev, "pcm: trigger resume stream %d dir %d cmd %d\n", + spcm->pcm.pcm_id, substream->stream, cmd); + /* set up hw_params */ ret = sof_pcm_prepare(component, substream); if (ret < 0) { @@ -389,6 +392,10 @@ static int sof_pcm_trigger(struct snd_soc_component *component, spcm->stream[substream->stream].suspend_ignored = true; return 0; } + + dev_dbg(sdev->dev, "pcm: trigger suspend stream %d dir %d cmd %d\n", + spcm->pcm.pcm_id, substream->stream, cmd); + /* fallthrough */ case SNDRV_PCM_TRIGGER_STOP: stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_STOP; diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 0fd5567237a8d5..943045997e9942 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -17,6 +17,8 @@ static int sof_restore_kcontrols(struct snd_sof_dev *sdev) int ipc_cmd, ctrl_type; int ret = 0; + dev_err(sdev->dev, "%s\n", __func__); + /* restore kcontrol values */ list_for_each_entry(scontrol, &sdev->kcontrol_list, list) { /* reset readback offset for scontrol after resuming */ @@ -69,6 +71,8 @@ static int sof_restore_pipelines(struct snd_sof_dev *sdev) struct sof_ipc_cmd_hdr *hdr; int ret; + dev_err(sdev->dev, "%s\n", __func__); + /* restore pipeline components */ list_for_each_entry_reverse(swidget, &sdev->widget_list, list) { struct sof_ipc_comp_reply r; @@ -202,6 +206,8 @@ static int sof_send_pm_ctx_ipc(struct snd_sof_dev *sdev, int cmd) struct sof_ipc_pm_ctx pm_ctx; struct sof_ipc_reply reply; + dev_err(sdev->dev, "%s\n", __func__); + memset(&pm_ctx, 0, sizeof(pm_ctx)); /* configure ctx save ipc message */ @@ -220,6 +226,7 @@ static int sof_set_hw_params_upon_resume(struct snd_sof_dev *sdev) snd_pcm_state_t state; int dir; + dev_err(sdev->dev, "%s\n", __func__); /* * SOF requires hw_params to be set-up internally upon resume. * So, set the flag to indicate this for those streams that @@ -265,6 +272,8 @@ static int sof_resume(struct device *dev, bool runtime_resume) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + /* do nothing if dsp resume callbacks are not set */ if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume) return 0; @@ -337,6 +346,8 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + /* do nothing if dsp suspend callback is not set */ if (!sof_ops(sdev)->suspend) return 0; @@ -393,6 +404,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) int snd_sof_runtime_suspend(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_suspend(dev, true); } EXPORT_SYMBOL(snd_sof_runtime_suspend); @@ -401,12 +413,14 @@ int snd_sof_runtime_idle(struct device *dev) { struct snd_sof_dev *sdev = dev_get_drvdata(dev); + dev_err(dev, "%s\n", __func__); return snd_sof_dsp_runtime_idle(sdev); } EXPORT_SYMBOL(snd_sof_runtime_idle); int snd_sof_runtime_resume(struct device *dev) { + dev_err(dev, "%s\n", __func__); return sof_resume(dev, true); } EXPORT_SYMBOL(snd_sof_runtime_resume); @@ -468,6 +482,8 @@ int snd_sof_resume(struct device *dev) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + if (snd_sof_dsp_d0i3_on_suspend(sdev)) { /* resume from D0I3 */ dev_dbg(sdev->dev, "DSP will exit from D0i3...\n"); @@ -497,6 +513,8 @@ int snd_sof_suspend(struct device *dev) struct snd_sof_dev *sdev = dev_get_drvdata(dev); int ret; + dev_err(dev, "%s\n", __func__); + if (snd_sof_dsp_d0i3_on_suspend(sdev)) { /* suspend to D0i3 */ dev_dbg(sdev->dev, "DSP is trying to enter D0i3...\n"); From 43937344d509de156810cce18ce7947c46af8a8a Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 5 Nov 2019 10:44:49 +0800 Subject: [PATCH 45/59] ALSA: HDA: intel-dsp-config: add DMI info for Dell laptop The laptop doesn't use DMIC, but use SOF. Signed-off-by: Bard Liao --- sound/hda/intel-dsp-config.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/hda/intel-dsp-config.c b/sound/hda/intel-dsp-config.c index be1df80ed01337..6e6e8662534d8a 100644 --- a/sound/hda/intel-dsp-config.c +++ b/sound/hda/intel-dsp-config.c @@ -204,6 +204,12 @@ static const struct config_entry config_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "Google"), } }, + { + .ident = "Dell laptop", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc"), + } + }, {} } }, From 03fe97183c553d8fb27276ba1909a5ad337903b4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 10 Jun 2019 19:31:58 -0500 Subject: [PATCH 46/59] soundwire: cadence_master: handle multiple status reports per Slave When a Slave reports multiple status in the sticky bits, find the latest configuration from the mirror of the PING frame status and update the status directly. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 35 +++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 362fb6e49bfe09..a97552a49babe8 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -673,13 +673,36 @@ static int cdns_update_slave_status(struct sdw_cdns *cdns, /* first check if Slave reported multiple status */ if (set_status > 1) { + u32 val; + dev_warn_ratelimited(cdns->dev, - "Slave reported multiple Status: %d\n", - mask); - /* - * TODO: we need to reread the status here by - * issuing a PING cmd - */ + "Slave %d reported multiple Status: %d\n", + i, mask); + + /* check latest status extracted from PING commands */ + val = cdns_readl(cdns, CDNS_MCP_SLAVE_STAT); + val >>= (i * 2); + + switch (val & 0x3) { + case 0: + status[i] = SDW_SLAVE_UNATTACHED; + break; + case 1: + status[i] = SDW_SLAVE_ATTACHED; + break; + case 2: + status[i] = SDW_SLAVE_ALERT; + break; + case 3: + default: + status[i] = SDW_SLAVE_RESERVED; + break; + } + + dev_warn_ratelimited(cdns->dev, + "Slave %d status updated to %d\n", + i, status[i]); + } } From d368de87672ae64f47f314958521251cef953154 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 24 Apr 2018 15:28:28 +0530 Subject: [PATCH 47/59] 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. FIXME: remove checkpatch report WARNING: Reusing the krealloc arg is almost always a bug + group->rates = krealloc(group->rates, Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/Kconfig | 4 + drivers/soundwire/Makefile | 3 + drivers/soundwire/bus.c | 6 + drivers/soundwire/bus.h | 46 +- .../soundwire/generic_bandwidth_allocation.c | 407 ++++++++++++++++++ drivers/soundwire/intel.c | 3 + drivers/soundwire/stream.c | 12 + include/linux/soundwire/sdw.h | 3 + 8 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 drivers/soundwire/generic_bandwidth_allocation.c diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index c725d0a8b288b6..eb73c91b7dba87 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. @@ -31,4 +32,7 @@ config SOUNDWIRE_INTEL 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 89b29819dd3ad8..eb37d69cbdca96 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -7,6 +7,9 @@ soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.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-objs += debugfs.o endif diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 23bdcb9a468c81..37662c8a683acc 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -30,6 +30,12 @@ int sdw_add_bus_master(struct sdw_bus *bus) return -EINVAL; } + if (!bus->compute_params) { + dev_err(bus->dev, + "Bandwidth allocation not configured, compute_parems 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 ee362472003a93..39a433b2cfb0c6 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -68,6 +68,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]; @@ -153,9 +154,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 00000000000000..dc7ffa987963ac --- /dev/null +++ b/drivers/soundwire/generic_bandwidth_allocation.c @@ -0,0 +1,407 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// Copyright(c) 2015-2019 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; + + 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); + + 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, true, + 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; + } + } +} + +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, + true, 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) { + group->max_size += 1; + group->rates = krealloc(group->rates, + (sizeof(int) * group->max_size), + GFP_KERNEL); + if (!group->rates) + return -ENOMEM; + } + + 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) + 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) + goto out; + + 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 out; + + _sdw_compute_port_params(bus, params, group.count); + +out: + kfree(params); + 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 12369950420e7a..c4c51fdc14b657 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1284,6 +1284,9 @@ static int intel_master_probe(struct sdw_master_device *md, void *link_ctx) /* set driver data, accessed by snd_soc_dai_set_drvdata() */ dev_set_drvdata(&md->dev, &sdw->cdns); + /* use generic bandwidth allocation algorithm */ + sdw->cdns.bus.compute_params = sdw_compute_params; + ret = sdw_add_bus_master(&sdw->cdns.bus); if (ret) { dev_err(&md->dev, "sdw_add_bus_master fail: %d\n", ret); diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 3e8ff16fed052b..0aace6ef596b4a 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -26,6 +26,8 @@ int sdw_rows[SDW_FRAME_ROWS] = {48, 50, 60, 64, 75, 80, 125, 147, 192, 200, 240, 256, 72, 144, 90, 180}; int sdw_cols[SDW_FRAME_COLS] = {2, 4, 6, 8, 10, 12, 14, 16}; +EXPORT_SYMBOL(sdw_rows); +EXPORT_SYMBOL(sdw_cols); int sdw_find_col_index(int col) { @@ -1775,6 +1777,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); if (ret < 0) { diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 72b2e0438abbae..6f6c917d17feaa 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -910,6 +910,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, From 4adcfc836bca5d152c13c4af2681dcc402c3859a Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Fri, 2 Aug 2019 10:14:46 +0800 Subject: [PATCH 48/59] soundwire: dynamic_allocation: set grp_ctrl_valid false Some Realtek codecs don't support grp_ctrl_valid. Set it to false to prevent DPN_BlockCtrl2 reg write failed. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/generic_bandwidth_allocation.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c index dc7ffa987963ac..6124c6f8561f32 100644 --- a/drivers/soundwire/generic_bandwidth_allocation.c +++ b/drivers/soundwire/generic_bandwidth_allocation.c @@ -54,7 +54,7 @@ static void sdw_compute_slave_ports(struct sdw_master_runtime *m_rt, ch = sdw_ch_mask_to_ch(p_rt->ch_mask); sdw_fill_xport_params(&p_rt->transport_params, - p_rt->num, true, + p_rt->num, false, SDW_BLK_GRP_CNT_1, sample_int, port_bo, port_bo >> 8, t_data->hstart, @@ -97,7 +97,7 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt, no_ch = sdw_ch_mask_to_ch(p_rt->ch_mask); sdw_fill_xport_params(&p_rt->transport_params, p_rt->num, - true, SDW_BLK_GRP_CNT_1, sample_int, + false, SDW_BLK_GRP_CNT_1, sample_int, port_bo, port_bo >> 8, hstart, hstop, (SDW_BLK_GRP_CNT_1 * no_ch), 0x0); From 990b17ff879367187dfb422a1b0ef267b9eb7e2d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 30 Jul 2019 16:14:18 -0500 Subject: [PATCH 49/59] soundwire: bus: fix device number leak on errors If the programming of the dev_number fails due to an IO error, a new device_number will be assigned, resulting in a leak. Make sure we only assign a device_number once per Slave device. Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 39 ++++++++++++++++++++++------------- include/linux/soundwire/sdw.h | 4 +++- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 37662c8a683acc..db717ac8b3d7b3 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -500,26 +500,37 @@ static int sdw_get_device_num(struct sdw_slave *slave) static int sdw_assign_device_num(struct sdw_slave *slave) { int ret, dev_num; + bool new_device = false; + + dev_dbg(slave->bus->dev, "in %s\n", __func__); /* check first if device number is assigned, if so reuse that */ if (!slave->dev_num) { - mutex_lock(&slave->bus->bus_lock); - dev_num = sdw_get_device_num(slave); - mutex_unlock(&slave->bus->bus_lock); - if (dev_num < 0) { - dev_err(slave->bus->dev, "Get dev_num failed: %d\n", - dev_num); - return dev_num; + if (!slave->dev_num_sticky) { + mutex_lock(&slave->bus->bus_lock); + dev_num = sdw_get_device_num(slave); + mutex_unlock(&slave->bus->bus_lock); + if (dev_num < 0) { + dev_err(slave->bus->dev, "Get dev_num failed: %d\n", + dev_num); + return dev_num; + } + slave->dev_num = dev_num; + slave->dev_num_sticky = dev_num; + new_device = true; + } else { + slave->dev_num = slave->dev_num_sticky; } - } else { + } + + if (!new_device) dev_info(slave->bus->dev, - "Slave already registered dev_num:%d\n", + "Slave already registered, reusing dev_num:%d\n", slave->dev_num); - /* Clear the slave->dev_num to transfer message on device 0 */ - dev_num = slave->dev_num; - slave->dev_num = 0; - } + /* Clear the slave->dev_num to transfer message on device 0 */ + dev_num = slave->dev_num; + slave->dev_num = 0; ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num); if (ret < 0) { @@ -529,7 +540,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave) } /* After xfer of msg, restore dev_num */ - slave->dev_num = dev_num; + slave->dev_num = slave->dev_num_sticky; return 0; } diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 6f6c917d17feaa..8688006194edd6 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -546,7 +546,8 @@ struct sdw_slave_ops { * @debugfs: Slave debugfs * @node: node for bus list * @port_ready: Port ready completion flag for each Slave port - * @dev_num: Device Number assigned by Bus + * @dev_num: Current Device Number, values can be 0 or dev_num_sticky + * @dev_num_sticky: one-time static Device Number assigned by Bus * @probed: boolean tracking driver state * @probe_complete: completion utility to control potential races * on startup between driver probe/initialization and SoundWire @@ -575,6 +576,7 @@ struct sdw_slave { struct list_head node; struct completion *port_ready; u16 dev_num; + u16 dev_num_sticky; bool probed; struct completion probe_complete; struct completion enumeration_complete; From 600ec99c14fdc2aa44b0c6affd1af0b322f5f8d6 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 19 Aug 2019 12:13:03 -0500 Subject: [PATCH 50/59] soundwire: intel: modify DMAT field for ALH The current value for this field is too low and results in some samples getting dropped and playback being faster than normal. Increase the value to a safer value. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- 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 c4c51fdc14b657..22736b1113e03c 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -104,7 +104,7 @@ MODULE_PARM_DESC(sdw_md_flags, "SoundWire Intel Master device flags (0x0 all off #define SDW_ALH_STRMZCFG(x) (0x000 + (0x4 * (x))) #define SDW_ALH_NUM_STREAMS 64 -#define SDW_ALH_STRMZCFG_DMAT_VAL 0x3 +#define SDW_ALH_STRMZCFG_DMAT_VAL 0xf #define SDW_ALH_STRMZCFG_DMAT GENMASK(7, 0) #define SDW_ALH_STRMZCFG_CHN GENMASK(19, 16) From 98ba2e6a42aeb528b38783abdd1bfaa5d0d844dc Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Mon, 19 Aug 2019 12:18:05 -0500 Subject: [PATCH 51/59] soundwire: intel: fix factor of two in MCLK handling Somehow Intel folks were confused, the property is 2x what the mclk frequency actually is, as checked with the actual bus frequency. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/intel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 22736b1113e03c..33e81941dc9b8c 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1211,6 +1211,7 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus) "intel-sdw-ip-clock", &prop->mclk_freq); + prop->mclk_freq /= 2; fwnode_property_read_u32(link, "intel-quirk-mask", &quirk_mask); From 58818918e94175cb1daafb2d638fd7cacf4d12a1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 26 Jun 2019 12:29:42 -0500 Subject: [PATCH 52/59] soundwire: cadence_master: log register write info useful for debug, but can be verbose so only enable if strictly needed. Signed-off-by: Bard Liao Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/cadence_master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index a97552a49babe8..d7ecd6ffcbccfe 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -196,6 +196,7 @@ static inline u32 cdns_readl(struct sdw_cdns *cdns, int offset) static inline void cdns_writel(struct sdw_cdns *cdns, int offset, u32 value) { + dev_vdbg(cdns->dev, "%s %x %x\n", __func__, offset, value); writel(value, cdns->registers + offset); } From 6bf1dfb99a4783e5edebde0225f60ba089bc002b Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Mon, 10 Jun 2019 19:21:39 -0500 Subject: [PATCH 53/59] add more traces to bus code Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index db717ac8b3d7b3..20aed3af2d1f93 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -502,6 +502,9 @@ static int sdw_assign_device_num(struct sdw_slave *slave) int ret, dev_num; bool new_device = false; + dev_dbg(slave->bus->dev, "in %s\n", __func__); + + dev_dbg(slave->bus->dev, "in %s\n", __func__); /* check first if device number is assigned, if so reuse that */ @@ -532,6 +535,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave) dev_num = slave->dev_num; slave->dev_num = 0; + dev_dbg(slave->bus->dev, "in %s, writing dev_num %d\n", __func__, dev_num); ret = sdw_write_no_pm(slave, SDW_SCP_DEVNUMBER, dev_num); if (ret < 0) { dev_err(&slave->dev, "Program device_num %d failed: %d\n", @@ -649,6 +653,8 @@ static int sdw_program_device_num(struct sdw_bus *bus) } while (ret == 0 && count < (SDW_MAX_DEVICES * 2)); + dev_err(bus->dev, "End of %s ret %d\n", __func__, ret); + return ret; } @@ -1136,14 +1142,17 @@ int sdw_handle_slave_status(struct sdw_bus *bus, break; case SDW_SLAVE_ATTACHED: - if (slave->status == SDW_SLAVE_ATTACHED) + if (slave->status == SDW_SLAVE_ATTACHED) { + dev_err(bus->dev, "Slave %d status attached, was already attached, ignoring\n", i); break; - + } prev_status = slave->status; sdw_modify_slave_status(slave, SDW_SLAVE_ATTACHED); - if (prev_status == SDW_SLAVE_ALERT) + if (prev_status == SDW_SLAVE_ALERT) { + dev_err(bus->dev, "Slave %d status attached, was alert, ignoring\n", i); break; + } attached_initializing = true; @@ -1161,6 +1170,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus, break; } + dev_err(bus->dev, "Updating Slave %d status\n", i); ret = sdw_update_slave_status(slave, status[i]); if (ret) dev_err(slave->bus->dev, From fbcd0aee674b379f045664d3a818cb7e85534e42 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 1 Aug 2019 18:31:34 -0500 Subject: [PATCH 54/59] add traces for bus Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/bus.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 20aed3af2d1f93..101bfcd22b38bb 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1080,6 +1080,8 @@ int sdw_handle_slave_status(struct sdw_bus *bus, bool attached_initializing; int i, ret = 0; + dev_dbg(bus->dev, "%s: start\n", __func__); + /* first check if any Slaves fell off the bus */ for (i = 1; i <= SDW_MAX_DEVICES; i++) { mutex_lock(&bus->bus_lock); @@ -1175,10 +1177,16 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (ret) dev_err(slave->bus->dev, "Update Slave status failed:%d\n", ret); + if (attached_initializing) complete(&slave->initialization_complete); + + dev_err(bus->dev, "%s: Updating Slave %d status done\n", + __func__, i); } + dev_dbg(bus->dev, "%s: end\n", __func__); + return ret; } EXPORT_SYMBOL(sdw_handle_slave_status); From ce172fea428fef25ef900be7ff841c1c64225104 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Wed, 11 Sep 2019 20:18:19 -0500 Subject: [PATCH 55/59] [HACK] soundwire: add dynamic DEBUG in makefile Signed-off-by: Pierre-Louis Bossart --- drivers/soundwire/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index eb37d69cbdca96..76a5c52b12b412 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -2,6 +2,7 @@ # # Makefile for soundwire core # +ccflags-y += -DDEBUG #Bus Objs soundwire-bus-objs := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o From 6d7a593fe7cbffd30ac10a6d3f7f25ef8028d18a Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 18 Nov 2019 10:50:32 +0900 Subject: [PATCH 56/59] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter This patch uses rtd instead of pcm at snd_soc_pcm_component_new/free() parameter. This is prepare for dai_link remove bug fix on topology. Signed-off-by: Kuninori Morimoto --- include/sound/soc-component.h | 4 ++-- sound/soc/soc-component.c | 8 +++----- sound/soc/soc-pcm.c | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 6aa3ecb7b6d371..f8fadf0e696aaf 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -412,7 +412,7 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream, unsigned long offset); int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma); -int snd_soc_pcm_component_new(struct snd_pcm *pcm); -void snd_soc_pcm_component_free(struct snd_pcm *pcm); +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd); +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd); #endif /* __SOC_COMPONENT_H */ diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 98ef0666add2af..1590e805d016d8 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -498,9 +498,8 @@ int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, return -EINVAL; } -int snd_soc_pcm_component_new(struct snd_pcm *pcm) +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; int ret; @@ -516,13 +515,12 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm) return 0; } -void snd_soc_pcm_component_free(struct snd_pcm *pcm) +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; for_each_rtd_components(rtd, rtdcom, component) if (component->driver->pcm_destruct) - component->driver->pcm_destruct(component, pcm); + component->driver->pcm_destruct(component, rtd->pcm); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4bf71e3211d807..c624d30bfa3c63 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2898,7 +2898,7 @@ static void soc_pcm_private_free(struct snd_pcm *pcm) /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(pcm); + snd_soc_pcm_component_free(rtd); } /* create a new pcm */ @@ -3036,7 +3036,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops); - ret = snd_soc_pcm_component_new(pcm); + ret = snd_soc_pcm_component_new(rtd); if (ret < 0) { dev_err(rtd->dev, "ASoC: pcm constructor failed: %d\n", ret); return ret; From ac926ced50b103c973c29b8aa735d3dfdda49597 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 18 Nov 2019 10:51:11 +0900 Subject: [PATCH 57/59] ASoC: soc-pcm: remove soc_pcm_private_free() soc-topology adds extra dai_link by using snd_soc_add_dai_link(), and removes it by snd_soc_romove_dai_link(). This snd_soc_add/remove_dai_link() and/or its related functions are unbalanced before, and now, these are balance-uped. But, it finds the random operation issue, and it is reported by Pierre-Louis. When card was released, topology will call snd_soc_remove_dai_link() via (A). static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; /* This should be called before snd_card_free() */ (A) soc_remove_link_components(card); /* free the ALSA card at first; this syncs with pending operations */ if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } /* remove and free each DAI */ (X) soc_remove_link_dais(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... } At (A), topology calls snd_soc_remove_dai_link(). Then topology rtd, and its related all data are freed. Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free() is called. static void soc_pcm_private_free(struct snd_pcm *pcm) { struct snd_soc_pcm_runtime *rtd = pcm->private_data; /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); snd_soc_pcm_component_free(rtd); } Here, it gets rtd via pcm->private_data. But, topology related rtd are already freed at (A). Normal sound card has no damage, becase it frees rtd at (C). These are finalizing rtd related data. Thus, these should be called when rtd was freed, not sound card was freed. It is very natural and understandable. In other words, pcm->private_free = soc_pcm_private_free() is no longer needed. Extra issue is that there is zero chance to call soc_remove_dai() for topology related dai at (X). Because (A) removes rtd connection from card too, and, (X) is based on card connected rtd. This means, (X) need to be called before (C) (= for normal sound) and (A) (= for topology). Now, I want to focus this patch which is the reason why snd_card_free() = (B) is located there. commit 4efda5f2130da033aeedc5b3205569893b910de2 ("ASoC: Fix use-after-free at card unregistration") Original snd_card_free() was called last of this function. But moved to top to avoid use-after-free issue. The issue was happen at soc_pcm_free() which was pcm->private_free, today it is updated/renamed to soc_pcm_private_free(). In other words, (B) need to be called before (C) (= for normal sound) and (A) (= for topology), because it needs (not yet freed) rtd. But, (A) need to be called before (B), because it needs card->snd_card pointer. If we call flush_delayed_work() and snd_soc_pcm_component_free() (= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)), there is no reason to call snd_card_free() at top of this function. It can be called end of this function, again. But, in such case, it will likely break unbind again, as Takashi-san reported. When unbind is performed in a busy state, the code may release still-in-use resources. At least we need to call snd_card_disconnect_sync() at the first place. The final code will be... static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; if (card->snd_card) (Z) snd_card_disconnect_sync(card->snd_card); (X) soc_remove_link_dais(card); (A) soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } } To avoid release still-in-use resources, call snd_card_disconnect_sync() at (Z). (X) is needed for both non-topology and topology. topology removes rtd via (A), and non topology removes rtd via (C). snd_card_free() is no longer related to use-after-free issue. Thus, locating (B) is no problem. Fixes: df95a16d2a9626 ("ASoC: soc-core: fix RIP warning on card removal") Fixes: bc7a9091e5b927 ("ASoC: soc-core: add soc_unbind_dai_link()") Reported-by: Pierre-Louis Bossart Signed-off-by: Kuninori Morimoto Tested-by: Pierre-Louis Bossart --- sound/soc/soc-core.c | 19 +++++++++++-------- sound/soc/soc-pcm.c | 10 ---------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 977a7bfad51979..e3a53ef1db043a 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -419,6 +419,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) list_del(&rtd->list); + flush_delayed_work(&rtd->delayed_work); + snd_soc_pcm_component_free(rtd); + /* * we don't need to call kfree() for rtd->dev * see @@ -1945,19 +1948,14 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, { struct snd_soc_dai_link *link, *_link; - /* This should be called before snd_card_free() */ - soc_remove_link_components(card); - - /* free the ALSA card at first; this syncs with pending operations */ - if (card->snd_card) { - snd_card_free(card->snd_card); - card->snd_card = NULL; - } + if (card->snd_card) + snd_card_disconnect_sync(card->snd_card); snd_soc_dapm_shutdown(card); /* remove and free each DAI */ soc_remove_link_dais(card); + soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) snd_soc_remove_dai_link(card, link); @@ -1972,6 +1970,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, /* remove the card */ if (card_probed && card->remove) card->remove(card); + + if (card->snd_card) { + snd_card_free(card->snd_card); + card->snd_card = NULL; + } } static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index c624d30bfa3c63..2c4f50c445912d 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2892,15 +2892,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) return ret; } -static void soc_pcm_private_free(struct snd_pcm *pcm) -{ - struct snd_soc_pcm_runtime *rtd = pcm->private_data; - - /* need to sync the delayed work before releasing resources */ - flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(rtd); -} - /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) { @@ -3042,7 +3033,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) return ret; } - pcm->private_free = soc_pcm_private_free; pcm->no_device_suspend = true; out: dev_info(rtd->card->dev, "%s <-> %s mapping ok\n", From c2c0a0786fe427f441c9ad4bb0657d6ebc90f427 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 14 Nov 2019 13:08:08 +0800 Subject: [PATCH 58/59] ASoC: SOF: Intel: hda: fix MSI issues by merging ipc and stream irq handlers The existing code uses two handlers for a shared edge-based MSI interrupts. In corner cases, interrupts are lost, leading to IPC timeouts. Those timeouts do not appear in legacy mode. This patch merges the two handlers into a single one. Follow-up patches will extend this work to SoundWire interrupts which are also shared with IPC and stream interrupts. Signed-off-by: Bard Liao --- sound/soc/sof/intel/apl.c | 1 - sound/soc/sof/intel/cnl.c | 1 - sound/soc/sof/intel/hda-ipc.c | 14 +++++----- sound/soc/sof/intel/hda-stream.c | 16 ++++++----- sound/soc/sof/intel/hda.c | 48 ++++++++++++++++++-------------- sound/soc/sof/intel/hda.h | 6 ++-- 6 files changed, 45 insertions(+), 41 deletions(-) diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index 7daa8eb456c8d1..6f45e14f2b2e32 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -41,7 +41,6 @@ const struct snd_sof_dsp_ops sof_apl_ops = { .block_write = sof_block_write, /* doorbell */ - .irq_handler = hda_dsp_ipc_irq_handler, .irq_thread = hda_dsp_ipc_irq_thread, /* ipc */ diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 0e1e265f3f3b3b..470d9e1e122904 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -231,7 +231,6 @@ const struct snd_sof_dsp_ops sof_cnl_ops = { .block_write = sof_block_write, /* doorbell */ - .irq_handler = hda_dsp_ipc_irq_handler, .irq_thread = cnl_ipc_irq_thread, /* ipc */ diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 0fd2153c176953..fac9f9762bb22c 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -237,14 +237,14 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context) return IRQ_HANDLED; } -/* is this IRQ for ADSP ? - we only care about IPC here */ -irqreturn_t hda_dsp_ipc_irq_handler(int irq, void *context) +/* Check if it is an IPC and disable IPC interrupt if yes */ +bool check_ipc(struct snd_sof_dev *sdev) { - struct snd_sof_dev *sdev = context; - int ret = IRQ_NONE; + bool ret = false; u32 irq_status; - spin_lock(&sdev->hw_lock); + /* The function can be called at irq thread, so use spin_lock_irq */ + spin_lock_irq(&sdev->hw_lock); /* store status */ irq_status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIS); @@ -260,11 +260,11 @@ irqreturn_t hda_dsp_ipc_irq_handler(int irq, void *context) snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPIC, HDA_DSP_ADSPIC_IPC, 0); - ret = IRQ_WAKE_THREAD; + ret = true; } out: - spin_unlock(&sdev->hw_lock); + spin_unlock_irq(&sdev->hw_lock); return ret; } diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 29ab4328167016..441d174df867f3 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -549,22 +549,23 @@ int hda_dsp_stream_hw_free(struct snd_sof_dev *sdev, return 0; } -irqreturn_t hda_dsp_stream_interrupt(int irq, void *context) +bool is_stream_irq(struct snd_sof_dev *sdev) { - struct hdac_bus *bus = context; - int ret = IRQ_WAKE_THREAD; + struct hdac_bus *bus = sof_to_bus(sdev); + bool ret = true; u32 status; - spin_lock(&bus->reg_lock); + /* The function can be called at irq thread, so use spin_lock_irq */ + spin_lock_irq(&bus->reg_lock); status = snd_hdac_chip_readl(bus, INTSTS); dev_vdbg(bus->dev, "stream irq, INTSTS status: 0x%x\n", status); /* Register inaccessible, ignore it.*/ if (status == 0xffffffff) - ret = IRQ_NONE; + ret = false; - spin_unlock(&bus->reg_lock); + spin_unlock_irq(&bus->reg_lock); return ret; } @@ -602,7 +603,8 @@ static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status) irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context) { - struct hdac_bus *bus = context; + struct snd_sof_dev *sdev = context; + struct hdac_bus *bus = sof_to_bus(sdev); #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) u32 rirb_status; #endif diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 91bd88fddac7aa..7a399d2663c90d 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -499,6 +499,28 @@ static const struct sof_intel_dsp_desc return chip_info; } +irqreturn_t hda_dsp_interrupt_handler(int irq, void *context) +{ + struct snd_sof_dev *sdev = context; + + if (check_ipc(sdev) || is_stream_irq(sdev)) + return IRQ_WAKE_THREAD; + + return IRQ_NONE; +} + +irqreturn_t hda_dsp_interrupt_thread(int irq, void *context) +{ + struct snd_sof_dev *sdev = context; + + if (check_ipc(sdev)) + sof_ops(sdev)->irq_thread(irq, sdev); + if (is_stream_irq(sdev)) + hda_dsp_stream_threaded_handler(irq, sdev); + + return IRQ_HANDLED; +} + int hda_dsp_probe(struct snd_sof_dev *sdev) { struct pci_dev *pci = to_pci_dev(sdev->dev); @@ -603,9 +625,7 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) */ if (hda_use_msi && pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_MSI) > 0) { dev_info(sdev->dev, "use msi interrupt mode\n"); - hdev->irq = pci_irq_vector(pci, 0); - /* ipc irq number is the same of hda irq */ - sdev->ipc_irq = hdev->irq; + sdev->ipc_irq = pci_irq_vector(pci, 0); /* initialised to "false" by kzalloc() */ sdev->msi_enabled = true; } @@ -616,28 +636,17 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) * in IO-APIC mode, hda->irq and ipc_irq are using the same * irq number of pci->irq */ - hdev->irq = pci->irq; sdev->ipc_irq = pci->irq; } - dev_dbg(sdev->dev, "using HDA IRQ %d\n", hdev->irq); - ret = request_threaded_irq(hdev->irq, hda_dsp_stream_interrupt, - hda_dsp_stream_threaded_handler, - IRQF_SHARED, "AudioHDA", bus); - if (ret < 0) { - dev_err(sdev->dev, "error: failed to register HDA IRQ %d\n", - hdev->irq); - goto free_irq_vector; - } - dev_dbg(sdev->dev, "using IPC IRQ %d\n", sdev->ipc_irq); - ret = request_threaded_irq(sdev->ipc_irq, hda_dsp_ipc_irq_handler, - sof_ops(sdev)->irq_thread, IRQF_SHARED, - "AudioDSP", sdev); + ret = request_threaded_irq(sdev->ipc_irq, hda_dsp_interrupt_handler, + hda_dsp_interrupt_thread, + IRQF_SHARED, "AudioDSP", sdev); if (ret < 0) { dev_err(sdev->dev, "error: failed to register IPC IRQ %d\n", sdev->ipc_irq); - goto free_hda_irq; + goto free_irq_vector; } pci_set_master(pci); @@ -668,8 +677,6 @@ int hda_dsp_probe(struct snd_sof_dev *sdev) free_ipc_irq: free_irq(sdev->ipc_irq, sdev); -free_hda_irq: - free_irq(hdev->irq, bus); free_irq_vector: if (sdev->msi_enabled) pci_free_irq_vectors(pci); @@ -715,7 +722,6 @@ int hda_dsp_remove(struct snd_sof_dev *sdev) SOF_HDA_PPCTL_GPROCEN, 0); free_irq(sdev->ipc_irq, sdev); - free_irq(hda->irq, bus); if (sdev->msi_enabled) pci_free_irq_vectors(pci); diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 18d7e72bf9b722..ab78a7cf845b44 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -406,8 +406,6 @@ struct sof_intel_hda_dev { /* the maximum number of streams (playback + capture) supported */ u32 stream_max; - int irq; - /* PM related */ bool l1_support_changed;/* during suspend, is L1SEN changed or not */ @@ -511,11 +509,12 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, struct snd_pcm_hw_params *params); int hda_dsp_stream_trigger(struct snd_sof_dev *sdev, struct hdac_ext_stream *stream, int cmd); -irqreturn_t hda_dsp_stream_interrupt(int irq, void *context); irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context); int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, struct hdac_stream *stream); +bool check_ipc(struct snd_sof_dev *sdev); +bool is_stream_irq(struct snd_sof_dev *sdev); struct hdac_ext_stream * hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction); @@ -540,7 +539,6 @@ void hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev); int hda_dsp_ipc_get_mailbox_offset(struct snd_sof_dev *sdev); int hda_dsp_ipc_get_window_offset(struct snd_sof_dev *sdev, u32 id); -irqreturn_t hda_dsp_ipc_irq_handler(int irq, void *context); irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context); int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir); From c50f2c841707cd2882a4beb0be349d8c9cd25c02 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Thu, 31 Oct 2019 16:57:54 +0800 Subject: [PATCH 59/59] ASoC: SOF: merge sof and sdw irq Now we have a single irq for SOF interrupts. We can further merge sof and sdw irqs. Signed-off-by: Bard Liao --- drivers/soundwire/intel_init.c | 28 ++-------------------------- include/linux/soundwire/sdw_intel.h | 17 +++++++++++++++++ sound/soc/sof/intel/hda.c | 13 +++++++++++++ 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index ed2a83989c72ff..235a2d533d3641 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -163,21 +163,7 @@ void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable) } EXPORT_SYMBOL(sdw_intel_enable_irq); -static irqreturn_t sdw_intel_irq(int irq, void *dev_id) -{ - struct sdw_intel_ctx *ctx = dev_id; - u32 int_status; - - int_status = readl(ctx->mmio_base + HDA_DSP_REG_ADSPIS2); - if (int_status & HDA_DSP_REG_ADSPIC2_SNDW) { - sdw_intel_enable_irq(ctx->mmio_base, false); - return IRQ_WAKE_THREAD; - } - - return IRQ_NONE; -} - -static irqreturn_t sdw_intel_thread(int irq, void *dev_id) +irqreturn_t sdw_intel_thread(int irq, void *dev_id) { struct sdw_intel_ctx *ctx = dev_id; struct sdw_intel_link_res *link; @@ -188,6 +174,7 @@ static irqreturn_t sdw_intel_thread(int irq, void *dev_id) sdw_intel_enable_irq(ctx->mmio_base, true); return IRQ_HANDLED; } +EXPORT_SYMBOL(sdw_intel_thread); static struct sdw_intel_ctx *sdw_intel_probe_controller(struct sdw_intel_res *res) @@ -199,7 +186,6 @@ static struct sdw_intel_ctx u32 link_mask; int count; int i; - int ret; if (!res) return NULL; @@ -261,15 +247,6 @@ static struct sdw_intel_ctx list_add_tail(&link->list, &ctx->link_list); } - ret = request_threaded_irq(ctx->irq, - sdw_intel_irq, sdw_intel_thread, - IRQF_SHARED, KBUILD_MODNAME, ctx); - if (ret < 0) { - dev_err(&adev->dev, "unable to grab IRQ %d, disabling device\n", - res->irq); - goto err; - } - return ctx; err: @@ -415,7 +392,6 @@ EXPORT_SYMBOL(sdw_intel_startup); */ void sdw_intel_exit(struct sdw_intel_ctx *ctx) { - free_irq(ctx->irq, ctx); sdw_intel_cleanup(ctx); kfree(ctx); } diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 9a9ce9fd1d71b2..8c07f4094f5d87 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -131,4 +131,21 @@ void sdw_intel_exit(struct sdw_intel_ctx *ctx); void sdw_intel_enable_irq(void __iomem *mmio_base, bool enable); +irqreturn_t sdw_intel_thread(int irq, void *dev_id); + +#define SDW_ADSPIS2 0x14 +#define SDW_ADSPIC_SNDW BIT(5) + +static inline +bool is_sdw_intel_irq(struct sdw_intel_ctx *ctx) +{ + u32 int_status; + + int_status = readl(ctx->mmio_base + SDW_ADSPIS2); + if (int_status & SDW_ADSPIC_SNDW) + return true; + + return false; +} + #endif diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 7a399d2663c90d..d3ebcdf4c18748 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -502,21 +502,34 @@ static const struct sof_intel_dsp_desc irqreturn_t hda_dsp_interrupt_handler(int irq, void *context) { struct snd_sof_dev *sdev = context; + struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; if (check_ipc(sdev) || is_stream_irq(sdev)) return IRQ_WAKE_THREAD; + if (hdev->sdw && is_sdw_intel_irq(hdev->sdw)) { + /* disable SDW interrupt */ + hda_sdw_int_enable(sdev, false); + return IRQ_WAKE_THREAD; + } + return IRQ_NONE; } irqreturn_t hda_dsp_interrupt_thread(int irq, void *context) { struct snd_sof_dev *sdev = context; + struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata; if (check_ipc(sdev)) sof_ops(sdev)->irq_thread(irq, sdev); if (is_stream_irq(sdev)) hda_dsp_stream_threaded_handler(irq, sdev); + if (hdev->sdw && is_sdw_intel_irq(hdev->sdw)) { + /* disable SDW interrupt */ + hda_sdw_int_enable(sdev, false); + sdw_intel_thread(irq, hdev->sdw); + } return IRQ_HANDLED; }