ahci: add "em_buffer" attribute for AHCI hosts
PATCH v3 1/1 ahci: add "em_buffer" attribute for AHCI hosts
Add "em_buffer" attribute for SATA AHCI hosts to provide a way for userland to access AHCI EM (enclosure management) buffer directly if the host supports EM.
AHCI driver should support SGPIO EM messages. However the SATA/AHCI specs did not define the SGPIO message format filled in EM buffer. Different HW vendors may have different definitions. The mainly purpose of this attribute is to solve this issue by allowing HW vendors to provide userland drivers and tools for their SGPIO initiators.
---
v2: rebase this patch on the new upstream branch of linux-next.git
v3: do not use "ahci_em_messages" module parameter as EM message type
control, instead by detecting supported types at initialization. Remove EM message structure shared with userland, insteady by simply writing the raw message into the buffer.
Move write function for this attribute from libata-scsi.c to libachi.c.
Add read function.
Add mecessary definitions for EM in ahci.h.
drivers/ata/ahci.c | 2 +- drivers/ata/ahci.h | 16 ++++--- drivers/ata/libahci.c | 114 +++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 112 insertions(+), 20 deletions(-)
drivers/ata/ahci.c +1 -1
@@ -1185,7 +1185,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)/* set enclosure management message type */ if (ap->flags &ATA_FLAG_EM ) - ap->em_message_type = ahci_em_messages; + ap->em_message_type = hpriv->em_msg_type;/* disabled/not-implemented port */
drivers/ata/ahci.h +10 -6
@@ -220,13 +220,16 @@ enum {ICH_MAP =0x90 ,/* ICH MAP register */ /* em constants */ -EM_MAX_SLOTS =8 , -EM_MAX_RETRY =5 , +EM_MAX_SLOTS =8 , +EM_MAX_RETRY =5 ,/* em_ctl bits */ -EM_CTL_RST = (1 <<9 ),/* Reset */ -EM_CTL_TM = (1 <<8 ),/* Transmit Message */ - EM_CTL_ALHD = (1 << 26), /* Activity LED */ +EM_CTL_RST = (1 <<9 ),/* Reset */ +EM_CTL_TM = (1 <<8 ),/* Transmit Message */ +EM_CTL_MR = (1 <<0 ),/* Message Recieved */ +EM_CTL_ALHD = (1 <<26 ),/* Activity LED */ +EM_CTL_XMT = (1 <<25 ),/* Transmit Only */ + EM_CTL_SMB = (1 << 24), /* Single Message Buffer */ }; struct ahci_cmd_hdr {
@@ -282,9 +285,10 @@ struct ahci_host_priv {u32 saved_cap2;/* saved initial cap2 */ u32 saved_port_map;/* saved initial port_map */ u32 em_loc;/* enclosure management location */ +u32 em_buf_sz;/* EM buffer size in byte*/ +u32 em_msg_type;/* EM message type */ }; -extern int ahci_em_messages;extern int ahci_ignore_sss;extern struct scsi_host_template ahci_sht;
drivers/ata/libahci.c +101 -13
159 lines not on this page · raw message
Signature · 1 line
1.6.4.2
Hello,
Quoted 18 lines · Harry Zhang
drivers/ata/ahci.c +1
-1@@ -1185,7 +1185,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)/* set enclosure management message type */ if (ap->flags &ATA_FLAG_EM ) - ap->em_message_type = ahci_em_messages; + ap->em_message_type = hpriv->em_msg_type;/* disabled/not-implemented port */ drivers/ata/ahci.h +0
-0
drivers/ata/ahci.h enum { · line 224
Quoted 7 lines
drivers/ata/ahci.h +2
-2@@ -220,13 +220,16 @@ enum {ICH_MAP =0x90 ,/* ICH MAP register */ /* em constants */ -EM_MAX_SLOTS =8 , - EM_MAX_RETRY = 5, +EM_MAX_SLOTS =8 , + EM_MAX_RETRY = 5,
Why change the indentation?
drivers/ata/ahci.h struct ahci_host_priv { · line 292
Quoted 13 lines
drivers/ata/ahci.h +2
-1};struct ahci_cmd_hdr {@@ -282,9 +285,10 @@ struct ahci_host_priv {u32 saved_cap2;/* saved initial cap2 */ u32 saved_port_map;/* saved initial port_map */ u32 em_loc;/* enclosure management location */ +u32 em_buf_sz;/* EM buffer size in byte*/ +u32 em_msg_type;/* EM message type */ }; -extern int ahci_em_messages;extern int ahci_ignore_sss;
Please separate out em message enable control changes into a separate patch.
Quoted 10 lines
drivers/ata/ahci.h +10
-0+static ssize_t ahci_read_em_buffer (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct Scsi_Host *shost =class_to_shost (dev); +struct ata_port *ap =ata_shost_to_port (shost); +struct ahci_host_priv *hpriv = ap->host->private_data; +void __iomem *mmio = hpriv->mmio; +void __iomem *em_mmio = mmio + hpriv->em_loc; +u32 em_ctl, msg; +unsigned long flags;
drivers/ata/ahci.h struct ahci_host_priv {
Quoted 10 lines
drivers/ata/ahci.h +10
-0+int i; + +spin_lock_irqsave (ap->lock, flags); + + em_ctl =readl (mmio +HOST_EM_CTL ); +if (!(ap->flags &ATA_FLAG_EM ) || em_ctl &EM_CTL_XMT || + !(em_ctl &EM_CTL_MR )) { +spin_unlock_irqrestore (ap->lock, flags); +return -EINVAL ; + }
Wouldn't !MR be better represented by -EAGAIN?
drivers/ata/ahci.h
Quoted 10 lines
drivers/ata/ahci.h +10
-0+if (!(em_ctl &EM_CTL_SMB )) + em_mmio += hpriv->em_buf_sz; + +for (i =0 ; i < hpriv->em_buf_sz; i +=4 ) { + msg =readl (em_mmio + i); + buf[i] = msg &0xff ; + buf[i +1 ] = (msg >>8 ) &0xff ; + buf[i +2 ] = (msg >>16 ) &0xff ; + buf[i +3 ] = (msg >>24 ) &0xff ; + }
Maybe it would be a good idea to check em_buf_sz against PAGE_SIZE? Hmmm... a question, does the standard define anything about endian?
Other than the above small points, things generally look good to me. Implementing proper r/w blocking semantics would be nice but given that ahci doesn't implement EM interrupts (right?), I think the minimal implementation is okay.
Thanks.
Signature · 1 line
tejun