lkml.org 
[lkml]   [2005]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH][I2C] Marvell mv64xxx i2c driver
Hi Mark,

> Marvell makes a line of host bridge for PPC and MIPS systems. On
> those bridges is an i2c controller. This patch adds the driver for
> that i2c controller.
>
> Please let me know if you see any problems with this patch.

I do not feel qualified for a full review of this code. However, I
noticed the following minor issues:

> +config I2C_MV64XXX
> + tristate "Marvell mv64xxx I2C Controller"
> + depends on I2C && MV64X60

&& EXPERIMENTAL?

> diff -Nru a/include/linux/i2c-id.h b/include/linux/i2c-id.h
> --- a/include/linux/i2c-id.h 2005-01-25 18:15:24 -07:00
> +++ b/include/linux/i2c-id.h 2005-01-25 18:15:24 -07:00
> @@ -200,6 +200,7 @@
>
> #define I2C_ALGO_SIBYTE 0x150000 /* Broadcom SiByte SOCs */
> #define I2C_ALGO_SGI 0x160000 /* SGI algorithm */
> +#define I2C_ALGO_MV64XXX 0x170000 /* Marvell mv64xxx i2c ctlr */

0x170000 is reserved within the legacy i2c project for an USB algorithm,
and 0x180000 for virtual busses. Could you please use 0x190000 instead,
so as to avoid future collisions?

> -#define MV64340_I2C_SLAVE_ADDR 0xc000
> -#define MV64340_I2C_EXTENDED_SLAVE_ADDR 0xc010
> -#define MV64340_I2C_DATA 0xc004
> -#define MV64340_I2C_CONTROL 0xc008
> -#define MV64340_I2C_STATUS_BAUDE_RATE 0xc00C
> -#define MV64340_I2C_SOFT_RESET 0xc01c
> +#define MV64XXX_I2C_CTLR_NAME "mv64xxx i2c"
> +#define MV64XXX_I2C_OFFSET 0xc000
> +#define MV64XXX_I2C_REG_BLOCK_SIZE 0x0020

You have a tab instead of space before MV64XXX_I2C_CTLR_NAME, it seems.
Also, you want to align the numerical values using only tabs, no space.

Thanks,
--
Jean Delvare
http://khali.linux-fr.org/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.357 / U:2.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site