Re: [PATCH v7 3/5] media: Documentation: Update link frequency driver documentation

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

On Fri, Dec 13, 2024 at 07:15:55AM +0000, Sakari Ailus wrote:
> On Thu, Dec 12, 2024 at 05:53:53PM +0100, Jacopo Mondi wrote:
> > On Tue, Dec 10, 2024 at 09:59:04AM +0200, Sakari Ailus wrote:
> > > Add the get_mbus_config() as the means for conveying the link frequency
> > > towards the receiver drivers.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  Documentation/driver-api/media/tx-rx.rst | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
> > > index dd09484df1d3..1430c330fd52 100644
> > > --- a/Documentation/driver-api/media/tx-rx.rst
> > > +++ b/Documentation/driver-api/media/tx-rx.rst
> > > @@ -49,6 +49,10 @@ Link frequency
> > >  The :ref:`V4L2_CID_LINK_FREQ <v4l2-cid-link-freq>` control is used to tell the
> > >  receiver the frequency of the bus (i.e. it is not the same as the symbol rate).
> > >
> > > +On devices where the link frequency isn't configurable, the link_freq field of
> > > +struct v4l2_mbus_config is recommended over controls for conveying the link
> > > +frequency to the downstream driver in the pipeline.
> > 
> > When read in its entirety, the paragraphs
> > 
> >  Link frequency
> >  ^^^^^^^^^^^^^^
> > 
> >  The :ref:`V4L2_CID_LINK_FREQ <v4l2-cid-link-freq>` control is used to tell the
> >  receiver the frequency of the bus (i.e. it is not the same as the symbol rate).
> > 
> >  On devices where the link frequency isn't configurable, the link_freq field of
> >  struct v4l2_mbus_config is recommended over controls for conveying the link
> >  frequency.
> > 
> > Sounds simpler without the last 7 words.
> > 
> > A nit and just tastes maybe
> 
> I used:
> 
> On devices where the link frequency isn't configurable, using the ``link_freq``
> field of struct v4l2_mbus_config is recommended over controls.

Is it a recommendation for the transmitter driver or the receiver driver
? If it's a recommendation for the transmitter driver, does that mean it
should not implement V4L2_CID_LINK_FREQ in that case ? How about the
need to expose the link frequency to userspace when it's fixed ?

I think the documentation needs further clarification to clearly explain
what transmitters should do and what receivers should do.

> > 
> > I like where this is going!
> > 
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

-- 
Regards,

Laurent Pinchart