[PATCHv4 0/4] Initial SMR drive support

[Hannes Reinecke]

This is a first stab at implementing SMR support.
The powers that be decided to call the ATA implementation
'ZAC' (zoned access commands), and the SCSI implementation
'ZBC' (zoned block commands).

This is just basic enablement to get ZAC and ZBC drives
handled correctly.
The first three patches update the libata SATL to handle
ZAC devices correctly, and the last patch updates the 'sd'
to work correctly with ZBC devices.
The 'sd' driver will not automatically bind to ZBC
devices; for testing purposes I have added a
module parameter 'attach_zbc' to the 'sd' driver.
This allows for easy testing of ZBC devices.

None of the specific commands like 'report zones' or
'reset write pointer' have been implemented yet as the
actual format is still not finalized.

This patch is made against the core-for-3.17 tree from hch.
Changes to v3:
- Remove setting of HAW_ZBC flag

Hannes Reinecke (4):
libata: consolidate ata_dev_classify()
libata: Implement ATA_DEV_ZAC
libata-scsi: Update SATL for ZAC drives
sd: Optionally attach to ZBC devices

drivers/ata/libahci.c | 11 +++----
drivers/ata/libata-core.c | 34 +++++++++++++-------
drivers/ata/libata-eh.c | 7 +++--
drivers/ata/libata-scsi.c | 30 ++++++++++++++++--
drivers/ata/libata-sff.c | 2 +-
drivers/ata/libata-transport.c | 1 +
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 +-
drivers/scsi/sd.c | 25 +++++++++++----
include/linux/libata.h | 8 +++--
include/scsi/libsas.h | 11 ++-----
18 files changed, 114 insertions(+), 115 deletions(-)

[Hannes Reinecke]

ZAC (zoned-access command) drives translate into ZBC (Zoned block
command) device type for SCSI. So implement the correct mappings
into libata-scsi and update the SCSI command set versions.

