Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1562890
| 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 |
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
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