2022-11-11 02:16:23

by Bard Liao

[permalink] [raw]
Subject: [PATCH 0/2] 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.

Pierre-Louis Bossart (2):
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/rt711-sdca-sdw.c | 1 -
4 files changed, 10 insertions(+), 9 deletions(-)

--
2.25.1



2022-11-11 02:34:48

by Bard Liao

[permalink] [raw]
Subject: [PATCH 1/2] soundwire: remove is_sdca boolean property

From: Pierre-Louis Bossart <[email protected]>

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 <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/bus.c | 4 ++--
include/linux/soundwire/sdw.h | 2 --
sound/soc/codecs/rt1316-sdw.c | 1 -
sound/soc/codecs/rt711-sdca-sdw.c | 1 -
4 files changed, 2 insertions(+), 6 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 ed0a11436362..8b27401237f7 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/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c
index 4120842fe699..6ca8795eed68 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;

--
2.25.1


2022-11-11 03:01:08

by Bard Liao

[permalink] [raw]
Subject: [PATCH 2/2] soundwire: enable optional clock registers for SoundWire 1.2 devices

From: Pierre-Louis Bossart <[email protected]>

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 <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
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)
--
2.25.1


2022-11-23 14:59:43

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] soundwire: remove is_sdca boolean property

On 11-11-22, 10:16, Bard Liao wrote:
> From: Pierre-Louis Bossart <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Rander Wang <[email protected]>
> Reviewed-by: P?ter Ujfalusi <[email protected]>
> Signed-off-by: Bard Liao <[email protected]>
> ---
> drivers/soundwire/bus.c | 4 ++--
> include/linux/soundwire/sdw.h | 2 --
> sound/soc/codecs/rt1316-sdw.c | 1 -
> sound/soc/codecs/rt711-sdca-sdw.c | 1 -

The change lgtm, but needs ACK from Mark. Please split the ASoC bit to
separate patch and copy Mark (that can be first patch here)

> 4 files changed, 2 insertions(+), 6 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 ed0a11436362..8b27401237f7 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/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c
> index 4120842fe699..6ca8795eed68 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;
>
> --
> 2.25.1

--
~Vinod