The enclosed code change suggestion is proposed to modernize the
enclosure drivers. The changes outlined are but only compile &
checkpatch tested so do not propagate. We will be testing this on a real
enclosure... Inspection is invited. We would however like the answer to
a question (with two parts) and a few others:
1) Is /sys/.../enclosure_device:#/device_power an acceptable node name
for switching the power on and off for the target?
2) Do you think that target power management be made part of the Linux
power management hierarchy (and clues/pointers to how would be
nice :-) )
3) Add other enclosure bits (eg: Lock & LED control) while we are here?
4) Should the enclosure_component retained values be changed to
bit-fields?
5) Does Evolution work better than Outlook for delivering patch content?
Coincident repair of enclosure and ses device support. Fix problems with
setting enclosure values careful to preserve other settings. Corrected
fault setting as well. Modernize code. Add support for Device Power
Management.
Future: Add target device power-cycle to error recovery escalation,
between target reset and host bus adapter reset (has to be added to each
lib<transport> handler, difficult to visualize in error-handler common
code :-( ) ?
Signed-off-by: mark_salyzyn@xxxxxxxxxxxxxx
include/linux/enclosure.h | 6 +++
drivers/misc/enclosure.c | 45 +++++++++++++++++++-----
drivers/scsi/ses.c | 83
+++++++++++++++++++++++++++++++++-------------
3 files changed, 103 insertions(+), 31 deletions(-)
diff -rup a/include/linux/enclosure.h b/include/linux/enclosure.h
--- a/include/linux/enclosure.h 2011-11-11 10:14:07.000000000 -0500
+++ b/include/linux/enclosure.h 2011-11-11 10:09:00.000000000 -0500
@@ -79,6 +79,11 @@ struct enclosure_component_callbacks {
int (*set_locate)(struct enclosure_device *,
struct enclosure_component *,
enum enclosure_component_setting);
+ void (*get_device_power)(struct enclosure_device *,
+ struct enclosure_component *);
+ int (*set_device_power)(struct enclosure_device *,
+ struct enclosure_component *,
+ enum enclosure_component_setting);
};
@@ -91,6 +96,7 @@ struct enclosure_component {
int fault;
int active;
int locate;
+ int device_power;
enum enclosure_status status;
};
diff -rup a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
--- a/drivers/misc/enclosure.c 2011-11-11 10:19:45.000000000 -0500
+++ b/drivers/misc/enclosure.c 2011-11-11 10:36:35.000000000 -0500
@@ -416,10 +416,10 @@ static ssize_t set_component_fault(struc
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_fault)
- edev->cb->set_fault(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_fault)
+ edev->cb->set_fault(edev, ecomp, (int)val);
return count;
}
@@ -474,10 +474,10 @@ static ssize_t set_component_active(stru
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_active)
- edev->cb->set_active(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_active)
+ edev->cb->set_active(edev, ecomp, (int)val);
return count;
}
@@ -498,10 +498,34 @@ static ssize_t set_component_locate(stru
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_locate)
- edev->cb->set_locate(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_locate)
+ edev->cb->set_locate(edev, ecomp, (int)val);
+ return count;
+}
+
+static ssize_t get_component_device_power(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_device_power)
+ edev->cb->get_device_power(edev, ecomp);
+ return snprintf(buf, 40, "%d\n", ecomp->device_power);
+}
+
+static ssize_t set_component_device_power(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);
+ unsigned long val;
+
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_device_power)
+ edev->cb->set_device_power(edev, ecomp, (int)val);
return count;
}
@@ -522,6 +546,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_I
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
set_component_locate);
+static DEVICE_ATTR(device_power, S_IRUGO | S_IWUSR,
get_component_device_power,
+ set_component_device_power);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL);
static struct attribute *enclosure_component_attrs[] = {
@@ -529,6 +555,7 @@ static struct attribute *enclosure_compo
&dev_attr_status.attr,
&dev_attr_active.attr,
&dev_attr_locate.attr,
+ &dev_attr_device_power.attr,
&dev_attr_type.attr,
NULL
};
diff -rup a/drivers/scsi/ses.c b/drivers/scsi/ses.c
--- a/drivers/scsi/ses.c 2011-11-11 10:14:19.000000000 -0500
+++ b/drivers/scsi/ses.c 2011-11-11 10:35:14.000000000 -0500
@@ -167,18 +167,38 @@ static void ses_get_fault(struct enclosu
ecomp->fault = (desc[3] & 0x60) >> 4;
}
+static void ses_read_page2_descriptor(struct enclosure_device *edev,
+ struct enclosure_component *ecomp,
+ unsigned char *desc)
+{
+ unsigned char *desc_ptr = ses_get_page2_descriptor(edev, ecomp);
+
+ if (desc_ptr)
+ memcpy(desc, desc_ptr, 4);
+ /* preserve active state */
+ if (ecomp->active)
+ desc[2] |= 0x80;
+ else
+ desc[2] &= ~0x80;
+ /* Clear all reserved/alternate-purpose bits */
+ desc[2] &= 0xE7;
+ desc[3] &= 0x3C;
+}
+
static int ses_set_fault(struct enclosure_device *edev,
struct enclosure_component *ecomp,
enum enclosure_component_setting val)
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[3] &= ~0x20;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x02;
+ desc[3] |= 0x20;
break;
default:
/* SES doesn't do the SGPIO blink settings */
@@ -214,12 +234,46 @@ static int ses_set_locate(struct enclosu
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[2] &= ~0x02;
+ break;
+ case ENCLOSURE_SETTING_ENABLED:
+ desc[2] |= 0x02;
+ break;
+ default:
+ /* SES doesn't do the SGPIO blink settings */
+ return -EINVAL;
+ }
+ return ses_set_page2_descriptor(edev, ecomp, desc);
+}
+
+static void ses_get_device_power(struct enclosure_device *edev,
+ struct enclosure_component *ecomp)
+{
+ unsigned char *desc;
+
+ desc = ses_get_page2_descriptor(edev, ecomp);
+ if (desc)
+ ecomp->device_power = (desc[3] & 0x10) ? 0 : 1;
+}
+
+static int ses_set_device_power(struct enclosure_device *edev,
+ struct enclosure_component *ecomp,
+ enum enclosure_component_setting val)
+{
+ unsigned char desc[4] = {0 };
+
+ ses_read_page2_descriptor(edev, ecomp, desc);
+ switch (val) {
+ case ENCLOSURE_SETTING_DISABLED:
+ /* one is disabled */
+ desc[3] |= 0x10;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x02;
+ desc[3] &= ~0x10;
break;
default:
/* SES doesn't do the SGPIO blink settings */
@@ -234,13 +288,15 @@ static int ses_set_active(struct enclosu
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[2] &= ~0x80;
ecomp->active = 0;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x80;
+ desc[2] |= 0x80;
ecomp->active = 1;
break;
default:
@@ -256,6 +312,8 @@ static struct enclosure_component_callba
.get_status = ses_get_status,
.get_locate = ses_get_locate,
.set_locate = ses_set_locate,
+ .get_device_power = ses_get_device_power,
+ .set_device_power = ses_set_device_power,
.set_active = ses_set_active,
};
@@ -264,25 +322,6 @@ struct ses_host_edev {
struct enclosure_device *edev;
};
-#if 0
-int ses_match_host(struct enclosure_device *edev, void *data)
-{
- struct ses_host_edev *sed = data;
- struct scsi_device *sdev;
-
- if (!scsi_is_sdev_device(edev->edev.parent))
- return 0;
-
- sdev = to_scsi_device(edev->edev.parent);
-
- if (sdev->host != sed->shost)
- return 0;
-
- sed->edev = edev;
- return 1;
-}
-#endif /* 0 */
-
static void ses_process_descriptor(struct enclosure_component *ecomp,
unsigned char *desc)
{
Coincident repair of enclosure and ses device support. Fix problems with
setting enclosure values careful to preserve other settings. Corrected
fault setting to. Modernize code. Add support for Device Power Enable.
Signed-off-by: mark_salyzyn@xxxxxxxxxxxxxx
include/linux/enclosure.h | 6 +++
drivers/misc/enclosure.c | 45 +++++++++++++++++++-----
drivers/scsi/ses.c | 83 +++++++++++++++++++++++++++++++++-------------
3 files changed, 103 insertions(+), 31 deletions(-)
diff -rup a/include/linux/enclosure.h b/include/linux/enclosure.h
--- a/include/linux/enclosure.h 2011-11-11 10:14:07.000000000 -0500
+++ b/include/linux/enclosure.h 2011-11-11 10:09:00.000000000 -0500
@@ -79,6 +79,11 @@ struct enclosure_component_callbacks {
int (*set_locate)(struct enclosure_device *,
struct enclosure_component *,
enum enclosure_component_setting);
+ void (*get_device_power)(struct enclosure_device *,
+ struct enclosure_component *);
+ int (*set_device_power)(struct enclosure_device *,
+ struct enclosure_component *,
+ enum enclosure_component_setting);
};
@@ -91,6 +96,7 @@ struct enclosure_component {
int fault;
int active;
int locate;
+ int device_power;
enum enclosure_status status;
};
diff -rup a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
--- a/drivers/misc/enclosure.c 2011-11-11 10:19:45.000000000 -0500
+++ b/drivers/misc/enclosure.c 2011-11-11 10:36:35.000000000 -0500
@@ -416,10 +416,10 @@ static ssize_t set_component_fault(struc
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_fault)
- edev->cb->set_fault(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_fault)
+ edev->cb->set_fault(edev, ecomp, (int)val);
return count;
}
@@ -474,10 +474,10 @@ static ssize_t set_component_active(stru
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_active)
- edev->cb->set_active(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_active)
+ edev->cb->set_active(edev, ecomp, (int)val);
return count;
}
@@ -498,10 +498,34 @@ static ssize_t set_component_locate(stru
{
struct enclosure_device *edev = to_enclosure_device(cdev->parent);
struct enclosure_component *ecomp = to_enclosure_component(cdev);
- int val = simple_strtoul(buf, NULL, 0);
+ unsigned long val;
- if (edev->cb->set_locate)
- edev->cb->set_locate(edev, ecomp, val);
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_locate)
+ edev->cb->set_locate(edev, ecomp, (int)val);
+ return count;
+}
+
+static ssize_t get_component_device_power(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_device_power)
+ edev->cb->get_device_power(edev, ecomp);
+ return snprintf(buf, 40, "%d\n", ecomp->device_power);
+}
+
+static ssize_t set_component_device_power(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);
+ unsigned long val;
+
+ if (!strict_strtoul(buf, 0, &val) && edev->cb->set_device_power)
+ edev->cb->set_device_power(edev, ecomp, (int)val);
return count;
}
@@ -522,6 +546,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_I
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
set_component_locate);
+static DEVICE_ATTR(device_power, S_IRUGO | S_IWUSR, get_component_device_power,
+ set_component_device_power);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL);
static struct attribute *enclosure_component_attrs[] = {
@@ -529,6 +555,7 @@ static struct attribute *enclosure_compo
&dev_attr_status.attr,
&dev_attr_active.attr,
&dev_attr_locate.attr,
+ &dev_attr_device_power.attr,
&dev_attr_type.attr,
NULL
};
diff -rup a/drivers/scsi/ses.c b/drivers/scsi/ses.c
--- a/drivers/scsi/ses.c 2011-11-11 10:14:19.000000000 -0500
+++ b/drivers/scsi/ses.c 2011-11-11 10:35:14.000000000 -0500
@@ -167,18 +167,38 @@ static void ses_get_fault(struct enclosu
ecomp->fault = (desc[3] & 0x60) >> 4;
}
+static void ses_read_page2_descriptor(struct enclosure_device *edev,
+ struct enclosure_component *ecomp,
+ unsigned char *desc)
+{
+ unsigned char *desc_ptr = ses_get_page2_descriptor(edev, ecomp);
+
+ if (desc_ptr)
+ memcpy(desc, desc_ptr, 4);
+ /* preserve active state */
+ if (ecomp->active)
+ desc[2] |= 0x80;
+ else
+ desc[2] &= ~0x80;
+ /* Clear all reserved/alternate-purpose bits */
+ desc[2] &= 0xE7;
+ desc[3] &= 0x3C;
+}
+
static int ses_set_fault(struct enclosure_device *edev,
struct enclosure_component *ecomp,
enum enclosure_component_setting val)
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[3] &= ~0x20;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x02;
+ desc[3] |= 0x20;
break;
default:
/* SES doesn't do the SGPIO blink settings */
@@ -214,12 +234,46 @@ static int ses_set_locate(struct enclosu
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[2] &= ~0x02;
+ break;
+ case ENCLOSURE_SETTING_ENABLED:
+ desc[2] |= 0x02;
+ break;
+ default:
+ /* SES doesn't do the SGPIO blink settings */
+ return -EINVAL;
+ }
+ return ses_set_page2_descriptor(edev, ecomp, desc);
+}
+
+static void ses_get_device_power(struct enclosure_device *edev,
+ struct enclosure_component *ecomp)
+{
+ unsigned char *desc;
+
+ desc = ses_get_page2_descriptor(edev, ecomp);
+ if (desc)
+ ecomp->device_power = (desc[3] & 0x10) ? 0 : 1;
+}
+
+static int ses_set_device_power(struct enclosure_device *edev,
+ struct enclosure_component *ecomp,
+ enum enclosure_component_setting val)
+{
+ unsigned char desc[4] = {0 };
+
+ ses_read_page2_descriptor(edev, ecomp, desc);
+ switch (val) {
+ case ENCLOSURE_SETTING_DISABLED:
+ /* one is disabled */
+ desc[3] |= 0x10;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x02;
+ desc[3] &= ~0x10;
break;
default:
/* SES doesn't do the SGPIO blink settings */
@@ -234,13 +288,15 @@ static int ses_set_active(struct enclosu
{
unsigned char desc[4] = {0 };
+ ses_read_page2_descriptor(edev, ecomp, desc);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
/* zero is disabled */
+ desc[2] &= ~0x80;
ecomp->active = 0;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[2] = 0x80;
+ desc[2] |= 0x80;
ecomp->active = 1;
break;
default:
@@ -256,6 +312,8 @@ static struct enclosure_component_callba
.get_status = ses_get_status,
.get_locate = ses_get_locate,
.set_locate = ses_set_locate,
+ .get_device_power = ses_get_device_power,
+ .set_device_power = ses_set_device_power,
.set_active = ses_set_active,
};
@@ -264,25 +322,6 @@ struct ses_host_edev {
struct enclosure_device *edev;
};
-#if 0
-int ses_match_host(struct enclosure_device *edev, void *data)
-{
- struct ses_host_edev *sed = data;
- struct scsi_device *sdev;
-
- if (!scsi_is_sdev_device(edev->edev.parent))
- return 0;
-
- sdev = to_scsi_device(edev->edev.parent);
-
- if (sdev->host != sed->shost)
- return 0;
-
- sed->edev = edev;
- return 1;
-}
-#endif /* 0 */
-
static void ses_process_descriptor(struct enclosure_component *ecomp,
unsigned char *desc)
{
