Re: [PATCH] HPET driver

[Andrew Morton <akpm@osdl.org>]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> > 	[PATCH] HPET driver
> > 	
>
> Was this posted on lkml, or simply snuck in?

Was posted on lkml, was fairly widely reviewed, had comments from hch and
others, had several fixes from myself and from Robert and a long discussion
wrt the readq() implementation.

I'm not very happy with the driver, if only because its size and its prior
defect rate indicates that it probably has more problems.  But it provides
support for new hardware which is being shipped in real products and we
need it.  So ultimately one has to jam it into the tree in the hope that
doing so will get it a bit more attention.

> This should NOT have been merged without changes.  Please fix ASAP, or 
> revert and keep in -mm for a while.

Translation: Andrew has to personally understand and review every line
which goes into the kernel, unaided.  Sorry, that doesn't work.

The patch was reviewed on lkml and has been in -mm for test and review for
five weeks.  The initial reviews were not complete and sufficient attention
was not paid to it while it was in -mm.

To improve this process I need to find a way of provoking more attention
toward patches which I am not particularly confident about (and this one
certainly fell in that category).  Thus far I've done that by merging them
into the main tree, which does work quite nicely.  Perhaps I should send
these patches to lkml beforehand with big warning labels on them.

wrt the readq() implementation: I reverted the generic implementation based
on concerns raised on lkml by Eric Biederman.  As a generic readq/writeq
implementation seems to be a new R&D project I decided to leave the
implementation private to the HPET driver until someone takes all of this
on.

wrt the hpets list locking: yeah, I noticed that, mentioned it to Robert
then forgot all about it.  Mea Culpa.

wrt the request_irq() bug: yipes.  Robert, please fix.

wrt the new miscdev minor: yes, devices.txt should be updated.  When the
patch was first posted it was using a new major, but Robert changed that
based on review comments.