RE: [PATCH v3 2/7] dt-bindings: media: Add MAX2175 binding description

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.shanmugasundaram@xxxxxxxxxxxxxx> ---
> >  .../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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux