* [PATCH 0/5] Feature enhancements for ses module [not found] <1408987546-2418-1-git-send-email-songliubraving@fb.com> @ 2014-08-25 17:34 ` Song Liu 2014-10-22 19:12 ` Jens Axboe [not found] ` <1408987546-2418-3-git-send-email-songliubraving@fb.com> ` (3 subsequent siblings) 4 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-08-25 17:34 UTC (permalink / raw) To: linux-scsi@vger.kernel.org; +Cc: Hannes Reinecke, Dan Williams, Jens Axboe From: Song Liu [mailto:songliubraving@fb.com] Sent: Monday, August 25, 2014 10:26 AM To: Song Liu Subject: [PATCH 0/5] Feature enhancements for ses module These patches include a few enhancements to ses module: 1: close potential race condition by at enclosure race condition 2,3,4: add enclosure id and device slot, so we can create symlink in /dev/disk/by-slot: # ls -d /dev/disk/by-slot/* /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 5: add ability to power on/off device with power_status file in sysfs. Dan Williams (4): SES: close potential registration race SES: generate KOBJ_CHANGE on enclosure attach SES: add enclosure logical id SES: add reliable slot attribute Song Liu (1): SES: Add power_status to SES enclosure component drivers/misc/enclosure.c | 98 +++++++++++++++++++++++++++---- drivers/scsi/ses.c | 145 +++++++++++++++++++++++++++++++++++++++------- include/linux/enclosure.h | 13 ++++- 3 files changed, 220 insertions(+), 36 deletions(-) -- 1.8.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Feature enhancements for ses module 2014-08-25 17:34 ` [PATCH 0/5] Feature enhancements for ses module Song Liu @ 2014-10-22 19:12 ` Jens Axboe 2014-10-22 22:28 ` Douglas Gilbert 2014-10-23 8:45 ` Christoph Hellwig 0 siblings, 2 replies; 19+ messages in thread From: Jens Axboe @ 2014-10-22 19:12 UTC (permalink / raw) To: Song Liu, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig On 08/25/2014 11:34 AM, Song Liu wrote: > From: Song Liu [mailto:songliubraving@fb.com] > Sent: Monday, August 25, 2014 10:26 AM > To: Song Liu > Subject: [PATCH 0/5] Feature enhancements for ses module > > These patches include a few enhancements to ses module: > > 1: close potential race condition by at enclosure race condition > > 2,3,4: add enclosure id and device slot, so we can create symlink > in /dev/disk/by-slot: > # ls -d /dev/disk/by-slot/* > /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 > > 5: add ability to power on/off device with power_status file in > sysfs. > > Dan Williams (4): > SES: close potential registration race > SES: generate KOBJ_CHANGE on enclosure attach > SES: add enclosure logical id > SES: add reliable slot attribute > > Song Liu (1): > SES: Add power_status to SES enclosure component Guys, where are we with these? Seemed like they got reviewed (and updates/fixes posted), then nothing else happened. Would be nice to get these upstream, so we don't have to carry them at FB. -- Jens Axboe ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Feature enhancements for ses module 2014-10-22 19:12 ` Jens Axboe @ 2014-10-22 22:28 ` Douglas Gilbert 2014-10-22 23:01 ` Song Liu 2014-10-23 8:45 ` Christoph Hellwig 1 sibling, 1 reply; 19+ messages in thread From: Douglas Gilbert @ 2014-10-22 22:28 UTC (permalink / raw) To: Jens Axboe, Song Liu, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig On 14-10-22 09:12 PM, Jens Axboe wrote: > On 08/25/2014 11:34 AM, Song Liu wrote: >> From: Song Liu [mailto:songliubraving@fb.com] >> Sent: Monday, August 25, 2014 10:26 AM >> To: Song Liu >> Subject: [PATCH 0/5] Feature enhancements for ses module >> >> These patches include a few enhancements to ses module: >> >> 1: close potential race condition by at enclosure race condition >> >> 2,3,4: add enclosure id and device slot, so we can create symlink >> in /dev/disk/by-slot: >> # ls -d /dev/disk/by-slot/* >> /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 >> >> 5: add ability to power on/off device with power_status file in >> sysfs. Had a rude awakening with sg_ses recently when setting a field in the enclosure control dpage. That is what is being done in point 5: above. The time honoured technique is to read the corresponding enclosure status dpage, find the correct element, twiddle the field of interest, set the SELECT bit and write it back. The idea is maintain any other field settings in that element. And this is what the ses module does. There is at least one SES device out there that rejects the write if there are bits set in RESERVED locations. According to SPC-4 a device may do that. Look at the status (read) and control (write) variants of each element type: many times the control variant has less fields. To fix that the read-modify-write cycle needs to be changed to a read-mask-modify-write cycle where the "mask" is the allowable write mask (there would be one for each element type). This is ugly and may create problems in the future if and when T10 adds a new field in an element. About that time T10 should think about refining the meaning of RESERVED in SES to outlaw SES devices creating this particular time waster. Doug Gilbert >> Dan Williams (4): >> SES: close potential registration race >> SES: generate KOBJ_CHANGE on enclosure attach >> SES: add enclosure logical id >> SES: add reliable slot attribute >> >> Song Liu (1): >> SES: Add power_status to SES enclosure component > > Guys, where are we with these? Seemed like they got reviewed (and > updates/fixes posted), then nothing else happened. Would be nice to get > these upstream, so we don't have to carry them at FB. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 0/5] Feature enhancements for ses module 2014-10-22 22:28 ` Douglas Gilbert @ 2014-10-22 23:01 ` Song Liu 2014-10-23 1:17 ` Douglas Gilbert 0 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-10-22 23:01 UTC (permalink / raw) To: dgilbert@interlog.com, Jens Axboe, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig Hi Doug, I am not sure whether I fully understand the scenario. Actually patch 5 tries to clear all reserved bits: + 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; Would this work for device that rejects request with 1 in RESERVED areas? Thanks, Song > -----Original Message----- > From: Douglas Gilbert [mailto:dgilbert@interlog.com] > Sent: Wednesday, October 22, 2014 3:29 PM > To: Jens Axboe; Song Liu; linux-scsi@vger.kernel.org > Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > Subject: Re: [PATCH 0/5] Feature enhancements for ses module > > On 14-10-22 09:12 PM, Jens Axboe wrote: > > On 08/25/2014 11:34 AM, Song Liu wrote: > >> From: Song Liu [mailto:songliubraving@fb.com] > >> Sent: Monday, August 25, 2014 10:26 AM > >> To: Song Liu > >> Subject: [PATCH 0/5] Feature enhancements for ses module > >> > >> These patches include a few enhancements to ses module: > >> > >> 1: close potential race condition by at enclosure race condition > >> > >> 2,3,4: add enclosure id and device slot, so we can create symlink > >> in /dev/disk/by-slot: > >> # ls -d /dev/disk/by-slot/* > >> /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 > >> > >> 5: add ability to power on/off device with power_status file in > >> sysfs. > > Had a rude awakening with sg_ses recently when setting a field in the > enclosure control dpage. That is what is being done in point 5: above. > > The time honoured technique is to read the corresponding enclosure status > dpage, find the correct element, twiddle the field of interest, set the SELECT bit > and write it back. The idea is maintain any other field settings in that element. > And this is what the ses module does. > > There is at least one SES device out there that rejects the write if there are bits > set in RESERVED locations. According to SPC-4 a device may do that. Look at > the status (read) and control (write) variants of each element type: many times > the control variant has less fields. > > To fix that the read-modify-write cycle needs to be changed to a read-mask- > modify-write cycle where the "mask" is the allowable write mask (there would > be one for each element type). > > This is ugly and may create problems in the future if and when > T10 adds a new field in an element. About that time T10 should think about > refining the meaning of RESERVED in SES to outlaw SES devices creating this > particular time waster. > > Doug Gilbert > > >> Dan Williams (4): > >> SES: close potential registration race > >> SES: generate KOBJ_CHANGE on enclosure attach > >> SES: add enclosure logical id > >> SES: add reliable slot attribute > >> > >> Song Liu (1): > >> SES: Add power_status to SES enclosure component > > > > Guys, where are we with these? Seemed like they got reviewed (and > > updates/fixes posted), then nothing else happened. Would be nice to > > get these upstream, so we don't have to carry them at FB. > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Feature enhancements for ses module 2014-10-22 23:01 ` Song Liu @ 2014-10-23 1:17 ` Douglas Gilbert 2014-10-23 3:42 ` Song Liu 2014-10-28 0:32 ` Song Liu 0 siblings, 2 replies; 19+ messages in thread From: Douglas Gilbert @ 2014-10-23 1:17 UTC (permalink / raw) To: Song Liu, Jens Axboe, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig On 14-10-23 01:01 AM, Song Liu wrote: > Hi Doug, > > I am not sure whether I fully understand the scenario. Actually patch 5 tries to clear all reserved bits: > > + 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; > > Would this work for device that rejects request with 1 in RESERVED areas? That is a pretty asymmetric element type, assuming we are talking about the "enclosure control" and "enclosure status" elements (i.e. etc=0xe). My guess would be: dest_desc[0] = 0x80 | (src_desc[0] & 40); dest_desc[1] = 0x80 & src_desc[1]; dest_desc[2] = (pc_req << 6) | pc_delay; dest_desc[3] = 0xff & src_desc[3]; or if you have a new power_off_duration: dest_desc[3] = (power_off_duration << 2) | (src_desc[3] & 0x3); In byte 0 the top bit (SELECT) must be set or the enclosure will ignore any other settings in that element. If the PRDFAIL bit is already set, then that setting will be maintained. SES-3 has a note about clearing DISABLE and SWAP. In byte 1 is if the identifier (LED ?) is active (saying blinking) prior to this power cycle request, then it will stay blinking until the power drops. If the enclosure was really clever it might keep blinking after the power cycle :-) Also notice that the requested power cycle can be cancelled up to the "time until power cycle" drops to zero. >> -----Original Message----- >> From: Douglas Gilbert [mailto:dgilbert@interlog.com] >> Sent: Wednesday, October 22, 2014 3:29 PM >> To: Jens Axboe; Song Liu; linux-scsi@vger.kernel.org >> Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig >> Subject: Re: [PATCH 0/5] Feature enhancements for ses module >> >> On 14-10-22 09:12 PM, Jens Axboe wrote: >>> On 08/25/2014 11:34 AM, Song Liu wrote: >>>> From: Song Liu [mailto:songliubraving@fb.com] >>>> Sent: Monday, August 25, 2014 10:26 AM >>>> To: Song Liu >>>> Subject: [PATCH 0/5] Feature enhancements for ses module >>>> >>>> These patches include a few enhancements to ses module: >>>> >>>> 1: close potential race condition by at enclosure race condition >>>> >>>> 2,3,4: add enclosure id and device slot, so we can create symlink >>>> in /dev/disk/by-slot: >>>> # ls -d /dev/disk/by-slot/* >>>> /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 >>>> >>>> 5: add ability to power on/off device with power_status file in >>>> sysfs. >> >> Had a rude awakening with sg_ses recently when setting a field in the >> enclosure control dpage. That is what is being done in point 5: above. >> >> The time honoured technique is to read the corresponding enclosure status >> dpage, find the correct element, twiddle the field of interest, set the SELECT bit >> and write it back. The idea is maintain any other field settings in that element. >> And this is what the ses module does. >> >> There is at least one SES device out there that rejects the write if there are bits >> set in RESERVED locations. According to SPC-4 a device may do that. Look at >> the status (read) and control (write) variants of each element type: many times >> the control variant has less fields. >> >> To fix that the read-modify-write cycle needs to be changed to a read-mask- >> modify-write cycle where the "mask" is the allowable write mask (there would >> be one for each element type). >> >> This is ugly and may create problems in the future if and when >> T10 adds a new field in an element. About that time T10 should think about >> refining the meaning of RESERVED in SES to outlaw SES devices creating this >> particular time waster. >> >> Doug Gilbert >> >>>> Dan Williams (4): >>>> SES: close potential registration race >>>> SES: generate KOBJ_CHANGE on enclosure attach >>>> SES: add enclosure logical id >>>> SES: add reliable slot attribute >>>> >>>> Song Liu (1): >>>> SES: Add power_status to SES enclosure component >>> >>> Guys, where are we with these? Seemed like they got reviewed (and >>> updates/fixes posted), then nothing else happened. Would be nice to >>> get these upstream, so we don't have to carry them at FB. >>> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 0/5] Feature enhancements for ses module 2014-10-23 1:17 ` Douglas Gilbert @ 2014-10-23 3:42 ` Song Liu 2014-10-28 0:32 ` Song Liu 1 sibling, 0 replies; 19+ messages in thread From: Song Liu @ 2014-10-23 3:42 UTC (permalink / raw) To: dgilbert@interlog.com, Jens Axboe, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig Hi Doug, The power on/off field together with "fault", "locate", and "active" are for HDD (i.e. DeviceSlot and ArrayDeviceSlot). They are not for enclosure or other elements. So we are not dealing with power off duration, etc. here. Thanks, Song > -----Original Message----- > From: Douglas Gilbert [mailto:dgilbert@interlog.com] > Sent: Wednesday, October 22, 2014 6:17 PM > To: Song Liu; Jens Axboe; linux-scsi@vger.kernel.org > Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > Subject: Re: [PATCH 0/5] Feature enhancements for ses module > > On 14-10-23 01:01 AM, Song Liu wrote: > > Hi Doug, > > > > I am not sure whether I fully understand the scenario. Actually patch 5 tries to > clear all reserved bits: > > > > + 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; > > > > Would this work for device that rejects request with 1 in RESERVED areas? > > > That is a pretty asymmetric element type, assuming we are talking about the > "enclosure control" and "enclosure status" elements (i.e. etc=0xe). My guess > would be: > > dest_desc[0] = 0x80 | (src_desc[0] & 40); > dest_desc[1] = 0x80 & src_desc[1]; > dest_desc[2] = (pc_req << 6) | pc_delay; > dest_desc[3] = 0xff & src_desc[3]; > or if you have a new power_off_duration: > dest_desc[3] = (power_off_duration << 2) | (src_desc[3] & 0x3); > > In byte 0 the top bit (SELECT) must be set or the enclosure will ignore any other > settings in that element. If the PRDFAIL bit is already set, then that setting will > be maintained. > SES-3 has a note about clearing DISABLE and SWAP. > > In byte 1 is if the identifier (LED ?) is active (saying blinking) prior to this power > cycle request, then it will stay blinking until the power drops. If the enclosure > was really clever it might keep blinking after the power cycle :-) > > Also notice that the requested power cycle can be cancelled up to the "time > until power cycle" drops to zero. > > >> -----Original Message----- > >> From: Douglas Gilbert [mailto:dgilbert@interlog.com] > >> Sent: Wednesday, October 22, 2014 3:29 PM > >> To: Jens Axboe; Song Liu; linux-scsi@vger.kernel.org > >> Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > >> Subject: Re: [PATCH 0/5] Feature enhancements for ses module > >> > >> On 14-10-22 09:12 PM, Jens Axboe wrote: > >>> On 08/25/2014 11:34 AM, Song Liu wrote: > >>>> From: Song Liu [mailto:songliubraving@fb.com] > >>>> Sent: Monday, August 25, 2014 10:26 AM > >>>> To: Song Liu > >>>> Subject: [PATCH 0/5] Feature enhancements for ses module > >>>> > >>>> These patches include a few enhancements to ses module: > >>>> > >>>> 1: close potential race condition by at enclosure race condition > >>>> > >>>> 2,3,4: add enclosure id and device slot, so we can create symlink > >>>> in /dev/disk/by-slot: > >>>> # ls -d /dev/disk/by-slot/* > >>>> /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 > >>>> > >>>> 5: add ability to power on/off device with power_status file in > >>>> sysfs. > >> > >> Had a rude awakening with sg_ses recently when setting a field in the > >> enclosure control dpage. That is what is being done in point 5: above. > >> > >> The time honoured technique is to read the corresponding enclosure > >> status dpage, find the correct element, twiddle the field of > >> interest, set the SELECT bit and write it back. The idea is maintain any other > field settings in that element. > >> And this is what the ses module does. > >> > >> There is at least one SES device out there that rejects the write if > >> there are bits set in RESERVED locations. According to SPC-4 a device > >> may do that. Look at the status (read) and control (write) variants > >> of each element type: many times the control variant has less fields. > >> > >> To fix that the read-modify-write cycle needs to be changed to a > >> read-mask- modify-write cycle where the "mask" is the allowable write > >> mask (there would be one for each element type). > >> > >> This is ugly and may create problems in the future if and when > >> T10 adds a new field in an element. About that time T10 should think > >> about refining the meaning of RESERVED in SES to outlaw SES devices > >> creating this particular time waster. > >> > >> Doug Gilbert > >> > >>>> Dan Williams (4): > >>>> SES: close potential registration race > >>>> SES: generate KOBJ_CHANGE on enclosure attach > >>>> SES: add enclosure logical id > >>>> SES: add reliable slot attribute > >>>> > >>>> Song Liu (1): > >>>> SES: Add power_status to SES enclosure component > >>> > >>> Guys, where are we with these? Seemed like they got reviewed (and > >>> updates/fixes posted), then nothing else happened. Would be nice to > >>> get these upstream, so we don't have to carry them at FB. > >>> > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" > > in the body of a message to majordomo@vger.kernel.org More majordomo > > info at > > https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/majo > > rdomo- > info.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=J3jGe56dIPfS5TN6DM > > > 82UYbbeR1j2viaiSJI40tv6lE%3D%0A&m=CIfJIr9ka37ZOOTBDMcaf3cmkg1%2BV > Rv4oz > > > 0zbf%2Fb24o%3D%0A&s=792fd953749b38a8e8181e2f36a2b9102ae9a3096f85f > 95690 > > 4aa8201dde45d6 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 0/5] Feature enhancements for ses module 2014-10-23 1:17 ` Douglas Gilbert 2014-10-23 3:42 ` Song Liu @ 2014-10-28 0:32 ` Song Liu 1 sibling, 0 replies; 19+ messages in thread From: Song Liu @ 2014-10-28 0:32 UTC (permalink / raw) To: dgilbert@interlog.com, Jens Axboe, linux-scsi@vger.kernel.org Cc: Hannes Reinecke, Dan Williams, Christoph Hellwig Hi Doug, I agree what it is difficult to handle all elements, and thus using sg_ses probably makes more sense. However, it helps to handle some HDD related fields in the kernel, as the kernel can generate mapping between a device to the SES device element (or array device element): /sys/block/sdc/device/enclosure_deviceXXX/ With patch 5, we can easily power off a running HDD by echo off > /sys/block/sdc/device/enclosure_deviceXXX/power_status This is very useful for systems like Cold Storage, where HDDs are being powered on/off frequently. In current code, ses_set_page2_descriptor already clear reserved field for all elements, and only send non-zero for the device element or array device. Patch 5 also handles reserved field of these two elements in init_device_slot_control: dest_desc[2] &= 0xde; dest_desc[3] &= 0x3c; So I think we can control fault, locate, active, and power_status of each HDD without issue. Please let me know your suggestion on this. Thanks, Song > -----Original Message----- > From: Song Liu > Sent: Wednesday, October 22, 2014 8:42 PM > To: 'dgilbert@interlog.com'; Jens Axboe; linux-scsi@vger.kernel.org > Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > Subject: RE: [PATCH 0/5] Feature enhancements for ses module > > Hi Doug, > > The power on/off field together with "fault", "locate", and "active" are for HDD > (i.e. DeviceSlot and ArrayDeviceSlot). They are not for enclosure or other > elements. So we are not dealing with power off duration, etc. here. > > Thanks, > Song > > > -----Original Message----- > > From: Douglas Gilbert [mailto:dgilbert@interlog.com] > > Sent: Wednesday, October 22, 2014 6:17 PM > > To: Song Liu; Jens Axboe; linux-scsi@vger.kernel.org > > Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > > Subject: Re: [PATCH 0/5] Feature enhancements for ses module > > > > On 14-10-23 01:01 AM, Song Liu wrote: > > > Hi Doug, > > > > > > I am not sure whether I fully understand the scenario. Actually > > > patch 5 tries to > > clear all reserved bits: > > > > > > + 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; > > > > > > Would this work for device that rejects request with 1 in RESERVED areas? > > > > > > That is a pretty asymmetric element type, assuming we are talking > > about the "enclosure control" and "enclosure status" elements (i.e. > > etc=0xe). My guess would be: > > > > dest_desc[0] = 0x80 | (src_desc[0] & 40); > > dest_desc[1] = 0x80 & src_desc[1]; > > dest_desc[2] = (pc_req << 6) | pc_delay; > > dest_desc[3] = 0xff & src_desc[3]; or if you have a new > > power_off_duration: > > dest_desc[3] = (power_off_duration << 2) | (src_desc[3] & 0x3); > > > > In byte 0 the top bit (SELECT) must be set or the enclosure will > > ignore any other settings in that element. If the PRDFAIL bit is > > already set, then that setting will be maintained. > > SES-3 has a note about clearing DISABLE and SWAP. > > > > In byte 1 is if the identifier (LED ?) is active (saying blinking) > > prior to this power cycle request, then it will stay blinking until > > the power drops. If the enclosure was really clever it might keep > > blinking after the power cycle :-) > > > > Also notice that the requested power cycle can be cancelled up to the > > "time until power cycle" drops to zero. > > > > >> -----Original Message----- > > >> From: Douglas Gilbert [mailto:dgilbert@interlog.com] > > >> Sent: Wednesday, October 22, 2014 3:29 PM > > >> To: Jens Axboe; Song Liu; linux-scsi@vger.kernel.org > > >> Cc: Hannes Reinecke; Dan Williams; Christoph Hellwig > > >> Subject: Re: [PATCH 0/5] Feature enhancements for ses module > > >> > > >> On 14-10-22 09:12 PM, Jens Axboe wrote: > > >>> On 08/25/2014 11:34 AM, Song Liu wrote: > > >>>> From: Song Liu [mailto:songliubraving@fb.com] > > >>>> Sent: Monday, August 25, 2014 10:26 AM > > >>>> To: Song Liu > > >>>> Subject: [PATCH 0/5] Feature enhancements for ses module > > >>>> > > >>>> These patches include a few enhancements to ses module: > > >>>> > > >>>> 1: close potential race condition by at enclosure race condition > > >>>> > > >>>> 2,3,4: add enclosure id and device slot, so we can create symlink > > >>>> in /dev/disk/by-slot: > > >>>> # ls -d /dev/disk/by-slot/* > > >>>> /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 > > >>>> > > >>>> 5: add ability to power on/off device with power_status file in > > >>>> sysfs. > > >> > > >> Had a rude awakening with sg_ses recently when setting a field in > > >> the enclosure control dpage. That is what is being done in point 5: above. > > >> > > >> The time honoured technique is to read the corresponding enclosure > > >> status dpage, find the correct element, twiddle the field of > > >> interest, set the SELECT bit and write it back. The idea is > > >> maintain any other > > field settings in that element. > > >> And this is what the ses module does. > > >> > > >> There is at least one SES device out there that rejects the write > > >> if there are bits set in RESERVED locations. According to SPC-4 a > > >> device may do that. Look at the status (read) and control (write) > > >> variants of each element type: many times the control variant has less > fields. > > >> > > >> To fix that the read-modify-write cycle needs to be changed to a > > >> read-mask- modify-write cycle where the "mask" is the allowable > > >> write mask (there would be one for each element type). > > >> > > >> This is ugly and may create problems in the future if and when > > >> T10 adds a new field in an element. About that time T10 should > > >> think about refining the meaning of RESERVED in SES to outlaw SES > > >> devices creating this particular time waster. > > >> > > >> Doug Gilbert > > >> > > >>>> Dan Williams (4): > > >>>> SES: close potential registration race > > >>>> SES: generate KOBJ_CHANGE on enclosure attach > > >>>> SES: add enclosure logical id > > >>>> SES: add reliable slot attribute > > >>>> > > >>>> Song Liu (1): > > >>>> SES: Add power_status to SES enclosure component > > >>> > > >>> Guys, where are we with these? Seemed like they got reviewed (and > > >>> updates/fixes posted), then nothing else happened. Would be nice > > >>> to get these upstream, so we don't have to carry them at FB. > > >>> > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" > > > in the body of a message to majordomo@vger.kernel.org More majordomo > > > info at > > > https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/ma > > > jo > > > rdomo- > > > info.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=J3jGe56dIPfS5TN6DM > > > > > > 82UYbbeR1j2viaiSJI40tv6lE%3D%0A&m=CIfJIr9ka37ZOOTBDMcaf3cmkg1%2BV > > Rv4oz > > > > > > 0zbf%2Fb24o%3D%0A&s=792fd953749b38a8e8181e2f36a2b9102ae9a3096f85f > > 95690 > > > 4aa8201dde45d6 > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Feature enhancements for ses module 2014-10-22 19:12 ` Jens Axboe 2014-10-22 22:28 ` Douglas Gilbert @ 2014-10-23 8:45 ` Christoph Hellwig 1 sibling, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2014-10-23 8:45 UTC (permalink / raw) To: Jens Axboe Cc: Song Liu, linux-scsi@vger.kernel.org, Hannes Reinecke, Dan Williams, Christoph Hellwig On Wed, Oct 22, 2014 at 01:12:01PM -0600, Jens Axboe wrote: > Guys, where are we with these? Seemed like they got reviewed (and > updates/fixes posted), then nothing else happened. Would be nice to get > these upstream, so we don't have to carry them at FB. Nothign happened, I guess mostly like due to the odd way it was posted, seemingly in reply to something else that wasn't sent in mutt. I'll way for Song and Doug to agree on the reserved field handling and will apply it once all remaining issues there are sorted out. ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <1408987546-2418-3-git-send-email-songliubraving@fb.com>]
* [PATCH 2/5] SES: generate KOBJ_CHANGE on enclosure attach [not found] ` <1408987546-2418-3-git-send-email-songliubraving@fb.com> @ 2014-08-25 17:34 ` Song Liu [not found] ` <54081A32.9070303@suse.de> 0 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-08-25 17:34 UTC (permalink / raw) To: linux-scsi@vger.kernel.org; +Cc: Hannes Reinecke, Dan Williams, Jens Axboe From: Song Liu [mailto:songliubraving@fb.com] Sent: Monday, August 25, 2014 10:26 AM To: Song Liu Cc: Dan Williams; Hannes Reinecke Subject: [PATCH 2/5] SES: generate KOBJ_CHANGE on enclosure attach From: Dan Williams <dan.j.williams@intel.com> In support of a /dev/disk/by-slot populated with data from the enclosure and ses modules udev needs notification when the new interface files/links are available. Otherwise, any udev rules specified for the disk cannot assume that the enclosure topology has settled. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@suse.de> --- drivers/scsi/ses.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index c2e8a98..8f0a62a 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -349,7 +349,8 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev, if (scomp->addr != efd->addr) continue; - enclosure_add_device(edev, i, efd->dev); + if (enclosure_add_device(edev, i, efd->dev) == 0) + kobject_uevent(&efd->dev->kobj, KOBJ_CHANGE); return 1; } return 0; -- 1.8.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <54081A32.9070303@suse.de>]
* RE: [PATCH 2/5] SES: generate KOBJ_CHANGE on enclosure attach [not found] ` <54081A32.9070303@suse.de> @ 2014-09-12 21:03 ` Song Liu 0 siblings, 0 replies; 19+ messages in thread From: Song Liu @ 2014-09-12 21:03 UTC (permalink / raw) To: Hannes Reinecke, linux-scsi@vger.kernel.org; +Cc: Dan Williams, Jens Axboe >From 741aa4b6043af741e28c0c414d48249f878353dc Mon Sep 17 00:00:00 2001 From: Dan Williams <dan.j.williams@intel.com> Date: Thu, 21 Aug 2014 11:43:22 -0700 Subject: [PATCH 2/5] SES: generate KOBJ_CHANGE on enclosure attach In support of a /dev/disk/by-slot populated with data from the enclosure and ses modules udev needs notification when the new interface files/links are available. Otherwise, any udev rules specified for the disk cannot assume that the enclosure topology has settled. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Reviewed-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/ses.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index c52fd98..87cf970b 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -349,7 +349,8 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev, if (scomp->addr != efd->addr) continue; - enclosure_add_device(edev, i, efd->dev); + if (enclosure_add_device(edev, i, efd->dev) == 0) + kobject_uevent(&efd->dev->kobj, KOBJ_CHANGE); return 1; } return 0; -- 1.8.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <1408987546-2418-4-git-send-email-songliubraving@fb.com>]
* [PATCH 3/5] SES: add enclosure logical id [not found] ` <1408987546-2418-4-git-send-email-songliubraving@fb.com> @ 2014-08-25 17:34 ` Song Liu [not found] ` <54081ABD.9080104@suse.de> 0 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-08-25 17:34 UTC (permalink / raw) To: linux-scsi@vger.kernel.org; +Cc: Dan Williams, Hannes Reinecke, Jens Axboe From: Song Liu [mailto:songliubraving@fb.com] Sent: Monday, August 25, 2014 10:26 AM To: Song Liu Cc: Dan Williams; Hannes Reinecke Subject: [PATCH 3/5] SES: add enclosure logical id From: Dan Williams <dan.j.williams@intel.com> Export the NAA logical id for the enclosure. This is optionally available from the sas_transport_class, but it is really a property of the enclosure. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@suse.de> --- drivers/misc/enclosure.c | 13 +++++++++++++ drivers/scsi/ses.c | 9 +++++++++ include/linux/enclosure.h | 1 + 3 files changed, 23 insertions(+) diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 15faf61..646068a 100644 --- a/drivers/misc/enclosure.c +++ b/drivers/misc/enclosure.c @@ -395,8 +395,21 @@ static ssize_t components_show(struct device *cdev, } static DEVICE_ATTR_RO(components); +static ssize_t id_show(struct device *cdev, + struct device_attribute *attr, + char *buf) +{ + struct enclosure_device *edev = to_enclosure_device(cdev); + + if (edev->cb->show_id) + return edev->cb->show_id(edev, buf); + return 0; +} +static DEVICE_ATTR_RO(id); + static struct attribute *enclosure_class_attrs[] = { &dev_attr_components.attr, + &dev_attr_id.attr, NULL, }; ATTRIBUTE_GROUPS(enclosure_class); diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 8f0a62a..61deb4e 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -258,6 +258,14 @@ static int ses_set_active(struct enclosure_device *edev, return ses_set_page2_descriptor(edev, ecomp, desc); } +static int ses_show_id(struct enclosure_device *edev, char *buf) { + struct ses_device *ses_dev = edev->scratch; + unsigned long long id = get_unaligned_be64(ses_dev->page1+8+4); + + return sprintf(buf, "%#llx\n", id); +} + static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_fault = ses_get_fault, .set_fault = ses_set_fault, @@ -265,6 +273,7 @@ static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_locate = ses_get_locate, .set_locate = ses_set_locate, .set_active = ses_set_active, + .show_id = ses_show_id, }; struct ses_host_edev { diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index a835d33..807622b 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -79,6 +79,7 @@ struct enclosure_component_callbacks { int (*set_locate)(struct enclosure_device *, struct enclosure_component *, enum enclosure_component_setting); + int (*show_id)(struct enclosure_device *, char *buf); }; -- 1.8.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <54081ABD.9080104@suse.de>]
* RE: [PATCH 3/5] SES: add enclosure logical id [not found] ` <54081ABD.9080104@suse.de> @ 2014-09-12 21:04 ` Song Liu 0 siblings, 0 replies; 19+ messages in thread From: Song Liu @ 2014-09-12 21:04 UTC (permalink / raw) To: Hannes Reinecke, linux-scsi@vger.kernel.org; +Cc: Dan Williams, Jens Axboe From 849dd255bdd2158cb621697e41448cf6b230d2f4 Mon Sep 17 00:00:00 2001 From: Dan Williams <dan.j.williams@intel.com> Date: Thu, 21 Aug 2014 11:43:24 -0700 Subject: [PATCH 3/5] SES: add enclosure logical id Export the NAA logical id for the enclosure. This is optionally available from the sas_transport_class, but it is really a property of the enclosure. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@suse.de> --- drivers/misc/enclosure.c | 13 +++++++++++++ drivers/scsi/ses.c | 9 +++++++++ include/linux/enclosure.h | 1 + 3 files changed, 23 insertions(+) diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 15faf61..18b87de 100644 --- a/drivers/misc/enclosure.c +++ b/drivers/misc/enclosure.c @@ -395,8 +395,21 @@ static ssize_t components_show(struct device *cdev, } static DEVICE_ATTR_RO(components); +static ssize_t id_show(struct device *cdev, + struct device_attribute *attr, + char *buf) +{ + struct enclosure_device *edev = to_enclosure_device(cdev); + + if (edev->cb->show_id) + return edev->cb->show_id(edev, buf); + return -EINVAL; +} +static DEVICE_ATTR_RO(id); + static struct attribute *enclosure_class_attrs[] = { &dev_attr_components.attr, + &dev_attr_id.attr, NULL, }; ATTRIBUTE_GROUPS(enclosure_class); diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 87cf970b..696d5d8 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -258,6 +258,14 @@ static int ses_set_active(struct enclosure_device *edev, return ses_set_page2_descriptor(edev, ecomp, desc); } +static int ses_show_id(struct enclosure_device *edev, char *buf) +{ + struct ses_device *ses_dev = edev->scratch; + unsigned long long id = get_unaligned_be64(ses_dev->page1+8+4); + + return sprintf(buf, "%#llx\n", id); +} + static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_fault = ses_get_fault, .set_fault = ses_set_fault, @@ -265,6 +273,7 @@ static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_locate = ses_get_locate, .set_locate = ses_set_locate, .set_active = ses_set_active, + .show_id = ses_show_id, }; struct ses_host_edev { diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index a835d33..807622b 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -79,6 +79,7 @@ struct enclosure_component_callbacks { int (*set_locate)(struct enclosure_device *, struct enclosure_component *, enum enclosure_component_setting); + int (*show_id)(struct enclosure_device *, char *buf); }; -- 1.8.1 > -----Original Message----- > From: Hannes Reinecke [mailto:hare@suse.de] > Sent: Thursday, September 4, 2014 12:55 AM > To: Song Liu; linux-scsi@vger.kernel.org > Cc: Dan Williams; Jens Axboe > Subject: Re: [PATCH 3/5] SES: add enclosure logical id > > On 08/25/2014 07:34 PM, Song Liu wrote: > > From: Song Liu [mailto:songliubraving@fb.com] > > Sent: Monday, August 25, 2014 10:26 AM > > To: Song Liu > > Cc: Dan Williams; Hannes Reinecke > > Subject: [PATCH 3/5] SES: add enclosure logical id > > > > From: Dan Williams <dan.j.williams@intel.com> > > > > Export the NAA logical id for the enclosure. This is optionally available > from the sas_transport_class, but it is really a property of the enclosure. > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Song Liu <songliubraving@fb.com> > > Reviewed-by: Jens Axboe <axboe@fb.com> > > Cc: Hannes Reinecke <hare@suse.de> > > --- > > drivers/misc/enclosure.c | 13 +++++++++++++ > > drivers/scsi/ses.c | 9 +++++++++ > > include/linux/enclosure.h | 1 + > > 3 files changed, 23 insertions(+) > > > > diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index > > 15faf61..646068a 100644 > > --- a/drivers/misc/enclosure.c > > +++ b/drivers/misc/enclosure.c > > @@ -395,8 +395,21 @@ static ssize_t components_show(struct device > > *cdev, } static DEVICE_ATTR_RO(components); > > > > +static ssize_t id_show(struct device *cdev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct enclosure_device *edev = to_enclosure_device(cdev); > > + > > + if (edev->cb->show_id) > > + return edev->cb->show_id(edev, buf); > > + return 0; > > +} > > +static DEVICE_ATTR_RO(id); > > + > > static struct attribute *enclosure_class_attrs[] = { > > &dev_attr_components.attr, > > + &dev_attr_id.attr, > > NULL, > > }; > > ATTRIBUTE_GROUPS(enclosure_class); > > Maybe you should return -EINVAL or something here; '0' would mean an > enclosure id of length '0', which is a different meaning from 'enclosure id not > available'. > > > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index > > 8f0a62a..61deb4e 100644 > > --- a/drivers/scsi/ses.c > > +++ b/drivers/scsi/ses.c > > @@ -258,6 +258,14 @@ static int ses_set_active(struct enclosure_device > *edev, > > return ses_set_page2_descriptor(edev, ecomp, desc); } > > > > +static int ses_show_id(struct enclosure_device *edev, char *buf) { > > + struct ses_device *ses_dev = edev->scratch; > > + unsigned long long id = get_unaligned_be64(ses_dev->page1+8+4); > > + > > + return sprintf(buf, "%#llx\n", id); > > +} > > + > > static struct enclosure_component_callbacks ses_enclosure_callbacks = { > > .get_fault = ses_get_fault, > > .set_fault = ses_set_fault, > > @@ -265,6 +273,7 @@ static struct enclosure_component_callbacks > ses_enclosure_callbacks = { > > .get_locate = ses_get_locate, > > .set_locate = ses_set_locate, > > .set_active = ses_set_active, > > + .show_id = ses_show_id, > > }; > > > > struct ses_host_edev { > > diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h > > index a835d33..807622b 100644 > > --- a/include/linux/enclosure.h > > +++ b/include/linux/enclosure.h > > @@ -79,6 +79,7 @@ struct enclosure_component_callbacks { > > int (*set_locate)(struct enclosure_device *, > > struct enclosure_component *, > > enum enclosure_component_setting); > > + int (*show_id)(struct enclosure_device *, char *buf); > > }; > > > > > > -- > > 1.8.1 > > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > hare@suse.de +49 911 74053 688 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <1408987546-2418-5-git-send-email-songliubraving@fb.com>]
* [PATCH 4/5] SES: add reliable slot attribute [not found] ` <1408987546-2418-5-git-send-email-songliubraving@fb.com> @ 2014-08-25 17:34 ` Song Liu [not found] ` <54081AEE.3030200@suse.de> 0 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-08-25 17:34 UTC (permalink / raw) To: linux-scsi@vger.kernel.org; +Cc: Dan Williams, Hannes Reinecke, Jens Axboe From: Song Liu [mailto:songliubraving@fb.com] Sent: Monday, August 25, 2014 10:26 AM To: Song Liu Cc: Dan Williams; Hannes Reinecke Subject: [PATCH 4/5] SES: add reliable slot attribute From: Dan Williams <dan.j.williams@intel.com> The name provided by firmware is in a vendor specific format, publish the slot number to have a reliable mechanism for identifying slots across firmware implementations. If the enclosure does not provide a slot number fallback to the component number which is guaranteed unique, and usually mirrors the slot number. Cleaned up the unused ses_component.desc in the process. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@suse.de> --- drivers/misc/enclosure.c | 20 +++++++++++++++++++- drivers/scsi/ses.c | 17 ++++++++++++----- include/linux/enclosure.h | 1 + 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 646068a..de335bc 100644 --- a/drivers/misc/enclosure.c +++ b/drivers/misc/enclosure.c @@ -145,8 +145,10 @@ enclosure_register(struct device *dev, const char *name, int components, if (err) goto err; - for (i = 0; i < components; i++) + for (i = 0; i < components; i++) { edev->component[i].number = -1; + edev->component[i].slot = -1; + } mutex_lock(&container_list_lock); list_add_tail(&edev->node, &container_list); @@ -552,6 +554,20 @@ static ssize_t get_component_type(struct device *cdev, return snprintf(buf, 40, "%s\n", enclosure_type[ecomp->type]); } +static ssize_t get_component_slot(struct device *cdev, + struct device_attribute *attr, char *buf) { + struct enclosure_component *ecomp = to_enclosure_component(cdev); + int slot; + + /* if the enclosure does not override then use 'number' as a stand-in */ + if (ecomp->slot >= 0) + slot = ecomp->slot; + else + slot = ecomp->number; + + return snprintf(buf, 40, "%d\n", slot); } static DEVICE_ATTR(fault, S_IRUGO | S_IWUSR, get_component_fault, set_component_fault); @@ -562,6 +578,7 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR, get_component_active, static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate, set_component_locate); static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL); +static DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL); static struct attribute *enclosure_component_attrs[] = { &dev_attr_fault.attr, @@ -569,6 +586,7 @@ static struct attribute *enclosure_component_attrs[] = { &dev_attr_active.attr, &dev_attr_locate.attr, &dev_attr_type.attr, + &dev_attr_slot.attr, NULL }; ATTRIBUTE_GROUPS(enclosure_component); diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 61deb4e..bafa301 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -47,7 +47,6 @@ struct ses_device { struct ses_component { u64 addr; - unsigned char *desc; }; static int ses_probe(struct device *dev) @@ -307,19 +306,26 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, int invalid = desc[0] & 0x80; enum scsi_protocol proto = desc[0] & 0x0f; u64 addr = 0; + int slot = -1; struct ses_component *scomp = ecomp->scratch; unsigned char *d; - scomp->desc = desc; - if (invalid) return; switch (proto) { + case SCSI_PROTOCOL_FCP: + if (eip) { + d = desc + 4; + slot = d[3]; + } + break; case SCSI_PROTOCOL_SAS: - if (eip) + if (eip) { + d = desc + 4; + slot = d[3]; d = desc + 8; - else + } else d = desc + 4; /* only take the phy0 addr */ addr = (u64)d[12] << 56 | @@ -335,6 +341,7 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, /* FIXME: Need to add more protocols than just SAS */ break; } + ecomp->slot = slot; scomp->addr = addr; } diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index 807622b..0f826c1 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -92,6 +92,7 @@ struct enclosure_component { int fault; int active; int locate; + int slot; enum enclosure_status status; }; -- 1.8.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <54081AEE.3030200@suse.de>]
* RE: [PATCH 4/5] SES: add reliable slot attribute [not found] ` <54081AEE.3030200@suse.de> @ 2014-09-12 21:04 ` Song Liu 0 siblings, 0 replies; 19+ messages in thread From: Song Liu @ 2014-09-12 21:04 UTC (permalink / raw) To: Hannes Reinecke, linux-scsi@vger.kernel.org; +Cc: Dan Williams, Jens Axboe >From 1cb4f9b4c57425cc3462b9615ac5bd013f7f4a88 Mon Sep 17 00:00:00 2001 From: Dan Williams <dan.j.williams@intel.com> Date: Thu, 21 Aug 2014 11:43:26 -0700 Subject: [PATCH 4/5] SES: add reliable slot attribute The name provided by firmware is in a vendor specific format, publish the slot number to have a reliable mechanism for identifying slots across firmware implementations. If the enclosure does not provide a slot number fallback to the component number which is guaranteed unique, and usually mirrors the slot number. Cleaned up the unused ses_component.desc in the process. Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Song Liu <songliubraving@fb.com> Reviewed-by: Jens Axboe <axboe@fb.com> Reviewed-by: Hannes Reinecke <hare@suse.de> --- drivers/misc/enclosure.c | 20 +++++++++++++++++++- drivers/scsi/ses.c | 17 ++++++++++++----- include/linux/enclosure.h | 1 + 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 18b87de..2e3eafa 100644 --- a/drivers/misc/enclosure.c +++ b/drivers/misc/enclosure.c @@ -145,8 +145,10 @@ enclosure_register(struct device *dev, const char *name, int components, if (err) goto err; - for (i = 0; i < components; i++) + for (i = 0; i < components; i++) { edev->component[i].number = -1; + edev->component[i].slot = -1; + } mutex_lock(&container_list_lock); list_add_tail(&edev->node, &container_list); @@ -552,6 +554,20 @@ static ssize_t get_component_type(struct device *cdev, return snprintf(buf, 40, "%s\n", enclosure_type[ecomp->type]); } +static ssize_t get_component_slot(struct device *cdev, + struct device_attribute *attr, char *buf) +{ + struct enclosure_component *ecomp = to_enclosure_component(cdev); + int slot; + + /* if the enclosure does not override then use 'number' as a stand-in */ + if (ecomp->slot >= 0) + slot = ecomp->slot; + else + slot = ecomp->number; + + return snprintf(buf, 40, "%d\n", slot); +} static DEVICE_ATTR(fault, S_IRUGO | S_IWUSR, get_component_fault, set_component_fault); @@ -562,6 +578,7 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR, get_component_active, static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate, set_component_locate); static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL); +static DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL); static struct attribute *enclosure_component_attrs[] = { &dev_attr_fault.attr, @@ -569,6 +586,7 @@ static struct attribute *enclosure_component_attrs[] = { &dev_attr_active.attr, &dev_attr_locate.attr, &dev_attr_type.attr, + &dev_attr_slot.attr, NULL }; ATTRIBUTE_GROUPS(enclosure_component); diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 696d5d8..3e59a5d 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -47,7 +47,6 @@ struct ses_device { struct ses_component { u64 addr; - unsigned char *desc; }; static int ses_probe(struct device *dev) @@ -307,19 +306,26 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, int invalid = desc[0] & 0x80; enum scsi_protocol proto = desc[0] & 0x0f; u64 addr = 0; + int slot = -1; struct ses_component *scomp = ecomp->scratch; unsigned char *d; - scomp->desc = desc; - if (invalid) return; switch (proto) { + case SCSI_PROTOCOL_FCP: + if (eip) { + d = desc + 4; + slot = d[3]; + } + break; case SCSI_PROTOCOL_SAS: - if (eip) + if (eip) { + d = desc + 4; + slot = d[3]; d = desc + 8; - else + } else d = desc + 4; /* only take the phy0 addr */ addr = (u64)d[12] << 56 | @@ -335,6 +341,7 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, /* FIXME: Need to add more protocols than just SAS */ break; } + ecomp->slot = slot; scomp->addr = addr; } diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index 807622b..0f826c1 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -92,6 +92,7 @@ struct enclosure_component { int fault; int active; int locate; + int slot; enum enclosure_status status; }; -- 1.8.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <1408987546-2418-6-git-send-email-songliubraving@fb.com>]
* [PATCH 5/5] SES: Add power_status to SES enclosure component [not found] ` <1408987546-2418-6-git-send-email-songliubraving@fb.com> @ 2014-08-25 17:34 ` Song Liu [not found] ` <54081BAA.1070304@suse.de> 0 siblings, 1 reply; 19+ messages in thread From: Song Liu @ 2014-08-25 17:34 UTC (permalink / raw) To: linux-scsi@vger.kernel.org; +Cc: Dan Williams, Hannes Reinecke, Jens Axboe From: Song Liu [mailto:songliubraving@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 <songliubraving@fb.com> Acked-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@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; +} + 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] &= 0xdf; break; case ENCLOSURE_SETTING_ENABLED: - desc[3] = 0x20; + desc[3] |= 0x20; break; default: /* SES doesn't do the SGPIO blink settings */ @@ -219,14 +241,22 @@ static int ses_set_locate(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[2] &= 0xfd; break; case ENCLOSURE_SETTING_ENABLED: - desc[2] = 0x02; + desc[2] |= 0x02; break; default: /* SES doesn't do the SGPIO blink settings */ @@ -239,15 +269,23 @@ static int ses_set_active(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[2] &= 0x7f; ecomp->active = 0; break; case ENCLOSURE_SETTING_ENABLED: - desc[2] = 0x80; + desc[2] |= 0x80; ecomp->active = 1; break; default: @@ -265,12 +303,53 @@ static int ses_show_id(struct enclosure_device *edev, char *buf) return sprintf(buf, "%#llx\n", id); } +static void ses_get_power_status(struct enclosure_device *edev, + struct enclosure_component *ecomp) { + unsigned char *desc; + + desc = ses_get_page2_descriptor(edev, ecomp); + if (desc) + ecomp->power_status = (desc[3] & 0x10) ? 0 : 1; } + +static int ses_set_power_status(struct enclosure_device *edev, + struct enclosure_component *ecomp, + int val) +{ + 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) { + /* power = 1 is device_off = 0 and vice versa */ + case 0: + desc[3] |= 0x10; + break; + case 1: + desc[3] &= 0xef; + break; + default: + return -EINVAL; + } + ecomp->power_status = val; + return ses_set_page2_descriptor(edev, ecomp, desc); } + static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_fault = ses_get_fault, .set_fault = ses_set_fault, .get_status = ses_get_status, .get_locate = ses_get_locate, .set_locate = ses_set_locate, + .get_power_status = ses_get_power_status, + .set_power_status = ses_set_power_status, .set_active = ses_set_active, .show_id = ses_show_id, }; @@ -448,6 +527,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, ecomp = &edev->component[components++]; if (!IS_ERR(ecomp)) { + ses_get_power_status(edev, ecomp); if (addl_desc_ptr) ses_process_descriptor(ecomp, addl_desc_ptr); diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index 0f826c1..7be22da 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -79,6 +79,11 @@ struct enclosure_component_callbacks { int (*set_locate)(struct enclosure_device *, struct enclosure_component *, enum enclosure_component_setting); + void (*get_power_status)(struct enclosure_device *, + struct enclosure_component *); + int (*set_power_status)(struct enclosure_device *, + struct enclosure_component *, + int); int (*show_id)(struct enclosure_device *, char *buf); }; @@ -94,6 +99,7 @@ struct enclosure_component { int locate; int slot; enum enclosure_status status; + int power_status; }; struct enclosure_device { -- 1.8.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <54081BAA.1070304@suse.de>]
* RE: [PATCH 5/5] SES: Add power_status to SES enclosure component [not found] ` <54081BAA.1070304@suse.de> @ 2014-09-12 21:07 ` Song Liu 2014-09-12 21:08 ` Song Liu 1 sibling, 0 replies; 19+ messages in thread From: Song Liu @ 2014-09-12 21:07 UTC (permalink / raw) To: Hannes Reinecke, linux-scsi@vger.kernel.org; +Cc: Dan Williams, Jens Axboe 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:hare@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:songliubraving@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 <songliubraving@fb.com> > > Acked-by: Dan Williams <dan.j.williams@intel.com> > > Reviewed-by: Jens Axboe <axboe@fb.com> > > Cc: Hannes Reinecke <hare@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] &= 0xdf; > > break; > > case ENCLOSURE_SETTING_ENABLED: > > - desc[3] = 0x20; > > + desc[3] |= 0x20; > > break; > > default: > > /* SES doesn't do the SGPIO blink settings */ @@ -219,14 > +241,22 @@ > > static int ses_set_locate(struct enclosure_device *edev, > Hmm? Garbled patch? > > > struct enclosure_component *ecomp, > > enum enclosure_component_setting val) { > > - unsigned char desc[4] = {0 }; > > + unsigned char desc[4]; > Why did you remove the initialisation here? > > > + 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[2] &= 0xfd; > > break; > > case ENCLOSURE_SETTING_ENABLED: > > - desc[2] = 0x02; > > + desc[2] |= 0x02; > > break; > > default: > > /* SES doesn't do the SGPIO blink settings */ @@ -239,15 > +269,23 @@ static int ses_set_active(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[2] &= 0x7f; > > ecomp->active = 0; > > break; > > case ENCLOSURE_SETTING_ENABLED: > > - desc[2] = 0x80; > > + desc[2] |= 0x80; > > ecomp->active = 1; > > break; > > default: > > @@ -265,12 +303,53 @@ static int ses_show_id(struct enclosure_device > *edev, char *buf) > > return sprintf(buf, "%#llx\n", id); > > } > > > > +static void ses_get_power_status(struct enclosure_device *edev, > > + struct enclosure_component *ecomp) { > > + unsigned char *desc; > > + > > + desc = ses_get_page2_descriptor(edev, ecomp); > > + if (desc) > > + ecomp->power_status = (desc[3] & 0x10) ? 0 : 1; } > > + > > +static int ses_set_power_status(struct enclosure_device *edev, > > + struct enclosure_component *ecomp, > > + int val) > > +{ > > + 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) { > > + /* power = 1 is device_off = 0 and vice versa */ > > + case 0: > > + desc[3] |= 0x10; > > + break; > > + case 1: > > + desc[3] &= 0xef; > > + break; > > + default: > > + return -EINVAL; > > + } > > + ecomp->power_status = val; > > + return ses_set_page2_descriptor(edev, ecomp, desc); } > > + > > static struct enclosure_component_callbacks ses_enclosure_callbacks = { > > .get_fault = ses_get_fault, > > .set_fault = ses_set_fault, > > .get_status = ses_get_status, > > .get_locate = ses_get_locate, > > .set_locate = ses_set_locate, > > + .get_power_status = ses_get_power_status, > > + .set_power_status = ses_set_power_status, > > .set_active = ses_set_active, > > .show_id = ses_show_id, > > }; > > @@ -448,6 +527,7 @@ static void ses_enclosure_data_process(struct > enclosure_device *edev, > > ecomp = &edev- > >component[components++]; > > > > if (!IS_ERR(ecomp)) { > > + ses_get_power_status(edev, ecomp); > > if (addl_desc_ptr) > > > ses_process_descriptor(ecomp, > > > addl_desc_ptr); > > diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h > > index 0f826c1..7be22da 100644 > > --- a/include/linux/enclosure.h > > +++ b/include/linux/enclosure.h > > @@ -79,6 +79,11 @@ struct enclosure_component_callbacks { > > int (*set_locate)(struct enclosure_device *, > > struct enclosure_component *, > > enum enclosure_component_setting); > > + void (*get_power_status)(struct enclosure_device *, > > + struct enclosure_component *); > > + int (*set_power_status)(struct enclosure_device *, > > + struct enclosure_component *, > > + int); > > int (*show_id)(struct enclosure_device *, char *buf); }; > > > > @@ -94,6 +99,7 @@ struct enclosure_component { > > int locate; > > int slot; > > enum enclosure_status status; > > + int power_status; > > }; > > > > struct enclosure_device { > > -- > > 1.8.1 > > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > hare@suse.de +49 911 74053 688 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 5/5] SES: Add power_status to SES enclosure component [not found] ` <54081BAA.1070304@suse.de> 2014-09-12 21:07 ` Song Liu @ 2014-09-12 21:08 ` Song Liu 1 sibling, 0 replies; 19+ messages in thread From: Song Liu @ 2014-09-12 21:08 UTC (permalink / raw) To: Hannes Reinecke, linux-scsi@vger.kernel.org; +Cc: Dan Williams, Jens Axboe From 53549716d9f965e59f3a84feb5ebae9d18232b52 Mon Sep 17 00:00:00 2001 From: Song Liu <songliubraving@fb.com> Date: Tue, 19 Aug 2014 23:58:58 -0700 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 <songliubraving@fb.com> Acked-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Jens Axboe <axboe@fb.com> Cc: Hannes Reinecke <hare@suse.de> --- drivers/misc/enclosure.c | 38 ++++++++++++++++++ drivers/scsi/ses.c | 98 ++++++++++++++++++++++++++++++++++++++++++----- include/linux/enclosure.h | 6 +++ 3 files changed, 133 insertions(+), 9 deletions(-) diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 2e3eafa..819f2f2 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,40 @@ 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, "%s\n", ecomp->power_status ? "on" : "off"); +} + +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; + + if (strncmp(buf, "on", 2) == 0 && + (buf[2] == '\n' || buf[2] == '\0')) + val = 1; + else if (strncmp(buf, "off", 3) == 0 && + (buf[3] == '\n' || buf[3] == '\0')) + val = 0; + else + return -EINVAL; + + if (edev->cb->set_power_status) + edev->cb->set_power_status(edev, ecomp, val); + return count; +} + static ssize_t get_component_type(struct device *cdev, struct device_attribute *attr, char *buf) { @@ -577,6 +612,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 +622,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 3e59a5d..8ba3c78 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] &= 0xdf; break; case ENCLOSURE_SETTING_ENABLED: - desc[3] = 0x20; + desc[3] |= 0x20; break; default: /* SES doesn't do the SGPIO blink settings */ @@ -219,14 +241,22 @@ static int ses_set_locate(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[2] &= 0xfd; break; case ENCLOSURE_SETTING_ENABLED: - desc[2] = 0x02; + desc[2] |= 0x02; break; default: /* SES doesn't do the SGPIO blink settings */ @@ -239,15 +269,23 @@ static int ses_set_active(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[2] &= 0x7f; ecomp->active = 0; break; case ENCLOSURE_SETTING_ENABLED: - desc[2] = 0x80; + desc[2] |= 0x80; ecomp->active = 1; break; default: @@ -265,12 +303,53 @@ static int ses_show_id(struct enclosure_device *edev, char *buf) return sprintf(buf, "%#llx\n", id); } +static void ses_get_power_status(struct enclosure_device *edev, + struct enclosure_component *ecomp) +{ + unsigned char *desc; + + desc = ses_get_page2_descriptor(edev, ecomp); + if (desc) + ecomp->power_status = (desc[3] & 0x10) ? 0 : 1; +} + +static int ses_set_power_status(struct enclosure_device *edev, + struct enclosure_component *ecomp, + int val) +{ + 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) { + /* power = 1 is device_off = 0 and vice versa */ + case 0: + desc[3] |= 0x10; + break; + case 1: + desc[3] &= 0xef; + break; + default: + return -EINVAL; + } + ecomp->power_status = val; + return ses_set_page2_descriptor(edev, ecomp, desc); +} + static struct enclosure_component_callbacks ses_enclosure_callbacks = { .get_fault = ses_get_fault, .set_fault = ses_set_fault, .get_status = ses_get_status, .get_locate = ses_get_locate, .set_locate = ses_set_locate, + .get_power_status = ses_get_power_status, + .set_power_status = ses_set_power_status, .set_active = ses_set_active, .show_id = ses_show_id, }; @@ -449,6 +528,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, ecomp = &edev->component[components++]; if (!IS_ERR(ecomp)) { + ses_get_power_status(edev, ecomp); if (addl_desc_ptr) ses_process_descriptor( ecomp, diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index 0f826c1..7be22da 100644 --- a/include/linux/enclosure.h +++ b/include/linux/enclosure.h @@ -79,6 +79,11 @@ struct enclosure_component_callbacks { int (*set_locate)(struct enclosure_device *, struct enclosure_component *, enum enclosure_component_setting); + void (*get_power_status)(struct enclosure_device *, + struct enclosure_component *); + int (*set_power_status)(struct enclosure_device *, + struct enclosure_component *, + int); int (*show_id)(struct enclosure_device *, char *buf); }; @@ -94,6 +99,7 @@ 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:hare@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:songliubraving@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 <songliubraving@fb.com> > > > Acked-by: Dan Williams <dan.j.williams@intel.com> > > > Reviewed-by: Jens Axboe <axboe@fb.com> > > > Cc: Hannes Reinecke <hare@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] &= 0xdf; > > > break; > > > case ENCLOSURE_SETTING_ENABLED: > > > - desc[3] = 0x20; > > > + desc[3] |= 0x20; > > > break; > > > default: > > > /* SES doesn't do the SGPIO blink settings */ @@ -219,14 > > +241,22 @@ > > > static int ses_set_locate(struct enclosure_device *edev, > > Hmm? Garbled patch? > > > > > struct enclosure_component *ecomp, > > > enum enclosure_component_setting val) { > > > - unsigned char desc[4] = {0 }; > > > + unsigned char desc[4]; > > Why did you remove the initialisation here? > > > > > + 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[2] &= 0xfd; > > > break; > > > case ENCLOSURE_SETTING_ENABLED: > > > - desc[2] = 0x02; > > > + desc[2] |= 0x02; > > > break; > > > default: > > > /* SES doesn't do the SGPIO blink settings */ @@ -239,15 > > +269,23 @@ static int ses_set_active(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[2] &= 0x7f; > > > ecomp->active = 0; > > > break; > > > case ENCLOSURE_SETTING_ENABLED: > > > - desc[2] = 0x80; > > > + desc[2] |= 0x80; > > > ecomp->active = 1; > > > break; > > > default: > > > @@ -265,12 +303,53 @@ static int ses_show_id(struct enclosure_device > > *edev, char *buf) > > > return sprintf(buf, "%#llx\n", id); } > > > > > > +static void ses_get_power_status(struct enclosure_device *edev, > > > + struct enclosure_component *ecomp) { > > > + unsigned char *desc; > > > + > > > + desc = ses_get_page2_descriptor(edev, ecomp); > > > + if (desc) > > > + ecomp->power_status = (desc[3] & 0x10) ? 0 : 1; } > > > + > > > +static int ses_set_power_status(struct enclosure_device *edev, > > > + struct enclosure_component *ecomp, > > > + int val) > > > +{ > > > + 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) { > > > + /* power = 1 is device_off = 0 and vice versa */ > > > + case 0: > > > + desc[3] |= 0x10; > > > + break; > > > + case 1: > > > + desc[3] &= 0xef; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + ecomp->power_status = val; > > > + return ses_set_page2_descriptor(edev, ecomp, desc); } > > > + > > > static struct enclosure_component_callbacks ses_enclosure_callbacks = > { > > > .get_fault = ses_get_fault, > > > .set_fault = ses_set_fault, > > > .get_status = ses_get_status, > > > .get_locate = ses_get_locate, > > > .set_locate = ses_set_locate, > > > + .get_power_status = ses_get_power_status, > > > + .set_power_status = ses_set_power_status, > > > .set_active = ses_set_active, > > > .show_id = ses_show_id, > > > }; > > > @@ -448,6 +527,7 @@ static void ses_enclosure_data_process(struct > > enclosure_device *edev, > > > ecomp = &edev- > > >component[components++]; > > > > > > if (!IS_ERR(ecomp)) { > > > + ses_get_power_status(edev, ecomp); > > > if (addl_desc_ptr) > > > > > ses_process_descriptor(ecomp, > > > > > addl_desc_ptr); > > > diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h > > > index 0f826c1..7be22da 100644 > > > --- a/include/linux/enclosure.h > > > +++ b/include/linux/enclosure.h > > > @@ -79,6 +79,11 @@ struct enclosure_component_callbacks { > > > int (*set_locate)(struct enclosure_device *, > > > struct enclosure_component *, > > > enum enclosure_component_setting); > > > + void (*get_power_status)(struct enclosure_device *, > > > + struct enclosure_component *); > > > + int (*set_power_status)(struct enclosure_device *, > > > + struct enclosure_component *, > > > + int); > > > int (*show_id)(struct enclosure_device *, char *buf); }; > > > > > > @@ -94,6 +99,7 @@ struct enclosure_component { > > > int locate; > > > int slot; > > > enum enclosure_status status; > > > + int power_status; > > > }; > > > > > > struct enclosure_device { > > > -- > > > 1.8.1 > > > > > Cheers, > > > > Hannes > > -- > > Dr. Hannes Reinecke zSeries & Storage > > hare@suse.de +49 911 74053 688 > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 0/5] Feature enhancements for ses module
@ 2014-12-30 22:46 Song Liu
2015-01-05 19:26 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Song Liu @ 2014-12-30 22:46 UTC (permalink / raw)
To: linux-scsi; +Cc: hch, hare, dgilbert, Song Liu
1: close potential race condition by at enclosure race condition
2,3,4: add enclosure id and device slot, so we can create symlink
in /dev/disk/by-slot:
# ls -d /dev/disk/by-slot/*
/dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0
5: add ability to power on/off device with power_status file in
sysfs.
Due to the complexity of SES standard, the module is not to replace implement \
all features of sg_ses (sg3_utils).
Patch 5 and existing features for device element and array device elements control \
of HDDs. It is helpful to handle some HDD related fields in the kernel, as the \
kernel can generate mapping between a device to the SES device element (or array \
device element):
/sys/block/sdc/device/enclosure_deviceXXX/
With patch 5, we can easily power off a running HDD by
echo off > /sys/block/sdc/device/enclosure_deviceXXX/power_status
This is very useful for systems like Cold Storage, where HDDs are being powered \
on/off frequently
Dan Williams (4):
ses: close potential registration race
ses: generate KOBJ_CHANGE on enclosure attach
ses: add enclosure logical id
ses: add reliable slot attribute
Song Liu (1):
ses: Add power_status to SES device slot
drivers/misc/enclosure.c | 106 +++++++++++++++++++++++++++++----
drivers/scsi/ses.c | 148 +++++++++++++++++++++++++++++++++++++++-------
include/linux/enclosure.h | 13 +++-
3 files changed, 232 insertions(+), 35 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 0/5] Feature enhancements for ses module 2014-12-30 22:46 [PATCH 0/5] Feature enhancements for ses module Song Liu @ 2015-01-05 19:26 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2015-01-05 19:26 UTC (permalink / raw) To: Song Liu; +Cc: linux-scsi, hare, dgilbert Thanks, applied to scsi-for-3.20. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-01-05 19:26 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1408987546-2418-1-git-send-email-songliubraving@fb.com>
2014-08-25 17:34 ` [PATCH 0/5] Feature enhancements for ses module Song Liu
2014-10-22 19:12 ` Jens Axboe
2014-10-22 22:28 ` Douglas Gilbert
2014-10-22 23:01 ` Song Liu
2014-10-23 1:17 ` Douglas Gilbert
2014-10-23 3:42 ` Song Liu
2014-10-28 0:32 ` Song Liu
2014-10-23 8:45 ` Christoph Hellwig
[not found] ` <1408987546-2418-3-git-send-email-songliubraving@fb.com>
2014-08-25 17:34 ` [PATCH 2/5] SES: generate KOBJ_CHANGE on enclosure attach Song Liu
[not found] ` <54081A32.9070303@suse.de>
2014-09-12 21:03 ` Song Liu
[not found] ` <1408987546-2418-4-git-send-email-songliubraving@fb.com>
2014-08-25 17:34 ` [PATCH 3/5] SES: add enclosure logical id Song Liu
[not found] ` <54081ABD.9080104@suse.de>
2014-09-12 21:04 ` Song Liu
[not found] ` <1408987546-2418-5-git-send-email-songliubraving@fb.com>
2014-08-25 17:34 ` [PATCH 4/5] SES: add reliable slot attribute Song Liu
[not found] ` <54081AEE.3030200@suse.de>
2014-09-12 21:04 ` Song Liu
[not found] ` <1408987546-2418-6-git-send-email-songliubraving@fb.com>
2014-08-25 17:34 ` [PATCH 5/5] SES: Add power_status to SES enclosure component Song Liu
[not found] ` <54081BAA.1070304@suse.de>
2014-09-12 21:07 ` Song Liu
2014-09-12 21:08 ` Song Liu
2014-12-30 22:46 [PATCH 0/5] Feature enhancements for ses module Song Liu
2015-01-05 19:26 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).