linux-kernel - Re: [PATCH] OMAP: I2C driver for TI OMAP boards #2

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 31 Jul 2006 16:53:47 -0700
From:	David Brownell <david-b@...bell.net>
To:	Jean Delvare <khali@...ux-fr.org>
Cc:	Komal Shah <komal_shah802003@...oo.com>, akpm@...l.org,
	gregkh@...e.de, i2c@...sensors.org, imre.deak@...ia.com,
	juha.yrjola@...idboot.com, linux-kernel@...r.kernel.org,
	r-woodruff2@...com, tony@...mide.com
Subject: Re: [PATCH] OMAP: I2C driver for TI OMAP boards #2

On Monday 31 July 2006 9:13 am, Jean Delvare wrote:
>	 If you want things to improve, please help by
> reviewing Komal's driver. I think I understand you already commented on
> it, but I'd like you to really review it, and add a formal approval to
> it (e.g. Signed-off-by or Acked-by). Then I'll review it for merge.

The issues noted in the code are still almost all low priority
(non-blocking).

 - The FIXME about choosing the address is very low priority,
   and would affect only multi-master systems.  The fix would
   involve defining a new i2c-specific struct for platform_data,
   updating various boards to use it (e.g. OSK can use 400 MHz),
   and wouldn't change behavior for any board I've ever seen.

 - Likewise with the REVISIT for the bus speed to use.  They'd
   be fixed with the same patch.

 - The REVISIT about maybe a better way to probe is also low
   priority; someone with a board that needs better probing
   could address it at that time.  (Then restest any changes
   on multiple generations of silicion ... which IMO is the
   role the linux-omap tree should play.)

 - The revisit about adap->retries is still up in the air,
   and was a question in my submission from last year.
   How exactly is that supposed to be used?  Right now
   it's neither initialized (except to zero) nor tested.

Re coding style issues, I didn't give it a detailed nitpick
but I did easily notice two things worth fixing:

 - Some lines are more than 80 characters, so they'll wrap
   on standard editor windows.

 - There are a couple instances of hidden whitespace to
   remove:  at end of line, or space-before-tab.

This doesn't include the drivers/Makefile change to push i2c
linkage up near the beginning with other "system" busses,
but that can be a separate patch in any case (assuming that
it's still needed).

Assuming those two coding style things get resolved first,

Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>

- Dave
-
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/