Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2014-08-21 Thread Wolfram Sang

> I happen to have access to a kzm9d board meanwhile. Do you have the
> board enablement patches still somewhere?

Ping. Did you get this one?



signature.asc
Description: Digital signature


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2014-07-18 Thread Wolfram Sang
On Tue, Jun 17, 2014 at 11:57:24AM +0200, Wolfram Sang wrote:
> On Tue, Feb 18, 2014 at 06:38:43PM +0100, Wolfram Sang wrote:
> > On Tue, Sep 03, 2013 at 05:49:29PM +0100, Ian Molton wrote:
> > > Add a driver for the EMMA mobile I2C block.
> > > 
> > > The driver supports low and high-speed interrupt driven PIO transfers.
> > > 
> > > Signed-off-by: Ian Molton 
> > 
> > > --- a/drivers/i2c/busses/Kconfig
> > > +++ b/drivers/i2c/busses/Kconfig
> > > @@ -777,6 +777,16 @@ config I2C_RCAR
> > > This driver can also be built as a module.  If so, the module
> > > will be called i2c-rcar.
> > >  
> > > +config I2C_EM
> > > + tristate "EMMA Mobile series I2C adapter"
> > > + depends on I2C && HAVE_CLK
> > > + help
> > > +   If you say yes to this option, support will be included for the
> > > +   I2C interface on the Renesas Electronics EM/EV family of processors.
> > > +
> > > +   This driver can also be built as a module.  If so, the module
> > > +   will be called i2c-em
> > > +
> > 
> > Please keep it sorted.
> 
> Ping. Still any interest in this one?

I happen to have access to a kzm9d board meanwhile. Do you have the
board enablement patches still somewhere?



signature.asc
Description: Digital signature


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2014-06-17 Thread Wolfram Sang
On Tue, Feb 18, 2014 at 06:38:43PM +0100, Wolfram Sang wrote:
> On Tue, Sep 03, 2013 at 05:49:29PM +0100, Ian Molton wrote:
> > Add a driver for the EMMA mobile I2C block.
> > 
> > The driver supports low and high-speed interrupt driven PIO transfers.
> > 
> > Signed-off-by: Ian Molton 
> 
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -777,6 +777,16 @@ config I2C_RCAR
> >   This driver can also be built as a module.  If so, the module
> >   will be called i2c-rcar.
> >  
> > +config I2C_EM
> > +   tristate "EMMA Mobile series I2C adapter"
> > +   depends on I2C && HAVE_CLK
> > +   help
> > + If you say yes to this option, support will be included for the
> > + I2C interface on the Renesas Electronics EM/EV family of processors.
> > +
> > + This driver can also be built as a module.  If so, the module
> > + will be called i2c-em
> > +
> 
> Please keep it sorted.

Ping. Still any interest in this one?



signature.asc
Description: Digital signature


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2014-02-18 Thread Wolfram Sang
On Tue, Sep 03, 2013 at 05:49:29PM +0100, Ian Molton wrote:
> Add a driver for the EMMA mobile I2C block.
> 
> The driver supports low and high-speed interrupt driven PIO transfers.
> 
> Signed-off-by: Ian Molton 

> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -777,6 +777,16 @@ config I2C_RCAR
> This driver can also be built as a module.  If so, the module
> will be called i2c-rcar.
>  
> +config I2C_EM
> + tristate "EMMA Mobile series I2C adapter"
> + depends on I2C && HAVE_CLK
> + help
> +   If you say yes to this option, support will be included for the
> +   I2C interface on the Renesas Electronics EM/EV family of processors.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called i2c-em
> +

Please keep it sorted.

> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-em.c
> @@ -0,0 +1,501 @@
> +/*
> + * Copyright 2013 Codethink Ltd.
> + * Parts Copyright 2010 Renesas Electronics Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA.
> + */

Skip the address please.

> +static int em_i2c_xfer(struct i2c_adapter *, struct i2c_msg[], int);

You can skip this forward declaration by reordering.

> +
> +struct em_i2c_device {
> + struct i2c_adapter  adap;
> + wait_queue_head_t   i2c_wait;
> + void __iomem*membase;
> + struct clk  *clk;
> + struct clk  *sclk;
> + int irq;
> + int flags;
> + int pending;
> + spinlock_t  irq_lock;
> +};
> +
> +#define to_em_i2c(adap) (struct em_i2c_device *)i2c_get_adapdata(adap)
> +
> +static u32 em_i2c_func(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}

Have you tried SMBUS_QUICK (via 'i2cdetect -q')?

> +
> +static struct i2c_algorithm em_i2c_algo = {
> + .master_xfer = em_i2c_xfer,
> + .smbus_xfer = NULL,

No need to specify the NULL.

> + .functionality = em_i2c_func,
> +};
> +

