[v2,6/7] dt-bindings: media: Add Renesas R-Car DRIF binding

  Add binding documentation for Renesas R-Car Digital Radio Interface
(DRIF) controller.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 .../devicetree/bindings/media/renesas,drif.txt     | 202 +++++++++++++++++++++
 1 file changed, 202 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
  

Hi Ramesh,

Thank you for the patch.

On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
> Add binding documentation for Renesas R-Car Digital Radio Interface
> (DRIF) controller.
> 
> Signed-off-by: Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> ---
>  .../devicetree/bindings/media/renesas,drif.txt     | 202 ++++++++++++++++++
>  1 file changed, 202 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file mode
> 100644
> index 0000000..1f3feaf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,202 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +------------------------------------------------------------
> +
> +R-Car Gen3 DRIF is a SPI like receive only slave device. A general
> +representation of DRIF interfacing with a master device is shown below.
> +
> ++---------------------+                +---------------------+
> +|                     |-----SCK------->|CLK                  |
> +|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
> +|                     |-----SD0------->|D0                   |
> +|                     |-----SD1------->|D1                   |
> ++---------------------+                +---------------------+
> +
> +As per datasheet, each DRIF channel (drifn) is made up of two internal
> +channels (drifn0 & drifn1). These two internal channels share the common
> +CLK & SYNC. Each internal channel has its own dedicated resources like
> +irq, dma channels, address space & clock. This internal split is not
> +visible to the external master device.
> +
> +The device tree model represents each internal channel as a separate node.
> +The internal channels sharing the CLK & SYNC are tied together by their
> +phandles using a new property called "renesas,bonding". For the rest of
> +the documentation, unless explicitly stated, the word channel implies an
> +internal channel.
> +
> +When both internal channels are enabled they need to be managed together
> +as one (i.e.) they cannot operate alone as independent devices. Out of the
> +two, one of them needs to act as a primary device that accepts common
> +properties of both the internal channels. This channel is identified by a
> +new property called "renesas,primary-bond".
> +
> +To summarize,
> +   - When both the internal channels that are bonded together are enabled,
> +     the zeroth channel is selected as primary-bond. This channels accepts
> +     properties common to all the members of the bond.
> +   - When only one of the bonded channels need to be enabled, the property
> +     "renesas,bonding" or "renesas,primary-bond" will have no effect. That
> +     enabled channel can act alone as any other independent device.
> +
> +Required properties of an internal channel:
> +-------------------------------------------
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
> R8A7795 SoC.
> +	      "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
> device.
> +	      When compatible with the generic version, nodes must list the
> +	      SoC-specific version corresponding to the platform first
> +	      followed by the generic version.
> +- reg: offset and length of that channel.
> +- interrupts: associated with that channel.
> +- clocks: phandle and clock specifier of that channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channels.
> +- dma-names: names of the DMA channel: "rx".
> +- renesas,bonding: phandle to the other channel.
> +
> +Optional properties of an internal channel:
> +-------------------------------------------
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of an internal channel when:
> +	- It is the only enabled channel of the bond (or)
> +	- If it acts as primary among enabled bonds
> +--------------------------------------------------------
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- renesas,primary-bond: empty property indicating the channel acts as
> primary
> +			among the bonded channels.
> +- port: child port node of a channel that defines the local and remote
> +	endpoints. The remote endpoint is assumed to be a third party tuner
> +	device endpoint.
> +
> +Optional properties of an internal channel when:
> +	- It is the only enabled channel of the bond (or)
> +	- If it acts as primary among enabled bonds
> +--------------------------------------------------------
> +- renesas,syncmd       : sync mode
> +			 0 (Frame start sync pulse mode. 1-bit width pulse
> +			    indicates start of a frame)
> +			 1 (L/R sync or I2S mode) (default)
> +- renesas,lsb-first    : empty property indicates lsb bit is received
> first.
> +			 When not defined msb bit is received first (default)
> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for low/high
> +			 respectively. The default is 1 (active high)
> +- renesas,dtdl         : delay between sync signal and start of reception.
> +			 The possible values are represented in 0.5 clock
> +			 cycle units and the range is 0 to 4. The default
> +			 value is 2 (i.e.) 1 clock cycle delay.
> +- renesas,syncdl       : delay between end of reception and sync signal
> edge.
> +			 The possible values are represented in 0.5 clock
> +			 cycle units and the range is 0 to 4 & 6. The default
> +			 value is 0 (i.e.) no delay.

Most of these properties are pretty similar to the video bus properties 
defined at the endpoint level in 
Documentation/devicetree/bindings/media/video-interfaces.txt. I believe it 
would make sense to use OF graph and try to standardize these properties 
similarly.

