Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
On 09/06/2014 02:52 PM, Tejun Heo wrote: Hello, On Sat, Sep 06, 2014 at 10:21:51AM +0200, Hannes Reinecke wrote: Well, yes, in principle. I was looking into that, too. But then I figured that moving to ata_taskfile would be a major overhaul for libsas, which would be quite beyond scope here. And all for a puny little patch. Hmm? Just allocate a ata_taskfile stuct on stack when invoking the function. The change would be a lot smaller actually. Which was actually my first attempt, but then I figured I'd be increasing the stacksize in doing so. But sure, if you're okay with it I'll be redoing the patch. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
On Sun, Sep 07, 2014 at 01:24:47PM +0200, Hannes Reinecke wrote: Which was actually my first attempt, but then I figured I'd be increasing the stacksize in doing so. But sure, if you're okay with it I'll be redoing the patch. The struct is only 32 bytes. I don't think it's gonna make any meaningful difference. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
On 09/06/2014 01:42 AM, Tejun Heo wrote: Hello, Hannes. Sorry about the delay. On Wed, Jul 30, 2014 at 09:55:08AM +0200, Hannes Reinecke wrote: ata_dev_classify() just uses the 'lbah' and 'lbam' fields from the taskfile, so we can as well use those as arguments and rip out the custom code from sas_ata. I wonder whether it'd easier to just make sas code pass in ata_taskfile instead? The interface which takes three consecutive u8's is kinda error-prone. Well, yes, in principle. I was looking into that, too. But then I figured that moving to ata_taskfile would be a major overhaul for libsas, which would be quite beyond scope here. And all for a puny little patch. --- a/drivers/scsi/aic94xx/aic94xx_task.c +++ b/drivers/scsi/aic94xx/aic94xx_task.c @@ -373,10 +373,10 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task, if (unlikely(task-ata_task.device_control_reg_update)) scb-header.opcode = CONTROL_ATA_DEV; - else if (dev-sata_dev.command_set == ATA_COMMAND_SET) - scb-header.opcode = INITIATE_ATA_TASK; - else + else if (dev-sata_dev.class == ATA_DEV_ATAPI) scb-header.opcode = INITIATE_ATAPI_TASK; + else + scb-header.opcode = INITIATE_ATA_TASK; Are these changes covered by the patch description? Looks like the patch is mixing two separate logical changes. Okay, I'll be splitting them up. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
Hello, On Sat, Sep 06, 2014 at 10:21:51AM +0200, Hannes Reinecke wrote: Well, yes, in principle. I was looking into that, too. But then I figured that moving to ata_taskfile would be a major overhaul for libsas, which would be quite beyond scope here. And all for a puny little patch. Hmm? Just allocate a ata_taskfile stuct on stack when invoking the function. The change would be a lot smaller actually. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] libata: consolidate ata_dev_classify()
ata_dev_classify() just uses the 'lbah' and 'lbam'
fields from the taskfile, so we can as well use those
as arguments and rip out the custom code from sas_ata.
Cc: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libahci.c | 11 +++
drivers/ata/libata-core.c | 14 +
drivers/ata/libata-sff.c| 2 +-
drivers/ata/sata_fsl.c | 11 +++
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_sil24.c| 2 +-
drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
drivers/scsi/isci/request.c | 4 +--
drivers/scsi/libsas/sas_ata.c | 63 +
drivers/scsi/mvsas/mv_sas.c | 4 +--
drivers/scsi/pm8001/pm8001_hwi.c| 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c| 2 +-
include/linux/libata.h | 2 +-
include/scsi/libsas.h | 11 ++-
14 files changed, 44 insertions(+), 96 deletions(-)
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce04..09f9fcb 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1185,16 +1185,15 @@ static void ahci_dev_config(struct ata_device *dev)
unsigned int ahci_dev_classify(struct ata_port *ap)
{
void __iomem *port_mmio = ahci_port_base(ap);
- struct ata_taskfile tf;
+ u8 lbah, lbam, lbal;
u32 tmp;
tmp = readl(port_mmio + PORT_SIG);
- tf.lbah = (tmp 24)0xff;
- tf.lbam = (tmp 16)0xff;
- tf.lbal = (tmp 8) 0xff;
- tf.nsect= (tmp) 0xff;
+ lbah= (tmp 24)0xff;
+ lbam= (tmp 16)0xff;
+ lbal= (tmp 8) 0xff;
- return ata_dev_classify(tf);
+ return ata_dev_classify(lbah, lbam, lbal);
}
EXPORT_SYMBOL_GPL(ahci_dev_classify);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..2c4d742 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1032,7 +1032,9 @@ const char *sata_spd_string(unsigned int spd)
/**
* ata_dev_classify - determine device type based on ATA-spec signature
- * @tf: ATA taskfile register set for device to be identified
+ * @lbah: LBA high byte (LBA bits 23:16)
+ * @lbam: LBA mid byte (LBA bits 15:8)
+ * @lbal: LBA low byte (LBA bits 7:0)
*
* Determine from taskfile register contents whether a device is
* ATA or ATAPI, as per Signature and persistence section
@@ -1045,7 +1047,7 @@ const char *sata_spd_string(unsigned int spd)
* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
* %ATA_DEV_UNKNOWN the event of failure.
*/
-unsigned int ata_dev_classify(const struct ata_taskfile *tf)
+unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
/* Apple's open source Darwin code hints that some devices only
* put a proper signature into the LBA mid/high registers,
@@ -1068,22 +1070,22 @@ unsigned int ata_dev_classify(const struct ata_taskfile
*tf)
* SEMB signature. This is worked around in
* ata_dev_read_id().
*/
- if ((tf-lbam == 0) (tf-lbah == 0)) {
+ if ((lbam == 0) (lbah == 0)) {
DPRINTK(found ATA device by sig\n);
return ATA_DEV_ATA;
}
- if ((tf-lbam == 0x14) (tf-lbah == 0xeb)) {
+ if ((lbam == 0x14) (lbah == 0xeb)) {
DPRINTK(found ATAPI device by sig\n);
return ATA_DEV_ATAPI;
}
- if ((tf-lbam == 0x69) (tf-lbah == 0x96)) {
+ if ((lbam == 0x69) (lbah == 0x96)) {
DPRINTK(found PMP device by sig\n);
return ATA_DEV_PMP;
}
- if ((tf-lbam == 0x3c) (tf-lbah == 0xc3)) {
+ if ((lbam == 0x3c) (lbah == 0xc3)) {
DPRINTK(found SEMB device by sig (could be ATA device)\n);
return ATA_DEV_SEMB;
}
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 1121153..ec9b5db 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1903,7 +1903,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev,
int present,
return ATA_DEV_NONE;
/* determine if device is ATA or ATAPI */
- class = ata_dev_classify(tf);
+ class = ata_dev_classify(tf.lbah, tf.lbam, tf.lbal);
if (class == ATA_DEV_UNKNOWN) {
/* If the device failed diagnostic, it's likely to
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 616a6d2..2194fa0 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -807,8 +807,8 @@ static unsigned int sata_fsl_dev_classify(struct ata_port
*ap)
{
struct sata_fsl_host_priv *host_priv = ap-host-private_data;
void __iomem *hcr_base = host_priv-hcr_base;
- struct ata_taskfile tf;
u32 temp;
+ u8 lbah, lbam, lbal;
temp =
[PATCH 1/4] libata: consolidate ata_dev_classify()
ata_dev_classify() just uses the 'lbah' and 'lbam'
fields from the taskfile, so we can as well use those
as arguments and rip out the custom code from sas_ata.
Cc: Tejun Heo t...@kernel.org
Cc: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libahci.c | 11 +++
drivers/ata/libata-core.c | 14 +
drivers/ata/libata-sff.c| 2 +-
drivers/ata/sata_fsl.c | 11 +++
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_sil24.c| 2 +-
drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
drivers/scsi/isci/request.c | 4 +--
drivers/scsi/libsas/sas_ata.c | 63 +
drivers/scsi/mvsas/mv_sas.c | 4 +--
drivers/scsi/pm8001/pm8001_hwi.c| 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c| 2 +-
include/linux/libata.h | 2 +-
include/scsi/libsas.h | 11 ++-
14 files changed, 44 insertions(+), 96 deletions(-)
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce04..09f9fcb 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1185,16 +1185,15 @@ static void ahci_dev_config(struct ata_device *dev)
unsigned int ahci_dev_classify(struct ata_port *ap)
{
void __iomem *port_mmio = ahci_port_base(ap);
- struct ata_taskfile tf;
+ u8 lbah, lbam, lbal;
u32 tmp;
tmp = readl(port_mmio + PORT_SIG);
- tf.lbah = (tmp 24)0xff;
- tf.lbam = (tmp 16)0xff;
- tf.lbal = (tmp 8) 0xff;
- tf.nsect= (tmp) 0xff;
+ lbah= (tmp 24)0xff;
+ lbam= (tmp 16)0xff;
+ lbal= (tmp 8) 0xff;
- return ata_dev_classify(tf);
+ return ata_dev_classify(lbah, lbam, lbal);
}
EXPORT_SYMBOL_GPL(ahci_dev_classify);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..2c4d742 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1032,7 +1032,9 @@ const char *sata_spd_string(unsigned int spd)
/**
* ata_dev_classify - determine device type based on ATA-spec signature
- * @tf: ATA taskfile register set for device to be identified
+ * @lbah: LBA high byte (LBA bits 23:16)
+ * @lbam: LBA mid byte (LBA bits 15:8)
+ * @lbal: LBA low byte (LBA bits 7:0)
*
* Determine from taskfile register contents whether a device is
* ATA or ATAPI, as per Signature and persistence section
@@ -1045,7 +1047,7 @@ const char *sata_spd_string(unsigned int spd)
* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
* %ATA_DEV_UNKNOWN the event of failure.
*/
-unsigned int ata_dev_classify(const struct ata_taskfile *tf)
+unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
/* Apple's open source Darwin code hints that some devices only
* put a proper signature into the LBA mid/high registers,
@@ -1068,22 +1070,22 @@ unsigned int ata_dev_classify(const struct ata_taskfile
*tf)
* SEMB signature. This is worked around in
* ata_dev_read_id().
*/
- if ((tf-lbam == 0) (tf-lbah == 0)) {
+ if ((lbam == 0) (lbah == 0)) {
DPRINTK(found ATA device by sig\n);
return ATA_DEV_ATA;
}
- if ((tf-lbam == 0x14) (tf-lbah == 0xeb)) {
+ if ((lbam == 0x14) (lbah == 0xeb)) {
DPRINTK(found ATAPI device by sig\n);
return ATA_DEV_ATAPI;
}
- if ((tf-lbam == 0x69) (tf-lbah == 0x96)) {
+ if ((lbam == 0x69) (lbah == 0x96)) {
DPRINTK(found PMP device by sig\n);
return ATA_DEV_PMP;
}
- if ((tf-lbam == 0x3c) (tf-lbah == 0xc3)) {
+ if ((lbam == 0x3c) (lbah == 0xc3)) {
DPRINTK(found SEMB device by sig (could be ATA device)\n);
return ATA_DEV_SEMB;
}
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 1121153..ec9b5db 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1903,7 +1903,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev,
int present,
return ATA_DEV_NONE;
/* determine if device is ATA or ATAPI */
- class = ata_dev_classify(tf);
+ class = ata_dev_classify(tf.lbah, tf.lbam, tf.lbal);
if (class == ATA_DEV_UNKNOWN) {
/* If the device failed diagnostic, it's likely to
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 616a6d2..2194fa0 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -807,8 +807,8 @@ static unsigned int sata_fsl_dev_classify(struct ata_port
*ap)
{
struct sata_fsl_host_priv *host_priv = ap-host-private_data;
void __iomem *hcr_base = host_priv-hcr_base;
- struct ata_taskfile tf;
u32 temp;
+ u8 lbah, lbam,
[PATCH 1/4] libata: consolidate ata_dev_classify()
ata_dev_classify() just uses the 'lbah' and 'lbam'
fields from the taskfile, so we can as well use those
as arguments and rip out the custom code from sas_ata.
Cc: Tejun Heo t...@kernel.org
Cc: Dan Williams dan.j.willi...@intel.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libahci.c | 11 +++
drivers/ata/libata-core.c | 14 +
drivers/ata/libata-sff.c| 2 +-
drivers/ata/sata_fsl.c | 11 +++
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_sil24.c| 2 +-
drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
drivers/scsi/isci/request.c | 4 +--
drivers/scsi/libsas/sas_ata.c | 63 +
drivers/scsi/mvsas/mv_sas.c | 4 +--
drivers/scsi/pm8001/pm8001_hwi.c| 2 +-
drivers/scsi/pm8001/pm80xx_hwi.c| 2 +-
include/linux/libata.h | 2 +-
include/scsi/libsas.h | 11 ++-
14 files changed, 44 insertions(+), 96 deletions(-)
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce04..09f9fcb 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1185,16 +1185,15 @@ static void ahci_dev_config(struct ata_device *dev)
unsigned int ahci_dev_classify(struct ata_port *ap)
{
void __iomem *port_mmio = ahci_port_base(ap);
- struct ata_taskfile tf;
+ u8 lbah, lbam, lbal;
u32 tmp;
tmp = readl(port_mmio + PORT_SIG);
- tf.lbah = (tmp 24)0xff;
- tf.lbam = (tmp 16)0xff;
- tf.lbal = (tmp 8) 0xff;
- tf.nsect= (tmp) 0xff;
+ lbah= (tmp 24)0xff;
+ lbam= (tmp 16)0xff;
+ lbal= (tmp 8) 0xff;
- return ata_dev_classify(tf);
+ return ata_dev_classify(lbah, lbam, lbal);
}
EXPORT_SYMBOL_GPL(ahci_dev_classify);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..2c4d742 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1032,7 +1032,9 @@ const char *sata_spd_string(unsigned int spd)
/**
* ata_dev_classify - determine device type based on ATA-spec signature
- * @tf: ATA taskfile register set for device to be identified
+ * @lbah: LBA high byte (LBA bits 23:16)
+ * @lbam: LBA mid byte (LBA bits 15:8)
+ * @lbal: LBA low byte (LBA bits 7:0)
*
* Determine from taskfile register contents whether a device is
* ATA or ATAPI, as per Signature and persistence section
@@ -1045,7 +1047,7 @@ const char *sata_spd_string(unsigned int spd)
* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
* %ATA_DEV_UNKNOWN the event of failure.
*/
-unsigned int ata_dev_classify(const struct ata_taskfile *tf)
+unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
/* Apple's open source Darwin code hints that some devices only
* put a proper signature into the LBA mid/high registers,
@@ -1068,22 +1070,22 @@ unsigned int ata_dev_classify(const struct ata_taskfile
*tf)
* SEMB signature. This is worked around in
* ata_dev_read_id().
*/
- if ((tf-lbam == 0) (tf-lbah == 0)) {
+ if ((lbam == 0) (lbah == 0)) {
DPRINTK(found ATA device by sig\n);
return ATA_DEV_ATA;
}
- if ((tf-lbam == 0x14) (tf-lbah == 0xeb)) {
+ if ((lbam == 0x14) (lbah == 0xeb)) {
DPRINTK(found ATAPI device by sig\n);
return ATA_DEV_ATAPI;
}
- if ((tf-lbam == 0x69) (tf-lbah == 0x96)) {
+ if ((lbam == 0x69) (lbah == 0x96)) {
DPRINTK(found PMP device by sig\n);
return ATA_DEV_PMP;
}
- if ((tf-lbam == 0x3c) (tf-lbah == 0xc3)) {
+ if ((lbam == 0x3c) (lbah == 0xc3)) {
DPRINTK(found SEMB device by sig (could be ATA device)\n);
return ATA_DEV_SEMB;
}
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 1121153..ec9b5db 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1903,7 +1903,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev,
int present,
return ATA_DEV_NONE;
/* determine if device is ATA or ATAPI */
- class = ata_dev_classify(tf);
+ class = ata_dev_classify(tf.lbah, tf.lbam, tf.lbal);
if (class == ATA_DEV_UNKNOWN) {
/* If the device failed diagnostic, it's likely to
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 616a6d2..2194fa0 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -807,8 +807,8 @@ static unsigned int sata_fsl_dev_classify(struct ata_port
*ap)
{
struct sata_fsl_host_priv *host_priv = ap-host-private_data;
void __iomem *hcr_base = host_priv-hcr_base;
- struct ata_taskfile tf;
u32 temp;
+ u8 lbah, lbam,
