[PATCH 0/1] UBUNTU: Add hdaps_ec driver to support newer ThinkPads (Hard Drive Active Protection System)

[Amit Kucheria]

On Mon, Apr 27, 2009 at 09:43:34AM +0200, Stefan Bader wrote:
> Tim Gardner wrote:
> > Brad Figg wrote:
> >> Please pull from :
> >>     git://kernel.ubuntu.com/bradf/ubuntu-karmic master
> >>
> >> Bug: #297213
> >>
> >> Hardy had two drivers which handled the HardDisk Active
> >> Protection System (hdaps and hdaps_ec) which is present in IBM
> >> ThinkPads.  One of the two drivers was dropped when Intrepid
> >> development began and due to that there are a number of ThinkPads
> >> (newer models) for which this is not supported.
> >>
> >> This patch brings back in the missing driver.
> >>
> >> The code for this patch was taken from http://tpctl.sourceforge.net
> >> and is the 0.40 release of December 16, 2008.
> >>
> >> Brad Figg (1):
> >>   UBUNTU: Add hdaps_ec driver to support newer ThinkPads (Hard Drive
> >>     Active Protection System)
> >>
> >>  ubuntu/misc/Kconfig    |   13 +
> >>  ubuntu/misc/Makefile   |    2 +-
> >>  ubuntu/misc/hdaps_ec.c |  880 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 894 insertions(+), 1 deletions(-)
> >>  create mode 100644 ubuntu/misc/hdaps_ec.c
> >>
> >>
> > 
> > How is this one better then drivers/hwmon/hdaps.c ? The in-kernel driver 
> > appears to contain a superset of DMI table information, i.e., supported 
> > platforms. Furthermore, it corrects some problems that exist in 
> > ubuntu/misc/hdaps_ec.c, such as I/O space registration, only loads if at 
> > least one DMI match is found, and uses in-kernel functionality for 
> > polled input devices.
> > 
> > So, NAK from me unless you can convince me otherwise.
> > 
> > rtg
> 
> This might not be completely true anymore but at the time I did that driver for 
> Hardy LUM the following was:
> 
> - The in kernel driver could not do independent axis inversion which was
>    required for some models
> - The driver from tpctl would use thinkpad_ec to channel acces to the embedded
>    controller, which allowed the other tool from that package concurrent access.
>    (arguable benefit)
> 
> Upstreaming sounded always like "soonish" without anything happening. The 
> problem is/was the questionable origins (unclear). As we not necessarily need 
> the other part of tpctl, on option would be to update the in kernel driver to 
> support more models (do the additional inversion logic and merge the DMi 
> information). An interesting question there would be whether access to the ec 
> needs syncing with other stuff (like thinpad-acpi)

NACK adding this alternative driver to our tree. This driver's upstream had made no known effort to get the driver upstream. We should ask them to submit it for inclusion into staging atleast. We had a discussion about this with a user (Whoopie) on irc today.

Now if we really want to support this feature, someone should find the changes compared to the in-kernel one and try to get that pushed upstream.

Regards,
Amit

-- 
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || amit at canonical.com
----------------------------------------------------------------------