[RFC PATCH] soundwire: bus: Add flag to mark DPN_BlockCtrl1 as readonly
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Mon Mar 9 19:05:22 CET 2020
On 3/9/20 12:37 PM, 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."
Interesting.
I think it's a spec issue that you want to bring to the MIPI Audio WG
attention.
The note below says 'the DPx_BlockCtrl1 Register remains as Read-Write,
but the value written to the Read-Only field is not used'
Ignoring a value and returning an error are two different behaviors indeed.
My recommendation would be to add a DisCo property stating the
WordLength value can be used by the bus code but not written to the
Slave device registers.
>
> 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 ro_blockctrl1_reg
> flag in struct sdw_port_runtime.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla at linaro.org>
> ---
>
> I will send patch for WSA881x to include this change once this patch
> is accepted.
>
> drivers/soundwire/bus.h | 2 ++
> drivers/soundwire/stream.c | 17 ++++++++++-------
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
> index 204204a26db8..791e8d14093e 100644
> --- a/drivers/soundwire/bus.h
> +++ b/drivers/soundwire/bus.h
> @@ -79,6 +79,7 @@ int sdw_find_col_index(int col);
> * @num: Port number. For audio streams, valid port number ranges from
> * [1,14]
> * @ch_mask: Channel mask
> + * @ro_blockctrl1_reg: Read Only flag for DPN_BlockCtrl1 register
> * @transport_params: Transport parameters
> * @port_params: Port parameters
> * @port_node: List node for Master or Slave port_list
> @@ -89,6 +90,7 @@ int sdw_find_col_index(int col);
> struct sdw_port_runtime {
> int num;
> int ch_mask;
> + bool ro_blockctrl1_reg;
> struct sdw_transport_params transport_params;
> struct sdw_port_params port_params;
> struct list_head port_node;
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 00348d1fc606..4491643aeb4a 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 (!p_rt->ro_blockctrl1_reg) {
> + /* 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 */
> @@ -1195,6 +1197,7 @@ static struct sdw_port_runtime
>
> p_rt->ch_mask = port_config[port_index].ch_mask;
> p_rt->num = port_config[port_index].num;
> + p_rt->ro_blockctrl1_reg = port_config[port_index].ro_blockctrl1_reg;
>
> return p_rt;
> }
>
More information about the Alsa-devel
mailing list