* [PATCH 1/3] topology: use inclusive language for bclk
2020-09-18 21:28 [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Pierre-Louis Bossart
@ 2020-09-18 21:28 ` Pierre-Louis Bossart
2020-09-18 21:28 ` [PATCH 2/3] topology: use inclusive language for fsync Pierre-Louis Bossart
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2020-09-18 21:28 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, broonie, Pierre-Louis Bossart
use bclk_provider for structure fields, 'codec_provider' and
'codec_consumer' for options and modify #defines to use CP and CC
suffixes.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
include/sound/uapi/asoc.h | 11 +++++++----
include/topology.h | 2 +-
src/topology/pcm.c | 36 ++++++++++++++++++++++++++----------
3 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/include/sound/uapi/asoc.h b/include/sound/uapi/asoc.h
index 4efb4ec4..ceafb1a9 100644
--- a/include/sound/uapi/asoc.h
+++ b/include/sound/uapi/asoc.h
@@ -169,10 +169,13 @@
#define SND_SOC_TPLG_LNK_FLGBIT_VOICE_WAKEUP (1 << 3)
/* DAI topology BCLK parameter
- * For the backwards capability, by default codec is bclk master
+ * For the backwards capability, by default codec is bclk provider
*/
-#define SND_SOC_TPLG_BCLK_CM 0 /* codec is bclk master */
-#define SND_SOC_TPLG_BCLK_CS 1 /* codec is bclk slave */
+#define SND_SOC_TPLG_BCLK_CP 0 /* codec is bclk provider */
+#define SND_SOC_TPLG_BCLK_CC 1 /* codec is bclk consumer */
+/* keep previous definitions for compatibility */
+#define SND_SOC_TPLG_BCLK_CM SND_SOC_TPLG_BCLK_CP
+#define SND_SOC_TPLG_BCLK_CS SND_SOC_TPLG_BCLK_CC
/* DAI topology FSYNC parameter
* For the backwards capability, by default codec is fsync master
@@ -335,7 +338,7 @@ struct snd_soc_tplg_hw_config {
__u8 clock_gated; /* SND_SOC_TPLG_DAI_CLK_GATE_ value */
__u8 invert_bclk; /* 1 for inverted BCLK, 0 for normal */
__u8 invert_fsync; /* 1 for inverted frame clock, 0 for normal */
- __u8 bclk_master; /* SND_SOC_TPLG_BCLK_ value */
+ __u8 bclk_provider; /* SND_SOC_TPLG_BCLK_ value */
__u8 fsync_master; /* SND_SOC_TPLG_FSYNC_ value */
__u8 mclk_direction; /* SND_SOC_TPLG_MCLK_ value */
__le16 reserved; /* for 32bit alignment */
diff --git a/include/topology.h b/include/topology.h
index 1f52e66e..6c970649 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -1028,7 +1028,7 @@ struct snd_tplg_hw_config_template {
unsigned char clock_gated; /* SND_SOC_TPLG_DAI_CLK_GATE_ value */
unsigned char invert_bclk; /* 1 for inverted BCLK, 0 for normal */
unsigned char invert_fsync; /* 1 for inverted frame clock, 0 for normal */
- unsigned char bclk_master; /* SND_SOC_TPLG_BCLK_ value */
+ unsigned char bclk_provider; /* SND_SOC_TPLG_BCLK_ value */
unsigned char fsync_master; /* SND_SOC_TPLG_FSYNC_ value */
unsigned char mclk_direction; /* SND_SOC_TPLG_MCLK_ value */
unsigned short reserved; /* for 32bit alignment */
diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index 191b7a0a..f05df348 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -1411,6 +1411,7 @@ int tplg_parse_hw_config(snd_tplg_t *tplg, snd_config_t *cfg,
snd_config_t *n;
const char *id, *val = NULL;
int ret, ival;
+ bool provider_legacy;
elem = tplg_elem_new_common(tplg, cfg, NULL, SND_TPLG_TYPE_HW_CONFIG);
if (!elem)
@@ -1451,8 +1452,15 @@ int tplg_parse_hw_config(snd_tplg_t *tplg, snd_config_t *cfg,
continue;
}
- if (strcmp(id, "bclk") == 0 ||
- strcmp(id, "bclk_master") == 0) {
+ provider_legacy = false;
+ if (strcmp(id, "bclk_master") == 0) {
+ SNDERR("deprecated option %s, please use 'bclk'\n", id);
+ provider_legacy = true;
+ }
+
+ if (provider_legacy ||
+ strcmp(id, "bclk") == 0) {
+
if (snd_config_get_string(n, &val) < 0)
return -EINVAL;
@@ -1462,11 +1470,19 @@ int tplg_parse_hw_config(snd_tplg_t *tplg, snd_config_t *cfg,
*/
SNDERR("deprecated bclk value '%s'", val);
- hw_cfg->bclk_master = SND_SOC_TPLG_BCLK_CS;
+ hw_cfg->bclk_provider = SND_SOC_TPLG_BCLK_CC;
} else if (!strcmp(val, "codec_slave")) {
- hw_cfg->bclk_master = SND_SOC_TPLG_BCLK_CS;
+ SNDERR("deprecated bclk value '%s', use 'codec_consumer'", val);
+
+ hw_cfg->bclk_provider = SND_SOC_TPLG_BCLK_CC;
+ } else if (!strcmp(val, "codec_consumer")) {
+ hw_cfg->bclk_provider = SND_SOC_TPLG_BCLK_CC;
} else if (!strcmp(val, "codec_master")) {
- hw_cfg->bclk_master = SND_SOC_TPLG_BCLK_CM;
+ SNDERR("deprecated bclk value '%s', use 'codec_provider", val);
+
+ hw_cfg->bclk_provider = SND_SOC_TPLG_BCLK_CP;
+ } else if (!strcmp(val, "codec_provider")) {
+ hw_cfg->bclk_provider = SND_SOC_TPLG_BCLK_CP;
}
continue;
}
@@ -1623,10 +1639,10 @@ int tplg_save_hw_config(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
if (err >= 0 && hc->fmt)
err = tplg_save_printf(dst, pfx, "\tformat '%s'\n",
get_audio_hw_format_name(hc->fmt));
- if (err >= 0 && hc->bclk_master)
+ if (err >= 0 && hc->bclk_provider)
err = tplg_save_printf(dst, pfx, "\tbclk '%s'\n",
- hc->bclk_master == SND_SOC_TPLG_BCLK_CS ?
- "codec_slave" : "codec_master");
+ hc->bclk_provider == SND_SOC_TPLG_BCLK_CC ?
+ "codec_consumer" : "codec_provider");
if (err >= 0 && hc->bclk_rate)
err = tplg_save_printf(dst, pfx, "\tbclk_freq %u\n",
hc->bclk_rate);
@@ -1791,7 +1807,7 @@ static int set_link_hw_config(struct snd_soc_tplg_hw_config *cfg,
cfg->clock_gated = tpl->clock_gated;
cfg->invert_bclk = tpl->invert_bclk;
cfg->invert_fsync = tpl->invert_fsync;
- cfg->bclk_master = tpl->bclk_master;
+ cfg->bclk_provider = tpl->bclk_provider;
cfg->fsync_master = tpl->fsync_master;
cfg->mclk_direction = tpl->mclk_direction;
cfg->reserved = tpl->reserved;
@@ -2174,7 +2190,7 @@ next:
hw->clock_gated = link->hw_config[i].clock_gated;
hw->invert_bclk = link->hw_config[i].invert_bclk;
hw->invert_fsync = link->hw_config[i].invert_fsync;
- hw->bclk_master = link->hw_config[i].bclk_master;
+ hw->bclk_provider = link->hw_config[i].bclk_provider;
hw->fsync_master = link->hw_config[i].fsync_master;
hw->mclk_direction = link->hw_config[i].mclk_direction;
hw->mclk_rate = link->hw_config[i].mclk_rate;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] topology: use inclusive language for fsync
2020-09-18 21:28 [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Pierre-Louis Bossart
2020-09-18 21:28 ` [PATCH 1/3] topology: use inclusive language for bclk Pierre-Louis Bossart
@ 2020-09-18 21:28 ` Pierre-Louis Bossart
2020-09-18 21:28 ` [PATCH 3/3] topology: use inclusive language in documentation Pierre-Louis Bossart
2020-09-29 7:18 ` [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Takashi Iwai
3 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2020-09-18 21:28 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, broonie, Pierre-Louis Bossart
use fsync_provider for structure fields, 'codec_provider' and
'codec_consumer' for options and modify #defines to use CP and CC
suffixes.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
include/sound/uapi/asoc.h | 11 +++++++----
include/topology.h | 2 +-
src/topology/pcm.c | 39 +++++++++++++++++++++++++++------------
3 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/include/sound/uapi/asoc.h b/include/sound/uapi/asoc.h
index ceafb1a9..f32c5697 100644
--- a/include/sound/uapi/asoc.h
+++ b/include/sound/uapi/asoc.h
@@ -178,10 +178,13 @@
#define SND_SOC_TPLG_BCLK_CS SND_SOC_TPLG_BCLK_CC
/* DAI topology FSYNC parameter
- * For the backwards capability, by default codec is fsync master
+ * For the backwards capability, by default codec is fsync provider
*/
-#define SND_SOC_TPLG_FSYNC_CM 0 /* codec is fsync master */
-#define SND_SOC_TPLG_FSYNC_CS 1 /* codec is fsync slave */
+#define SND_SOC_TPLG_FSYNC_CP 0 /* codec is fsync provider */
+#define SND_SOC_TPLG_FSYNC_CC 1 /* codec is fsync consumer */
+/* keep previous definitions for compatibility */
+#define SND_SOC_TPLG_FSYNC_CM SND_SOC_TPLG_FSYNC_CP
+#define SND_SOC_TPLG_FSYNC_CS SND_SOC_TPLG_FSYNC_CC
/*
* Block Header.
@@ -339,7 +342,7 @@ struct snd_soc_tplg_hw_config {
__u8 invert_bclk; /* 1 for inverted BCLK, 0 for normal */
__u8 invert_fsync; /* 1 for inverted frame clock, 0 for normal */
__u8 bclk_provider; /* SND_SOC_TPLG_BCLK_ value */
- __u8 fsync_master; /* SND_SOC_TPLG_FSYNC_ value */
+ __u8 fsync_provider; /* SND_SOC_TPLG_FSYNC_ value */
__u8 mclk_direction; /* SND_SOC_TPLG_MCLK_ value */
__le16 reserved; /* for 32bit alignment */
__le32 mclk_rate; /* MCLK or SYSCLK freqency in Hz */
diff --git a/include/topology.h b/include/topology.h
index 6c970649..4ade20df 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -1029,7 +1029,7 @@ struct snd_tplg_hw_config_template {
unsigned char invert_bclk; /* 1 for inverted BCLK, 0 for normal */
unsigned char invert_fsync; /* 1 for inverted frame clock, 0 for normal */
unsigned char bclk_provider; /* SND_SOC_TPLG_BCLK_ value */
- unsigned char fsync_master; /* SND_SOC_TPLG_FSYNC_ value */
+ unsigned char fsync_provider; /* SND_SOC_TPLG_FSYNC_ value */
unsigned char mclk_direction; /* SND_SOC_TPLG_MCLK_ value */
unsigned short reserved; /* for 32bit alignment */
unsigned int mclk_rate; /* MCLK or SYSCLK freqency in Hz */
diff --git a/src/topology/pcm.c b/src/topology/pcm.c
index f05df348..888aa59a 100644
--- a/src/topology/pcm.c
+++ b/src/topology/pcm.c
@@ -1504,8 +1504,15 @@ int tplg_parse_hw_config(snd_tplg_t *tplg, snd_config_t *cfg,
continue;
}
- if (strcmp(id, "fsync") == 0 ||
- strcmp(id, "fsync_master") == 0) {
+ provider_legacy = false;
+ if (strcmp(id, "fsync_master") == 0) {
+ SNDERR("deprecated option %s, please use 'fsync'\n", id);
+ provider_legacy = true;
+ }
+
+ if (provider_legacy ||
+ strcmp(id, "fsync") == 0) {
+
if (snd_config_get_string(n, &val) < 0)
return -EINVAL;
@@ -1515,11 +1522,19 @@ int tplg_parse_hw_config(snd_tplg_t *tplg, snd_config_t *cfg,
*/
SNDERR("deprecated fsync value '%s'", val);
- hw_cfg->fsync_master = SND_SOC_TPLG_FSYNC_CS;
+ hw_cfg->fsync_provider = SND_SOC_TPLG_FSYNC_CC;
} else if (!strcmp(val, "codec_slave")) {
- hw_cfg->fsync_master = SND_SOC_TPLG_FSYNC_CS;
- } else if (!strcmp(val, "codec_master")) {
- hw_cfg->fsync_master = SND_SOC_TPLG_FSYNC_CM;
+ SNDERR("deprecated fsync value '%s', use 'codec_consumer'", val);
+
+ hw_cfg->fsync_provider = SND_SOC_TPLG_FSYNC_CC;
+ } else if (!strcmp(val, "codec_consumer")) {
+ hw_cfg->fsync_provider = SND_SOC_TPLG_FSYNC_CC;
+ } else if (!strcmp(val, "codec_master")) {
+ SNDERR("deprecated fsync value '%s', use 'codec_provider'", val);
+
+ hw_cfg->fsync_provider = SND_SOC_TPLG_FSYNC_CP;
+ } else if (!strcmp(val, "codec_provider")) {
+ hw_cfg->fsync_provider = SND_SOC_TPLG_FSYNC_CP;
}
continue;
}
@@ -1648,10 +1663,10 @@ int tplg_save_hw_config(snd_tplg_t *tplg ATTRIBUTE_UNUSED,
hc->bclk_rate);
if (err >= 0 && hc->invert_bclk)
err = tplg_save_printf(dst, pfx, "\tbclk_invert 1\n");
- if (err >= 0 && hc->fsync_master)
- err = tplg_save_printf(dst, pfx, "\tfsync_master '%s'\n",
- hc->fsync_master == SND_SOC_TPLG_FSYNC_CS ?
- "codec_slave" : "codec_master");
+ if (err >= 0 && hc->fsync_provider)
+ err = tplg_save_printf(dst, pfx, "\tfsync_provider '%s'\n",
+ hc->fsync_provider == SND_SOC_TPLG_FSYNC_CC ?
+ "codec_consumer" : "codec_provider");
if (err >= 0 && hc->fsync_rate)
err = tplg_save_printf(dst, pfx, "\tfsync_freq %u\n",
hc->fsync_rate);
@@ -1808,7 +1823,7 @@ static int set_link_hw_config(struct snd_soc_tplg_hw_config *cfg,
cfg->invert_bclk = tpl->invert_bclk;
cfg->invert_fsync = tpl->invert_fsync;
cfg->bclk_provider = tpl->bclk_provider;
- cfg->fsync_master = tpl->fsync_master;
+ cfg->fsync_provider = tpl->fsync_provider;
cfg->mclk_direction = tpl->mclk_direction;
cfg->reserved = tpl->reserved;
cfg->mclk_rate = tpl->mclk_rate;
@@ -2191,7 +2206,7 @@ next:
hw->invert_bclk = link->hw_config[i].invert_bclk;
hw->invert_fsync = link->hw_config[i].invert_fsync;
hw->bclk_provider = link->hw_config[i].bclk_provider;
- hw->fsync_master = link->hw_config[i].fsync_master;
+ hw->fsync_provider = link->hw_config[i].fsync_provider;
hw->mclk_direction = link->hw_config[i].mclk_direction;
hw->mclk_rate = link->hw_config[i].mclk_rate;
hw->bclk_rate = link->hw_config[i].bclk_rate;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] topology: use inclusive language in documentation
2020-09-18 21:28 [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Pierre-Louis Bossart
2020-09-18 21:28 ` [PATCH 1/3] topology: use inclusive language for bclk Pierre-Louis Bossart
2020-09-18 21:28 ` [PATCH 2/3] topology: use inclusive language for fsync Pierre-Louis Bossart
@ 2020-09-18 21:28 ` Pierre-Louis Bossart
2020-09-29 7:18 ` [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Takashi Iwai
3 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2020-09-18 21:28 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, broonie, Pierre-Louis Bossart
Use codec_provider and codec_consumer for bclk and fsync
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
include/topology.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/topology.h b/include/topology.h
index 4ade20df..d1feee4d 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -658,8 +658,8 @@ extern "C" {
*
* id "1" # used for binding to the config
* format "I2S" # physical audio format.
- * bclk "master" # Platform is master of bit clock
- * fsync "slave" # Platform is slave of fsync
+ * bclk "codec_provider" # Codec provides the bit clock
+ * fsync "codec_consumer" # Codec follows the fsync
* }
* </pre>
*
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology
2020-09-18 21:28 [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Pierre-Louis Bossart
` (2 preceding siblings ...)
2020-09-18 21:28 ` [PATCH 3/3] topology: use inclusive language in documentation Pierre-Louis Bossart
@ 2020-09-29 7:18 ` Takashi Iwai
2020-09-29 13:44 ` Pierre-Louis Bossart
3 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2020-09-29 7:18 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: alsa-devel, broonie
On Fri, 18 Sep 2020 23:28:29 +0200,
Pierre-Louis Bossart wrote:
>
> The SOF (Sound Open Firmware) tree contains a lot of references in
> topology files to 'codec_slave'/'codec_master' terms, which in turn
> come from alsa-lib and ALSA/ASoC topology support at the kernel
> level. These terms are no longer compatible with the guidelines
> adopted by the kernel community [1] and need to change in
> backwards-compatible ways.
>
> The main/secondary terms typically suggested in guidelines don't mean
> anything for clocks, this patchset suggests instead the use of
> 'provider' and 'consumer' terms, with the 'codec' prefix kept to make
> it clear that the codec is the reference. The CM/CS suffixes are also
> replaced by CP/CC.
>
> It can be argued that the change of suffix is invasive, but finding a
> replacement that keeps the M and S shortcuts has proven difficult in
> quite a few contexts.
>
> The previous definitions are kept for backwards-compatibility so this
> change should not have any functional impact. It is suggested that new
> contributions only use the new terms but there is no requirement to
> transition immediately to the new definitions for existing code. Intel
> will however update all its past contributions related to bit
> clock/frame sync configurations immediately.
>
> This suggestion is easier to review first at the alsa-lib level, and
> if agreed follow-up contributions for the Linux kernel [2] and SOF
> firmware [3] will be provided.
It's OK from alsa-lib POV; although the uapi header change isn't 100%
safe, the user of this header is really our ones, so it's practically
acceptable, I suppose.
Could you submit the changes for kernel, so that it can be merged in
time? Basically alsa-lib is synced with kernel, so the kernel should
be changed at first.
thanks,
Takashi
>
> Feedback welcome
> ~Pierre
>
> [1] https://lkml.org/lkml/2020/7/4/229
> [2] https://github.com/plbossart/sound/tree/fix/inclusive-language-bclk-fsync
> [3] https://github.com/plbossart/sof/tree/fix/inclusive-language-bclk-fsync
>
> Changes since RFC:
> replaced 'follower' by 'consumer' as suggested by Jaroslav and Marc
> minor cleanups
>
> Pierre-Louis Bossart (3):
> topology: use inclusive language for bclk
> topology: use inclusive language for fsync
> topology: use inclusive language in documentation
>
> include/sound/uapi/asoc.h | 22 +++++++-----
> include/topology.h | 8 ++---
> src/topology/pcm.c | 75 +++++++++++++++++++++++++++------------
> 3 files changed, 71 insertions(+), 34 deletions(-)
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology
2020-09-29 7:18 ` [PATCH 0/3] alsa-lib/ASoC: use inclusive language for bclk/fsync/topology Takashi Iwai
@ 2020-09-29 13:44 ` Pierre-Louis Bossart
0 siblings, 0 replies; 6+ messages in thread
From: Pierre-Louis Bossart @ 2020-09-29 13:44 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, broonie
>> The SOF (Sound Open Firmware) tree contains a lot of references in
>> topology files to 'codec_slave'/'codec_master' terms, which in turn
>> come from alsa-lib and ALSA/ASoC topology support at the kernel
>> level. These terms are no longer compatible with the guidelines
>> adopted by the kernel community [1] and need to change in
>> backwards-compatible ways.
>>
>> The main/secondary terms typically suggested in guidelines don't mean
>> anything for clocks, this patchset suggests instead the use of
>> 'provider' and 'consumer' terms, with the 'codec' prefix kept to make
>> it clear that the codec is the reference. The CM/CS suffixes are also
>> replaced by CP/CC.
>>
>> It can be argued that the change of suffix is invasive, but finding a
>> replacement that keeps the M and S shortcuts has proven difficult in
>> quite a few contexts.
>>
>> The previous definitions are kept for backwards-compatibility so this
>> change should not have any functional impact. It is suggested that new
>> contributions only use the new terms but there is no requirement to
>> transition immediately to the new definitions for existing code. Intel
>> will however update all its past contributions related to bit
>> clock/frame sync configurations immediately.
>>
>> This suggestion is easier to review first at the alsa-lib level, and
>> if agreed follow-up contributions for the Linux kernel [2] and SOF
>> firmware [3] will be provided.
>
> It's OK from alsa-lib POV; although the uapi header change isn't 100%
> safe, the user of this header is really our ones, so it's practically
> acceptable, I suppose.
>
> Could you submit the changes for kernel, so that it can be merged in
> time? Basically alsa-lib is synced with kernel, so the kernel should
> be changed at first.
Ack, will do, thanks for the review.
^ permalink raw reply [flat|nested] 6+ messages in thread