[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20061203132602.07a271de.akpm@osdl.org>
Date: Sun, 3 Dec 2006 13:26:02 -0800
From: Andrew Morton <akpm@...l.org>
To: minyard@....org
Cc: Linux Kernel <linux-kernel@...r.kernel.org>,
OpenIPMI Developers <openipmi-developer@...ts.sourceforge.net>,
Carol Hebert <cah@...ibm.com>
Subject: Re: [Patch 1/12] IPMI: Fix device model name
On Fri, 1 Dec 2006 22:20:53 -0600
Corey Minyard <minyard@....org> wrote:
>
> This patch adds the product id to the driver model platform device
> name, in addition to the device id. The IPMI spec does not require
> that individual BMCs in a system have unique devices IDs, but it
> does require that the product id/device id combination be unique.
>
> This also removes a redundant check and cleans up error handling
> when the sysfs registration fails.
>
> Signed-off-by: Corey Minyard <minyard@....org>
> Cc: Carol Hebert <cah@...ibm.com>
>
> Index: linux-2.6.19/drivers/char/ipmi/ipmi_msghandler.c
> ===================================================================
> --- linux-2.6.19.orig/drivers/char/ipmi/ipmi_msghandler.c
> +++ linux-2.6.19/drivers/char/ipmi/ipmi_msghandler.c
> @@ -1817,13 +1817,12 @@ static int __find_bmc_prod_dev_id(struct
> struct bmc_device *bmc = dev_get_drvdata(dev);
>
> return (bmc->id.product_id == id->product_id
> - && bmc->id.product_id == id->product_id
> && bmc->id.device_id == id->device_id);
> }
>
> static struct bmc_device *ipmi_find_bmc_prod_dev_id(
> struct device_driver *drv,
> - unsigned char product_id, unsigned char device_id)
> + unsigned int product_id, unsigned char device_id)
> {
> struct prod_dev_id id = {
> .product_id = product_id,
> @@ -1940,6 +1939,9 @@ static ssize_t guid_show(struct device *
>
> static void remove_files(struct bmc_device *bmc)
> {
> + if (!bmc->dev)
> + return;
> +
> device_remove_file(&bmc->dev->dev,
> &bmc->device_id_attr);
> device_remove_file(&bmc->dev->dev,
> @@ -1973,7 +1975,8 @@ cleanup_bmc_device(struct kref *ref)
> bmc = container_of(ref, struct bmc_device, refcount);
>
> remove_files(bmc);
> - platform_device_unregister(bmc->dev);
> + if (bmc->dev)
> + platform_device_unregister(bmc->dev);
platform_device_unregister(NULL) appears to be legal.
> kfree(bmc);
> }
>
> @@ -1990,6 +1993,7 @@ static void ipmi_bmc_unregister(ipmi_smi
>
> mutex_lock(&ipmidriver_mutex);
> kref_put(&bmc->refcount, cleanup_bmc_device);
> + intf->bmc = NULL;
> mutex_unlock(&ipmidriver_mutex);
> }
>
> ...
>
> } else {
> - bmc->dev = platform_device_alloc("ipmi_bmc",
> - bmc->id.device_id);
> + char name[14];
> + unsigned char orig_dev_id = bmc->id.device_id;
> + int warn_printed = 0;
> +
> + snprintf(name, sizeof(name),
> + "ipmi_bmc.%4.4x", bmc->id.product_id);
> +
> + while (ipmi_find_bmc_prod_dev_id(&ipmidriver,
> + bmc->id.product_id,
> + bmc->id.device_id))
> + {
The brace should be on the previous line.
> + if (!warn_printed) {
> + printk(KERN_WARNING PFX
> + "This machine has two different BMCs"
> + " with the same product id and device"
> + " id. This is an error in the"
> + " firmware, but incrementing the"
> + " device id to work around the problem."
> + " Prod ID = 0x%x, Dev ID = 0x%x\n",
> + bmc->id.product_id, bmc->id.device_id);
> + warn_printed = 1;
Perhaps warn_printed should be static
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists