diff mbox series

[v2,1/8] soundwire: stream: Add specific prep/deprep commands to port_prep callback

Message ID 20230118160452.2385494-2-sbinding@opensource.cirrus.com
State New
Headers
Series ASoC: cs42l42: Add Soundwire support |

Commit Message

Stefan Binding Jan. 18, 2023, 4:04 p.m. UTC 
  Currently, port_prep callback only has commands for PRE_PREP, PREP,
and POST_PREP, which doesn't directly say whether this is for a
prepare or deprepare call. Extend the command list enum to say
whether the call is for prepare or deprepare aswell.

Also remove SDW_OPS_PORT_PREP from sdw_port_prep_ops as this is unused,
and update this enum to be simpler and more consistent with enum
sdw_clk_stop_type.

Note: Currently, the only users of SDW_OPS_PORT_POST_PREP are codec
drivers sound/soc/codecs/wsa881x.c and sound/soc/codecs/wsa883x.c, both
of which seem to assume that POST_PREP only occurs after a prepare,
even though it would also have occurred after a deprepare. Since it
doesn't make sense to mark the port prepared after a deprepare, changing
the enum to separate PORT_DEPREP from PORT_PREP should make the check
for PORT_PREP in those drivers be more logical.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/soundwire/stream.c    | 4 ++--
 include/linux/soundwire/sdw.h | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

Comments

Pierre-Louis Bossart Jan. 18, 2023, 4:37 p.m. UTC | #1 
On 1/18/23 10:04, Stefan Binding wrote:
> Currently, port_prep callback only has commands for PRE_PREP, PREP,
> and POST_PREP, which doesn't directly say whether this is for a
> prepare or deprepare call. Extend the command list enum to say
> whether the call is for prepare or deprepare aswell.
> 
> Also remove SDW_OPS_PORT_PREP from sdw_port_prep_ops as this is unused,
> and update this enum to be simpler and more consistent with enum
> sdw_clk_stop_type.

yes, I don't know why this PORT_PREP was added, clearly the prepare part
is something that would be done with standard registers without the need
to inform the codec driver. The codec driver only need the pre- and
post- notifications.

Good cleanup!


> Note: Currently, the only users of SDW_OPS_PORT_POST_PREP are codec
> drivers sound/soc/codecs/wsa881x.c and sound/soc/codecs/wsa883x.c, both
> of which seem to assume that POST_PREP only occurs after a prepare,
> even though it would also have occurred after a deprepare. Since it
> doesn't make sense to mark the port prepared after a deprepare, changing
> the enum to separate PORT_DEPREP from PORT_PREP should make the check
> for PORT_PREP in those drivers be more logical.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
>  drivers/soundwire/stream.c    | 4 ++--
>  include/linux/soundwire/sdw.h | 8 +++++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index df3b36670df4c..1652fb5737d9d 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -469,7 +469,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
>  	}
>  
>  	/* Inform slave about the impending port prepare */
> -	sdw_do_port_prep(s_rt, prep_ch, SDW_OPS_PORT_PRE_PREP);
> +	sdw_do_port_prep(s_rt, prep_ch, prep ? SDW_OPS_PORT_PRE_PREP : SDW_OPS_PORT_PRE_DEPREP);
>  
>  	/* Prepare Slave port implementing CP_SM */
>  	if (!dpn_prop->simple_ch_prep_sm) {
> @@ -501,7 +501,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
>  	}
>  
>  	/* Inform slaves about ports prepared */
> -	sdw_do_port_prep(s_rt, prep_ch, SDW_OPS_PORT_POST_PREP);
> +	sdw_do_port_prep(s_rt, prep_ch, prep ? SDW_OPS_PORT_POST_PREP : SDW_OPS_PORT_POST_DEPREP);
>  
>  	/* Disable interrupt after Port de-prepare */
>  	if (!prep && intr)
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index 3cd2a761911ff..547fc1b30a51a 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -569,13 +569,15 @@ struct sdw_prepare_ch {
>   * enum sdw_port_prep_ops: Prepare operations for Data Port
>   *
>   * @SDW_OPS_PORT_PRE_PREP: Pre prepare operation for the Port
> - * @SDW_OPS_PORT_PREP: Prepare operation for the Port
> + * @SDW_OPS_PORT_PRE_DEPREP: Pre deprepare operation for the Port
>   * @SDW_OPS_PORT_POST_PREP: Post prepare operation for the Port
> + * @SDW_OPS_PORT_POST_DEPREP: Post deprepare operation for the Port
>   */
>  enum sdw_port_prep_ops {
>  	SDW_OPS_PORT_PRE_PREP = 0,
> -	SDW_OPS_PORT_PREP = 1,
> -	SDW_OPS_PORT_POST_PREP = 2,
> +	SDW_OPS_PORT_PRE_DEPREP,
> +	SDW_OPS_PORT_POST_PREP,
> +	SDW_OPS_PORT_POST_DEPREP,
>  };
>  
>  /**
diff mbox series

Patch

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index df3b36670df4c..1652fb5737d9d 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -469,7 +469,7 @@  static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 	}
 
 	/* Inform slave about the impending port prepare */
-	sdw_do_port_prep(s_rt, prep_ch, SDW_OPS_PORT_PRE_PREP);
+	sdw_do_port_prep(s_rt, prep_ch, prep ? SDW_OPS_PORT_PRE_PREP : SDW_OPS_PORT_PRE_DEPREP);
 
 	/* Prepare Slave port implementing CP_SM */
 	if (!dpn_prop->simple_ch_prep_sm) {
@@ -501,7 +501,7 @@  static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 	}
 
 	/* Inform slaves about ports prepared */
-	sdw_do_port_prep(s_rt, prep_ch, SDW_OPS_PORT_POST_PREP);
+	sdw_do_port_prep(s_rt, prep_ch, prep ? SDW_OPS_PORT_POST_PREP : SDW_OPS_PORT_POST_DEPREP);
 
 	/* Disable interrupt after Port de-prepare */
 	if (!prep && intr)
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 3cd2a761911ff..547fc1b30a51a 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -569,13 +569,15 @@  struct sdw_prepare_ch {
  * enum sdw_port_prep_ops: Prepare operations for Data Port
  *
  * @SDW_OPS_PORT_PRE_PREP: Pre prepare operation for the Port
- * @SDW_OPS_PORT_PREP: Prepare operation for the Port
+ * @SDW_OPS_PORT_PRE_DEPREP: Pre deprepare operation for the Port
  * @SDW_OPS_PORT_POST_PREP: Post prepare operation for the Port
+ * @SDW_OPS_PORT_POST_DEPREP: Post deprepare operation for the Port
  */
 enum sdw_port_prep_ops {
 	SDW_OPS_PORT_PRE_PREP = 0,
-	SDW_OPS_PORT_PREP = 1,
-	SDW_OPS_PORT_POST_PREP = 2,
+	SDW_OPS_PORT_PRE_DEPREP,
+	SDW_OPS_PORT_POST_PREP,
+	SDW_OPS_PORT_POST_DEPREP,
 };
 
 /**