Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1339931
| From | Darren Hart <dvhart@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 |
| Date | 2016-02-22 22:20 +0100 |
| Message-ID | <r563E-3jn-29@gated-at.bofh.it> (permalink) |
| References | <qTjpE-39M-21@gated-at.bofh.it> <r2PgC-KD-5@gated-at.bofh.it> <r2PgD-KD-31@gated-at.bofh.it> <r44wV-5K3-1@gated-at.bofh.it> <r4Uvw-32j-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Mon, Feb 22, 2016 at 09:56:50AM +0100, Michał Kępień wrote:
> > > /*
> > > * Certain keys are flagged as KE_IGNORE. All of these are either
> > > * notifications (rather than requests for change) or are also sent
> > > @@ -513,6 +533,7 @@ static int __init dell_wmi_init(void)
> > > {
> > > int err;
> > > acpi_status status;
> > > + struct calling_interface_buffer *buffer;
> >
> > Please place the longest line first, and move int err to the last declaration.
> > When changing declarations of local variables, please use "Reverse Christmas
> > Tree" order (longest line to shortest line) wherever possible.
>
> Thanks, I'll keep that in mind for the future, though putting the
> WMI-enabling SMBIOS request in a separate function renders the need for
> the buffer variable in dell_wmi_init() void, so v4 won't touch this area
> any more.
>
> > Pali's point about documenting the hardcoded values and eliminating the code
> > duplication with a function (inline) is a good one.
>
> I plan to only put a comment next to 0x51534554 as 0x10000 is apparently
> just something pulled out of a hat (as the link provided in the commit
> message proves) and input[3] should be self-explanatory due to the name
> of the variable whose value is put into it.
>
> By the way, is there any kernel-wide or subsystem-wide policy for
> marking a function inline? I mean, this is hardly time-critical code,
> so is your suggestion to make it inline just a preference or am I
> unaware of some rule?
I suggested inline because the code was inline before and there would be no size
overhead to continue to make it inline. That said, the best source of guidance
on this is in CodingStyle, Chapter 15. While this function is quite small and
static to the file, it is not performance critical as you say. So, upon closer
inspection, there is no real need to be inline. And in fact, as there is no need
for it, it perhaps should not be. Thanks for raising the question.
>
> > Otherwise, this series looks good to me. Looking forward to merging v4.
>
> I'll try to post a v4 within the next couple of days.
Great, thank you.
--
Darren Hart
Intel Open Source Technology Center
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v3 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110 Michał Kępień <kernel@kempniu.pl> - 2016-02-16 16:00 +0100
[PATCH v3 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error() Michał Kępień <kernel@kempniu.pl> - 2016-02-16 16:00 +0100
[PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-16 16:00 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-02-16 16:20 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-16 23:00 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Darren Hart <dvhart@infradead.org> - 2016-02-20 02:30 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-22 10:00 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-02-22 10:10 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-22 10:40 +0100
Re: [PATCH v3 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Darren Hart <dvhart@infradead.org> - 2016-02-22 22:20 +0100
[PATCH v3 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-16 16:00 +0100
[PATCH v4 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
[PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:30 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 21:30 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:50 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Darren Hart <dvhart@infradead.org> - 2016-03-01 00:00 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-03-02 12:50 +0100
Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-03-03 12:40 +0100
[PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
Re: [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110 Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:10 +0100
[PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
Re: [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
[PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error() Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:30 +0100
Re: [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error() Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
[PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:30 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:10 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:40 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 21:40 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:50 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 22:00 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Darren Hart <dvhart@infradead.org> - 2016-03-01 00:10 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-03-02 13:40 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Darren Hart <dvhart@infradead.org> - 2016-03-03 18:20 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-03-03 19:50 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Darren Hart <dvhart@infradead.org> - 2016-03-03 21:50 +0100
Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Darren Hart <dvhart@infradead.org> - 2016-03-01 00:10 +0100
csiph-web