> +static int em_i2c_wait_for_event(struct em_i2c_device *i2c_dev, u16 *status)
> +{
> + int interrupted;
> +
> + do {
> + interrupted = wait_event_interruptible_timeout(
> + i2c_dev->i2c_wait, i2c_dev->pending,
> + i2c_dev->adap.timeout);

Have you tested signals extensively? It can be done right, yet it is
complex. Most drivers decide to skip the interruptible.

> +
> + if (i2c_dev->pending) {
> + spin_lock_irq(&i2c_dev->irq_lock);
> + i2c_dev->pending = 0;
> + spin_unlock_irq(&i2c_dev->irq_lock);
> + *status = readl(i2c_dev->membase + I2C_OFS_IICSE0);
> + return 0;
> + }
> +
> + } while (interrupted);
> +
> + *status = 0;
> +
> + return -ETIMEDOUT;
> +}
> +
> +static int em_i2c_stop(struct em_i2c_device *i2c_dev)
> +{
> + u16 status;
> +
> + /* Send Stop condition */
> + writel((readl(i2c_dev->membase + I2C_OFS_IICC0) | I2C_BIT_SPT0 |
> + I2C_BIT_SPIE0), i2c_dev->membase + I2C_OFS_IICC0);

I'd think a em_set_bit() function would make the code more readable.

> +
> + /* Wait for stop condition */
> + em_i2c_wait_for_event(i2c_dev, &status);
> + /* FIXME - check status? */

What about the FIXME?

> +
> + if ((readl(i2c_dev->membase + I2C_OFS_IICSE0) & I2C_BIT_SPD0) != 0)

!= 0 is superfluous, same for == 1 later.

> + return 0;
> +
> + return -EBUSY;
> +}
> +
> + /* Send / receive data */
> + do {
> + /* Arbitration, Extension mode or Slave mode are errors*/
> + if (status & (I2C_BIT_EXC0 | I2C_BIT_COI0 | I2C_BIT_ALD0)
> + || !(status & I2C_BIT_MSTS0))
> + goto out_reset;
> +
> + if (!(status & I2C_BIT_TRC0)) { /* Read transaction */
> +
> + /* msg->flags is Write type */
> + if (!(msg->flags & I2C_M_RD))
> + goto out_reset;
> +
> + if (count == msg->len)
> + break;
> +
> + msg->buf[count++] =
> + readl(i2c_dev->membase + I2C_OFS_IIC0);
> +

Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2014-01-08 Thread Ian Molton

On 11/12/13 11:59, Wolfram Sang wrote:

No need to resend. I have it on my todo-list. Yet, by glimpsing at it I
found some issues which need a proper review for which I didn't have the
time so far. I hope to have it done by this week.


Ping :)

-Ian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2013-12-11 Thread Wolfram Sang
On Wed, Dec 11, 2013 at 11:09:04AM +0900, Simon Horman wrote:
> On Fri, Dec 06, 2013 at 08:52:38PM +, Ian Molton wrote:
> > On 25/09/13 05:45, Simon Horman wrote:
> > 
> > >Hi Ian,
> > >
> > >I spoke with Magnus and in turn Ben about this at LinuxCon in New Orleans
> > >last week.
> > >
> > >Basically the position of Magnus and I is that any support for this
> > >hardware is an incremental improvement on the current situation: no
> > >support.
> > >
> > >With this in mind from an shmobile point of view I am happy for this code.
> > >And there is no need to wait for a review from Magnus.
> > 
> > Has anyone merged this for upstream yet? If not, where should I send it?
> 
> I don't believe it has been merged.
> 
> My suggestion is to re-post it with my Ack with
> Wolfram Sang  CCed.

No need to resend. I have it on my todo-list. Yet, by glimpsing at it I
found some issues which need a proper review for which I didn't have the
time so far. I hope to have it done by this week.



signature.asc
Description: Digital signature


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2013-12-10 Thread Simon Horman
On Fri, Dec 06, 2013 at 08:52:38PM +, Ian Molton wrote:
> On 25/09/13 05:45, Simon Horman wrote:
> 
> >Hi Ian,
> >
> >I spoke with Magnus and in turn Ben about this at LinuxCon in New Orleans
> >last week.
> >
> >Basically the position of Magnus and I is that any support for this
> >hardware is an incremental improvement on the current situation: no
> >support.
> >
> >With this in mind from an shmobile point of view I am happy for this code.
> >And there is no need to wait for a review from Magnus.
> 
> Has anyone merged this for upstream yet? If not, where should I send it?

I don't believe it has been merged.

My suggestion is to re-post it with my Ack with
Wolfram Sang  CCed.

Also, I think you need to provide documentation for the bindings in
Documentation/devicetree/bindings/i2c.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2013-12-06 Thread Ian Molton

On 25/09/13 05:45, Simon Horman wrote:


Hi Ian,

I spoke with Magnus and in turn Ben about this at LinuxCon in New Orleans
last week.

Basically the position of Magnus and I is that any support for this
hardware is an incremental improvement on the current situation: no
support.

With this in mind from an shmobile point of view I am happy for this code.
And there is no need to wait for a review from Magnus.


Has anyone merged this for upstream yet? If not, where should I send it?

-Ian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2013-09-24 Thread Simon Horman
On Thu, Sep 05, 2013 at 03:04:29PM +0900, Simon Horman wrote:
> On Tue, Sep 03, 2013 at 05:49:29PM +0100, Ian Molton wrote:
> > Add a driver for the EMMA mobile I2C block.
> > 
> > The driver supports low and high-speed interrupt driven PIO transfers.
> > 
> > Signed-off-by: Ian Molton 
> 
> Magnus, could you find some time to review this?

Hi Ian,

I spoke with Magnus and in turn Ben about this at LinuxCon in New Orleans
last week.

Basically the position of Magnus and I is that any support for this
hardware is an incremental improvement on the current situation: no
support.

With this in mind from an shmobile point of view I am happy for this code.
And there is no need to wait for a review from Magnus.

Acked-by: Simon Horman 

> > ---
> >  drivers/i2c/busses/Kconfig  |   10 +
> >  drivers/i2c/busses/Makefile |1 +
> >  drivers/i2c/busses/i2c-em.c |  501 
> > +++
> >  3 files changed, 512 insertions(+)
> >  create mode 100644 drivers/i2c/busses/i2c-em.c
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index dc6dea6..d66d4b4 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -777,6 +777,16 @@ config I2C_RCAR
> >   This driver can also be built as a module.  If so, the module
> >   will be called i2c-rcar.
> >  
> > +config I2C_EM
> > +   tristate "EMMA Mobile series I2C adapter"
> > +   depends on I2C && HAVE_CLK
> > +   help
> > + If you say yes to this option, support will be included for the
> > + I2C interface on the Renesas Electronics EM/EV family of processors.
> > +
> > + This driver can also be built as a module.  If so, the module
> > + will be called i2c-em
> > +
> >  comment "External I2C/SMBus adapter drivers"
> >  
> >  config I2C_DIOLAN_U2C
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index d00997f..d330706 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -42,6 +42,7 @@ i2c-designware-platform-objs := i2c-designware-platdrv.o
> >  obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
> >  i2c-designware-pci-objs := i2c-designware-pcidrv.o
> >  obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
> > +obj-$(CONFIG_I2C_EM)+= i2c-em.o
> >  obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
> >  obj-$(CONFIG_I2C_HIGHLANDER)   += i2c-highlander.o
> >  obj-$(CONFIG_I2C_IBM_IIC)  += i2c-ibm_iic.o
> > diff --git a/drivers/i2c/busses/i2c-em.c b/drivers/i2c/busses/i2c-em.c
> > new file mode 100644
> > index 000..d7e91b4
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-em.c
> > @@ -0,0 +1,501 @@
> > +/*
> > + * Copyright 2013 Codethink Ltd.
> > + * Parts Copyright 2010 Renesas Electronics Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2
> > + * as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software Foundation,
> > + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* I2C Registers */
> > +#define I2C_OFS_IICACT00x00/* start */
> > +#define I2C_OFS_IIC0   0x04/* shift */
> > +#define I2C_OFS_IICC0  0x08/* control */
> > +#define I2C_OFS_SVA0   0x0c/* slave address */
> > +#define I2C_OFS_IICCL0 0x10/* clock select */
> > +#define I2C_OFS_IICX0  0x14/* extention */
> > +#define I2C_OFS_IICS0  0x18/* status */
> > +#define I2C_OFS_IICSE0 0x1c/* status For emulation */
> > +#define I2C_OFS_IICF0  0x20/* IIC flag */
> > +
> > +/* I2C IICACT0 Masks */
> > +#define I2C_BIT_IICE0  0x0001
> > +
> > +/* I2C IICC0 Masks */
> > +#define I2C_BIT_LREL0  0x0040
> > +#define I2C_BIT_WREL0  0x0020
> > +#define I2C_BIT_SPIE0  0x0010
> > +#define I2C_BIT_WTIM0  0x0008
> > +#define I2C_BIT_ACKE0  0x0004
> > +#define I2C_BIT_STT0   0x0002
> > +#define I2C_BIT_SPT0   0x0001
> > +
> > +/* I2C IICCL0 Masks */
> > +#define I2C_BIT_SMC0   0x0008
> > +#define I2C_BIT_DFC0   0x0004
> > +
> > +/* I2C IICSE0 Masks */
> > +#define I2C_BIT_MSTS0  0x0080
> > +#define I2C_BIT_ALD0

Re: [PATCH 1/2] I2C: EMMA Mobile I2C master driver

2013-09-04 Thread Simon Horman
On Tue, Sep 03, 2013 at 05:49:29PM +0100, Ian Molton wrote:
> Add a driver for the EMMA mobile I2C block.
> 
> The driver supports low and high-speed interrupt driven PIO transfers.
> 
> Signed-off-by: Ian Molton 

Magnus, could you find some time to review this?

> ---
>  drivers/i2c/busses/Kconfig  |   10 +
>  drivers/i2c/busses/Makefile |1 +
>  drivers/i2c/busses/i2c-em.c |  501 
> +++
>  3 files changed, 512 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-em.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index dc6dea6..d66d4b4 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -777,6 +777,16 @@ config I2C_RCAR
> This driver can also be built as a module.  If so, the module
> will be called i2c-rcar.
>  
> +config I2C_EM
> + tristate "EMMA Mobile series I2C adapter"
> + depends on I2C && HAVE_CLK
> + help
> +   If you say yes to this option, support will be included for the
> +   I2C interface on the Renesas Electronics EM/EV family of processors.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called i2c-em
> +
>  comment "External I2C/SMBus adapter drivers"
>  
>  config I2C_DIOLAN_U2C
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index d00997f..d330706 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -42,6 +42,7 @@ i2c-designware-platform-objs := i2c-designware-platdrv.o
>  obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
>  i2c-designware-pci-objs := i2c-designware-pcidrv.o
>  obj-$(CONFIG_I2C_EG20T)  += i2c-eg20t.o
> +obj-$(CONFIG_I2C_EM)+= i2c-em.o
>  obj-$(CONFIG_I2C_GPIO)   += i2c-gpio.o
>  obj-$(CONFIG_I2C_HIGHLANDER) += i2c-highlander.o
>  obj-$(CONFIG_I2C_IBM_IIC)+= i2c-ibm_iic.o
> diff --git a/drivers/i2c/busses/i2c-em.c b/drivers/i2c/busses/i2c-em.c
> new file mode 100644
> index 000..d7e91b4
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-em.c
> @@ -0,0 +1,501 @@
> +/*
> + * Copyright 2013 Codethink Ltd.
> + * Parts Copyright 2010 Renesas Electronics Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* I2C Registers */
> +#define I2C_OFS_IICACT0  0x00/* start */
> +#define I2C_OFS_IIC0 0x04/* shift */
> +#define I2C_OFS_IICC00x08/* control */
> +#define I2C_OFS_SVA0 0x0c/* slave address */
> +#define I2C_OFS_IICCL0   0x10/* clock select */
> +#define I2C_OFS_IICX00x14/* extention */
> +#define I2C_OFS_IICS00x18/* status */
> +#define I2C_OFS_IICSE0   0x1c/* status For emulation */
> +#define I2C_OFS_IICF00x20/* IIC flag */
> +
> +/* I2C IICACT0 Masks */
> +#define I2C_BIT_IICE00x0001
> +
> +/* I2C IICC0 Masks */
> +#define I2C_BIT_LREL00x0040
> +#define I2C_BIT_WREL00x0020
> +#define I2C_BIT_SPIE00x0010
> +#define I2C_BIT_WTIM00x0008
> +#define I2C_BIT_ACKE00x0004
> +#define I2C_BIT_STT0 0x0002
> +#define I2C_BIT_SPT0 0x0001
> +
> +/* I2C IICCL0 Masks */
> +#define I2C_BIT_SMC0 0x0008
> +#define I2C_BIT_DFC0 0x0004
> +
> +/* I2C IICSE0 Masks */
> +#define I2C_BIT_MSTS00x0080
> +#define I2C_BIT_ALD0 0x0040
> +#define I2C_BIT_EXC0 0x0020
> +#define I2C_BIT_COI0 0x0010
> +#define I2C_BIT_TRC0 0x0008
> +#define I2C_BIT_ACKD00x0004
> +#define I2C_BIT_STD0 0x0002
> +#define I2C_BIT_SPD0 0x0001
> +
> +/* I2C IICF0 Masks */
> +#define I2C_BIT_STCF 0x0080
> +#define I2C_BIT_IICBSY   0x0040
> +#define I2C_BIT_STCEN0x0002
> +#define I2C_BIT_IICRSV   0x0001
> +
> +static int em_i2c_xfer(struct i2c_adapter *, struct i2c_msg[], int);
> +
> +struct em_i2c_device {
> + struct i2c_adapter  adap;
> + wait_queue_head_t   i2c_wait;
> + void __iomem