diff mbox series

[13/14] soundwire: mipi-disco: add new properties from 2.0 spec

Message ID 20240827130707.298477-14-yung-chuan.liao@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series soundwire: mipi-disco: add partial SoundWire Disco 2.1 support | expand

Commit Message

Bard Liao Aug. 27, 2024, 1:07 p.m. UTC 
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The DisCo for SoundWire 2.0 spec adds support for new
'mipi-sdw-sdca-interrupt-register-list' and
'mipi-sdw-commit-register-supported'.

This patch only adds the definitions and property reads, but the use
of these properties will come at some point in the future when needed.

Note a slight conceptual disconnect between the MIPI DisCo definition
of a boolean property and the Linux implementation. The latter only
checks the presence of the property to set its value to 'true',
whereas the MIPI definitions allow for a property with a 'false'
value. This patch relies on a read_u8() even if the DisCo property is
defined as boolean.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/mipi_disco.c | 13 +++++++++++++
 include/linux/soundwire/sdw.h  |  7 ++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

Comments

Charles Keepax Sept. 9, 2024, 3:06 p.m. UTC | #1 
On Tue, Aug 27, 2024 at 09:07:06PM +0800, Bard Liao wrote:
> diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
> index d6eb63bf1252..2215c53f95de 100644
> --- a/drivers/soundwire/mipi_disco.c
> +++ b/drivers/soundwire/mipi_disco.c
> @@ -398,6 +398,19 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
>  	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
>  				 &prop->sink_ports);
>  
> +	device_property_read_u32(dev, "mipi-sdw-sdca-interrupt-register-list",
> +				 &prop->sdca_interrupt_register_list);
> +
> +	/*
> +	 * The specification defines the property value as boolean, but
> +	 * the value can be defined as zero. This is not aligned the
> +	 * implementation of device_property_read_bool() which only checks
> +	 * the presence of the property.
> +	 * Let's use read_u8 to work-around this conceptual disconnect.
> +	 */
> +	device_property_read_u8(dev, "mipi-sdw-commit-register-supported",
> +				&prop->commit_register_supported);

Would this not be a case for the new helper added earlier in the
series? Or is this some third type of boolean?

Thanks,
Charles
Liao, Bard Sept. 10, 2024, 9:17 a.m. UTC | #2 
> -----Original Message-----
> From: Charles Keepax <ckeepax@opensource.cirrus.com>
> Sent: Monday, September 9, 2024 11:06 PM
> To: Bard Liao <yung-chuan.liao@linux.intel.com>
> Cc: linux-sound@vger.kernel.org; vkoul@kernel.org; vinod.koul@linaro.org;
> linux-kernel@vger.kernel.org; pierre-louis.bossart@linux.intel.com; Liao, Bard
> <bard.liao@intel.com>
> Subject: Re: [PATCH 13/14] soundwire: mipi-disco: add new properties from
> 2.0 spec
> 
> On Tue, Aug 27, 2024 at 09:07:06PM +0800, Bard Liao wrote:
> > diff --git a/drivers/soundwire/mipi_disco.c
> b/drivers/soundwire/mipi_disco.c
> > index d6eb63bf1252..2215c53f95de 100644
> > --- a/drivers/soundwire/mipi_disco.c
> > +++ b/drivers/soundwire/mipi_disco.c
> > @@ -398,6 +398,19 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
> >  	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
> >  				 &prop->sink_ports);
> >
> > +	device_property_read_u32(dev, "mipi-sdw-sdca-interrupt-register-
> list",
> > +				 &prop->sdca_interrupt_register_list);
> > +
> > +	/*
> > +	 * The specification defines the property value as boolean, but
> > +	 * the value can be defined as zero. This is not aligned the
> > +	 * implementation of device_property_read_bool() which only checks
> > +	 * the presence of the property.
> > +	 * Let's use read_u8 to work-around this conceptual disconnect.
> > +	 */
> > +	device_property_read_u8(dev, "mipi-sdw-commit-register-
> supported",
> > +				&prop->commit_register_supported);
> 
> Would this not be a case for the new helper added earlier in the
> series? Or is this some third type of boolean?

Thanks Charles. Yes, we should use mipi_device_property_read_bool().
Will send a v2 to fix.

> 
> Thanks,
> Charles
diff mbox series

Patch

diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index d6eb63bf1252..2215c53f95de 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -398,6 +398,19 @@  int sdw_slave_read_prop(struct sdw_slave *slave)
 	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
 				 &prop->sink_ports);
 
+	device_property_read_u32(dev, "mipi-sdw-sdca-interrupt-register-list",
+				 &prop->sdca_interrupt_register_list);
+
+	/*
+	 * The specification defines the property value as boolean, but
+	 * the value can be defined as zero. This is not aligned the
+	 * implementation of device_property_read_bool() which only checks
+	 * the presence of the property.
+	 * Let's use read_u8 to work-around this conceptual disconnect.
+	 */
+	device_property_read_u8(dev, "mipi-sdw-commit-register-supported",
+				&prop->commit_register_supported);
+
 	/*
 	 * Read dp0 properties - we don't rely on the 'mipi-sdw-dp-0-supported'
 	 * property since the 'mipi-sdw-dp0-subproperties' property is logically
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 66feaa79ecfc..952514f044f0 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -335,8 +335,11 @@  struct sdw_dpn_prop {
  * @master_count: Number of Masters present on this Slave
  * @source_ports: Bitmap identifying source ports
  * @sink_ports: Bitmap identifying sink ports
- * @scp_int1_mask: SCP_INT1_MASK desired settings
  * @quirks: bitmask identifying deltas from the MIPI specification
+ * @sdca_interrupt_register_list: indicates which sets of SDCA interrupt status
+ * and masks are supported
+ * @commit_register_supported: is PCP_Commit register supported
+ * @scp_int1_mask: SCP_INT1_MASK desired settings
  * @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.
@@ -363,6 +366,8 @@  struct sdw_slave_prop {
 	u32 source_ports;
 	u32 sink_ports;
 	u32 quirks;
+	u32 sdca_interrupt_register_list;
+	u8 commit_register_supported;
 	u8 scp_int1_mask;
 	bool clock_reg_supported;
 	bool use_domain_irq;