[PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Mark Haverkamp]

Re-send of the patch diffed against the latest scsi-misc-2.6 git tree.

This patch adds some files into the /sys/class/scsi_host/hostN
directories for aacraid adapters.  The file names are pretty much self-
explanitory:

aac_driver_version: Driver version and date
aac_bios_version: Adapter's bios version
aac_kernel_version: Adapter's kernel version
aac_monitor_version: Adapter's monitor version
aac_model: Adapter's model name/number
aac_serial_number: Adapter's serial number

This is information that used to be supplied by the /proc interface.

Signed-off-by: Mark Haverkamp <markh@osdl.org>


--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -533,6 +533,135 @@ static long aac_compat_cfg_ioctl(struct 
 }
 #endif
 
+static ssize_t aac_show_host_version(struct class_device *class_dev,
+		char *buf)
+{
+	int len;
+
+	len = snprintf(buf, PAGE_SIZE, "Adaptec Raid Controller: %s %s\n",
+		  AAC_DRIVER_VERSION,
+		  AAC_DRIVER_BUILD_DATE);
+	return len;
+}
+
+static ssize_t aac_show_model(struct class_device *class_dev,
+		char *buf)
+{
+	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+	int len;
+
+	len = snprintf(buf, PAGE_SIZE, "Vendor: %s Model: %s\n",
+		  aac_drivers[dev->cardtype].vname,
+		  aac_drivers[dev->cardtype].model);
+	return len;
+}
+
+static ssize_t aac_show_kernel_version(struct class_device *class_dev,
+		char *buf)
+{
+	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+	int len, tmp;
+
+	tmp = le32_to_cpu(dev->adapter_info.kernelrev);
+	len = snprintf(buf, PAGE_SIZE, "kernel: %d.%d-%d[%d]\n", 
+	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
+	  le32_to_cpu(dev->adapter_info.kernelbuild));
+	return len;
+}
+
+static ssize_t aac_show_monitor_version(struct class_device *class_dev,
+		char *buf)
+{
+	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+	int len, tmp;
+
+	tmp = le32_to_cpu(dev->adapter_info.monitorrev);
+	len = snprintf(buf, PAGE_SIZE, "monitor: %d.%d-%d[%d]\n", 
+	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
+	  le32_to_cpu(dev->adapter_info.monitorbuild));
+	return len;
+}
+
+static ssize_t aac_show_bios_version(struct class_device *class_dev,
+		char *buf)
+{
+	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+	int len, tmp;
+
+	tmp = le32_to_cpu(dev->adapter_info.biosrev);
+	len = snprintf(buf, PAGE_SIZE, "bios: %d.%d-%d[%d]\n", 
+	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
+	  le32_to_cpu(dev->adapter_info.biosbuild));
+	return len;
+}
+
+static ssize_t aac_show_serial_number(struct class_device *class_dev,
+		char *buf)
+{
+	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
+	int len = 0;
+
+	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)
+		len = snprintf(buf, PAGE_SIZE, "serial: %x\n",
+		  le32_to_cpu(dev->adapter_info.serial[0]));
+	return len;
+}
+
+
+static struct class_device_attribute aac_host_version = {
+	.attr = {
+		.name = "aac_driver_version",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_host_version,
+};
+static struct class_device_attribute aac_model = {
+	.attr = {
+		.name = "aac_model",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_model,
+};
+static struct class_device_attribute aac_kernel_version = {
+	.attr = {
+		.name = "aac_kernel_version",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_kernel_version,
+};
+static struct class_device_attribute aac_monitor_version = {
+	.attr = {
+		.name = "aac_monitor_version",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_monitor_version,
+};
+static struct class_device_attribute aac_bios_version = {
+	.attr = {
+		.name = "aac_bios_version",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_bios_version,
+};
+static struct class_device_attribute aac_serial_number = {
+	.attr = {
+		.name = "aac_serial_number",
+		.mode = S_IRUGO,
+	},
+	.show = aac_show_serial_number,
+};
+
+static struct class_device_attribute *aac_attrs[] = {
+	&aac_host_version,
+	&aac_model,
+	&aac_kernel_version,
+	&aac_monitor_version,
+	&aac_bios_version,
+	&aac_serial_number,
+	NULL
+};
+
+
 static struct file_operations aac_cfg_fops = {
 	.owner		= THIS_MODULE,
 	.ioctl		= aac_cfg_ioctl,
@@ -553,6 +682,7 @@ static struct scsi_host_template aac_dri
 #endif
 	.queuecommand   		= aac_queuecommand,
 	.bios_param     		= aac_biosparm,	
+	.shost_attrs			= aac_attrs,
 	.slave_configure		= aac_slave_configure,
 	.eh_abort_handler		= aac_eh_abort,
 	.eh_host_reset_handler		= aac_eh_reset,

-- 
Mark Haverkamp <markh@osdl.org>

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Christoph Hellwig]

On Tue, May 17, 2005 at 01:34:57PM -0700, Mark Haverkamp wrote:
> Re-send of the patch diffed against the latest scsi-misc-2.6 git tree.
> 
> This patch adds some files into the /sys/class/scsi_host/hostN
> directories for aacraid adapters.  The file names are pretty much self-
> explanitory:
> 
> aac_driver_version: Driver version and date
> aac_bios_version: Adapter's bios version
> aac_kernel_version: Adapter's kernel version
> aac_monitor_version: Adapter's monitor version
> aac_model: Adapter's model name/number
> aac_serial_number: Adapter's serial number

Can you remove all those aac_ prefixes?

> +static ssize_t aac_show_host_version(struct class_device *class_dev,
> +		char *buf)
> +{
> +	int len;
> +
> +	len = snprintf(buf, PAGE_SIZE, "Adaptec Raid Controller: %s %s\n",
> +		  AAC_DRIVER_VERSION,
> +		  AAC_DRIVER_BUILD_DATE);
> +	return len;
> +}

the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
data and version neither, use MODULE_VERSION instead.

> +
> +static ssize_t aac_show_model(struct class_device *class_dev,
> +		char *buf)
> +{
> +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;

No need to cast.

> +	int len;
> +
> +	len = snprintf(buf, PAGE_SIZE, "Vendor: %s Model: %s\n",
> +		  aac_drivers[dev->cardtype].vname,
> +		  aac_drivers[dev->cardtype].model);
> +	return len;

please use two different attributes and remove the prefix.

> +static ssize_t aac_show_kernel_version(struct class_device *class_dev,
> +		char *buf)
> +{
> +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> +	int len, tmp;
> +
> +	tmp = le32_to_cpu(dev->adapter_info.kernelrev);
> +	len = snprintf(buf, PAGE_SIZE, "kernel: %d.%d-%d[%d]\n", 
> +	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
> +	  le32_to_cpu(dev->adapter_info.kernelbuild));
> +	return len;
> +}

Again please remove the prefix.  Also what does kernel version mean in
this context?

> +static ssize_t aac_show_monitor_version(struct class_device *class_dev,
> +		char *buf)
> +{
> +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> +	int len, tmp;
> +
> +	tmp = le32_to_cpu(dev->adapter_info.monitorrev);
> +	len = snprintf(buf, PAGE_SIZE, "monitor: %d.%d-%d[%d]\n", 
> +	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
> +	  le32_to_cpu(dev->adapter_info.monitorbuild));
> +	return len;

Again no prefix please.

> +}
> +
> +static ssize_t aac_show_bios_version(struct class_device *class_dev,
> +		char *buf)
> +{
> +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> +	int len, tmp;
> +
> +	tmp = le32_to_cpu(dev->adapter_info.biosrev);
> +	len = snprintf(buf, PAGE_SIZE, "bios: %d.%d-%d[%d]\n", 
> +	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
> +	  le32_to_cpu(dev->adapter_info.biosbuild));
> +	return len;
> +}

dito

> +
> +static ssize_t aac_show_serial_number(struct class_device *class_dev,
> +		char *buf)
> +{
> +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> +	int len = 0;
> +
> +	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)
> +		len = snprintf(buf, PAGE_SIZE, "serial: %x\n",
> +		  le32_to_cpu(dev->adapter_info.serial[0]));
> +	return len;
> +}

dito

RE: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Salyzyn, Mark]

Christoph Hellwig [mailto:hch@infradead.org] writes:

>> +static ssize_t aac_show_kernel_version(struct class_device
*class_dev,
>> +		char *buf)
>> +{
>> +	struct aac_dev *dev = (struct
aac_dev*)class_to_shost(class_dev)->>hostdata;
>> +	int len, tmp;
>> +
>> +	tmp = le32_to_cpu(dev->adapter_info.kernelrev);
>> +	len = snprintf(buf, PAGE_SIZE, "kernel: %d.%d-%d[%d]\n", 
>> +	  tmp >> 24, (tmp >> 16) & 0xff, tmp & 0xff,
>> +	  le32_to_cpu(dev->adapter_info.kernelbuild));
>> +	return len;
>> +}

>Again please remove the prefix.  Also what does kernel version mean in
>this context?

This is the kernel running in the Firmware of the Adapter.

I would be concerned about "kernel_version" confusion in the namespace
of the /sys/ filesystem. Dropping the prefix is not an issue, but do we
have documented naming standards for driver & firmware specific details?

-- Mark Salyzyn

RE: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Arjan van de Ven]

> I would be concerned about "kernel_version" confusion in the namespace
> of the /sys/ filesystem.

how about calling it firmware_kernel_version or hba_kernel_version?

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Christoph Hellwig]

On Mon, May 23, 2005 at 07:56:58AM -0400, Salyzyn, Mark wrote:
> >Again please remove the prefix.  Also what does kernel version mean in
> >this context?
> 
> This is the kernel running in the Firmware of the Adapter.

shouldn't we call this fw_version or firmware_version then?

> I would be concerned about "kernel_version" confusion in the namespace
> of the /sys/ filesystem. Dropping the prefix is not an issue, but do we
> have documented naming standards for driver & firmware specific details?

Unfortunately not.  I tried to get these attributes standardized at the
driver core level, but gregkh vetoed that.

The first scsi driver with lots of version attributes is the Emulex lpfc
driver so it might help to follow that one.

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Mark Haverkamp]

On Sun, 2005-05-22 at 17:54 +0100, Christoph Hellwig wrote:
> On Tue, May 17, 2005 at 01:34:57PM -0700, Mark Haverkamp wrote:
[...]
> > +static ssize_t aac_show_host_version(struct class_device *class_dev,
> > +		char *buf)
> > +{
> > +	int len;
> > +
> > +	len = snprintf(buf, PAGE_SIZE, "Adaptec Raid Controller: %s %s\n",
> > +		  AAC_DRIVER_VERSION,
> > +		  AAC_DRIVER_BUILD_DATE);
> > +	return len;
> > +}
> 
> the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
> data and version neither, use MODULE_VERSION instead.

Using MODULE_VERSION in the snprinf doesn't seem to work:

drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared (first use in this function)

> 
> > +
> > +static ssize_t aac_show_model(struct class_device *class_dev,
> > +		char *buf)
> > +{
> > +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> 
> No need to cast.

It looks like the cast is needed.  If I remove it I get a compile
warning.

drivers/scsi/aacraid/linit.c: In function `aac_show_model':
drivers/scsi/aacraid/linit.c:548: warning: initialization from incompatible pointer type


-- 
Mark Haverkamp <markh@osdl.org>

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Christoph Hellwig]

On Mon, May 23, 2005 at 11:36:48AM -0700, Mark Haverkamp wrote:
> > the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
> > data and version neither, use MODULE_VERSION instead.
> 
> Using MODULE_VERSION in the snprinf doesn't seem to work:
> 
> drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
> drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared (first use in this function)

sorry, MODULE_VERSION is a macro similar to MODULE_LICENSE or MODULE_AUTHOR.
You say MODULE_VERSION("something"); outside any function and you can find
the version out using modinfo and sysfs (not sure the latter is implemented
yet, but there were patches floating around)

> > > +
> > > +static ssize_t aac_show_model(struct class_device *class_dev,
> > > +		char *buf)
> > > +{
> > > +	struct aac_dev *dev = (struct aac_dev*)class_to_shost(class_dev)->hostdata;
> > 
> > No need to cast.
> 
> It looks like the cast is needed.  If I remove it I get a compile
> warning.
> 
> drivers/scsi/aacraid/linit.c: In function `aac_show_model':
> drivers/scsi/aacraid/linit.c:548: warning: initialization from incompatible pointer type

Sorry, you're right.  hostdata is an unsigned long.

RE: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Salyzyn, Mark]

Christoph Hellwig [mailto:hch@infradead.org] writes:
> On Mon, May 23, 2005 at 11:36:48AM -0700, Mark Haverkamp wrote:
>> > the "Adaptec Raid Controller:" doesn't belong in here.  The driver
build
>> > data and version neither, use MODULE_VERSION instead.
>> 
>> Using MODULE_VERSION in the snprinf doesn't seem to work:
>> 
>> drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
>> drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared
(first use in this function)
>
>sorry, MODULE_VERSION is a macro similar to MODULE_LICENSE or
>MODULE_AUTHOR.
>You say MODULE_VERSION("something"); outside any function and you can
find
>the version out using modinfo and sysfs (not sure the latter is
implemented
>yet, but there were patches floating around)

modinfo reports the binaries modules information. The sysfs version
would report the actual in-memory driver information. How do we make
sure the sysfs report of the MODULE_VERSION patch is driven home? :-)

