All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Waychison <mikew@google.com>
To: Greg KH <greg@kroah.com>
Cc: torvalds@linux-foundation.org, San Mehat <san@google.com>,
	Aaron Durbin <adurbin@google.com>,
	Duncan Laurie <dlaurie@google.com>,
	linux-kernel@vger.kernel.org, Tim Hockin <thockin@google.com>
Subject: Re: [PATCH v1 3/6] driver: Google EFI SMI
Date: Tue, 25 Jan 2011 15:12:52 -0800	[thread overview]
Message-ID: <AANLkTi=XhRX-=k8UmmQFshP6fgF_q2c-9gEHuH0mWu0a@mail.gmail.com> (raw)
In-Reply-To: <20110125031752.GA9846@kroah.com>

On Mon, Jan 24, 2011 at 7:17 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Jan 24, 2011 at 04:24:49PM -0800, Mike Waychison wrote:
>> +struct gsmi_ioctl {
>> +     uint16_t        length;         /* total length including data */
>> +     int             version;        /* structure version */
>> +     int             command;        /* ioctl command */
>
> Ick.  Use proper data types if you are going to create a new ioctl.
> Same goes for the structures above (hint, use __u32 and friends, not the
> unit??_t crap.
>
> I'd strongly suggest NOT creating a new ioctl though, that' just going
> to be a pain in the long run.

Ok.  I'll change these.  Even if the __u32 vs u32 vs uint32_t
differentiation is non-sense :(

>
>
>> +     union {
>> +             struct gsmi_get_nvram_var       get_nvram_var;
>> +             struct gsmi_get_next_var        get_next_var;
>> +             struct gsmi_set_nvram_var       set_nvram_var;
>> +             struct gsmi_set_event_log       set_event_log;
>> +             struct gsmi_clear_event_log     clear_event_log;
>> +     } gsmi_cmd;
>> +} __packed;
>> +
>> +#define GSMI_BASE                    'G'
>> +#define GSMI_IOCTL_VERSION           1
>> +#define GSMI_IOCTL                   _IOWR(GSMI_BASE, GSMI_IOCTL_VERSION, \
>> +                                         struct gsmi_ioctl)
>> +
>> +#endif /* _LINUX_GSMI_H */
>> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
>> index 18fd130..34f5dfa 100644
>> --- a/include/linux/miscdevice.h
>> +++ b/include/linux/miscdevice.h
>> @@ -40,6 +40,7 @@
>>  #define BTRFS_MINOR          234
>>  #define AUTOFS_MINOR         235
>>  #define MAPPER_CTRL_MINOR    236
>> +#define GOOGLE_SMI_MINOR     242
>>  #define MISC_DYNAMIC_MINOR   255
>
> Why make this a static number and not just use a dynamic one?

Well, we use static device numbers, which is why it has historically
been static as well.  Yes, that puts us squarely in the 1990s :)

I can change this guy too though.

  reply	other threads:[~2011-01-25 23:13 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-25  0:24 [PATCH v1 0/6] google firmware support Mike Waychison
2011-01-25  0:24 ` [PATCH v1 1/6] Add oops notification chain Mike Waychison
2011-01-25  2:06   ` Greg KH
2011-01-25 20:01     ` Mike Waychison
2011-01-25 21:36       ` Jeff Garzik
2011-01-25 21:43         ` Aaron Durbin
2011-01-25 21:54           ` Jeff Garzik
2011-01-25 22:21             ` Aaron Durbin
2011-01-26  2:48               ` Greg KH
2011-01-26 21:50                 ` Mike Waychison
2011-01-25  0:24 ` [PATCH v1 2/6] Introduce CONFIG_GOOGLE_FIRMWARE Mike Waychison
2011-01-25  0:24 ` [PATCH v1 3/6] driver: Google EFI SMI Mike Waychison
2011-01-25  3:17   ` Greg KH
2011-01-25 23:12     ` Mike Waychison [this message]
2011-01-26  2:46       ` Greg KH
2011-01-26 23:58         ` Mike Waychison
2011-01-27  1:22           ` Mike Waychison
2011-01-27 23:41             ` Mike Waychison
2011-01-28  2:56               ` Greg KH
2011-02-20  4:44               ` Matt Domsch
2011-02-21 13:58                 ` Matthew Garrett
2011-01-27 10:43           ` Alan Cox
2011-01-27 19:22             ` Mike Waychison
2011-01-28  2:55               ` Greg KH
2011-01-28  2:59           ` Greg KH
2011-01-25  0:24 ` [PATCH v1 4/6] driver: Google Bootlog Mike Waychison
2011-01-25  0:49   ` Alan Cox
2011-01-25  1:38     ` Mike Waychison
2011-01-25  9:43       ` Alan Cox
2011-01-25  0:25 ` [PATCH v1 5/6] Allow prepending to the dmesg Mike Waychison
2011-01-25  1:01   ` Andrew Morton
2011-01-25  0:25 ` [PATCH v1 6/6] driver: Google Memory Console Mike Waychison
2011-01-25  2:00   ` Greg KH
2011-01-25  3:01 ` [PATCH v1 0/6] google firmware support Greg KH
2011-01-25 19:58   ` Mike Waychison
2011-01-26  2:47     ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='AANLkTi=XhRX-=k8UmmQFshP6fgF_q2c-9gEHuH0mWu0a@mail.gmail.com' \
    --to=mikew@google.com \
    --cc=adurbin@google.com \
    --cc=dlaurie@google.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=san@google.com \
    --cc=thockin@google.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.