RE: [PATCH 5/5] SES: Add power_status to SES enclosure component
New patch in next email...
Change to let power_status show on or off
Initialization of desc was removed because it will be initialized in
init_device_slot_control, so no need to initialize at define.
Thanks,
Song
-Original Message-
From: Hannes Reinecke [mailto:h...@suse.de]
Sent: Thursday, September 4, 2014 12:59 AM
To: Song Liu; linux-scsi@vger.kernel.org
Cc: Dan Williams; Jens Axboe
Subject: Re: [PATCH 5/5] SES: Add power_status to SES enclosure component
On 08/25/2014 07:34 PM, Song Liu wrote:
From: Song Liu [mailto:songliubrav...@fb.com]
Sent: Monday, August 25, 2014 10:26 AM
To: Song Liu
Cc: Hannes Reinecke
Subject: [PATCH 5/5] SES: Add power_status to SES enclosure component
Add power_status to SES enclosure component, so we can power on/off
the HDDs behind the enclosure.
Check firmware status in ses_set_* before sending control pages to
firmware.
Signed-off-by: Song Liu songliubrav...@fb.com
Acked-by: Dan Williams dan.j.willi...@intel.com
Reviewed-by: Jens Axboe ax...@fb.com
Cc: Hannes Reinecke h...@suse.de
---
drivers/misc/enclosure.c | 29 ++
drivers/scsi/ses.c| 98
++-
include/linux/enclosure.h | 6 +++
3 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
de335bc..0331dfe 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -148,6 +148,7 @@ enclosure_register(struct device *dev, const char
*name, int components,
for (i = 0; i components; i++) {
edev-component[i].number = -1;
edev-component[i].slot = -1;
+ edev-component[i].power_status = 1;
}
mutex_lock(container_list_lock);
@@ -546,6 +547,31 @@ static ssize_t set_component_locate(struct
device *cdev,
return count;
}
+static ssize_t get_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp =
to_enclosure_component(cdev);
+
+ if (edev-cb-get_power_status)
+ edev-cb-get_power_status(edev, ecomp);
+ return snprintf(buf, 40, %d\n, ecomp-power_status); }
+
+static ssize_t set_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ const char *buf, size_t count) {
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp =
to_enclosure_component(cdev);
+ int val = simple_strtoul(buf, NULL, 0);
+
+ if (edev-cb-set_power_status)
+ edev-cb-set_power_status(edev, ecomp, val);
+ return count;
+}
+
Just using a number here doesn't seem to be very instructive; what is this
number? Can't we have a decode setting here to allow even the uninitiated
to do some sensible decisions here?
static ssize_t get_component_type(struct device *cdev,
struct device_attribute *attr, char *buf)
{ @@ -577,6 +603,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR,
get_component_active,
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
set_component_locate);
+static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR,
get_component_power_status,
+ set_component_power_status);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL); static
DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
@@ -585,6 +613,7 @@ static struct attribute
*enclosure_component_attrs[] = {
dev_attr_status.attr,
dev_attr_active.attr,
dev_attr_locate.attr,
+ dev_attr_power_status.attr,
dev_attr_type.attr,
dev_attr_slot.attr,
NULL
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index
bafa301..ea6b262 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -67,6 +67,20 @@ static int ses_probe(struct device *dev) #define
SES_TIMEOUT (30 * HZ) #define SES_RETRIES 3
+static void init_device_slot_control(unsigned char *dest_desc,
+struct enclosure_component *ecomp,
+unsigned char *status)
+{
+ memcpy(dest_desc, status, 4);
+ dest_desc[0] = 0;
+ /* only clear byte 1 for ENCLOSURE_COMPONENT_DEVICE */
+ if (ecomp-type == ENCLOSURE_COMPONENT_DEVICE)
+ dest_desc[1] = 0;
+ dest_desc[2] = 0xde;
+ dest_desc[3] = 0x3c;
+}
+
+
static int ses_recv_diag(struct scsi_device *sdev, int page_code,
void *buf, int bufflen)
{
@@ -178,14 +192,22 @@ static int ses_set_fault(struct enclosure_device
*edev,
struct
RE: [PATCH 5/5] SES: Add power_status to SES enclosure component
@@ struct enclosure_component {
int locate;
int slot;
enum enclosure_status status;
+ int power_status;
};
struct enclosure_device {
--
1.8.1
-Original Message-
From: Song Liu
Sent: Friday, September 12, 2014 2:08 PM
To: 'Hannes Reinecke'; linux-scsi@vger.kernel.org
Cc: Dan Williams; Jens Axboe
Subject: RE: [PATCH 5/5] SES: Add power_status to SES enclosure component
New patch in next email...
Change to let power_status show on or off
Initialization of desc was removed because it will be initialized in
init_device_slot_control, so no need to initialize at define.
Thanks,
Song
-Original Message-
From: Hannes Reinecke [mailto:h...@suse.de]
Sent: Thursday, September 4, 2014 12:59 AM
To: Song Liu; linux-scsi@vger.kernel.org
Cc: Dan Williams; Jens Axboe
Subject: Re: [PATCH 5/5] SES: Add power_status to SES enclosure
component
On 08/25/2014 07:34 PM, Song Liu wrote:
From: Song Liu [mailto:songliubrav...@fb.com]
Sent: Monday, August 25, 2014 10:26 AM
To: Song Liu
Cc: Hannes Reinecke
Subject: [PATCH 5/5] SES: Add power_status to SES enclosure
component
Add power_status to SES enclosure component, so we can power on/off
the HDDs behind the enclosure.
Check firmware status in ses_set_* before sending control pages to
firmware.
Signed-off-by: Song Liu songliubrav...@fb.com
Acked-by: Dan Williams dan.j.willi...@intel.com
Reviewed-by: Jens Axboe ax...@fb.com
Cc: Hannes Reinecke h...@suse.de
---
drivers/misc/enclosure.c | 29 ++
drivers/scsi/ses.c| 98
++-
include/linux/enclosure.h | 6 +++
3 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index de335bc..0331dfe 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -148,6 +148,7 @@ enclosure_register(struct device *dev, const
char
*name, int components,
for (i = 0; i components; i++) {
edev-component[i].number = -1;
edev-component[i].slot = -1;
+ edev-component[i].power_status = 1;
}
mutex_lock(container_list_lock);
@@ -546,6 +547,31 @@ static ssize_t set_component_locate(struct
device *cdev,
return count;
}
+static ssize_t get_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp =
to_enclosure_component(cdev);
+
+ if (edev-cb-get_power_status)
+ edev-cb-get_power_status(edev, ecomp);
+ return snprintf(buf, 40, %d\n, ecomp-power_status); }
+
+static ssize_t set_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ const char *buf, size_t count) {
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp =
to_enclosure_component(cdev);
+ int val = simple_strtoul(buf, NULL, 0);
+
+ if (edev-cb-set_power_status)
+ edev-cb-set_power_status(edev, ecomp, val);
+ return count;
+}
+
Just using a number here doesn't seem to be very instructive; what is
this number? Can't we have a decode setting here to allow even the
uninitiated to do some sensible decisions here?
static ssize_t get_component_type(struct device *cdev,
struct device_attribute *attr, char *buf)
{ @@ -577,6 +603,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR,
get_component_active,
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR,
get_component_locate,
set_component_locate);
+static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR,
get_component_power_status,
+set_component_power_status);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL);
static DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
@@ -585,6 +613,7 @@ static struct attribute
*enclosure_component_attrs[] = {
dev_attr_status.attr,
dev_attr_active.attr,
dev_attr_locate.attr,
+ dev_attr_power_status.attr,
dev_attr_type.attr,
dev_attr_slot.attr,
NULL
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index
bafa301..ea6b262 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -67,6 +67,20 @@ static int ses_probe(struct device *dev) #define
SES_TIMEOUT (30 * HZ) #define SES_RETRIES 3
+static void init_device_slot_control(unsigned char *dest_desc,
+ struct enclosure_component *ecomp,
+ unsigned char *status)
+{
+ memcpy(dest_desc, status, 4);
+ dest_desc
Re: [PATCH 5/5] SES: Add power_status to SES enclosure component
On 08/25/2014 07:34 PM, Song Liu wrote:
From: Song Liu [mailto:songliubrav...@fb.com]
Sent: Monday, August 25, 2014 10:26 AM
To: Song Liu
Cc: Hannes Reinecke
Subject: [PATCH 5/5] SES: Add power_status to SES enclosure component
Add power_status to SES enclosure component, so we can power on/off the HDDs
behind the enclosure.
Check firmware status in ses_set_* before sending control pages to firmware.
Signed-off-by: Song Liu songliubrav...@fb.com
Acked-by: Dan Williams dan.j.willi...@intel.com
Reviewed-by: Jens Axboe ax...@fb.com
Cc: Hannes Reinecke h...@suse.de
---
drivers/misc/enclosure.c | 29 ++
drivers/scsi/ses.c| 98
++-
include/linux/enclosure.h | 6 +++
3 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
de335bc..0331dfe 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -148,6 +148,7 @@ enclosure_register(struct device *dev, const char *name,
int components,
for (i = 0; i components; i++) {
edev-component[i].number = -1;
edev-component[i].slot = -1;
+ edev-component[i].power_status = 1;
}
mutex_lock(container_list_lock);
@@ -546,6 +547,31 @@ static ssize_t set_component_locate(struct device *cdev,
return count;
}
+static ssize_t get_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp = to_enclosure_component(cdev);
+
+ if (edev-cb-get_power_status)
+ edev-cb-get_power_status(edev, ecomp);
+ return snprintf(buf, 40, %d\n, ecomp-power_status); }
+
+static ssize_t set_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev-parent);
+ struct enclosure_component *ecomp = to_enclosure_component(cdev);
+ int val = simple_strtoul(buf, NULL, 0);
+
+ if (edev-cb-set_power_status)
+ edev-cb-set_power_status(edev, ecomp, val);
+ return count;
+}
+
Just using a number here doesn't seem to be very instructive; what
is this number? Can't we have a decode setting here to allow even
the uninitiated to do some sensible decisions here?
static ssize_t get_component_type(struct device *cdev,
struct device_attribute *attr, char *buf) {
@@ -577,6 +603,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR,
get_component_active,
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
set_component_locate);
+static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR,
get_component_power_status,
+set_component_power_status);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL); static
DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
@@ -585,6 +613,7 @@ static struct attribute *enclosure_component_attrs[] = {
dev_attr_status.attr,
dev_attr_active.attr,
dev_attr_locate.attr,
+ dev_attr_power_status.attr,
dev_attr_type.attr,
dev_attr_slot.attr,
NULL
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index bafa301..ea6b262
100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -67,6 +67,20 @@ static int ses_probe(struct device *dev) #define
SES_TIMEOUT (30 * HZ) #define SES_RETRIES 3
+static void init_device_slot_control(unsigned char *dest_desc,
+ struct enclosure_component *ecomp,
+ unsigned char *status)
+{
+ memcpy(dest_desc, status, 4);
+ dest_desc[0] = 0;
+ /* only clear byte 1 for ENCLOSURE_COMPONENT_DEVICE */
+ if (ecomp-type == ENCLOSURE_COMPONENT_DEVICE)
+ dest_desc[1] = 0;
+ dest_desc[2] = 0xde;
+ dest_desc[3] = 0x3c;
+}
+
+
static int ses_recv_diag(struct scsi_device *sdev, int page_code,
void *buf, int bufflen)
{
@@ -178,14 +192,22 @@ static int ses_set_fault(struct enclosure_device *edev,
struct enclosure_component *ecomp,
enum enclosure_component_setting val) {
- unsigned char desc[4] = {0 };
+ unsigned char desc[4];
+ unsigned char *desc_ptr;
+
+ desc_ptr = ses_get_page2_descriptor(edev, ecomp);
+
+ if (!desc_ptr)
+ return -EIO;
+
+ init_device_slot_control(desc, ecomp, desc_ptr);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
- /* zero is disabled */
+ desc[3] =
