| Message ID | 20200903201024.1109914-1-pierre-louis.bossart@linux.intel.com |
|---|---|
| Headers | show |
| Series | alsa-lib/ASoC: use inclusive language for bclk/fsync/topology | expand |
Dne 03. 09. 20 v 22:10 Pierre-Louis Bossart napsal(a): > 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 'follower' terms, with the 'codec' prefix kept to make > it clear that the codec is the reference. The CM/CF suffixes are also > replaced by CP/CF. Only my 2 cents: It's just another word combo. See bellow for sources for others. I would prefer probably provider/consumer . It sounds more technic. [1] https://en.wikipedia.org/wiki/Master/slave_(technology) [2] https://www.zdnet.com/article/linux-team-approves-new-terminology-bans-terms-like-blacklist-and-slave/ Jaroslav
On 9/3/20 3:42 PM, Jaroslav Kysela wrote: > Dne 03. 09. 20 v 22:10 Pierre-Louis Bossart napsal(a): >> 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 'follower' terms, with the 'codec' prefix kept to make >> it clear that the codec is the reference. The CM/CF suffixes are also >> replaced by CP/CF. > > Only my 2 cents: It's just another word combo. See bellow for sources for others. > > I would prefer probably provider/consumer . It sounds more technic. Thanks Jaroslav for chiming in. I had a similar set of comments in internal reviews, but we didn't really have any consensus and I have not seen good guidance specifically for clocks. Provider/consumer is typically used for discrete data exchange with some sort of locking and buffer fullness metric, but for clocks we'd want something that hints at one device following the timing defined by another. "follow" or "track" seem clearer than 'consume' IMHO, but I will side with the majority, this is an RFC which can be modified at will.
On Thu, Sep 03, 2020 at 04:32:22PM -0500, Pierre-Louis Bossart wrote: > On 9/3/20 3:42 PM, Jaroslav Kysela wrote: > > > > Only my 2 cents: It's just another word combo. See bellow for sources for others. > > > > I would prefer probably provider/consumer . It sounds more technic. > Thanks Jaroslav for chiming in. I had a similar set of comments in internal > reviews, but we didn't really have any consensus and I have not seen good > guidance specifically for clocks. > Provider/consumer is typically used for discrete data exchange with some > sort of locking and buffer fullness metric, but for clocks we'd want > something that hints at one device following the timing defined by another. > "follow" or "track" seem clearer than 'consume' IMHO, but I will side with > the majority, this is an RFC which can be modified at will. Producer/consumer is already quite widely used for clocks (possibly following the regulator API which was templated off the clock API and uses consumer). The follow/track stuff definitely seems awkward to me. Have we seen any movement from anyone like CODEC vendors on this?
On 9/4/20 3:50 AM, Mark Brown wrote: > On Thu, Sep 03, 2020 at 04:32:22PM -0500, Pierre-Louis Bossart wrote: >> On 9/3/20 3:42 PM, Jaroslav Kysela wrote: > >>> >>> Only my 2 cents: It's just another word combo. See bellow for sources for others. >>> >>> I would prefer probably provider/consumer . It sounds more technic. > >> Thanks Jaroslav for chiming in. I had a similar set of comments in internal >> reviews, but we didn't really have any consensus and I have not seen good >> guidance specifically for clocks. > >> Provider/consumer is typically used for discrete data exchange with some >> sort of locking and buffer fullness metric, but for clocks we'd want >> something that hints at one device following the timing defined by another. > >> "follow" or "track" seem clearer than 'consume' IMHO, but I will side with >> the majority, this is an RFC which can be modified at will. > > Producer/consumer is already quite widely used for clocks (possibly > following the regulator API which was templated off the clock API and > uses consumer). The follow/track stuff definitely seems awkward to me. > Have we seen any movement from anyone like CODEC vendors on this? No, I haven't seen any input from CODEC vendors. I'll use consumer then since that's preferred by both Jaroslav and Mark. Thanks for the feedback.
>> /* 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 */ > > Is it 100% compatible? Note that the uapi/* header is a copy from the > kernel header, and it means that we'll change the same for the kernel, > too. It's absolutely 100% compatible by design. I was planning to update the kernel uapi header to align changes, but the volume of code is much lower on the alsa-lib side. Will resubmit with the preferred provider/consumer wording.
On Tue, Sep 08, 2020 at 08:39:13AM -0500, Pierre-Louis Bossart wrote: > > > - __u8 bclk_master; /* SND_SOC_TPLG_BCLK_ value */ > > > + __u8 bclk_provider; /* SND_SOC_TPLG_BCLK_ value */ > > Is it 100% compatible? Note that the uapi/* header is a copy from the > > kernel header, and it means that we'll change the same for the kernel, > > too. > It's absolutely 100% compatible by design. > I was planning to update the kernel uapi header to align changes, but the > volume of code is much lower on the alsa-lib side. Will resubmit with the > preferred provider/consumer wording. It's binary compatible but it'd break the build for any existing code using the UAPI headers.