On Wed, Feb 09, 2005 at 02:33:59PM -0700, Mark A. Greer wrote:
>
> I can't find any definitive policy on this. I kind of like the explicit
> return, I don't know why. I've had others make the same comment,
> though, so I'll remove them since it obviously bothers people.
>
> Attached is a repl
Bartlomiej Zolnierkiewicz wrote:
Thanks, I see that you did global replacement of __devinit
by __init and __devexit by __exit - it seems correct *only* if:
- there can be only one i2c controller in the system
- there can be only one host bridge in the system
- i2c core calls ->probe only once durin
On Tue, 08 Feb 2005 17:32:11 -0700, Mark A. Greer <[EMAIL PROTECTED]> wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
> >Hi,
> >
> >just a minor thing
> >
> >
> >
> >>+static int __devinit
> >>+mv64xxx_i2c_init(void)
> >>+{
> >>+ return driver_register(&mv64xxx_i2c_driver);
> >>+}
> >>
> >>
> >
Bartlomiej Zolnierkiewicz wrote:
Hi,
just a minor thing
+static int __devinit
+mv64xxx_i2c_init(void)
+{
+ return driver_register(&mv64xxx_i2c_driver);
+}
__init
+static void __devexit
+mv64xxx_i2c_exit(void)
+{
+ driver_unregister(&mv64xxx_i2c_driver);
+ return;
+}
Hi,
just a minor thing
> +static int __devinit
> +mv64xxx_i2c_init(void)
> +{
> + return driver_register(&mv64xxx_i2c_driver);
> +}
__init
> +static void __devexit
> +mv64xxx_i2c_exit(void)
> +{
> + driver_unregister(&mv64xxx_i2c_driver);
> + return;
> +}
__exit
these functi
Hi all,
Quoting myself:
> I am as surprised as you are to see this here. I2C_ALGO_AU1550 should
> really be made a different value. There is also a problem with
> I2C_ALGO_PCA and I2C_ALGO_SIBYTE having the same value, which was
> already reported to Greg some days ago if memory serves. I think I
Hi Mark, Alexey,
> > >+ /* 0x17 - USB */
> > >+ /* 0x18 - Virtual buses */
> > >+#define I2C_ALGO_MV64XXX 0x19 /* Marvell mv64xxx i2c ctlr
> > >*/
> >
> > While I searched for typos and y
Alexey Dobriyan wrote:
On Thursday 03 February 2005 21:12, Mark A. Greer wrote:
+ mv64xxx_i2c_fsm(drv_data, status);
It can set drv_data->rc to -ENODEV or -EIO. In both cases ->action goes to
MV64XXX_I2C_ACTION_SEND_STOP and mv64xxx_i2c_do_action() will writel()
something. Is it correc
On Thursday 03 February 2005 21:12, Mark A. Greer wrote:
> > >+ mv64xxx_i2c_fsm(drv_data, status);
> >
> >It can set drv_data->rc to -ENODEV or -EIO. In both cases ->action goes to
> >MV64XXX_I2C_ACTION_SEND_STOP and mv64xxx_i2c_do_action() will writel()
> >something. Is it correct to _no
Hi, Alexey.
--
Alexey Dobriyan wrote:
On Wednesday 02 February 2005 19:26, Mark A. Greer wrote:
How's this (a complete replacement for previous patch)?
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
+ If you say yes to this option, support will be included fo
Alexey Dobriyan wrote:
On Tue, 01 Feb 2005 10:54:32 -0700, Mark A. Greer wrote:
+struct mv64xxx_i2c_data {
+ void *reg_base;
ioremap() returns "void __iomem *".
Okay.
+static void __devexit
+mv64xxx_i2c_unmap_regs(struct mv64xxx_i2c_data *drv_data)
+{
+ if (!drv_data->reg_base) {
+ io
On Wed, Feb 02, 2005 at 03:15:14AM +0200, Alexey Dobriyan wrote:
> P. S.: struct mv64xxx_i2c_data revisited...
>
> > + uintstate;
> > + ulong reg_base_p;
>
> Silly request, but... Maybe this should be changed to plain old "unsigned int"
> and "unsigned
Greg KH wrote:
How about a whole new patch that I could apply? That would be better :)
Oops, sorry. :)
diff -Nru a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
--- a/drivers/i2c/busses/Kconfig2005-02-01 10:45:00 -07:00
+++ b/drivers/i2c/busses/Kconfig2005-02-01 10:45
On Mon, Jan 31, 2005 at 11:41:28AM -0700, Mark A. Greer wrote:
> Greg KH wrote:
>
> >On Tue, Jan 25, 2005 at 06:26:45PM -0700, Mark A. Greer wrote:
> >
> >
> >>+static inline void
> >>+mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
> >>
> >>
> >
> >This is a much too big of a f
Greg KH wrote:
On Tue, Jan 25, 2005 at 06:26:45PM -0700, Mark A. Greer wrote:
+static inline void
+mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
This is a much too big of a function to be "inline". Please change it.
Same for your other inline functions, that's not really n
On Tue, Jan 25, 2005 at 06:26:45PM -0700, Mark A. Greer wrote:
> +static inline void
> +mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
This is a much too big of a function to be "inline". Please change it.
Same for your other inline functions, that's not really needed, right?
> +
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. Howe
Jean Delvare wrote:
Hi Mark,
Thanks for the commenting, Jean.
+config I2C_MV64XXX
+ tristate "Marvell mv64xxx I2C Controller"
+ depends on I2C && MV64X60
&& EXPERIMENTAL?
Yes, I guess that's the correct thing to do. I'll add that.
diff -Nru a/include/linux/i2c-id.h b/include/linux/i2c-id.
Jean Delvare wrote:
+config I2C_MV64XXX
+ tristate "Marvell mv64xxx I2C Controller"
+ depends on I2C && MV64X60
&& EXPERIMENTAL?
Done.
+#define I2C_ALGO_MV64XXX 0x17 /* Marvell mv64xxx i2c ctlr */
0x17 is reserved within the legacy i2c project for an USB algorithm,
and 0
On Wed, Jan 26, 2005 at 02:56:55PM -0700, Mark A. Greer wrote:
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "i2c-mv64xxx.h"
Please put +static inlin
Greg KH wrote:
Please put
Done.
You have a lot of pr_debug and other printk() for stuff in this driver.
Please use dev_dbg(), dev_err() and friends instead. That way you get a
consistant message, that points to the exact device that is causing the
message.
Cool. Done.
You have some big inline fu
21 matches
Mail list logo
