| Message ID | 20110920221051.1094.39578.stgit@localhost6.localdomain6 |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} > > libsas now handles: > 1/ limiting ata scanning to lun0 > 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only > supports 64) > 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices > > Cc: Xiangliang Yu <yuxiangl@marvell.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/scsi/mvsas/mv_init.c | 4 ++-- > drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------ > drivers/scsi/mvsas/mv_sas.h | 2 -- > 3 files changed, 2 insertions(+), 34 deletions(-) > > diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c > index 4e9af66..052861b 100644 > --- a/drivers/scsi/mvsas/mv_init.c > +++ b/drivers/scsi/mvsas/mv_init.c > @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = { > .name = DRV_NAME, > .queuecommand = sas_queuecommand, > .target_alloc = sas_target_alloc, > - .slave_configure = mvs_slave_configure, > + .slave_configure = sas_slave_configure, > .slave_destroy = sas_slave_destroy, > .scan_finished = mvs_scan_finished, > .scan_start = mvs_scan_start, > @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = { > .use_clustering = ENABLE_CLUSTERING, > .eh_device_reset_handler = sas_eh_device_reset_handler, > .eh_bus_reset_handler = sas_eh_bus_reset_handler, > - .slave_alloc = mvs_slave_alloc, > + .slave_alloc = sas_slave_alloc, > .target_destroy = sas_target_destroy, > -int mvs_slave_configure(struct scsi_device *sdev) > -{ > - struct domain_device *dev = sdev_to_domain_dev(sdev); > - int ret = sas_slave_configure(sdev); > - > - if (ret) > - return ret; > - if (!dev_is_sata(dev)) { > - sas_change_queue_depth(sdev, > - MVS_QUEUE_SIZE, > - SCSI_QDEPTH_DEFAULT); > - } > - return 0; Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/21/2011 10:26 PM, Xiangliang Yu wrote: > >> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} >> >> libsas now handles: >> 1/ limiting ata scanning to lun0 >> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only >> supports 64) >> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices >> >> Cc: Xiangliang Yu<yuxiangl@marvell.com> >> Signed-off-by: Dan Williams<dan.j.williams@intel.com> >> --- >> drivers/scsi/mvsas/mv_init.c | 4 ++-- >> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------ >> drivers/scsi/mvsas/mv_sas.h | 2 -- >> 3 files changed, 2 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c >> index 4e9af66..052861b 100644 >> --- a/drivers/scsi/mvsas/mv_init.c >> +++ b/drivers/scsi/mvsas/mv_init.c >> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = { >> .name = DRV_NAME, >> .queuecommand = sas_queuecommand, >> .target_alloc = sas_target_alloc, >> - .slave_configure = mvs_slave_configure, >> + .slave_configure = sas_slave_configure, >> .slave_destroy = sas_slave_destroy, >> .scan_finished = mvs_scan_finished, >> .scan_start = mvs_scan_start, >> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = { >> .use_clustering = ENABLE_CLUSTERING, >> .eh_device_reset_handler = sas_eh_device_reset_handler, >> .eh_bus_reset_handler = sas_eh_bus_reset_handler, >> - .slave_alloc = mvs_slave_alloc, >> + .slave_alloc = sas_slave_alloc, >> .target_destroy = sas_target_destroy, >> -int mvs_slave_configure(struct scsi_device *sdev) >> -{ >> - struct domain_device *dev = sdev_to_domain_dev(sdev); >> - int ret = sas_slave_configure(sdev); >> - >> - if (ret) >> - return ret; >> - if (!dev_is_sata(dev)) { >> - sas_change_queue_depth(sdev, >> - MVS_QUEUE_SIZE, >> - SCSI_QDEPTH_DEFAULT); >> - } >> - return 0; > Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue. > Thanks! There is no reason to keep a no-op function there. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 21, 2011 at 7:26 PM, Xiangliang Yu <yuxiangl@marvell.com> wrote: > >> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} >> >> libsas now handles: >> 1/ limiting ata scanning to lun0 >> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only >> supports 64) >> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices >> >> Cc: Xiangliang Yu <yuxiangl@marvell.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> drivers/scsi/mvsas/mv_init.c | 4 ++-- >> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------ >> drivers/scsi/mvsas/mv_sas.h | 2 -- >> 3 files changed, 2 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c >> index 4e9af66..052861b 100644 >> --- a/drivers/scsi/mvsas/mv_init.c >> +++ b/drivers/scsi/mvsas/mv_init.c >> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = { >> .name = DRV_NAME, >> .queuecommand = sas_queuecommand, >> .target_alloc = sas_target_alloc, >> - .slave_configure = mvs_slave_configure, >> + .slave_configure = sas_slave_configure, >> .slave_destroy = sas_slave_destroy, >> .scan_finished = mvs_scan_finished, >> .scan_start = mvs_scan_start, >> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = { >> .use_clustering = ENABLE_CLUSTERING, >> .eh_device_reset_handler = sas_eh_device_reset_handler, >> .eh_bus_reset_handler = sas_eh_bus_reset_handler, >> - .slave_alloc = mvs_slave_alloc, >> + .slave_alloc = sas_slave_alloc, >> .target_destroy = sas_target_destroy, >> -int mvs_slave_configure(struct scsi_device *sdev) >> -{ >> - struct domain_device *dev = sdev_to_domain_dev(sdev); >> - int ret = sas_slave_configure(sdev); >> - >> - if (ret) >> - return ret; >> - if (!dev_is_sata(dev)) { >> - sas_change_queue_depth(sdev, >> - MVS_QUEUE_SIZE, >> - SCSI_QDEPTH_DEFAULT); >> - } >> - return 0; > Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue. That's exactly the point of the patch. The isci driver encountered a bug that the mvsas driver had fixed in its private mvs_slave_alloc() routine. If I had taken the same approach and "fixed" it with a isci_slave_alloc() routine that would still leave aic94xx or any other future libsas driver to rediscover the fix. If you later find a problem with sas_slave_configure() there's a good chance the fix will be applicable to all libsas drivers. But if it is truly a mvsas specific issue you can always re-introduce mvs_slave_configure(). -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Subject: Re: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} > > On Wed, Sep 21, 2011 at 7:26 PM, Xiangliang Yu <yuxiangl@marvell.com> wrote: > > > >> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} > >> > >> libsas now handles: > >> 1/ limiting ata scanning to lun0 > >> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only > >> supports 64) > >> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices > >> > >> Cc: Xiangliang Yu <yuxiangl@marvell.com> > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> > >> --- > >> drivers/scsi/mvsas/mv_init.c | 4 ++-- > >> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------ > >> drivers/scsi/mvsas/mv_sas.h | 2 -- > >> 3 files changed, 2 insertions(+), 34 deletions(-) > >> > >> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c > >> index 4e9af66..052861b 100644 > >> --- a/drivers/scsi/mvsas/mv_init.c > >> +++ b/drivers/scsi/mvsas/mv_init.c > >> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = { > >> .name = DRV_NAME, > >> .queuecommand = sas_queuecommand, > >> .target_alloc = sas_target_alloc, > >> - .slave_configure = mvs_slave_configure, > >> + .slave_configure = sas_slave_configure, > >> .slave_destroy = sas_slave_destroy, > >> .scan_finished = mvs_scan_finished, > >> .scan_start = mvs_scan_start, > >> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = { > >> .use_clustering = ENABLE_CLUSTERING, > >> .eh_device_reset_handler = sas_eh_device_reset_handler, > >> .eh_bus_reset_handler = sas_eh_bus_reset_handler, > >> - .slave_alloc = mvs_slave_alloc, > >> + .slave_alloc = sas_slave_alloc, > >> .target_destroy = sas_target_destroy, > >> -int mvs_slave_configure(struct scsi_device *sdev) > >> -{ > >> - struct domain_device *dev = sdev_to_domain_dev(sdev); > >> - int ret = sas_slave_configure(sdev); > >> - > >> - if (ret) > >> - return ret; > >> - if (!dev_is_sata(dev)) { > >> - sas_change_queue_depth(sdev, > >> - MVS_QUEUE_SIZE, > >> - SCSI_QDEPTH_DEFAULT); > >> - } > >> - return 0; > > Can you don't remove mvs_slave_configure function? Someday it will be useful, > such as fixing some issue. > > That's exactly the point of the patch. The isci driver encountered a > bug that the mvsas driver had fixed in its private mvs_slave_alloc() > routine. If I had taken the same approach and "fixed" it with a > isci_slave_alloc() routine that would still leave aic94xx or any other > future libsas driver to rediscover the fix. > > If you later find a problem with sas_slave_configure() there's a good > chance the fix will be applicable to all libsas drivers. But if it is > truly a mvsas specific issue you can always re-introduce > mvs_slave_configure(). Sound good. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c index 4e9af66..052861b 100644 --- a/drivers/scsi/mvsas/mv_init.c +++ b/drivers/scsi/mvsas/mv_init.c @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = { .name = DRV_NAME, .queuecommand = sas_queuecommand, .target_alloc = sas_target_alloc, - .slave_configure = mvs_slave_configure, + .slave_configure = sas_slave_configure, .slave_destroy = sas_slave_destroy, .scan_finished = mvs_scan_finished, .scan_start = mvs_scan_start, @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = { .use_clustering = ENABLE_CLUSTERING, .eh_device_reset_handler = sas_eh_device_reset_handler, .eh_bus_reset_handler = sas_eh_bus_reset_handler, - .slave_alloc = mvs_slave_alloc, + .slave_alloc = sas_slave_alloc, .target_destroy = sas_target_destroy, .ioctl = sas_ioctl, .shost_attrs = mvst_host_attrs, diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index 4196eee..f2ac01f 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -276,36 +276,6 @@ static void mvs_bytes_dmaed(struct mvs_info *mvi, int i) PORTE_BYTES_DMAED); } -int mvs_slave_alloc(struct scsi_device *scsi_dev) -{ - struct domain_device *dev = sdev_to_domain_dev(scsi_dev); - if (dev_is_sata(dev)) { - /* We don't need to rescan targets - * if REPORT_LUNS request is failed - */ - if (scsi_dev->lun > 0) - return -ENXIO; - scsi_dev->tagged_supported = 1; - } - - return sas_slave_alloc(scsi_dev); -} - -int mvs_slave_configure(struct scsi_device *sdev) -{ - struct domain_device *dev = sdev_to_domain_dev(sdev); - int ret = sas_slave_configure(sdev); - - if (ret) - return ret; - if (!dev_is_sata(dev)) { - sas_change_queue_depth(sdev, - MVS_QUEUE_SIZE, - SCSI_QDEPTH_DEFAULT); - } - return 0; -} - void mvs_scan_start(struct Scsi_Host *shost) { int i, j; diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h index 44d7885..21ee8b0 100644 --- a/drivers/scsi/mvsas/mv_sas.h +++ b/drivers/scsi/mvsas/mv_sas.h @@ -459,8 +459,6 @@ int mvs_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func, void *funcdata); void __devinit mvs_set_sas_addr(struct mvs_info *mvi, int port_id, u32 off_lo, u32 off_hi, u64 sas_addr); -int mvs_slave_alloc(struct scsi_device *scsi_dev); -int mvs_slave_configure(struct scsi_device *sdev); void mvs_scan_start(struct Scsi_Host *shost); int mvs_scan_finished(struct Scsi_Host *shost, unsigned long time); int mvs_queue_command(struct sas_task *task, const int num,
libsas now handles: 1/ limiting ata scanning to lun0 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only supports 64) 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices Cc: Xiangliang Yu <yuxiangl@marvell.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/scsi/mvsas/mv_init.c | 4 ++-- drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------ drivers/scsi/mvsas/mv_sas.h | 2 -- 3 files changed, 2 insertions(+), 34 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html