diff mbox series

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

Message ID 20241210075906.609490-4-sakari.ailus@linux.intel.com (mailing list archive)
State New
Headers show
Series Use V4L2 mbus config for conveying MEI CSI link frequency | expand

Commit Message

Sakari Ailus Dec. 10, 2024, 7:59 a.m. UTC
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(+)

Comments

Jacopo Mondi Dec. 12, 2024, 4:53 p.m. UTC | #1
Hi Sakari

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 like where this is going!

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>


> +
>  ``.enable_streams()`` and ``.disable_streams()`` callbacks
>  ^^^^^^^^^^^^^^^^^^^^^^^^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> --
> 2.39.5
>
>
Sakari Ailus Dec. 13, 2024, 7:15 a.m. UTC | #2
Hi Jacopo,

On Thu, Dec 12, 2024 at 05:53:53PM +0100, Jacopo Mondi wrote:
> Hi Sakari
> 
> 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.

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

Thank you!
Laurent Pinchart Dec. 15, 2024, 5:02 p.m. UTC | #3
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>
Sakari Ailus Dec. 16, 2024, 8:07 a.m. UTC | #4
Hi Laurent,

On Sun, Dec 15, 2024 at 07:02:40PM +0200, Laurent Pinchart wrote:
> 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 ?

For the transmitter. Receivers do not implement get_mbus_config op. I can
add a note on this.

The only reason this has been exposed to the user space is because it's
been a control that has been modifiable. Also HDMI to CSI-2 bridges work
this way.

The LINK_FREQ control is also an integer menu so the same control couldn't
be used as-is. These were the reasons why the earlier review concluded with
impelmenting this via get_mbus_config().

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

I can add this.

> 
> > > 
> > > I like where this is going!
> > > 
> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
Sakari Ailus Dec. 16, 2024, 8:08 a.m. UTC | #5
On Mon, Dec 16, 2024 at 08:07:26AM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> On Sun, Dec 15, 2024 at 07:02:40PM +0200, Laurent Pinchart wrote:
> > 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 ?
> 
> For the transmitter. Receivers do not implement get_mbus_config op. I can
> add a note on this.

This text is already under "Transmitter drivers".
Laurent Pinchart Dec. 16, 2024, 11:20 a.m. UTC | #6
On Mon, Dec 16, 2024 at 08:07:26AM +0000, Sakari Ailus wrote:
> On Sun, Dec 15, 2024 at 07:02:40PM +0200, Laurent Pinchart wrote:
> > 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 ?
> 
> For the transmitter. Receivers do not implement get_mbus_config op. I can
> add a note on this.

They don't implement it, but they call it. I wasn't sure if this
documentation was from the point of view of the caller or callee. As you
mentioned separately that it's located in the transmitter section, that
makes it clearer, but we can also improve the test:

Drivers that have a fixed link frequency should report it through the
``.get_mbus_config()`` subdev operation, in the``link_freq`` field of
struct v4l2_mbus_config, instead of through controls.

(I'll let you fix the markup for the mention of the .get_mbus_config()
operation.)

> The only reason this has been exposed to the user space is because it's
> been a control that has been modifiable. Also HDMI to CSI-2 bridges work
> this way.
> 
> The LINK_FREQ control is also an integer menu so the same control couldn't
> be used as-is. These were the reasons why the earlier review concluded with
> impelmenting this via get_mbus_config().
> 
> > I think the documentation needs further clarification to clearly explain
> > what transmitters should do and what receivers should do.
> 
> I can add this.
> 
> > > > 
> > > > I like where this is going!
> > > > 
> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Sakari Ailus Dec. 16, 2024, 12:05 p.m. UTC | #7
Hi Laurent,

On Mon, Dec 16, 2024 at 01:20:26PM +0200, Laurent Pinchart wrote:
> On Mon, Dec 16, 2024 at 08:07:26AM +0000, Sakari Ailus wrote:
> > On Sun, Dec 15, 2024 at 07:02:40PM +0200, Laurent Pinchart wrote:
> > > 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 ?
> > 
> > For the transmitter. Receivers do not implement get_mbus_config op. I can
> > add a note on this.
> 
> They don't implement it, but they call it. I wasn't sure if this
> documentation was from the point of view of the caller or callee. As you
> mentioned separately that it's located in the transmitter section, that
> makes it clearer, but we can also improve the test:
> 
> Drivers that have a fixed link frequency should report it through the
> ``.get_mbus_config()`` subdev operation, in the``link_freq`` field of
> struct v4l2_mbus_config, instead of through controls.

This isn't correct: the link frequency isn't fixed, it's just not
user-configurable. How about:

Drivers that do not have user-configurable link frequency should report it
through the ``.get_mbus_config()`` subdev pad operation, in the ``link_freq``
field of struct v4l2_mbus_config, instead of through controls.

The ReST seems to be in line with the rest.
Laurent Pinchart Dec. 16, 2024, 1:51 p.m. UTC | #8
On Mon, Dec 16, 2024 at 12:05:19PM +0000, Sakari Ailus wrote:
> On Mon, Dec 16, 2024 at 01:20:26PM +0200, Laurent Pinchart wrote:
> > On Mon, Dec 16, 2024 at 08:07:26AM +0000, Sakari Ailus wrote:
> > > On Sun, Dec 15, 2024 at 07:02:40PM +0200, Laurent Pinchart wrote:
> > > > 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 ?
> > > 
> > > For the transmitter. Receivers do not implement get_mbus_config op. I can
> > > add a note on this.
> > 
> > They don't implement it, but they call it. I wasn't sure if this
> > documentation was from the point of view of the caller or callee. As you
> > mentioned separately that it's located in the transmitter section, that
> > makes it clearer, but we can also improve the test:
> > 
> > Drivers that have a fixed link frequency should report it through the
> > ``.get_mbus_config()`` subdev operation, in the``link_freq`` field of
> > struct v4l2_mbus_config, instead of through controls.
> 
> This isn't correct: the link frequency isn't fixed, it's just not
> user-configurable. How about:
> 
> Drivers that do not have user-configurable link frequency should report it
> through the ``.get_mbus_config()`` subdev pad operation, in the ``link_freq``
> field of struct v4l2_mbus_config, instead of through controls.

I'm OK with that.

> The ReST seems to be in line with the rest.

:-)
diff mbox series

Patch

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.
+
 ``.enable_streams()`` and ``.disable_streams()`` callbacks
 ^^^^^^^^^^^^^^^^^^^^^^^^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~