| Message ID | 20230105193039.287677-2-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/debugfs: use octal permissions instead of symbolic permissions | expand |
On Thu, Jan 05, 2023 at 04:30:39PM -0300, Maíra Canal wrote: > The structs drm_debugfs_info and drm_debugfs_entry don't have > descriptions for their parameters, which is causing the following warnings: > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'name' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'show' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'driver_features' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'data' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'dev' not described in 'drm_debugfs_entry' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'file' not described in 'drm_debugfs_entry' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'list' not described in 'drm_debugfs_entry' > > Therefore, fix the warnings by adding descriptions to all struct > parameters. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Maíra Canal <mcanal@igalia.com> > --- > include/drm/drm_debugfs.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > index 53b7297260a5..7616f457ce70 100644 > --- a/include/drm/drm_debugfs.h > +++ b/include/drm/drm_debugfs.h > @@ -86,9 +86,22 @@ struct drm_info_node { > * core. > */ > struct drm_debugfs_info { > + /** @name: File name */ > const char *name; > + > + /** > + * @show: > + * > + * Show callback. &seq_file->private will be set to the &struct > + * drm_debugfs_entry corresponding to the instance of this info > + * on a given &struct drm_device. So this is a bit late, but why don't we pass that drm_debugfs_entry as an explicit parameter? Or maybe just the struct drm_device, because that and the void *data is really all there is to pass along. Would give us more type-safety, which really is the main reason for having drm-specific debugfs functions. Either way, on the series: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + */ > int (*show)(struct seq_file*, void*); > + > + /** @driver_features: Required driver features for this entry. */ > u32 driver_features; > + > + /** @data: Driver-private data, should not be device-specific. */ > void *data; > }; > > @@ -99,8 +112,13 @@ struct drm_debugfs_info { > * drm_debugfs_info on a &struct drm_device. > */ > struct drm_debugfs_entry { > + /** @dev: &struct drm_device for this node. */ > struct drm_device *dev; > + > + /** @file: Template for this node. */ > struct drm_debugfs_info file; > + > + /** @list: Linked list of all device nodes. */ > struct list_head list; > }; > > -- > 2.39.0 >
On 1/6/23 17:19, Daniel Vetter wrote: > On Thu, Jan 05, 2023 at 04:30:39PM -0300, Maíra Canal wrote: >> The structs drm_debugfs_info and drm_debugfs_entry don't have >> descriptions for their parameters, which is causing the following warnings: >> >> include/drm/drm_debugfs.h:93: warning: Function parameter or member >> 'name' not described in 'drm_debugfs_info' >> include/drm/drm_debugfs.h:93: warning: Function parameter or member >> 'show' not described in 'drm_debugfs_info' >> include/drm/drm_debugfs.h:93: warning: Function parameter or member >> 'driver_features' not described in 'drm_debugfs_info' >> include/drm/drm_debugfs.h:93: warning: Function parameter or member >> 'data' not described in 'drm_debugfs_info' >> include/drm/drm_debugfs.h:105: warning: Function parameter or member >> 'dev' not described in 'drm_debugfs_entry' >> include/drm/drm_debugfs.h:105: warning: Function parameter or member >> 'file' not described in 'drm_debugfs_entry' >> include/drm/drm_debugfs.h:105: warning: Function parameter or member >> 'list' not described in 'drm_debugfs_entry' >> >> Therefore, fix the warnings by adding descriptions to all struct >> parameters. >> >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >> Signed-off-by: Maíra Canal <mcanal@igalia.com> >> --- >> include/drm/drm_debugfs.h | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h >> index 53b7297260a5..7616f457ce70 100644 >> --- a/include/drm/drm_debugfs.h >> +++ b/include/drm/drm_debugfs.h >> @@ -86,9 +86,22 @@ struct drm_info_node { >> * core. >> */ >> struct drm_debugfs_info { >> + /** @name: File name */ >> const char *name; >> + >> + /** >> + * @show: >> + * >> + * Show callback. &seq_file->private will be set to the &struct >> + * drm_debugfs_entry corresponding to the instance of this info >> + * on a given &struct drm_device. > > So this is a bit late, but why don't we pass that drm_debugfs_entry as an > explicit parameter? Or maybe just the struct drm_device, because that and > the void *data is really all there is to pass along. Would give us more > type-safety, which really is the main reason for having drm-specific > debugfs functions. It seems like a better idea to pass the drm_debugfs_entry as an explicit parameter. I can work on it, but I guess it is a better idea to finish the conversion of all drm_debugfs_create_files() to drm_debugfs_add_files() and then perform the change in the show() signature. Best Regards, - Maíra Canal > > Either way, on the series: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> + */ >> int (*show)(struct seq_file*, void*); >> + >> + /** @driver_features: Required driver features for this entry. */ >> u32 driver_features; >> + >> + /** @data: Driver-private data, should not be device-specific. */ >> void *data; >> }; >> >> @@ -99,8 +112,13 @@ struct drm_debugfs_info { >> * drm_debugfs_info on a &struct drm_device. >> */ >> struct drm_debugfs_entry { >> + /** @dev: &struct drm_device for this node. */ >> struct drm_device *dev; >> + >> + /** @file: Template for this node. */ >> struct drm_debugfs_info file; >> + >> + /** @list: Linked list of all device nodes. */ >> struct list_head list; >> }; >> >> -- >> 2.39.0 >> >
On 1/5/23 16:30, Maíra Canal wrote: > The structs drm_debugfs_info and drm_debugfs_entry don't have > descriptions for their parameters, which is causing the following warnings: > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'name' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'show' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'driver_features' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:93: warning: Function parameter or member > 'data' not described in 'drm_debugfs_info' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'dev' not described in 'drm_debugfs_entry' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'file' not described in 'drm_debugfs_entry' > include/drm/drm_debugfs.h:105: warning: Function parameter or member > 'list' not described in 'drm_debugfs_entry' > > Therefore, fix the warnings by adding descriptions to all struct > parameters. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Maíra Canal <mcanal@igalia.com> Applied series to drm-misc-next. Best Regards, - Maíra Canal > --- > include/drm/drm_debugfs.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > index 53b7297260a5..7616f457ce70 100644 > --- a/include/drm/drm_debugfs.h > +++ b/include/drm/drm_debugfs.h > @@ -86,9 +86,22 @@ struct drm_info_node { > * core. > */ > struct drm_debugfs_info { > + /** @name: File name */ > const char *name; > + > + /** > + * @show: > + * > + * Show callback. &seq_file->private will be set to the &struct > + * drm_debugfs_entry corresponding to the instance of this info > + * on a given &struct drm_device. > + */ > int (*show)(struct seq_file*, void*); > + > + /** @driver_features: Required driver features for this entry. */ > u32 driver_features; > + > + /** @data: Driver-private data, should not be device-specific. */ > void *data; > }; > > @@ -99,8 +112,13 @@ struct drm_debugfs_info { > * drm_debugfs_info on a &struct drm_device. > */ > struct drm_debugfs_entry { > + /** @dev: &struct drm_device for this node. */ > struct drm_device *dev; > + > + /** @file: Template for this node. */ > struct drm_debugfs_info file; > + > + /** @list: Linked list of all device nodes. */ > struct list_head list; > }; >
On Mon, Jan 09, 2023 at 08:28:20AM -0300, Maíra Canal wrote: > On 1/6/23 17:19, Daniel Vetter wrote: > > On Thu, Jan 05, 2023 at 04:30:39PM -0300, Maíra Canal wrote: > > > The structs drm_debugfs_info and drm_debugfs_entry don't have > > > descriptions for their parameters, which is causing the following warnings: > > > > > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > > > 'name' not described in 'drm_debugfs_info' > > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > > > 'show' not described in 'drm_debugfs_info' > > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > > > 'driver_features' not described in 'drm_debugfs_info' > > > include/drm/drm_debugfs.h:93: warning: Function parameter or member > > > 'data' not described in 'drm_debugfs_info' > > > include/drm/drm_debugfs.h:105: warning: Function parameter or member > > > 'dev' not described in 'drm_debugfs_entry' > > > include/drm/drm_debugfs.h:105: warning: Function parameter or member > > > 'file' not described in 'drm_debugfs_entry' > > > include/drm/drm_debugfs.h:105: warning: Function parameter or member > > > 'list' not described in 'drm_debugfs_entry' > > > > > > Therefore, fix the warnings by adding descriptions to all struct > > > parameters. > > > > > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Signed-off-by: Maíra Canal <mcanal@igalia.com> > > > --- > > > include/drm/drm_debugfs.h | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h > > > index 53b7297260a5..7616f457ce70 100644 > > > --- a/include/drm/drm_debugfs.h > > > +++ b/include/drm/drm_debugfs.h > > > @@ -86,9 +86,22 @@ struct drm_info_node { > > > * core. > > > */ > > > struct drm_debugfs_info { > > > + /** @name: File name */ > > > const char *name; > > > + > > > + /** > > > + * @show: > > > + * > > > + * Show callback. &seq_file->private will be set to the &struct > > > + * drm_debugfs_entry corresponding to the instance of this info > > > + * on a given &struct drm_device. > > > > So this is a bit late, but why don't we pass that drm_debugfs_entry as an > > explicit parameter? Or maybe just the struct drm_device, because that and > > the void *data is really all there is to pass along. Would give us more > > type-safety, which really is the main reason for having drm-specific > > debugfs functions. > > It seems like a better idea to pass the drm_debugfs_entry as an explicit > parameter. I can work on it, but I guess it is a better idea to finish > the conversion of all drm_debugfs_create_files() to drm_debugfs_add_files() > and then perform the change in the show() signature. So drm_debugfs_entry still feels like a bit too high level, do callers really need that? They get the void * and I guess need the struct drm_device * This really starts to matter more when we also roll this out for connector/crtc, then you can give them directly a pointer to that. And the drm_debugfs_entry thing becomes an implementation detail entirely. Or do I miss something? Also yes we can do that later on, it shouldn't be too annyoing to roll out. -Daniel > > Best Regards, > - Maíra Canal > > > > > Either way, on the series: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > + */ > > > int (*show)(struct seq_file*, void*); > > > + > > > + /** @driver_features: Required driver features for this entry. */ > > > u32 driver_features; > > > + > > > + /** @data: Driver-private data, should not be device-specific. */ > > > void *data; > > > }; > > > @@ -99,8 +112,13 @@ struct drm_debugfs_info { > > > * drm_debugfs_info on a &struct drm_device. > > > */ > > > struct drm_debugfs_entry { > > > + /** @dev: &struct drm_device for this node. */ > > > struct drm_device *dev; > > > + > > > + /** @file: Template for this node. */ > > > struct drm_debugfs_info file; > > > + > > > + /** @list: Linked list of all device nodes. */ > > > struct list_head list; > > > }; > > > -- > > > 2.39.0 > > > > >
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h index 53b7297260a5..7616f457ce70 100644 --- a/include/drm/drm_debugfs.h +++ b/include/drm/drm_debugfs.h @@ -86,9 +86,22 @@ struct drm_info_node { * core. */ struct drm_debugfs_info { + /** @name: File name */ const char *name; + + /** + * @show: + * + * Show callback. &seq_file->private will be set to the &struct + * drm_debugfs_entry corresponding to the instance of this info + * on a given &struct drm_device. + */ int (*show)(struct seq_file*, void*); + + /** @driver_features: Required driver features for this entry. */ u32 driver_features; + + /** @data: Driver-private data, should not be device-specific. */ void *data; }; @@ -99,8 +112,13 @@ struct drm_debugfs_info { * drm_debugfs_info on a &struct drm_device. */ struct drm_debugfs_entry { + /** @dev: &struct drm_device for this node. */ struct drm_device *dev; + + /** @file: Template for this node. */ struct drm_debugfs_info file; + + /** @list: Linked list of all device nodes. */ struct list_head list; };
The structs drm_debugfs_info and drm_debugfs_entry don't have descriptions for their parameters, which is causing the following warnings: include/drm/drm_debugfs.h:93: warning: Function parameter or member 'name' not described in 'drm_debugfs_info' include/drm/drm_debugfs.h:93: warning: Function parameter or member 'show' not described in 'drm_debugfs_info' include/drm/drm_debugfs.h:93: warning: Function parameter or member 'driver_features' not described in 'drm_debugfs_info' include/drm/drm_debugfs.h:93: warning: Function parameter or member 'data' not described in 'drm_debugfs_info' include/drm/drm_debugfs.h:105: warning: Function parameter or member 'dev' not described in 'drm_debugfs_entry' include/drm/drm_debugfs.h:105: warning: Function parameter or member 'file' not described in 'drm_debugfs_entry' include/drm/drm_debugfs.h:105: warning: Function parameter or member 'list' not described in 'drm_debugfs_entry' Therefore, fix the warnings by adding descriptions to all struct parameters. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Maíra Canal <mcanal@igalia.com> --- include/drm/drm_debugfs.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)