Re: [patch] ata: ahci: Enclosure Management via LED rev2
On Sat, 01 Dec 2007 18:28:54 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote: > Kristen Carlson Accardi wrote: > > Enclosure Management via LED > > > > This patch implements Enclosure Management via the LED protocol as > > specified in AHCI specification. > > > > Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> > > --- > > This revision makes the change to the comment requested by Mark > > Lord, fixes some bugs in the bit shifting for writing the new led > > state, and implements a show function so that led status can be > > read as well as written. > > Overall looks pretty good, from a technical review perspective. > > Two worries: > > 1) exporting ata_scsi_find_dev(), and assuming a scsi device is > attached. the latter can be fixed by a !NULL check (and should be), > but its a bit of a layering violation since long term we want to make > the SCSI simulator optional for all ATA devices. > > 2) vaguely related to #1, I'm not so sure the attributes should be > implemented directly in ahci. if this __or something like it__ > appears on non-Intel hardware, the code should be somewhere more > generic. > When I first started developing this patch, I did have a more generic approach - It adds lots of complexity that isn't needed for this simple protocol, and one of the problems I encountered was that for different EM protocols, you'd probably want to have a different set of attributes defined. Also, even using the same protocol, you may have hardware that supports more attributes. For example, in the case of this LED protocol, some implementations may support the Activity LED (software controlled), and some may not. For protocols like SGPIO, they have a lot of attributes defined by the spec, but I'm guessing hardware may not support all of them. When I tried to abstract hardware and protocol away and make some kind of generic enclosure management framework, it turned into this big ordeal. So, I can keep going along those lines, but to me it started to seem silly since I had no other hardware that I knew of that was going to be helped by all this. I thought maybe the right thing to do was to keep it simple and then wait for other the hardware to appear. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] ata: ahci: Enclosure Management via LED rev2
On Sat, 01 Dec 2007 18:28:54 -0500 Jeff Garzik [EMAIL PROTECTED] wrote: Kristen Carlson Accardi wrote: Enclosure Management via LED This patch implements Enclosure Management via the LED protocol as specified in AHCI specification. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- This revision makes the change to the comment requested by Mark Lord, fixes some bugs in the bit shifting for writing the new led state, and implements a show function so that led status can be read as well as written. Overall looks pretty good, from a technical review perspective. Two worries: 1) exporting ata_scsi_find_dev(), and assuming a scsi device is attached. the latter can be fixed by a !NULL check (and should be), but its a bit of a layering violation since long term we want to make the SCSI simulator optional for all ATA devices. 2) vaguely related to #1, I'm not so sure the attributes should be implemented directly in ahci. if this __or something like it__ appears on non-Intel hardware, the code should be somewhere more generic. When I first started developing this patch, I did have a more generic approach - It adds lots of complexity that isn't needed for this simple protocol, and one of the problems I encountered was that for different EM protocols, you'd probably want to have a different set of attributes defined. Also, even using the same protocol, you may have hardware that supports more attributes. For example, in the case of this LED protocol, some implementations may support the Activity LED (software controlled), and some may not. For protocols like SGPIO, they have a lot of attributes defined by the spec, but I'm guessing hardware may not support all of them. When I tried to abstract hardware and protocol away and make some kind of generic enclosure management framework, it turned into this big ordeal. So, I can keep going along those lines, but to me it started to seem silly since I had no other hardware that I knew of that was going to be helped by all this. I thought maybe the right thing to do was to keep it simple and then wait for other the hardware to appear. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] ata: ahci: Enclosure Management via LED rev2
Kristen Carlson Accardi wrote: Enclosure Management via LED This patch implements Enclosure Management via the LED protocol as specified in AHCI specification. Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> --- This revision makes the change to the comment requested by Mark Lord, fixes some bugs in the bit shifting for writing the new led state, and implements a show function so that led status can be read as well as written. Overall looks pretty good, from a technical review perspective. Two worries: 1) exporting ata_scsi_find_dev(), and assuming a scsi device is attached. the latter can be fixed by a !NULL check (and should be), but its a bit of a layering violation since long term we want to make the SCSI simulator optional for all ATA devices. 2) vaguely related to #1, I'm not so sure the attributes should be implemented directly in ahci. if this __or something like it__ appears on non-Intel hardware, the code should be somewhere more generic. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] ata: ahci: Enclosure Management via LED rev2
Kristen Carlson Accardi wrote: Enclosure Management via LED This patch implements Enclosure Management via the LED protocol as specified in AHCI specification. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- This revision makes the change to the comment requested by Mark Lord, fixes some bugs in the bit shifting for writing the new led state, and implements a show function so that led status can be read as well as written. Overall looks pretty good, from a technical review perspective. Two worries: 1) exporting ata_scsi_find_dev(), and assuming a scsi device is attached. the latter can be fixed by a !NULL check (and should be), but its a bit of a layering violation since long term we want to make the SCSI simulator optional for all ATA devices. 2) vaguely related to #1, I'm not so sure the attributes should be implemented directly in ahci. if this __or something like it__ appears on non-Intel hardware, the code should be somewhere more generic. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] ata: ahci: Enclosure Management via LED rev2
Enclosure Management via LED
This patch implements Enclosure Management via the LED protocol as specified
in AHCI specification.
Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]>
---
This revision makes the change to the comment requested by Mark Lord,
fixes some bugs in the bit shifting for writing the new led state,
and implements a show function so that led status can be read as
well as written.
drivers/ata/ahci.c| 184 +-
drivers/ata/libata-scsi.c |5 -
include/linux/libata.h|2
3 files changed, 187 insertions(+), 4 deletions(-)
Index: 2.6-git/drivers/ata/ahci.c
===
--- 2.6-git.orig/drivers/ata/ahci.c 2007-11-30 12:04:12.0 -0800
+++ 2.6-git/drivers/ata/ahci.c 2007-11-30 18:02:19.0 -0800
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#define DRV_NAME "ahci"
@@ -92,6 +93,8 @@ enum {
HOST_IRQ_STAT = 0x08, /* interrupt status */
HOST_PORTS_IMPL = 0x0c, /* bitmap of implemented ports */
HOST_VERSION= 0x10, /* AHCI spec. version compliancy */
+ HOST_EM_LOC = 0x1c, /* Enclosure Management location */
+ HOST_EM_CTL = 0x20, /* Enclosure Management Control */
/* HOST_CTL bits */
HOST_RESET = (1 << 0), /* reset controller; self-clear */
@@ -99,6 +102,7 @@ enum {
HOST_AHCI_EN= (1 << 31), /* AHCI enabled */
/* HOST_CAP bits */
+ HOST_CAP_EMS= (1 << 6), /* Enclosure Management support */
HOST_CAP_SSC= (1 << 14), /* Slumber capable */
HOST_CAP_PMP= (1 << 17), /* Port Multiplier support */
HOST_CAP_CLO= (1 << 24), /* Command List Override support */
@@ -193,6 +197,10 @@ enum {
ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
ATA_FLAG_IPM,
AHCI_LFLAG_COMMON = ATA_LFLAG_SKIP_D2H_BSY,
+
+ /* em_ctl bits */
+ EM_CTL_RST = (1 << 9), /* Reset */
+ EM_CTL_TM = (1 << 8), /* Transmit Message */
};
struct ahci_cmd_hdr {
@@ -216,6 +224,7 @@ struct ahci_host_priv {
u32 port_map; /* port map to use */
u32 saved_cap; /* saved initial cap */
u32 saved_port_map; /* saved initial port_map */
+ u32 em_loc; /* enclosure management location */
};
struct ahci_port_priv {
@@ -231,6 +240,7 @@ struct ahci_port_priv {
unsigned intncq_saw_dmas:1;
unsigned intncq_saw_sdb:1;
u32 intr_mask; /* interrupts to enable */
+ u16 led_state; /* saved current led state */
};
static int ahci_scr_read(struct ata_port *ap, unsigned int sc_reg, u32 *val);
@@ -572,6 +582,11 @@ static struct pci_driver ahci_pci_driver
#endif
};
+static int ahci_em_messages = 1;
+module_param(ahci_em_messages, int, 0444);
+/* add other LED protocol types when they become supported */
+MODULE_PARM_DESC(ahci_em_messages,
+ "Set AHCI Enclosure Management Message type (0 = disabled, 1 = LED");
static inline int ahci_nr_ports(u32 cap)
{
@@ -1079,6 +1094,148 @@ static int ahci_reset_controller(struct
return 0;
}
+/** LED Enclosure Management routines /
+static int ahci_reset_em(struct ata_host *host)
+{
+ void __iomem *mmio = host->iomap[AHCI_PCI_BAR];
+ u32 em_ctl;
+
+ em_ctl = readl(mmio + HOST_EM_CTL);
+ if ((em_ctl & EM_CTL_TM) || (em_ctl & EM_CTL_RST))
+ return -EINVAL;
+
+ writel(em_ctl | EM_CTL_RST, mmio + HOST_EM_CTL);
+ return 0;
+}
+
+static int ahci_transmit_led_message(struct ata_port *ap, int led_num,
+ int state)
+{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+ void __iomem *mmio = ap->host->iomap[AHCI_PCI_BAR];
+ struct ahci_port_priv *pp = ap->private_data;
+ u32 em_ctl;
+ u32 message[] = {0, 0};
+ unsigned int flags;
+
+ spin_lock_irqsave(ap->lock, flags);
+
+ /*
+* if we are still busy transmitting a previous message,
+* do not allow
+*/
+ em_ctl = readl(mmio + HOST_EM_CTL);
+ if (em_ctl & EM_CTL_TM) {
+ spin_unlock_irqrestore(ap->lock, flags);
+ return -EINVAL;
+ }
+
+ /*
+* create message header - this is all zero except for
+* the message size, which is 4 bytes.
+*/
+ message[0] |= (4 << 8);
+
+ pp->led_state &= ~(7 << (3*led_num));
+
+ /*
+* create the actual message
+* This does not support Port Multiplier at this time
+* due to lack of hardware for
Re: [patch] ata: ahci: Enclosure Management via LED rev2
Enclosure Management via LED
This patch implements Enclosure Management via the LED protocol as specified
in AHCI specification.
Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
---
This revision makes the change to the comment requested by Mark Lord,
fixes some bugs in the bit shifting for writing the new led state,
and implements a show function so that led status can be read as
well as written.
drivers/ata/ahci.c| 184 +-
drivers/ata/libata-scsi.c |5 -
include/linux/libata.h|2
3 files changed, 187 insertions(+), 4 deletions(-)
Index: 2.6-git/drivers/ata/ahci.c
===
--- 2.6-git.orig/drivers/ata/ahci.c 2007-11-30 12:04:12.0 -0800
+++ 2.6-git/drivers/ata/ahci.c 2007-11-30 18:02:19.0 -0800
@@ -44,6 +44,7 @@
#include linux/dmi.h
#include scsi/scsi_host.h
#include scsi/scsi_cmnd.h
+#include scsi/scsi_device.h
#include linux/libata.h
#define DRV_NAME ahci
@@ -92,6 +93,8 @@ enum {
HOST_IRQ_STAT = 0x08, /* interrupt status */
HOST_PORTS_IMPL = 0x0c, /* bitmap of implemented ports */
HOST_VERSION= 0x10, /* AHCI spec. version compliancy */
+ HOST_EM_LOC = 0x1c, /* Enclosure Management location */
+ HOST_EM_CTL = 0x20, /* Enclosure Management Control */
/* HOST_CTL bits */
HOST_RESET = (1 0), /* reset controller; self-clear */
@@ -99,6 +102,7 @@ enum {
HOST_AHCI_EN= (1 31), /* AHCI enabled */
/* HOST_CAP bits */
+ HOST_CAP_EMS= (1 6), /* Enclosure Management support */
HOST_CAP_SSC= (1 14), /* Slumber capable */
HOST_CAP_PMP= (1 17), /* Port Multiplier support */
HOST_CAP_CLO= (1 24), /* Command List Override support */
@@ -193,6 +197,10 @@ enum {
ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
ATA_FLAG_IPM,
AHCI_LFLAG_COMMON = ATA_LFLAG_SKIP_D2H_BSY,
+
+ /* em_ctl bits */
+ EM_CTL_RST = (1 9), /* Reset */
+ EM_CTL_TM = (1 8), /* Transmit Message */
};
struct ahci_cmd_hdr {
@@ -216,6 +224,7 @@ struct ahci_host_priv {
u32 port_map; /* port map to use */
u32 saved_cap; /* saved initial cap */
u32 saved_port_map; /* saved initial port_map */
+ u32 em_loc; /* enclosure management location */
};
struct ahci_port_priv {
@@ -231,6 +240,7 @@ struct ahci_port_priv {
unsigned intncq_saw_dmas:1;
unsigned intncq_saw_sdb:1;
u32 intr_mask; /* interrupts to enable */
+ u16 led_state; /* saved current led state */
};
static int ahci_scr_read(struct ata_port *ap, unsigned int sc_reg, u32 *val);
@@ -572,6 +582,11 @@ static struct pci_driver ahci_pci_driver
#endif
};
+static int ahci_em_messages = 1;
+module_param(ahci_em_messages, int, 0444);
+/* add other LED protocol types when they become supported */
+MODULE_PARM_DESC(ahci_em_messages,
+ Set AHCI Enclosure Management Message type (0 = disabled, 1 = LED);
static inline int ahci_nr_ports(u32 cap)
{
@@ -1079,6 +1094,148 @@ static int ahci_reset_controller(struct
return 0;
}
+/** LED Enclosure Management routines /
+static int ahci_reset_em(struct ata_host *host)
+{
+ void __iomem *mmio = host-iomap[AHCI_PCI_BAR];
+ u32 em_ctl;
+
+ em_ctl = readl(mmio + HOST_EM_CTL);
+ if ((em_ctl EM_CTL_TM) || (em_ctl EM_CTL_RST))
+ return -EINVAL;
+
+ writel(em_ctl | EM_CTL_RST, mmio + HOST_EM_CTL);
+ return 0;
+}
+
+static int ahci_transmit_led_message(struct ata_port *ap, int led_num,
+ int state)
+{
+ struct ahci_host_priv *hpriv = ap-host-private_data;
+ void __iomem *mmio = ap-host-iomap[AHCI_PCI_BAR];
+ struct ahci_port_priv *pp = ap-private_data;
+ u32 em_ctl;
+ u32 message[] = {0, 0};
+ unsigned int flags;
+
+ spin_lock_irqsave(ap-lock, flags);
+
+ /*
+* if we are still busy transmitting a previous message,
+* do not allow
+*/
+ em_ctl = readl(mmio + HOST_EM_CTL);
+ if (em_ctl EM_CTL_TM) {
+ spin_unlock_irqrestore(ap-lock, flags);
+ return -EINVAL;
+ }
+
+ /*
+* create message header - this is all zero except for
+* the message size, which is 4 bytes.
+*/
+ message[0] |= (4 8);
+
+ pp-led_state = ~(7 (3*led_num));
+
+ /*
+* create the actual message
+* This does not support Port Multiplier at this time
+
