linux-kernel - Re: [PATCH] OMAP: Keypad driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20060722101340.79494.qmail@web37908.mail.mud.yahoo.com>
Date:	Sat, 22 Jul 2006 03:13:40 -0700 (PDT)
From:	Komal Shah <komal_shah802003@...oo.com>
To:	Andrew Morton <akpm@...l.org>
Cc:	linux-input@...ey.karlin.mff.cuni.cz, dtor_core@...ritech.net,
	tony@...mide.com, ext-timo.teras@...ia.com,
	juha.yrjola@...idboot.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] OMAP: Keypad driver

--- Andrew Morton <akpm@...l.org> wrote:

> 
> The code looks clean.
> 
> - Could use setup_timer().
> 
> - Check the return value from device_create_file() (and any and all
> such
>   similar functions), handle failure appropriately.
> 
> - I don't think the tasklet stopping code is correct. 
> tasklet_disable()
>   will prevent the callback from being called.  The del_timer_sync()
> will
>   kill off the timer.  But the just-killed timer handler might have
> left
>   the tasklet scheduled, and although it will not call the handler,
> the
>   high-level tasklet code can still execute, and will then start
> playing
>   with now-freed data.  A final tasklet_kill() should fix that up.
> 
> - Perhaps the probe function is requesting the IRQ too early?  The
> IRQ
>   handler can perhaps be called while the hardware is still being set
> up. 

Ok. I will move it before 

  omap_writew(0, OMAP_MPUIO_BASE + OMAP_MPUIO_KBD_MASKIT);

> 
> - Use INTF_TRIGGER_FALLING (etc), not the now-deprecated
> SA_TRIGGER_FALLING.

Thanx for the review. I will update the driver and submit the patch.

> 
> - The changelog needs work.  What's an OMAP? ;)

Ok. May changelog needs URL entry like http://linux.omap.com _or_
http://www.ti.com/omap :)

> 
> - Finally, by what authority does random_person@...oo.com submit
> Nokia
>   and TI's code??  Please review Section 11 of
>   Documentation/SubmittingPatches, seek and obtain signoffs from the
> other
>   authors and then add your own, thanks.

I have CCed all the authors mentioned in the top of the driver file,
and I thought they will just reply with Signed-off-by: line. I never
added my name in Copyright, even when I did some re-organization of
omap keypad driver, which I thought was not major change :)

---Komal Shah
http://komalshah.blogspot.com/

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/