[PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Xiubo Li]

This property used for configuring whether the LSB or the MSB is transmitted
first for the fifo data.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Cc: Nicolin Chen <nicoleotsuka@gmail.com>
---
 Documentation/devicetree/bindings/sound/fsl-sai.txt | 8 ++++----
 sound/soc/fsl/fsl_sai.c                             | 6 +++---
 sound/soc/fsl/fsl_sai.h                             | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
index 06a405e..4956b14 100644
--- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
@@ -20,9 +20,9 @@ Required properties:
   See ../pinctrl/pinctrl-bindings.txt for details of the property values.
 - big-endian: Boolean property, required if all the FTM_PWM registers
   are big-endian rather than little-endian.
-- big-endian-data: If this property is absent, the little endian mode will
-  be in use as default, or the big endian mode will be in use for all the
-  fifo data.
+- lsb-first: Configures whether the LSB or the MSB is transmitted first for
+  the fifo data. If this property is absent, the MSB is transmitted first as
+  default, or the LSB is transmitted first.
 - fsl,sai-synchronous-rx: This is a boolean property. If present, indicating
   that SAI will work in the synchronous mode (sync Tx with Rx) which means
   both the transimitter and receiver will send and receive data by following
@@ -53,5 +53,5 @@ sai2: sai@40031000 {
 	      dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
 		   <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
 	      big-endian;
-	      big-endian-data;
+	      lsb-first;
 };
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index a6eb784..7eeb1dd 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -175,7 +175,7 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 	bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
 	u32 val_cr2 = 0, val_cr4 = 0;
 
-	if (!sai->big_endian_data)
+	if (!sai->is_lsb_first)
 		val_cr4 |= FSL_SAI_CR4_MF;
 
 	/* DAI mode */
@@ -304,7 +304,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 	val_cr5 |= FSL_SAI_CR5_WNW(word_width);
 	val_cr5 |= FSL_SAI_CR5_W0W(word_width);
 
-	if (sai->big_endian_data)
+	if (sai->is_lsb_first)
 		val_cr5 |= FSL_SAI_CR5_FBT(0);
 	else
 		val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1);
@@ -573,7 +573,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx6sx-sai"))
 		sai->sai_on_imx = true;
 
-	sai->big_endian_data = of_property_read_bool(np, "big-endian-data");
+	sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(&pdev->dev, res);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 2cded44..3466720 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -132,7 +132,7 @@ struct fsl_sai {
 	struct clk *bus_clk;
 	struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
 
-	bool big_endian_data;
+	bool is_lsb_first;
 	bool is_dsp_mode;
 	bool sai_on_imx;
 	bool synchronous[2];
-- 
1.8.5

Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Mark Rutland]

On Mon, Aug 25, 2014 at 08:16:01AM +0100, Xiubo Li wrote:
> This property used for configuring whether the LSB or the MSB is transmitted
> first for the fifo data.

I don't follow the rationale for this change.

This looks like a pointless renaming.

Why is this any better?

Thanks,
Mark.

> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> Cc: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl-sai.txt | 8 ++++----
>  sound/soc/fsl/fsl_sai.c                             | 6 +++---
>  sound/soc/fsl/fsl_sai.h                             | 2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> index 06a405e..4956b14 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> @@ -20,9 +20,9 @@ Required properties:
>    See ../pinctrl/pinctrl-bindings.txt for details of the property values.
>  - big-endian: Boolean property, required if all the FTM_PWM registers
>    are big-endian rather than little-endian.
> -- big-endian-data: If this property is absent, the little endian mode will
> -  be in use as default, or the big endian mode will be in use for all the
> -  fifo data.
> +- lsb-first: Configures whether the LSB or the MSB is transmitted first for
> +  the fifo data. If this property is absent, the MSB is transmitted first as
> +  default, or the LSB is transmitted first.
>  - fsl,sai-synchronous-rx: This is a boolean property. If present, indicating
>    that SAI will work in the synchronous mode (sync Tx with Rx) which means
>    both the transimitter and receiver will send and receive data by following
> @@ -53,5 +53,5 @@ sai2: sai@40031000 {
>  	      dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
>  		   <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
>  	      big-endian;
> -	      big-endian-data;
> +	      lsb-first;
>  };
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index a6eb784..7eeb1dd 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -175,7 +175,7 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
>  	bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
>  	u32 val_cr2 = 0, val_cr4 = 0;
>  
> -	if (!sai->big_endian_data)
> +	if (!sai->is_lsb_first)
>  		val_cr4 |= FSL_SAI_CR4_MF;
>  
>  	/* DAI mode */
> @@ -304,7 +304,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
>  	val_cr5 |= FSL_SAI_CR5_WNW(word_width);
>  	val_cr5 |= FSL_SAI_CR5_W0W(word_width);
>  
> -	if (sai->big_endian_data)
> +	if (sai->is_lsb_first)
>  		val_cr5 |= FSL_SAI_CR5_FBT(0);
>  	else
>  		val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1);
> @@ -573,7 +573,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
>  	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx6sx-sai"))
>  		sai->sai_on_imx = true;
>  
> -	sai->big_endian_data = of_property_read_bool(np, "big-endian-data");
> +	sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	base = devm_ioremap_resource(&pdev->dev, res);
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index 2cded44..3466720 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -132,7 +132,7 @@ struct fsl_sai {
>  	struct clk *bus_clk;
>  	struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
>  
> -	bool big_endian_data;
> +	bool is_lsb_first;
>  	bool is_dsp_mode;
>  	bool sai_on_imx;
>  	bool synchronous[2];
> -- 
> 1.8.5
> 
> 

RE: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Li.Xiubo]

> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland@arm.com]
> Sent: Tuesday, August 26, 2014 5:04 PM
> To: Xiubo Li-B47053
> Cc: broonie@kernel.org; nicoleotsuka@gmail.com; timur@tabi.org;
> lgirdwood@gmail.com; perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-
> project.org; galak@codeaurora.org; robh+dt@kernel.org; Pawel Moll; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of
> 'big-endian-data'.
> 
> On Mon, Aug 25, 2014 at 08:16:01AM +0100, Xiubo Li wrote:
> > This property used for configuring whether the LSB or the MSB is transmitted
> > first for the fifo data.
> 
> I don't follow the rationale for this change.
> 
> This looks like a pointless renaming.
> 
> Why is this any better?
> 

This is originally to indicate whether the LSB firstly or MSB firstly will be
transmitted to the CODEC or received from the CODEC, and there has nothing
Relation to the memory data.

Generally, if the audio data in big endian format, which will be using the bytes
Reversion ? Here this can only be used to bits reversion.

Thanks,


BRs
Xiubo


> Thanks,
> Mark.
> 
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > Cc: Nicolin Chen <nicoleotsuka@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/sound/fsl-sai.txt | 8 ++++----
> >  sound/soc/fsl/fsl_sai.c                             | 6 +++---
> >  sound/soc/fsl/fsl_sai.h                             | 2 +-
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> > index 06a405e..4956b14 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> > +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> > @@ -20,9 +20,9 @@ Required properties:
> >    See ../pinctrl/pinctrl-bindings.txt for details of the property values.
> >  - big-endian: Boolean property, required if all the FTM_PWM registers
> >    are big-endian rather than little-endian.
> > -- big-endian-data: If this property is absent, the little endian mode will
> > -  be in use as default, or the big endian mode will be in use for all the
> > -  fifo data.
> > +- lsb-first: Configures whether the LSB or the MSB is transmitted first for
> > +  the fifo data. If this property is absent, the MSB is transmitted first
> as
> > +  default, or the LSB is transmitted first.
> >  - fsl,sai-synchronous-rx: This is a boolean property. If present,
> indicating
> >    that SAI will work in the synchronous mode (sync Tx with Rx) which means
> >    both the transimitter and receiver will send and receive data by
> following
> > @@ -53,5 +53,5 @@ sai2: sai@40031000 {
> >  	      dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
> >  		   <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
> >  	      big-endian;
> > -	      big-endian-data;
> > +	      lsb-first;
> >  };
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index a6eb784..7eeb1dd 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -175,7 +175,7 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai
> *cpu_dai,
> >  	bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
> >  	u32 val_cr2 = 0, val_cr4 = 0;
> >
> > -	if (!sai->big_endian_data)
> > +	if (!sai->is_lsb_first)
> >  		val_cr4 |= FSL_SAI_CR4_MF;
> >
> >  	/* DAI mode */
> > @@ -304,7 +304,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
> *substream,
> >  	val_cr5 |= FSL_SAI_CR5_WNW(word_width);
> >  	val_cr5 |= FSL_SAI_CR5_W0W(word_width);
> >
> > -	if (sai->big_endian_data)
> > +	if (sai->is_lsb_first)
> >  		val_cr5 |= FSL_SAI_CR5_FBT(0);
> >  	else
> >  		val_cr5 |= FSL_SAI_CR5_FBT(word_width - 1);
> > @@ -573,7 +573,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
> >  	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx6sx-sai"))
> >  		sai->sai_on_imx = true;
> >
> > -	sai->big_endian_data = of_property_read_bool(np, "big-endian-data");
> > +	sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
> >
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	base = devm_ioremap_resource(&pdev->dev, res);
> > diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> > index 2cded44..3466720 100644
> > --- a/sound/soc/fsl/fsl_sai.h
> > +++ b/sound/soc/fsl/fsl_sai.h
> > @@ -132,7 +132,7 @@ struct fsl_sai {
> >  	struct clk *bus_clk;
> >  	struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
> >
> > -	bool big_endian_data;
> > +	bool is_lsb_first;
> >  	bool is_dsp_mode;
> >  	bool sai_on_imx;
> >  	bool synchronous[2];
> > --
> > 1.8.5
> >
> >

Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Nicolin Chen]

On Tue, Aug 26, 2014 at 7:12 PM, Li.Xiubo@freescale.com
<Li.Xiubo@freescale.com> wrote:
> > > This property used for configuring whether the LSB or the MSB is transmitted
> > > first for the fifo data.
> >
> > I don't follow the rationale for this change.
> >
> > This looks like a pointless renaming.
> >
> > Why is this any better?
> >
>
> This is originally to indicate whether the LSB firstly or MSB firstly will be
> transmitted to the CODEC or received from the CODEC, and there has nothing
> Relation to the memory data.
>
> Generally, if the audio data in big endian format, which will be using the bytes
> Reversion ? Here this can only be used to bits reversion.

That's why I suggested previously: mention it in the commit comment.

People will have no idea about what's the exact reason for this change.
Any functional enhancement? Or a bug fix(more likely for this one).
I'm not sure what I described in the previous mail is correct or not.
But you probably should have added it into the commit comments.

And Xiubo, this patch still hasn't fixed the issue of breaking the old DT,
although system can pass during the probe() section.

Thank you,
Nicolin

RE: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Li.Xiubo]

> Subject: Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of
> 'big-endian-data'.
> 
> On Tue, Aug 26, 2014 at 7:12 PM, Li.Xiubo@freescale.com
> <Li.Xiubo@freescale.com> wrote:
> > > > This property used for configuring whether the LSB or the MSB is
> transmitted
> > > > first for the fifo data.
> > >
> > > I don't follow the rationale for this change.
> > >
> > > This looks like a pointless renaming.
> > >
> > > Why is this any better?
> > >
> >
> > This is originally to indicate whether the LSB firstly or MSB firstly will
> be
> > transmitted to the CODEC or received from the CODEC, and there has nothing
> > Relation to the memory data.
> >
> > Generally, if the audio data in big endian format, which will be using the
> bytes
> > Reversion ? Here this can only be used to bits reversion.
> 
> That's why I suggested previously: mention it in the commit comment.
> 
> People will have no idea about what's the exact reason for this change.
> Any functional enhancement? Or a bug fix(more likely for this one).
> I'm not sure what I described in the previous mail is correct or not.
> But you probably should have added it into the commit comments.
> 
> And Xiubo, this patch still hasn't fixed the issue of breaking the old DT,
> although system can pass during the probe() section.
> 

Beside LS1+, who else is using this now ? And the LS1 dts files are still doing
The upstream for now.

Thanks,

BRs
Xiubo

Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Tue, Aug 26, 2014 at 11:53:56PM -0700, Nicolin Chen wrote:

> That's why I suggested previously: mention it in the commit comment.

> People will have no idea about what's the exact reason for this change.
> Any functional enhancement? Or a bug fix(more likely for this one).
> I'm not sure what I described in the previous mail is correct or not.
> But you probably should have added it into the commit comments.

Right, this is a good idea - make sure people can understand what the
goal of the change is.

> And Xiubo, this patch still hasn't fixed the issue of breaking the old DT,
> although system can pass during the probe() section.

Indeed.  Please do this.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of 'big-endian-data'.

[Li.Xiubo]

> Subject: Re: [PATCH] ASoC: fsl-sai: using 'lsb-first' property instead of
> 'big-endian-data'.
> 
> On Tue, Aug 26, 2014 at 11:53:56PM -0700, Nicolin Chen wrote:
> 
> > That's why I suggested previously: mention it in the commit comment.
> 
> > People will have no idea about what's the exact reason for this change.
> > Any functional enhancement? Or a bug fix(more likely for this one).
> > I'm not sure what I described in the previous mail is correct or not.
> > But you probably should have added it into the commit comments.
> 
> Right, this is a good idea - make sure people can understand what the
> goal of the change is.
> 

Okay, I will add this.

Thanks,

BRs
Xiubo