[U-Boot] [PATCH v2 4/6] i2c: kona: Add Kona I2C driver

Heiko Schocher hs at denx.de
Tue Feb 4 08:22:31 CET 2014


Hello Darwin,

Am 03.02.2014 23:12, schrieb Darwin Rambo:
> Add support for the Kona I2C controller found on Broadcom mobile SoCs.
>
> Signed-off-by: Darwin Rambo<drambo at broadcom.com>
> Reviewed-by: Steve Rae<srae at broadcom.com>
> Reviewed-by: Tim Kryger<tkryger at linaro.org>
> ---

What has changed in the v2 of your patch?

>   drivers/i2c/Makefile   |    1 +
>   drivers/i2c/kona_i2c.c |  730 ++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 731 insertions(+)
>   create mode 100644 drivers/i2c/kona_i2c.c
[...]
> diff --git a/drivers/i2c/kona_i2c.c b/drivers/i2c/kona_i2c.c
> new file mode 100644
> index 0000000..9f18b74
> --- /dev/null
> +++ b/drivers/i2c/kona_i2c.c
> @@ -0,0 +1,730 @@
> +/*
> + * Copyright 2013 Broadcom Corporation.  All rights reserved.
> + *
> + * SPDX-License-Identifier:      GPL-2.0+
> + */
> +
> +#include<common.h>
> +#include<asm/io.h>
> +#include<asm/errno.h>
> +#include<asm/arch/sysmap.h>
> +#include<asm/kona-common/clk.h>
> +#include<i2c.h>
> +
> +/* Hardware register offsets and field defintions */
> +#define CS_OFFSET				0x00000020
> +#define CS_ACK_SHIFT				3
> +#define CS_ACK_MASK				0x00000008
> +#define CS_ACK_CMD_GEN_START			0x00000000
> +#define CS_ACK_CMD_GEN_RESTART			0x00000001
> +#define CS_CMD_SHIFT				1
> +#define CS_CMD_CMD_NO_ACTION			0x00000000
> +#define CS_CMD_CMD_START_RESTART		0x00000001
> +#define CS_CMD_CMD_STOP				0x00000002
> +#define CS_EN_SHIFT				0
> +#define CS_EN_CMD_ENABLE_BSC			0x00000001
> +
> +#define TIM_OFFSET				0x00000024
> +#define TIM_PRESCALE_SHIFT			6
> +#define TIM_P_SHIFT				3
> +#define TIM_NO_DIV_SHIFT			2
> +#define TIM_DIV_SHIFT				0
> +
> +#define DAT_OFFSET				0x00000028
> +
> +#define TOUT_OFFSET				0x0000002c
> +
> +#define TXFCR_OFFSET				0x0000003c
> +#define TXFCR_FIFO_FLUSH_MASK			0x00000080
> +#define TXFCR_FIFO_EN_MASK			0x00000040
> +
> +#define IER_OFFSET				0x00000044
> +#define IER_READ_COMPLETE_INT_MASK		0x00000010
> +#define IER_I2C_INT_EN_MASK			0x00000008
> +#define IER_FIFO_INT_EN_MASK			0x00000002
> +#define IER_NOACK_EN_MASK			0x00000001
> +
> +#define ISR_OFFSET				0x00000048
> +#define ISR_RESERVED_MASK			0xffffff60
> +#define ISR_CMDBUSY_MASK			0x00000080
> +#define ISR_READ_COMPLETE_MASK			0x00000010
> +#define ISR_SES_DONE_MASK			0x00000008
> +#define ISR_ERR_MASK				0x00000004
> +#define ISR_TXFIFOEMPTY_MASK			0x00000002
> +#define ISR_NOACK_MASK				0x00000001
> +
> +#define CLKEN_OFFSET				0x0000004c
> +#define CLKEN_AUTOSENSE_OFF_MASK		0x00000080
> +#define CLKEN_M_SHIFT				4
> +#define CLKEN_N_SHIFT				1
> +#define CLKEN_CLKEN_MASK			0x00000001
> +
> +#define FIFO_STATUS_OFFSET			0x00000054
> +#define FIFO_STATUS_RXFIFO_EMPTY_MASK		0x00000004
> +#define FIFO_STATUS_TXFIFO_EMPTY_MASK		0x00000010
> +
> +#define HSTIM_OFFSET				0x00000058
> +#define HSTIM_HS_MODE_MASK			0x00008000
> +#define HSTIM_HS_HOLD_SHIFT			10
> +#define HSTIM_HS_HIGH_PHASE_SHIFT		5
> +#define HSTIM_HS_SETUP_SHIFT			0
> +
> +#define PADCTL_OFFSET				0x0000005c
> +#define PADCTL_PAD_OUT_EN_MASK			0x00000004
> +
> +#define RXFCR_OFFSET				0x00000068
> +#define RXFCR_NACK_EN_SHIFT			7
> +#define RXFCR_READ_COUNT_SHIFT			0
> +#define RXFIFORDOUT_OFFSET			0x0000006c
> +
> +/* Locally used constants */
> +#define MAX_RX_FIFO_SIZE		64U	/* bytes */
> +#define MAX_TX_FIFO_SIZE		64U	/* bytes */
> +
> +#define I2C_TIMEOUT			100000	/* usecs */
> +
> +#define WAIT_INT_CHK			100	/* usecs */
> +#if I2C_TIMEOUT % WAIT_INT_CHK
> +#error I2C_TIMEOUT must be a multiple of WAIT_INT_CHK
> +#endif
> +
> +/* Operations that can be commanded to the controller */
> +enum bcm_kona_cmd_t {
> +	BCM_CMD_NOACTION = 0,
> +	BCM_CMD_START,
> +	BCM_CMD_RESTART,
> +	BCM_CMD_STOP,
> +};
> +
> +enum bus_speed_index {
> +	BCM_SPD_100K = 0,
> +	BCM_SPD_400K,
> +	BCM_SPD_1MHZ,
> +};
> +
> +/* Internal divider settings for standard mode, fast mode and fast mode plus */
> +struct bus_speed_cfg {
> +	uint8_t time_m;		/* Number of cycles for setup time */
> +	uint8_t time_n;		/* Number of cycles for hold time */
> +	uint8_t prescale;	/* Prescale divider */
> +	uint8_t time_p;		/* Timing coefficient */
> +	uint8_t no_div;		/* Disable clock divider */
> +	uint8_t time_div;	/* Post-prescale divider */
> +};
> +
> +static const struct bus_speed_cfg std_cfg_table[] = {
> +	[BCM_SPD_100K] = {0x01, 0x01, 0x03, 0x06, 0x00, 0x02},
> +	[BCM_SPD_400K] = {0x05, 0x01, 0x03, 0x05, 0x01, 0x02},
> +	[BCM_SPD_1MHZ] = {0x01, 0x01, 0x03, 0x01, 0x01, 0x03},
> +};
> +
> +struct bcm_kona_i2c_dev {
> +	void *base;
> +	uint speed;
> +	const struct bus_speed_cfg *std_cfg;
> +};
> +
> +/* Keep these two defines in sync */
> +#define DEF_SPD 100000
> +#define DEF_SPD_ENUM BCM_SPD_100K
> +
> +#define DEF_DEVICE(num) \
> +{(void *)CONFIG_SYS_I2C_BASE##num, DEF_SPD,&std_cfg_table[DEF_SPD_ENUM]}
> +
> +static struct bcm_kona_i2c_dev g_i2c_devs[CONFIG_SYS_MAX_I2C_BUS] = {
> +#ifdef CONFIG_SYS_I2C_BASE0
> +	DEF_DEVICE(0),
> +#endif
> +#ifdef CONFIG_SYS_I2C_BASE1
> +	DEF_DEVICE(1),
> +#endif
> +#ifdef CONFIG_SYS_I2C_BASE2
> +	DEF_DEVICE(2),
> +#endif
> +#ifdef CONFIG_SYS_I2C_BASE3
> +	DEF_DEVICE(3),
> +#endif
> +#ifdef CONFIG_SYS_I2C_BASE4
> +	DEF_DEVICE(4),
> +#endif
> +#ifdef CONFIG_SYS_I2C_BASE5
> +	DEF_DEVICE(5),
> +#endif
> +};
> +
> +#define I2C_M_TEN	0x0010	/* ten bit address */
> +#define I2C_M_RD	0x0001	/* read data */
> +#define I2C_M_NOSTART	0x4000	/* no restart between msgs */
> +
> +struct i2c_msg {
> +	uint16_t addr;
> +	uint16_t flags;
> +	uint16_t len;
> +	uint8_t *buf;
> +};
> +
> +static void bcm_kona_i2c_send_cmd_to_ctrl(struct bcm_kona_i2c_dev *dev,
> +					  enum bcm_kona_cmd_t cmd)
> +{
> +	debug("%s, %d\n", __func__, cmd);
> +
> +	switch (cmd) {
> +	case BCM_CMD_NOACTION:
> +		writel((CS_CMD_CMD_NO_ACTION<<  CS_CMD_SHIFT) |

Nitpicking, should be:
writel((CS_CMD_CMD_NO_ACTION << CS_CMD_SHIFT)
                             ^  ^

Such formatting style seems to go through your complete patch ... maybe
you got this source from somewhere, and you do not want to change this.
But if so, please add such information in your commit message, see:

http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign

Thanks!

Beside of this, your patch looks good to me.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


More information about the U-Boot mailing list