drm: connector: Clarify drm_connecter.ddc field documentation

Submitted by Laurent Pinchart on Aug. 4, 2019, 12:26 p.m.

Details

Message ID 20190804122653.6587-1-laurent.pinchart@ideasonboard.com
State New
Headers show
Series "drm: connector: Clarify drm_connecter.ddc field documentation" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Laurent Pinchart Aug. 4, 2019, 12:26 p.m.
Improve the documentation of the ddc field by using DDC and I2C
consistently, and explaining more clearly what the field points to.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/drm/drm_connector.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index fc5d08438333..0d01662aa40e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1315,10 +1315,10 @@  struct drm_connector {
 
 	/**
 	 * @ddc: associated ddc adapter.
-	 * A connector usually has its associated ddc adapter. If a driver uses
-	 * this field, then an appropriate symbolic link is created in connector
-	 * sysfs directory to make it easy for the user to tell which i2c
-	 * adapter is for a particular display.
+	 * When the connector carries DDC signals, this field points to the I2C
+	 * adapter connected to the DDC signals, if any. When this field is not
+	 * NULL a symbolic link is created in the connector's sysfs directory
+	 * to expose the I2C adapter used by the connector.
 	 *
 	 * The field should be set by calling drm_connector_init_with_ddc().
 	 */

Comments

On Sun, Aug 04, 2019 at 03:26:53PM +0300, Laurent Pinchart wrote:
> Improve the documentation of the ddc field by using DDC and I2C
> consistently, and explaining more clearly what the field points to.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/drm/drm_connector.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index fc5d08438333..0d01662aa40e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1315,10 +1315,10 @@ struct drm_connector {
>  
>  	/**
>  	 * @ddc: associated ddc adapter.
> -	 * A connector usually has its associated ddc adapter. If a driver uses
> -	 * this field, then an appropriate symbolic link is created in connector
> -	 * sysfs directory to make it easy for the user to tell which i2c
> -	 * adapter is for a particular display.
> +	 * When the connector carries DDC signals, this field points to the I2C

s/DDC/a DDC/

> +	 * adapter connected to the DDC signals, if any. When this field is not
s/not NULL/is set/ otherwise parsing hiccup on the double negative :-)


Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	 * NULL a symbolic link is created in the connector's sysfs directory
> +	 * to expose the I2C adapter used by the connector.
>  	 *
>  	 * The field should be set by calling drm_connector_init_with_ddc().
>  	 */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel,

On Mon, Aug 05, 2019 at 06:28:15PM +0200, Daniel Vetter wrote:
> On Sun, Aug 04, 2019 at 03:26:53PM +0300, Laurent Pinchart wrote:
> > Improve the documentation of the ddc field by using DDC and I2C
> > consistently, and explaining more clearly what the field points to.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/drm/drm_connector.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index fc5d08438333..0d01662aa40e 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -1315,10 +1315,10 @@ struct drm_connector {
> >  
> >  	/**
> >  	 * @ddc: associated ddc adapter.
> > -	 * A connector usually has its associated ddc adapter. If a driver uses
> > -	 * this field, then an appropriate symbolic link is created in connector
> > -	 * sysfs directory to make it easy for the user to tell which i2c
> > -	 * adapter is for a particular display.
> > +	 * When the connector carries DDC signals, this field points to the I2C
> 
> s/DDC/a DDC/

DDC is carried by two signals (at least for DDC2, the mostly
unimplemented DDC1 used a single signal), so "carries DDC signals" seems
appropriate to me.

> > +	 * adapter connected to the DDC signals, if any. When this field is not
> s/not NULL/is set/ otherwise parsing hiccup on the double negative :-)
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > +	 * NULL a symbolic link is created in the connector's sysfs directory
> > +	 * to expose the I2C adapter used by the connector.
> >  	 *
> >  	 * The field should be set by calling drm_connector_init_with_ddc().
> >  	 */
On Mon, Aug 5, 2019 at 7:15 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Daniel,
>
> On Mon, Aug 05, 2019 at 06:28:15PM +0200, Daniel Vetter wrote:
> > On Sun, Aug 04, 2019 at 03:26:53PM +0300, Laurent Pinchart wrote:
> > > Improve the documentation of the ddc field by using DDC and I2C
> > > consistently, and explaining more clearly what the field points to.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  include/drm/drm_connector.h | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index fc5d08438333..0d01662aa40e 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -1315,10 +1315,10 @@ struct drm_connector {
> > >
> > >     /**
> > >      * @ddc: associated ddc adapter.
> > > -    * A connector usually has its associated ddc adapter. If a driver uses
> > > -    * this field, then an appropriate symbolic link is created in connector
> > > -    * sysfs directory to make it easy for the user to tell which i2c
> > > -    * adapter is for a particular display.
> > > +    * When the connector carries DDC signals, this field points to the I2C
> >
> > s/DDC/a DDC/
>
> DDC is carried by two signals (at least for DDC2, the mostly
> unimplemented DDC1 used a single signal), so "carries DDC signals" seems
> appropriate to me.

I overlooked the plural, looks good as-is.
-Daniel

>
> > > +    * adapter connected to the DDC signals, if any. When this field is not
> > s/not NULL/is set/ otherwise parsing hiccup on the double negative :-)
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > > +    * NULL a symbolic link is created in the connector's sysfs directory
> > > +    * to expose the I2C adapter used by the connector.
> > >      *
> > >      * The field should be set by calling drm_connector_init_with_ddc().
> > >      */
>
> --
> Regards,
>
> Laurent Pinchart