Signed-off-by: Hannes Reinecke <***@suse.de>
---
drivers/ata/libata-scsi.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bea6e7f..1db6eab 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1969,6 +1969,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
{
const u8 versions[] = {
+ 0x00,
0x60, /* SAM-3 (no version claimed) */

0x03,
@@ -1977,6 +1978,20 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
0x02,
0x60 /* SPC-3 (no version claimed) */
};
+ const u8 versions_zbc[] = {
+ 0x00,
+ 0xA0, /* SAM-5 (no version claimed) */
+
+ 0x04,
+ 0xC0, /* SBC-3 (no version claimed) */
+
+ 0x04,
+ 0x60, /* SPC-4 (no version claimed) */
+
+ 0x60,
+ 0x20, /* ZBC (no version claimed) */
+ };
+
u8 hdr[] = {
TYPE_DISK,
0,
@@ -1991,6 +2006,11 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
if (ata_id_removeable(args->id))
hdr[1] |= (1 << 7);

+ if (args->dev->class == ATA_DEV_ZAC) {
+ hdr[0] = TYPE_ZBC;
+ hdr[2] = 0x6; /* ZBC is defined in SPC-4 */
+ }
+
memcpy(rbuf, hdr, sizeof(hdr));
memcpy(&rbuf[8], "ATA ", 8);
ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
@@ -2003,7 +2023,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
if (rbuf[32] == 0 || rbuf[32] == ' ')
memcpy(&rbuf[32], "n/a ", 4);

- memcpy(rbuf + 59, versions, sizeof(versions));
+ if (args->dev->class == ATA_DEV_ZAC)
+ memcpy(rbuf + 58, versions_zbc, sizeof(versions_zbc));
+ else
+ memcpy(rbuf + 58, versions, sizeof(versions));

return 0;
}

[Tejun Heo]

Post by Hannes Reinecke
ZAC (zoned-access command) drives translate into ZBC (Zoned block
command) device type for SCSI. So implement the correct mappings
into libata-scsi and update the SCSI command set versions.

Patch 2-3 look good to me.

Thanks.

[Hannes Reinecke]

ZBC drives are close to disk devices, so sd.c is well
suited as a testbed for ZBC devices.
This patch introduces a module option 'attach_zbc' to
sd which will make the sd driver accept ZBC
devices as normal disk drives.

Signed-off-by: Hannes Reinecke <***@suse.de>
---
drivers/scsi/sd.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 377a520..f957a85 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -91,6 +91,13 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR);
MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK);
MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
+MODULE_ALIAS_SCSI_DEVICE(TYPE_ZBC);
+
+static bool sd_attach_zbc;
+
+module_param_named(attach_zbc, sd_attach_zbc, bool, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(attach_zbc,
+ "Attach to ZBC devices (default=0)");

#if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
#define SD_MINORS 16
@@ -161,7 +168,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr,
static const char temp[] = "temporary ";
int len;

- if (sdp->type != TYPE_DISK)
+ if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
/* no cache control on RBC devices; theoretically they
* can do it, but there's probably so many exceptions
* it's not worth the risk */
@@ -259,7 +266,7 @@ allow_restart_store(struct device *dev, struct device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

- if (sdp->type != TYPE_DISK)
+ if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;

sdp->allow_restart = simple_strtoul(buf, NULL, 10);
@@ -389,7 +396,7 @@ provisioning_mode_store(struct device *dev, struct device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

- if (sdp->type != TYPE_DISK)
+ if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;

if (!strncmp(buf, lbp_mode[SD_LBP_UNMAP], 20))
@@ -456,7 +463,7 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

- if (sdp->type != TYPE_DISK)
+ if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;

err = kstrtoul(buf, 10, &max);
@@ -2537,7 +2544,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
struct scsi_mode_data data;
struct scsi_sense_hdr sshdr;

- if (sdp->type != TYPE_DISK)
+ if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return;

if (sdkp->protection_type == 0)
@@ -2957,7 +2964,13 @@ static int sd_probe(struct device *dev)
int error;

error = -ENODEV;
- if (sdp->type != TYPE_DISK && sdp->type != TYPE_MOD && sdp->type != TYPE_RBC)
+ if (sdp->type != TYPE_DISK &&
+ sdp->type != TYPE_MOD &&
+ sdp->type != TYPE_RBC &&
+ sdp->type != TYPE_ZBC)
+ goto out;
+
+ if (sdp->type == TYPE_ZBC && !sd_attach_zbc)
goto out;

SCSI_LOG_HLQUEUE(3, sdev_printk(KERN_INFO, sdp,

[Hannes Reinecke]

Add new ATA device type for ZAC devices.

Signed-off-by: Hannes Reinecke <***@suse.de>
---
drivers/ata/libata-core.c | 20 ++++++++++++++------
drivers/ata/libata-eh.c | 7 +++++--
drivers/ata/libata-scsi.c | 5 +++--
drivers/ata/libata-transport.c | 1 +
include/linux/libata.h | 6 ++++--
5 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2c4d742..5f23804 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1044,8 +1044,8 @@ const char *sata_spd_string(unsigned int spd)
* None.
*
* RETURNS:
- * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
- * %ATA_DEV_UNKNOWN the event of failure.
+ * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP,
+ * %ATA_DEV_ZAC, or %ATA_DEV_UNKNOWN the event of failure.
*/
unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
{
@@ -1090,6 +1090,11 @@ unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
return ATA_DEV_SEMB;
}

+ if ((lbam == 0xcd) && (lbah == 0xab)) {
+ DPRINTK("found ZAC device by sig\n");
+ return ATA_DEV_ZAC;
+ }
+
DPRINTK("unknown device\n");
return ATA_DEV_UNKNOWN;
}
@@ -1330,7 +1335,7 @@ static int ata_hpa_resize(struct ata_device *dev)
int rc;

/* do we need to do it? */
- if (dev->class != ATA_DEV_ATA ||
+ if ((dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) ||
!ata_id_has_lba(dev->id) || !ata_id_hpa_enabled(dev->id) ||
(dev->horkage & ATA_HORKAGE_BROKEN_HPA))
return 0;
@@ -1890,6 +1895,7 @@ retry:
case ATA_DEV_SEMB:
class = ATA_DEV_ATA; /* some hard drives report SEMB sig */
case ATA_DEV_ATA:
+ case ATA_DEV_ZAC:
tf.command = ATA_CMD_ID_ATA;
break;
case ATA_DEV_ATAPI:
@@ -1981,7 +1987,7 @@ retry:
rc = -EINVAL;
reason = "device reports invalid type";

- if (class == ATA_DEV_ATA) {
+ if (class == ATA_DEV_ATA || class == ATA_DEV_ZAC) {
if (!ata_id_is_ata(id) && !ata_id_is_cfa(id))
goto err_out;
if (ap->host->flags & ATA_HOST_IGNORE_ATA &&
@@ -2016,7 +2022,8 @@ retry:
goto retry;
}

- if ((flags & ATA_READID_POSTRESET) && class == ATA_DEV_ATA) {
+ if ((flags & ATA_READID_POSTRESET) &&
+ (class == ATA_DEV_ATA || class == ATA_DEV_ZAC)) {
/*
* The exact sequence expected by certain pre-ATA4 drives is:
* SRST RESET
@@ -2281,7 +2288,7 @@ int ata_dev_configure(struct ata_device *dev)
sizeof(modelbuf));

/* ATA-specific feature tests */
- if (dev->class == ATA_DEV_ATA) {
+ if (dev->class == ATA_DEV_ATA || dev->class == ATA_DEV_ZAC) {
if (ata_id_is_cfa(id)) {
/* CPRM may make this media unusable */
if (id[ATA_ID_CFA_KEY_MGMT] & 1)
@@ -4034,6 +4041,7 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
if (ata_class_enabled(new_class) &&
new_class != ATA_DEV_ATA &&
new_class != ATA_DEV_ATAPI &&
+ new_class != ATA_DEV_ZAC &&
new_class != ATA_DEV_SEMB) {
ata_dev_info(dev, "class mismatch %u != %u\n",
dev->class, new_class);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 6760fc4..f620c4a 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1809,6 +1809,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,

switch (qc->dev->class) {
case ATA_DEV_ATA:
+ case ATA_DEV_ZAC:
if (err & ATA_ICRC)
qc->err_mask |= AC_ERR_ATA_BUS;
if (err & ATA_UNC)
@@ -3791,7 +3792,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
struct ata_eh_context *ehc = &link->eh_context;
unsigned long tmp;

- if (dev->class != ATA_DEV_ATA)
+ if (dev->class != ATA_DEV_ATA &&
+ dev->class != ATA_DEV_ZAC)
continue;
if (!(ehc->i.dev_action[dev->devno] &
ATA_EH_PARK))
@@ -3872,7 +3874,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,

/* retry flush if necessary */
ata_for_each_dev(dev, link, ALL) {
- if (dev->class != ATA_DEV_ATA)
+ if (dev->class != ATA_DEV_ATA &&
+ dev->class != ATA_DEV_ZAC)
continue;
rc = ata_eh_maybe_retry_flush(dev);
if (rc)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0586f66..bea6e7f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -235,7 +235,8 @@ static ssize_t ata_scsi_park_store(struct device *device,
rc = -ENODEV;
goto unlock;
}
- if (dev->class != ATA_DEV_ATA) {
+ if (dev->class != ATA_DEV_ATA &&
+ dev->class != ATA_DEV_ZAC) {
rc = -EOPNOTSUPP;
goto unlock;
}
@@ -3412,7 +3413,7 @@ static inline int __ata_scsi_queuecmd(struct scsi_cmnd *scmd,
ata_xlat_func_t xlat_func;
int rc = 0;

- if (dev->class == ATA_DEV_ATA) {
+ if (dev->class == ATA_DEV_ATA || dev->class == ATA_DEV_ZAC) {
if (unlikely(!scmd->cmd_len || scmd->cmd_len > dev->cdb_len))
goto bad_cdb_len;

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index e37413228..3227b7c 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -143,6 +143,7 @@ static struct {
{ ATA_DEV_PMP_UNSUP, "pmp" },
{ ATA_DEV_SEMB, "semb" },
{ ATA_DEV_SEMB_UNSUP, "semb" },
+ { ATA_DEV_ZAC, "zac" },
{ ATA_DEV_NONE, "none" }
};
ata_bitfield_name_search(class, ata_class_names)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3412aee..37f482b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -191,7 +191,8 @@ enum {
ATA_DEV_PMP_UNSUP = 6, /* SATA port multiplier (unsupported) */
ATA_DEV_SEMB = 7, /* SEMB */
ATA_DEV_SEMB_UNSUP = 8, /* SEMB (unsupported) */
- ATA_DEV_NONE = 9, /* no device */
+ ATA_DEV_ZAC = 9, /* ZAC device */
+ ATA_DEV_NONE = 10, /* no device */

/* struct ata_link flags */
ATA_LFLAG_NO_HRST = (1 << 1), /* avoid hardreset */
@@ -1490,7 +1491,8 @@ static inline unsigned int ata_tag_internal(unsigned int tag)
static inline unsigned int ata_class_enabled(unsigned int class)
{
return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI ||
- class == ATA_DEV_PMP || class == ATA_DEV_SEMB;
+ class == ATA_DEV_PMP || class == ATA_DEV_SEMB ||
+ class == ATA_DEV_ZAC;
}

static inline unsigned int ata_class_disabled(unsigned int class)

[Hannes Reinecke]

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 <***@intel.com>
Signed-off-by: Hannes Reinecke <***@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 = ioread32(hcr_base + SIGNATURE);

@@ -816,12 +816,11 @@ static unsigned int sata_fsl_dev_classify(struct ata_port *ap)
VPRINTK("HStatus = 0x%x\n", ioread32(hcr_base + HSTATUS));
VPRINTK("HControl = 0x%x\n", ioread32(hcr_base + HCONTROL));

- tf.lbah = (temp >> 24) & 0xff;
- tf.lbam = (temp >> 16) & 0xff;
- tf.lbal = (temp >> 8) & 0xff;
- tf.nsect = temp & 0xff;
+ lbah = (temp >> 24) & 0xff;
+ lbam = (temp >> 16) & 0xff;
+ lbal = (temp >> 8) & 0xff;

- return ata_dev_classify(&tf);
+ return ata_dev_classify(lbah, lbam, lbal);
}

static int sata_fsl_hardreset(struct ata_link *link, unsigned int *class,
diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index 0698278..7d95cc4 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -649,7 +649,7 @@ static int inic_hardreset(struct ata_link *link, unsigned int *class,
}

inic_tf_read(ap, &tf);
- *class = ata_dev_classify(&tf);
+ *class = ata_dev_classify(tf. lbah, tf.lbam, tf.lbal);
}

return 0;
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 0534890..c69ebaf 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -686,7 +686,7 @@ static int sil24_softreset(struct ata_link *link, unsigned int *class,
}

sil24_read_tf(ap, 0, &tf);
- *class = ata_dev_classify(&tf);
+ *class = ata_dev_classify(tf. lbah, tf.lbam, tf.lbal);

DPRINTK("EXIT, class=%u\n", *class);
return 0;
diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index 59b86e2..7abbe42 100644
--- 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;

scb->ata_task.proto_conn_rate = (1 << 5); /* STP */
if (dev->port->oob_mode == SAS_OOB_MODE)
@@ -387,7 +387,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task,
if (likely(!task->ata_task.device_control_reg_update))
scb->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
scb->ata_task.fis.flags &= 0xF0; /* PM_PORT field shall be 0 */
- if (dev->sata_dev.command_set == ATAPI_COMMAND_SET)
+ if (dev->sata_dev.class == ATA_DEV_ATAPI)
memcpy(scb->ata_task.atapi_packet, task->ata_task.atapi_packet,
16);
scb->ata_task.sister_scb = cpu_to_le16(0xFFFF);
@@ -399,7 +399,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task,
if (task->ata_task.dma_xfer)
flags |= DATA_XFER_MODE_DMA;
if (task->ata_task.use_ncq &&
- dev->sata_dev.command_set != ATAPI_COMMAND_SET)
+ dev->sata_dev.class != ATA_DEV_ATAPI)
flags |= ATA_Q_TYPE_NCQ;
flags |= data_dir_flags[task->data_dir];
scb->ata_task.ata_flags = flags;
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 56e3809..cfd0084 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -694,7 +694,7 @@ sci_io_request_construct_sata(struct isci_request *ireq,
}

/* ATAPI */
- if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+ if (dev->sata_dev.class == ATA_DEV_ATAPI &&
task->ata_task.fis.command == ATA_CMD_PACKET) {
sci_atapi_construct(ireq);
return SCI_SUCCESS;
@@ -2980,7 +2980,7 @@ static void sci_request_started_state_enter(struct sci_base_state_machine *sm)
state = SCI_REQ_SMP_WAIT_RESP;
} else if (task && sas_protocol_ata(task->task_proto) &&
!task->ata_task.use_ncq) {
- if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+ if (dev->sata_dev.class == ATA_DEV_ATAPI &&
task->ata_task.fis.command == ATA_CMD_PACKET) {
state = SCI_REQ_ATAPI_WAIT_H2D;
} else if (task->data_dir == DMA_NONE) {
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 766098a..865de86 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -138,7 +138,7 @@ static void sas_ata_task_done(struct sas_task *task)

if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_STAT_GOOD ||
((stat->stat == SAM_STAT_CHECK_CONDITION &&
- dev->sata_dev.command_set == ATAPI_COMMAND_SET))) {
+ dev->sata_dev.class == ATA_DEV_ATAPI))) {
memcpy(dev->sata_dev.fis, resp->ending_fis, ATA_RESP_FIS_SIZE);

if (!link->sactive) {
@@ -278,7 +278,7 @@ static struct sas_internal *dev_to_sas_internal(struct domain_device *dev)
return to_sas_internal(dev->port->ha->core.shost->transportt);
}

-static void sas_get_ata_command_set(struct domain_device *dev);
+static int sas_get_ata_command_set(struct domain_device *dev);

int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
{
@@ -303,8 +303,7 @@ int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
}
memcpy(dev->frame_rcvd, &dev->sata_dev.rps_resp.rps.fis,
sizeof(struct dev_to_host_fis));
- /* TODO switch to ata_dev_classify() */
- sas_get_ata_command_set(dev);
+ dev->sata_dev.class = sas_get_ata_command_set(dev);
}
return 0;
}
@@ -425,18 +424,7 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
if (ret && ret != -EAGAIN)
sas_ata_printk(KERN_ERR, dev, "reset failed (errno=%d)\n", ret);

- /* XXX: if the class changes during the reset the upper layer
- * should be informed, if the device has gone away we assume
- * libsas will eventually delete it
- */
- switch (dev->sata_dev.command_set) {
- case ATA_COMMAND_SET:
- *class = ATA_DEV_ATA;
- break;
- case ATAPI_COMMAND_SET:
- *class = ATA_DEV_ATAPI;
- break;
- }
+ *class = dev->sata_dev.class;

ap->cbl = ATA_CBL_SATA;
return ret;
@@ -626,50 +614,15 @@ void sas_ata_task_abort(struct sas_task *task)
complete(waiting);
}

-static void sas_get_ata_command_set(struct domain_device *dev)
+static int sas_get_ata_command_set(struct domain_device *dev)
{
struct dev_to_host_fis *fis =
(struct dev_to_host_fis *) dev->frame_rcvd;

if (dev->dev_type == SAS_SATA_PENDING)
- return;
+ return ATA_DEV_UNKNOWN;

- if ((fis->sector_count == 1 && /* ATA */
- fis->lbal == 1 &&
- fis->lbam == 0 &&
- fis->lbah == 0 &&
- fis->device == 0)
- ||
- (fis->sector_count == 0 && /* CE-ATA (mATA) */
- fis->lbal == 0 &&
- fis->lbam == 0xCE &&
- fis->lbah == 0xAA &&
- (fis->device & ~0x10) == 0))
-
- dev->sata_dev.command_set = ATA_COMMAND_SET;
-
- else if ((fis->interrupt_reason == 1 && /* ATAPI */
- fis->lbal == 1 &&
- fis->byte_count_low == 0x14 &&
- fis->byte_count_high == 0xEB &&
- (fis->device & ~0x10) == 0))
-
- dev->sata_dev.command_set = ATAPI_COMMAND_SET;
-
- else if ((fis->sector_count == 1 && /* SEMB */
- fis->lbal == 1 &&
- fis->lbam == 0x3C &&
- fis->lbah == 0xC3 &&
- fis->device == 0)
- ||
- (fis->interrupt_reason == 1 && /* SATA PM */
- fis->lbal == 1 &&
- fis->byte_count_low == 0x69 &&
- fis->byte_count_high == 0x96 &&
- (fis->device & ~0x10) == 0))
-
- /* Treat it as a superset? */
- dev->sata_dev.command_set = ATAPI_COMMAND_SET;
+ return ata_dev_classify(fis->lbah, fis->lbam, fis->lbal);
}

void sas_probe_sata(struct asd_sas_port *port)
@@ -775,7 +728,7 @@ int sas_discover_sata(struct domain_device *dev)
if (dev->dev_type == SAS_SATA_PM)
return -ENODEV;

- sas_get_ata_command_set(dev);
+ dev->sata_dev.class = sas_get_ata_command_set(dev);
sas_fill_in_rphy(dev, dev->rphy);

res = sas_notify_lldd_dev_found(dev);
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 6c1f223..44625b8 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -479,7 +479,7 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,

if (task->ata_task.use_ncq)
flags |= MCH_FPDMA;
- if (dev->sata_dev.command_set == ATAPI_COMMAND_SET) {
+ if (dev->sata_dev.class == ATA_DEV_ATAPI) {
if (task->ata_task.fis.command != ATA_CMD_ID_ATAPI)
flags |= MCH_ATAPI;
}
@@ -546,7 +546,7 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
/* fill in command FIS and ATAPI CDB */
memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
- if (dev->sata_dev.command_set == ATAPI_COMMAND_SET)
+ if (dev->sata_dev.class == ATA_DEV_ATAPI)
memcpy(buf_cmd + STP_ATAPI_CMD,
task->ata_task.atapi_packet, 16);

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index a97be01..70f9b45 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -4356,7 +4356,7 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO\n"));
}
if (task->ata_task.use_ncq &&
- dev->sata_dev.command_set != ATAPI_COMMAND_SET) {
+ dev->sata_dev.class != ATA_DEV_ATAPI) {
ATAP = 0x07; /* FPDMA */
PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA\n"));
}
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index d70587f..b1f47f7 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4059,7 +4059,7 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO\n"));
}
if (task->ata_task.use_ncq &&
- dev->sata_dev.command_set != ATAPI_COMMAND_SET) {
+ dev->sata_dev.class != ATA_DEV_ATAPI) {
ATAP = 0x07; /* FPDMA */
PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA\n"));
}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..3412aee 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1172,7 +1172,7 @@ extern int ata_std_qc_defer(struct ata_queued_cmd *qc);
extern void ata_noop_qc_prep(struct ata_queued_cmd *qc);
extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg,
unsigned int n_elem);
-extern unsigned int ata_dev_classify(const struct ata_taskfile *tf);
+extern unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal);
extern void ata_dev_disable(struct ata_device *adev);
extern void ata_id_string(const u16 *id, unsigned char *s,
unsigned int ofs, unsigned int len);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ef7872c..8528a09 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -161,17 +161,12 @@ struct expander_device {
};

/* ---------- SATA device ---------- */
-enum ata_command_set {
- ATA_COMMAND_SET = 0,
- ATAPI_COMMAND_SET = 1,
-};
-
#define ATA_RESP_FIS_SIZE 24

struct sata_device {
- enum ata_command_set command_set;
- struct smp_resp rps_resp; /* report_phy_sata_resp */
- u8 port_no; /* port number, if this is a PM (Port) */
+ unsigned int class;
+ struct smp_resp rps_resp; /* report_phy_sata_resp */
+ u8 port_no; /* port number, if this is a PM (Port) */

struct ata_port *ap;
struct ata_host ata_host;

[Tejun Heo]

Hello, Hannes.

Sorry about the delay.

Post by Hannes Reinecke
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.

Post by Hannes Reinecke
--- 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.

Thanks.

[Hannes Reinecke]

Post by Tejun Heo
Hello, Hannes.
Sorry about the delay.

Post by Hannes Reinecke
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 overhau=
l=20
for libsas, which would be quite beyond scope here.
And all for a puny little patch.

Post by Tejun Heo

Post by Hannes Reinecke
--- 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,

Post by Tejun Heo

Post by Hannes Reinecke
if (unlikely(task->ata_task.device_control_reg_update))
scb->header.opcode =3D CONTROL_ATA_DEV;
- else if (dev->sata_dev.command_set =3D=3D ATA_COMMAND_SET)
- scb->header.opcode =3D INITIATE_ATA_TASK;
- else
+ else if (dev->sata_dev.class =3D=3D ATA_DEV_ATAPI)
scb->header.opcode =3D INITIATE_ATAPI_TASK;
+ else
+ scb->header.opcode =3D 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
--=20
Dr. Hannes Reinecke zSeries & Storage
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tejun Heo]

Hello,

Post by Hannes Reinecke
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.

[Hannes Reinecke]

Post by Tejun Heo
Hello,

Post by Hannes Reinecke
Well, yes, in principle. I was looking into that, too.
But then I figured that moving to ata_taskfile would be a major over=

haul for

Post by Tejun Heo

Post by Hannes Reinecke
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
--=20
Dr. Hannes Reinecke zSeries & Storage
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tejun Heo]

Post by Hannes Reinecke
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.