--- On Tue, 2/12/08, James Bottomley James.Bottomley@HansenPartnership.com> wrote:

quoted

I apologize for taking so long to review this patch. 

I obviously agree

quoted

wholeheartedly with Luben.  The problem I ran into

while trying to

quoted

design an enclosure management interface for the SATA

devices is that

quoted

there is all this vendor defined stuff.  For example,

for the AHCI LED

quoted

protocol, the only "defined" LED is

'activity'.  For LED2 and LED3 it

quoted

is up to hardware vendors to define these.  For SGPIO

there's all kinds

quoted

of ways for hw vendors to customize.  I felt that it

was going to be a

quoted

maintainance nightmare to have to keep track of

various vendors

quoted

enclosure implementations in the ahci driver, and that

it'd be better

quoted

to just have user space libraries take care of that. 

Plus, that way a

quoted

vendor doesn't have to get a patch into the kernel

to get their new

quoted

spiffy wizzy bang blinky lights working (think of how

long it takes

quoted

something to even get into a vendor kernel, which is

what these guys

quoted

care about...).  So I'm still not sold on having

an enclosure

quoted

abstraction in the kernel - at least for the SATA

controllers.

Correct me if I'm wrong, but didn't the original
AHCI enclosure patch
expose activity LEDs via sysfs?

I'm not saying there aren't a lot of non standard
pieces that need to be
activated by direct commands or other user activated
protocol.  I am
saying there are a lot of standard pieces that we could do
with showing
in a uniform manner.

Which is already the case without the SES kernel bloat.
Case in point, the excellent user-space application
"lsscsi" would clearly show which device is SES.

And the excellent user-space application "sg_ses" could
control an SES device.

The pieces I think are absolutely standard are

1. Actual enclosure presence (is this device in an
enclosure)

"lsscsi"

2. Activity LED, this seems to be a feature of every
enclosure.

So that means that it needs a kernel representation?
If this indeed were the case, for every "feature" of every
type of device (not only SCSI) then the kernel itself would
be insurmountably big.

I also think the following are reasonably standard (based
on the fact
that most enclosure standards recommend but don't
require this):

3. Locate LED (for locating the device).  Even if you only
have an
activity LED, this is usually done by flashing the activity
LED in a
well defined pattern.
4. Fault.  this is the least standardised of the lot, but
does seem to
be present in about every enclosure implementation.

All I've done is standardise these four pieces ... the
services actually
take into account that it might not be possible to do
certain of these
(like fault).

And none of this means that it needs a kernel representation.

1. You're not "standardizing" any known, in-practice,
kernel representation, that is already in practice and
thusly needs a kernel representation.

2. The kernel itself is not using nor needing this
"representation" in order to function properly (the kernel).

Leaving control of SES devices to user-space makes both
the kernel and the vendors happy.  All the kernel needs
to do is expose the SES device to user-space as it currently
does.  It makes it so much easier both to vendors and to
the kernel to stay out of unnecessary representations.

Vendors may choose to distribute their own applications
to control their hardware, as long as the kernel exposes
an SES device and provides functionality, as opposed to
policy of any kind.

    Luben