From ba8ec0f675d5c6bd3c16f7cc8e96c9c893fec9ab Mon Sep 17 00:00:00 2001 From: Srinivasa Rao Mandadapu Date: Wed, 13 Jul 2022 20:24:27 +0530 Subject: soundwire: qcom: Update error prints to debug prints Update error prints to debug prints to avoid redundant logging in kernel boot time, as these prints are informative prints in irq handler. Signed-off-by: Srinivasa Rao Mandadapu Reviewed-by: Andrew Halaney Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/1657724067-19004-1-git-send-email-quic_srivasam@quicinc.com Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 9df970eeca45..976ae75d1126 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -573,11 +573,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id) break; case SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED: case SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS: - dev_err_ratelimited(swrm->dev, "%s: SWR new slave attached\n", - __func__); + dev_dbg_ratelimited(swrm->dev, "SWR new slave attached\n"); swrm->reg_read(swrm, SWRM_MCP_SLV_STATUS, &slave_status); if (swrm->slave_status == slave_status) { - dev_err(swrm->dev, "Slave status not changed %x\n", + dev_dbg(swrm->dev, "Slave status not changed %x\n", slave_status); } else { qcom_swrm_get_device_status(swrm); -- cgit v1.2.3-58-ga151 From adc62cbd6b2108c592e09b9af8071ff93f160c71 Mon Sep 17 00:00:00 2001 From: Khalid Masum Date: Wed, 17 Aug 2022 13:48:59 +0600 Subject: soundwire: intel: Remove unnecessary TODO The capabilities enabled for multi-link are required as part of the programming sequences, even when a stream uses a single link we still use the syncArm/syncGo sequences. Therefore the TODO is no longer necessary. Suggested-by: Pierre-Louis Bossart Signed-off-by: Khalid Masum Link: https://lore.kernel.org/r/20220817074859.4759-1-khalid.masum.92@gmail.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 8d4000664fa3..18158825a6d8 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -75,7 +75,6 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, /* * Initialize multi_link flag - * TODO: populate this flag by reading property from FW node */ bus->multi_link = false; if (bus->ops->read_prop) { -- cgit v1.2.3-58-ga151 From df55100551a34bddab02dff48d0296bda0659c02 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 11:09:19 +0800 Subject: soundwire: dmi-quirks: add remapping for HP Omen 16-k0005TX The DSDT for this device has a number of problems: a) it lists rt711 on link0 and link1, but link1 is disabled b) the rt711 entry on link0 uses the wrong v2 instead of v3 (SDCA) c) the rt1316 amplifier on link3 is not listed. Add a remapping table to work-around these BIOS shenanigans. BugLink: https://github.com/thesofproject/sof/issues/5955 Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823030919.2346629-1-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/dmi-quirks.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/drivers/soundwire/dmi-quirks.c b/drivers/soundwire/dmi-quirks.c index 747983743a14..f81cdd83ec26 100644 --- a/drivers/soundwire/dmi-quirks.c +++ b/drivers/soundwire/dmi-quirks.c @@ -55,7 +55,26 @@ static const struct adr_remap dell_sku_0A3E[] = { {} }; +/* + * The HP Omen 16-k0005TX does not expose the correct version of RT711 on link0 + * and does not expose a RT1316 on link3 + */ +static const struct adr_remap hp_omen_16[] = { + /* rt711-sdca on link0 */ + { + 0x000020025d071100ull, + 0x000030025d071101ull + }, + /* rt1316-sdca on link3 */ + { + 0x000120025d071100ull, + 0x000330025d131601ull + }, + {} +}; + static const struct dmi_system_id adr_remap_quirk_table[] = { + /* TGL devices */ { .matches = { DMI_MATCH(DMI_SYS_VENDOR, "HP"), @@ -78,6 +97,14 @@ static const struct dmi_system_id adr_remap_quirk_table[] = { }, .driver_data = (void *)dell_sku_0A3E, }, + /* ADL devices */ + { + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "HP"), + DMI_MATCH(DMI_PRODUCT_NAME, "OMEN by HP Gaming Laptop 16-k0xxx"), + }, + .driver_data = (void *)hp_omen_16, + }, {} }; -- cgit v1.2.3-58-ga151 From 9af8c36aabe5294e147a0df61e95f941a7fff4cd Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:01:57 +0800 Subject: soundwire: bus: remove use of __func__ in dev_dbg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module and function information can be added with 'modprobe foo dyndbg=+pmf' Suggested-by: Greg KH Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Péter Ujfalusi Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823050158.2671245-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 18158825a6d8..704f75c0bae2 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -815,13 +815,13 @@ static void sdw_modify_slave_status(struct sdw_slave *slave, mutex_lock(&bus->bus_lock); dev_vdbg(bus->dev, - "%s: changing status slave %d status %d new status %d\n", - __func__, slave->dev_num, slave->status, status); + "changing status slave %d status %d new status %d\n", + slave->dev_num, slave->status, status); if (status == SDW_SLAVE_UNATTACHED) { dev_dbg(&slave->dev, - "%s: initializing enumeration and init completion for Slave %d\n", - __func__, slave->dev_num); + "initializing enumeration and init completion for Slave %d\n", + slave->dev_num); init_completion(&slave->enumeration_complete); init_completion(&slave->initialization_complete); @@ -829,8 +829,8 @@ static void sdw_modify_slave_status(struct sdw_slave *slave, } else if ((status == SDW_SLAVE_ATTACHED) && (slave->status == SDW_SLAVE_UNATTACHED)) { dev_dbg(&slave->dev, - "%s: signaling enumeration completion for Slave %d\n", - __func__, slave->dev_num); + "signaling enumeration completion for Slave %d\n", + slave->dev_num); complete(&slave->enumeration_complete); } @@ -1837,8 +1837,8 @@ int sdw_handle_slave_status(struct sdw_bus *bus, "Update Slave status failed:%d\n", ret); if (attached_initializing) { dev_dbg(&slave->dev, - "%s: signaling initialization completion for Slave %d\n", - __func__, slave->dev_num); + "signaling initialization completion for Slave %d\n", + slave->dev_num); complete(&slave->initialization_complete); -- cgit v1.2.3-58-ga151 From 63198aaa91acf64237bf5cfde3803ed388815150 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:01:58 +0800 Subject: soundwire: intel: remove use of __func__ in dev_dbg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module and function information can be added with 'modprobe foo dyndbg=+pmf' Suggested-by: Greg KH Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Péter Ujfalusi Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823050158.2671245-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 89d1d0d021fc..abb159d76b4a 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -290,11 +290,11 @@ static int intel_link_power_up(struct sdw_intel *sdw) syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24; if (!*shim_mask) { - dev_dbg(sdw->cdns.dev, "%s: powering up all links\n", __func__); + dev_dbg(sdw->cdns.dev, "powering up all links\n"); /* we first need to program the SyncPRD/CPU registers */ dev_dbg(sdw->cdns.dev, - "%s: first link up, programming SYNCPRD\n", __func__); + "first link up, programming SYNCPRD\n"); /* set SyncPRD period */ sync_reg = intel_readl(shim, SDW_SHIM_SYNC); @@ -476,7 +476,7 @@ static int intel_link_power_down(struct sdw_intel *sdw) if (!*shim_mask) { - dev_dbg(sdw->cdns.dev, "%s: powering down all links\n", __func__); + dev_dbg(sdw->cdns.dev, "powering down all links\n"); /* Link power down sequence */ link_control = intel_readl(shim, SDW_SHIM_LCTL); @@ -1553,11 +1553,11 @@ static int intel_resume_child_device(struct device *dev, void *data) struct sdw_slave *slave = dev_to_sdw_dev(dev); if (!slave->probed) { - dev_dbg(dev, "%s: skipping device, no probed driver\n", __func__); + dev_dbg(dev, "skipping device, no probed driver\n"); return 0; } if (!slave->dev_num_sticky) { - dev_dbg(dev, "%s: skipping device, never detected on bus\n", __func__); + dev_dbg(dev, "skipping device, never detected on bus\n"); return 0; } @@ -1643,7 +1643,7 @@ static int __maybe_unused intel_suspend(struct device *dev) } if (pm_runtime_suspended(dev)) { - dev_dbg(dev, "%s: pm_runtime status: suspended\n", __func__); + dev_dbg(dev, "pm_runtime status: suspended\n"); clock_stop_quirks = sdw->link_res->clock_stop_quirks; @@ -1764,7 +1764,7 @@ static int __maybe_unused intel_resume(struct device *dev) multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); if (pm_runtime_suspended(dev)) { - dev_dbg(dev, "%s: pm_runtime status was suspended, forcing active\n", __func__); + dev_dbg(dev, "pm_runtime status was suspended, forcing active\n"); /* follow required sequence from runtime_pm.rst */ pm_runtime_disable(dev); -- cgit v1.2.3-58-ga151 From 88de0a8f48bc2339e8c279315320e6f406ff5f96 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 12:50:02 +0800 Subject: soundwire: bus: rename sdw_ida as sdw_bus_ida To avoid confusions with follow-up patches using a IDA mechanism for peripheral 'device number' allocation, rename sdw_ida as sdw_bus_ida. Pure rename, no functionality change. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823045004.2670658-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 704f75c0bae2..1823a7414d04 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -11,11 +11,11 @@ #include "bus.h" #include "sysfs_local.h" -static DEFINE_IDA(sdw_ida); +static DEFINE_IDA(sdw_bus_ida); static int sdw_get_id(struct sdw_bus *bus) { - int rc = ida_alloc(&sdw_ida, GFP_KERNEL); + int rc = ida_alloc(&sdw_bus_ida, GFP_KERNEL); if (rc < 0) return rc; @@ -178,7 +178,7 @@ void sdw_bus_master_delete(struct sdw_bus *bus) sdw_master_device_del(bus); sdw_bus_debugfs_exit(bus); - ida_free(&sdw_ida, bus->id); + ida_free(&sdw_bus_ida, bus->id); } EXPORT_SYMBOL(sdw_bus_master_delete); -- cgit v1.2.3-58-ga151 From c60561014257699d81ab392eb6cb6389ad8907df Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 12:50:03 +0800 Subject: soundwire: bus: allow device number to be unique at system level The SoundWire specification allows the device number to be allocated at will. When a system includes multiple SoundWire links, the device number scope is limited to the link to which the device is attached. However, for integration/debug it can be convenient to have a unique device number across the system. This patch adds a 'dev_num_ida_min' field at the bus level, which when set will be used to allocate an IDA. The allocation happens when a hardware device reports as ATTACHED. If any error happens during the enumeration, the allocated IDA is not freed - the device number will be reused if/when the device re-joins the bus. The IDA is only freed when the Linux device is unregistered. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823045004.2670658-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 23 +++++++++++++++++------ include/linux/soundwire/sdw.h | 4 ++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 1823a7414d04..d773eee71bc1 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -12,6 +12,7 @@ #include "sysfs_local.h" static DEFINE_IDA(sdw_bus_ida); +static DEFINE_IDA(sdw_peripheral_ida); static int sdw_get_id(struct sdw_bus *bus) { @@ -156,9 +157,11 @@ static int sdw_delete_slave(struct device *dev, void *data) mutex_lock(&bus->bus_lock); - if (slave->dev_num) /* clear dev_num if assigned */ + if (slave->dev_num) { /* clear dev_num if assigned */ clear_bit(slave->dev_num, bus->assigned); - + if (bus->dev_num_ida_min) + ida_free(&sdw_peripheral_ida, slave->dev_num); + } list_del_init(&slave->node); mutex_unlock(&bus->bus_lock); @@ -638,10 +641,18 @@ static int sdw_get_device_num(struct sdw_slave *slave) { int bit; - bit = find_first_zero_bit(slave->bus->assigned, SDW_MAX_DEVICES); - if (bit == SDW_MAX_DEVICES) { - bit = -ENODEV; - goto err; + if (slave->bus->dev_num_ida_min) { + bit = ida_alloc_range(&sdw_peripheral_ida, + slave->bus->dev_num_ida_min, SDW_MAX_DEVICES, + GFP_KERNEL); + if (bit < 0) + goto err; + } else { + bit = find_first_zero_bit(slave->bus->assigned, SDW_MAX_DEVICES); + if (bit == SDW_MAX_DEVICES) { + bit = -ENODEV; + goto err; + } } /* diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 39058c841469..a2b31d25ea27 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -889,6 +889,9 @@ struct sdw_master_ops { * meaningful if multi_link is set. If set to 1, hardware-based * synchronization will be used even if a stream only uses a single * SoundWire segment. + * @dev_num_ida_min: if set, defines the minimum values for the IDA + * used to allocate system-unique device numbers. This value needs to be + * identical across all SoundWire bus in the system. */ struct sdw_bus { struct device *dev; @@ -913,6 +916,7 @@ struct sdw_bus { u32 bank_switch_timeout; bool multi_link; int hw_sync_min_links; + int dev_num_ida_min; }; int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent, -- cgit v1.2.3-58-ga151 From 1f2dcf3a154acf2c33c95859d8a2023fae596a04 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 12:50:04 +0800 Subject: soundwire: intel: set dev_num_ida_min The allowed values for SoundWire device numbers are between 1 and 11 (inclusive). HDaudio/iDISP codecs typically use SDI values 0..3 (inclusive). To allow for a unique peripheral SDI/dev_number across HDaudio and SoundWire buses, we set the minimum base to 4. This still allows for 8 SoundWire peripherals in the system, currently more than needed in actual products. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823045004.2670658-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index abb159d76b4a..25ec9c272239 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -22,6 +22,9 @@ #include "bus.h" #include "intel.h" +/* IDA min selected to avoid conflicts with HDaudio/iDISP SDI values */ +#define INTEL_DEV_NUM_IDA_MIN 4 + #define INTEL_MASTER_SUSPEND_DELAY_MS 3000 #define INTEL_MASTER_RESET_ITERATIONS 10 @@ -1307,6 +1310,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev, cdns->msg_count = 0; bus->link_id = auxdev->id; + bus->dev_num_ida_min = INTEL_DEV_NUM_IDA_MIN; sdw_cdns_probe(cdns); -- cgit v1.2.3-58-ga151 From 7f8170686d1733321841b21da8c2cd09ddb922b7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:36 +0800 Subject: soundwire: intel: cleanup definition of LCOUNT Add definition in header file rather than hidden in code. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 2 +- drivers/soundwire/intel_init.c | 2 +- include/linux/soundwire/sdw_intel.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 25ec9c272239..2dddead2df61 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -138,7 +138,7 @@ static int intel_reg_show(struct seq_file *s_file, void *data) if (!buf) return -ENOMEM; - links = intel_readl(s, SDW_SHIM_LCAP) & GENMASK(2, 0); + links = intel_readl(s, SDW_SHIM_LCAP) & SDW_SHIM_LCAP_LCOUNT_MASK; ret = scnprintf(buf, RD_BUF, "Register Value\n"); ret += scnprintf(buf + ret, RD_BUF - ret, "\nShim\n"); diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 824f4f32d4dc..d091513919df 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -306,7 +306,7 @@ sdw_intel_startup_controller(struct sdw_intel_ctx *ctx) /* Check SNDWLCAP.LCOUNT */ caps = ioread32(ctx->mmio_base + ctx->shim_base + SDW_SHIM_LCAP); - caps &= GENMASK(2, 0); + caps &= SDW_SHIM_LCAP_LCOUNT_MASK; /* Check HW supported vs property value */ if (caps < ctx->count) { diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index ec16ae49e6a4..49a3c265529b 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -15,7 +15,10 @@ #define SDW_LINK_SIZE 0x10000 /* Intel SHIM Registers Definition */ +/* LCAP */ #define SDW_SHIM_LCAP 0x0 +#define SDW_SHIM_LCAP_LCOUNT_MASK GENMASK(2, 0) + #define SDW_SHIM_LCTL 0x4 #define SDW_SHIM_IPPTR 0x8 #define SDW_SHIM_SYNC 0xC -- cgit v1.2.3-58-ga151 From feaa24aa408f117dbf074b580d38131dc819505a Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:37 +0800 Subject: soundwire: intel: regroup definitions for LCTL No functionality change, just regroup offset and bitfield definitions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 49a3c265529b..d9f51f43e42c 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -19,7 +19,14 @@ #define SDW_SHIM_LCAP 0x0 #define SDW_SHIM_LCAP_LCOUNT_MASK GENMASK(2, 0) +/* LCTL */ #define SDW_SHIM_LCTL 0x4 + +#define SDW_SHIM_LCTL_SPA BIT(0) +#define SDW_SHIM_LCTL_SPA_MASK GENMASK(3, 0) +#define SDW_SHIM_LCTL_CPA BIT(8) +#define SDW_SHIM_LCTL_CPA_MASK GENMASK(11, 8) + #define SDW_SHIM_IPPTR 0x8 #define SDW_SHIM_SYNC 0xC @@ -39,11 +46,6 @@ #define SDW_SHIM_WAKEEN 0x190 #define SDW_SHIM_WAKESTS 0x192 -#define SDW_SHIM_LCTL_SPA BIT(0) -#define SDW_SHIM_LCTL_SPA_MASK GENMASK(3, 0) -#define SDW_SHIM_LCTL_CPA BIT(8) -#define SDW_SHIM_LCTL_CPA_MASK GENMASK(11, 8) - #define SDW_SHIM_SYNC_SYNCPRD_VAL_24 (24000 / SDW_CADENCE_GSYNC_KHZ - 1) #define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1) #define SDW_SHIM_SYNC_SYNCPRD GENMASK(14, 0) -- cgit v1.2.3-58-ga151 From c36b610047463a37203e1158aeaf90f0a82f45dc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:38 +0800 Subject: soundwire: intel: remove IPPTR unused definition This read-only register only defines an offset which is known already and a version which isn't used. Remove unused definition. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index d9f51f43e42c..581a9ba32f82 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -27,7 +27,6 @@ #define SDW_SHIM_LCTL_CPA BIT(8) #define SDW_SHIM_LCTL_CPA_MASK GENMASK(11, 8) -#define SDW_SHIM_IPPTR 0x8 #define SDW_SHIM_SYNC 0xC #define SDW_SHIM_CTLSCAP(x) (0x010 + 0x60 * (x)) -- cgit v1.2.3-58-ga151 From ca33a58d12d3c75a3b28cc97037563bb6e03be7c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:39 +0800 Subject: soundwire: intel: cleanup SHIM SYNC Regroup offset and bitfields, no functionality change Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 581a9ba32f82..66503cf29f48 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -27,8 +27,17 @@ #define SDW_SHIM_LCTL_CPA BIT(8) #define SDW_SHIM_LCTL_CPA_MASK GENMASK(11, 8) +/* SYNC */ #define SDW_SHIM_SYNC 0xC +#define SDW_SHIM_SYNC_SYNCPRD_VAL_24 (24000 / SDW_CADENCE_GSYNC_KHZ - 1) +#define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1) +#define SDW_SHIM_SYNC_SYNCPRD GENMASK(14, 0) +#define SDW_SHIM_SYNC_SYNCCPU BIT(15) +#define SDW_SHIM_SYNC_CMDSYNC_MASK GENMASK(19, 16) +#define SDW_SHIM_SYNC_CMDSYNC BIT(16) +#define SDW_SHIM_SYNC_SYNCGO BIT(24) + #define SDW_SHIM_CTLSCAP(x) (0x010 + 0x60 * (x)) #define SDW_SHIM_CTLS0CM(x) (0x012 + 0x60 * (x)) #define SDW_SHIM_CTLS1CM(x) (0x014 + 0x60 * (x)) @@ -45,14 +54,6 @@ #define SDW_SHIM_WAKEEN 0x190 #define SDW_SHIM_WAKESTS 0x192 -#define SDW_SHIM_SYNC_SYNCPRD_VAL_24 (24000 / SDW_CADENCE_GSYNC_KHZ - 1) -#define SDW_SHIM_SYNC_SYNCPRD_VAL_38_4 (38400 / SDW_CADENCE_GSYNC_KHZ - 1) -#define SDW_SHIM_SYNC_SYNCPRD GENMASK(14, 0) -#define SDW_SHIM_SYNC_SYNCCPU BIT(15) -#define SDW_SHIM_SYNC_CMDSYNC_MASK GENMASK(19, 16) -#define SDW_SHIM_SYNC_CMDSYNC BIT(16) -#define SDW_SHIM_SYNC_SYNCGO BIT(24) - #define SDW_SHIM_PCMSCAP_ISS GENMASK(3, 0) #define SDW_SHIM_PCMSCAP_OSS GENMASK(7, 4) #define SDW_SHIM_PCMSCAP_BSS GENMASK(12, 8) -- cgit v1.2.3-58-ga151 From c27ce5c9dd178f7218de6fd29651ad04bd496d5c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:40 +0800 Subject: soundwire: intel: remove unused PDM capabilities We removed PDM support a long time ago but kept the definitions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 3 +-- include/linux/soundwire/sdw_intel.h | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 2dddead2df61..01be62fa6c83 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -170,9 +170,8 @@ static int intel_reg_show(struct seq_file *s_file, void *data) ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_PCMSYCHC(i, j)); } - ret += scnprintf(buf + ret, RD_BUF - ret, "\n PDMSCAP, IOCTL, CTMCTL\n"); + ret += scnprintf(buf + ret, RD_BUF - ret, "\n IOCTL, CTMCTL\n"); - ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_PDMSCAP(i)); ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_IOCTL(i)); ret += intel_sprintf(s, false, buf, ret, SDW_SHIM_CTMCTL(i)); } diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 66503cf29f48..34af5bc010b3 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -47,7 +47,6 @@ #define SDW_SHIM_PCMSYCHM(x, y) (0x022 + (0x60 * (x)) + (0x2 * (y))) #define SDW_SHIM_PCMSYCHC(x, y) (0x042 + (0x60 * (x)) + (0x2 * (y))) -#define SDW_SHIM_PDMSCAP(x) (0x062 + 0x60 * (x)) #define SDW_SHIM_IOCTL(x) (0x06C + 0x60 * (x)) #define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) @@ -63,11 +62,6 @@ #define SDW_SHIM_PCMSYCM_STREAM GENMASK(13, 8) #define SDW_SHIM_PCMSYCM_DIR BIT(15) -#define SDW_SHIM_PDMSCAP_ISS GENMASK(3, 0) -#define SDW_SHIM_PDMSCAP_OSS GENMASK(7, 4) -#define SDW_SHIM_PDMSCAP_BSS GENMASK(12, 8) -#define SDW_SHIM_PDMSCAP_CPSS GENMASK(15, 13) - #define SDW_SHIM_IOCTL_MIF BIT(0) #define SDW_SHIM_IOCTL_CO BIT(1) #define SDW_SHIM_IOCTL_COE BIT(2) -- cgit v1.2.3-58-ga151 From bd45a65dad8e13ba47f8fb3d6a6c68adecc96db7 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:41 +0800 Subject: soundwire: intel: add comment for control stream cap/chmap add comment and newline to mark out control stream capabilities and chmap. These registers are unused by the driver, only dumped in debugfs. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 34af5bc010b3..7c1d111c5f3f 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -38,11 +38,13 @@ #define SDW_SHIM_SYNC_CMDSYNC BIT(16) #define SDW_SHIM_SYNC_SYNCGO BIT(24) +/* Control stream capabililities and channel mask */ #define SDW_SHIM_CTLSCAP(x) (0x010 + 0x60 * (x)) #define SDW_SHIM_CTLS0CM(x) (0x012 + 0x60 * (x)) #define SDW_SHIM_CTLS1CM(x) (0x014 + 0x60 * (x)) #define SDW_SHIM_CTLS2CM(x) (0x016 + 0x60 * (x)) #define SDW_SHIM_CTLS3CM(x) (0x018 + 0x60 * (x)) + #define SDW_SHIM_PCMSCAP(x) (0x020 + 0x60 * (x)) #define SDW_SHIM_PCMSYCHM(x, y) (0x022 + (0x60 * (x)) + (0x2 * (y))) -- cgit v1.2.3-58-ga151 From 40f7a3ddf4e4eff8868d0b0e4ed072f41ab6a528 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:42 +0800 Subject: soundwire: intel: cleanup PCM stream capabilities Regroup offset and bitfields Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 7c1d111c5f3f..958628e936ea 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -45,8 +45,13 @@ #define SDW_SHIM_CTLS2CM(x) (0x016 + 0x60 * (x)) #define SDW_SHIM_CTLS3CM(x) (0x018 + 0x60 * (x)) +/* PCM Stream capabilities */ #define SDW_SHIM_PCMSCAP(x) (0x020 + 0x60 * (x)) +#define SDW_SHIM_PCMSCAP_ISS GENMASK(3, 0) +#define SDW_SHIM_PCMSCAP_OSS GENMASK(7, 4) +#define SDW_SHIM_PCMSCAP_BSS GENMASK(12, 8) + #define SDW_SHIM_PCMSYCHM(x, y) (0x022 + (0x60 * (x)) + (0x2 * (y))) #define SDW_SHIM_PCMSYCHC(x, y) (0x042 + (0x60 * (x)) + (0x2 * (y))) #define SDW_SHIM_IOCTL(x) (0x06C + 0x60 * (x)) @@ -55,10 +60,6 @@ #define SDW_SHIM_WAKEEN 0x190 #define SDW_SHIM_WAKESTS 0x192 -#define SDW_SHIM_PCMSCAP_ISS GENMASK(3, 0) -#define SDW_SHIM_PCMSCAP_OSS GENMASK(7, 4) -#define SDW_SHIM_PCMSCAP_BSS GENMASK(12, 8) - #define SDW_SHIM_PCMSYCM_LCHN GENMASK(3, 0) #define SDW_SHIM_PCMSYCM_HCHN GENMASK(7, 4) #define SDW_SHIM_PCMSYCM_STREAM GENMASK(13, 8) -- cgit v1.2.3-58-ga151 From 5c0d256201ec0e47e42ca8cafd3bee54f1c923f0 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:43 +0800 Subject: soundwire: intel: cleanup PCM Stream channel map and channel count Regroup offset and bitfield definitions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 958628e936ea..a1810701642e 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -52,19 +52,23 @@ #define SDW_SHIM_PCMSCAP_OSS GENMASK(7, 4) #define SDW_SHIM_PCMSCAP_BSS GENMASK(12, 8) +/* PCM Stream Channel Map */ #define SDW_SHIM_PCMSYCHM(x, y) (0x022 + (0x60 * (x)) + (0x2 * (y))) -#define SDW_SHIM_PCMSYCHC(x, y) (0x042 + (0x60 * (x)) + (0x2 * (y))) -#define SDW_SHIM_IOCTL(x) (0x06C + 0x60 * (x)) -#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) -#define SDW_SHIM_WAKEEN 0x190 -#define SDW_SHIM_WAKESTS 0x192 +/* PCM Stream Channel Count */ +#define SDW_SHIM_PCMSYCHC(x, y) (0x042 + (0x60 * (x)) + (0x2 * (y))) #define SDW_SHIM_PCMSYCM_LCHN GENMASK(3, 0) #define SDW_SHIM_PCMSYCM_HCHN GENMASK(7, 4) #define SDW_SHIM_PCMSYCM_STREAM GENMASK(13, 8) #define SDW_SHIM_PCMSYCM_DIR BIT(15) +#define SDW_SHIM_IOCTL(x) (0x06C + 0x60 * (x)) +#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) + +#define SDW_SHIM_WAKEEN 0x190 +#define SDW_SHIM_WAKESTS 0x192 + #define SDW_SHIM_IOCTL_MIF BIT(0) #define SDW_SHIM_IOCTL_CO BIT(1) #define SDW_SHIM_IOCTL_COE BIT(2) -- cgit v1.2.3-58-ga151 From 3ea29d33651db069080d1e3b12aea5ef36d766d1 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:44 +0800 Subject: soundwire: intel: cleanup IO control Regroup offset and bitfield definitions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-10-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index a1810701642e..a2e1927ac8ac 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -63,11 +63,8 @@ #define SDW_SHIM_PCMSYCM_STREAM GENMASK(13, 8) #define SDW_SHIM_PCMSYCM_DIR BIT(15) +/* IO control */ #define SDW_SHIM_IOCTL(x) (0x06C + 0x60 * (x)) -#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) - -#define SDW_SHIM_WAKEEN 0x190 -#define SDW_SHIM_WAKESTS 0x192 #define SDW_SHIM_IOCTL_MIF BIT(0) #define SDW_SHIM_IOCTL_CO BIT(1) @@ -79,6 +76,11 @@ #define SDW_SHIM_IOCTL_CIBD BIT(8) #define SDW_SHIM_IOCTL_DIBD BIT(9) +#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) + +#define SDW_SHIM_WAKEEN 0x190 +#define SDW_SHIM_WAKESTS 0x192 + #define SDW_SHIM_CTMCTL_DACTQE BIT(0) #define SDW_SHIM_CTMCTL_DODS BIT(1) #define SDW_SHIM_CTMCTL_DOAIS GENMASK(4, 3) -- cgit v1.2.3-58-ga151 From bc7b9595392407549158651bc760c9bda2ab8b5d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:45 +0800 Subject: soundwire: intel: cleanup AC Timing Control Regroup offset and bitfield definitions Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index a2e1927ac8ac..3a56fd5a6331 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -76,11 +76,12 @@ #define SDW_SHIM_IOCTL_CIBD BIT(8) #define SDW_SHIM_IOCTL_DIBD BIT(9) -#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) - #define SDW_SHIM_WAKEEN 0x190 #define SDW_SHIM_WAKESTS 0x192 +/* AC Timing control */ +#define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) + #define SDW_SHIM_CTMCTL_DACTQE BIT(0) #define SDW_SHIM_CTMCTL_DODS BIT(1) #define SDW_SHIM_CTMCTL_DOAIS GENMASK(4, 3) -- cgit v1.2.3-58-ga151 From 279e46bc298629b26436c90bd4b5d104cc1e0fb2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 23 Aug 2022 13:38:46 +0800 Subject: soundwire: intel: cleanup WakeEnable and WakeStatus Regroup offset and bitfield definitions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220823053846.2684635-12-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- include/linux/soundwire/sdw_intel.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 3a56fd5a6331..2e9fd91572d4 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -76,9 +76,16 @@ #define SDW_SHIM_IOCTL_CIBD BIT(8) #define SDW_SHIM_IOCTL_DIBD BIT(9) +/* Wake Enable*/ #define SDW_SHIM_WAKEEN 0x190 + +#define SDW_SHIM_WAKEEN_ENABLE BIT(0) + +/* Wake Status */ #define SDW_SHIM_WAKESTS 0x192 +#define SDW_SHIM_WAKESTS_STATUS BIT(0) + /* AC Timing control */ #define SDW_SHIM_CTMCTL(x) (0x06E + 0x60 * (x)) @@ -86,9 +93,6 @@ #define SDW_SHIM_CTMCTL_DODS BIT(1) #define SDW_SHIM_CTMCTL_DOAIS GENMASK(4, 3) -#define SDW_SHIM_WAKEEN_ENABLE BIT(0) -#define SDW_SHIM_WAKESTS_STATUS BIT(0) - /* Intel ALH Register definitions */ #define SDW_ALH_STRMZCFG(x) (0x000 + (0x4 * (x))) #define SDW_ALH_NUM_STREAMS 64 -- cgit v1.2.3-58-ga151 From fbbc73a20f38dcadf8a250bc761962588cd91f7e Mon Sep 17 00:00:00 2001 From: Simon Trimmer Date: Wed, 14 Sep 2022 17:02:44 +0100 Subject: soundwire: cadence: fix updating slave status when a bus has multiple peripherals The cadence IP explicitly reports slave status changes with bits for each possible change. The function cdns_update_slave_status() attempts to translate this into the current status of each of the slaves. However when there are multiple peripherals on a bus any slave that did not have a status change when the work function ran would not have it's status updated - the array is initialised to a value that equates to UNATTACHED and this can cause spurious reports that slaves had dropped off the bus. In the case where a slave has no status change or has multiple status changes the value from the last PING command is used. Signed-off-by: Simon Trimmer Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220914160248.1047627-2-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 57 +++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 4fbb19557f5e..245191d22ccd 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -782,6 +782,7 @@ static int cdns_update_slave_status(struct sdw_cdns *cdns, enum sdw_slave_status status[SDW_MAX_DEVICES + 1]; bool is_slave = false; u32 mask; + u32 val; int i, set_status; memset(status, 0, sizeof(status)); @@ -789,41 +790,38 @@ static int cdns_update_slave_status(struct sdw_cdns *cdns, for (i = 0; i <= SDW_MAX_DEVICES; i++) { mask = (slave_intstat >> (i * CDNS_MCP_SLAVE_STATUS_NUM)) & CDNS_MCP_SLAVE_STATUS_BITS; - if (!mask) - continue; - is_slave = true; set_status = 0; - if (mask & CDNS_MCP_SLAVE_INTSTAT_RESERVED) { - status[i] = SDW_SLAVE_RESERVED; - set_status++; - } + if (mask) { + is_slave = true; - if (mask & CDNS_MCP_SLAVE_INTSTAT_ATTACHED) { - status[i] = SDW_SLAVE_ATTACHED; - set_status++; - } - - if (mask & CDNS_MCP_SLAVE_INTSTAT_ALERT) { - status[i] = SDW_SLAVE_ALERT; - set_status++; - } + if (mask & CDNS_MCP_SLAVE_INTSTAT_RESERVED) { + status[i] = SDW_SLAVE_RESERVED; + set_status++; + } - if (mask & CDNS_MCP_SLAVE_INTSTAT_NPRESENT) { - status[i] = SDW_SLAVE_UNATTACHED; - set_status++; - } + if (mask & CDNS_MCP_SLAVE_INTSTAT_ATTACHED) { + status[i] = SDW_SLAVE_ATTACHED; + set_status++; + } - /* first check if Slave reported multiple status */ - if (set_status > 1) { - u32 val; + if (mask & CDNS_MCP_SLAVE_INTSTAT_ALERT) { + status[i] = SDW_SLAVE_ALERT; + set_status++; + } - dev_warn_ratelimited(cdns->dev, - "Slave %d reported multiple Status: %d\n", - i, mask); + if (mask & CDNS_MCP_SLAVE_INTSTAT_NPRESENT) { + status[i] = SDW_SLAVE_UNATTACHED; + set_status++; + } + } - /* check latest status extracted from PING commands */ + /* + * check that there was a single reported Slave status and when + * there is not use the latest status extracted from PING commands + */ + if (set_status != 1) { val = cdns_readl(cdns, CDNS_MCP_SLAVE_STAT); val >>= (i * 2); @@ -842,11 +840,6 @@ static int cdns_update_slave_status(struct sdw_cdns *cdns, status[i] = SDW_SLAVE_RESERVED; break; } - - dev_warn_ratelimited(cdns->dev, - "Slave %d status updated to %d\n", - i, status[i]); - } } -- cgit v1.2.3-58-ga151 From f605f32e59d8021a032746c2ca73b1adc46873d7 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 14 Sep 2022 17:02:45 +0100 Subject: soundwire: bus: Don't lose unattach notifications Ensure that if sdw_handle_slave_status() sees a peripheral has dropped off the bus it reports it to the client driver. If there are any devices reporting on address 0 it bails out after programming the device IDs. So it never reaches the second loop that calls sdw_update_slave_status(). If the missing device is one that is now showing as unenumerated it has been given a device ID so will report as attached next time sdw_handle_slave_status() runs. With the previous code the client driver would only see another ATTACHED notification because the UNATTACHED state was lost when sdw_handle_slave_status() bailed out after programming the device ID. This shows up most when the peripheral has to be reset after downloading updated firmware and there are multiple of these peripherals on the bus. They will all return to unenumerated state after the reset, and then there is a mix of unattached, attached and unenumerated PING states from the peripherals, as each is reset and they reboot. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220914160248.1047627-3-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index d773eee71bc1..1cc858b4107d 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1767,6 +1767,11 @@ int sdw_handle_slave_status(struct sdw_bus *bus, dev_warn(&slave->dev, "Slave %d state check1: UNATTACHED, status was %d\n", i, slave->status); sdw_modify_slave_status(slave, SDW_SLAVE_UNATTACHED); + + /* Ensure driver knows that peripheral unattached */ + ret = sdw_update_slave_status(slave, status[i]); + if (ret < 0) + dev_warn(&slave->dev, "Update Slave status failed:%d\n", ret); } } -- cgit v1.2.3-58-ga151 From 7297f8fa9a4312701c5066cd0d22e1a252cbb2d7 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 14 Sep 2022 17:02:46 +0100 Subject: soundwire: bus: Don't re-enumerate before status is UNATTACHED Don't re-enumerate a peripheral on #0 until we have seen and handled an UNATTACHED notification for that peripheral. Without this, it is possible for the UNATTACHED status to be missed and so the slave->status remains at ATTACHED. If slave->status never changes to UNATTACHED the child driver will never be notified of the UNATTACH, and the code in sdw_handle_slave_status() will skip the second part of enumeration because the slave->status has not changed. This scenario can happen because PINGs are handled in a workqueue function which is working from a snapshot of an old PING, and there is no guarantee when this function will run. A peripheral could report attached in the PING being handled by sdw_handle_slave_status(), but has since reverted to device #0 and is then found in the loop in sdw_program_device_num(). Previously the code would not have updated slave->status to UNATTACHED because it had not yet handled a PING where that peripheral had UNATTACHED. This situation happens fairly frequently with multiple peripherals on a bus that are intentionally reset (for example after downloading firmware). Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220914160248.1047627-4-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 1cc858b4107d..6e569a875a9b 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -773,6 +773,16 @@ static int sdw_program_device_num(struct sdw_bus *bus) if (sdw_compare_devid(slave, id) == 0) { found = true; + /* + * To prevent skipping state-machine stages don't + * program a device until we've seen it UNATTACH. + * Must return here because no other device on #0 + * can be detected until this one has been + * assigned a device ID. + */ + if (slave->status != SDW_SLAVE_UNATTACHED) + return 0; + /* * Assign a new dev_num to this Slave and * not mark it present. It will be marked -- cgit v1.2.3-58-ga151 From 0c5e99c41504b74dcfa9f3643f55cacab5c1e41f Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 14 Sep 2022 17:02:47 +0100 Subject: soundwire: cadence: Fix lost ATTACHED interrupts when enumerating The correct way to handle interrupts is to clear the bits we are about to handle _before_ handling them. Thus if the condition then re-asserts during the handling we won't lose it. This patch changes cdns_update_slave_status_work() to do this. The previous code cleared the interrupts after handling them. The problem with this is that when handling enumeration of devices the ATTACH statuses can be accidentally cleared and so some or all of the devices never complete their enumeration. Thus we can have a situation like this: - one or more devices are reverting to ID #0 - accumulated status bits indicate some devices attached and some on ID #0. (Remember: status bits are sticky until they are handled) - Because of device on #0 sdw_handle_slave_status() programs the device ID and exits without handling the other status, expecting to get an ATTACHED from this reprogrammed device. - The device immediately starts reporting ATTACHED in PINGs, which will assert its CDNS_MCP_SLAVE_INTSTAT_ATTACHED bit. - cdns_update_slave_status_work() clears INTSTAT0/1. If the initial status had CDNS_MCP_SLAVE_INTSTAT_ATTACHED bit set it will be cleared. - The ATTACHED change for the device has now been lost. - cdns_update_slave_status_work() clears CDNS_MCP_INT_SLAVE_MASK so if the new ATTACHED state had set it, it will be cleared without ever having been handled. Unless there is some other state change from another device to cause a new interrupt, the ATTACHED state of the reprogrammed device will never cause an interrupt so its enumeration will not be completed. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220914160248.1047627-5-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 245191d22ccd..be9cd47f31ec 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -954,9 +954,22 @@ static void cdns_update_slave_status_work(struct work_struct *work) u32 device0_status; int retry_count = 0; + /* + * Clear main interrupt first so we don't lose any assertions + * that happen during this function. + */ + cdns_writel(cdns, CDNS_MCP_INTSTAT, CDNS_MCP_INT_SLAVE_MASK); + slave0 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT0); slave1 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1); + /* + * Clear the bits before handling so we don't lose any + * bits that re-assert. + */ + cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0); + cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1); + /* combine the two status */ slave_intstat = ((u64)slave1 << 32) | slave0; @@ -964,8 +977,6 @@ static void cdns_update_slave_status_work(struct work_struct *work) update_status: cdns_update_slave_status(cdns, slave_intstat); - cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0); - cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1); /* * When there is more than one peripheral per link, it's @@ -982,6 +993,11 @@ update_status: * attention with PING commands. There is no need to check for * ALERTS since they are not allowed until a non-zero * device_number is assigned. + * + * Do not clear the INTSTAT0/1. While looping to enumerate devices on + * #0 there could be status changes on other devices - these must + * be kept in the INTSTAT so they can be handled when all #0 devices + * have been handled. */ device0_status = cdns_readl(cdns, CDNS_MCP_SLAVE_STAT); @@ -1001,8 +1017,7 @@ update_status: } } - /* clear and unmask Slave interrupt now */ - cdns_writel(cdns, CDNS_MCP_INTSTAT, CDNS_MCP_INT_SLAVE_MASK); + /* unmask Slave interrupt now */ cdns_updatel(cdns, CDNS_MCP_INTMASK, CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK); -- cgit v1.2.3-58-ga151 From 72124f07f0fab446caeea1f545d8c58b7549a899 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 14 Sep 2022 17:02:48 +0100 Subject: soundwire: bus: Don't exit early if no device IDs were programmed Only exit sdw_handle_slave_status() right after calling sdw_program_device_num() if it actually programmed an ID into at least one device. sdw_handle_slave_status() should protect itself against phantom device #0 ATTACHED indications. In that case there is no actual device still on #0. The early exit relies on there being a status change to ATTACHED on the reprogrammed device to trigger another call to sdw_handle_slave_status() which will then handle the status of all peripherals. If no device was actually programmed with an ID there won't be a new ATTACHED indication. This can lead to the status of other peripherals not being handled. The status passed to sdw_handle_slave_status() is obviously always from a point of time in the past, and may indicate accumulated unhandled events (depending how the bus manager operates). It's possible that a device ID is reprogrammed but the last PING status captured state just before that, when it was still reporting on ID #0. Then sdw_handle_slave_status() is called with this PING info, just before a new PING status is available showing it now on its new ID. So sdw_handle_slave_status() will receive a phantom report of a device on #0, but it will not find one. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220914160248.1047627-6-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 6e569a875a9b..8eded1a55227 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -729,7 +729,7 @@ void sdw_extract_slave_id(struct sdw_bus *bus, } EXPORT_SYMBOL(sdw_extract_slave_id); -static int sdw_program_device_num(struct sdw_bus *bus) +static int sdw_program_device_num(struct sdw_bus *bus, bool *programmed) { u8 buf[SDW_NUM_DEV_ID_REGISTERS] = {0}; struct sdw_slave *slave, *_s; @@ -739,6 +739,8 @@ static int sdw_program_device_num(struct sdw_bus *bus) int count = 0, ret; u64 addr; + *programmed = false; + /* No Slave, so use raw xfer api */ ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0, SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf); @@ -797,6 +799,8 @@ static int sdw_program_device_num(struct sdw_bus *bus) return ret; } + *programmed = true; + break; } } @@ -1756,7 +1760,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus, { enum sdw_slave_status prev_status; struct sdw_slave *slave; - bool attached_initializing; + bool attached_initializing, id_programmed; int i, ret = 0; /* first check if any Slaves fell off the bus */ @@ -1787,14 +1791,23 @@ int sdw_handle_slave_status(struct sdw_bus *bus, if (status[0] == SDW_SLAVE_ATTACHED) { dev_dbg(bus->dev, "Slave attached, programming device number\n"); - ret = sdw_program_device_num(bus); - if (ret < 0) - dev_err(bus->dev, "Slave attach failed: %d\n", ret); + /* - * programming a device number will have side effects, - * so we deal with other devices at a later time + * Programming a device number will have side effects, + * so we deal with other devices at a later time. + * This relies on those devices reporting ATTACHED, which will + * trigger another call to this function. This will only + * happen if at least one device ID was programmed. + * Error returns from sdw_program_device_num() are currently + * ignored because there's no useful recovery that can be done. + * Returning the error here could result in the current status + * of other devices not being handled, because if no device IDs + * were programmed there's nothing to guarantee a status change + * to trigger another call to this function. */ - return ret; + sdw_program_device_num(bus, &id_programmed); + if (id_programmed) + return 0; } /* Continue to check other slave statuses */ -- cgit v1.2.3-58-ga151 From ba05b39d265bdd16913f7684600d9d41e2796745 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 16 Sep 2022 11:35:05 +0100 Subject: soundwire: cadence: Don't overwrite msg->buf during write commands The buf passed in struct sdw_msg must only be written for a READ, in that case the RDATA part of the response is the data value of the register. For a write command there is no RDATA, and buf should be assumed to be const and unmodifable. The original caller should not expect its data buffer to be corrupted by an sdw_nwrite(). Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220916103505.1562210-1-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index be9cd47f31ec..3ef472049980 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -544,9 +544,12 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns, return SDW_CMD_IGNORED; } - /* fill response */ - for (i = 0; i < count; i++) - msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, cdns->response_buf[i]); + if (msg->flags == SDW_MSG_FLAG_READ) { + /* fill response */ + for (i = 0; i < count; i++) + msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, + cdns->response_buf[i]); + } return SDW_CMD_OK; } -- cgit v1.2.3-58-ga151 From 8039b6f3e5c777e41df34f8e996af18555a4f303 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Fri, 16 Sep 2022 14:53:51 +0100 Subject: soundwire: qcom: update status from device id 1 By default autoenumeration is enabled on QCom SoundWire controller which means the core should not be dealing with device 0 w.r.t enumeration. During Enumeration if SoundWire core sees status[0] as SDW_SLAVE_ATTACHED and start programming the device id, however reading DEVID registers return zeros which does not match to any of the slaves in the list and the core attempts to park this device to Group 13. This results in adding SoundWire device with enumeration address 0:0:0:0 Fix this by not passing device 0 status to SoundWire core. Signed-off-by: Srinivas Kandagatla Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220916135352.19114-1-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 976ae75d1126..c331e2db60fd 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -440,7 +440,7 @@ static void qcom_swrm_get_device_status(struct qcom_swrm_ctrl *ctrl) ctrl->reg_read(ctrl, SWRM_MCP_SLV_STATUS, &val); ctrl->slave_status = val; - for (i = 0; i < SDW_MAX_DEVICES; i++) { + for (i = 1; i <= SDW_MAX_DEVICES; i++) { u32 s; s = (val >> (i * 2)); -- cgit v1.2.3-58-ga151 From ed8d07acec73c34cbebd209ff7a37051424de60c Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Fri, 16 Sep 2022 14:53:52 +0100 Subject: soundwire: qcom: do not send status of device 0 during alert Device0 can not be in alert status. And for consistency reasons do not send status of device0 to core. Signed-off-by: Srinivas Kandagatla Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220916135352.19114-2-srinivas.kandagatla@linaro.org Signed-off-by: Vinod Koul --- drivers/soundwire/qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index c331e2db60fd..ba3c2ce84817 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -420,7 +420,7 @@ static int qcom_swrm_get_alert_slave_dev_num(struct qcom_swrm_ctrl *ctrl) ctrl->reg_read(ctrl, SWRM_MCP_SLV_STATUS, &val); - for (dev_num = 0; dev_num < SDW_MAX_DEVICES; dev_num++) { + for (dev_num = 1; dev_num <= SDW_MAX_DEVICES; dev_num++) { status = (val >> (dev_num * SWRM_MCP_SLV_STATUS_SZ)); if ((status & SWRM_MCP_SLV_STATUS_MASK) == SDW_SLAVE_ALERT) { -- cgit v1.2.3-58-ga151 From 560458df5f144fb3547a9fde8e0b7b2143ef0711 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Sat, 17 Sep 2022 15:02:56 +0100 Subject: soundwire: bus: Fix wrong port number in sdw_handle_slave_alerts() for_each_set_bit() gives the bit-number counting from 0 (LSbit==0). When processing INTSTAT2, bit 0 is DP4 so the port number is (bit + 4). Likewise for INTSTAT3 bit 0 is DP11 so port number is (bit + 11). Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220917140256.689678-1-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 8eded1a55227..df0ae869ee51 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1622,7 +1622,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) port = buf2[0] & SDW_SCP_INTSTAT2_PORT4_10; for_each_set_bit(bit, &port, 8) { /* scp2 ports start from 4 */ - port_num = bit + 3; + port_num = bit + 4; sdw_handle_port_interrupt(slave, port_num, &port_status[port_num]); @@ -1634,7 +1634,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) port = buf2[1] & SDW_SCP_INTSTAT3_PORT11_14; for_each_set_bit(bit, &port, 8) { /* scp3 ports start from 11 */ - port_num = bit + 10; + port_num = bit + 11; sdw_handle_port_interrupt(slave, port_num, &port_status[port_num]); -- cgit v1.2.3-58-ga151 From 3ed96fb4a6d8426f766687ff02fc5fb5f91575fd Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Sat, 17 Sep 2022 13:35:17 +0100 Subject: soundwire: cadence: Write to correct address for each FIFO chunk _cdns_xfer_msg() must add the fragment offset to msg->addr to get the base target address of each FIFO chunk. Otherwise every chunk will be written to the first 32 register addresses. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220917123517.229153-1-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 3ef472049980..ca241bbeadd9 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -569,7 +569,7 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd, } base = CDNS_MCP_CMD_BASE; - addr = msg->addr; + addr = msg->addr + offset; for (i = 0; i < count; i++) { data = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num); -- cgit v1.2.3-58-ga151 From 7f6bad4dfde0ec1d479fdcbbb62bccdbf3a93bb4 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Sat, 17 Sep 2022 16:48:21 +0100 Subject: soundwire: cadence: Fix error check in cdns_xfer_msg() _cdns_xfer_msg() returns an sdw_command_response value, not a negative error code. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220917154822.690472-1-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index ca241bbeadd9..3543a923ee6b 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -708,7 +708,7 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg) for (i = 0; i < msg->len / CDNS_MCP_CMD_LEN; i++) { ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN, CDNS_MCP_CMD_LEN, false); - if (ret < 0) + if (ret != SDW_CMD_OK) goto exit; } -- cgit v1.2.3-58-ga151 From bafb1eacfbd98c6cdbca7e1723ef933ad371cd51 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Sat, 17 Sep 2022 16:48:22 +0100 Subject: soundwire: cadence: Simplify error paths in cdns_xfer_msg() There's no need to goto an exit label to return from cdns_xfer_msg(). It doesn't do any cleanup, only a return statement. Replace the gotos with returns. Signed-off-by: Richard Fitzgerald Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20220917154822.690472-2-rf@opensource.cirrus.com Signed-off-by: Vinod Koul --- drivers/soundwire/cadence_master.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 3543a923ee6b..30b8c628fdbd 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -709,17 +709,14 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg) ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN, CDNS_MCP_CMD_LEN, false); if (ret != SDW_CMD_OK) - goto exit; + return ret; } if (!(msg->len % CDNS_MCP_CMD_LEN)) - goto exit; - - ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN, - msg->len % CDNS_MCP_CMD_LEN, false); + return SDW_CMD_OK; -exit: - return ret; + return _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN, + msg->len % CDNS_MCP_CMD_LEN, false); } EXPORT_SYMBOL(cdns_xfer_msg); -- cgit v1.2.3-58-ga151 From c6867cda906aadbce5e71efde9c78a26108b2bad Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:11 +0800 Subject: soundwire: intel: fix error handling on dai registration issues The call to intel_register_dai() may fail because of memory allocation issues or problems reported by the ASoC core. In all cases, when a error is thrown the component is not registered, it's invalid to unregister it. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-2-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 01be62fa6c83..3bb29bc00d5a 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1432,7 +1432,6 @@ int intel_link_startup(struct auxiliary_device *auxdev) ret = intel_register_dai(sdw); if (ret) { dev_err(dev, "DAI registration failed: %d\n", ret); - snd_soc_unregister_component(dev); goto err_interrupt; } -- cgit v1.2.3-58-ga151 From 54f391dd1b9fc34b37fc1824f9bd24430167f683 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:12 +0800 Subject: soundwire: intel: simplify flow and use devm_ for DAI registration We already use devm_ for memory allocation but not for component/DAI registration. The resource management can be based on devm_ in all cases. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-3-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 3bb29bc00d5a..a6fe91f2d964 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1203,8 +1203,8 @@ static int intel_register_dai(struct sdw_intel *sdw) if (ret) return ret; - return snd_soc_register_component(cdns->dev, &dai_component, - dais, num_dai); + return devm_snd_soc_register_component(cdns->dev, &dai_component, + dais, num_dai); } static int sdw_master_read_intel_prop(struct sdw_bus *bus) @@ -1489,7 +1489,6 @@ err_init: static void intel_link_remove(struct auxiliary_device *auxdev) { - struct device *dev = &auxdev->dev; struct sdw_cdns *cdns = auxiliary_get_drvdata(auxdev); struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_bus *bus = &cdns->bus; @@ -1502,7 +1501,6 @@ static void intel_link_remove(struct auxiliary_device *auxdev) if (!bus->prop.hw_disabled) { intel_debugfs_exit(sdw); sdw_cdns_enable_interrupt(cdns, false); - snd_soc_unregister_component(dev); } sdw_bus_master_delete(bus); } -- cgit v1.2.3-58-ga151 From aa425707c3c8aacfbc37b88a68d89b909b6291c3 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:13 +0800 Subject: soundwire: intel: move DAI registration and debugfs init earlier These two steps can and should be done before starting up the clock and the bus operation. This is a first step before re-grouping functionality in well-defined callbacks. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-4-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index a6fe91f2d964..d7852cc7dd96 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1393,6 +1393,15 @@ int intel_link_startup(struct auxiliary_device *auxdev) intel_pdi_ch_update(sdw); + /* Register DAIs */ + ret = intel_register_dai(sdw); + if (ret) { + dev_err(dev, "DAI registration failed: %d\n", ret); + goto err_init; + } + + intel_debugfs_init(sdw); + ret = sdw_cdns_enable_interrupt(cdns, true); if (ret < 0) { dev_err(dev, "cannot enable interrupts\n"); @@ -1428,15 +1437,6 @@ int intel_link_startup(struct auxiliary_device *auxdev) sdw_cdns_check_self_clearing_bits(cdns, __func__, true, INTEL_MASTER_RESET_ITERATIONS); - /* Register DAIs */ - ret = intel_register_dai(sdw); - if (ret) { - dev_err(dev, "DAI registration failed: %d\n", ret); - goto err_interrupt; - } - - intel_debugfs_init(sdw); - /* Enable runtime PM */ if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) { pm_runtime_set_autosuspend_delay(dev, -- cgit v1.2.3-58-ga151 From 30cbae662ba989ada383cf3db2a4c6400b9ae1fc Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:14 +0800 Subject: soundwire: intel: move all PDI initialization under intel_register_dai() Start regrouping functionality in high-level functions. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-5-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index d7852cc7dd96..676de04c5070 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1171,11 +1171,20 @@ static int intel_create_dai(struct sdw_cdns *cdns, static int intel_register_dai(struct sdw_intel *sdw) { + struct sdw_cdns_stream_config config; struct sdw_cdns *cdns = &sdw->cdns; struct sdw_cdns_streams *stream; struct snd_soc_dai_driver *dais; int num_dai, ret, off = 0; + /* Read the PDI config and initialize cadence PDI */ + intel_pdi_init(sdw, &config); + ret = sdw_cdns_pdi_init(cdns, config); + if (ret) + return ret; + + intel_pdi_ch_update(sdw); + /* DAIs are created based on total number of PDIs supported */ num_dai = cdns->pcm.num_pdi; @@ -1347,7 +1356,6 @@ static int intel_link_probe(struct auxiliary_device *auxdev, int intel_link_startup(struct auxiliary_device *auxdev) { - struct sdw_cdns_stream_config config; struct device *dev = &auxdev->dev; struct sdw_cdns *cdns = auxiliary_get_drvdata(auxdev); struct sdw_intel *sdw = cdns_to_intel(cdns); @@ -1385,14 +1393,6 @@ int intel_link_startup(struct auxiliary_device *auxdev) if (ret) goto err_init; - /* Read the PDI config and initialize cadence PDI */ - intel_pdi_init(sdw, &config); - ret = sdw_cdns_pdi_init(cdns, config); - if (ret) - goto err_init; - - intel_pdi_ch_update(sdw); - /* Register DAIs */ ret = intel_register_dai(sdw); if (ret) { -- cgit v1.2.3-58-ga151 From a658fd8d2b60f040ea042fd79bbd9c58e5f2b911 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:15 +0800 Subject: soundwire: intel: remove clock_stop parameter in intel_shim_init() Simplify signature before further code reshuffling. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-6-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 676de04c5070..2d828d98e153 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -391,7 +391,7 @@ static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw) /* at this point Integration Glue has full control of the I/Os */ } -static int intel_shim_init(struct sdw_intel *sdw, bool clock_stop) +static int intel_shim_init(struct sdw_intel *sdw) { void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; @@ -1277,14 +1277,10 @@ static struct sdw_master_ops sdw_intel_ops = { static int intel_init(struct sdw_intel *sdw) { - bool clock_stop; - /* Initialize shim and controller */ intel_link_power_up(sdw); - clock_stop = sdw_cdns_is_clock_stop(&sdw->cdns); - - intel_shim_init(sdw, clock_stop); + intel_shim_init(sdw); return 0; } -- cgit v1.2.3-58-ga151 From bc8729476a537ff372c33588189c6ef5b39ce081 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:16 +0800 Subject: soundwire: intel: move shim initialization before power up/down Move code around before additional simplification. No functionality change. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-7-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 159 +++++++++++++++++++++++----------------------- 1 file changed, 79 insertions(+), 80 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 2d828d98e153..140cf36eb407 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -260,86 +260,6 @@ static void intel_debugfs_exit(struct sdw_intel *sdw) {} /* * shim ops */ - -static int intel_link_power_up(struct sdw_intel *sdw) -{ - unsigned int link_id = sdw->instance; - void __iomem *shim = sdw->link_res->shim; - u32 *shim_mask = sdw->link_res->shim_mask; - struct sdw_bus *bus = &sdw->cdns.bus; - struct sdw_master_prop *prop = &bus->prop; - u32 spa_mask, cpa_mask; - u32 link_control; - int ret = 0; - u32 syncprd; - u32 sync_reg; - - mutex_lock(sdw->link_res->shim_lock); - - /* - * The hardware relies on an internal counter, typically 4kHz, - * to generate the SoundWire SSP - which defines a 'safe' - * synchronization point between commands and audio transport - * and allows for multi link synchronization. The SYNCPRD value - * is only dependent on the oscillator clock provided to - * the IP, so adjust based on _DSD properties reported in DSDT - * tables. The values reported are based on either 24MHz - * (CNL/CML) or 38.4 MHz (ICL/TGL+). - */ - if (prop->mclk_freq % 6000000) - syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_38_4; - else - syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24; - - if (!*shim_mask) { - dev_dbg(sdw->cdns.dev, "powering up all links\n"); - - /* we first need to program the SyncPRD/CPU registers */ - dev_dbg(sdw->cdns.dev, - "first link up, programming SYNCPRD\n"); - - /* set SyncPRD period */ - sync_reg = intel_readl(shim, SDW_SHIM_SYNC); - u32p_replace_bits(&sync_reg, syncprd, SDW_SHIM_SYNC_SYNCPRD); - - /* Set SyncCPU bit */ - sync_reg |= SDW_SHIM_SYNC_SYNCCPU; - intel_writel(shim, SDW_SHIM_SYNC, sync_reg); - - /* Link power up sequence */ - link_control = intel_readl(shim, SDW_SHIM_LCTL); - - /* only power-up enabled links */ - spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, sdw->link_res->link_mask); - cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, sdw->link_res->link_mask); - - link_control |= spa_mask; - - ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask); - if (ret < 0) { - dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret); - goto out; - } - - /* SyncCPU will change once link is active */ - ret = intel_wait_bit(shim, SDW_SHIM_SYNC, - SDW_SHIM_SYNC_SYNCCPU, 0); - if (ret < 0) { - dev_err(sdw->cdns.dev, - "Failed to set SHIM_SYNC: %d\n", ret); - goto out; - } - } - - *shim_mask |= BIT(link_id); - - sdw->cdns.link_up = true; -out: - mutex_unlock(sdw->link_res->shim_lock); - - return ret; -} - /* this needs to be called with shim_lock */ static void intel_shim_glue_to_master_ip(struct sdw_intel *sdw) { @@ -456,6 +376,85 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable) mutex_unlock(sdw->link_res->shim_lock); } +static int intel_link_power_up(struct sdw_intel *sdw) +{ + unsigned int link_id = sdw->instance; + void __iomem *shim = sdw->link_res->shim; + u32 *shim_mask = sdw->link_res->shim_mask; + struct sdw_bus *bus = &sdw->cdns.bus; + struct sdw_master_prop *prop = &bus->prop; + u32 spa_mask, cpa_mask; + u32 link_control; + int ret = 0; + u32 syncprd; + u32 sync_reg; + + mutex_lock(sdw->link_res->shim_lock); + + /* + * The hardware relies on an internal counter, typically 4kHz, + * to generate the SoundWire SSP - which defines a 'safe' + * synchronization point between commands and audio transport + * and allows for multi link synchronization. The SYNCPRD value + * is only dependent on the oscillator clock provided to + * the IP, so adjust based on _DSD properties reported in DSDT + * tables. The values reported are based on either 24MHz + * (CNL/CML) or 38.4 MHz (ICL/TGL+). + */ + if (prop->mclk_freq % 6000000) + syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_38_4; + else + syncprd = SDW_SHIM_SYNC_SYNCPRD_VAL_24; + + if (!*shim_mask) { + dev_dbg(sdw->cdns.dev, "powering up all links\n"); + + /* we first need to program the SyncPRD/CPU registers */ + dev_dbg(sdw->cdns.dev, + "first link up, programming SYNCPRD\n"); + + /* set SyncPRD period */ + sync_reg = intel_readl(shim, SDW_SHIM_SYNC); + u32p_replace_bits(&sync_reg, syncprd, SDW_SHIM_SYNC_SYNCPRD); + + /* Set SyncCPU bit */ + sync_reg |= SDW_SHIM_SYNC_SYNCCPU; + intel_writel(shim, SDW_SHIM_SYNC, sync_reg); + + /* Link power up sequence */ + link_control = intel_readl(shim, SDW_SHIM_LCTL); + + /* only power-up enabled links */ + spa_mask = FIELD_PREP(SDW_SHIM_LCTL_SPA_MASK, sdw->link_res->link_mask); + cpa_mask = FIELD_PREP(SDW_SHIM_LCTL_CPA_MASK, sdw->link_res->link_mask); + + link_control |= spa_mask; + + ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask); + if (ret < 0) { + dev_err(sdw->cdns.dev, "Failed to power up link: %d\n", ret); + goto out; + } + + /* SyncCPU will change once link is active */ + ret = intel_wait_bit(shim, SDW_SHIM_SYNC, + SDW_SHIM_SYNC_SYNCCPU, 0); + if (ret < 0) { + dev_err(sdw->cdns.dev, + "Failed to set SHIM_SYNC: %d\n", ret); + goto out; + } + } + + *shim_mask |= BIT(link_id); + + sdw->cdns.link_up = true; +out: + mutex_unlock(sdw->link_res->shim_lock); + + return ret; +} + static int intel_link_power_down(struct sdw_intel *sdw) { u32 link_control, spa_mask, cpa_mask; -- cgit v1.2.3-58-ga151 From b81bcdb424d05a18b4e928e2f9f6c25d90c5c35d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:17 +0800 Subject: soundwire: intel: remove intel_init() wrapper We can directly call intel_link_power_up and do power_up+shim_init in the same function. This simplifies the code with a better symmetry between power_up and power_down operations. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-8-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 140cf36eb407..4eeb2b5c1594 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -311,15 +311,13 @@ static void intel_shim_master_ip_to_glue(struct sdw_intel *sdw) /* at this point Integration Glue has full control of the I/Os */ } -static int intel_shim_init(struct sdw_intel *sdw) +/* this needs to be called with shim_lock */ +static void intel_shim_init(struct sdw_intel *sdw) { void __iomem *shim = sdw->link_res->shim; unsigned int link_id = sdw->instance; - int ret = 0; u16 ioctl = 0, act = 0; - mutex_lock(sdw->link_res->shim_lock); - /* Initialize Shim */ ioctl |= SDW_SHIM_IOCTL_BKE; intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl); @@ -344,10 +342,6 @@ static int intel_shim_init(struct sdw_intel *sdw) act |= SDW_SHIM_CTMCTL_DODS; intel_writew(shim, SDW_SHIM_CTMCTL(link_id), act); usleep_range(10, 15); - - mutex_unlock(sdw->link_res->shim_lock); - - return ret; } static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable) @@ -449,6 +443,9 @@ static int intel_link_power_up(struct sdw_intel *sdw) *shim_mask |= BIT(link_id); sdw->cdns.link_up = true; + + intel_shim_init(sdw); + out: mutex_unlock(sdw->link_res->shim_lock); @@ -1274,16 +1271,6 @@ static struct sdw_master_ops sdw_intel_ops = { .post_bank_switch = intel_post_bank_switch, }; -static int intel_init(struct sdw_intel *sdw) -{ - /* Initialize shim and controller */ - intel_link_power_up(sdw); - - intel_shim_init(sdw); - - return 0; -} - /* * probe and init (aux_dev_id argument is required by function prototype but not used) */ @@ -1384,7 +1371,7 @@ int intel_link_startup(struct auxiliary_device *auxdev) } /* Initialize shim, controller */ - ret = intel_init(sdw); + ret = intel_link_power_up(sdw); if (ret) goto err_init; @@ -1773,7 +1760,7 @@ static int __maybe_unused intel_resume(struct device *dev) pm_runtime_idle(dev); } - ret = intel_init(sdw); + ret = intel_link_power_up(sdw); if (ret) { dev_err(dev, "%s failed: %d\n", __func__, ret); return ret; @@ -1862,7 +1849,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) clock_stop_quirks = sdw->link_res->clock_stop_quirks; if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) { - ret = intel_init(sdw); + ret = intel_link_power_up(sdw); if (ret) { dev_err(dev, "%s failed: %d\n", __func__, ret); return ret; @@ -1910,7 +1897,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) true, INTEL_MASTER_RESET_ITERATIONS); } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) { - ret = intel_init(sdw); + ret = intel_link_power_up(sdw); if (ret) { dev_err(dev, "%s failed: %d\n", __func__, ret); return ret; @@ -1991,7 +1978,7 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) if (!clock_stop0) dev_err(dev, "%s invalid configuration, clock was not stopped", __func__); - ret = intel_init(sdw); + ret = intel_link_power_up(sdw); if (ret) { dev_err(dev, "%s failed: %d\n", __func__, ret); return ret; -- cgit v1.2.3-58-ga151 From 0b59d4c9475893fa1fa435502c0e0cc6df879ac9 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:18 +0800 Subject: soundwire: intel: simplify read ops assignment We can assign the right callback directly in the ops structure. No functionality change. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-9-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 4eeb2b5c1594..66aab02b9323 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1261,7 +1261,7 @@ static int intel_prop_read(struct sdw_bus *bus) } static struct sdw_master_ops sdw_intel_ops = { - .read_prop = sdw_master_read_prop, + .read_prop = intel_prop_read, .override_adr = sdw_dmi_override_adr, .xfer_msg = cdns_xfer_msg, .xfer_msg_defer = cdns_xfer_msg_defer, @@ -1304,8 +1304,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev, sdw_cdns_probe(cdns); - /* Set property read ops */ - sdw_intel_ops.read_prop = intel_prop_read; + /* Set ops */ bus->ops = &sdw_intel_ops; /* set driver data, accessed by snd_soc_dai_get_drvdata() */ -- cgit v1.2.3-58-ga151 From 0f3c54c22ae19c1ad86afb138c45424645213a44 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:19 +0800 Subject: soundwire: intel: introduce intel_shim_check_wake() helper Add new helper before code partitioning in order to avoid direct read from specific register. No functionality change. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-10-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 66aab02b9323..2ca924622153 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -344,6 +344,17 @@ static void intel_shim_init(struct sdw_intel *sdw) usleep_range(10, 15); } +static int intel_shim_check_wake(struct sdw_intel *sdw) +{ + void __iomem *shim; + u16 wake_sts; + + shim = sdw->link_res->shim; + wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS); + + return wake_sts & BIT(sdw->instance); +} + static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable) { void __iomem *shim = sdw->link_res->shim; @@ -1491,8 +1502,6 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) struct device *dev = &auxdev->dev; struct sdw_intel *sdw; struct sdw_bus *bus; - void __iomem *shim; - u16 wake_sts; sdw = auxiliary_get_drvdata(auxdev); bus = &sdw->cdns.bus; @@ -1503,10 +1512,7 @@ int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) return 0; } - shim = sdw->link_res->shim; - wake_sts = intel_readw(shim, SDW_SHIM_WAKESTS); - - if (!(wake_sts & BIT(sdw->instance))) + if (!intel_shim_check_wake(sdw)) return 0; /* disable WAKEEN interrupt ASAP to prevent interrupt flood */ -- cgit v1.2.3-58-ga151 From 8d875da7319de6c6b89d2644cd1e36b8d5ac5eb5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:20 +0800 Subject: soundwire: intel: introduce helpers to start bus There are 3 different sequences to start the bus, let's move the functionality to helpers. There should be no functionality change, except in error cases where the flow is improved with more consistent disabling of interrupts and powering down. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-11-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 388 +++++++++++++++++++++++----------------------- 1 file changed, 195 insertions(+), 193 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index 2ca924622153..abe14436d874 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1223,6 +1223,174 @@ static int intel_register_dai(struct sdw_intel *sdw) dais, num_dai); } +static int intel_start_bus(struct sdw_intel *sdw) +{ + struct device *dev = sdw->cdns.dev; + struct sdw_cdns *cdns = &sdw->cdns; + struct sdw_bus *bus = &cdns->bus; + int ret; + + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret); + return ret; + } + + /* + * follow recommended programming flows to avoid timeouts when + * gsync is enabled + */ + if (bus->multi_link) + intel_shim_sync_arm(sdw); + + ret = sdw_cdns_init(cdns); + if (ret < 0) { + dev_err(dev, "%s: unable to initialize Cadence IP: %d\n", __func__, ret); + goto err_interrupt; + } + + ret = sdw_cdns_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "%s: unable to exit bus reset sequence: %d\n", __func__, ret); + goto err_interrupt; + } + + if (bus->multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(dev, "%s: sync go failed: %d\n", __func__, ret); + goto err_interrupt; + } + } + sdw_cdns_check_self_clearing_bits(cdns, __func__, + true, INTEL_MASTER_RESET_ITERATIONS); + + return 0; + +err_interrupt: + sdw_cdns_enable_interrupt(cdns, false); + return ret; +} + +static int intel_start_bus_after_reset(struct sdw_intel *sdw) +{ + struct device *dev = sdw->cdns.dev; + struct sdw_cdns *cdns = &sdw->cdns; + struct sdw_bus *bus = &cdns->bus; + bool clock_stop0; + int status; + int ret; + + /* + * An exception condition occurs for the CLK_STOP_BUS_RESET + * case if one or more masters remain active. In this condition, + * all the masters are powered on for they are in the same power + * domain. Master can preserve its context for clock stop0, so + * there is no need to clear slave status and reset bus. + */ + clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); + + if (!clock_stop0) { + + /* + * make sure all Slaves are tagged as UNATTACHED and + * provide reason for reinitialization + */ + + status = SDW_UNATTACH_REQUEST_MASTER_RESET; + sdw_clear_slave_status(bus, status); + + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "cannot enable interrupts during resume\n"); + return ret; + } + + /* + * follow recommended programming flows to avoid + * timeouts when gsync is enabled + */ + if (bus->multi_link) + intel_shim_sync_arm(sdw); + + /* + * Re-initialize the IP since it was powered-off + */ + sdw_cdns_init(&sdw->cdns); + + } else { + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "cannot enable interrupts during resume\n"); + return ret; + } + } + + ret = sdw_cdns_clock_restart(cdns, !clock_stop0); + if (ret < 0) { + dev_err(dev, "unable to restart clock during resume\n"); + goto err_interrupt; + } + + if (!clock_stop0) { + ret = sdw_cdns_exit_reset(cdns); + if (ret < 0) { + dev_err(dev, "unable to exit bus reset sequence during resume\n"); + goto err_interrupt; + } + + if (bus->multi_link) { + ret = intel_shim_sync_go(sdw); + if (ret < 0) { + dev_err(sdw->cdns.dev, "sync go failed during resume\n"); + goto err_interrupt; + } + } + } + sdw_cdns_check_self_clearing_bits(cdns, __func__, true, INTEL_MASTER_RESET_ITERATIONS); + + return 0; + +err_interrupt: + sdw_cdns_enable_interrupt(cdns, false); + return ret; +} + +static void intel_check_clock_stop(struct sdw_intel *sdw) +{ + struct device *dev = sdw->cdns.dev; + bool clock_stop0; + + clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); + if (!clock_stop0) + dev_err(dev, "%s: invalid configuration, clock was not stopped\n", __func__); +} + +static int intel_start_bus_after_clock_stop(struct sdw_intel *sdw) +{ + struct device *dev = sdw->cdns.dev; + struct sdw_cdns *cdns = &sdw->cdns; + int ret; + + ret = sdw_cdns_enable_interrupt(cdns, true); + if (ret < 0) { + dev_err(dev, "%s: cannot enable interrupts: %d\n", __func__, ret); + return ret; + } + + ret = sdw_cdns_clock_restart(cdns, false); + if (ret < 0) { + dev_err(dev, "%s: unable to restart clock: %d\n", __func__, ret); + sdw_cdns_enable_interrupt(cdns, false); + return ret; + } + + sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime no_quirks", + true, INTEL_MASTER_RESET_ITERATIONS); + + return 0; +} + static int sdw_master_read_intel_prop(struct sdw_bus *bus) { struct sdw_master_prop *prop = &bus->prop; @@ -1368,7 +1536,6 @@ int intel_link_startup(struct auxiliary_device *auxdev) multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); if (!multi_link) { dev_dbg(dev, "Multi-link is disabled\n"); - bus->multi_link = false; } else { /* * hardware-based synchronization is required regardless @@ -1376,9 +1543,9 @@ int intel_link_startup(struct auxiliary_device *auxdev) * synchronization is gated by gsync when the multi-master * mode is set. */ - bus->multi_link = true; bus->hw_sync_min_links = 1; } + bus->multi_link = multi_link; /* Initialize shim, controller */ ret = intel_link_power_up(sdw); @@ -1389,45 +1556,17 @@ int intel_link_startup(struct auxiliary_device *auxdev) ret = intel_register_dai(sdw); if (ret) { dev_err(dev, "DAI registration failed: %d\n", ret); - goto err_init; + goto err_power_up; } intel_debugfs_init(sdw); - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts\n"); - goto err_init; - } - - /* - * follow recommended programming flows to avoid timeouts when - * gsync is enabled - */ - if (multi_link) - intel_shim_sync_arm(sdw); - - ret = sdw_cdns_init(cdns); - if (ret < 0) { - dev_err(dev, "unable to initialize Cadence IP\n"); - goto err_interrupt; - } - - ret = sdw_cdns_exit_reset(cdns); - if (ret < 0) { - dev_err(dev, "unable to exit bus reset sequence\n"); - goto err_interrupt; - } - - if (multi_link) { - ret = intel_shim_sync_go(sdw); - if (ret < 0) { - dev_err(dev, "sync go failed: %d\n", ret); - goto err_interrupt; - } + /* start bus */ + ret = intel_start_bus(sdw); + if (ret) { + dev_err(dev, "bus start failed: %d\n", ret); + goto err_power_up; } - sdw_cdns_check_self_clearing_bits(cdns, __func__, - true, INTEL_MASTER_RESET_ITERATIONS); /* Enable runtime PM */ if (!(link_flags & SDW_INTEL_MASTER_DISABLE_PM_RUNTIME)) { @@ -1473,8 +1612,8 @@ int intel_link_startup(struct auxiliary_device *auxdev) sdw->startup_done = true; return 0; -err_interrupt: - sdw_cdns_enable_interrupt(cdns, false); +err_power_up: + intel_link_power_down(sdw); err_init: return ret; } @@ -1738,7 +1877,6 @@ static int __maybe_unused intel_resume(struct device *dev) struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_bus *bus = &cdns->bus; int link_flags; - bool multi_link; int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { @@ -1748,7 +1886,6 @@ static int __maybe_unused intel_resume(struct device *dev) } link_flags = md_flags >> (bus->link_id * 8); - multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); if (pm_runtime_suspended(dev)) { dev_dbg(dev, "pm_runtime status was suspended, forcing active\n"); @@ -1777,41 +1914,13 @@ static int __maybe_unused intel_resume(struct device *dev) */ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET); - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } - - /* - * follow recommended programming flows to avoid timeouts when - * gsync is enabled - */ - if (multi_link) - intel_shim_sync_arm(sdw); - - ret = sdw_cdns_init(&sdw->cdns); - if (ret < 0) { - dev_err(dev, "unable to initialize Cadence IP during resume\n"); - return ret; - } - - ret = sdw_cdns_exit_reset(cdns); + ret = intel_start_bus(sdw); if (ret < 0) { - dev_err(dev, "unable to exit bus reset sequence during resume\n"); + dev_err(dev, "cannot start bus during resume\n"); + intel_link_power_down(sdw); return ret; } - if (multi_link) { - ret = intel_shim_sync_go(sdw); - if (ret < 0) { - dev_err(dev, "sync go failed during resume\n"); - return ret; - } - } - sdw_cdns_check_self_clearing_bits(cdns, __func__, - true, INTEL_MASTER_RESET_ITERATIONS); - /* * after system resume, the pm_runtime suspend() may kick in * during the enumeration, before any children device force the @@ -1824,7 +1933,7 @@ static int __maybe_unused intel_resume(struct device *dev) */ pm_runtime_mark_last_busy(dev); - return ret; + return 0; } static int __maybe_unused intel_resume_runtime(struct device *dev) @@ -1833,10 +1942,6 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) struct sdw_intel *sdw = cdns_to_intel(cdns); struct sdw_bus *bus = &cdns->bus; u32 clock_stop_quirks; - bool clock_stop0; - int link_flags; - bool multi_link; - int status; int ret; if (bus->prop.hw_disabled || !sdw->startup_done) { @@ -1848,15 +1953,12 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) /* unconditionally disable WAKEEN interrupt */ intel_shim_wake(sdw, false); - link_flags = md_flags >> (bus->link_id * 8); - multi_link = !(link_flags & SDW_INTEL_MASTER_DISABLE_MULTI_LINK); - clock_stop_quirks = sdw->link_res->clock_stop_quirks; if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) { ret = intel_link_power_up(sdw); if (ret) { - dev_err(dev, "%s failed: %d\n", __func__, ret); + dev_err(dev, "%s: power_up failed after teardown: %d\n", __func__, ret); return ret; } @@ -1866,145 +1968,45 @@ static int __maybe_unused intel_resume_runtime(struct device *dev) */ sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET); - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } - - /* - * follow recommended programming flows to avoid - * timeouts when gsync is enabled - */ - if (multi_link) - intel_shim_sync_arm(sdw); - - ret = sdw_cdns_init(&sdw->cdns); - if (ret < 0) { - dev_err(dev, "unable to initialize Cadence IP during resume\n"); - return ret; - } - - ret = sdw_cdns_exit_reset(cdns); + ret = intel_start_bus(sdw); if (ret < 0) { - dev_err(dev, "unable to exit bus reset sequence during resume\n"); + dev_err(dev, "%s: cannot start bus after teardown: %d\n", __func__, ret); + intel_link_power_down(sdw); return ret; } - if (multi_link) { - ret = intel_shim_sync_go(sdw); - if (ret < 0) { - dev_err(dev, "sync go failed during resume\n"); - return ret; - } - } - sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime TEARDOWN", - true, INTEL_MASTER_RESET_ITERATIONS); } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) { ret = intel_link_power_up(sdw); if (ret) { - dev_err(dev, "%s failed: %d\n", __func__, ret); + dev_err(dev, "%s: power_up failed after bus reset: %d\n", __func__, ret); return ret; } - /* - * An exception condition occurs for the CLK_STOP_BUS_RESET - * case if one or more masters remain active. In this condition, - * all the masters are powered on for they are in the same power - * domain. Master can preserve its context for clock stop0, so - * there is no need to clear slave status and reset bus. - */ - clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); - - if (!clock_stop0) { - - /* - * make sure all Slaves are tagged as UNATTACHED and - * provide reason for reinitialization - */ - - status = SDW_UNATTACH_REQUEST_MASTER_RESET; - sdw_clear_slave_status(bus, status); - - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } - - /* - * follow recommended programming flows to avoid - * timeouts when gsync is enabled - */ - if (multi_link) - intel_shim_sync_arm(sdw); - - /* - * Re-initialize the IP since it was powered-off - */ - sdw_cdns_init(&sdw->cdns); - - } else { - ret = sdw_cdns_enable_interrupt(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); - return ret; - } - } - - ret = sdw_cdns_clock_restart(cdns, !clock_stop0); + ret = intel_start_bus_after_reset(sdw); if (ret < 0) { - dev_err(dev, "unable to restart clock during resume\n"); + dev_err(dev, "%s: cannot start bus after reset: %d\n", __func__, ret); + intel_link_power_down(sdw); return ret; } - - if (!clock_stop0) { - ret = sdw_cdns_exit_reset(cdns); - if (ret < 0) { - dev_err(dev, "unable to exit bus reset sequence during resume\n"); - return ret; - } - - if (multi_link) { - ret = intel_shim_sync_go(sdw); - if (ret < 0) { - dev_err(sdw->cdns.dev, "sync go failed during resume\n"); - return ret; - } - } - } - sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime BUS_RESET", - true, INTEL_MASTER_RESET_ITERATIONS); - } else if (!clock_stop_quirks) { - clock_stop0 = sdw_cdns_is_clock_stop(&sdw->cdns); - if (!clock_stop0) - dev_err(dev, "%s invalid configuration, clock was not stopped", __func__); + intel_check_clock_stop(sdw); ret = intel_link_power_up(sdw); if (ret) { - dev_err(dev, "%s failed: %d\n", __func__, ret); + dev_err(dev, "%s: power_up failed: %d\n", __func__, ret); return ret; } - ret = sdw_cdns_enable_interrupt(cdns, true); + ret = intel_start_bus_after_clock_stop(sdw); if (ret < 0) { - dev_err(dev, "cannot enable interrupts during resume\n"); + dev_err(dev, "%s: cannot start bus after clock stop: %d\n", __func__, ret); + intel_link_power_down(sdw); return ret; } - - ret = sdw_cdns_clock_restart(cdns, false); - if (ret < 0) { - dev_err(dev, "unable to resume master during resume\n"); - return ret; - } - - sdw_cdns_check_self_clearing_bits(cdns, "intel_resume_runtime no_quirks", - true, INTEL_MASTER_RESET_ITERATIONS); } else { - dev_err(dev, "%s clock_stop_quirks %x unsupported\n", + dev_err(dev, "%s: clock_stop_quirks %x unsupported\n", __func__, clock_stop_quirks); ret = -EINVAL; } -- cgit v1.2.3-58-ga151 From 503ae285944a5e99ad3e0c36852ffe2680288418 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 20 Sep 2022 01:57:21 +0800 Subject: soundwire: intel: add helper to stop bus We have three nearly identical sequences to stop the clock, let's introduce a helper to reuse the same code. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Rander Wang Signed-off-by: Bard Liao Link: https://lore.kernel.org/r/20220919175721.354679-12-yung-chuan.liao@linux.intel.com Signed-off-by: Vinod Koul --- drivers/soundwire/intel.c | 83 +++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index abe14436d874..15fe083e0402 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -1391,6 +1391,38 @@ static int intel_start_bus_after_clock_stop(struct sdw_intel *sdw) return 0; } +static int intel_stop_bus(struct sdw_intel *sdw, bool clock_stop) +{ + struct device *dev = sdw->cdns.dev; + struct sdw_cdns *cdns = &sdw->cdns; + bool wake_enable = false; + int ret; + + if (clock_stop) { + ret = sdw_cdns_clock_stop(cdns, true); + if (ret < 0) + dev_err(dev, "%s: cannot stop clock: %d\n", __func__, ret); + else + wake_enable = true; + } + + ret = sdw_cdns_enable_interrupt(cdns, false); + if (ret < 0) { + dev_err(dev, "%s: cannot disable interrupts: %d\n", __func__, ret); + return ret; + } + + ret = intel_link_power_down(sdw); + if (ret) { + dev_err(dev, "%s: Link power down failed: %d\n", __func__, ret); + return ret; + } + + intel_shim_wake(sdw, wake_enable); + + return 0; +} + static int sdw_master_read_intel_prop(struct sdw_bus *bus) { struct sdw_master_prop *prop = &bus->prop; @@ -1790,20 +1822,12 @@ static int __maybe_unused intel_suspend(struct device *dev) return 0; } - ret = sdw_cdns_enable_interrupt(cdns, false); + ret = intel_stop_bus(sdw, false); if (ret < 0) { - dev_err(dev, "cannot disable interrupts on suspend\n"); + dev_err(dev, "%s: cannot stop bus: %d\n", __func__, ret); return ret; } - ret = intel_link_power_down(sdw); - if (ret) { - dev_err(dev, "Link power down failed: %d\n", ret); - return ret; - } - - intel_shim_wake(sdw, false); - return 0; } @@ -1824,44 +1848,19 @@ static int __maybe_unused intel_suspend_runtime(struct device *dev) clock_stop_quirks = sdw->link_res->clock_stop_quirks; if (clock_stop_quirks & SDW_INTEL_CLK_STOP_TEARDOWN) { - - ret = sdw_cdns_enable_interrupt(cdns, false); + ret = intel_stop_bus(sdw, 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\n", ret); + dev_err(dev, "%s: cannot stop bus during teardown: %d\n", + __func__, ret); return ret; } - - intel_shim_wake(sdw, false); - - } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET || - !clock_stop_quirks) { - bool wake_enable = true; - - ret = sdw_cdns_clock_stop(cdns, true); - if (ret < 0) { - dev_err(dev, "cannot enable clock stop on suspend\n"); - wake_enable = false; - } - - ret = sdw_cdns_enable_interrupt(cdns, false); + } else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET || !clock_stop_quirks) { + ret = intel_stop_bus(sdw, true); 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\n", ret); + dev_err(dev, "%s: cannot stop bus during clock_stop: %d\n", + __func__, ret); return ret; } - - intel_shim_wake(sdw, wake_enable); } else { dev_err(dev, "%s clock_stop_quirks %x unsupported\n", __func__, clock_stop_quirks); -- cgit v1.2.3-58-ga151