LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] soundwire: add read_only_wordlength flag
@ 2020-03-11 11:35 Srinivas Kandagatla
  2020-03-11 11:35 ` [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties Srinivas Kandagatla
  2020-03-11 11:35 ` [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag Srinivas Kandagatla
  0 siblings, 2 replies; 7+ messages in thread
From: Srinivas Kandagatla @ 2020-03-11 11:35 UTC (permalink / raw)
  To: broonie, vkoul
  Cc: pierre-louis.bossart, linux-kernel, alsa-devel, Srinivas Kandagatla

According to SoundWire Specification Version 1.2.
"A Data Port number X (in the range 0-14) which supports only one
value of WordLength may implement the WordLength field in the
DPX_BlockCtrl1 Register as Read-Only, returning the fixed value of
WordLength in response to reads."

As WSA881x interfaces in PDM mode making the only field "WordLength"
in DPX_BlockCtrl1" fixed and read-only. Behaviour of writing to this
register on WSA881x soundwire slave with Qualcomm Soundwire Controller
is throwing up an error. Not sure how other controllers deal with
writing to readonly registers, but this patch provides a way to avoid
writes to DPN_BlockCtrl1 register by providing a read_only_wordlength
flag in struct sdw_dpn_prop


Srinivas Kandagatla (2):
  soundwire: stream: Add read_only_wordlength flag to port properties
  ASoC: wsa881x: mark read_only_wordlength flag

 drivers/soundwire/stream.c    | 16 +++++++++-------
 include/linux/soundwire/sdw.h |  2 ++
 sound/soc/codecs/wsa881x.c    |  4 ++++
 3 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties
  2020-03-11 11:35 [PATCH 0/2] soundwire: add read_only_wordlength flag Srinivas Kandagatla
@ 2020-03-11 11:35 ` Srinivas Kandagatla
  2020-03-20 13:56   ` Vinod Koul
  2020-03-11 11:35 ` [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag Srinivas Kandagatla
  1 sibling, 1 reply; 7+ messages in thread
From: Srinivas Kandagatla @ 2020-03-11 11:35 UTC (permalink / raw)
  To: broonie, vkoul
  Cc: pierre-louis.bossart, linux-kernel, alsa-devel, Srinivas Kandagatla

According to SoundWire Specification Version 1.2.
"A Data Port number X (in the range 0-14) which supports only one
value of WordLength may implement the WordLength field in the
DPX_BlockCtrl1 Register as Read-Only, returning the fixed value of
WordLength in response to reads."

As WSA881x interfaces in PDM mode making the only field "WordLength"
in DPX_BlockCtrl1" fixed and read-only. Behaviour of writing to this
register on WSA881x soundwire slave with Qualcomm Soundwire Controller
is throwing up an error. Not sure how other controllers deal with
writing to readonly registers, but this patch provides a way to avoid
writes to DPN_BlockCtrl1 register by providing a read_only_wordlength
flag in struct sdw_dpn_prop

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/stream.c    | 16 +++++++++-------
 include/linux/soundwire/sdw.h |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 178ae92b8cc1..7fb89a94d9c0 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -167,13 +167,15 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 		return ret;
 	}
 
-	/* Program DPN_BlockCtrl1 register */
-	ret = sdw_write(s_rt->slave, addr2, (p_params->bps - 1));
-	if (ret < 0) {
-		dev_err(&s_rt->slave->dev,
-			"DPN_BlockCtrl1 register write failed for port %d\n",
-			t_params->port_num);
-		return ret;
+	if (!dpn_prop->read_only_wordlength) {
+		/* Program DPN_BlockCtrl1 register */
+		ret = sdw_write(s_rt->slave, addr2, (p_params->bps - 1));
+		if (ret < 0) {
+			dev_err(&s_rt->slave->dev,
+				"DPN_BlockCtrl1 register write failed for port %d\n",
+				t_params->port_num);
+			return ret;
+		}
 	}
 
 	/* Program DPN_SampleCtrl1 register */
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index b451bb622335..2dfe14ed3bb0 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -284,6 +284,7 @@ struct sdw_dpn_audio_mode {
  * @max_async_buffer: Number of samples that this port can buffer in
  * asynchronous modes
  * @block_pack_mode: Type of block port mode supported
+ * @read_only_wordlength: Read Only wordlength field in DPN_BlockCtrl1 register
  * @port_encoding: Payload Channel Sample encoding schemes supported
  * @audio_modes: Audio modes supported
  */
@@ -307,6 +308,7 @@ struct sdw_dpn_prop {
 	u32 modes;
 	u32 max_async_buffer;
 	bool block_pack_mode;
+	bool read_only_wordlength;
 	u32 port_encoding;
 	struct sdw_dpn_audio_mode *audio_modes;
 };
-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag
  2020-03-11 11:35 [PATCH 0/2] soundwire: add read_only_wordlength flag Srinivas Kandagatla
  2020-03-11 11:35 ` [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties Srinivas Kandagatla
@ 2020-03-11 11:35 ` Srinivas Kandagatla
  2020-03-11 18:58   ` Pierre-Louis Bossart
  2020-03-20 14:02   ` Vinod Koul
  1 sibling, 2 replies; 7+ messages in thread
From: Srinivas Kandagatla @ 2020-03-11 11:35 UTC (permalink / raw)
  To: broonie, vkoul
  Cc: pierre-louis.bossart, linux-kernel, alsa-devel, Srinivas Kandagatla

WSA881x works in PDM mode so the wordlength is fixed, which also makes
the only field "WordLength" in DPN_BlockCtrl1 register a read-only.
Writing to this register will throw up errors with Qualcomm Controller.
So use ro_blockctrl1_reg flag to mark this field as read-only so that
core will not write to this register.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wsa881x.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index b59f1d0e7f84..35b44b297f9e 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -394,6 +394,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
 		.min_ch = 1,
 		.max_ch = 1,
 		.simple_ch_prep_sm = true,
+		.read_only_wordlength = true,
 	}, {
 		/* COMP */
 		.num = 2,
@@ -401,6 +402,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
 		.min_ch = 1,
 		.max_ch = 1,
 		.simple_ch_prep_sm = true,
+		.read_only_wordlength = true,
 	}, {
 		/* BOOST */
 		.num = 3,
@@ -408,6 +410,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
 		.min_ch = 1,
 		.max_ch = 1,
 		.simple_ch_prep_sm = true,
+		.read_only_wordlength = true,
 	}, {
 		/* VISENSE */
 		.num = 4,
@@ -415,6 +418,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
 		.min_ch = 1,
 		.max_ch = 1,
 		.simple_ch_prep_sm = true,
+		.read_only_wordlength = true,
 	}
 };
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag
  2020-03-11 11:35 ` [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag Srinivas Kandagatla
@ 2020-03-11 18:58   ` Pierre-Louis Bossart
  2020-03-12  9:54     ` Srinivas Kandagatla
  2020-03-20 14:02   ` Vinod Koul
  1 sibling, 1 reply; 7+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-11 18:58 UTC (permalink / raw)
  To: Srinivas Kandagatla, broonie, vkoul; +Cc: linux-kernel, alsa-devel



On 3/11/20 6:35 AM, Srinivas Kandagatla wrote:
> WSA881x works in PDM mode so the wordlength is fixed, which also makes
> the only field "WordLength" in DPN_BlockCtrl1 register a read-only.
> Writing to this register will throw up errors with Qualcomm Controller.

it'd be good to clarify the nature of these errors, i.e.

a) is this the Slave device responding with a NAK?
b) is this the Slave device responding with COMMAND_IGNORED, and those 
responses not handled by the controller?
c) something else?

Thanks!

> So use ro_blockctrl1_reg flag to mark this field as read-only so that
> core will not write to this register.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   sound/soc/codecs/wsa881x.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
> index b59f1d0e7f84..35b44b297f9e 100644
> --- a/sound/soc/codecs/wsa881x.c
> +++ b/sound/soc/codecs/wsa881x.c
> @@ -394,6 +394,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>   		.min_ch = 1,
>   		.max_ch = 1,
>   		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>   	}, {
>   		/* COMP */
>   		.num = 2,
> @@ -401,6 +402,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>   		.min_ch = 1,
>   		.max_ch = 1,
>   		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>   	}, {
>   		/* BOOST */
>   		.num = 3,
> @@ -408,6 +410,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>   		.min_ch = 1,
>   		.max_ch = 1,
>   		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>   	}, {
>   		/* VISENSE */
>   		.num = 4,
> @@ -415,6 +418,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>   		.min_ch = 1,
>   		.max_ch = 1,
>   		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>   	}
>   };
>   
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag
  2020-03-11 18:58   ` Pierre-Louis Bossart
@ 2020-03-12  9:54     ` Srinivas Kandagatla
  0 siblings, 0 replies; 7+ messages in thread
From: Srinivas Kandagatla @ 2020-03-12  9:54 UTC (permalink / raw)
  To: Pierre-Louis Bossart, broonie, vkoul; +Cc: linux-kernel, alsa-devel



On 11/03/2020 18:58, Pierre-Louis Bossart wrote:
> 
> On 3/11/20 6:35 AM, Srinivas Kandagatla wrote:
>> WSA881x works in PDM mode so the wordlength is fixed, which also makes
>> the only field "WordLength" in DPN_BlockCtrl1 register a read-only.
>> Writing to this register will throw up errors with Qualcomm Controller.
> 
> it'd be good to clarify the nature of these errors, i.e.
> 
> a) is this the Slave device responding with a NAK?
> b) is this the Slave device responding with COMMAND_IGNORED, and those 
> responses not handled by the controller?
> c) something else?


With the current version of Qcom SoundWire controller driver, it can 
only know if the write succeeded or not based on completion interrupt. 
There are no details of the actual error code visible to the driver. But 
the new version of the same controller is expected to return better 
error codes.

thanks,
srini

> 
> Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties
  2020-03-11 11:35 ` [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties Srinivas Kandagatla
@ 2020-03-20 13:56   ` Vinod Koul
  0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-03-20 13:56 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: broonie, pierre-louis.bossart, linux-kernel, alsa-devel

On 11-03-20, 11:35, Srinivas Kandagatla wrote:
> According to SoundWire Specification Version 1.2.
> "A Data Port number X (in the range 0-14) which supports only one
> value of WordLength may implement the WordLength field in the
> DPX_BlockCtrl1 Register as Read-Only, returning the fixed value of
> WordLength in response to reads."
> 
> As WSA881x interfaces in PDM mode making the only field "WordLength"
> in DPX_BlockCtrl1" fixed and read-only. Behaviour of writing to this
> register on WSA881x soundwire slave with Qualcomm Soundwire Controller
> is throwing up an error. Not sure how other controllers deal with
> writing to readonly registers, but this patch provides a way to avoid
> writes to DPN_BlockCtrl1 register by providing a read_only_wordlength
> flag in struct sdw_dpn_prop

Applied, thanks

I will send a tag, so that mark can apply the second patch for asoc

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag
  2020-03-11 11:35 ` [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag Srinivas Kandagatla
  2020-03-11 18:58   ` Pierre-Louis Bossart
@ 2020-03-20 14:02   ` Vinod Koul
  1 sibling, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-03-20 14:02 UTC (permalink / raw)
  To: broonie, Srinivas Kandagatla
  Cc: pierre-louis.bossart, linux-kernel, alsa-devel

Hi Mark,

On 11-03-20, 11:35, Srinivas Kandagatla wrote:
> WSA881x works in PDM mode so the wordlength is fixed, which also makes
> the only field "WordLength" in DPN_BlockCtrl1 register a read-only.
> Writing to this register will throw up errors with Qualcomm Controller.
> So use ro_blockctrl1_reg flag to mark this field as read-only so that
> core will not write to this register.

I have applied the sdw patch. Since this depends on sdw patch, feel free
to pull the below tag before you apply this

The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:

  Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git topic/ro_wordlength

for you to fetch changes up to a9107de4b03604ce0d279315c91b31b8065ee4ea:

  soundwire: stream: Add read_only_wordlength flag to port properties (2020-03-20 19:24:59 +0530)

----------------------------------------------------------------
Srinivas Kandagatla (1):
      soundwire: stream: Add read_only_wordlength flag to port properties

 drivers/soundwire/stream.c    | 16 +++++++++-------
 include/linux/soundwire/sdw.h |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  sound/soc/codecs/wsa881x.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
> index b59f1d0e7f84..35b44b297f9e 100644
> --- a/sound/soc/codecs/wsa881x.c
> +++ b/sound/soc/codecs/wsa881x.c
> @@ -394,6 +394,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>  		.min_ch = 1,
>  		.max_ch = 1,
>  		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>  	}, {
>  		/* COMP */
>  		.num = 2,
> @@ -401,6 +402,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>  		.min_ch = 1,
>  		.max_ch = 1,
>  		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>  	}, {
>  		/* BOOST */
>  		.num = 3,
> @@ -408,6 +410,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>  		.min_ch = 1,
>  		.max_ch = 1,
>  		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>  	}, {
>  		/* VISENSE */
>  		.num = 4,
> @@ -415,6 +418,7 @@ static struct sdw_dpn_prop wsa_sink_dpn_prop[WSA881X_MAX_SWR_PORTS] = {
>  		.min_ch = 1,
>  		.max_ch = 1,
>  		.simple_ch_prep_sm = true,
> +		.read_only_wordlength = true,
>  	}
>  };
>  
> -- 
> 2.21.0

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-20 14:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 11:35 [PATCH 0/2] soundwire: add read_only_wordlength flag Srinivas Kandagatla
2020-03-11 11:35 ` [PATCH 1/2] soundwire: stream: Add read_only_wordlength flag to port properties Srinivas Kandagatla
2020-03-20 13:56   ` Vinod Koul
2020-03-11 11:35 ` [PATCH 2/2] ASoC: wsa881x: mark read_only_wordlength flag Srinivas Kandagatla
2020-03-11 18:58   ` Pierre-Louis Bossart
2020-03-12  9:54     ` Srinivas Kandagatla
2020-03-20 14:02   ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).