Re: Fwd: [PATCH 1/3] i2c: Add Device Tree support to the NomadikI2C driver

[Lee Jones]

On 15/06/12 14:37, Srinidhi Kasagar wrote:
> On Fri, Jun 15, 2012 at 15:18:58 +0200, Lee Jones wrote:
> > On 15/06/12 14:05, Srinidhi Kasagar wrote:
> > > On Fri, Jun 15, 2012 at 14:45:11 +0200, Lee Jones wrote:
> > > > On 15/06/12 12:50, Srinidhi Kasagar wrote:
> > > > > [...]
> > > > > > From: Lee Jones<lee.jones@xxxxxxxxxx>
> > > > > > Date: Tue, 17 Apr 2012 16:04:13 +0100
> > > > > > Subject: [PATCH 1/1] i2c: Add Device Tree support to the Nomadik I2C driver
> > > > > >
> > > > > > Here we apply the bindings required for successful Device Tree
> > > > > > probing of the i2c-nomadik driver. We also apply a fall-back
> > > > > > configuration in case either one is not provided, or a required
> > > > > > element is missing from the one supplied.
> > > > > >
> > > > > > Cc: linux-i2c@xxxxxxxxxxxxxxx
> > > > > > Signed-off-by: Lee Jones<lee.jones@xxxxxxxxxx>
> > > > > > ---
> > > > > >     drivers/i2c/busses/i2c-nomadik.c |   82 +++++++++++++++++++++++++++++++++++++-
> > > > > >     1 file changed, 80 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> > > > > > index a92440d..58e8114 100644
> > > > > > --- a/drivers/i2c/busses/i2c-nomadik.c
> > > > > > +++ b/drivers/i2c/busses/i2c-nomadik.c
> > > > > > @@ -23,6 +23,7 @@
> > > > > >     #include<linux/io.h>
> > > > > >     #include<linux/regulator/consumer.h>
> > > > > >     #include<linux/pm_runtime.h>
> > > > > > +#include<linux/of.h>
> > > > > >
> > > > > >     #include<plat/i2c.h>
> > > > > >
> > > > > > @@ -899,15 +900,86 @@ static const struct i2c_algorithm nmk_i2c_algo = {
> > > > > >           .functionality  = nmk_i2c_functionality
> > > > > >     };
> > > > > >
> > > > > > +static struct nmk_i2c_controller u8500_i2c = {
> > > > > > +       /*
> > > > > > +        * Slave data setup time; 250ns, 100ns, and 10ns, which
> > > > > > +        * is 14, 6 and 2 respectively for a 48Mhz i2c clock.
> > > > > > +        */
> > > > > > +       .slsu           = 0xe,
>
> And BTW, I forgot to mention: This slsu stuff is not needed at all.
> It is required only in case of slave mode operation, which the
> driver does not support. You can perhaps consider deprecating
> this parameter from device tree list as well as from platform data.

Thanks. I will add it to my TODO list for a latter patch-set.

> > > > > > +       .tft            = 1,      /* Tx FIFO threshold */
> > > > > > +       .rft            = 8,      /* Rx FIFO threshold */
> > > > > > +       .clk_freq       = 100000, /* std. mode operation */
> > > > > > +       .timeout        = 200,    /* Slave response timeout(ms) */
> > > > > > +       .sm             = I2C_FREQ_MODE_FAST,
> > > > >
> > > > > How is this possible? you are setting clk_freq as 100kb/s and mode
> > > > > as fast mode which is supposed to be 400kb/s.
> > > >
> > > > That's not how I read it:
> > >
> > > But it is not readable. It confuses people.
> >
> > I understood it. :)
> >
> > If you think it's unclear speak to the author, Linus. He's CC'ed.
>
> It is!
>
> /srinidhi


--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/