| Message ID | 20190807162808.119141-1-sean@poorly.run (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Fix kerneldoc warns in connector-related docs | expand |
Hi Sean. On Wed, Aug 07, 2019 at 12:28:04PM -0400, Sean Paul wrote: > From: Sean Paul <seanpaul@chromium.org> > > Fixes the following warnings: > ../drivers/gpu/drm/drm_connector.c:989: WARNING: Unexpected indentation. > ../drivers/gpu/drm/drm_connector.c:993: WARNING: Unexpected indentation. > ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase reference start-string without end-string. > ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase reference start-string without end-string. Thanks, 4 less warnings.. > > Fixes: 1b27fbdde1df ("drm: Add drm_atomic_get_(old|new)_connector_for_encoder() helpers") > Fixes: bb5a45d40d50 ("drm/hdcp: update content protection property with uevent") > Cc: Ramalingam C <ramalingam.c@intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Pekka Paalanen <pekka.paalanen@collabora.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <maxime.ripard@bootlin.com> > Cc: Sean Paul <sean@poorly.run> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > drivers/gpu/drm/drm_connector.c | 10 ++++++---- > include/drm/drm_connector.h | 4 ++-- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 354798bad576..4c766624b20d 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -986,12 +986,14 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = { > * - Kernel sends uevent with the connector id and property id through > * @drm_hdcp_update_content_protection, upon below kernel triggered > * scenarios: > - * DESIRED -> ENABLED (authentication success) > - * ENABLED -> DESIRED (termination of authentication) > + * > + * - DESIRED -> ENABLED (authentication success) > + * - ENABLED -> DESIRED (termination of authentication) > * - Please note no uevents for userspace triggered property state changes, > * which can't fail such as > - * DESIRED/ENABLED -> UNDESIRED > - * UNDESIRED -> DESIRED > + * > + * - DESIRED/ENABLED -> UNDESIRED > + * - UNDESIRED -> DESIRED > * - Userspace is responsible for polling the property or listen to uevents > * to determine when the value transitions from ENABLED to DESIRED. > * This signifies the link is no longer protected and userspace should > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 0b9997e27689..e391f9c05f98 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -543,8 +543,8 @@ struct drm_connector_state { > * > * This is also used in the atomic helpers to map encoders to their > * current and previous connectors, see > - * &drm_atomic_get_old_connector_for_encoder() and > - * &drm_atomic_get_new_connector_for_encoder(). > + * &drm_atomic_get_old_connector_for_encoder and > + * &drm_atomic_get_new_connector_for_encoder. Please fix this to use () for the functions and drop the "&". This is what we use in drm kernel-doc for functions. See for example function references in doc of writeback_job in the same file. With this fixed: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > * > * NOTE: Atomic drivers must fill this out (either themselves or through > * helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will > -- > Sean Paul, Software Engineer, Google / Chromium OS
On Wed, Aug 07, 2019 at 07:30:23PM +0200, Sam Ravnborg wrote: > Hi Sean. > > On Wed, Aug 07, 2019 at 12:28:04PM -0400, Sean Paul wrote: > > From: Sean Paul <seanpaul@chromium.org> > > > > Fixes the following warnings: > > ../drivers/gpu/drm/drm_connector.c:989: WARNING: Unexpected indentation. > > ../drivers/gpu/drm/drm_connector.c:993: WARNING: Unexpected indentation. > > ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase reference start-string without end-string. > > ../include/drm/drm_connector.h:544: WARNING: Inline interpreted text or phrase reference start-string without end-string. > > Thanks, 4 less warnings.. > > > > Fixes: 1b27fbdde1df ("drm: Add drm_atomic_get_(old|new)_connector_for_encoder() helpers") > > Fixes: bb5a45d40d50 ("drm/hdcp: update content protection property with uevent") > > Cc: Ramalingam C <ramalingam.c@intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: Pekka Paalanen <pekka.paalanen@collabora.com> > > Cc: Sam Ravnborg <sam@ravnborg.org> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maxime Ripard <maxime.ripard@bootlin.com> > > Cc: Sean Paul <sean@poorly.run> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > drivers/gpu/drm/drm_connector.c | 10 ++++++---- > > include/drm/drm_connector.h | 4 ++-- > > 2 files changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > > index 354798bad576..4c766624b20d 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -986,12 +986,14 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = { > > * - Kernel sends uevent with the connector id and property id through > > * @drm_hdcp_update_content_protection, upon below kernel triggered > > * scenarios: > > - * DESIRED -> ENABLED (authentication success) > > - * ENABLED -> DESIRED (termination of authentication) > > + * > > + * - DESIRED -> ENABLED (authentication success) > > + * - ENABLED -> DESIRED (termination of authentication) > > * - Please note no uevents for userspace triggered property state changes, > > * which can't fail such as > > - * DESIRED/ENABLED -> UNDESIRED > > - * UNDESIRED -> DESIRED > > + * > > + * - DESIRED/ENABLED -> UNDESIRED > > + * - UNDESIRED -> DESIRED > > * - Userspace is responsible for polling the property or listen to uevents > > * to determine when the value transitions from ENABLED to DESIRED. > > * This signifies the link is no longer protected and userspace should > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 0b9997e27689..e391f9c05f98 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -543,8 +543,8 @@ struct drm_connector_state { > > * > > * This is also used in the atomic helpers to map encoders to their > > * current and previous connectors, see > > - * &drm_atomic_get_old_connector_for_encoder() and > > - * &drm_atomic_get_new_connector_for_encoder(). > > + * &drm_atomic_get_old_connector_for_encoder and > > + * &drm_atomic_get_new_connector_for_encoder. > Please fix this to use () for the functions and drop the "&". > This is what we use in drm kernel-doc for functions. > See for example function references in doc of writeback_job in the same file. Doing this won't get a hyperlink in the docs. It seems like these hooks aren't recognized as functions by sphinx (not sure didn't look into it too deeply). The other "_funcs.*" hooks are also handled with '&' (there are lots of examples in drm_connector.h). I think preserving the hyperlinks probably outweighs the missing (), thoughts? Sean > > With this fixed: > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > > * > > * NOTE: Atomic drivers must fill this out (either themselves or through > > * helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will > > -- > > Sean Paul, Software Engineer, Google / Chromium OS
Hi Sean. > > > --- a/include/drm/drm_connector.h > > > +++ b/include/drm/drm_connector.h > > > @@ -543,8 +543,8 @@ struct drm_connector_state { > > > * > > > * This is also used in the atomic helpers to map encoders to their > > > * current and previous connectors, see > > > - * &drm_atomic_get_old_connector_for_encoder() and > > > - * &drm_atomic_get_new_connector_for_encoder(). > > > + * &drm_atomic_get_old_connector_for_encoder and > > > + * &drm_atomic_get_new_connector_for_encoder. > > Please fix this to use () for the functions and drop the "&". > > This is what we use in drm kernel-doc for functions. > > See for example function references in doc of writeback_job in the same file. > > Doing this won't get a hyperlink in the docs. It seems like these hooks aren't > recognized as functions by sphinx (not sure didn't look into it too deeply). The > other "_funcs.*" hooks are also handled with '&' (there are lots of examples in > drm_connector.h). > > I think preserving the hyperlinks probably outweighs the missing (), thoughts? Hmm, I actually tested it here - with sphinx_1.4.9. The links was preserved, the only difference was that they had the () prefixed to their name. Do you happen to use an older sphinx version? Sam
On Fri, Aug 09, 2019 at 10:15:51PM +0200, Sam Ravnborg wrote: > Hi Sean. > > > > > --- a/include/drm/drm_connector.h > > > > +++ b/include/drm/drm_connector.h > > > > @@ -543,8 +543,8 @@ struct drm_connector_state { > > > > * > > > > * This is also used in the atomic helpers to map encoders to their > > > > * current and previous connectors, see > > > > - * &drm_atomic_get_old_connector_for_encoder() and > > > > - * &drm_atomic_get_new_connector_for_encoder(). > > > > + * &drm_atomic_get_old_connector_for_encoder and > > > > + * &drm_atomic_get_new_connector_for_encoder. > > > Please fix this to use () for the functions and drop the "&". > > > This is what we use in drm kernel-doc for functions. > > > See for example function references in doc of writeback_job in the same file. > > > > Doing this won't get a hyperlink in the docs. It seems like these hooks aren't > > recognized as functions by sphinx (not sure didn't look into it too deeply). The > > other "_funcs.*" hooks are also handled with '&' (there are lots of examples in > > drm_connector.h). > > > > I think preserving the hyperlinks probably outweighs the missing (), thoughts? > > Hmm, I actually tested it here - with sphinx_1.4.9. > The links was preserved, the only difference was that they had the () > prefixed to their name. > > Do you happen to use an older sphinx version? I'm on 1.7.9. I just rechecked and was a bit confused in my last mail. The drm_atomic_get_*_connector_for_encoder links do work with (), it's the ones drm_connector_helper_funcs in the paragraph above that need the '&'. So I'll switch up these 2 and leave the others as-is. Cool? Sean > > Sam
Hi Sean. > > > > > --- a/include/drm/drm_connector.h > > > > > +++ b/include/drm/drm_connector.h > > > > > @@ -543,8 +543,8 @@ struct drm_connector_state { > > > > > * > > > > > * This is also used in the atomic helpers to map encoders to their > > > > > * current and previous connectors, see > > > > > - * &drm_atomic_get_old_connector_for_encoder() and > > > > > - * &drm_atomic_get_new_connector_for_encoder(). > > > > > + * &drm_atomic_get_old_connector_for_encoder and > > > > > + * &drm_atomic_get_new_connector_for_encoder. > > > > Please fix this to use () for the functions and drop the "&". > > > > This is what we use in drm kernel-doc for functions. > > > > See for example function references in doc of writeback_job in the same file. > > > > > > Doing this won't get a hyperlink in the docs. It seems like these hooks aren't > > > recognized as functions by sphinx (not sure didn't look into it too deeply). The > > > other "_funcs.*" hooks are also handled with '&' (there are lots of examples in > > > drm_connector.h). > > > > > > I think preserving the hyperlinks probably outweighs the missing (), thoughts? > > > > Hmm, I actually tested it here - with sphinx_1.4.9. > > The links was preserved, the only difference was that they had the () > > prefixed to their name. > > > > Do you happen to use an older sphinx version? > > I'm on 1.7.9. I just rechecked and was a bit confused in my last mail. The > drm_atomic_get_*_connector_for_encoder links do work with (), it's the ones > drm_connector_helper_funcs in the paragraph above that need the '&'. So I'll > switch up these 2 and leave the others as-is. Cool? Perfect! You can add my: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> and then apply. Sam
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 354798bad576..4c766624b20d 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -986,12 +986,14 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = { * - Kernel sends uevent with the connector id and property id through * @drm_hdcp_update_content_protection, upon below kernel triggered * scenarios: - * DESIRED -> ENABLED (authentication success) - * ENABLED -> DESIRED (termination of authentication) + * + * - DESIRED -> ENABLED (authentication success) + * - ENABLED -> DESIRED (termination of authentication) * - Please note no uevents for userspace triggered property state changes, * which can't fail such as - * DESIRED/ENABLED -> UNDESIRED - * UNDESIRED -> DESIRED + * + * - DESIRED/ENABLED -> UNDESIRED + * - UNDESIRED -> DESIRED * - Userspace is responsible for polling the property or listen to uevents * to determine when the value transitions from ENABLED to DESIRED. * This signifies the link is no longer protected and userspace should diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 0b9997e27689..e391f9c05f98 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -543,8 +543,8 @@ struct drm_connector_state { * * This is also used in the atomic helpers to map encoders to their * current and previous connectors, see - * &drm_atomic_get_old_connector_for_encoder() and - * &drm_atomic_get_new_connector_for_encoder(). + * &drm_atomic_get_old_connector_for_encoder and + * &drm_atomic_get_new_connector_for_encoder. * * NOTE: Atomic drivers must fill this out (either themselves or through * helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will