Sadly, we can't access the string that was placed into the .modinfo
section (the name is a concatenation of the 'verison' with the line
number of the MODULE_VERSION 'call') from within the driver program for
use in the host_version function.

-- Mark Salyzyn

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Mark Haverkamp]

On Mon, 2005-05-23 at 19:46 +0100, Christoph Hellwig wrote:
> On Mon, May 23, 2005 at 11:36:48AM -0700, Mark Haverkamp wrote:
> > > the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
> > > data and version neither, use MODULE_VERSION instead.
> > 
> > Using MODULE_VERSION in the snprinf doesn't seem to work:
> > 
> > drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
> > drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared (first use in this function)
> 
> sorry, MODULE_VERSION is a macro similar to MODULE_LICENSE or MODULE_AUTHOR.
> You say MODULE_VERSION("something"); outside any function and you can find
> the version out using modinfo and sysfs (not sure the latter is implemented
> yet, but there were patches floating around)


I haven't seen anything to display MODULE_VERSION in sysfs.  How about
If I just print out AAC_DRIVER_VERSION which is what is passed into the
MODULE_VERSION macro.

Mark.

-- 
Mark Haverkamp <markh@osdl.org>

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Matt Domsch]

On Mon, May 23, 2005 at 01:49:30PM -0700, Mark Haverkamp wrote:
> On Mon, 2005-05-23 at 19:46 +0100, Christoph Hellwig wrote:
> > On Mon, May 23, 2005 at 11:36:48AM -0700, Mark Haverkamp wrote:
> > > > the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
> > > > data and version neither, use MODULE_VERSION instead.
> > > 
> > > Using MODULE_VERSION in the snprinf doesn't seem to work:
> > > 
> > > drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
> > > drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared (first use in this function)
> > 
> > sorry, MODULE_VERSION is a macro similar to MODULE_LICENSE or MODULE_AUTHOR.
> > You say MODULE_VERSION("something"); outside any function and you can find
> > the version out using modinfo and sysfs (not sure the latter is implemented
> > yet, but there were patches floating around)
> 
> I haven't seen anything to display MODULE_VERSION in sysfs.

