Hi Laurent, Thanks for the review comments.
> > On Tuesday 07 Feb 2017 15:02:32 Ramesh Shanmugasundaram wrote: > > Add device tree binding documentation for MAX2175 Rf to bits tuner > > device. > > > > Signed-off-by: Ramesh Shanmugasundaram > > <ramesh.shanmugasunda...@bp.renesas.com> --- > > .../devicetree/bindings/media/i2c/max2175.txt | 61 > +++++++++++++++++++ > > .../devicetree/bindings/property-units.txt | 1 + > > 2 files changed, 62 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/i2c/max2175.txt > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt > > b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file > > mode > > 100644 > > index 0000000..f591ab4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt > > @@ -0,0 +1,61 @@ > > +Maxim Integrated MAX2175 RF to Bits tuner > > +----------------------------------------- > > + > > +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver > > +with RF to Bits(r) front-end designed for software-defined radio > solutions. > > + > > +Required properties: > > +-------------------- > > +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner. > > +- clocks: phandle to the fixed xtal clock. > > +- clock-names: name of the fixed xtal clock. > > I would mention that the name has to be "xtal". Maybe something like > > - clock-names: name of the fixed xtal clock, shall be "xtal". Agreed. > > > +- port: child port node of a tuner that defines the local and remote > > + endpoints. The remote endpoint is assumed to be an SDR device > > + that is capable of receiving the digital samples from the tuner. > > You should refer to the OF graphs bindings here. How about the following > to document the port node ? > > - port: child port node corresponding to the I2S output, in accordance > with the video interface bindings defined in > Documentation/devicetree/bindings/media/video-interfaces.txt. The port > node must contain at least one endpoint. Agreed. > > > +Optional properties: > > +-------------------- > > +- maxim,slave : phandle to the master tuner if it is a slave. > This > > + is used to define two tuners in diversity mode > > + (1 master, 1 slave). By default each tuner is an > > + individual master. > > It seems weird to me to name a property "slave" when it points to the > master tuner. Shouldn't it be named "maxim,master" ? Agreed. > > > +- maxim,refout-load-pF: load capacitance value (in pF) on reference > > + output drive level. The possible load values are > > + 0 (default - refout disabled) > > + 10 > > + 20 > > + 30 > > + 40 > > + 60 > > + 70 > > +- maxim,am-hiz : empty property indicates AM Hi-Z filter path > is > > + selected for AM antenna input. By default this > > + filter path is not used. > > Isn't this something that should be selected at runtime through a control > ? Or does the hardware design dictate whether the filter has to be used or > must not be used ? This is dictated by the h/w design and not selectable at run-time. I will update these changes in the next patchset. Thanks, Ramesh
