* [PATCH] ses: Handle non-unique element descriptors
@ 2011-07-04 15:16 Ankit Jain
2011-07-04 15:35 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Ankit Jain @ 2011-07-04 15:16 UTC (permalink / raw)
To: linux-scsi; +Cc: James Bottomley, Hannes Reinecke
Some SES devices give non-unique Element Descriptors as part of the
Element Descriptor diag page. Since we use these for creating sysfs
entries, they need to be unique.
Eg:
$ sg_ses -p 7 /dev/sg0
FTS CORP TXS6_SAS20BPX12 0500
enclosure services device
Element descriptor In diagnostic page:
generation code: 0x0
element descriptor by type list
Element type: Array device, subenclosure id: 0
Overall descriptor: ArrayDevicesInSubEnclsr0
Element 1 descriptor: ArrayDevice00
Element 2 descriptor: ArrayDevice01
Element 3 descriptor: ArrayDevice02
Element 4 descriptor: ArrayDevice03
Element 5 descriptor: ArrayDevice03
Element 6 descriptor: ArrayDevice03
Element 7 descriptor: ArrayDevice03
Element 8 descriptor: ArrayDevice03
Element 9 descriptor: ArrayDevice03
Element 10 descriptor: ArrayDevice03
Element 11 descriptor: ArrayDevice03
Element 12 descriptor: ArrayDevice03
Based on scsi-misc
Signed-off-by: Ankit Jain <jankit@suse.de>
---
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 00e5fca..8087055 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -239,6 +239,24 @@ static void enclosure_component_release(struct device *dev)
put_device(dev->parent);
}
+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *edev,
+ const char *name)
+{
+ int i;
+ if (!edev || !name || !name[0])
+ return NULL;
+
+ for (i=0; i<edev->components; i++) {
+ struct enclosure_component *ecomp = &edev->component[i];
+ if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name))
+ return ecomp;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(enclosure_component_find_by_name);
+
static const struct attribute_group *enclosure_groups[];
/**
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index eba183c..abfd962 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -421,12 +421,17 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
- if (create)
+ if (create) {
+ if (enclosure_component_find_by_name(edev, name))
+ /* name is not unique, already used
+ * set to NULL, so that enclosure_component_register
+ * will assign us a new one */
+ name = NULL;
ecomp = enclosure_component_register(edev,
components++,
type_ptr[0],
name);
- else
+ } else
ecomp = &edev->component[components++];
if (!IS_ERR(ecomp) && addl_desc_ptr)
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 9a33c5f..7e48ba9 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -122,6 +122,8 @@ void enclosure_unregister(struct enclosure_device *);
struct enclosure_component *
enclosure_component_register(struct enclosure_device *, unsigned int,
enum enclosure_component_type, const char *);
+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *, const char *);
int enclosure_add_device(struct enclosure_device *enclosure, int component,
struct device *dev);
int enclosure_remove_device(struct enclosure_device *, struct device *);
--
Ankit Jain
SUSE Labs
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors
2011-07-04 15:16 [PATCH] ses: Handle non-unique element descriptors Ankit Jain
@ 2011-07-04 15:35 ` James Bottomley
2011-07-04 22:59 ` Ankit Jain
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2011-07-04 15:35 UTC (permalink / raw)
To: Ankit Jain; +Cc: linux-scsi, James Bottomley, Hannes Reinecke
On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote:
> Some SES devices give non-unique Element Descriptors as part of the
> Element Descriptor diag page. Since we use these for creating sysfs
> entries, they need to be unique.
>
> Eg:
> $ sg_ses -p 7 /dev/sg0
> FTS CORP TXS6_SAS20BPX12 0500
> enclosure services device
> Element descriptor In diagnostic page:
> generation code: 0x0
> element descriptor by type list
> Element type: Array device, subenclosure id: 0
> Overall descriptor: ArrayDevicesInSubEnclsr0
> Element 1 descriptor: ArrayDevice00
> Element 2 descriptor: ArrayDevice01
> Element 3 descriptor: ArrayDevice02
> Element 4 descriptor: ArrayDevice03
> Element 5 descriptor: ArrayDevice03
> Element 6 descriptor: ArrayDevice03
> Element 7 descriptor: ArrayDevice03
> Element 8 descriptor: ArrayDevice03
> Element 9 descriptor: ArrayDevice03
> Element 10 descriptor: ArrayDevice03
> Element 11 descriptor: ArrayDevice03
> Element 12 descriptor: ArrayDevice03
What is the external visible labelling of this topology? It's
completely weird that the enclosure would burn in non-unique names
unless there's some reason for it.
> Signed-off-by: Ankit Jain <jankit@suse.de>
>
> ---
> diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
> index 00e5fca..8087055 100644
> --- a/drivers/misc/enclosure.c
> +++ b/drivers/misc/enclosure.c
> @@ -239,6 +239,24 @@ static void enclosure_component_release(struct device *dev)
> put_device(dev->parent);
> }
>
> +struct enclosure_component *
> +enclosure_component_find_by_name(struct enclosure_device *edev,
> + const char *name)
> +{
> + int i;
> + if (!edev || !name || !name[0])
> + return NULL;
> +
> + for (i=0; i<edev->components; i++) {
> + struct enclosure_component *ecomp = &edev->component[i];
> + if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name))
> + return ecomp;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(enclosure_component_find_by_name);
> +
> static const struct attribute_group *enclosure_groups[];
>
> /**
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index eba183c..abfd962 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -421,12 +421,17 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
> if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
> type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
>
> - if (create)
> + if (create) {
> + if (enclosure_component_find_by_name(edev, name))
> + /* name is not unique, already used
> + * set to NULL, so that enclosure_component_register
> + * will assign us a new one */
> + name = NULL;
This just assigns a random name ... if we actually have one, we should
probably just make it unique.
James
> ecomp = enclosure_component_register(edev,
> components++,
> type_ptr[0],
> name);
> - else
> + } else
> ecomp = &edev->component[components++];
>
> if (!IS_ERR(ecomp) && addl_desc_ptr)
> diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
> index 9a33c5f..7e48ba9 100644
> --- a/include/linux/enclosure.h
> +++ b/include/linux/enclosure.h
> @@ -122,6 +122,8 @@ void enclosure_unregister(struct enclosure_device *);
> struct enclosure_component *
> enclosure_component_register(struct enclosure_device *, unsigned int,
> enum enclosure_component_type, const char *);
> +struct enclosure_component *
> +enclosure_component_find_by_name(struct enclosure_device *, const char *);
> int enclosure_add_device(struct enclosure_device *enclosure, int component,
> struct device *dev);
> int enclosure_remove_device(struct enclosure_device *, struct device *);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors
2011-07-04 15:35 ` James Bottomley
@ 2011-07-04 22:59 ` Ankit Jain
2011-07-04 23:00 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Ankit Jain @ 2011-07-04 22:59 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi, James Bottomley, Hannes Reinecke
On 07/04/2011 09:05 PM, James Bottomley wrote:
> On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote:
>> Some SES devices give non-unique Element Descriptors as part of the
>> Element Descriptor diag page. Since we use these for creating sysfs
>> entries, they need to be unique.
>>
>> Eg:
>> $ sg_ses -p 7 /dev/sg0
>> FTS CORP TXS6_SAS20BPX12 0500
>> enclosure services device
>> Element descriptor In diagnostic page:
>> generation code: 0x0
>> element descriptor by type list
>> Element type: Array device, subenclosure id: 0
>> Overall descriptor: ArrayDevicesInSubEnclsr0
>> Element 1 descriptor: ArrayDevice00
>> Element 2 descriptor: ArrayDevice01
>> Element 3 descriptor: ArrayDevice02
>> Element 4 descriptor: ArrayDevice03
>> Element 5 descriptor: ArrayDevice03
>> Element 6 descriptor: ArrayDevice03
>> Element 7 descriptor: ArrayDevice03
>> Element 8 descriptor: ArrayDevice03
>> Element 9 descriptor: ArrayDevice03
>> Element 10 descriptor: ArrayDevice03
>> Element 11 descriptor: ArrayDevice03
>> Element 12 descriptor: ArrayDevice03
>
> What is the external visible labelling of this topology? It's
> completely weird that the enclosure would burn in non-unique names
> unless there's some reason for it.
I'm not sure what you mean by "external visible labelling". The system has a
SAS expanded backplane. I don't have access to the hardware now, but p7 looked
like this:
$ sg_ses -p 7 /dev/sg0
FTS CORP TXS6_SAS20BPX12 0500
enclosure services device
Element descriptor In diagnostic page:
generation code: 0x0
element descriptor by type list
Element type: Array device, subenclosure id: 0
Overall descriptor: ArrayDevicesInSubEnclsr0
Element 1 descriptor: ArrayDevice00
Element 2 descriptor: ArrayDevice01
Element 3 descriptor: ArrayDevice02
Element 4 descriptor: ArrayDevice03
Element 5 descriptor: ArrayDevice03
Element 6 descriptor: ArrayDevice03
Element 7 descriptor: ArrayDevice03
Element 8 descriptor: ArrayDevice03
Element 9 descriptor: ArrayDevice03
Element 10 descriptor: ArrayDevice03
Element 11 descriptor: ArrayDevice03
Element 12 descriptor: ArrayDevice03
Element type: SAS connector, subenclosure id: 0
Overall descriptor: ConnectorsInSubEnclsr0
Element 1 descriptor: Connector04
Element 2 descriptor: Connector05
Element 3 descriptor: Connector06
Element 4 descriptor: Connector07
Element 5 descriptor: Connector00
Element 6 descriptor: Connector01
Element 7 descriptor: Connector02
Element 8 descriptor: Connector03
Element 9 descriptor: Connector08
Element 10 descriptor: Connector09
Element 11 descriptor: Connector10
Element 12 descriptor: Connector11
Element type: Temperature sense, subenclosure id: 0
Overall descriptor: TempSensorsInSubEnclsr0
Element 1 descriptor: TempSense01
Element 2 descriptor: TempSense02
Element 3 descriptor: TempSense03
Element type: Voltage sensor, subenclosure id: 0
Overall descriptor: VoltageSensorsInSubEnclsr0
Element 1 descriptor: VoltageSense01
Element 2 descriptor: VoltageSense02
Element 3 descriptor: VoltageSense03
Element 4 descriptor: VoltageSense04
Element type: Current sensor, subenclosure id: 0
Overall descriptor: CurrentSensorsInSubEnclsr0
Element 1 descriptor: CurrentSense01
Element type: Enclosure, subenclosure id: 0
Overall descriptor: EnclosureElementInSubEnclsr0
Element 1 descriptor: EnclosureElement01
Element type: SAS expander, subenclosure id: 0
Overall descriptor: SAS Expander
Element 1 descriptor: Expander0
AFAICS, the spec doesn't seem to say that the names have to be unique.
But, now that I think about it a bit, possibly some of the "array device
slots" might have been vacant, and so descriptors could have been repeated.
This could probably be checked with the status page.
But then I'm not sure, whether sysfs entries should or shouldn't be created
for some cases (eg. status code!=OK).
>> - if (create)
>> + if (create) {
>> + if (enclosure_component_find_by_name(edev, name))
>> + /* name is not unique, already used
>> + * set to NULL, so that enclosure_component_register
>> + * will assign us a new one */
>> + name = NULL;
>
> This just assigns a random name ... if we actually have one, we should
> probably just make it unique.
Agreed. Would something like (below) be acceptable? I'll post the patch
separately, if it is:
---
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 00e5fca..f4b53fd 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -239,6 +239,23 @@ static void enclosure_component_release(struct device *dev)
put_device(dev->parent);
}
+static struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *edev,
+ const char *name)
+{
+ int i;
+ if (!edev || !name || !name[0])
+ return NULL;
+
+ for (i=0; i<edev->components; i++) {
+ struct enclosure_component *ecomp = &edev->component[i];
+ if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name))
+ return ecomp;
+ }
+
+ return NULL;
+}
+
static const struct attribute_group *enclosure_groups[];
/**
@@ -276,9 +293,13 @@ enclosure_component_register(struct enclosure_device *edev,
ecomp->number = number;
cdev = &ecomp->cdev;
cdev->parent = get_device(&edev->edev);
- if (name && name[0])
- dev_set_name(cdev, "%s", name);
- else
+
+ if (name && name[0]) {
+ if (enclosure_component_find_by_name (edev, name))
+ dev_set_name(cdev, "%s_%u", name, number);
+ else
+ dev_set_name(cdev, "%s", name);
+ } else
dev_set_name(cdev, "%u", number);
cdev->release = enclosure_component_release;
--
Ankit Jain
SUSE Labs
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors
2011-07-04 22:59 ` Ankit Jain
@ 2011-07-04 23:00 ` James Bottomley
2011-07-11 10:24 ` Ankit Jain
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2011-07-04 23:00 UTC (permalink / raw)
To: Ankit Jain; +Cc: linux-scsi, Hannes Reinecke
On Tue, 2011-07-05 at 04:29 +0530, Ankit Jain wrote:
> On 07/04/2011 09:05 PM, James Bottomley wrote:
> > On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote:
> >> Some SES devices give non-unique Element Descriptors as part of the
> >> Element Descriptor diag page. Since we use these for creating sysfs
> >> entries, they need to be unique.
> >>
> >> Eg:
> >> $ sg_ses -p 7 /dev/sg0
> >> FTS CORP TXS6_SAS20BPX12 0500
> >> enclosure services device
> >> Element descriptor In diagnostic page:
> >> generation code: 0x0
> >> element descriptor by type list
> >> Element type: Array device, subenclosure id: 0
> >> Overall descriptor: ArrayDevicesInSubEnclsr0
> >> Element 1 descriptor: ArrayDevice00
> >> Element 2 descriptor: ArrayDevice01
> >> Element 3 descriptor: ArrayDevice02
> >> Element 4 descriptor: ArrayDevice03
> >> Element 5 descriptor: ArrayDevice03
> >> Element 6 descriptor: ArrayDevice03
> >> Element 7 descriptor: ArrayDevice03
> >> Element 8 descriptor: ArrayDevice03
> >> Element 9 descriptor: ArrayDevice03
> >> Element 10 descriptor: ArrayDevice03
> >> Element 11 descriptor: ArrayDevice03
> >> Element 12 descriptor: ArrayDevice03
> >
> > What is the external visible labelling of this topology? It's
> > completely weird that the enclosure would burn in non-unique names
> > unless there's some reason for it.
>
> I'm not sure what you mean by "external visible labelling". The system has a
> SAS expanded backplane. I don't have access to the hardware now, but p7 looked
> like this:
The element descriptors usually correspond with labelling on the case of
whatever it is the enclosure is embedded in. Their job is to describe
the location of the device, so they usually get burned in with whatever
the default labelling scheme for the physical device is. So what I want
to know is what is the labelling scheme in this case?
James
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors
2011-07-04 23:00 ` James Bottomley
@ 2011-07-11 10:24 ` Ankit Jain
0 siblings, 0 replies; 5+ messages in thread
From: Ankit Jain @ 2011-07-11 10:24 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi, Hannes Reinecke
On 07/05/2011 04:30 AM, James Bottomley wrote:
> On Tue, 2011-07-05 at 04:29 +0530, Ankit Jain wrote:
>> On 07/04/2011 09:05 PM, James Bottomley wrote:
>>> On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote:
>>>> Some SES devices give non-unique Element Descriptors as part of the
>>>> Element Descriptor diag page. Since we use these for creating sysfs
>>>> entries, they need to be unique.
>>>>
>>>> Eg:
>>>> $ sg_ses -p 7 /dev/sg0
>>>> FTS CORP TXS6_SAS20BPX12 0500
>>>> enclosure services device
>>>> Element descriptor In diagnostic page:
>>>> generation code: 0x0
>>>> element descriptor by type list
>>>> Element type: Array device, subenclosure id: 0
>>>> Overall descriptor: ArrayDevicesInSubEnclsr0
>>>> Element 1 descriptor: ArrayDevice00
>>>> Element 2 descriptor: ArrayDevice01
>>>> Element 3 descriptor: ArrayDevice02
>>>> Element 4 descriptor: ArrayDevice03
>>>> Element 5 descriptor: ArrayDevice03
>>>> Element 6 descriptor: ArrayDevice03
>>>> Element 7 descriptor: ArrayDevice03
>>>> Element 8 descriptor: ArrayDevice03
>>>> Element 9 descriptor: ArrayDevice03
>>>> Element 10 descriptor: ArrayDevice03
>>>> Element 11 descriptor: ArrayDevice03
>>>> Element 12 descriptor: ArrayDevice03
>>>
>>> What is the external visible labelling of this topology? It's
>>> completely weird that the enclosure would burn in non-unique names
>>> unless there's some reason for it.
>>
>> I'm not sure what you mean by "external visible labelling". The system has a
>> SAS expanded backplane. I don't have access to the hardware now, but p7 looked
>> like this:
>
> The element descriptors usually correspond with labelling on the case of
> whatever it is the enclosure is embedded in. Their job is to describe
> the location of the device, so they usually get burned in with whatever
> the default labelling scheme for the physical device is. So what I want
> to know is what is the labelling scheme in this case?
I don't have direct access to the box, so I asked the person who does,
and he says: "the labelling is just 1-12 unless I'm mistaken.".
The duplicate descriptors is a firmware bug, but since the specification
doesn't say that these have to be unique, we should handle these, IMHO.
--
Ankit Jain
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-07-11 10:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-04 15:16 [PATCH] ses: Handle non-unique element descriptors Ankit Jain
2011-07-04 15:35 ` James Bottomley
2011-07-04 22:59 ` Ankit Jain
2011-07-04 23:00 ` James Bottomley
2011-07-11 10:24 ` Ankit Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox