[U-Boot] [PATCH] i2c: adding driver for pxa27x, pxa3xx, pxa25x

[Marek Vasut]

On Wednesday, August 12, 2015 at 08:43:52 PM, Andrew Ruder wrote:

Commit message is missing :'-(

> Cc: Marek Vasut <marex at denx.de>
> Cc: Heiko Schocher <hs at denx.de>
> Signed-off-by: Andrew Ruder <andrew.ruder at elecsyscorp.com>
> ---
> 
> This driver was written before the driver model stuff was really around (or
> at least based on a U-Boot version that only had very preliminary support)
> so there might be some older conventions in use here.  I have tested on a
> PXA270 board running upstream master with this driver, however - so to
> some extent this all still works fine.  Let me know if things need to
> change to be accepted, and I'll try to find some time to update!
> 
>  drivers/i2c/Makefile  |   1 +
>  drivers/i2c/pxa_i2c.c | 796
> ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 797
> insertions(+)
>  create mode 100644 drivers/i2c/pxa_i2c.c
> 
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 9b45248..cf2c8a8 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_SYS_I2C_MXS) += mxs_i2c.o
>  obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o
>  obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o
>  obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o
> +obj-$(CONFIG_SYS_I2C_PXA) += pxa_i2c.o
>  obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o
>  obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o
>  obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o
> diff --git a/drivers/i2c/pxa_i2c.c b/drivers/i2c/pxa_i2c.c
> new file mode 100644
> index 0000000..859d6cf
> --- /dev/null
> +++ b/drivers/i2c/pxa_i2c.c
> @@ -0,0 +1,796 @@
> +/*
> + *  pxa_i2c.c
> + *
> + *  I2C adapter for the PXA I2C bus access.
> + *
> + *  Copyright (C) 2002 Intrinsyc Software Inc.
> + *  Copyright (C) 2004-2005 Deep Blue Solutions Ltd.
> + *  Copyright (C) 2014 Andrew Ruder
> + *
> + *  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 is a heavily modified version of the PXA I2C driver from the
> Linux + *  kernel 3.15.10.
> + */
> +
> +#include <common.h>
> +#include <compiler.h>
> +#include <i2c.h>
> +#include <asm/arch/pxa-regs.h>
> +
> +#include <asm/errno.h>
> +#include <asm/io.h>
> +
> +#define MAX_TRANSFER_MS 5000
> +#define WAIT_MASTER_MS 64
> +#define ABORT_LENGTH_MS 50
> +
> +/* Set up some defaults for various configurable pieces of this driver
> + * so that most of the time we don't need extra things defined

Multiline comment should be of the form:

/*
 * foo
 * bar
 */

> + */
> +#if !defined(CONFIG_SYS_PXA_STD_I2C_SPEED)
> +#define CONFIG_SYS_PXA_STD_I2C_SPEED 100000
> +#endif
> +
> +#if !defined(CONFIG_SYS_PXA_STD_I2C_SLAVE)
> +#define CONFIG_SYS_PXA_STD_I2C_SLAVE 1
> +#endif
> +
> +#if !defined(CONFIG_SYS_PXA_PWR_I2C_SPEED)
> +#define CONFIG_SYS_PXA_PWR_I2C_SPEED 40000
> +#endif
> +
> +#if !defined(CONFIG_SYS_PXA_PWR_I2C_SLAVE)
> +#define CONFIG_SYS_PXA_PWR_I2C_SLAVE 1
> +#endif

Please remove all this stuff, it's only going to hide bugs.

> +#if defined(CONFIG_CPU_MONAHANS) && defined(CONFIG_PXA_PWR_I2C)
> +#warning Monahans + PWRI2C is unlikely to work without additional testing
> +#endif
> +

[...]

> +#define ISR_ITE		(1 << 6)	   /* tx buffer empty */
> +#define ISR_IRF		(1 << 7)	   /* rx buffer full */
> +#define ISR_GCAD	(1 << 8)	   /* general call address detected */
> +#define ISR_SAD		(1 << 9)	   /* slave address detected */
> +#define ISR_BED		(1 << 10)	   /* bus error no ACK/NAK */
> +
> +struct pxa_i2c_msg {
> +	u16 addr;
> +	u16 flags;
> +#define I2C_M_TEN		0x0010	/* this is a ten bit chip address */

Be consistent about using 1 << n or 0x0n00 please. Just use the BIT(n) macro.

> +#define I2C_M_RD		0x0001	/* read data, from slave to master */
> +#define I2C_M_STOP		0x8000	/* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
> +#define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING 
*/
> +#define I2C_M_RECV_LEN		0x0400	/* length will be first received 
byte */
> +#define I2C_M_FIRST		0x0800	/* First message in a chain */
> +	u16 len;
> +	u8 *buf;
> +};
> +
> +struct pxa_i2c_state {
> +	struct pxa_i2c_msg	*msg;
> +	unsigned int		msg_num;
> +	unsigned int		msg_ptr;
> +};
> +
> +#define STD_I2C_BASE 0x40301680
> +#if defined(CONFIG_CPU_PXA27X) || defined(CONFIG_CPU_PXA25X)
> +#define PWR_I2C_BASE	0x40f00180
> +#define IBMR_OFFSET	0x00
> +#define IDBR_OFFSET	0x08
> +#define ICR_OFFSET	0x10
> +#define ISR_OFFSET	0x18
> +#define ISAR_OFFSET	0x20
> +#elif defined(CONFIG_CPU_MONAHANS)
> +#define PWR_I2C_BASE	0x40f500c0
> +#define IBMR_OFFSET	0x00
> +#define IDBR_OFFSET	0x04
> +#define ICR_OFFSET	0x08
> +#define ISR_OFFSET	0x0c
> +#define ISAR_OFFSET	0x10
> +#endif
> +
> +static inline void __iomem *
> +pxa_i2c_base(const struct i2c_adapter *adap)
> +{
> +	return (adap->hwadapnr == 0) ? (void *)STD_I2C_BASE : (void
> *)PWR_I2C_BASE; +}
> +
> +static inline void __iomem *
> +pxa_i2c_ibmr(const struct i2c_adapter *adap)
> +{
> +	return (void *)((char *)pxa_i2c_base(adap) + IBMR_OFFSET);

No need for these hideous casts, just use pxa_i2c_base() + OFFSET in the code.

> +}

[...]

> +static void
> +i2c_pxa_abort(struct i2c_adapter *adap)
> +{
> +	unsigned long long end;
> +
> +	end = get_ticks() + (get_tbclk() / 1000) * ABORT_LENGTH_MS;
> +
> +	while ((get_ticks() < end) &&

Please use get_timer() to implement the delay loop.

time = get_timer(0);
while (true) {
	reg = readl();
	if (reg & bit)
		break;

	if (get_timer(time) > timeout)
		return -ETIMEDOUT;
}

Also, the lpc32xx patches recently implemented a wait_for_bit() function.
If you make this generic, that'd be really cool.

> +	       (readl(_IBMR(adap)) & 0x1) == 0) {
> +		unsigned long icr = readl(_ICR(adap));
> +
> +		icr &= ~ICR_START;
> +		icr |= ICR_ACKNAK | ICR_STOP | ICR_TB;
> +
> +		writel(icr, _ICR(adap));
> +
> +		show_state(adap);
> +
> +		mdelay(1);
> +	}
> +
> +	writel(readl(_ICR(adap)) & ~(ICR_MA | ICR_START | ICR_STOP),
> +	       _ICR(adap));

Please use clrsetbits_le32();

> +}
[...]