> +
> +Example
> +--------
> +
> +SoC common dtsi file
> +
> +		drif00: rif@e6f40000 {
> +			compatible = "renesas,r8a7795-drif",
> +				     "renesas,rcar-gen3-drif";
> +			reg = <0 0xe6f40000 0 0x64>;
> +			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 515>;
> +			clock-names = "fck";
> +			dmas = <&dmac1 0x20>, <&dmac2 0x20>;
> +			dma-names = "rx", "rx";
> +			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> +			renesas,bonding = <&drif01>;
> +			status = "disabled";
> +		};
> +
> +		drif01: rif@e6f50000 {
> +			compatible = "renesas,r8a7795-drif",
> +				     "renesas,rcar-gen3-drif";
> +			reg = <0 0xe6f50000 0 0x64>;
> +			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 514>;
> +			clock-names = "fck";
> +			dmas = <&dmac1 0x22>, <&dmac2 0x22>;
> +			dma-names = "rx", "rx";
> +			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> +			renesas,bonding = <&drif00>;
> +			status = "disabled";
> +		};
> +
> +
> +Board specific dts file
> +
> +(1) Both internal channels enabled, primary-bond = 0
> +-----------------------------------------------------
> +
> +When interfacing with a third party tuner device with two data pins as
> shown +below.
> +
> ++---------------------+                +---------------------+
> +|                     |-----SCK------->|CLK                  |
> +|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
> +|                     |-----SD0------->|D0                   |
> +|                     |-----SD1------->|D1                   |
> ++---------------------+                +---------------------+
> +
> +pfc {
> +	...
> +
> +	drif0_pins: drif0 {
> +		groups = "drif0_ctrl_a", "drif0_data0_a",
> +				 "drif0_data1_a";
> +		function = "drif0";
> +	};
> +	...
> +}
> +
> +&drif00 {
> +	pinctrl-0 = <&drif0_pins>;
> +	pinctrl-names = "default";
> +	renesas,syncac-active = <1>;
> +	renesas,primary-bond;
> +	status = "okay";
> +	port {
> +		drif0_ep: endpoint {
> +		     remote-endpoint = <&tuner_ep>;
> +		};
> +	};
> +};
> +
> +&drif01 {
> +	status = "okay";
> +};
> +
> +(2) Internal channel 1 alone is enabled:
> +----------------------------------------
> +
> +When interfacing with a third party tuner device with one data pin as shown
> +below.
> +
> ++---------------------+                +---------------------+
> +|                     |-----SCK------->|CLK                  |
> +|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
> +|                     |                |D0 (unused)          |
> +|                     |-----SD-------->|D1                   |
> ++---------------------+                +---------------------+
> +
> +pfc {
> +	...
> +
> +	drif0_pins: drif0 {
> +		groups = "drif0_ctrl_a", "drif0_data1_a";
> +		function = "drif0";
> +	};
> +	...
> +}
> +
> +&drif01 {
> +	pinctrl-0 = <&drif0_pins>;
> +	pinctrl-names = "default";
> +	renesas,syncac-active = <0>;
> +	status = "okay";
> +	port {
> +		drif0_ep: endpoint {
> +		     remote-endpoint = <&tuner_ep>;
> +		};
> +	};
> +};
  

Hi Laurent,

On Thu, Dec 22, 2016 at 6:05 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
>> Add binding documentation for Renesas R-Car Digital Radio Interface
>> (DRIF) controller.
>>
>> Signed-off-by: Ramesh Shanmugasundaram
>> <ramesh.shanmugasundaram@bp.renesas.com> ---
>>  .../devicetree/bindings/media/renesas,drif.txt     | 202 ++++++++++++++++++
>>  1 file changed, 202 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file mode
>> 100644
>> index 0000000..1f3feaf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt

>> +Optional properties of an internal channel when:
>> +     - It is the only enabled channel of the bond (or)
>> +     - If it acts as primary among enabled bonds
>> +--------------------------------------------------------
>> +- renesas,syncmd       : sync mode
>> +                      0 (Frame start sync pulse mode. 1-bit width pulse
>> +                         indicates start of a frame)
>> +                      1 (L/R sync or I2S mode) (default)
>> +- renesas,lsb-first    : empty property indicates lsb bit is received
>> first.
>> +                      When not defined msb bit is received first (default)
>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for low/high
>> +                      respectively. The default is 1 (active high)
>> +- renesas,dtdl         : delay between sync signal and start of reception.
>> +                      The possible values are represented in 0.5 clock
>> +                      cycle units and the range is 0 to 4. The default
>> +                      value is 2 (i.e.) 1 clock cycle delay.
>> +- renesas,syncdl       : delay between end of reception and sync signal
>> edge.
>> +                      The possible values are represented in 0.5 clock
>> +                      cycle units and the range is 0 to 4 & 6. The default
>> +                      value is 0 (i.e.) no delay.
>
> Most of these properties are pretty similar to the video bus properties
> defined at the endpoint level in
> Documentation/devicetree/bindings/media/video-interfaces.txt. I believe it
> would make sense to use OF graph and try to standardize these properties
> similarly.

Note that the last two properties match the those in
Documentation/devicetree/bindings/spi/sh-msiof.txt.
We may want to use one DRIF channel as a plain SPI slave with the
(modified) MSIOF driver in the future.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Laurent, Geert,

Thanks for the review comments.

> > On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:

> >> Add binding documentation for Renesas R-Car Digital Radio Interface

> >> (DRIF) controller.

> >>

> >> Signed-off-by: Ramesh Shanmugasundaram

> >> <ramesh.shanmugasundaram@bp.renesas.com> ---

> >>  .../devicetree/bindings/media/renesas,drif.txt     | 202

> ++++++++++++++++++

> >>  1 file changed, 202 insertions(+)

> >>  create mode 100644

> >> Documentation/devicetree/bindings/media/renesas,drif.txt

> >>

> >> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt

> >> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file

> >> mode

> >> 100644

> >> index 0000000..1f3feaf

> >> --- /dev/null

> >> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt

> 

> >> +Optional properties of an internal channel when:

> >> +     - It is the only enabled channel of the bond (or)

> >> +     - If it acts as primary among enabled bonds

> >> +--------------------------------------------------------

> >> +- renesas,syncmd       : sync mode

> >> +                      0 (Frame start sync pulse mode. 1-bit width

> pulse

> >> +                         indicates start of a frame)

> >> +                      1 (L/R sync or I2S mode) (default)

> >> +- renesas,lsb-first    : empty property indicates lsb bit is received

> >> first.

> >> +                      When not defined msb bit is received first

> >> +(default)

> >> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for

> low/high

> >> +                      respectively. The default is 1 (active high)

> >> +- renesas,dtdl         : delay between sync signal and start of

> reception.

> >> +                      The possible values are represented in 0.5 clock

> >> +                      cycle units and the range is 0 to 4. The default

> >> +                      value is 2 (i.e.) 1 clock cycle delay.

> >> +- renesas,syncdl       : delay between end of reception and sync

> signal

> >> edge.

> >> +                      The possible values are represented in 0.5 clock

> >> +                      cycle units and the range is 0 to 4 & 6. The

> default

> >> +                      value is 0 (i.e.) no delay.

> >

> > Most of these properties are pretty similar to the video bus

> > properties defined at the endpoint level in

> > Documentation/devicetree/bindings/media/video-interfaces.txt. I

> > believe it would make sense to use OF graph and try to standardize

> > these properties similarly.

> 

> Note that the last two properties match the those in

> Documentation/devicetree/bindings/spi/sh-msiof.txt.

> We may want to use one DRIF channel as a plain SPI slave with the

> (modified) MSIOF driver in the future.


Should I leave it as it is or modify these as in video-interfaces.txt? Shall we conclude on this please?

Thanks,
Ramesh
  

On 01/03/2017 04:20 PM, Ramesh Shanmugasundaram wrote:
> Hi Laurent, Geert,
> 
> Thanks for the review comments.
> 
>>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
>>>> Add binding documentation for Renesas R-Car Digital Radio Interface
>>>> (DRIF) controller.
>>>>
>>>> Signed-off-by: Ramesh Shanmugasundaram
>>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
>>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202
>> ++++++++++++++++++
>>>>  1 file changed, 202 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/media/renesas,drif.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
>>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file
>>>> mode
>>>> 100644
>>>> index 0000000..1f3feaf
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
>>
>>>> +Optional properties of an internal channel when:
>>>> +     - It is the only enabled channel of the bond (or)
>>>> +     - If it acts as primary among enabled bonds
>>>> +--------------------------------------------------------
>>>> +- renesas,syncmd       : sync mode
>>>> +                      0 (Frame start sync pulse mode. 1-bit width
>> pulse
>>>> +                         indicates start of a frame)
>>>> +                      1 (L/R sync or I2S mode) (default)
>>>> +- renesas,lsb-first    : empty property indicates lsb bit is received
>>>> first.
>>>> +                      When not defined msb bit is received first
>>>> +(default)
>>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for
>> low/high

Shouldn't this be 'renesas,sync-active' instead of syncac-active?

I'm not sure if syncac is intended or if it is a typo.

>>>> +                      respectively. The default is 1 (active high)
>>>> +- renesas,dtdl         : delay between sync signal and start of
>> reception.
>>>> +                      The possible values are represented in 0.5 clock
>>>> +                      cycle units and the range is 0 to 4. The default
>>>> +                      value is 2 (i.e.) 1 clock cycle delay.
>>>> +- renesas,syncdl       : delay between end of reception and sync
>> signal
>>>> edge.
>>>> +                      The possible values are represented in 0.5 clock
>>>> +                      cycle units and the range is 0 to 4 & 6. The
>> default
>>>> +                      value is 0 (i.e.) no delay.
>>>
>>> Most of these properties are pretty similar to the video bus
>>> properties defined at the endpoint level in
>>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
>>> believe it would make sense to use OF graph and try to standardize
>>> these properties similarly.

Other than sync-active, is there really anything else that is similar? And
even the sync-active isn't a good fit since here there is only one sync
signal instead of two for video (h and vsync).

Regards,

	Hans

>> Note that the last two properties match the those in
>> Documentation/devicetree/bindings/spi/sh-msiof.txt.
>> We may want to use one DRIF channel as a plain SPI slave with the
>> (modified) MSIOF driver in the future.
> 
> Should I leave it as it is or modify these as in video-interfaces.txt? Shall we conclude on this please?
> 
> Thanks,
> Ramesh
> N?????r??y???b?X???v?^?)?{.n?+????{???bj)???w*jg?????????j/???z????2?????&?)??a?????G???h??j:+v???w??
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hello Hans,

On Monday 09 Jan 2017 14:36:55 Hans Verkuil wrote:
> On 01/03/2017 04:20 PM, Ramesh Shanmugasundaram wrote:
> >>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
> >>>> Add binding documentation for Renesas R-Car Digital Radio Interface
> >>>> (DRIF) controller.
> >>>> 
> >>>> Signed-off-by: Ramesh Shanmugasundaram
> >>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
> >>>> 
> >>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202 +++++++++++++
> >>>>  1 file changed, 202 insertions(+)
> >>>>  create mode 100644
> >>>> 
> >>>> Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>> 
> >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file
> >>>> mode 100644
> >>>> index 0000000..1f3feaf
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>> 
> >>>> +Optional properties of an internal channel when:
> >>>> +     - It is the only enabled channel of the bond (or)
> >>>> +     - If it acts as primary among enabled bonds
> >>>> +--------------------------------------------------------
> >>>> +- renesas,syncmd       : sync mode
> >>>> +                      0 (Frame start sync pulse mode. 1-bit width
> >>>> pulse
> >>>> +                         indicates start of a frame)
> >>>> +                      1 (L/R sync or I2S mode) (default)
> >>>> +- renesas,lsb-first    : empty property indicates lsb bit is received
> >>>> first.
> >>>> +                      When not defined msb bit is received first
> >>>> +(default)
> >>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for
> >>>> low/high
> 
> Shouldn't this be 'renesas,sync-active' instead of syncac-active?
> 
> I'm not sure if syncac is intended or if it is a typo.
> 
> >>>> +                      respectively. The default is 1 (active high)
> >>>> +- renesas,dtdl         : delay between sync signal and start of
> >>>> reception.
> >>>> +                      The possible values are represented in 0.5 clock
> >>>> +                      cycle units and the range is 0 to 4. The default
> >>>> +                      value is 2 (i.e.) 1 clock cycle delay.
> >>>> +- renesas,syncdl       : delay between end of reception and sync
> >>>> signal edge.
> >>>> +                      The possible values are represented in 0.5 clock
> >>>> +                      cycle units and the range is 0 to 4 & 6. The
> >>>> default
> >>>> +                      value is 0 (i.e.) no delay.
> >>> 
> >>> Most of these properties are pretty similar to the video bus
> >>> properties defined at the endpoint level in
> >>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
> >>> believe it would make sense to use OF graph and try to standardize
> >>> these properties similarly.
> 
> Other than sync-active, is there really anything else that is similar? And
> even the sync-active isn't a good fit since here there is only one sync
> signal instead of two for video (h and vsync).

That's why I said similar, not identical :-) My point is that, if we consider 
that we could connect multiple sources to the DRIF, using OF graph would make 
sense, and the above properties should then be defined per endpoint. If we 
define them per endpoint we should then also try standardize the ones that are 
not really Renesas-specific (that's at least syncac-active). For the syncmd 
and lsb-first properties, it could also make sense to query them from the 
connected subdev at runtime, as they're similar in purpose to formats and 
media bus configuration (struct v4l2_mbus_config).

I'm not an SDR expert, so I'd like to have your opinion on this.

> >> Note that the last two properties match the those in
> >> Documentation/devicetree/bindings/spi/sh-msiof.txt.
> >> We may want to use one DRIF channel as a plain SPI slave with the
> >> (modified) MSIOF driver in the future.
> > 
> > Should I leave it as it is or modify these as in video-interfaces.txt?
> > Shall we conclude on this please?
  

Hi Laurent,

> > >>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
> > >>>> Add binding documentation for Renesas R-Car Digital Radio
> > >>>> Interface
> > >>>> (DRIF) controller.
> > >>>>
> > >>>> Signed-off-by: Ramesh Shanmugasundaram
> > >>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
> > >>>>
> > >>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202
> +++++++++++++
> > >>>>  1 file changed, 202 insertions(+)  create mode 100644
> > >>>>
> > >>>> Documentation/devicetree/bindings/media/renesas,drif.txt
> > >>>>
> > >>>> diff --git
> > >>>> a/Documentation/devicetree/bindings/media/renesas,drif.txt
> > >>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new
> > >>>> file mode 100644 index 0000000..1f3feaf
> > >>>> --- /dev/null
> > >>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> > >>>>
> > >>>> +Optional properties of an internal channel when:
> > >>>> +     - It is the only enabled channel of the bond (or)
> > >>>> +     - If it acts as primary among enabled bonds
> > >>>> +--------------------------------------------------------
> > >>>> +- renesas,syncmd       : sync mode
> > >>>> +                      0 (Frame start sync pulse mode. 1-bit
> > >>>> +width
> > >>>> pulse
> > >>>> +                         indicates start of a frame)
> > >>>> +                      1 (L/R sync or I2S mode) (default)
> > >>>> +- renesas,lsb-first    : empty property indicates lsb bit is
> received
> > >>>> first.
> > >>>> +                      When not defined msb bit is received first
> > >>>> +(default)
> > >>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for
> > >>>> low/high
> >
> > Shouldn't this be 'renesas,sync-active' instead of syncac-active?
> >
> > I'm not sure if syncac is intended or if it is a typo.
> >
> > >>>> +                      respectively. The default is 1 (active high)
> > >>>> +- renesas,dtdl         : delay between sync signal and start of
> > >>>> reception.
> > >>>> +                      The possible values are represented in 0.5
> clock
> > >>>> +                      cycle units and the range is 0 to 4. The
> default
> > >>>> +                      value is 2 (i.e.) 1 clock cycle delay.
> > >>>> +- renesas,syncdl       : delay between end of reception and sync
> > >>>> signal edge.
> > >>>> +                      The possible values are represented in 0.5
> clock
> > >>>> +                      cycle units and the range is 0 to 4 & 6.
> > >>>> + The
> > >>>> default
> > >>>> +                      value is 0 (i.e.) no delay.
> > >>>
> > >>> Most of these properties are pretty similar to the video bus
> > >>> properties defined at the endpoint level in
> > >>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
> > >>> believe it would make sense to use OF graph and try to standardize
> > >>> these properties similarly.
> >
> > Other than sync-active, is there really anything else that is similar?
> > And even the sync-active isn't a good fit since here there is only one
> > sync signal instead of two for video (h and vsync).
> 
> That's why I said similar, not identical :-) My point is that, if we
> consider that we could connect multiple sources to the DRIF, using OF
> graph would make sense, and the above properties should then be defined
> per endpoint.

Thanks for the clarifications. I have some questions.

- Assuming two devices are interfaced with DRIF and they are represented using two endpoints, the control signal related properties of DRIF might still need to be same for both endpoints? For e.g. syncac-active cannot be different in both endpoints?

- I suppose "lsb-first", "dtdl" & "syncdl" may be defined per endpoint. However, h/w manual says same register values needs to be programmed for both the internal channels of a channel. Same with "syncmd" property.

We could still define them as per endpoint property with a note that they need to be same. But I am not sure if that is what you intended?

 If we define them per endpoint we should then also try
> standardize the ones that are not really Renesas-specific (that's at least
> syncac-active).

OK. I will call it "sync-active".

 For the syncmd and lsb-first properties, it could also
> make sense to query them from the connected subdev at runtime, as they're
> similar in purpose to formats and media bus configuration (struct
> v4l2_mbus_config).

May I know in bit more detail about what you had in mind? Please correct me if my understanding is wrong here but when I looked at the code

1) mbus_config is part of subdev_video_ops only. I assume we don't want to support this as part of tuner subdev. The next closest is pad_ops with "struct v4l2_mbus_framefmt" but it is fully video specific config unless I come up with new MEDIA_BUS_FMT_xxxx in media-bus-format.h and use the code field? For e.g.
	
#define MEDIA_BUS_FMT_SDR_I2S_PADHI_BE       0x7001
#define MEDIA_BUS_FMT_SDR_I2S_PADHI_LE       0x7002

2) The framework does not seem to mandate pad ops for all subdev. As the tuner can be any third party subdev, is it fair to assume that these properties can be queried from subdev?

3) Assuming pad ops is not available on the subdev shouldn't we still need a way to define these properties on DRIF DT?

> 
> I'm not an SDR expert, so I'd like to have your opinion on this.
> 
> > >> Note that the last two properties match the those in
> > >> Documentation/devicetree/bindings/spi/sh-msiof.txt.
> > >> We may want to use one DRIF channel as a plain SPI slave with the
> > >> (modified) MSIOF driver in the future.
> > >
> > > Should I leave it as it is or modify these as in video-interfaces.txt?
> > > Shall we conclude on this please?
> 

Thanks,
Ramesh

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hello Hans,

Do you have any further feedback on this?

Thanks, Chris


> From: Ramesh Shanmugasundaram
> Sent: 10 January 2017 09:31
> Hi Laurent,
> 
> > > >>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram
> wrote:
> > > >>>> Add binding documentation for Renesas R-Car Digital Radio
> > > >>>> Interface
> > > >>>> (DRIF) controller.
> > > >>>>
> > > >>>> Signed-off-by: Ramesh Shanmugasundaram
> > > >>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
> > > >>>>
> > > >>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202
> > +++++++++++++
> > > >>>>  1 file changed, 202 insertions(+)  create mode 100644
> > > >>>>
> > > >>>> Documentation/devicetree/bindings/media/renesas,drif.txt
> > > >>>>
> > > >>>> diff --git
> > > >>>> a/Documentation/devicetree/bindings/media/renesas,drif.txt
> > > >>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new
> > > >>>> file mode 100644 index 0000000..1f3feaf
> > > >>>> --- /dev/null
> > > >>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> > > >>>>
> > > >>>> +Optional properties of an internal channel when:
> > > >>>> +     - It is the only enabled channel of the bond (or)
> > > >>>> +     - If it acts as primary among enabled bonds
> > > >>>> +--------------------------------------------------------
> > > >>>> +- renesas,syncmd       : sync mode
> > > >>>> +                      0 (Frame start sync pulse mode. 1-bit
> > > >>>> +width
> > > >>>> pulse
> > > >>>> +                         indicates start of a frame)
> > > >>>> +                      1 (L/R sync or I2S mode) (default)
> > > >>>> +- renesas,lsb-first    : empty property indicates lsb bit is
> > received
> > > >>>> first.
> > > >>>> +                      When not defined msb bit is received
> > > >>>> +first
> > > >>>> +(default)
> > > >>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1
> > > >>>> +for
> > > >>>> low/high
> > >
> > > Shouldn't this be 'renesas,sync-active' instead of syncac-active?
> > >
> > > I'm not sure if syncac is intended or if it is a typo.
> > >
> > > >>>> +                      respectively. The default is 1 (active high)
> > > >>>> +- renesas,dtdl         : delay between sync signal and start of
> > > >>>> reception.
> > > >>>> +                      The possible values are represented in
> > > >>>> + 0.5
> > clock
> > > >>>> +                      cycle units and the range is 0 to 4. The
> > default
> > > >>>> +                      value is 2 (i.e.) 1 clock cycle delay.
> > > >>>> +- renesas,syncdl       : delay between end of reception and sync
> > > >>>> signal edge.
> > > >>>> +                      The possible values are represented in
> > > >>>> + 0.5
> > clock
> > > >>>> +                      cycle units and the range is 0 to 4 & 6.
> > > >>>> + The
> > > >>>> default
> > > >>>> +                      value is 0 (i.e.) no delay.
> > > >>>
> > > >>> Most of these properties are pretty similar to the video bus
> > > >>> properties defined at the endpoint level in
> > > >>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
> > > >>> believe it would make sense to use OF graph and try to
> > > >>> standardize these properties similarly.
> > >
> > > Other than sync-active, is there really anything else that is similar?
> > > And even the sync-active isn't a good fit since here there is only
> > > one sync signal instead of two for video (h and vsync).
> >
> > That's why I said similar, not identical :-) My point is that, if we
> > consider that we could connect multiple sources to the DRIF, using OF
> > graph would make sense, and the above properties should then be
> > defined per endpoint.
> 
> Thanks for the clarifications. I have some questions.
> 
> - Assuming two devices are interfaced with DRIF and they are represented
> using two endpoints, the control signal related properties of DRIF might still
> need to be same for both endpoints? For e.g. syncac-active cannot be
> different in both endpoints?
> 
> - I suppose "lsb-first", "dtdl" & "syncdl" may be defined per endpoint.
> However, h/w manual says same register values needs to be programmed
> for both the internal channels of a channel. Same with "syncmd" property.
> 
> We could still define them as per endpoint property with a note that they
> need to be same. But I am not sure if that is what you intended?
> 
>  If we define them per endpoint we should then also try
> > standardize the ones that are not really Renesas-specific (that's at
> > least syncac-active).
> 
> OK. I will call it "sync-active".
> 
>  For the syncmd and lsb-first properties, it could also
> > make sense to query them from the connected subdev at runtime, as
> > they're similar in purpose to formats and media bus configuration
> > (struct v4l2_mbus_config).
> 
> May I know in bit more detail about what you had in mind? Please correct me
> if my understanding is wrong here but when I looked at the code
> 
> 1) mbus_config is part of subdev_video_ops only. I assume we don't want to
> support this as part of tuner subdev. The next closest is pad_ops with "struct
> v4l2_mbus_framefmt" but it is fully video specific config unless I come up
> with new MEDIA_BUS_FMT_xxxx in media-bus-format.h and use the code
> field? For e.g.
> 
> #define MEDIA_BUS_FMT_SDR_I2S_PADHI_BE       0x7001
> #define MEDIA_BUS_FMT_SDR_I2S_PADHI_LE       0x7002
> 
> 2) The framework does not seem to mandate pad ops for all subdev. As the
> tuner can be any third party subdev, is it fair to assume that these properties
> can be queried from subdev?
> 
> 3) Assuming pad ops is not available on the subdev shouldn't we still need a
> way to define these properties on DRIF DT?
> 
> >
> > I'm not an SDR expert, so I'd like to have your opinion on this.
> >
> > > >> Note that the last two properties match the those in
> > > >> Documentation/devicetree/bindings/spi/sh-msiof.txt.
> > > >> We may want to use one DRIF channel as a plain SPI slave with the
> > > >> (modified) MSIOF driver in the future.
> > > >
> > > > Should I leave it as it is or modify these as in video-interfaces.txt?
> > > > Shall we conclude on this please?
> >
> 
> Thanks,
> Ramesh

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

On 01/10/2017 10:31 AM, Ramesh Shanmugasundaram wrote:
> Hi Laurent,
> 
>>>>>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
>>>>>>> Add binding documentation for Renesas R-Car Digital Radio
>>>>>>> Interface
>>>>>>> (DRIF) controller.
>>>>>>>
>>>>>>> Signed-off-by: Ramesh Shanmugasundaram
>>>>>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
>>>>>>>
>>>>>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202
>> +++++++++++++
>>>>>>>  1 file changed, 202 insertions(+)  create mode 100644
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/media/renesas,drif.txt
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/media/renesas,drif.txt
>>>>>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new
>>>>>>> file mode 100644 index 0000000..1f3feaf
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
>>>>>>>
>>>>>>> +Optional properties of an internal channel when:
>>>>>>> +     - It is the only enabled channel of the bond (or)
>>>>>>> +     - If it acts as primary among enabled bonds
>>>>>>> +--------------------------------------------------------
>>>>>>> +- renesas,syncmd       : sync mode
>>>>>>> +                      0 (Frame start sync pulse mode. 1-bit
>>>>>>> +width
>>>>>>> pulse
>>>>>>> +                         indicates start of a frame)
>>>>>>> +                      1 (L/R sync or I2S mode) (default)
>>>>>>> +- renesas,lsb-first    : empty property indicates lsb bit is
>> received
>>>>>>> first.
>>>>>>> +                      When not defined msb bit is received first
>>>>>>> +(default)
>>>>>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for
>>>>>>> low/high
>>>
>>> Shouldn't this be 'renesas,sync-active' instead of syncac-active?
>>>
>>> I'm not sure if syncac is intended or if it is a typo.
>>>
>>>>>>> +                      respectively. The default is 1 (active high)
>>>>>>> +- renesas,dtdl         : delay between sync signal and start of
>>>>>>> reception.
>>>>>>> +                      The possible values are represented in 0.5
>> clock
>>>>>>> +                      cycle units and the range is 0 to 4. The
>> default
>>>>>>> +                      value is 2 (i.e.) 1 clock cycle delay.
>>>>>>> +- renesas,syncdl       : delay between end of reception and sync
>>>>>>> signal edge.
>>>>>>> +                      The possible values are represented in 0.5
>> clock
>>>>>>> +                      cycle units and the range is 0 to 4 & 6.
>>>>>>> + The
>>>>>>> default
>>>>>>> +                      value is 0 (i.e.) no delay.

Are these properties actually going to be used by anyone? Just curious.

>>>>>>
>>>>>> Most of these properties are pretty similar to the video bus
>>>>>> properties defined at the endpoint level in
>>>>>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
>>>>>> believe it would make sense to use OF graph and try to standardize
>>>>>> these properties similarly.
>>>
>>> Other than sync-active, is there really anything else that is similar?
>>> And even the sync-active isn't a good fit since here there is only one
>>> sync signal instead of two for video (h and vsync).
>>
>> That's why I said similar, not identical :-) My point is that, if we
>> consider that we could connect multiple sources to the DRIF, using OF
>> graph would make sense, and the above properties should then be defined
>> per endpoint.
> 
> Thanks for the clarifications. I have some questions.
> 
> - Assuming two devices are interfaced with DRIF and they are represented using two endpoints, the control signal related properties of DRIF might still need to be same for both endpoints? For e.g. syncac-active cannot be different in both endpoints?

Usually that's the case, but HW designers are weird :-)

> 
> - I suppose "lsb-first", "dtdl" & "syncdl" may be defined per endpoint. However, h/w manual says same register values needs to be programmed for both the internal channels of a channel. Same with "syncmd" property.
> 
> We could still define them as per endpoint property with a note that they need to be same. But I am not sure if that is what you intended?
> 
>  If we define them per endpoint we should then also try
>> standardize the ones that are not really Renesas-specific (that's at least
>> syncac-active).
> 
> OK. I will call it "sync-active".

That's better, yes.

> 
>  For the syncmd and lsb-first properties, it could also
>> make sense to query them from the connected subdev at runtime, as they're
>> similar in purpose to formats and media bus configuration (struct
>> v4l2_mbus_config).

I consider this unlikely. I wouldn't spend time on that as this can always be
done later.

> May I know in bit more detail about what you had in mind? Please correct me if my understanding is wrong here but when I looked at the code
> 
> 1) mbus_config is part of subdev_video_ops only. I assume we don't want to support this as part of tuner subdev. The next closest is pad_ops with "struct v4l2_mbus_framefmt" but it is fully video specific config unless I come up with new MEDIA_BUS_FMT_xxxx in media-bus-format.h and use the code field? For e.g.
> 	
> #define MEDIA_BUS_FMT_SDR_I2S_PADHI_BE       0x7001
> #define MEDIA_BUS_FMT_SDR_I2S_PADHI_LE       0x7002
> 
> 2) The framework does not seem to mandate pad ops for all subdev. As the tuner can be any third party subdev, is it fair to assume that these properties can be queried from subdev?
> 
> 3) Assuming pad ops is not available on the subdev shouldn't we still need a way to define these properties on DRIF DT?
> 
>>
>> I'm not an SDR expert, so I'd like to have your opinion on this.

Neither am I :-)

I think using the endpoint idea does make sense since this helps in describing the
routing. I am not sure what properties to put in there, though.

Can you describe a bit more which properties belong to which syncmd mode? That will
help me make a decision.

Regards,

	Hans

>>
>>>>> Note that the last two properties match the those in
>>>>> Documentation/devicetree/bindings/spi/sh-msiof.txt.
>>>>> We may want to use one DRIF channel as a plain SPI slave with the
>>>>> (modified) MSIOF driver in the future.
>>>>
>>>> Should I leave it as it is or modify these as in video-interfaces.txt?
>>>> Shall we conclude on this please?
>>
> 
> Thanks,
> Ramesh
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Hans,

Many thanks for the response & comments.

> Subject: Re: [PATCH v2 6/7] dt-bindings: media: Add Renesas R-Car DRIF
> binding
> 
> On 01/10/2017 10:31 AM, Ramesh Shanmugasundaram wrote:
> > Hi Laurent,
> >
> >>>>>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
> >>>>>>> Add binding documentation for Renesas R-Car Digital Radio
> >>>>>>> Interface
> >>>>>>> (DRIF) controller.
> >>>>>>>
> >>>>>>> Signed-off-by: Ramesh Shanmugasundaram
> >>>>>>> <ramesh.shanmugasundaram@bp.renesas.com> ---
> >>>>>>>
> >>>>>>>  .../devicetree/bindings/media/renesas,drif.txt     | 202
> >> +++++++++++++
> >>>>>>>  1 file changed, 202 insertions(+)  create mode 100644
> >>>>>>>
> >>>>>>> Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>>>>>
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>>>>> b/Documentation/devicetree/bindings/media/renesas,drif.txt new
> >>>>>>> file mode 100644 index 0000000..1f3feaf
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> >>>>>>>
> >>>>>>> +Optional properties of an internal channel when:
> >>>>>>> +     - It is the only enabled channel of the bond (or)
> >>>>>>> +     - If it acts as primary among enabled bonds
> >>>>>>> +--------------------------------------------------------
> >>>>>>> +- renesas,syncmd       : sync mode
> >>>>>>> +                      0 (Frame start sync pulse mode. 1-bit
> >>>>>>> +width
> >>>>>>> pulse
> >>>>>>> +                         indicates start of a frame)
> >>>>>>> +                      1 (L/R sync or I2S mode) (default)
> >>>>>>> +- renesas,lsb-first    : empty property indicates lsb bit is
> >> received
> >>>>>>> first.
> >>>>>>> +                      When not defined msb bit is received
> >>>>>>> +first
> >>>>>>> +(default)
> >>>>>>> +- renesas,syncac-active: Indicates sync signal polarity, 0/1
> >>>>>>> +for
> >>>>>>> low/high
> >>>
> >>> Shouldn't this be 'renesas,sync-active' instead of syncac-active?
> >>>
> >>> I'm not sure if syncac is intended or if it is a typo.
> >>>
> >>>>>>> +                      respectively. The default is 1 (active
> high)
> >>>>>>> +- renesas,dtdl         : delay between sync signal and start of
> >>>>>>> reception.
> >>>>>>> +                      The possible values are represented in
> >>>>>>> + 0.5
> >> clock
> >>>>>>> +                      cycle units and the range is 0 to 4. The
> >> default
> >>>>>>> +                      value is 2 (i.e.) 1 clock cycle delay.
> >>>>>>> +- renesas,syncdl       : delay between end of reception and sync
> >>>>>>> signal edge.
> >>>>>>> +                      The possible values are represented in
> >>>>>>> + 0.5
> >> clock
> >>>>>>> +                      cycle units and the range is 0 to 4 & 6.
> >>>>>>> + The
> >>>>>>> default
> >>>>>>> +                      value is 0 (i.e.) no delay.
> 
> Are these properties actually going to be used by anyone? Just curious.

Yes. Each of this property should be set appropriately based on the master device it interfaces with. 

> 
> >>>>>>
> >>>>>> Most of these properties are pretty similar to the video bus
> >>>>>> properties defined at the endpoint level in
> >>>>>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
> >>>>>> believe it would make sense to use OF graph and try to
> >>>>>> standardize these properties similarly.
> >>>
> >>> Other than sync-active, is there really anything else that is similar?
> >>> And even the sync-active isn't a good fit since here there is only
> >>> one sync signal instead of two for video (h and vsync).
> >>
> >> That's why I said similar, not identical :-) My point is that, if we
> >> consider that we could connect multiple sources to the DRIF, using OF
> >> graph would make sense, and the above properties should then be
> >> defined per endpoint.
> >
> > Thanks for the clarifications. I have some questions.
> >
> > - Assuming two devices are interfaced with DRIF and they are represented
> using two endpoints, the control signal related properties of DRIF might
> still need to be same for both endpoints? For e.g. syncac-active cannot be
> different in both endpoints?
> 
> Usually that's the case, but HW designers are weird :-)
> 
> >
> > - I suppose "lsb-first", "dtdl" & "syncdl" may be defined per endpoint.
> However, h/w manual says same register values needs to be programmed for
> both the internal channels of a channel. Same with "syncmd" property.
> >
> > We could still define them as per endpoint property with a note that
> they need to be same. But I am not sure if that is what you intended?
> >
> >  If we define them per endpoint we should then also try
> >> standardize the ones that are not really Renesas-specific (that's at
> >> least syncac-active).
> >
> > OK. I will call it "sync-active".
> 
> That's better, yes.

Thanks.

> 
> >
> >  For the syncmd and lsb-first properties, it could also
> >> make sense to query them from the connected subdev at runtime, as
> >> they're similar in purpose to formats and media bus configuration
> >> (struct v4l2_mbus_config).
> 
> I consider this unlikely. I wouldn't spend time on that as this can always
> be done later.
> 
> > May I know in bit more detail about what you had in mind? Please
> > correct me if my understanding is wrong here but when I looked at the
> > code
> >
> > 1) mbus_config is part of subdev_video_ops only. I assume we don't want
> to support this as part of tuner subdev. The next closest is pad_ops with
> "struct v4l2_mbus_framefmt" but it is fully video specific config unless I
> come up with new MEDIA_BUS_FMT_xxxx in media-bus-format.h and use the code
> field? For e.g.
> >
> > #define MEDIA_BUS_FMT_SDR_I2S_PADHI_BE       0x7001
> > #define MEDIA_BUS_FMT_SDR_I2S_PADHI_LE       0x7002
> >
> > 2) The framework does not seem to mandate pad ops for all subdev. As the
> tuner can be any third party subdev, is it fair to assume that these
> properties can be queried from subdev?
> >
> > 3) Assuming pad ops is not available on the subdev shouldn't we still
> need a way to define these properties on DRIF DT?
> >
> >>
> >> I'm not an SDR expert, so I'd like to have your opinion on this.
> 
> Neither am I :-)
> 
> I think using the endpoint idea does make sense since this helps in
> describing the routing. 

OK.

I am not sure what properties to put in there,
> though.
> 
> Can you describe a bit more which properties belong to which syncmd mode?
> That will help me make a decision.

OK. The documentation for this control is not accurate in many places of the h/w manual. My observations are based on tests with MAX2175 tuner master device using I2S mode.

Applicable parameters for both Frame Start Sync pulse & I2S modes
 - sync-active
 - lsb/msb first
 - dtdl
 
For I2S mode, syncdl property should be set to value zero only. 

Below are timing diagram examples.

Frame start sync pulse mode:
-----------------------------

   <------------ 1 frame ------------------>
   .---.   .---.   .---.   .---.   .---.   .---.   .---.
___|   |___|   |___|   |___|   |___|   |___|   |___|   |___    SCK

   .-------.                               .-------.
___|       |_______________________________|       |_______    SYNC/SS

   |                                       |
   |-No data delay          No sync delay  |
__  ______  ______  ______  ______  ______  ______  ______
  \/      \/      \/      \/      \/      \/      \/           Rx Data
__/\______/\______/\______/\______/\______/\______/\______

   |-1 bit delay            No sync delay  |
__________  ______  ______  ______  ______  ______  ______
          \/      \/      \/      \/      \/      \/           Rx Data
__________/\______/\______/\______/\______/\______/\______

1 bit delay => DTDL = 1 (i.e.) 1 clock cycle delay
No Sync delay => SYNCDL = 0

I2S mode:
----------

   <------------ 1 frame -------------------------->
   .---.   .---.   .---.   .---.   .---.   .---.   .---.
___|   |___|   |___|   |___|   |___|   |___|   |___|   |___    SCK


___                         _______________________
   |                       |                       |          SYNC/SS
   +-----------------------+                       +-------


   |                                               |
   |-No data delay                   No sync delay |
__  ______  ______  ______  ______  ______  ______  ______
  \/      \/      \/      \/      \/      \/      \/           Rx Data
__/\______/\______/\______/\______/\______/\______/\______

   |-1 bit delay                   No sync delay  |
__________  ______  ______  ______  ______  ______  ______
          \/      \/      \/      \/      \/      \/           Rx Data
__________/\______/\______/\______/\______/\______/\______

Please share your thoughts.

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html