Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756318AbXKTQfq (ORCPT ); Tue, 20 Nov 2007 11:35:46 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752484AbXKTQfh (ORCPT ); Tue, 20 Nov 2007 11:35:37 -0500 Received: from emulex.emulex.com ([138.239.112.1]:62217 "EHLO emulex.emulex.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752483AbXKTQfg (ORCPT ); Tue, 20 Nov 2007 11:35:36 -0500 Message-ID: <47430CCE.50701@emulex.com> Date: Tue, 20 Nov 2007 11:35:26 -0500 From: James Smart Reply-To: James.Smart@Emulex.Com User-Agent: Thunderbird 2.0.0.9 (Windows/20071031) MIME-Version: 1.0 To: Jonathan McDowell CC: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Matt_Domsch@dell.com Subject: Re: [PATCH] Unify sysfs filenames for firmware version References: <20071120133824.GD31112@earth.li> In-Reply-To: <20071120133824.GD31112@earth.li> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 20 Nov 2007 16:35:26.0751 (UTC) FILETIME=[5B0832F0:01C82B93] Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4264 Lines: 86 The hearburn I have with these patches is that you are changing driver-specific attributes, not common ones as enforced/requested by a subsystem. As such, you are breaking a management interface for existing tools/scripts. There's been a long-standing request to create common device attributes, such as fw_version, but I don't think they ever made it into the kernel. Not only would the names be consistent, but the location would be consistent as well. I'd rather you took on that work, than proceed with these patches. -- james s Jonathan McDowell wrote: > Looking around sysfs in an attempt to pull out SCSI card firmware > versions I found 5 different filenames used to store the information. > Only one, fw_version, was used more than once. The patch below changes > the other drivers to use this filename too. > > I suspect the same applies to other subsystem drivers as well. I'll look > at them assuming this patch is well received. > > Signed-Off-By: Jonathan McDowell > > ----- > diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c > index 626bb3c..ae80d04 100644 > --- a/drivers/message/fusion/mptscsih.c > +++ b/drivers/message/fusion/mptscsih.c > @@ -3307,7 +3307,7 @@ mptscsih_version_fw_show(struct class_device *cdev, char *buf) > (ioc->facts.FWVersion.Word & 0x0000FF00) >> 8, > ioc->facts.FWVersion.Word & 0x000000FF); > } > -static CLASS_DEVICE_ATTR(version_fw, S_IRUGO, mptscsih_version_fw_show, NULL); > +static CLASS_DEVICE_ATTR(fw_version, S_IRUGO, mptscsih_version_fw_show, NULL); > > static ssize_t > mptscsih_version_bios_show(struct class_device *cdev, char *buf) > diff --git a/drivers/scsi/arcmsr/arcmsr_attr.c b/drivers/scsi/arcmsr/arcmsr_attr.c > index 7d7b0a5..646f24c 100644 > --- a/drivers/scsi/arcmsr/arcmsr_attr.c > +++ b/drivers/scsi/arcmsr/arcmsr_attr.c > @@ -352,7 +352,7 @@ static CLASS_DEVICE_ATTR(host_driver_posted_cmd, S_IRUGO, arcmsr_attr_host_drive > static CLASS_DEVICE_ATTR(host_driver_reset, S_IRUGO, arcmsr_attr_host_driver_reset, NULL); > static CLASS_DEVICE_ATTR(host_driver_abort, S_IRUGO, arcmsr_attr_host_driver_abort, NULL); > static CLASS_DEVICE_ATTR(host_fw_model, S_IRUGO, arcmsr_attr_host_fw_model, NULL); > -static CLASS_DEVICE_ATTR(host_fw_version, S_IRUGO, arcmsr_attr_host_fw_version, NULL); > +static CLASS_DEVICE_ATTR(fw_version, S_IRUGO, arcmsr_attr_host_fw_version, NULL); > static CLASS_DEVICE_ATTR(host_fw_request_len, S_IRUGO, arcmsr_attr_host_fw_request_len, NULL); > static CLASS_DEVICE_ATTR(host_fw_numbers_queue, S_IRUGO, arcmsr_attr_host_fw_numbers_queue, NULL); > static CLASS_DEVICE_ATTR(host_fw_sdram_size, S_IRUGO, arcmsr_attr_host_fw_sdram_size, NULL); > diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c > index 0844331..ed130ec 100644 > --- a/drivers/scsi/hptiop.c > +++ b/drivers/scsi/hptiop.c > @@ -634,7 +634,7 @@ static struct class_device_attribute hptiop_attr_version = { > > static struct class_device_attribute hptiop_attr_fw_version = { > .attr = { > - .name = "firmware-version", > + .name = "fw_version", > .mode = S_IRUGO, > }, > .show = hptiop_show_fw_version, > diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c > index 80a1121..32b1d2f 100644 > --- a/drivers/scsi/lpfc/lpfc_attr.c > +++ b/drivers/scsi/lpfc/lpfc_attr.c > @@ -888,7 +888,7 @@ static CLASS_DEVICE_ATTR(modeldesc, S_IRUGO, lpfc_modeldesc_show, NULL); > static CLASS_DEVICE_ATTR(modelname, S_IRUGO, lpfc_modelname_show, NULL); > static CLASS_DEVICE_ATTR(programtype, S_IRUGO, lpfc_programtype_show, NULL); > static CLASS_DEVICE_ATTR(portnum, S_IRUGO, lpfc_vportnum_show, NULL); > -static CLASS_DEVICE_ATTR(fwrev, S_IRUGO, lpfc_fwrev_show, NULL); > +static CLASS_DEVICE_ATTR(fw_version, S_IRUGO, lpfc_fwrev_show, NULL); > static CLASS_DEVICE_ATTR(hdw, S_IRUGO, lpfc_hdw_show, NULL); > static CLASS_DEVICE_ATTR(state, S_IRUGO, lpfc_state_show, NULL); > static CLASS_DEVICE_ATTR(option_rom_version, S_IRUGO, > ----- > > J. > - 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/