| Message ID | 20220225202614.225197-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm: Extend DEFINE_DRM_GEM_FOPS() for optional fops | expand |
On Fri, Feb 25, 2022 at 12:26:12PM -0800, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > Extend the helper macro so we don't have to throw it away if driver adds > support for optional fops, like show_fdinfo(). > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > include/drm/drm_gem.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 35e7f44c2a75..987e78b18244 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -327,7 +327,7 @@ struct drm_gem_object { > * non-static version of this you're probably doing it wrong and will break the > * THIS_MODULE reference by accident. > */ > -#define DEFINE_DRM_GEM_FOPS(name) \ > +#define DEFINE_DRM_GEM_FOPS(name, ...) \ > static const struct file_operations name = {\ > .owner = THIS_MODULE,\ > .open = drm_open,\ > @@ -338,6 +338,7 @@ struct drm_gem_object { > .read = drm_read,\ > .llseek = noop_llseek,\ > .mmap = drm_gem_mmap,\ > + ##__VA_ARGS__\ > } Would it not be less convoluted to make the macro only provide the initializers? So you'd get something like: static const struct file_operations foo = { DRM_GEM_FOPS, .my_stuff = whatever, }; > > void drm_gem_object_release(struct drm_gem_object *obj); > -- > 2.35.1
On Fri, Feb 25, 2022 at 12:36 PM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Fri, Feb 25, 2022 at 12:26:12PM -0800, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > Extend the helper macro so we don't have to throw it away if driver adds > > support for optional fops, like show_fdinfo(). > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > include/drm/drm_gem.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > > index 35e7f44c2a75..987e78b18244 100644 > > --- a/include/drm/drm_gem.h > > +++ b/include/drm/drm_gem.h > > @@ -327,7 +327,7 @@ struct drm_gem_object { > > * non-static version of this you're probably doing it wrong and will break the > > * THIS_MODULE reference by accident. > > */ > > -#define DEFINE_DRM_GEM_FOPS(name) \ > > +#define DEFINE_DRM_GEM_FOPS(name, ...) \ > > static const struct file_operations name = {\ > > .owner = THIS_MODULE,\ > > .open = drm_open,\ > > @@ -338,6 +338,7 @@ struct drm_gem_object { > > .read = drm_read,\ > > .llseek = noop_llseek,\ > > .mmap = drm_gem_mmap,\ > > + ##__VA_ARGS__\ > > } > > Would it not be less convoluted to make the macro only provide > the initializers? So you'd get something like: > > static const struct file_operations foo = { > DRM_GEM_FOPS, > .my_stuff = whatever, > }; > Hmm, I like my color of the bikeshed, but I guess it is a matter of opinion ;-) BR, -R
Hi Rob, > > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > > > index 35e7f44c2a75..987e78b18244 100644 > > > --- a/include/drm/drm_gem.h > > > +++ b/include/drm/drm_gem.h > > > @@ -327,7 +327,7 @@ struct drm_gem_object { > > > * non-static version of this you're probably doing it wrong and will break the > > > * THIS_MODULE reference by accident. > > > */ > > > -#define DEFINE_DRM_GEM_FOPS(name) \ > > > +#define DEFINE_DRM_GEM_FOPS(name, ...) \ > > > static const struct file_operations name = {\ > > > .owner = THIS_MODULE,\ > > > .open = drm_open,\ > > > @@ -338,6 +338,7 @@ struct drm_gem_object { > > > .read = drm_read,\ > > > .llseek = noop_llseek,\ > > > .mmap = drm_gem_mmap,\ > > > + ##__VA_ARGS__\ > > > } > > > > Would it not be less convoluted to make the macro only provide > > the initializers? So you'd get something like: > > > > static const struct file_operations foo = { > > DRM_GEM_FOPS, > > .my_stuff = whatever, > > }; > > > > Hmm, I like my color of the bikeshed, but I guess it is a matter of opinion ;-) Or less surprise. Most similar macros provides initializers only. Try "git grep DRM_.*OPS | grep define" in include/drm and take a look at the hits. Sam
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 35e7f44c2a75..987e78b18244 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -327,7 +327,7 @@ struct drm_gem_object { * non-static version of this you're probably doing it wrong and will break the * THIS_MODULE reference by accident. */ -#define DEFINE_DRM_GEM_FOPS(name) \ +#define DEFINE_DRM_GEM_FOPS(name, ...) \ static const struct file_operations name = {\ .owner = THIS_MODULE,\ .open = drm_open,\ @@ -338,6 +338,7 @@ struct drm_gem_object { .read = drm_read,\ .llseek = noop_llseek,\ .mmap = drm_gem_mmap,\ + ##__VA_ARGS__\ } void drm_gem_object_release(struct drm_gem_object *obj);