Re: [PATCH] ASoC: Document DAI signal polarity

From: "Benoît Thébaudeau" <benoit.thebaudeau.dev@gmail.com>
To: Anatol Pomozov <anatol.pomozov@gmail.com>
Cc: albertchen@realtek.com, Alsa-devel <alsa-devel@alsa-project.org>,
	broonie@kernel.org, lars@metafoo.de
Subject: Re: [PATCH] ASoC: Document DAI signal polarity
Date: Thu, 1 Oct 2015 21:18:35 +0200	[thread overview]
Message-ID: <CA+sos79oT68x4-PMpHSrfqNiO_43fDQ50hjxxBkr7=M=mjv=LQ@mail.gmail.com> (raw)
In-Reply-To: <1443722959-5094-1-git-send-email-anatol.pomozov@gmail.com>

Dear Anatol Pomozov,

On Thu, Oct 1, 2015 at 8:09 PM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote:
> Currently there is no clear definition of what is FSYNC polarity.
> Different drivers use its own definition of what is "normal" and what is
> "inverted" fsync. This leads to compatibility problems between drivers.
>
> For example TegraX1 driver assumes that DSP-A format with frames
> starting at rising FSYNC edge has "negative" polarity,
> while RT5677 assumes it is "positive" polarity.

This wording is confusing. "Normal"/"inverted" are more appropriate
than "positive"/"negative" here, because the latter would refer to the
falling/rising edges, not to the regular polarity as the former.

> Explicitly specify meaning of BCLK/FSYNC polarity to avoid future
> compatibility problems.
>
> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
> ---
>  include/sound/soc-dai.h | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 2df96b1..45198db 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -48,10 +48,22 @@ struct snd_compr_stream;
>  #define SND_SOC_DAIFMT_GATED           (0 << 4) /* clock is gated */
>
>  /*
> - * DAI hardware signal inversions.
> + * DAI hardware signal polarity.
>   *
> - * Specifies whether the DAI can also support inverted clocks for the specified
> - * format.

Please keep this sentence or rephrase it, but do not remove it: it
clarifies "normal" as meaning the default for the specified format
(I²S, L/R-justified, DSP A/B, AC'97, PDM).

> + * BCLK:
> + * - "normal" polarity means signal is available at rising edge of BCLK
> + * - "inverted" polarity means signal is available at falling edge of BCLK
> + *
> + * FSYNC "normal" polarity depends on the frame format:
> + * - I2S: frame consists of left then right channel data. Left channel starts
> + *      with falling FSYNC edge, right channel starts with rising FSYNC edge.
> + * - I2S right/left justified: frame consists of left then right channel data.

The L/R-justified formats are independent of I²S, so "I2S" should be
removed from this line.

> + *      Left channel starts with rising FSYNC edge, right channel starts with
> + *      falling FSYNC edge.
> + * - DSP A/B: Frame starts with rising FSYNC edge.
> + * - AC97: Frame starts with rising FSYNC edge.
> + *
> + * "Negative" FSYNC polarity is the one opposite of "normal" polarity.
>   */
>  #define SND_SOC_DAIFMT_NB_NF           (0 << 8) /* normal bit clock + frame */
>  #define SND_SOC_DAIFMT_NB_IF           (2 << 8) /* normal BCLK + inv FRM */
> --
> 2.6.0.rc2.230.g3dd15c0
>

Best regards,
Benoît
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel