Hi Geert, Thank you for the review comments. > On Wed, Oct 12, 2016 at 4:10 PM, Ramesh Shanmugasundaram > <ramesh.shanmugasundaram@xxxxxxxxxxxxxx> wrote: > > This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3 > SoCs. > > The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF > > device represents a channel and each channel can have one or two > > sub-channels respectively depending on the target board. > > > > DRIF supports only Rx functionality. It receives samples from a RF > > frontend tuner chip it is interfaced with. The combination of DRIF and > > the tuner device, which is registered as a sub-device, determines the > > receive sample rate and format. > > > > In order to be compliant as a V4L2 SDR device, DRIF needs to bind with > > the tuner device, which can be provided by a third party vendor. DRIF > > acts as a slave device and the tuner device acts as a master > > transmitting the samples. The driver allows asynchronous binding of a > > tuner device that is registered as a v4l2 sub-device. The driver can > > learn about the tuner it is interfaced with based on port endpoint > > properties of the device in device tree. The V4L2 SDR device inherits > > the controls exposed by the tuner device. > > > > The device can also be configured to use either one or both of the > > data pins at runtime based on the master (tuner) configuration. > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt > > @@ -0,0 +1,109 @@ > > +Renesas R-Car Gen3 DRIF controller (DRIF) > > +----------------------------------------- > > + > > +Required properties: > > +-------------------- > > +- compatible: "renesas,drif-r8a7795" if DRIF controller is a part of > R8A7795 SoC. > > "renesas,r8a7795-drif", as Rob already pointed out. Agreed. > > > + "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 each sub-channel. > > +- interrupts: associated with each sub-channel. > > +- clocks: phandles and clock specifiers for each sub-channel. > > +- clock-names: clock input name strings: "fck0", "fck1". > > +- pinctrl-0: pin control group to be used for this controller. > > +- pinctrl-names: must be "default". > > +- dmas: phandles to the DMA channels for each sub-channel. > > +- dma-names: names for the DMA channels: "rx0", "rx1". > > + > > +Required child nodes: > > +--------------------- > > +- Each DRIF channel can have one or both of the sub-channels enabled > > +in a > > + setup. The sub-channels are represented as a child node. The name > > +of the > > + child nodes are "sub-channel0" and "sub-channel1" respectively. > > +Each child > > + node supports the "status" property only, which is used to > > +enable/disable > > + the respective sub-channel. > > > +Example > > +-------- > > + > > +SoC common dtsi file > > + > > +drif0: rif@e6f40000 { > > + compatible = "renesas,drif-r8a7795", > > + "renesas,rcar-gen3-drif"; > > + reg = <0 0xe6f40000 0 0x64>, <0 0xe6f50000 0 0x64>; > > + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 515>, <&cpg CPG_MOD 514>; > > + clock-names = "fck0", "fck1"; > > + dmas = <&dmac1 0x20>, <&dmac1 0x22>; > > + dma-names = "rx0", "rx1"; > > I could not find the DMAC channels in the datasheet? It is mentioned only in v0.5 h/w manual. v0.52 manual introduced DRIF chapter but then some of the old references were missing :-(. There are few more doc anomalies, which I shall document in the next version of the patch. > Most modules are either tied to dmac0, or two both dmac1 and dmac2. > In the latter case, you want to list two sets of dmas, one for each DMAC. You are right. I have added both dmac1 & 2 now. > > > + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; > > + status = "disabled"; > > + > > + sub-channel0 { > > + status = "disabled"; > > + }; > > + > > + sub-channel1 { > > + status = "disabled"; > > + }; > > + > > +}; > > As you're modelling this in DT under a single device node, this means you > cannot use runtime PM to manage the module clocks of the individual > channels. > > An alternative could be to have two separate nodes for each channel, and > tie them together using a phandle. I agree & thanks for the suggestion. Is the below model looks anything closer? Appreciate your inputs. dtsi ------- 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"; power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; 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"; power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; status = "disabled"; }; drif0: rif@0 { compatible = "renesas,r8a7795-drif", "renesas,rcar-gen3-drif"; sub-channels = <&drif00>, <&drif01>; status = "disabled"; }; ------- board dts file would have something like this &drif00 { status = "okay"; }; &drif01 { status = "okay"; }; &drif0 { pinctrl-0 = <&drif0_pins>; pinctrl-names = "default"; status = "okay"; port { drif0_ep: endpoint { remote-endpoint = <&max2175_0_ep>; }; }; }; ------- The drif0X driver instance will help only in registering with genpd. The parent drif0 instance will parse "sub-channels" phandles and use the resources of respective enabled sub-channels using it's pdev. Thanks, Ramesh ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f