http://marc.theaimsgroup.com/?l=linux-kernel&m=110671970405497&w=2
was posted, not included yet, and I haven't pushed it again lately
waiting for 2.6.12 to settle.

> How about If I just print out AAC_DRIVER_VERSION which is what is
> passed into the MODULE_VERSION macro.

That's what others do (ethtool -i), so seems sane if you really need
it exported in sysfs before the above is merged.  What's the rush
though?  If it can wait a release, I'll push on the above then it'll
be there for all modules.

Thanks,
Matt

-- 
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

RE: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Salyzyn, Mark]

Matt Domsch writes:
> That's what others do (ethtool -i), so seems sane if you really need
> it exported in sysfs before the above is merged.  What's the rush
> though?  If it can wait a release, I'll push on the above then it'll
> be there for all modules.

The 'rush' is pressure from OEMs for management tools that function with
the 'in-box' driver. Today is a day for sarcasm it appears :->

I'll direct the management software team to use the sys entry exported
by your patch, we can drop the host_version requirement from the driver
...

-- Mark Salyzyn

Re: [PATCH] 2.6 aacraid: Add sysfs files for aacraid adapter information (re-diff)

[Mark Haverkamp]

On Mon, 2005-05-23 at 22:30 -0500, Matt Domsch wrote:
> On Mon, May 23, 2005 at 01:49:30PM -0700, Mark Haverkamp wrote:
> > On Mon, 2005-05-23 at 19:46 +0100, Christoph Hellwig wrote:
> > > On Mon, May 23, 2005 at 11:36:48AM -0700, Mark Haverkamp wrote:
> > > > > the "Adaptec Raid Controller:" doesn't belong in here.  The driver build
> > > > > data and version neither, use MODULE_VERSION instead.
> > > > 
> > > > Using MODULE_VERSION in the snprinf doesn't seem to work:
> > > > 
> > > > drivers/scsi/aacraid/linit.c: In function `aac_show_host_version':
> > > > drivers/scsi/aacraid/linit.c:541: error: `MODULE_VERSION' undeclared (first use in this function)
> > > 
> > > sorry, MODULE_VERSION is a macro similar to MODULE_LICENSE or MODULE_AUTHOR.
> > > You say MODULE_VERSION("something"); outside any function and you can find
> > > the version out using modinfo and sysfs (not sure the latter is implemented
> > > yet, but there were patches floating around)
> > 
> > I haven't seen anything to display MODULE_VERSION in sysfs.
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=110671970405497&w=2
> was posted, not included yet, and I haven't pushed it again lately
> waiting for 2.6.12 to settle.
> 
> > How about If I just print out AAC_DRIVER_VERSION which is what is
> > passed into the MODULE_VERSION macro.
> 
> That's what others do (ethtool -i), so seems sane if you really need
> it exported in sysfs before the above is merged.  What's the rush
> though?  If it can wait a release, I'll push on the above then it'll
> be there for all modules.

I just hadn't seen that patch before.  If it is going to be merged in,
then it seems like we should use that method instead of adding another.

Mark.

> 
> Thanks,
> Matt
> 
-- 
Mark Haverkamp <markh@osdl.org>