2006-05-01 20:43:11

by Thomas Horsten

[permalink] [raw]
Subject: [PATCH] MegaRAID driver management char device moved to misc

The MegaRAID driver's common management module (megaraid_mm.c) creates
a char device used by the management tool "megarc" from LSI Logic (and
possibly other management tools).

In 2.6 with udev, this device doesn't get created because it is not
registered in sysfs.

I first fixed this by registering a class "megaraid_mm", but realized
that this should probably be moved to misc devices, instead of taking
up a char major. This is because only 1 device is used, even if there
are multiple adapters - the minor is never used (the adapter info is
in the ioctl block sent to the driver, not detected based on the minor
number as one might think). So it is a complete waste to have an
entire major taken by this.

So it now uses a misc device which I named "megadev0" (the name that megarc
expects), and has a dynamic minor (previoulsy a dynamic major was used).

I have tested this on my own system with the megarc tool, and it works
just as fine as before (only now the device gets created correctly by
udev).

Please CC any replies to thomas dot horsten, at gmail.com, since my
main mailbox is flooded with spam and I'll probably not see your mail
if you're not in my whitelist.

Patch below:

diff -u linux-2.6.16.11/drivers/scsi/megaraid/megaraid_mm.c linux/drivers/scsi/megaraid/megaraid_mm.c
--- linux-2.6.16.11/drivers/scsi/megaraid/megaraid_mm.c 2006-04-24 21:20:24.000000000 +0100
+++ linux/drivers/scsi/megaraid/megaraid_mm.c 2006-05-01 21:22:41.000000000 +0100
@@ -59,7 +59,6 @@
EXPORT_SYMBOL(mraid_mm_unregister_adp);
EXPORT_SYMBOL(mraid_mm_adapter_app_handle);

-static int majorno;
static uint32_t drvr_ver = 0x02200206;

static int adapters_count_g;
@@ -76,6 +75,12 @@
.owner = THIS_MODULE,
};

+static struct miscdevice megaraid_mm_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "megadev0",
+ .fops = &lsi_fops,
+};
+
/**
* mraid_mm_open - open routine for char node interface
* @inod : unused
@@ -1197,15 +1202,16 @@
static int __init
mraid_mm_init(void)
{
+ int err;
+
// Announce the driver version
con_log(CL_ANN, (KERN_INFO "megaraid cmm: %s %s\n",
LSI_COMMON_MOD_VERSION, LSI_COMMON_MOD_EXT_VERSION));

- majorno = register_chrdev(0, "megadev", &lsi_fops);
-
- if (majorno < 0) {
- con_log(CL_ANN, ("megaraid cmm: cannot get major\n"));
- return majorno;
+ err = misc_register(&megaraid_mm_dev);
+ if (err < 0) {
+ con_log(CL_ANN, ("megaraid cmm: cannot register misc device\n"));
+ return err;
}

init_waitqueue_head(&wait_q);
@@ -1240,7 +1246,7 @@
{
con_log(CL_DLEVEL1 , ("exiting common mod\n"));

- unregister_chrdev(majorno, "megadev");
+ misc_deregister(&megaraid_mm_dev);
}

module_init(mraid_mm_init);
Only in linux/drivers/scsi/megaraid/: megaraid_mm.c.orig
Only in linux/drivers/scsi/megaraid/: megaraid_mm.c.th1
Only in linux/drivers/scsi/megaraid/: megaraid_mm.c~
diff -u linux-2.6.16.11/drivers/scsi/megaraid/megaraid_mm.h linux/drivers/scsi/megaraid/megaraid_mm.h
--- linux-2.6.16.11/drivers/scsi/megaraid/megaraid_mm.h 2006-04-24 21:20:24.000000000 +0100
+++ linux/drivers/scsi/megaraid/megaraid_mm.h 2006-05-01 21:12:33.000000000 +0100
@@ -22,6 +22,7 @@
#include <linux/moduleparam.h>
#include <linux/pci.h>
#include <linux/list.h>
+#include <linux/miscdevice.h>

#include "mbox_defs.h"
#include "megaraid_ioctl.h"



2006-05-04 13:37:39

by Thomas Horsten

[permalink] [raw]
Subject: RE: [PATCH] MegaRAID driver management char device moved to misc

On Thu, 4 May 2006, Ju, Seokmann wrote:

> Hi,
>
> For following reason, I cannot accept/approve this patch.
> I'll update further as I get clear.
>
> Thank you,
>
> > So it now uses a misc device which I named "megadev0" (the
> > name that megarc
> > expects), and has a dynamic minor (previoulsy a dynamic major
> > was used).
> The driver can not change device node name for backward compatibility.
> I'm checking with application team inside for further clarification and update here.

That is an invalid reason: There was no hardcoded device node previously
(it was using a dynamically assigned major).

The only tool I know which uses this device is "megarc" from LSI Logic.
This tool uses the char device /dev/megaraid0, which will be created
correctly when using my patch and udev (the recommended setup for Linux
2.6 systems). I have tested this and it works.

User-level applications should not rely on hardcoded device numbers in any
case, but should use the correct device from /dev or search for the device
they need in sysfs.

Thomas

2006-05-04 14:05:28

by Ju, Seokmann

[permalink] [raw]
Subject: RE: [PATCH] MegaRAID driver management char device moved to misc

Hi,
> > > So it now uses a misc device which I named "megadev0" (the
> > > name that megarc
> > > expects), and has a dynamic minor (previoulsy a dynamic major
> > > was used).
> > The driver can not change device node name for backward
> compatibility.
> > I'm checking with application team inside for further
> clarification and update here.
>
> That is an invalid reason: There was no hardcoded device node
> previously
> (it was using a dynamically assigned major).
You're right.
I misunderstood the whole idea of creating device node.
Besides this, I don't have any objection.
I would accept the patch based on descriptoin in flowing link.
http://www.us.kernel.org/pub/linux/utils/kernel/hotplug/udev_vs_devfs

Any other comment would be appreciated.

Thank you,

> -----Original Message-----
> From: Thomas Horsten [mailto:[email protected]]
> Sent: Thursday, May 04, 2006 9:38 AM
> To: Ju, Seokmann
> Cc: [email protected]; Kolli, Neela;
> [email protected]
> Subject: RE: [PATCH] MegaRAID driver management char device
> moved to misc
>
> On Thu, 4 May 2006, Ju, Seokmann wrote:
>
> > Hi,
> >
> > For following reason, I cannot accept/approve this patch.
> > I'll update further as I get clear.
> >
> > Thank you,
> >
> > > So it now uses a misc device which I named "megadev0" (the
> > > name that megarc
> > > expects), and has a dynamic minor (previoulsy a dynamic major
> > > was used).
> > The driver can not change device node name for backward
> compatibility.
> > I'm checking with application team inside for further
> clarification and update here.
>
> That is an invalid reason: There was no hardcoded device node
> previously
> (it was using a dynamically assigned major).
>
> The only tool I know which uses this device is "megarc" from
> LSI Logic.
> This tool uses the char device /dev/megaraid0, which will be created
> correctly when using my patch and udev (the recommended setup
> for Linux
> 2.6 systems). I have tested this and it works.
>
> User-level applications should not rely on hardcoded device
> numbers in any
> case, but should use the correct device from /dev or search
> for the device
> they need in sysfs.
>
> Thomas
>
>

2006-05-04 15:28:34

by Ju, Seokmann

[permalink] [raw]
Subject: RE: [PATCH] MegaRAID driver management char device moved to misc

Hi,

> > > > So it now uses a misc device which I named "megadev0" (the
> > > > name that megarc
> > > > expects), and has a dynamic minor (previoulsy a dynamic major
> > > > was used).
> > > The driver can not change device node name for backward
> > compatibility.
> > > I'm checking with application team inside for further
> > clarification and update here.
> >
> > That is an invalid reason: There was no hardcoded device node
> > previously
> > (it was using a dynamically assigned major).
> You're right.
> I misunderstood the whole idea of creating device node.
> Besides this, I don't have any objection.
> I would accept the patch based on descriptoin in flowing link.
> http://www.us.kernel.org/pub/linux/utils/kernel/hotplug/udev_vs_devfs
I need take back previous comment for accepting the patch.
There are several applications that is running with the driver.
And those applications are need to be verified with this patch before merge it to up-stream.
I'm sorry for creating confusion, today.

I'll update my final comment on the patch once all necessary verifications have done.

Thank you,

> -----Original Message-----
> From: Ju, Seokmann
> Sent: Thursday, May 04, 2006 10:05 AM
> To: 'Thomas Horsten'
> Cc: [email protected]; Kolli, Neela;
> [email protected]
> Subject: RE: [PATCH] MegaRAID driver management char device
> moved to misc
>
> Hi,
> > > > So it now uses a misc device which I named "megadev0" (the
> > > > name that megarc
> > > > expects), and has a dynamic minor (previoulsy a dynamic major
> > > > was used).
> > > The driver can not change device node name for backward
> > compatibility.
> > > I'm checking with application team inside for further
> > clarification and update here.
> >
> > That is an invalid reason: There was no hardcoded device node
> > previously
> > (it was using a dynamically assigned major).
> You're right.
> I misunderstood the whole idea of creating device node.
> Besides this, I don't have any objection.
> I would accept the patch based on descriptoin in flowing link.
> http://www.us.kernel.org/pub/linux/utils/kernel/hotplug/udev_vs_devfs
>
> Any other comment would be appreciated.
>
> Thank you,
>
> > -----Original Message-----
> > From: Thomas Horsten [mailto:[email protected]]
> > Sent: Thursday, May 04, 2006 9:38 AM
> > To: Ju, Seokmann
> > Cc: [email protected]; Kolli, Neela;
> > [email protected]
> > Subject: RE: [PATCH] MegaRAID driver management char device
> > moved to misc
> >
> > On Thu, 4 May 2006, Ju, Seokmann wrote:
> >
> > > Hi,
> > >
> > > For following reason, I cannot accept/approve this patch.
> > > I'll update further as I get clear.
> > >
> > > Thank you,
> > >
> > > > So it now uses a misc device which I named "megadev0" (the
> > > > name that megarc
> > > > expects), and has a dynamic minor (previoulsy a dynamic major
> > > > was used).
> > > The driver can not change device node name for backward
> > compatibility.
> > > I'm checking with application team inside for further
> > clarification and update here.
> >
> > That is an invalid reason: There was no hardcoded device node
> > previously
> > (it was using a dynamically assigned major).
> >
> > The only tool I know which uses this device is "megarc" from
> > LSI Logic.
> > This tool uses the char device /dev/megaraid0, which will be created
> > correctly when using my patch and udev (the recommended setup
> > for Linux
> > 2.6 systems). I have tested this and it works.
> >
> > User-level applications should not rely on hardcoded device
> > numbers in any
> > case, but should use the correct device from /dev or search
> > for the device
> > they need in sysfs.
> >
> > Thomas
> >
> >