[PATCH v2 0/2] ASoC/soundwire: revisit support for clock registers
SoundWire clock base and scale registers are only supported for SDCA devices. That's fine, but that leaves SoundWire 1.2 with optional registers not supported. This is a corner case that needs to be supported.
The change is mainly on soundwire. But rt1318-sdw.c was applied to ASoC tree recently and not in SoundWire tree yet. @Vinod, Are you good if we go throutgh ASoC tree? Or @Mark can provide a tag and we can go through SoundWire tree?
changes: v2: - remove is_sdca flag from rt1318-sdw codec driver, too.
Pierre-Louis Bossart (2): ASoC/soundwire: remove is_sdca boolean property soundwire: enable optional clock registers for SoundWire 1.2 devices
drivers/soundwire/bus.c | 11 ++++++----- include/linux/soundwire/sdw.h | 6 ++++-- sound/soc/codecs/rt1316-sdw.c | 1 - sound/soc/codecs/rt1318-sdw.c | 1 - sound/soc/codecs/rt711-sdca-sdw.c | 1 - 5 files changed, 10 insertions(+), 10 deletions(-)
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/bus.c | 4 ++-- include/linux/soundwire/sdw.h | 2 -- sound/soc/codecs/rt1316-sdw.c | 1 - sound/soc/codecs/rt1318-sdw.c | 1 - sound/soc/codecs/rt711-sdca-sdw.c | 1 - 5 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 76515c33e639..c23275b443ac 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1587,7 +1587,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) goto io_err; }
- if (slave->prop.is_sdca) { + if (slave->id.class_id) { ret = sdw_read_no_pm(slave, SDW_DP0_INT); if (ret < 0) { dev_err(&slave->dev, @@ -1724,7 +1724,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) goto io_err; }
- if (slave->prop.is_sdca) { + if (slave->id.class_id) { ret = sdw_read_no_pm(slave, SDW_DP0_INT); if (ret < 0) { dev_err(&slave->dev, diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 9e4537f409c2..8fb458931772 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -365,7 +365,6 @@ struct sdw_dpn_prop { * @sink_dpn_prop: Sink Data Port N properties * @scp_int1_mask: SCP_INT1_MASK desired settings * @quirks: bitmask identifying deltas from the MIPI specification - * @is_sdca: the Slave supports the SDCA specification */ struct sdw_slave_prop { u32 mipi_revision; @@ -389,7 +388,6 @@ struct sdw_slave_prop { struct sdw_dpn_prop *sink_dpn_prop; u8 scp_int1_mask; u32 quirks; - bool is_sdca; };
#define SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY BIT(0) diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c index 2db7ee6c6d33..fbc7e9c0254d 100644 --- a/sound/soc/codecs/rt1316-sdw.c +++ b/sound/soc/codecs/rt1316-sdw.c @@ -203,7 +203,6 @@ static int rt1316_read_prop(struct sdw_slave *slave)
prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; - prop->is_sdca = true;
prop->paging_support = true;
diff --git a/sound/soc/codecs/rt1318-sdw.c b/sound/soc/codecs/rt1318-sdw.c index f85f5ab2c6d0..8bc379215c34 100644 --- a/sound/soc/codecs/rt1318-sdw.c +++ b/sound/soc/codecs/rt1318-sdw.c @@ -353,7 +353,6 @@ static int rt1318_read_prop(struct sdw_slave *slave)
prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; - prop->is_sdca = true;
prop->paging_support = true;
diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c index 88a8392a58ed..693e11ed8d08 100644 --- a/sound/soc/codecs/rt711-sdca-sdw.c +++ b/sound/soc/codecs/rt711-sdca-sdw.c @@ -186,7 +186,6 @@ static int rt711_sdca_read_prop(struct sdw_slave *slave)
prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY; - prop->is_sdca = true;
prop->paging_support = true;
On Fri, Nov 18, 2022 at 10:58:06AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Acked-by: Mark Brown broonie@kernel.org
On 18-11-22, 15:39, Mark Brown wrote:
On Fri, Nov 18, 2022 at 10:58:06AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Acked-by: Mark Brown broonie@kernel.org
Hey Mark,
sound/soc/codecs/rt1318-sdw.c does not exist for me in sdw/next. Can I get a tag for the changes merged into ASoC for this
Thanks
On Wed, Nov 23, 2022 at 08:22:14PM +0530, Vinod Koul wrote:
On 18-11-22, 15:39, Mark Brown wrote:
On Fri, Nov 18, 2022 at 10:58:06AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Acked-by: Mark Brown broonie@kernel.org
sound/soc/codecs/rt1318-sdw.c does not exist for me in sdw/next. Can I get a tag for the changes merged into ASoC for this
Not reasonably, that's basically the entire tree since Linus doesn't like branches.
On 23-11-22, 15:25, Mark Brown wrote:
On Wed, Nov 23, 2022 at 08:22:14PM +0530, Vinod Koul wrote:
On 18-11-22, 15:39, Mark Brown wrote:
On Fri, Nov 18, 2022 at 10:58:06AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Acked-by: Mark Brown broonie@kernel.org
sound/soc/codecs/rt1318-sdw.c does not exist for me in sdw/next. Can I get a tag for the changes merged into ASoC for this
Not reasonably, that's basically the entire tree since Linus doesn't like branches.
Okay.
Anyway we are close to merge window, lets postpone this after that
Thanks
On Fri, Nov 18, 2022 at 10:58:06AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The Device_ID registers already tell us if a device supports the SDCA specification or not, in hindsight we never needed a property when the information is reported by both hardware and ACPI.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
Reviewed-by: Charles Keepax ckeepax@opensource.cirrus.com
Thanks, Charles
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The bus supports the mandatory clock registers for SDCA devices, these registers can also be optionally supported by SoundWire 1.2 devices that don't follow the SDCA class specification.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/bus.c | 7 ++++--- include/linux/soundwire/sdw.h | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index c23275b443ac..55d393247a0f 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1233,10 +1233,11 @@ static int sdw_slave_set_frequency(struct sdw_slave *slave)
/* * frequency base and scale registers are required for SDCA - * devices. They may also be used for 1.2+/non-SDCA devices, - * but we will need a DisCo property to cover this case + * devices. They may also be used for 1.2+/non-SDCA devices. + * Driver can set the property, we will need a DisCo property + * to discover this case from platform firmware. */ - if (!slave->id.class_id) + if (!slave->id.class_id && !slave->prop.clock_reg_supported) return 0;
if (!mclk_freq) { diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index 8fb458931772..9a49263c53cf 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -365,6 +365,9 @@ struct sdw_dpn_prop { * @sink_dpn_prop: Sink Data Port N properties * @scp_int1_mask: SCP_INT1_MASK desired settings * @quirks: bitmask identifying deltas from the MIPI specification + * @clock_reg_supported: the Peripheral implements the clock base and scale + * registers introduced with the SoundWire 1.2 specification. SDCA devices + * do not need to set this boolean property as the registers are required. */ struct sdw_slave_prop { u32 mipi_revision; @@ -388,6 +391,7 @@ struct sdw_slave_prop { struct sdw_dpn_prop *sink_dpn_prop; u8 scp_int1_mask; u32 quirks; + bool clock_reg_supported; };
#define SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY BIT(0)
On Fri, Nov 18, 2022 at 10:58:07AM +0800, Bard Liao wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
The bus supports the mandatory clock registers for SDCA devices, these registers can also be optionally supported by SoundWire 1.2 devices that don't follow the SDCA class specification.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com
Reviewed-by: Charles Keepax ckeepax@opensource.cirrus.com
Thanks, Charles
On 18-11-22, 10:58, Bard Liao wrote:
SoundWire clock base and scale registers are only supported for SDCA devices. That's fine, but that leaves SoundWire 1.2 with optional registers not supported. This is a corner case that needs to be supported.
The change is mainly on soundwire. But rt1318-sdw.c was applied to ASoC tree recently and not in SoundWire tree yet. @Vinod, Are you good if we go throutgh ASoC tree? Or @Mark can provide a tag and we can go through SoundWire tree?
Applied, thanks
participants (4)
-
Bard Liao -
Charles Keepax -
Mark Brown -
Vinod Koul