Re: [PATCH 4/5] v4l2-fwnode: Rework v4l2_fwnode_endpoint_parse documentation

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Sun, Oct 04, 2020 at 11:40:13AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Wed, Sep 30, 2020 at 05:48:10PM +0300, Sakari Ailus wrote:
> > Rework the documentation of v4l2_fwnode_endpoint_parse for better
> > readability, usefulness and correctness.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  include/media/v4l2-fwnode.h | 62 ++++++++++++++++++++++++-------------
> >  1 file changed, 40 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> > index 0f9a768b1a8d..0c28dc11e829 100644
> > --- a/include/media/v4l2-fwnode.h
> > +++ b/include/media/v4l2-fwnode.h
> > @@ -219,17 +219,26 @@ struct v4l2_fwnode_connector {
> >   * @vep: pointer to the V4L2 fwnode data structure
> >   *
> >   * This function parses the V4L2 fwnode endpoint specific parameters from the
> > - * firmware. The caller is responsible for assigning @vep.bus_type to a valid
> > - * media bus type. The caller may also set the default configuration for the
> > - * endpoint --- a configuration that shall be in line with the DT binding
> > - * documentation. Should a device support multiple bus types, the caller may
> > - * call this function once the correct type is found --- with a default
> > - * configuration valid for that type.
> > - *
> > - * It is also allowed to set @vep.bus_type to V4L2_MBUS_UNKNOWN. USING THIS
> > - * FEATURE REQUIRES "bus-type" PROPERTY IN DT BINDINGS. For old drivers,
> > - * guessing @vep.bus_type between CSI-2 D-PHY, parallel and BT.656 busses is
> > - * supported. NEVER RELY ON GUESSING @vep.bus_type IN NEW DRIVERS!
> > + * firmware. There are two ways to use this function, either by letting it
> > + * obtain the type of the bus (by setting the @vep.bus_type field to
> > + * V4L2_MBUS_UNKNOWN) or specifying the bus type explicitly to one of the &enum
> > + * v4l2_mbus_type types.
> > + *
> > + * When @vep.bus_type is V4L2_MBUS_UNKNOWN, the function will use the "bus-type"
> > + * property to determine the type when it is available. The caller is
> > + * responsible for validating the contents of @vep.bus_type field after the call
> > + * returns.
> 
> As an additional improvement, I wonder if it would make sense to turn
> the bus types into a bit flag, to let the caller tell which bus types it
> supports. Or maybe that would be overkill ?
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the review!

I wonder how useful that would be as drivers typically do bus type specific
checks right after this function returns. Although I wouldn't object a
patch doing that if it's seen useful somewhere.

-- 
Sakari Ailus