Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754277AbbFRJvl (ORCPT ); Thu, 18 Jun 2015 05:51:41 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:33359 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753594AbbFRJvd (ORCPT ); Thu, 18 Jun 2015 05:51:33 -0400 MIME-Version: 1.0 In-Reply-To: References: <1434102153-38581-1-git-send-email-Sreekanth.Reddy@avagotech.com> <1434102153-38581-11-git-send-email-Sreekanth.Reddy@avagotech.com> <20150615094654.GA5660@c203.arch.suse.de> <20150615115358.GB10429@c203.arch.suse.de> Date: Thu, 18 Jun 2015 15:21:31 +0530 Message-ID: Subject: Re: [PATCH 10/20] [SCSI] mpt3sas: Add branding string support for OEM's HBA From: Sreekanth Reddy To: Johannes Thumshirn Cc: "jejb@kernel.org" , Christoph Hellwig , "Martin K. Petersen" , "linux-scsi@vger.kernel.org" , "James E.J. Bottomley" , Sathya Prakash , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5444 Lines: 160 Hi, Any review comments on this patch. please let us known if any changes are required. Thanks, Sreekanth On Mon, Jun 15, 2015 at 5:30 PM, Sreekanth Reddy wrote: > On Mon, Jun 15, 2015 at 5:23 PM, Johannes Thumshirn wrote: >> On Mon, Jun 15, 2015 at 04:41:56PM +0530, Sreekanth Reddy wrote: >>> On Mon, Jun 15, 2015 at 3:16 PM, Johannes Thumshirn wrote: >>> > On Fri, Jun 12, 2015 at 03:12:22PM +0530, Sreekanth Reddy wrote: >>> >> Added the following Dell branding to the mpt3sas driver. >>> >> >>> >> "VendorID" "DeviceID" "SubsystemVendor ID" "SubsystemDevice ID" Dell Branding String >>> >> 0x1000 0x0097 0x1028 0x1F46 DELL 12Gbps HBA >>> >> >>> >> Signed-off-by: Sreekanth Reddy >>> >> --- >>> >> drivers/scsi/mpt3sas/mpt3sas_base.c | 36 ++++++++++++++++++++++++++++++++++++ >>> >> drivers/scsi/mpt3sas/mpt3sas_base.h | 11 +++++++++++ >>> >> 2 files changed, 47 insertions(+) >>> >> >>> >> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c >>> >> index 6739c0a..db362cb 100644 >>> >> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c >>> >> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c >>> >> @@ -2345,6 +2345,41 @@ _base_display_intel_branding(struct MPT3SAS_ADAPTER *ioc) >>> >> >>> >> >>> >> /** >>> >> + * _base_display_dell_branding - Display branding string >>> >> + * @ioc: per adapter object >>> >> + * >>> >> + * Return nothing. >>> >> + */ >>> >> +static void >>> >> +_base_display_dell_branding(struct MPT3SAS_ADAPTER *ioc) >>> >> +{ >>> >> + if (ioc->pdev->subsystem_vendor != PCI_VENDOR_ID_DELL) >>> >> + return; >>> >> + >>> >> + switch (ioc->pdev->device) { >>> >> + case MPI25_MFGPAGE_DEVID_SAS3008: >>> >> + switch (ioc->pdev->subsystem_device) { >>> >> + case MPT3SAS_DELL_12G_HBA_SSDID: >>> >> + pr_info(MPT3SAS_FMT "%s\n", ioc->name, >>> >> + MPT3SAS_DELL_12G_HBA_BRANDING); >>> >> + break; >>> >> + default: >>> >> + pr_info(MPT3SAS_FMT >>> >> + "Dell 12Gbps HBA: Subsystem ID: 0x%X\n", ioc->name, >>> >> + ioc->pdev->subsystem_device); >>> >> + break; >>> >> + } >>> >> + break; >>> >> + default: >>> >> + pr_info(MPT3SAS_FMT >>> >> + "Dell 12Gbps HBA: Subsystem ID: 0x%X\n", ioc->name, >>> >> + ioc->pdev->subsystem_device); >>> >> + break; >>> >> + } >>> >> +} >>> >> + >>> >> + >>> >> +/** >>> >> * _base_display_ioc_capabilities - Disply IOC's capabilities. >>> >> * @ioc: per adapter object >>> >> * >>> >> @@ -2374,6 +2409,7 @@ _base_display_ioc_capabilities(struct MPT3SAS_ADAPTER *ioc) >>> >> bios_version & 0x000000FF); >>> >> >>> >> _base_display_intel_branding(ioc); >>> >> + _base_display_dell_branding(ioc); >>> >> >>> >> pr_info(MPT3SAS_FMT "Protocol=(", ioc->name); >>> >> >>> >> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h >>> >> index 2318ef8..c4780c7 100644 >>> >> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h >>> >> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h >>> >> @@ -152,6 +152,17 @@ >>> >> #define MPT3SAS_INTEL_RS3UC080_SSDID 0x3524 >>> >> >>> >> /* >>> >> + * Dell HBA branding >>> >> + */ >>> >> +#define MPT3SAS_DELL_12G_HBA_BRANDING \ >>> >> + "Dell 12Gbps HBA" >>> >> + >>> >> +/* >>> >> + * Dell HBA SSDIDs >>> >> + */ >>> >> +#define MPT3SAS_DELL_12G_HBA_SSDID 0x1F46 >>> >> + >>> >> +/* >>> >> * status bits for ioc->diag_buffer_status >>> >> */ >>> >> #define MPT3_DIAG_BUFFER_IS_REGISTERED (0x01) >>> >> -- >>> >> 2.0.2 >>> >> >>> >> -- >>> >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >>> >> the body of a message to majordomo@vger.kernel.org >>> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> > >>> > This might be something you might consider re-organizing for your code-base >>> > merge as well, as it leads to a lot of code duplication the more OEMs you have. >>> >>> May I know which hunk of this patch may leads to code duplication for >>> more OEMs we add? Whether declaring individual function for each OEMs >>> leads to this code duplication. >> >> Hi Sreekanth, >> >> In >> [PATCH 11/20] [SCSI] mpt3sas: Add branding string support for OEM custom HBA >> you add _base_display_cisco_branding(). In >> [PATCH 10/20] [SCSI] mpt3sas: Add branding string support for OEM's HBA >> you add _base_display_dell_branding(). You already have >> _base_display_intel_branding(). >> >> Each of them follows the pattern: >> 1) Check if it is a OEM device for XXX >> 2) Check witch OEM device for XXX it is >> 3) Display a string based on that specific OEM device >> 4) Return > > Johannes, > > Thanks for detail explanation. I will take of this while merging the > drivers code. > > Thanks a lot, > Sreekanth >> >> Johannes > > > > -- > > Regards, > Sreekanth -- Regards, Sreekanth -- 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/