Re: [PATCH v4 1/3] dt-bindings: media: Add bindings for THine THP7312 ISP

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

On Fri, Oct 27, 2023 at 04:27:21PM +0000, Sakari Ailus wrote:
> On Fri, Oct 27, 2023 at 06:36:49PM +0300, Laurent Pinchart wrote:
> > On Fri, Oct 27, 2023 at 02:52:38PM +0000, Sakari Ailus wrote:
> > > On Fri, Oct 27, 2023 at 03:46:34PM +0300, Laurent Pinchart wrote:
> > > > > > > > +  port:
> > > > > > > > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > > > > > > > +    additionalProperties: false
> > > > > > > > +
> > > > > > > > +    properties:
> > > > > > > > +      endpoint:
> > > > > > > > +        $ref: /schemas/media/video-interfaces.yaml#
> > > > > > > > +        unevaluatedProperties: false
> > > > > > > > +
> > > > > > > > +        properties:
> > > > > > > > +          data-lanes:
> > > > > > > > +            description:
> > > > > > > > +              This property is for lane reordering between the THP7312 and the
> > > > > > > > +              SoC. The sensor supports either two-lane, or four-lane operation.
> > > > > > > > +              If this property is omitted four-lane operation is assumed. For
> > > > > > > > +              two-lane operation the property must be set to <1 2>.
> > > > > > > 
> > > > > > > How are link frequencies determined by the device?
> > > > > > 
> > > > > > The link frequency is not under direct control of the host. There are
> > > > > > registers to select between mode presets, each of them producing a
> > > > > > particular link frequency.
> > > > > 
> > > > > If they are known, one could only allow setting the modes with known-good
> > > > > link requencies. This could be added later on, too, affecting systems with
> > > > > link-frequencies set. I guess you could also add it now, as optional.
> > > > 
> > > > I vote for later :-)
> > > > 
> > > > > Where's the link frequency information available? In documentation or in
> > > > > on the firmware API?
> > > > 
> > > > In the documentation (which is not public I'm afraid).
> > > 
> > > Could you add this information in the driver?
> > > 
> > > Co-incidentally, the driver is lacking the LINK_FREQ control. It should be
> > > added.
> > 
> > Oops. Seems I got caught red-handed. I'll fix that (and add the
> > PIXEL_RATE control too).
> 
> :-)
> 
> The PIXEL_RATE is intended to be the pixel rate on the pixel array. The
> driver doesn't have HBLANK or VBLANK so is it even useful to have
> PIXEL_RATE?

I don't mind leaving it out, it's easier.

-- 
Regards,

Laurent Pinchart