Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1562890

Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation

From "Luis R. Rodriguez" <mcgrof@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK() annotation
Date 2017-01-19 17:50 +0100
Message-ID <t1nBp-3Mq-49@gated-at.bofh.it> (permalink)
References <sNLDz-4f3-1@gated-at.bofh.it> <sYORr-58s-3@gated-at.bofh.it> <sYORr-58s-7@gated-at.bofh.it> <t1jeq-X3-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Jan 19, 2017 at 12:31:11PM +0100, Greg KH wrote:
> On Thu, Jan 12, 2017 at 06:42:50AM -0800, Luis R. Rodriguez wrote:
> > +Invalid users of the custom fallback mechanism can be policed using::
> 
> Ick, no, why?  Why not just add a checkpatch rule instead?

If its easy to do, how would we do that?

> >  
> >          $ export COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
> >          $ make coccicheck MODE=report
> > diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c
> > index 2f452f1f7c8a..3f2aa35bc54d 100644
> > --- a/drivers/firmware/dell_rbu.c
> > +++ b/drivers/firmware/dell_rbu.c
> > @@ -586,6 +586,7 @@ static ssize_t read_rbu_image_type(struct file *filp, struct kobject *kobj,
> >  	return size;
> >  }
> >  
> > +DECLARE_FW_CUSTOM_FALLBACK("Documentation/dell_rbu.txt");
> 
> That's a pain.

It is easier with checkpatch?

> > diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> > index b1f9f0ccb8ac..e6ca19c03dcc 100644
> > --- a/include/linux/firmware.h
> > +++ b/include/linux/firmware.h
> > @@ -8,6 +8,13 @@
> >  #define FW_ACTION_NOHOTPLUG 0
> >  #define FW_ACTION_HOTPLUG 1
> >  
> > +/*
> > + * Helper for scripts/coccinelle/api/request_firmware-custom-fallback.cocci
> > + * and so users can also easily search for the documentation for the
> > + * respectively needed custom fallback mechanism.
> > + */
> > +#define DECLARE_FW_CUSTOM_FALLBACK(__usermode_helper)
> 
> So you really don't need to put anything "valid" in the define argument?
> This feels like such a horrid hack, I really don't like it, especially
> as we don't do it anywhere else in the kernel, right?  Why start now?

Correct me if I'm wrong but AFAICT we may not have had previous grammatical
policing done before so I think this is a question of how we would want to
handle such type of strategies. Indeed this is just one approach. Using
checkpatch is certainly possible as well, I however think using checkpatch
is a bit more hacky.

I could also just drop this completely but figured its worth discussion.

  Luis

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread


Thread

Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK()  annotation Greg KH <gregkh@linuxfoundation.org> - 2017-01-19 12:40 +0100
  Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK()  annotation Greg KH <gregkh@linuxfoundation.org> - 2017-01-19 17:20 +0100
    Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK()  annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-01-19 22:40 +0100
  Re: [PATCH v4 2/2] firmware: add DECLARE_FW_CUSTOM_FALLBACK()  annotation "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-01-19 17:50 +0100

csiph-web