On 14-02-03 11:22 PM, Heiko Schocher wrote: > 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<dra...@broadcom.com> >> Reviewed-by: Steve Rae<s...@broadcom.com> >> Reviewed-by: Tim Kryger<tkry...@linaro.org> >> --- > > What has changed in the v2 of your patch?
>From the initial patch set nothing has changed functionally, just some comment style cleanup. > >> 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) > ^ ^ Will fix this. > > 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 for the pointer. > > > Thanks! > > Beside of this, your patch looks good to me. Great, thanks for the review. > > bye, > Heiko _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot