Re: Laptop shock detection and harddisk protection

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

Hi!

> > On a related note, is there any plan to merge tp_smapi to mainline?
> > It seems you put a lot of work into it and I don't really see why it
> > should stay out of tree.
> 
> The only issue I'm aware of is finding a reasonably-named maintainer.

I'm willing to maintain this.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

On Fri 2008-09-12 09:59:47, Greg KH wrote:
> On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> > Shem Multinymous wrote:
> > >> That reduction comes because input device supports poll and
> > >> sysfs_notify_event() does about the same thing.  The uesrland daemon
> > >> can just poll on a node and read data nodes when poll event on the
> > >> node triggeres.
> > > 
> > > Agreed.
> > > There's another issue with the current sysfs interface, though: hdapsd
> > > needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> > > and y in separate attributes which cannot be read atomically together.
> > > We can add a sysfs file with "x y timestamp" readouts, though this is
> > > unusual for sysfs (and certainly incompatible with hwmon).
> > 
> > Yes, right.  Forgot about the atomicity part altogether.  Thanks for
> > bringing it up.
> > 
> > >> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
> > >> nodes, so I don't think it's a good idea to push out actual unloading
> > >> to another process especially as fork doesn't inherit mlockall.
> > > 
> > > I had in mind another daemon listening for "unload now" events, so no
> > > forking needed.
> > > This second daemon might make sense if we push the logic of deciding
> > > *which* disks to unload into userspace, since this logic is the same
> > > for the ThinkPad style and the HP style.
> > 
> > Hmmm... I can't (yet) see the benefit of having two separate userland
> > daemons.
> > 
> > >> On a related note, is there any plan to merge tp_smapi to mainline?
> > >> It seems you put a lot of work into it and I don't really see why it
> > >> should stay out of tree.
> > > 
> > > The only issue I'm aware of is finding a reasonably-named maintainer.
> > > On the technical side, the reviews on my lkml submission of
> > > thinkpad_ec+hdaps seemed good and all technical comments are since
> > > addressed. The code has been stable, well-tested and packaged by major
> > > distros for years.
> > 
> > Cool, can you please post the patch to the lkml and cc Greg
> > Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> > <akpm@linux-foundation.org> and me?
> 
> Sorry, but no, I can't accept this code as it is coming from a "known
> anonymous" person containing information that it is not known where it
> came from.

So... what are you worried about?

> In short, "Signed-off-by:" from people who are known to be anonymous is
> not allowed.

That's okay. Signed-off-by: by original author is not required for
mainline merge. All that is required is submitter legally getting it
under GPL, and signing that off.

Now.. what are you worried about?

a) Copyrights?

b) Patents?

c) Trademarks?

d) Trade secrets?

e) Contracts between Shem and ...?

f) Some other contracts?

g) any other applicable laws? which?

h) anything else?

i) anything you can't talk about because NDAs or similar?

Based on the answer, we can try to figure way forward. 

('power_now' is really useful for me; useful enough that I think about
reverse-engineering it from tp_smapi and reimplementation. Maybe even
doing it chinese-wall way if I can find second interested person. But
I need to know what the concerns are. ...plus, I guess Ciaran can help
with legal questions....)

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

Hi!

> >> 2. If we're gonna unify interface, how much can we unify the backend?
> >>   Some devices are based on polling, others interrupt.  For polling,
> >>   is it better to delegate the whole polling to userland or is it
> >>   better to do some of it in kernel (tp_smapi seems to be doing
> >>   this)?
> > 
> > The ThinkPad accelerometer needs to be polled at very regular
> > intervals (max jitter on the order of 10ms), which sounds like a job
> > for the kernel.
> 
> Yes, I agree.
> 
> > This is because in ThinkPads we actually have a 4-level pile:
> > [hdapsd userspace] -> [hdaps kernel] -> [embedded controller] ->
> > [accelerometer A2D]
> > What the kernel polls is actually is the H8S embedded controller (EC)
> > chip, which in turn does its own polling of the accelerometer A2D.
> > Now, the EC has a tiny buffer and strange buffering semantics, and it
> > has its own internal clock, so the software->EC polling should be very
> > regular to minmize EC buffer overflows/underruns.
> 
> So, I think the whole polling should be implemented inside the kernel
> and the kernel should notify userland when new data is available,
> which is about what the current joystick implementation does and can
> be achieved using sysfs_notify_event().

I like joystick/input interface slightly better. In some cases,
machines with accelerometers (openmoko) use them for input primarily.

HP interface will be more specialized (but less useful); still  userland daemon can handle the differences...

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

Hi!

> Short of a satisfying proposition regarding the questions raised, I just
> want to add two things that would be nice to solve in the future one way
> or another and should perhaps be taken into consideration from the
> beginning:
> 
> 1. Disable polling completely when it isn't required: once the hd has
>    spun down, there is no need to keep polling the sensors at all. Only
>    when the first request requiring the hd to spin up arrives, the
>    kernel needs to hold back for a short while to gather enough data
>    from the sensors, so shock protection is up and running again.

hdparm can already tell if disk is spinning or not. As userland is
polling, already, that may be enough?

> 2. Make shock protection interact nicely with suspend operations:
>    currently, we are out of luck if anything should happen after
>    processes have been frozen. This is particularly unfortunatel in the
>    case of s2disk.

I'd say that s2disk is similar to early boot... no protection there. 
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Laptop shock detection and harddisk protection

[Tejun Heo]

Hello, all.

Elias has been sending libata/ide shock protection patches[1] and for
libata I think it's only one or two iterations from being included.
The interface is pretty simple, it only has to write timeout values to
sysfs nodes for all disks.

Thomas is now working on implementing shock detection for HP laptops
where the interface is much simpler than the HDAPS one.  Interrupts
for danger imminent and danger over plus control for warning LED.

I browsed a little bit for HDAPS one and it seems all the pieces are
there but scattered.  The latest effort seems tp_smapi which Shem
Multinymous is working on.

So, overall, all the pieces are falling into places but many pieces
are not upstream yet and there also are interface differences for
different detection drivers.  The original hdaps uses polling on sysfs
nodes, the HP thing Thomas is working on is likely to use sysfs +
sysfs_notify_event() and probably another node to control LED.  The
tp_smapi seems to use joystick input device to transfer the data along
all the axises.

So, I think it's about time to decide how to proceed on this
acceleration detection and shock protection thing.

1. How should the shock interface look like?  As we're gonna need
   userland daemon one way or the other, we can use the userland
   daemon to glue all the interfaces but it would be much better to
   have a unified interface.  Although there seem to be several
   different variants, they don't differ all that much and creating a
   new interface every time is painful.  I think we can get by with a
   sysfs interface with notification.

2. If we're gonna unify interface, how much can we unify the backend?
   Some devices are based on polling, others interrupt.  For polling,
   is it better to delegate the whole polling to userland or is it
   better to do some of it in kernel (tp_smapi seems to be doing
   this)?

3. What about the userland daemon?  It would be best to have a unified
   daemon which can handle all instead of one for hdaps and another
   for hp (and so on).  If we can unify the interface, this will be
   much easier.

Thanks.

-- 
tejun

[1] http://thread.gmane.org/gmane.linux.ide/34103

Re: Laptop shock detection and harddisk protection

[Renato S. Yamane]

Tejun Heo escreveu:
> I browsed a little bit for HDAPS one and it seems all the pieces are
> there but scattered.  The latest effort seems tp_smapi which Shem
> Multinymous is working on.

I have a Lenovo Thinkpad T61 running tp_smapi and hdapsd over 2.6.26.2
Kernel with Elias patch:
<http://article.gmane.org/gmane.linux.drivers.hdaps.devel/1324/raw>

If you need some info, pls let me know.

> 1. How should the shock interface look like?  As we're gonna need
>    userland daemon one way or the other, we can use the userland
>    daemon to glue all the interfaces but it would be much better to
>    have a unified interface.  Although there seem to be several
>    different variants, they don't differ all that much and creating a
>    new interface every time is painful.  I think we can get by with a
>    sysfs interface with notification.

IMHO, userland daemon need only inform users when hdd heads was parking.
This is what KHDAPSMonitor do (<http://roy.marples.name/node/269>).
A daemon like this is used in Windows, but one more feature is present:
Is possible choose "Manual Unlock" (hdd heads is kept parked until I
click over daemon to unlock).

Best regards,
Renato S. Yamane

Re: Laptop shock detection and harddisk protection

[Austin Zhang]

On Wed, 2008-09-10 at 18:59 +0200, Tejun Heo wrote:

> 2. If we're gonna unify interface, how much can we unify the backend?
>    Some devices are based on polling, others interrupt.  For polling,
>    is it better to delegate the whole polling to userland or is it
>    better to do some of it in kernel (tp_smapi seems to be doing
>    this)?
Shock protection should be time-sensitive, if we put the whole polling 
into userland, will it be possible that the damage had happened before 
userland app can signal ATA idle command timely?

> 3. What about the userland daemon?  It would be best to have a unified
>    daemon which can handle all instead of one for hdaps and another
>    for hp (and so on).  If we can unify the interface, this will be
>    much easier.
> 
> Thanks.

Can this process "acceleration-detect --> inform ATA shock protect --> 
issue idle command" be done totally in kernel, avoiding to consume too 
many time for "acceleration-detect --> sysfs --> userland app --> sysfs 
--> inform ATA shock protect --> issue idle command" before HD was damaged? 
The userland daemon should be just a indicator (but of course it can pass 
params to driver) for the protection status rather than a judge.

Re: Laptop shock detection and harddisk protection

[Tejun Heo]

Austin Zhang wrote:
>> 2. If we're gonna unify interface, how much can we unify the backend?
>>    Some devices are based on polling, others interrupt.  For polling,
>>    is it better to delegate the whole polling to userland or is it
>>    better to do some of it in kernel (tp_smapi seems to be doing
>>    this)?
> Shock protection should be time-sensitive, if we put the whole polling 
> into userland, will it be possible that the damage had happened before 
> userland app can signal ATA idle command timely?

Yeah, it's time sensitive but it seems latency of tens of millisecs is
good enough and with mlocked user process, it's really not a problem.

>> 3. What about the userland daemon?  It would be best to have a unified
>>    daemon which can handle all instead of one for hdaps and another
>>    for hp (and so on).  If we can unify the interface, this will be
>>    much easier.
>>
>> Thanks.
> 
> Can this process "acceleration-detect --> inform ATA shock protect --> 
> issue idle command" be done totally in kernel, avoiding to consume too 
> many time for "acceleration-detect --> sysfs --> userland app --> sysfs 
> --> inform ATA shock protect --> issue idle command" before HD was damaged? 
> The userland daemon should be just a indicator (but of course it can pass 
> params to driver) for the protection status rather than a judge.

Again, it doesn't have to be that fast and the judgement part involves
complex floating arithmetics + user usage patterns (has the user typed
something recently, is lid closed kind of stuff).  I don't think it
fits in kernel.

Thanks.

-- 
tejun

Re: Laptop shock detection and harddisk protection

[Shem Multinymous]

Hi Tejun,

On Wed, Sep 10, 2008 at 12:59 PM, Tejun Heo <tj@kernel.org> wrote:

>The original hdaps uses polling on sysfs
> nodes, the HP thing Thomas is working on is likely to use sysfs +
> sysfs_notify_event() and probably another node to control LED.  The
> tp_smapi seems to use joystick input device to transfer the data along
> all the axises.

> 1. How should the shock interface look like?  As we're gonna need
>   userland daemon one way or the other, we can use the userland
>   daemon to glue all the interfaces but it would be much better to
>   have a unified interface.  Although there seem to be several
>   different variants, they don't differ all that much and creating a
>   new interface every time is painful.  I think we can get by with a
>   sysfs interface with notification.

Using the input device interface for the accelerometer (as done by
tp_smapi's hdaps + latest hdapsd) greatly reduces the number of
accelerometer-related timer interrupts on tickless kernels, as
measured by powertop. With syscall polling you have the kernal polling
the hardware at ~50Hz and then the userspace hdapsd polling the kernel
at ~50Hz. When they're out of phase so you can get up to 100
interrupts/sec. With an input device you're always at 50Hz. The phase
difference also induces a small extra delay in the shock handling
response.


> 2. If we're gonna unify interface, how much can we unify the backend?
>   Some devices are based on polling, others interrupt.  For polling,
>   is it better to delegate the whole polling to userland or is it
>   better to do some of it in kernel (tp_smapi seems to be doing
>   this)?

The ThinkPad accelerometer needs to be polled at very regular
intervals (max jitter on the order of 10ms), which sounds like a job
for the kernel.
This is because in ThinkPads we actually have a 4-level pile:
[hdapsd userspace] -> [hdaps kernel] -> [embedded controller] ->
[accelerometer A2D]
What the kernel polls is actually is the H8S embedded controller (EC)
chip, which in turn does its own polling of the accelerometer A2D.
Now, the EC has a tiny buffer and strange buffering semantics, and it
has its own internal clock, so the software->EC polling should be very
regular to minmize EC buffer overflows/underruns.


> 3. What about the userland daemon?  It would be best to have a unified
>   daemon which can handle all instead of one for hdaps and another
>   for hp (and so on).  If we can unify the interface, this will be
>   much easier.

Assuming the funky shock detection algorithms are done in userspace,
we need two interfaces: one for the acceleration data (an input device
works nicely) and one for "unload now" events.
On HP, the kernel can trigger the "unload now" event directly. On
ThinkPads, there's still the question of what the userspace shock
detection daemon should do when it detects a shock: should it unloads
the heads by itself, or trigger the generic "unload now" event and let
someone else do the actual unloading.

  Shem

Re: Laptop shock detection and harddisk protection

[Tejun Heo]

Hello, Shem Multinymous.

Shem Multinymous wrote:
> Hi Tejun,
>> 1. How should the shock interface look like?  As we're gonna need
>>   userland daemon one way or the other, we can use the userland
>>   daemon to glue all the interfaces but it would be much better to
>>   have a unified interface.  Although there seem to be several
>>   different variants, they don't differ all that much and creating a
>>   new interface every time is painful.  I think we can get by with a
>>   sysfs interface with notification.
> 
> Using the input device interface for the accelerometer (as done by
> tp_smapi's hdaps + latest hdapsd) greatly reduces the number of
> accelerometer-related timer interrupts on tickless kernels, as
> measured by powertop. With syscall polling you have the kernal polling
> the hardware at ~50Hz and then the userspace hdapsd polling the kernel
> at ~50Hz. When they're out of phase so you can get up to 100
> interrupts/sec. With an input device you're always at 50Hz. The phase
> difference also induces a small extra delay in the shock handling
> response.

That reduction comes because input device supports poll and
sysfs_notify_event() does about the same thing.  The uesrland daemon
can just poll on a node and read data nodes when poll event on the
node triggeres.

>> 2. If we're gonna unify interface, how much can we unify the backend?
>>   Some devices are based on polling, others interrupt.  For polling,
>>   is it better to delegate the whole polling to userland or is it
>>   better to do some of it in kernel (tp_smapi seems to be doing
>>   this)?
> 
> The ThinkPad accelerometer needs to be polled at very regular
> intervals (max jitter on the order of 10ms), which sounds like a job
> for the kernel.

Yes, I agree.

> This is because in ThinkPads we actually have a 4-level pile:
> [hdapsd userspace] -> [hdaps kernel] -> [embedded controller] ->
> [accelerometer A2D]
> What the kernel polls is actually is the H8S embedded controller (EC)
> chip, which in turn does its own polling of the accelerometer A2D.
> Now, the EC has a tiny buffer and strange buffering semantics, and it
> has its own internal clock, so the software->EC polling should be very
> regular to minmize EC buffer overflows/underruns.

So, I think the whole polling should be implemented inside the kernel
and the kernel should notify userland when new data is available,
which is about what the current joystick implementation does and can
be achieved using sysfs_notify_event().

>> 3. What about the userland daemon?  It would be best to have a unified
>>   daemon which can handle all instead of one for hdaps and another
>>   for hp (and so on).  If we can unify the interface, this will be
>>   much easier.
> 
> Assuming the funky shock detection algorithms are done in userspace,
> we need two interfaces: one for the acceleration data (an input device
> works nicely) and one for "unload now" events.
> On HP, the kernel can trigger the "unload now" event directly. On
> ThinkPads, there's still the question of what the userspace shock
> detection daemon should do when it detects a shock: should it unloads
> the heads by itself, or trigger the generic "unload now" event and let
> someone else do the actual unloading.

Unloading heads will be simple.  Just echoing timeout in ms to sysfs
nodes, so I don't think it's a good idea to push out actual unloading
to another process especially as fork doesn't inherit mlockall.

On a related note, is there any plan to merge tp_smapi to mainline?
It seems you put a lot of work into it and I don't really see why it
should stay out of tree.

Thanks.

-- 
tejun

Re: Laptop shock detection and harddisk protection

[Elias Oltmanns]

Tejun Heo <tj@kernel.org> wrote:
> Hello, Shem Multinymous.
>
> Shem Multinymous wrote:
[...]
>>> 2. If we're gonna unify interface, how much can we unify the backend?
>>>   Some devices are based on polling, others interrupt.  For polling,
>>>   is it better to delegate the whole polling to userland or is it
>>>   better to do some of it in kernel (tp_smapi seems to be doing
>>>   this)?
>> 
>> The ThinkPad accelerometer needs to be polled at very regular
>> intervals (max jitter on the order of 10ms), which sounds like a job
>> for the kernel.
>
> Yes, I agree.
>
>> This is because in ThinkPads we actually have a 4-level pile:
>> [hdapsd userspace] -> [hdaps kernel] -> [embedded controller] ->
>> [accelerometer A2D]
>> What the kernel polls is actually is the H8S embedded controller (EC)
>> chip, which in turn does its own polling of the accelerometer A2D.
>> Now, the EC has a tiny buffer and strange buffering semantics, and it
>> has its own internal clock, so the software->EC polling should be very
>> regular to minmize EC buffer overflows/underruns.
>
> So, I think the whole polling should be implemented inside the kernel
> and the kernel should notify userland when new data is available,
> which is about what the current joystick implementation does and can
> be achieved using sysfs_notify_event().

Short of a satisfying proposition regarding the questions raised, I just
want to add two things that would be nice to solve in the future one way
or another and should perhaps be taken into consideration from the
beginning:

1. Disable polling completely when it isn't required: once the hd has
   spun down, there is no need to keep polling the sensors at all. Only
   when the first request requiring the hd to spin up arrives, the
   kernel needs to hold back for a short while to gather enough data
   from the sensors, so shock protection is up and running again.
2. Make shock protection interact nicely with suspend operations:
   currently, we are out of luck if anything should happen after
   processes have been frozen. This is particularly unfortunatel in the
   case of s2disk.

As far as 1. is concerned, I'm not quite sure yet how to determine when
the disk has spun down since this may have happened according to vendor
specific timing rules. Once we have established that the disk is
sleeping safe and sound, sensor drivers can be notified to stop polling
the hardware and possibly notify clients (like hdapsd) about that fact.

Something along the lines of the disk_shock module approach suggested by
Bart in [1] sounds promising, but it still needs some careful thinking
when designing the interfaces to all components involved. As for the
suspend problem, we could either put some simplified logic into
disk_shock so it can take over once hdapsd has been frozen, or we can
find a way to keep hdapsd unfrozen until the hd has been put to sleep.
Of course, we'd have to think about resume as well, although it'll be
impossible to protect right from the start in the s2disk case.

Regards,

Elias

[1] http://marc.info/?l=linux-kernel&m=122021163324843&w=2

Re: Laptop shock detection and harddisk protection

[Shem Multinymous]

Hi Tejun,

On Thu, Sep 11, 2008 at 12:34 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Shem Multinymous.
>> Using the input device interface for the accelerometer (as done by
>> tp_smapi's hdaps + latest hdapsd) greatly reduces the number of
>> accelerometer-related timer interrupts on tickless kernels, as
>> measured by powertop. With syscall polling you have the kernal polling
>> the hardware at ~50Hz and then the userspace hdapsd polling the kernel
>> at ~50Hz. When they're out of phase so you can get up to 100
>> interrupts/sec. With an input device you're always at 50Hz. The phase
>> difference also induces a small extra delay in the shock handling
>> response.
>
> That reduction comes because input device supports poll and
> sysfs_notify_event() does about the same thing.  The uesrland daemon
> can just poll on a node and read data nodes when poll event on the
> node triggeres.

Agreed.
There's another issue with the current sysfs interface, though: hdapsd
needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
and y in separate attributes which cannot be read atomically together.
We can add a sysfs file with "x y timestamp" readouts, though this is
unusual for sysfs (and certainly incompatible with hwmon).


> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
> nodes, so I don't think it's a good idea to push out actual unloading
> to another process especially as fork doesn't inherit mlockall.

I had in mind another daemon listening for "unload now" events, so no
forking needed.
This second daemon might make sense if we push the logic of deciding
*which* disks to unload into userspace, since this logic is the same
for the ThinkPad style and the HP style.


> On a related note, is there any plan to merge tp_smapi to mainline?
> It seems you put a lot of work into it and I don't really see why it
> should stay out of tree.

The only issue I'm aware of is finding a reasonably-named maintainer.
On the technical side, the reviews on my lkml submission of
thinkpad_ec+hdaps seemed good and all technical comments are since
addressed. The code has been stable, well-tested and packaged by major
distros for years.

  Shem

Re: Laptop shock detection and harddisk protection

[Tejun Heo]

Shem Multinymous wrote:
>> That reduction comes because input device supports poll and
>> sysfs_notify_event() does about the same thing.  The uesrland daemon
>> can just poll on a node and read data nodes when poll event on the
>> node triggeres.
> 
> Agreed.
> There's another issue with the current sysfs interface, though: hdapsd
> needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> and y in separate attributes which cannot be read atomically together.
> We can add a sysfs file with "x y timestamp" readouts, though this is
> unusual for sysfs (and certainly incompatible with hwmon).

Yes, right.  Forgot about the atomicity part altogether.  Thanks for
bringing it up.

>> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
>> nodes, so I don't think it's a good idea to push out actual unloading
>> to another process especially as fork doesn't inherit mlockall.
> 
> I had in mind another daemon listening for "unload now" events, so no
> forking needed.
> This second daemon might make sense if we push the logic of deciding
> *which* disks to unload into userspace, since this logic is the same
> for the ThinkPad style and the HP style.

Hmmm... I can't (yet) see the benefit of having two separate userland
daemons.

>> On a related note, is there any plan to merge tp_smapi to mainline?
>> It seems you put a lot of work into it and I don't really see why it
>> should stay out of tree.
> 
> The only issue I'm aware of is finding a reasonably-named maintainer.
> On the technical side, the reviews on my lkml submission of
> thinkpad_ec+hdaps seemed good and all technical comments are since
> addressed. The code has been stable, well-tested and packaged by major
> distros for years.

Cool, can you please post the patch to the lkml and cc Greg
Kroah-Hartman <gregkh@suse.de>, Andrew Morton
<akpm@linux-foundation.org> and me?

Thanks.

-- 
tejun

Re: Laptop shock detection and harddisk protection

[Henrique de Moraes Holschuh]

On Thu, 11 Sep 2008, Tejun Heo wrote:
> > Using the input device interface for the accelerometer (as done by
> > tp_smapi's hdaps + latest hdapsd) greatly reduces the number of
> > accelerometer-related timer interrupts on tickless kernels, as
> > measured by powertop. With syscall polling you have the kernal polling
> > the hardware at ~50Hz and then the userspace hdapsd polling the kernel
> > at ~50Hz. When they're out of phase so you can get up to 100
> > interrupts/sec. With an input device you're always at 50Hz. The phase
> > difference also induces a small extra delay in the shock handling
> > response.
> 
> That reduction comes because input device supports poll and
> sysfs_notify_event() does about the same thing.  The uesrland daemon
> can just poll on a node and read data nodes when poll event on the
> node triggeres.

If you guys are going to bother with the accelerometer interface (which is
a completely separate issue from the "queue-freeze-and-park-heads" APIs and
sysfs interface), you better be aware that there IS an accelerometer class
in the works, that cathers for directly-attached devices.

I'd look there first.  And a generic sysfs interface for accelerometers IS a
reasonably hard problem, so I would have it well separate from the disk-park
side of things and while it gets sorted out, I'd leave it for userspace to
deal with the issue (it is not like there is much userspace to worry about
right now, just hdapsd which is only interested on the hdaps accel
interface.  Everyone else can do it in-kernel, because their firmware
already does the imminent-fall detection).

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Laptop shock detection and harddisk protection

[Bodo Eggert]

Tejun Heo <tj@kernel.org> wrote:

> 1. How should the shock interface look like?  As we're gonna need
>    userland daemon one way or the other, we can use the userland
>    daemon to glue all the interfaces but it would be much better to
>    have a unified interface.  Although there seem to be several
>    different variants, they don't differ all that much and creating a
>    new interface every time is painful.  I think we can get by with a
>    sysfs interface with notification.

It should provide a connection to the corresponding device. Imagine an USB
case having a sensor, being connected to a desktop system. You would want
to exactly protect the enclosed USB device, and possibly non-protected USB
devices, too, but not halt the internal disks.

Re: Laptop shock detection and harddisk protection

[Greg KH]

On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> Shem Multinymous wrote:
> >> That reduction comes because input device supports poll and
> >> sysfs_notify_event() does about the same thing.  The uesrland daemon
> >> can just poll on a node and read data nodes when poll event on the
> >> node triggeres.
> > 
> > Agreed.
> > There's another issue with the current sysfs interface, though: hdapsd
> > needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> > and y in separate attributes which cannot be read atomically together.
> > We can add a sysfs file with "x y timestamp" readouts, though this is
> > unusual for sysfs (and certainly incompatible with hwmon).
> 
> Yes, right.  Forgot about the atomicity part altogether.  Thanks for
> bringing it up.
> 
> >> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
> >> nodes, so I don't think it's a good idea to push out actual unloading
> >> to another process especially as fork doesn't inherit mlockall.
> > 
> > I had in mind another daemon listening for "unload now" events, so no
> > forking needed.
> > This second daemon might make sense if we push the logic of deciding
> > *which* disks to unload into userspace, since this logic is the same
> > for the ThinkPad style and the HP style.
> 
> Hmmm... I can't (yet) see the benefit of having two separate userland
> daemons.
> 
> >> On a related note, is there any plan to merge tp_smapi to mainline?
> >> It seems you put a lot of work into it and I don't really see why it
> >> should stay out of tree.
> > 
> > The only issue I'm aware of is finding a reasonably-named maintainer.
> > On the technical side, the reviews on my lkml submission of
> > thinkpad_ec+hdaps seemed good and all technical comments are since
> > addressed. The code has been stable, well-tested and packaged by major
> > distros for years.
> 
> Cool, can you please post the patch to the lkml and cc Greg
> Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> <akpm@linux-foundation.org> and me?

Sorry, but no, I can't accept this code as it is coming from a "known
anonymous" person containing information that it is not known where it
came from.

We went over this before a number of years ago, that's why this code
isn't in mainline :(

In short, "Signed-off-by:" from people who are known to be anonymous is
not allowed.

thanks,

greg k-h

Re: Laptop shock detection and harddisk protection

[Jeremy Fitzhardinge]

Shem Multinymous wrote:
> Hi Tejun,
>
> On Thu, Sep 11, 2008 at 12:34 PM, Tejun Heo <tj@kernel.org> wrote:
>   
>> Hello, Shem Multinymous.
>>     
>>> Using the input device interface for the accelerometer (as done by
>>> tp_smapi's hdaps + latest hdapsd) greatly reduces the number of
>>> accelerometer-related timer interrupts on tickless kernels, as
>>> measured by powertop. With syscall polling you have the kernal polling
>>> the hardware at ~50Hz and then the userspace hdapsd polling the kernel
>>> at ~50Hz. When they're out of phase so you can get up to 100
>>> interrupts/sec. With an input device you're always at 50Hz. The phase
>>> difference also induces a small extra delay in the shock handling
>>> response.
>>>       
>> That reduction comes because input device supports poll and
>> sysfs_notify_event() does about the same thing.  The uesrland daemon
>> can just poll on a node and read data nodes when poll event on the
>> node triggeres.
>>     
>
> Agreed.
> There's another issue with the current sysfs interface, though: hdapsd
> needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> and y in separate attributes which cannot be read atomically together.
> We can add a sysfs file with "x y timestamp" readouts, though this is
> unusual for sysfs (and certainly incompatible with hwmon).
>   

Assuming timestamp is always updated when the x,y values change, you can do:

	do {
		ts = read_timestamp();
		x = read_x();
		y = read_y();
		ts2 = read_timestamp();
	} while(ts != ts2);






    J

Re: Laptop shock detection and harddisk protection

[Tejun Heo]

Greg KH wrote:
>>>> On a related note, is there any plan to merge tp_smapi to mainline?
>>>> It seems you put a lot of work into it and I don't really see why it
>>>> should stay out of tree.
>>> The only issue I'm aware of is finding a reasonably-named maintainer.
>>> On the technical side, the reviews on my lkml submission of
>>> thinkpad_ec+hdaps seemed good and all technical comments are since
>>> addressed. The code has been stable, well-tested and packaged by major
>>> distros for years.
>> Cool, can you please post the patch to the lkml and cc Greg
>> Kroah-Hartman <gregkh@suse.de>, Andrew Morton
>> <akpm@linux-foundation.org> and me?
> 
> Sorry, but no, I can't accept this code as it is coming from a "known
> anonymous" person containing information that it is not known where it
> came from.
> 
> We went over this before a number of years ago, that's why this code
> isn't in mainline :(

Aieee... didn't know that and was wondering why the hell this wasn't
in mainline.  :-(

> In short, "Signed-off-by:" from people who are known to be anonymous is
> not allowed.

Shem Multinymous, I suppose nothing really has changd since the last
time?

Thanks.

-- 
tejun

Re: Laptop shock detection and harddisk protection

[Shem Multinymous]

Hi,

On Mon, Sep 15, 2008 at 4:29 AM, Tejun Heo <tj@kernel.org> wrote:
>> Sorry, but no, I can't accept this code as it is coming from a "known
>> anonymous" person containing information that it is not known where it
>> came from.
>>
>> We went over this before a number of years ago, that's why this code
>> isn't in mainline :(
>
> Shem Multinymous, I suppose nothing really has changd since the last
> time?

The only pertinent change is that I've added even more documentation
to thinkpad_ec.c, explaining how we're just following the published
H8S specs (plus some workarounds for experimentally observed bugs).

As noted before, I'd be more than happy to assign away copyrights or
discuss things further in private if this would help.

  Shem

Re: Laptop shock detection and harddisk protection

[Tejun Heo]

Shem Multinymous wrote:
> On Mon, Sep 15, 2008 at 4:29 AM, Tejun Heo <tj@kernel.org> wrote:
>>> Sorry, but no, I can't accept this code as it is coming from a "known
>>> anonymous" person containing information that it is not known where it
>>> came from.
>>>
>>> We went over this before a number of years ago, that's why this code
>>> isn't in mainline :(
>> Shem Multinymous, I suppose nothing really has changd since the last
>> time?
> 
> The only pertinent change is that I've added even more documentation
> to thinkpad_ec.c, explaining how we're just following the published
> H8S specs (plus some workarounds for experimentally observed bugs).
> 
> As noted before, I'd be more than happy to assign away copyrights or
> discuss things further in private if this would help.

I bet you had enough discussion about this last time but any chance
you can break out of the anonymity?  :-)

Thanks.

-- 
tejun

Re: Laptop shock detection and harddisk protection

[Elias Oltmanns]

Pavel Machek <pavel@suse.cz> wrote:
> Hi!
>
>> Short of a satisfying proposition regarding the questions raised, I just
>> want to add two things that would be nice to solve in the future one way
>> or another and should perhaps be taken into consideration from the
>> beginning:
>> 
>> 1. Disable polling completely when it isn't required: once the hd has
>>    spun down, there is no need to keep polling the sensors at all. Only
>>    when the first request requiring the hd to spin up arrives, the
>>    kernel needs to hold back for a short while to gather enough data
>>    from the sensors, so shock protection is up and running again.
>
> hdparm can already tell if disk is spinning or not. As userland is
> polling, already, that may be enough?

Yes, I know it *is* possible, I just think it may not be completely
trivial to get it right. The only way I can think of, right now, is some
sort of polling in the hdparm way, i.e. issuing a ATA_CMD_CHK_POWER from
time to time. The question is how we are to decide when and how often we
should poll for the current power state of the HD.

>
>> 2. Make shock protection interact nicely with suspend operations:
>>    currently, we are out of luck if anything should happen after
>>    processes have been frozen. This is particularly unfortunatel in the
>>    case of s2disk.
>
> I'd say that s2disk is similar to early boot... no protection there. 

Well, this is true for resume. However, I can't help thinking that we
may do better than that until the disk spins down, particularly while
writing the image to disk.

Regards,

Elias

Re: Laptop shock detection and harddisk protection

[Greg KH]

On Sun, Aug 17, 2008 at 09:45:21PM +0200, Pavel Machek wrote:
> On Fri 2008-09-12 09:59:47, Greg KH wrote:
> > On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> > > Shem Multinymous wrote:
> > > >> That reduction comes because input device supports poll and
> > > >> sysfs_notify_event() does about the same thing.  The uesrland daemon
> > > >> can just poll on a node and read data nodes when poll event on the
> > > >> node triggeres.
> > > > 
> > > > Agreed.
> > > > There's another issue with the current sysfs interface, though: hdapsd
> > > > needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> > > > and y in separate attributes which cannot be read atomically together.
> > > > We can add a sysfs file with "x y timestamp" readouts, though this is
> > > > unusual for sysfs (and certainly incompatible with hwmon).
> > > 
> > > Yes, right.  Forgot about the atomicity part altogether.  Thanks for
> > > bringing it up.
> > > 
> > > >> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
> > > >> nodes, so I don't think it's a good idea to push out actual unloading
> > > >> to another process especially as fork doesn't inherit mlockall.
> > > > 
> > > > I had in mind another daemon listening for "unload now" events, so no
> > > > forking needed.
> > > > This second daemon might make sense if we push the logic of deciding
> > > > *which* disks to unload into userspace, since this logic is the same
> > > > for the ThinkPad style and the HP style.
> > > 
> > > Hmmm... I can't (yet) see the benefit of having two separate userland
> > > daemons.
> > > 
> > > >> On a related note, is there any plan to merge tp_smapi to mainline?
> > > >> It seems you put a lot of work into it and I don't really see why it
> > > >> should stay out of tree.
> > > > 
> > > > The only issue I'm aware of is finding a reasonably-named maintainer.
> > > > On the technical side, the reviews on my lkml submission of
> > > > thinkpad_ec+hdaps seemed good and all technical comments are since
> > > > addressed. The code has been stable, well-tested and packaged by major
> > > > distros for years.
> > > 
> > > Cool, can you please post the patch to the lkml and cc Greg
> > > Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> > > <akpm@linux-foundation.org> and me?
> > 
> > Sorry, but no, I can't accept this code as it is coming from a "known
> > anonymous" person containing information that it is not known where it
> > came from.
> 
> So... what are you worried about?

Code created by access to specs that were not allowed to be published in
GPL form by someone who wants to remain anonymous.

Come on, we went over this the last time this came up a few years ago.
It's just not going to happen due to the legal issues involved, sorry.

If people want to get this kind of code into the tree, they can write a
new driver from scratch, based on public information on these chips.
And you will have to defend exactly where this information was found as
you can not do it by information found in this "tainted" driver, sorry,
that's not a legally viable way forward.

thanks,

greg k-h

Re: Laptop shock detection and harddisk protection

[Shem Multinymous]

On Wed, Sep 17, 2008 at 11:21 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
>>> 2. Make shock protection interact nicely with suspend operations:
>>>    currently, we are out of luck if anything should happen after
>>>    processes have been frozen. This is particularly unfortunatel in the
>>>    case of s2disk.
>>
>> I'd say that s2disk is similar to early boot... no protection there.
>
> Well, this is true for resume. However, I can't help thinking that we
> may do better than that until the disk spins down, particularly while
> writing the image to disk.

Agreed.
Practically, disk protection during suspend-to-disk sounds very
useful, given the typical usage scenario: you press button to
hibernate and start shuffling around your chair/desk in preparation
for leaving the room. Dropping the laptop is more likely than ever at
this point. Resume-from-disk tends to be done after settling down into
your chair/desk, so the drop risk is lower.

  Shem

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

On Wed 2008-09-17 11:04:05, Greg KH wrote:
> On Sun, Aug 17, 2008 at 09:45:21PM +0200, Pavel Machek wrote:
> > On Fri 2008-09-12 09:59:47, Greg KH wrote:
> > > On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> > > > Shem Multinymous wrote:
> > > > >> That reduction comes because input device supports poll and
> > > > >> sysfs_notify_event() does about the same thing.  The uesrland daemon
> > > > >> can just poll on a node and read data nodes when poll event on the
> > > > >> node triggeres.
> > > > > 
> > > > > Agreed.
> > > > > There's another issue with the current sysfs interface, though: hdapsd
> > > > > needs to read (x,y,timestamp) tuples, whereas sysfs provides just x
> > > > > and y in separate attributes which cannot be read atomically together.
> > > > > We can add a sysfs file with "x y timestamp" readouts, though this is
> > > > > unusual for sysfs (and certainly incompatible with hwmon).
> > > > 
> > > > Yes, right.  Forgot about the atomicity part altogether.  Thanks for
> > > > bringing it up.
> > > > 
> > > > >> Unloading heads will be simple.  Just echoing timeout in ms to sysfs
> > > > >> nodes, so I don't think it's a good idea to push out actual unloading
> > > > >> to another process especially as fork doesn't inherit mlockall.
> > > > > 
> > > > > I had in mind another daemon listening for "unload now" events, so no
> > > > > forking needed.
> > > > > This second daemon might make sense if we push the logic of deciding
> > > > > *which* disks to unload into userspace, since this logic is the same
> > > > > for the ThinkPad style and the HP style.
> > > > 
> > > > Hmmm... I can't (yet) see the benefit of having two separate userland
> > > > daemons.
> > > > 
> > > > >> On a related note, is there any plan to merge tp_smapi to mainline?
> > > > >> It seems you put a lot of work into it and I don't really see why it
> > > > >> should stay out of tree.
> > > > > 
> > > > > The only issue I'm aware of is finding a reasonably-named maintainer.
> > > > > On the technical side, the reviews on my lkml submission of
> > > > > thinkpad_ec+hdaps seemed good and all technical comments are since
> > > > > addressed. The code has been stable, well-tested and packaged by major
> > > > > distros for years.
> > > > 
> > > > Cool, can you please post the patch to the lkml and cc Greg
> > > > Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> > > > <akpm@linux-foundation.org> and me?
> > > 
> > > Sorry, but no, I can't accept this code as it is coming from a "known
> > > anonymous" person containing information that it is not known where it
> > > came from.
> > 
> > So... what are you worried about?
> 
> Code created by access to specs that were not allowed to be published in
> GPL form by someone who wants to remain anonymous.

That anonymous person may have problems if they signed NDA.

I don't think they did, they even list the sources:

 *  The embedded controller on ThinkPad laptops has a non-standard interface,
 *  where LPC channel 3 of the H8S EC chip is hooked up to IO ports
 *  0x1600-0x161F and implements (a special case of) the H8S LPC protocol.
 *  The EC LPC interface provides various system management services (currently
 *  known: battery information and accelerometer readouts). This driver
 *  provides access and mutual exclusion for the EC interface.
*
 *  The LPC protocol and terminology is documented here:
 *  "H8S/2104B Group Hardware Manual",
 *  http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf

H8S chip seems to be documented.

...even if you are right, why is it problem for _us_. We are not
covered by NDAs we did not sign.

So can you list your concerns for _us_? Copyrights? Patents? Trade
secrets? Contracts?

> If people want to get this kind of code into the tree, they can write a
> new driver from scratch, based on public information on these chips.
> And you will have to defend exactly where this information was found
> as

These are rather bigger requirements than signed-off-by and then what
Novell legal people require before putting stuff in distribution. So
you should explain why this bigger requirements are warranted.

> you can not do it by information found in this "tainted" driver, sorry,
> that's not a legally viable way forward.

As far as I can see, using any information I find anywhere is
perfectly legal for writing a driver, as long as I don't sign &
violate a NDA. sourceforge.net is rather well-known, public source of
information. Why should it be treated specially?!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Thomas Renninger]

On Thursday 18 September 2008 13:18:45 Pavel Machek wrote:
> On Wed 2008-09-17 11:04:05, Greg KH wrote:
> > On Sun, Aug 17, 2008 at 09:45:21PM +0200, Pavel Machek wrote:
> > > On Fri 2008-09-12 09:59:47, Greg KH wrote:
> > > > On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> > > > > Shem Multinymous wrote:
> > > > > >> That reduction comes because input device supports poll and
> > > > > >> sysfs_notify_event() does about the same thing.  The uesrland
> > > > > >> daemon can just poll on a node and read data nodes when poll
> > > > > >> event on the node triggeres.
> > > > > >
> > > > > > Agreed.
> > > > > > There's another issue with the current sysfs interface, though:
> > > > > > hdapsd needs to read (x,y,timestamp) tuples, whereas sysfs
> > > > > > provides just x and y in separate attributes which cannot be read
> > > > > > atomically together. We can add a sysfs file with "x y timestamp"
> > > > > > readouts, though this is unusual for sysfs (and certainly
> > > > > > incompatible with hwmon).
> > > > >
> > > > > Yes, right.  Forgot about the atomicity part altogether.  Thanks
> > > > > for bringing it up.
> > > > >
> > > > > >> Unloading heads will be simple.  Just echoing timeout in ms to
> > > > > >> sysfs nodes, so I don't think it's a good idea to push out
> > > > > >> actual unloading to another process especially as fork doesn't
> > > > > >> inherit mlockall.
> > > > > >
> > > > > > I had in mind another daemon listening for "unload now" events,
> > > > > > so no forking needed.
> > > > > > This second daemon might make sense if we push the logic of
> > > > > > deciding *which* disks to unload into userspace, since this logic
> > > > > > is the same for the ThinkPad style and the HP style.
> > > > >
> > > > > Hmmm... I can't (yet) see the benefit of having two separate
> > > > > userland daemons.
> > > > >
> > > > > >> On a related note, is there any plan to merge tp_smapi to
> > > > > >> mainline? It seems you put a lot of work into it and I don't
> > > > > >> really see why it should stay out of tree.
> > > > > >
> > > > > > The only issue I'm aware of is finding a reasonably-named
> > > > > > maintainer. On the technical side, the reviews on my lkml
> > > > > > submission of thinkpad_ec+hdaps seemed good and all technical
> > > > > > comments are since addressed. The code has been stable,
> > > > > > well-tested and packaged by major distros for years.
> > > > >
> > > > > Cool, can you please post the patch to the lkml and cc Greg
> > > > > Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> > > > > <akpm@linux-foundation.org> and me?
> > > >
> > > > Sorry, but no, I can't accept this code as it is coming from a "known
> > > > anonymous" person containing information that it is not known where
> > > > it came from.
> > >
> > > So... what are you worried about?
> >
> > Code created by access to specs that were not allowed to be published in
> > GPL form by someone who wants to remain anonymous.
>
> That anonymous person may have problems if they signed NDA.
>
> I don't think they did, they even list the sources:
>
>  *  The embedded controller on ThinkPad laptops has a non-standard
> interface, *  where LPC channel 3 of the H8S EC chip is hooked up to IO
> ports *  0x1600-0x161F and implements (a special case of) the H8S LPC
> protocol. *  The EC LPC interface provides various system management
> services (currently *  known: battery information and accelerometer
> readouts). This driver *  provides access and mutual exclusion for the EC
> interface.
> *
>  *  The LPC protocol and terminology is documented here:
>  *  "H8S/2104B Group Hardware Manual",
>  * 
> http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf
>
> H8S chip seems to be documented.
Hmm, the EC is not directly used, but ACPI functions of the HP device are 
used.
For the HP ACPI device: the ACPI functions can *very easily* be re-engineered
(which is common for all laptop_acpi.ko drivers):
ALRD -> is used by the driver to read out registers of the accelerometer
ALWR -> is used by the driver to write a registers of the accelerometer
BTW: HP likes to have support for their device.

The acceleromter chip itself is docuemented in detail here:
http://www.st.com/stonline/products/literature/ds/12094/lis3lv02dl.pdf

I also do not see any concerns.
Greg: Can you please add this one or explain in more detail what else you like 
to see to get this integerated.

Thanks,

      Thomas

Re: Laptop shock detection and harddisk protection

[Greg KH]

On Fri, Sep 19, 2008 at 11:03:58AM +0200, Thomas Renninger wrote:
> On Thursday 18 September 2008 13:18:45 Pavel Machek wrote:
> > On Wed 2008-09-17 11:04:05, Greg KH wrote:
> > > On Sun, Aug 17, 2008 at 09:45:21PM +0200, Pavel Machek wrote:
> > > > On Fri 2008-09-12 09:59:47, Greg KH wrote:
> > > > > On Fri, Sep 12, 2008 at 01:35:54AM +0200, Tejun Heo wrote:
> > > > > > Shem Multinymous wrote:
> > > > > > >> That reduction comes because input device supports poll and
> > > > > > >> sysfs_notify_event() does about the same thing.  The uesrland
> > > > > > >> daemon can just poll on a node and read data nodes when poll
> > > > > > >> event on the node triggeres.
> > > > > > >
> > > > > > > Agreed.
> > > > > > > There's another issue with the current sysfs interface, though:
> > > > > > > hdapsd needs to read (x,y,timestamp) tuples, whereas sysfs
> > > > > > > provides just x and y in separate attributes which cannot be read
> > > > > > > atomically together. We can add a sysfs file with "x y timestamp"
> > > > > > > readouts, though this is unusual for sysfs (and certainly
> > > > > > > incompatible with hwmon).
> > > > > >
> > > > > > Yes, right.  Forgot about the atomicity part altogether.  Thanks
> > > > > > for bringing it up.
> > > > > >
> > > > > > >> Unloading heads will be simple.  Just echoing timeout in ms to
> > > > > > >> sysfs nodes, so I don't think it's a good idea to push out
> > > > > > >> actual unloading to another process especially as fork doesn't
> > > > > > >> inherit mlockall.
> > > > > > >
> > > > > > > I had in mind another daemon listening for "unload now" events,
> > > > > > > so no forking needed.
> > > > > > > This second daemon might make sense if we push the logic of
> > > > > > > deciding *which* disks to unload into userspace, since this logic
> > > > > > > is the same for the ThinkPad style and the HP style.
> > > > > >
> > > > > > Hmmm... I can't (yet) see the benefit of having two separate
> > > > > > userland daemons.
> > > > > >
> > > > > > >> On a related note, is there any plan to merge tp_smapi to
> > > > > > >> mainline? It seems you put a lot of work into it and I don't
> > > > > > >> really see why it should stay out of tree.
> > > > > > >
> > > > > > > The only issue I'm aware of is finding a reasonably-named
> > > > > > > maintainer. On the technical side, the reviews on my lkml
> > > > > > > submission of thinkpad_ec+hdaps seemed good and all technical
> > > > > > > comments are since addressed. The code has been stable,
> > > > > > > well-tested and packaged by major distros for years.
> > > > > >
> > > > > > Cool, can you please post the patch to the lkml and cc Greg
> > > > > > Kroah-Hartman <gregkh@suse.de>, Andrew Morton
> > > > > > <akpm@linux-foundation.org> and me?
> > > > >
> > > > > Sorry, but no, I can't accept this code as it is coming from a "known
> > > > > anonymous" person containing information that it is not known where
> > > > > it came from.
> > > >
> > > > So... what are you worried about?
> > >
> > > Code created by access to specs that were not allowed to be published in
> > > GPL form by someone who wants to remain anonymous.
> >
> > That anonymous person may have problems if they signed NDA.
> >
> > I don't think they did, they even list the sources:
> >
> >  *  The embedded controller on ThinkPad laptops has a non-standard
> > interface, *  where LPC channel 3 of the H8S EC chip is hooked up to IO
> > ports *  0x1600-0x161F and implements (a special case of) the H8S LPC
> > protocol. *  The EC LPC interface provides various system management
> > services (currently *  known: battery information and accelerometer
> > readouts). This driver *  provides access and mutual exclusion for the EC
> > interface.
> > *
> >  *  The LPC protocol and terminology is documented here:
> >  *  "H8S/2104B Group Hardware Manual",
> >  * 
> > http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf
> >
> > H8S chip seems to be documented.
> Hmm, the EC is not directly used, but ACPI functions of the HP device are 
> used.
> For the HP ACPI device: the ACPI functions can *very easily* be re-engineered
> (which is common for all laptop_acpi.ko drivers):
> ALRD -> is used by the driver to read out registers of the accelerometer
> ALWR -> is used by the driver to write a registers of the accelerometer
> BTW: HP likes to have support for their device.
> 
> The acceleromter chip itself is docuemented in detail here:
> http://www.st.com/stonline/products/literature/ds/12094/lis3lv02dl.pdf
> 
> I also do not see any concerns.
> Greg: Can you please add this one or explain in more detail what else you like 
> to see to get this integerated.

If you, or anyone else, writes a new driver from the published
documents, that driver can be accepted.  It can not be based on the
existing code written by Shem in any form.

We can not accept this driver for reasons I've detailed numerous times
in the past.  That fact isn't going to change, sorry.

thanks,

greg k-h

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

> > > > Code created by access to specs that were not allowed to be published in
> > > > GPL form by someone who wants to remain anonymous.
> > >
> > > That anonymous person may have problems if they signed NDA.
> > >
> > > I don't think they did, they even list the sources:
> > >
> > >  *  The embedded controller on ThinkPad laptops has a non-standard
> > > interface, *  where LPC channel 3 of the H8S EC chip is hooked up to IO
> > > ports *  0x1600-0x161F and implements (a special case of) the H8S LPC
> > > protocol. *  The EC LPC interface provides various system management
> > > services (currently *  known: battery information and accelerometer
> > > readouts). This driver *  provides access and mutual exclusion for the EC
> > > interface.
> > > *
> > >  *  The LPC protocol and terminology is documented here:
> > >  *  "H8S/2104B Group Hardware Manual",
> > >  * 
> > > http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf
> > >
> > > H8S chip seems to be documented.
> > Hmm, the EC is not directly used, but ACPI functions of the HP device are 
> > used.
> > For the HP ACPI device: the ACPI functions can *very easily* be re-engineered
> > (which is common for all laptop_acpi.ko drivers):
> > ALRD -> is used by the driver to read out registers of the accelerometer
> > ALWR -> is used by the driver to write a registers of the accelerometer
> > BTW: HP likes to have support for their device.
> > 
> > The acceleromter chip itself is docuemented in detail here:
> > http://www.st.com/stonline/products/literature/ds/12094/lis3lv02dl.pdf
> > 
> > I also do not see any concerns.
> > Greg: Can you please add this one or explain in more detail what else you like 
> > to see to get this integerated.
> 
> If you, or anyone else, writes a new driver from the published
> documents, that driver can be accepted.  It can not be based on the
> existing code written by Shem in any form.

Can you detail what "published" means?

Either I can take his sources on sourceforge.net (quite well known
place, right) as published information, or I could not use other well
known sources such as wikipedia.

Sources on sourceforge.net seem published-enough to me, and if you
insist they can't be used, you should provide some reasons...

[And no, just calling it "tainted" is not enough.]
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Greg KH]

On Tue, Oct 07, 2008 at 10:40:11PM +0200, Pavel Machek wrote:
> 
> > > > > Code created by access to specs that were not allowed to be published in
> > > > > GPL form by someone who wants to remain anonymous.
> > > >
> > > > That anonymous person may have problems if they signed NDA.
> > > >
> > > > I don't think they did, they even list the sources:
> > > >
> > > >  *  The embedded controller on ThinkPad laptops has a non-standard
> > > > interface, *  where LPC channel 3 of the H8S EC chip is hooked up to IO
> > > > ports *  0x1600-0x161F and implements (a special case of) the H8S LPC
> > > > protocol. *  The EC LPC interface provides various system management
> > > > services (currently *  known: battery information and accelerometer
> > > > readouts). This driver *  provides access and mutual exclusion for the EC
> > > > interface.
> > > > *
> > > >  *  The LPC protocol and terminology is documented here:
> > > >  *  "H8S/2104B Group Hardware Manual",
> > > >  * 
> > > > http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf
> > > >
> > > > H8S chip seems to be documented.
> > > Hmm, the EC is not directly used, but ACPI functions of the HP device are 
> > > used.
> > > For the HP ACPI device: the ACPI functions can *very easily* be re-engineered
> > > (which is common for all laptop_acpi.ko drivers):
> > > ALRD -> is used by the driver to read out registers of the accelerometer
> > > ALWR -> is used by the driver to write a registers of the accelerometer
> > > BTW: HP likes to have support for their device.
> > > 
> > > The acceleromter chip itself is docuemented in detail here:
> > > http://www.st.com/stonline/products/literature/ds/12094/lis3lv02dl.pdf
> > > 
> > > I also do not see any concerns.
> > > Greg: Can you please add this one or explain in more detail what else you like 
> > > to see to get this integerated.
> > 
> > If you, or anyone else, writes a new driver from the published
> > documents, that driver can be accepted.  It can not be based on the
> > existing code written by Shem in any form.
> 
> Can you detail what "published" means?

Published in a way that has NOTHING to do with these source files.

> Either I can take his sources on sourceforge.net (quite well known
> place, right) as published information, or I could not use other well
> known sources such as wikipedia.

If the wikipedia information was written based on these source files,
no, we can't use that, sorry.

> Sources on sourceforge.net seem published-enough to me, and if you
> insist they can't be used, you should provide some reasons...
> 
> [And no, just calling it "tainted" is not enough.]

{sigh}

Again, for the last time:
  - this code was written by an anonymous person, using documents or
    information that was obtained and used in a manner that was not
    legal according to their employment agreement.
  - because of this, we can not use this code, because we KNOW the
    information was obtained in a improper manner.
  - so, to get something like this into the kernel, we need to rewrite
    the code, using information obtained LEGALLY from either the
    manufacturer of the chips or computers, or from another TOTALLY
    SEPARATE location.

Does that help explain this?

thanks,

greg k-h

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

> > > If you, or anyone else, writes a new driver from the published
> > > documents, that driver can be accepted.  It can not be based on the
> > > existing code written by Shem in any form.
> > 
> > Can you detail what "published" means?
> 
> Published in a way that has NOTHING to do with these source files.
> 
> > Either I can take his sources on sourceforge.net (quite well known
> > place, right) as published information, or I could not use other well
> > known sources such as wikipedia.
> 
> If the wikipedia information was written based on these source files,
> no, we can't use that, sorry.

How do I know?

> > Sources on sourceforge.net seem published-enough to me, and if you
> > insist they can't be used, you should provide some reasons...
> > 
> > [And no, just calling it "tainted" is not enough.]
> 
> {sigh}
> 
> Again, for the last time:
>   - this code was written by an anonymous person, using documents or
>     information that was obtained and used in a manner that was not
>     legal according to their employment agreement.

This code is written by anonymous person. He may have used documents
improperly, but I see no signs of that, and don't see why I should
believe you saying so.

If have proof of that, you should talk to sourceforge to take that
code down... or probably their employer should ask sourceforge to do
that.

The documents are on the web from more than year now, on
well-known. That seems to indicate that your theory is not true.

>   - because of this, we can not use this code, because we KNOW the
>     information was obtained in a improper manner.

Whole wikipedia is written by "anonymous" people. Does that mean that
all information in wikipedia was obtained in a improper manner?

>   - so, to get something like this into the kernel, we need to rewrite
>     the code, using information obtained LEGALLY from either the
>     manufacturer of the chips or computers, or from another TOTALLY
>     SEPARATE location.

I'm LEGALLY obtaining the information from sourceforge.net. That is
rather well-known, and non-anonymous source. They continue to publish
this information.

> Does that help explain this?

Unfortunately, no :-(.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Laptop shock detection and harddisk protection

[Greg KH]

On Tue, Oct 07, 2008 at 11:40:06PM +0200, Pavel Machek wrote:
> 
> > > > If you, or anyone else, writes a new driver from the published
> > > > documents, that driver can be accepted.  It can not be based on the
> > > > existing code written by Shem in any form.
> > > 
> > > Can you detail what "published" means?
> > 
> > Published in a way that has NOTHING to do with these source files.
> > 
> > > Either I can take his sources on sourceforge.net (quite well known
> > > place, right) as published information, or I could not use other well
> > > known sources such as wikipedia.
> > 
> > If the wikipedia information was written based on these source files,
> > no, we can't use that, sorry.
> 
> How do I know?

Unfortunatly, you can't, so don't use it.

> > > Sources on sourceforge.net seem published-enough to me, and if you
> > > insist they can't be used, you should provide some reasons...
> > > 
> > > [And no, just calling it "tainted" is not enough.]
> > 
> > {sigh}
> > 
> > Again, for the last time:
> >   - this code was written by an anonymous person, using documents or
> >     information that was obtained and used in a manner that was not
> >     legal according to their employment agreement.
> 
> This code is written by anonymous person. He may have used documents
> improperly, but I see no signs of that, and don't see why I should
> believe you saying so.
> 
> If have proof of that, you should talk to sourceforge to take that
> code down... or probably their employer should ask sourceforge to do
> that.
> 
> The documents are on the web from more than year now, on
> well-known. That seems to indicate that your theory is not true.

Yes, I know this is true, for a variety of reasons of what people
(including the individual in question) told me in private.

And I'm not going to go around asking for code to be taken down, as I'm
not the one whose contract was invalidated.

All I can say is that I can not accept such code into the Linux kernel
as it is known to be created in an illegal manner.

This has been discussed with both the Linux Foundations lawyers
(actually it was OSDL at the time), and with Novell's lawyers.

And as a Novell employee, you aren't allowed to put this code into the
kernel either, sorry, that's the rule we were told.

> >   - so, to get something like this into the kernel, we need to rewrite
> >     the code, using information obtained LEGALLY from either the
> >     manufacturer of the chips or computers, or from another TOTALLY
> >     SEPARATE location.
> 
> I'm LEGALLY obtaining the information from sourceforge.net. That is
> rather well-known, and non-anonymous source. They continue to publish
> this information.

If the information there is known to be posted in a manner that was
obtained illegally, it does not make the fact that you take it any more
"legal".

Ok, once again:
  - we know this code was created illegally.
  - that is why we (Linux Foundation / Novell) can not accept such code.

If some other company wants to step up, and put their "Signed-off-by:"
on it, I will be glad to have the Linux Foundation lawyers contact them
to find out if this is now acceptable, and from what background they
have gotten the information.

So can we please just drop this?  Or do you want me to point you at the
lawyers?

thanks,

greg k-h

Re: Laptop shock detection and harddisk protection

[Shem Multinymous]

I've been staying off the public discussion to avoid fuelling the flame;
my position has been stated earlier and in private communication.
But at this point I must go on record:

On Tue, Oct 7, 2008 at 5:19 PM, Greg KH <greg@kroah.com> wrote:
>  - this code was written by an anonymous person, using documents or
>    information that was obtained and used in a manner that was not
>    legal according to their employment agreement.

The above statement is false. I have never had access to non-public ThinkPad
specifications as part of my employment, nor have I been employed
by anyone who (to my knowledge) even possesses non-public ThinkPad
specifications.

I have contacted Greg KH privately in an effort to clarify the source
of his false impression.

  Shem

Re: Laptop shock detection and harddisk protection

[Pavel Machek]

Hi!

> > > > Either I can take his sources on sourceforge.net (quite well known
> > > > place, right) as published information, or I could not use other well
> > > > known sources such as wikipedia.
> > > 
> > > If the wikipedia information was written based on these source files,
> > > no, we can't use that, sorry.
> > 
> > How do I know?
> 
> Unfortunatly, you can't, so don't use it.

Not being able to use wikipedia is little strong, don't you think so?
What's next?

> > This code is written by anonymous person. He may have used documents
> > improperly, but I see no signs of that, and don't see why I should
> > believe you saying so.
> > 
> > If have proof of that, you should talk to sourceforge to take that
> > code down... or probably their employer should ask sourceforge to do
> > that.
> > 
> > The documents are on the web from more than year now, on
> > well-known. That seems to indicate that your theory is not true.
> 
> Yes, I know this is true, for a variety of reasons of what people
> (including the individual in question) told me in private.

Are you saying Shem told you he violated some contracts?

Ok, so can you tell us the details, so that we can evaluate this
ourselves, and (more importantly) so that he can defend himself? He
seems willing to point where specific pieces of code come from.

> And I'm not going to go around asking for code to be taken down, as I'm
> not the one whose contract was invalidated.

Right. So why do you care if this code goes into kernel? Your contract
is not violated, and neither me nor you signed any contract preventing
us from putting that code into kernel.

If you know the details, can you tell us which/whose contract is being
violated here? Maybe we can just get permission of those parties. Or
maybe your information is wrong...

> Ok, once again:
>   - we know this code was created illegally.

If you know this, please share details.

> So can we please just drop this?  Or do you want me to point you at the
> lawyers?

I'll talk to lawyers if I have to. I'd like to understand this.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html