Re: [PATCH v2 1/2] media: dt-bindings: media: i2c: Add MT9M114 camera sensor binding

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Tue, Feb 21, 2023 at 02:42:31PM +0200, Sakari Ailus wrote:
> On Tue, Feb 21, 2023 at 01:52:46AM +0200, Laurent Pinchart wrote:
> > > > > > > +        properties:
> > > > > > > +          bus-type:
> > > > > > > +            enum: [4, 5, 6]
> > > > > > 
> > > > > > With bus-type 5, shouldn't you have the parallel interface sync signal
> > > > > > polarity properties? Possibly also others if the hardware supports them.
> > > > > 
> > > > > As far as I can tell, the hardware has fixed polarities for all signals.
> > > > > Both hsync and vsync (called LINE_VALID and FRAME_VALID here) are active
> > > > > high.
> > > > 
> > > > Right, then you won't need these.
> > > > 
> > > > What about the link-frequencies property?
> > > 
> > > That's something I've meant to ask, should the link-frequencies property
> > > be specified for parallel buses too, or only CSI-2 ?
> > 
> > Any feedback on this ?
> 
> Yes.
> 
> It very likely matters there as well but probably less so than on CSI-2.
> 
> link-frequencies isn't specific to CSI-2 either.
> 
> > > We also need to document how a DT writer should select the link
> > > frequencies. The process is complicated, saying that the property is
> > > required without explaining how the values can be computed (EMC
> > > constraints aside, just from a functional point of view) isn't nice. If
> > > you explain this to me, I can put it in a patch :-)
> > 
> > Or on this ? :-)
> 
> Generally the hardware people will tell you this when it matters. :-) But
> on a development board you can probably have anything the hardware
> supports.
> 
> I guess we could add a few words on this to e.g.
> Documentation/driver-api/media/camera-sensor.rst .

Please :-) When the sensor driver supports multiple link frequencies,
and in particular a continuous range of frequencies, I want a document I
can point people to precisely answer the "I have no idea about those
stuff, what value do I put in my device tree?" question. It should
include a clear procedure to calculate a value, not just a generic
statement along the lines of "pick a value that is suitable for you".

> Most drivers will support a few frequencies only and sometimes the vendor,
> due to someone needing it, provides another configuration with a different
> frequency. Many sensors also have datasheets that are precise enough to
> calculate this yourself.

-- 
Regards,

Laurent Pinchart