| Message ID | 3d9fbdac702b93742841221e3e8d2468a3d18f12.1457465383.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Jyri, Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> With few nitpicking remarks on form. Regards Arnaud On 03/08/2016 09:14 PM, Jyri Sarha wrote: > Add IEC958 channel status helper that gets the audio properties from > snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to > produce the channel status bits already in audio stream configuration > phase. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > include/sound/pcm_iec958.h | 2 ++ > sound/core/pcm_iec958.c | 53 +++++++++++++++++++++++++++++++--------------- > 2 files changed, 38 insertions(+), 17 deletions(-) > > diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h > index 0eed397..36f023a 100644 > --- a/include/sound/pcm_iec958.h > +++ b/include/sound/pcm_iec958.h > @@ -6,4 +6,6 @@ > int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > size_t len); > > +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, > + u8 *cs, size_t len); > #endif > diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c > index 36b2d7a..27e0981 100644 > --- a/sound/core/pcm_iec958.c > +++ b/sound/core/pcm_iec958.c > @@ -9,30 +9,18 @@ > #include <linux/types.h> > #include <sound/asoundef.h> > #include <sound/pcm.h> > +#include <sound/pcm_params.h> > #include <sound/pcm_iec958.h> > > -/** > - * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status > - * @runtime: pcm runtime structure with ->rate filled in > - * @cs: channel status buffer, at least four bytes > - * @len: length of channel status buffer > - * > - * Create the consumer format channel status data in @cs of maximum size > - * @len corresponding to the parameters of the PCM runtime @runtime. > - * > - * Drivers may wish to tweak the contents of the buffer after creation. > - * > - * Returns: length of buffer, or negative error code if something failed. > - */ > -int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > - size_t len) > +static int create_iec958_consumer(uint rate, uint sample_width, > + u8 *cs, size_t len) > { > unsigned int fs, ws; > > if (len < 4) > return -EINVAL; > > - switch (runtime->rate) { > + switch (rate) { > case 32000: > fs = IEC958_AES3_CON_FS_32000; > break; > @@ -59,7 +47,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > } > > if (len > 4) { > - switch (snd_pcm_format_width(runtime->format)) { > + switch (sample_width) { > case 16: > ws = IEC958_AES4_CON_WORDLEN_20_16; > break; > @@ -71,6 +59,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > IEC958_AES4_CON_MAX_WORDLEN_24; > break; > case 24: > + case 32: /* Assume 24-bit width for 32-bit samples. */ > ws = IEC958_AES4_CON_WORDLEN_24_20 | > IEC958_AES4_CON_MAX_WORDLEN_24; > break; > @@ -92,4 +81,34 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > > return len; > } > + > +/** > + * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status > + * @runtime: pcm runtime structure with ->rate filled in > + * @cs: channel status buffer, at least four bytes > + * @len: length of channel status buffer > + * > + * Create the consumer format channel status data in @cs of maximum size > + * @len corresponding to the parameters of the PCM runtime @runtime. > + * > + * Drivers may wish to tweak the contents of the buffer after creation. > + * > + * Returns: length of buffer, or negative error code if something failed. > + */ > +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, > + size_t len) should be aligned on parenthesis > +{ > + return create_iec958_consumer(runtime->rate, > + snd_pcm_format_width(runtime->format), > + cs, len); > +} > EXPORT_SYMBOL(snd_pcm_create_iec958_consumer); > + > + No kernel-doc comment? > +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, Could be snd_pcm_hw_params_create_iec958_consumer to be coherent with functions declared in pcm.h > + u8 *cs, size_t len) should be aligned on parenthesis > +{ > + return create_iec958_consumer(params_rate(params), params_width(params), > + cs, len); > +} > +EXPORT_SYMBOL(snd_pcm_create_iec958_consumer_hw_params); >
On 03/09/16 12:08, Arnaud Pouliquen wrote: > Hello Jyri, > > Acked-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> > > With few nitpicking remarks on form. > > Regards > Arnaud > > On 03/08/2016 09:14 PM, Jyri Sarha wrote: >> Add IEC958 channel status helper that gets the audio properties from >> snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to >> produce the channel status bits already in audio stream configuration >> phase. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> include/sound/pcm_iec958.h | 2 ++ >> sound/core/pcm_iec958.c | 53 +++++++++++++++++++++++++++++++--------------- >> 2 files changed, 38 insertions(+), 17 deletions(-) >> >> diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h >> index 0eed397..36f023a 100644 >> --- a/include/sound/pcm_iec958.h >> +++ b/include/sound/pcm_iec958.h >> @@ -6,4 +6,6 @@ >> int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> size_t len); >> >> +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, >> + u8 *cs, size_t len); >> #endif >> diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c >> index 36b2d7a..27e0981 100644 >> --- a/sound/core/pcm_iec958.c >> +++ b/sound/core/pcm_iec958.c >> @@ -9,30 +9,18 @@ >> #include <linux/types.h> >> #include <sound/asoundef.h> >> #include <sound/pcm.h> >> +#include <sound/pcm_params.h> >> #include <sound/pcm_iec958.h> >> >> -/** >> - * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status >> - * @runtime: pcm runtime structure with ->rate filled in >> - * @cs: channel status buffer, at least four bytes >> - * @len: length of channel status buffer >> - * >> - * Create the consumer format channel status data in @cs of maximum size >> - * @len corresponding to the parameters of the PCM runtime @runtime. >> - * >> - * Drivers may wish to tweak the contents of the buffer after creation. >> - * >> - * Returns: length of buffer, or negative error code if something failed. >> - */ >> -int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> - size_t len) >> +static int create_iec958_consumer(uint rate, uint sample_width, >> + u8 *cs, size_t len) >> { >> unsigned int fs, ws; >> >> if (len < 4) >> return -EINVAL; >> >> - switch (runtime->rate) { >> + switch (rate) { >> case 32000: >> fs = IEC958_AES3_CON_FS_32000; >> break; >> @@ -59,7 +47,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> } >> >> if (len > 4) { >> - switch (snd_pcm_format_width(runtime->format)) { >> + switch (sample_width) { >> case 16: >> ws = IEC958_AES4_CON_WORDLEN_20_16; >> break; >> @@ -71,6 +59,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> IEC958_AES4_CON_MAX_WORDLEN_24; >> break; >> case 24: >> + case 32: /* Assume 24-bit width for 32-bit samples. */ >> ws = IEC958_AES4_CON_WORDLEN_24_20 | >> IEC958_AES4_CON_MAX_WORDLEN_24; >> break; >> @@ -92,4 +81,34 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> >> return len; >> } >> + >> +/** >> + * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status >> + * @runtime: pcm runtime structure with ->rate filled in >> + * @cs: channel status buffer, at least four bytes >> + * @len: length of channel status buffer >> + * >> + * Create the consumer format channel status data in @cs of maximum size >> + * @len corresponding to the parameters of the PCM runtime @runtime. >> + * >> + * Drivers may wish to tweak the contents of the buffer after creation. >> + * >> + * Returns: length of buffer, or negative error code if something failed. >> + */ >> +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, >> + size_t len) > should be aligned on parenthesis >> +{ >> + return create_iec958_consumer(runtime->rate, >> + snd_pcm_format_width(runtime->format), >> + cs, len); >> +} >> EXPORT_SYMBOL(snd_pcm_create_iec958_consumer); >> + >> + > No kernel-doc comment? Oh yes. It'll be mostly redundant with snd_pcm_create_iec958_consumer() but still it should be there. >> +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, > Could be snd_pcm_hw_params_create_iec958_consumer to be coherent with > functions declared in pcm.h I feel that iec958 should be earlier in the name because it is the common denominator in this c-file. Then again I have no strong opinions about this. >> + u8 *cs, size_t len) > should be aligned on parenthesis But it is. It just that the diff '+'-sign pushes the line above one char right, but the line bellow is not pushed because it indented with tabs (like it should be). >> +{ >> + return create_iec958_consumer(params_rate(params), params_width(params), >> + cs, len); >> +} >> +EXPORT_SYMBOL(snd_pcm_create_iec958_consumer_hw_params); >>
On Tue, Mar 08, 2016 at 10:14:25PM +0200, Jyri Sarha wrote: > Add IEC958 channel status helper that gets the audio properties from > snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to > produce the channel status bits already in audio stream configuration > phase. Takashi, are you OK with this one?
diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h index 0eed397..36f023a 100644 --- a/include/sound/pcm_iec958.h +++ b/include/sound/pcm_iec958.h @@ -6,4 +6,6 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, size_t len); +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, + u8 *cs, size_t len); #endif diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c index 36b2d7a..27e0981 100644 --- a/sound/core/pcm_iec958.c +++ b/sound/core/pcm_iec958.c @@ -9,30 +9,18 @@ #include <linux/types.h> #include <sound/asoundef.h> #include <sound/pcm.h> +#include <sound/pcm_params.h> #include <sound/pcm_iec958.h> -/** - * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status - * @runtime: pcm runtime structure with ->rate filled in - * @cs: channel status buffer, at least four bytes - * @len: length of channel status buffer - * - * Create the consumer format channel status data in @cs of maximum size - * @len corresponding to the parameters of the PCM runtime @runtime. - * - * Drivers may wish to tweak the contents of the buffer after creation. - * - * Returns: length of buffer, or negative error code if something failed. - */ -int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, - size_t len) +static int create_iec958_consumer(uint rate, uint sample_width, + u8 *cs, size_t len) { unsigned int fs, ws; if (len < 4) return -EINVAL; - switch (runtime->rate) { + switch (rate) { case 32000: fs = IEC958_AES3_CON_FS_32000; break; @@ -59,7 +47,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, } if (len > 4) { - switch (snd_pcm_format_width(runtime->format)) { + switch (sample_width) { case 16: ws = IEC958_AES4_CON_WORDLEN_20_16; break; @@ -71,6 +59,7 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, IEC958_AES4_CON_MAX_WORDLEN_24; break; case 24: + case 32: /* Assume 24-bit width for 32-bit samples. */ ws = IEC958_AES4_CON_WORDLEN_24_20 | IEC958_AES4_CON_MAX_WORDLEN_24; break; @@ -92,4 +81,34 @@ int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, return len; } + +/** + * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status + * @runtime: pcm runtime structure with ->rate filled in + * @cs: channel status buffer, at least four bytes + * @len: length of channel status buffer + * + * Create the consumer format channel status data in @cs of maximum size + * @len corresponding to the parameters of the PCM runtime @runtime. + * + * Drivers may wish to tweak the contents of the buffer after creation. + * + * Returns: length of buffer, or negative error code if something failed. + */ +int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs, + size_t len) +{ + return create_iec958_consumer(runtime->rate, + snd_pcm_format_width(runtime->format), + cs, len); +} EXPORT_SYMBOL(snd_pcm_create_iec958_consumer); + + +int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params, + u8 *cs, size_t len) +{ + return create_iec958_consumer(params_rate(params), params_width(params), + cs, len); +} +EXPORT_SYMBOL(snd_pcm_create_iec958_consumer_hw_params);
Add IEC958 channel status helper that gets the audio properties from snd_pcm_hw_params instead of snd_pcm_runtime. This is needed to produce the channel status bits already in audio stream configuration phase. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- include/sound/pcm_iec958.h | 2 ++ sound/core/pcm_iec958.c | 53 +++++++++++++++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 17 deletions(-)