lkml.org 
[lkml]   [2020]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 3/3] media: docs: v4l2-subdev: move generic paragraph to the introduction
Date
Hi Jacopo,

thanks for reviewing.

On 15/09/20 15:34, Jacopo Mondi wrote:
> Hi Luca,
>
> On Fri, Sep 04, 2020 at 11:51:41PM +0200, Luca Ceresoli wrote:
>> This paragraph provides generic information to explain what v4l2_subdev is
>> useful for. Placing it in the middle of paragraphs describing the details
>> of subdev registration does not make much sense. Move it near the beginning
>> of the section when the v4l2_subdev idea has just been introduced and
>> before going into its details.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>> Documentation/driver-api/media/v4l2-subdev.rst | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst
>> index fb66163deb38..1c1e3f9da142 100644
>> --- a/Documentation/driver-api/media/v4l2-subdev.rst
>> +++ b/Documentation/driver-api/media/v4l2-subdev.rst
>> @@ -12,6 +12,12 @@ Usually these are I2C devices, but not necessarily. In order to provide the
>> driver with a consistent interface to these sub-devices the
>> :c:type:`v4l2_subdev` struct (v4l2-subdev.h) was created.
>>
>> +The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and
>> +does not contain any knowledge about the underlying hardware. So a driver might
>> +contain several subdevs that use an I2C bus, but also a subdev that is
>> +controlled through GPIO pins. This distinction is only relevant when setting
>> +up the device, but once the subdev is registered it is completely transparent.
>> +
>> Each sub-device driver must have a :c:type:`v4l2_subdev` struct. This struct
>> can be stand-alone for simple sub-devices or it might be embedded in a larger
>> struct if more state information needs to be stored. Usually there is a
>> @@ -235,12 +241,6 @@ it can call ``v4l2_subdev_notify(sd, notification, arg)``. This macro checks
>> whether there is a ``notify()`` callback defined and returns ``-ENODEV`` if not.
>> Otherwise the result of the ``notify()`` call is returned.
>>
>> -The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and
>> -does not contain any knowledge about the underlying hardware. So a driver might
>> -contain several subdevs that use an I2C bus, but also a subdev that is
>> -controlled through GPIO pins. This distinction is only relevant when setting
>> -up the device, but once the subdev is registered it is completely transparent.
>> -
>
> Have you considered moving the whole part that describes how to call
> operations, which comes after the synchronous registration case to a
> dedicated sub-section ? The above paragraph makes sense in the context
> of describing why v4l2_subdev is advantageous as it abstract the
> underlying details under a unified call interface.
>
> This could become
>
> V4L2 sub-devices
> ----------------
>
> Intro
> ~~~~~
>
> Registration
> ~~~~~~~~~~~~
>
> **synchronous**
> **asynchronous**
>
> Operations call (or a better name :)
> ~~~~~~~~~~~~~~~
>
> What do you think ?

I think you're right. Incoming v2 with the new layout as you suggested.

--
Luca

\
 
 \ /
  Last update: 2020-09-21 22:39    [W:0.291 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site