[PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe

Shengjiu Wang posted 6 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe
Posted by Shengjiu Wang 2 months, 1 week ago
The IEC958 subframe format SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE are used
in HDMI and DisplayPort to describe the audio stream, but hardware device
may need to reorder the IEC958 bits for internal transmission, so need
these standard bits definitions for IEC958 subframe format.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 include/sound/asoundef.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/sound/asoundef.h b/include/sound/asoundef.h
index 09b2c3dffb30..7efd61568636 100644
--- a/include/sound/asoundef.h
+++ b/include/sound/asoundef.h
@@ -12,6 +12,15 @@
  *        Digital audio interface					    *
  *                                                                          *
  ****************************************************************************/
+/* IEC958 subframe format */
+#define IEC958_SUBFRAME_PREAMBLE_MASK	(0xf)
+#define IEC958_SUBFRAME_AUXILIARY_MASK	(0xf<<4)
+#define IEC958_SUBFRAME_SAMPLE_24_MASK	(0xffffff<<4)
+#define IEC958_SUBFRAME_SAMPLE_20_MASK	(0xfffff<<8)
+#define IEC958_SUBFRAME_VALIDITY	(0x1<<28)
+#define IEC958_SUBFRAME_USER_DATA	(0x1<<29)
+#define IEC958_SUBFRAME_CHANNEL_STATUS	(0x1<<30)
+#define IEC958_SUBFRAME_PARITY		(0x1<<31)
 
 /* AES/IEC958 channel status bits */
 #define IEC958_AES0_PROFESSIONAL	(1<<0)	/* 0 = consumer, 1 = professional */
-- 
2.34.1
Re: [PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe
Posted by Takashi Iwai 2 months, 1 week ago
On Thu, 24 Jul 2025 09:22:44 +0200,
Shengjiu Wang wrote:
> 
> The IEC958 subframe format SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE are used
> in HDMI and DisplayPort to describe the audio stream, but hardware device
> may need to reorder the IEC958 bits for internal transmission, so need
> these standard bits definitions for IEC958 subframe format.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  include/sound/asoundef.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/sound/asoundef.h b/include/sound/asoundef.h
> index 09b2c3dffb30..7efd61568636 100644
> --- a/include/sound/asoundef.h
> +++ b/include/sound/asoundef.h
> @@ -12,6 +12,15 @@
>   *        Digital audio interface					    *
>   *                                                                          *
>   ****************************************************************************/
> +/* IEC958 subframe format */
> +#define IEC958_SUBFRAME_PREAMBLE_MASK	(0xf)
> +#define IEC958_SUBFRAME_AUXILIARY_MASK	(0xf<<4)
> +#define IEC958_SUBFRAME_SAMPLE_24_MASK	(0xffffff<<4)
> +#define IEC958_SUBFRAME_SAMPLE_20_MASK	(0xfffff<<8)
> +#define IEC958_SUBFRAME_VALIDITY	(0x1<<28)
> +#define IEC958_SUBFRAME_USER_DATA	(0x1<<29)
> +#define IEC958_SUBFRAME_CHANNEL_STATUS	(0x1<<30)
> +#define IEC958_SUBFRAME_PARITY		(0x1<<31)

I'd use "U" suffix as it can reach to the MSB.
Also, you can put spaces around the operators to align with the
standard format, too.  I guess you followed to the other code there,
but following to the standard coding style would be better.

With those addressed, feel free to take my ack for this patch:

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi
Re: [PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe
Posted by Marc Kleine-Budde 2 months, 1 week ago
On 24.07.2025 09:37:09, Takashi Iwai wrote:
> On Thu, 24 Jul 2025 09:22:44 +0200,
> Shengjiu Wang wrote:
> > 
> > The IEC958 subframe format SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE are used
> > in HDMI and DisplayPort to describe the audio stream, but hardware device
> > may need to reorder the IEC958 bits for internal transmission, so need
> > these standard bits definitions for IEC958 subframe format.
> > 
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  include/sound/asoundef.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/include/sound/asoundef.h b/include/sound/asoundef.h
> > index 09b2c3dffb30..7efd61568636 100644
> > --- a/include/sound/asoundef.h
> > +++ b/include/sound/asoundef.h
> > @@ -12,6 +12,15 @@
> >   *        Digital audio interface					    *
> >   *                                                                          *
> >   ****************************************************************************/
> > +/* IEC958 subframe format */
> > +#define IEC958_SUBFRAME_PREAMBLE_MASK	(0xf)
> > +#define IEC958_SUBFRAME_AUXILIARY_MASK	(0xf<<4)
> > +#define IEC958_SUBFRAME_SAMPLE_24_MASK	(0xffffff<<4)
> > +#define IEC958_SUBFRAME_SAMPLE_20_MASK	(0xfffff<<8)
> > +#define IEC958_SUBFRAME_VALIDITY	(0x1<<28)
> > +#define IEC958_SUBFRAME_USER_DATA	(0x1<<29)
> > +#define IEC958_SUBFRAME_CHANNEL_STATUS	(0x1<<30)
> > +#define IEC958_SUBFRAME_PARITY		(0x1<<31)
> 
> I'd use "U" suffix as it can reach to the MSB.
> Also, you can put spaces around the operators to align with the
> standard format, too.  I guess you followed to the other code there,
> but following to the standard coding style would be better.
> 
> With those addressed, feel free to take my ack for this patch:

Or make use of the BIT() and GEN_MASK() helpers.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe
Posted by Shengjiu Wang 2 months, 1 week ago
On Thu, Jul 24, 2025 at 3:40 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 24.07.2025 09:37:09, Takashi Iwai wrote:
> > On Thu, 24 Jul 2025 09:22:44 +0200,
> > Shengjiu Wang wrote:
> > >
> > > The IEC958 subframe format SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE are used
> > > in HDMI and DisplayPort to describe the audio stream, but hardware device
> > > may need to reorder the IEC958 bits for internal transmission, so need
> > > these standard bits definitions for IEC958 subframe format.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  include/sound/asoundef.h | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/sound/asoundef.h b/include/sound/asoundef.h
> > > index 09b2c3dffb30..7efd61568636 100644
> > > --- a/include/sound/asoundef.h
> > > +++ b/include/sound/asoundef.h
> > > @@ -12,6 +12,15 @@
> > >   *        Digital audio interface                                      *
> > >   *                                                                          *
> > >   ****************************************************************************/
> > > +/* IEC958 subframe format */
> > > +#define IEC958_SUBFRAME_PREAMBLE_MASK      (0xf)
> > > +#define IEC958_SUBFRAME_AUXILIARY_MASK     (0xf<<4)
> > > +#define IEC958_SUBFRAME_SAMPLE_24_MASK     (0xffffff<<4)
> > > +#define IEC958_SUBFRAME_SAMPLE_20_MASK     (0xfffff<<8)
> > > +#define IEC958_SUBFRAME_VALIDITY   (0x1<<28)
> > > +#define IEC958_SUBFRAME_USER_DATA  (0x1<<29)
> > > +#define IEC958_SUBFRAME_CHANNEL_STATUS     (0x1<<30)
> > > +#define IEC958_SUBFRAME_PARITY             (0x1<<31)
> >
> > I'd use "U" suffix as it can reach to the MSB.
> > Also, you can put spaces around the operators to align with the
> > standard format, too.  I guess you followed to the other code there,
> > but following to the standard coding style would be better.
> >
> > With those addressed, feel free to take my ack for this patch:
>
> Or make use of the BIT() and GEN_MASK() helpers.

Is it acceptable to include the headers in this file?

Best regards
Shengjiu Wang


>
> Marc
>
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH v2 2/6] ALSA: Add definitions for the bits in IEC958 subframe
Posted by Takashi Iwai 2 months, 1 week ago
On Thu, 24 Jul 2025 12:14:10 +0200,
Shengjiu Wang wrote:
> 
> On Thu, Jul 24, 2025 at 3:40 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> >
> > On 24.07.2025 09:37:09, Takashi Iwai wrote:
> > > On Thu, 24 Jul 2025 09:22:44 +0200,
> > > Shengjiu Wang wrote:
> > > >
> > > > The IEC958 subframe format SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE are used
> > > > in HDMI and DisplayPort to describe the audio stream, but hardware device
> > > > may need to reorder the IEC958 bits for internal transmission, so need
> > > > these standard bits definitions for IEC958 subframe format.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  include/sound/asoundef.h | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/include/sound/asoundef.h b/include/sound/asoundef.h
> > > > index 09b2c3dffb30..7efd61568636 100644
> > > > --- a/include/sound/asoundef.h
> > > > +++ b/include/sound/asoundef.h
> > > > @@ -12,6 +12,15 @@
> > > >   *        Digital audio interface                                      *
> > > >   *                                                                          *
> > > >   ****************************************************************************/
> > > > +/* IEC958 subframe format */
> > > > +#define IEC958_SUBFRAME_PREAMBLE_MASK      (0xf)
> > > > +#define IEC958_SUBFRAME_AUXILIARY_MASK     (0xf<<4)
> > > > +#define IEC958_SUBFRAME_SAMPLE_24_MASK     (0xffffff<<4)
> > > > +#define IEC958_SUBFRAME_SAMPLE_20_MASK     (0xfffff<<8)
> > > > +#define IEC958_SUBFRAME_VALIDITY   (0x1<<28)
> > > > +#define IEC958_SUBFRAME_USER_DATA  (0x1<<29)
> > > > +#define IEC958_SUBFRAME_CHANNEL_STATUS     (0x1<<30)
> > > > +#define IEC958_SUBFRAME_PARITY             (0x1<<31)
> > >
> > > I'd use "U" suffix as it can reach to the MSB.
> > > Also, you can put spaces around the operators to align with the
> > > standard format, too.  I guess you followed to the other code there,
> > > but following to the standard coding style would be better.
> > >
> > > With those addressed, feel free to take my ack for this patch:
> >
> > Or make use of the BIT() and GEN_MASK() helpers.
> 
> Is it acceptable to include the headers in this file?

It's no UAPI, so should be fine (although I'm no big fan of
GEN_MASK()).

Other definitions can be converted with the macros, but it should be a
different patch.


thanks,

Takashi