lkml.org 
[lkml]   [2005]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] IPMI: driver model and sysfs support
On 8/30/05, Corey Minyard <minyard@acm.org> wrote:
> This is very good. I believe the structure is correct, but I'm not a
> sysfs expert.
>
> There are a few things we need to deal with, though.
>
> * There are some significant changes to versioning in the
> driver that are in the mm tree right now (you can pull them from
>
> http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.13-rc6/2.6.13-rc6-mm2/broken-out)

Gah, ok :), I'll try to apply with the -mm changes.

> * There are some coding style problems. You have places like:
> +static void ipmi_bmc_unregister(struct ipmi_si_device *si,
> + struct ipmi_bmc_device *bmc){
> The '{' for functions and structures needs to be on it's own line.
> Also, several:
> + if(bmc->guid_present){
> that need spaces after the 'if' and before the '{'.
> The patch also adds some trailing spaces to empty lines, the
> following is a telltale sign:
> -
> +
> There was also one place where you added unneeded braces to
> a single statement and another where you deleted the empty line
> between two functions.
> These are all standard kernel coding style rules.

Yes, I should have caught those, thanks.

> * I'd prefer to store the product id, device id, and manufacturer id
> decoded. This makes it easier to handle (no need to use "memcmp"
> to compare) and print. The printing of the product id, for instance,
> will be rather unnatural in product_id_show().

I was under the impression that they were just OEM strings, and not
necessarily ASCII strings? I'll have a look at the specs again.

> * guids are not printable strings (see guid_show()).

guid_show, sounds useful!

Thanks,
Yani
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-08-30 20:15    [W:0.037 / U:2.516 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site