Aspeed I2C driver, using Driver Model.

From: Maxim Sloyko <[email protected]>

aspeed-g5.dtsi include file is copied from
https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/dts/Makefile          |    2 +
 arch/arm/dts/aspeed-g5-evb.dts |   28 +
 arch/arm/dts/aspeed-g5.dtsi    | 1278 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1308 insertions(+)
 create mode 100644 arch/arm/dts/aspeed-g5-evb.dts
 create mode 100644 arch/arm/dts/aspeed-g5.dtsi

Quoted 5 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

aspeed-g5.dtsi include file is copied from
https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Quoted 8 lines

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/dts/Makefile          |    2 +
 arch/arm/dts/aspeed-g5-evb.dts |   28 +
 arch/arm/dts/aspeed-g5.dtsi    | 1278 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1308 insertions(+)
 create mode 100644 arch/arm/dts/aspeed-g5-evb.dts
 create mode 100644 arch/arm/dts/aspeed-g5.dtsi

Reviewed-by Simon Glass <[email protected]>

From: Maxim Sloyko <[email protected]>

Basically fixed FUC/FUN typo that went out of hand.

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/regs-scu.h | 73 ++++++++++++++++-------------
 arch/arm/mach-aspeed/ast-scu.c              |  2 +-
 2 files changed, 42 insertions(+), 33 deletions(-)

Hi Maxim,

Quoted 5 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

Basically fixed FUC/FUN typo that went out of hand.

Signed-off-by Maxim Sloyko <[email protected]>

This looks like a mix of whiltespace changes and other changes? If so, can you split it into two patches?

Quoted 5 lines

---
 arch/arm/include/asm/arch-aspeed/regs-scu.h | 73 ++++++++++++++++-------------
 arch/arm/mach-aspeed/ast-scu.c              |  2 +-
 2 files changed, 42 insertions(+), 33 deletions(-)

Regards,
Simon

From: Maxim Sloyko <[email protected]>

This is needed by I2C driver.

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  1 +
 arch/arm/mach-aspeed/ast-scu.c             | 11 +++++++++++
 2 files changed, 12 insertions(+)

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

index d248416..eb5aaa2 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

 extern u32 ast_scu_get_vga_memsize(void);
 

arch/arm/mach-aspeed/ast-scu.c +0 -0

index 280c421..e00dbe2 100644

arch/arm/mach-aspeed/ast-scu.c +7 -1

+        * (4 + 4 * DIV), according to datasheet
+        */
+       u32 apb_div = 4 + 4 * SCU_GET_PCLK_DIV(ast_scu_read(AST_SCU_CLK_SEL));
+       return h_pll / apb_div;
+}
+
+
 void ast_scu_show_system_info(void)
 {
 
--

2.8.0.rc3.226.g39d4020

Hi Maxim,

Quoted 2 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

For the subject, 'Add' rather than 'Added' (we use present tense)

Quoted 13 lines

This is needed by I2C driver.

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  1 +
 arch/arm/mach-aspeed/ast-scu.c             | 11 +++++++++++
 2 files changed, 12 insertions(+)

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

index d248416..eb5aaa2 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h extern void ast_scu_get_who_init_dram(void); · line 41

Quoted 4 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +1 -0

@@ -38,6 +38,7 @@ extern void ast_scu_get_who_init_dram(void);
 extern u32 ast_get_clk_source(void);
 extern u32 ast_get_h_pll_clk(void);
 extern u32 ast_get_ahbclk(void);
+extern u32 ast_get_apbclk(void);

Please add a comment as to what this does and what it returns.

Quoted 8 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

 extern u32 ast_scu_get_vga_memsize(void);
 

arch/arm/mach-aspeed/ast-scu.c +0 -0

index 280c421..e00dbe2 100644

arch/arm/mach-aspeed/ast-scu.c u32 ast_get_ahbclk(void) · line 323

Quoted 6 lines

arch/arm/mach-aspeed/ast-scu.c +3 -0

@@ -318,6 +318,17 @@ u32 ast_get_ahbclk(void)
 
 #endif /* AST_SOC_G5 */
 
+u32 ast_get_apbclk(void)
+{
+       u32 h_pll = ast_get_h_pll_clk();

Can this be ulong, or is there a reason it has to be exactly 32 bits?

blank line here (between declarations and code)

arch/arm/mach-aspeed/ast-scu.c u32 ast_get_ahbclk(void) · line 324

Quoted 1 line

arch/arm/mach-aspeed/ast-scu.c +1 -0

+       /* The formula for converting the bit pattern to divisor is

/*
* The formula...
* ...
*/

Quoted 11 lines

arch/arm/mach-aspeed/ast-scu.c +7 -1

+        * (4 + 4 * DIV), according to datasheet
+        */
+       u32 apb_div = 4 + 4 * SCU_GET_PCLK_DIV(ast_scu_read(AST_SCU_CLK_SEL));
+       return h_pll / apb_div;
+}
+
+
 void ast_scu_show_system_info(void)
 {
 
--

Quoted 2 lines

2.8.0.rc3.226.g39d4020

Regards,
Simon

[resend from correct address]

Quoted 7 lines · Simon Glass

Hi Maxim,

Quoted 2 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

For the subject, 'Add' rather than 'Added' (we use present tense)

Quoted 10 lines

Review

Quoted 10 lines

This is needed by I2C driver.

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  1 +
 arch/arm/mach-aspeed/ast-scu.c             | 11 +++++++++++
 2 files changed, 12 insertions(+)

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-aspeed/ast_scu.h

Quoted 10 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h extern void ast_scu_get_who_init_dram(void); · line 41

Quoted 8 lines

index d248416..eb5aaa2 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h +1 -0

@@ -38,6 +38,7 @@ extern void ast_scu_get_who_init_dram(void);
 extern u32 ast_get_clk_source(void);
 extern u32 ast_get_h_pll_clk(void);
 extern u32 ast_get_ahbclk(void);
+extern u32 ast_get_apbclk(void);

Please add a comment as to what this does and what it returns.

Quoted 5 lines

arch/arm/mach-aspeed/ast-

Quoted 5 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

 extern u32 ast_scu_get_vga_memsize(void);
 

arch/arm/mach-aspeed/ast- renamed from arch/arm/mach-aspeed/ast-scu.c +0 -0

scu.c

Quoted 21 lines

arch/arm/mach-aspeed/ast-scu.c u32 ast_get_ahbclk(void) · line 323

Quoted 10 lines

index 280c421..e00dbe2 100644

arch/arm/mach-aspeed/ast-scu.c +3 -0

@@ -318,6 +318,17 @@ u32 ast_get_ahbclk(void)
 
 #endif /* AST_SOC_G5 */
 
+u32 ast_get_apbclk(void)
+{
+       u32 h_pll = ast_get_h_pll_clk();

Can this be ulong, or is there a reason it has to be exactly 32 bits?

blank line here (between declarations and code)

arch/arm/mach-aspeed/ast-scu.c u32 ast_get_ahbclk(void) · line 324

Quoted 1 line

arch/arm/mach-aspeed/ast-scu.c +1 -0

+       /* The formula for converting the bit pattern to divisor is

/*
* The formula...
* ...
*/

Quoted 4 lines

arch/arm/mach-aspeed/ast-scu.c u32 ast_get_ahbclk(void)

Quoted 4 lines

arch/arm/mach-aspeed/ast-scu.c +3 -0

+        * (4 + 4 * DIV), according to datasheet
+        */
+       u32 apb_div = 4 + 4 * SCU_GET_PCLK_DIV(ast_scu_read(

AST_SCU_CLK_SEL));

Quoted 13 lines

Quoted 10 lines

arch/arm/mach-aspeed/ast-scu.c +4 -1

+       return h_pll / apb_div;
+}
+
+
 void ast_scu_show_system_info(void)
 {
 
--

2.8.0.rc3.226.g39d4020

Regards,
Simon

From: Maxim Sloyko <[email protected]>

In the absence of pinmux driver, I2C driver will be configuring pins directly.

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  5 +++++
 arch/arm/mach-aspeed/ast-scu.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

index eb5aaa2..80ebd6f 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h +1 -0

+
 #endif

arch/arm/mach-aspeed/ast-scu.c +28 -1

index e00dbe2..b5aa8bf 100644
@@ -507,3 +507,31 @@ void ast_scu_get_who_init_dram(void)
                break;
        }
 }
+
+void ast_scu_enable_i2c(u8 bus_num)
+{
+       if (bus_num > SCU_I2C_MAX_BUS_NUM) {
+               debug("%s: bus_num is out of range, must be [%d - %d]\n",
+                     __func__, SCU_I2C_MIN_BUS_NUM, SCU_I2C_MAX_BUS_NUM);
+               return;
+       }
+
+       if (bus_num == 0) {
+               /* Enable I2C Controllers */
+               clrbits_le32(AST_SCU_BASE + AST_SCU_RESET, SCU_RESET_I2C);
+       } else if (bus_num >= 3) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL5,
+                            SCU_FUN_PIN_I2C(bus_num));
+       /* In earlier versions of the SoC these pins are always assigned to
+        * respective I2C buses and require no configuration.
+        */
+#ifdef AST_SOC_G5
+       } else if (bus_num == 1) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA1 | SCU_FUN_PIN_SCL1);
+       } else if (bus_num == 2) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA2 | SCU_FUN_PIN_SCL2);
+#endif
+       }
+}
--

2.8.0.rc3.226.g39d4020

Hi Maxim,

Quoted 4 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

In the absence of pinmux driver, I2C driver will be configuring pins directly.

Commit subject s/Added/Add/

and please remove the '.' at the end.

Quoted 12 lines

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  5 +++++
 arch/arm/mach-aspeed/ast-scu.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

index eb5aaa2..80ebd6f 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h extern void ast_scu_init_eth(u8 num); · line 52

Quoted 7 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +4 -0

@@ -46,4 +46,9 @@ extern void ast_scu_init_eth(u8 num);
 extern void ast_scu_multi_func_eth(u8 num);
 extern void ast_scu_multi_func_romcs(u8 num);
 
+/* Enable I2C controller and pins for a particular device.
+ * Device numbering starts at 1
+ */
+extern void ast_scu_enable_i2c(u8 num);

I suspect this should be done as a pinctrl driver. See for drivers/pinctrl for examples.

Quoted 39 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +1 -0

+
 #endif

arch/arm/mach-aspeed/ast-scu.c +28 -1

index e00dbe2..b5aa8bf 100644
@@ -507,3 +507,31 @@ void ast_scu_get_who_init_dram(void)
                break;
        }
 }
+
+void ast_scu_enable_i2c(u8 bus_num)
+{
+       if (bus_num > SCU_I2C_MAX_BUS_NUM) {
+               debug("%s: bus_num is out of range, must be [%d - %d]\n",
+                     __func__, SCU_I2C_MIN_BUS_NUM, SCU_I2C_MAX_BUS_NUM);
+               return;
+       }
+
+       if (bus_num == 0) {
+               /* Enable I2C Controllers */
+               clrbits_le32(AST_SCU_BASE + AST_SCU_RESET, SCU_RESET_I2C);
+       } else if (bus_num >= 3) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL5,
+                            SCU_FUN_PIN_I2C(bus_num));
+       /* In earlier versions of the SoC these pins are always assigned to
+        * respective I2C buses and require no configuration.
+        */
+#ifdef AST_SOC_G5
+       } else if (bus_num == 1) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA1 | SCU_FUN_PIN_SCL1);
+       } else if (bus_num == 2) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA2 | SCU_FUN_PIN_SCL2);
+#endif
+       }
+}
--

Quoted 2 lines

2.8.0.rc3.226.g39d4020

Regards,
Simon

[resend from correct address]

Quoted 34 lines · Simon Glass

Hi Maxim,

Quoted 4 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

In the absence of pinmux driver, I2C driver will be configuring pins directly.

Commit subject s/Added/Add/

and please remove the '.' at the end.

Quoted 12 lines

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/ast_scu.h |  5 +++++
 arch/arm/mach-aspeed/ast-scu.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

arch/arm/include/asm/arch-aspeed/ast_scu.h +0 -0

index eb5aaa2..80ebd6f 100644

arch/arm/include/asm/arch-aspeed/ast_scu.h extern void ast_scu_init_eth(u8 num); · line 52

Quoted 7 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +4 -0

@@ -46,4 +46,9 @@ extern void ast_scu_init_eth(u8 num);
 extern void ast_scu_multi_func_eth(u8 num);
 extern void ast_scu_multi_func_romcs(u8 num);
 
+/* Enable I2C controller and pins for a particular device.
+ * Device numbering starts at 1
+ */
+extern void ast_scu_enable_i2c(u8 num);

I suspect this should be done as a pinctrl driver. See for drivers/pinctrl for examples.

Quoted 46 lines

Quoted 41 lines

arch/arm/include/asm/arch-aspeed/ast_scu.h +1 -0

+
 #endif

arch/arm/mach-aspeed/ast-scu.c +28 -1

index e00dbe2..b5aa8bf 100644
@@ -507,3 +507,31 @@ void ast_scu_get_who_init_dram(void)
                break;
        }
 }
+
+void ast_scu_enable_i2c(u8 bus_num)
+{
+       if (bus_num > SCU_I2C_MAX_BUS_NUM) {
+               debug("%s: bus_num is out of range, must be [%d - %d]\n",
+                     __func__, SCU_I2C_MIN_BUS_NUM, SCU_I2C_MAX_BUS_NUM);
+               return;
+       }
+
+       if (bus_num == 0) {
+               /* Enable I2C Controllers */
+               clrbits_le32(AST_SCU_BASE + AST_SCU_RESET, SCU_RESET_I2C);
+       } else if (bus_num >= 3) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL5,
+                            SCU_FUN_PIN_I2C(bus_num));
+       /* In earlier versions of the SoC these pins are always assigned to
+        * respective I2C buses and require no configuration.
+        */
+#ifdef AST_SOC_G5
+       } else if (bus_num == 1) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA1 | SCU_FUN_PIN_SCL1);
+       } else if (bus_num == 2) {
+               setbits_le32(AST_SCU_BASE + AST_SCU_FUN_PIN_CTRL8,
+                            SCU_FUN_PIN_SDA2 | SCU_FUN_PIN_SCL2);
+#endif
+       }
+}
--

Quoted 2 lines

2.8.0.rc3.226.g39d4020

Regards,
Simon

From: Maxim Sloyko <[email protected]>

The driver is very limited: only single master mode is supported and only byte-by-byte synchronous reads and writes are supported, no Pool Buffers or DMA.

Signed-off-by Maxim Sloyko <[email protected]>

---
 drivers/i2c/Kconfig   |   7 ++
 drivers/i2c/Makefile  |   1 +
 drivers/i2c/ast_i2c.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/ast_i2c.h | 143 +++++++++++++++++++++++
 4 files changed, 456 insertions(+)
 create mode 100644 drivers/i2c/ast_i2c.c
 create mode 100644 drivers/i2c/ast_i2c.h

drivers/i2c/Kconfig +7 -0

index 6e22bba..720c475 100644
@@ -90,6 +90,13 @@ config SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED
          enable status register. This config option can be enabled in such
          cases.
 
+config SYS_I2C_AST
+       bool "Aspeed I2C Controller"
+       depends on DM_I2C
+       help
+         Say yes here to select Aspeed I2C Host Controller. The driver
+         supports AST2500 and AST2400 controllers.
+
 config SYS_I2C_INTEL
        bool "Intel I2C/SMBUS driver"
        depends on DM_I2C

drivers/i2c/Makefile +1 -0

index 167424d..89e046e 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o
 obj-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
 obj-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o
 obj-$(CONFIG_SYS_I2C) += i2c_core.o
+obj-$(CONFIG_SYS_I2C_AST) += ast_i2c.o
 obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
 obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
 obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o

drivers/i2c/ast_i2c.c new file +11 -0

new file mode 100644
index 0000000..f2c132e
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>

drivers/i2c/ast_i2c.c new file +15 -0

+       debug("Enable Master for %p\n", i2c_bus->regs);
+       writel(AST_I2CD_MASTER_EN
+              | AST_I2CD_M_SDA_LOCK_EN
+              | AST_I2CD_MULTI_MASTER_DIS | AST_I2CD_M_SCL_DRIVE_EN,
+              &i2c_bus->regs->fcr);
+       debug("FCR: %p\n", &i2c_bus->regs->fcr);
+       /* Enable Interrupts */
+       writel(AST_I2CD_INTR_TX_ACK
+              | AST_I2CD_INTR_TX_NAK
+              | AST_I2CD_INTR_RX_DONE
+              | AST_I2CD_INTR_BUS_RECOVER_DONE
+              | AST_I2CD_INTR_NORMAL_STOP
+              | AST_I2CD_INTR_ABNORMAL, &i2c_bus->regs->icr);
+}
+

drivers/i2c/ast_i2c.c new file +16 -0

+{
+       /* Start and Send Device Address */
+       writel(devaddr, &i2c_base->trbbr);
+       writel(AST_I2CD_M_START_CMD | AST_I2CD_M_TX_CMD, &i2c_base->csr);
+       return ast_i2c_wait_tx(i2c_base);
+}
+
+static int ast_i2c_read_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8 *buffer,
+                            size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error =
+           ast_i2c_start_txn(i2c_base, (chip_addr << 1) | I2C_M_RD);
+       if (i2c_error < 0)
+               return i2c_error;

drivers/i2c/ast_i2c.c new file +30 -0

+       }
+       ast_i2c_clear_interrupts(i2c_base);
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;
+}
+
+static int ast_i2c_write_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8
+                             *buffer, size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error = ast_i2c_start_txn(i2c_base, (chip_addr << 1));
+       if (i2c_error < 0)
+               return i2c_error;
+
+       for (; len > 0; len--, buffer++) {
+               writel(*buffer, &i2c_base->trbbr);
+               writel(AST_I2CD_M_TX_CMD, &i2c_base->csr);
+               i2c_error = ast_i2c_wait_tx(i2c_base);
+               if (i2c_error < 0)
+                       return i2c_error;
+       }
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;

drivers/i2c/ast_i2c.c new file +25 -0

+       bool sda_high = csr & AST_I2CD_SDA_LINE_STS;
+       bool scl_high = csr & AST_I2CD_SCL_LINE_STS;
+       if (sda_high && scl_high) {
+               /* Bus is idle, no deblocking needed. */
+               return 0;
+       } else if (sda_high) {
+               /* Send stop command */
+               debug("Unterminated TXN in (%x), sending stop\n", csr);
+               deblock_error = ast_i2c_send_stop(i2c_base);
+       } else if (scl_high) {
+               /* Possibly stuck slave */
+               debug("Bus stuck (%x), attempting recovery\n", csr);
+               writel(AST_I2CD_BUS_RECOVER_CMD, &i2c_base->csr);
+               deblock_error = ast_i2c_wait_isr(
+                       i2c_base, AST_I2CD_INTR_BUS_RECOVER_DONE);
+       } else {
+               /* Just try to reinit the device. */
+               ast_i2c_init_bus(i2c_bus);
+       }
+
+       return deblock_error;
+}
+
+static int ast_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
+{

drivers/i2c/ast_i2c.c new file +13 -0

+                           ast_i2c_read_data(i2c_bus, msg->addr, msg->buf,
+                                               msg->len, (nmsgs == 1));
+               } else {
+                       debug("i2c_write: chip=0x%x, len=0x%x, flags=0x%x\n",
+                             msg->addr, msg->len, msg->flags);
+                       ret =
+                           ast_i2c_write_data(i2c_bus, msg->addr, msg->buf,
+                                                msg->len, (nmsgs == 1));
+               }
+               if (ret) {
+                       debug("%s: error (%d)\n", __func__, ret);
+                       return -EREMOTEIO;
+               }

drivers/i2c/ast_i2c.c new file +50 -0

+               return -EINVAL;
+       }
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+
+       i2c_bus->speed = speed;
+       /* TODO: get this from device tree */
+       u32 pclk = ast_get_apbclk();
+       u32 divider = pclk / speed;
+
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+       if (speed > 400000) {
+               debug("Enabling High Speed\n");
+               setbits_le32(&i2c_base->fcr, AST_I2CD_M_HIGH_SPEED_EN
+                            | AST_I2CD_M_SDA_DRIVE_1T_EN
+                            | AST_I2CD_SDA_DRIVE_1T_EN);
+               writel(0x3, &i2c_base->cactcr2);
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+       } else {
+               debug("Enabling Normal Speed\n");
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+               writel(AST_NO_TIMEOUT_CTRL, &i2c_base->cactcr2);
+       }
+
+       ast_i2c_clear_interrupts(i2c_base);
+       return 0;
+}
+
+static const struct dm_i2c_ops ast_i2c_ops = {
+       .xfer = ast_i2c_xfer,
+       .set_bus_speed = ast_i2c_set_speed,
+       .deblock = ast_i2c_deblock,
+};
+
+static const struct udevice_id ast_i2c_ids[] = {
+       {.compatible = "aspeed,ast2400-i2c-controller",},
+       {.compatible = "aspeed,ast2400-i2c-bus",},
+       {},
+};
+
+/* Tell GNU Indent to keep this as is: */
+/* *INDENT-OFF* */
+U_BOOT_DRIVER(i2c_aspeed) = {
+       .name = "i2c_aspeed",
+       .id = UCLASS_I2C,
+       .of_match = ast_i2c_ids,
+       .probe = ast_i2c_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_i2c),
+       .ops = &ast_i2c_ops,
+};
+/* *INDENT-ON* */

drivers/i2c/ast_i2c.h new file +19 -0

new file mode 100644
index 0000000..8926fd2
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef __AST_I2C_H_
+#define __AST_I2C_H_
+
+struct ast_i2c_regs {
+       uint32_t fcr;
+       uint32_t cactcr1;
+       uint32_t cactcr2;
+       uint32_t icr;
+       uint32_t isr;
+       uint32_t csr;
+       uint32_t sdar;
+       uint32_t pbcr;

drivers/i2c/ast_i2c.h new file +4 -0

+#define AST_I2CD_BUFF_SEL(x)                           (x << 20)
+#define AST_I2CD_M_SDA_LOCK_EN                 (0x1 << 16)
+#define AST_I2CD_MULTI_MASTER_DIS                      (0x1 << 15)
+#define AST_I2CD_M_SCL_DRIVE_EN                (0x1 << 14)

drivers/i2c/ast_i2c.h new file +82 -0

+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define AST_I2CD_tBUF                                          (0x1 << 28)
+#define AST_I2CD_tHDSTA                                                (0x1 << 24)
+#define AST_I2CD_tACST                                         (0x1 << 20)
+#define AST_I2CD_tCKHIGH                                       (0x1 << 16)
+#define AST_I2CD_tCKLOW                                                (0x1 << 12)
+#define AST_I2CD_tHDDAT                                                (0x1 << 10)
+#define AST_I2CD_CLK_TO_BASE_DIV                       (0x1 << 8)
+#define AST_I2CD_CLK_BASE_DIV                          (0x1)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define AST_I2CD_tTIMEOUT                                      (0x1)
+#define AST_NO_TIMEOUT_CTRL                                    0x0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define AST_I2CD_INTR_SDA_DL_TIMEOUT                   (0x1 << 14)
+#define AST_I2CD_INTR_BUS_RECOVER_DONE                 (0x1 << 13)
+#define AST_I2CD_INTR_SMBUS_ALERT                      (0x1 << 12)
+#define AST_I2CD_INTR_SMBUS_ARP_ADDR                   (0x1 << 11)
+#define AST_I2CD_INTR_SMBUS_DEV_ALERT_ADDR             (0x1 << 10)
+#define AST_I2CD_INTR_SMBUS_DEF_ADDR                   (0x1 << 9)
+#define AST_I2CD_INTR_GCALL_ADDR                       (0x1 << 8)
+#define AST_I2CD_INTR_SLAVE_MATCH                      (0x1 << 7)
+#define AST_I2CD_INTR_SCL_TIMEOUT                      (0x1 << 6)
+#define AST_I2CD_INTR_ABNORMAL                         (0x1 << 5)
+#define AST_I2CD_INTR_NORMAL_STOP                      (0x1 << 4)
+#define AST_I2CD_INTR_ARBIT_LOSS                       (0x1 << 3)
+#define AST_I2CD_INTR_RX_DONE                          (0x1 << 2)
+#define AST_I2CD_INTR_TX_NAK                           (0x1 << 1)
+#define AST_I2CD_INTR_TX_ACK                           (0x1 << 0)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define AST_I2CD_SDA_OE                                        (0x1 << 28)
+#define AST_I2CD_SDA_O                                 (0x1 << 27)
+#define AST_I2CD_SCL_OE                                        (0x1 << 26)
+#define AST_I2CD_SCL_O                                 (0x1 << 25)
+#define AST_I2CD_TX_TIMING                             (0x1 << 24)
+#define AST_I2CD_TX_STATUS                             (0x1 << 23)
+
+/* Tx State Machine */
+#define AST_I2CD_IDLE                                  0x0
+#define AST_I2CD_MACTIVE                               0x8
+#define AST_I2CD_MSTART                                        0x9
+#define AST_I2CD_MSTARTR                               0xa
+#define AST_I2CD_MSTOP                                 0xb
+#define AST_I2CD_MTXD                                  0xc
+#define AST_I2CD_MRXACK                                        0xd
+#define AST_I2CD_MRXD                                  0xe
+#define AST_I2CD_MTXACK                                0xf
+#define AST_I2CD_SWAIT                                 0x1
+#define AST_I2CD_SRXD                                  0x4
+#define AST_I2CD_STXACK                                0x5
+#define AST_I2CD_STXD                                  0x6
+#define AST_I2CD_SRXACK                                0x7
+#define AST_I2CD_RECOVER                               0x3
+
+#define AST_I2CD_SCL_LINE_STS                          (0x1 << 18)
+#define AST_I2CD_SDA_LINE_STS                          (0x1 << 17)
+#define AST_I2CD_BUS_BUSY_STS                          (0x1 << 16)
+#define AST_I2CD_SDA_OE_OUT_DIR                                (0x1 << 15)
+#define AST_I2CD_SDA_O_OUT_DIR                         (0x1 << 14)
+#define AST_I2CD_SCL_OE_OUT_DIR                                (0x1 << 13)
+#define AST_I2CD_SCL_O_OUT_DIR                         (0x1 << 12)
+#define AST_I2CD_BUS_RECOVER_CMD                       (0x1 << 11)
+#define AST_I2CD_S_ALT_EN                              (0x1 << 10)
+#define AST_I2CD_RX_DMA_ENABLE                         (0x1 << 9)
+#define AST_I2CD_TX_DMA_ENABLE                         (0x1 << 8)
+
+/* Command Bit */
+#define AST_I2CD_RX_BUFF_ENABLE                                (0x1 << 7)
+#define AST_I2CD_TX_BUFF_ENABLE                                (0x1 << 6)
+#define AST_I2CD_M_STOP_CMD                                    (0x1 << 5)
+#define AST_I2CD_M_S_RX_CMD_LAST                       (0x1 << 4)
+#define AST_I2CD_M_RX_CMD                                      (0x1 << 3)
+#define AST_I2CD_S_TX_CMD                                      (0x1 << 2)
+#define AST_I2CD_M_TX_CMD                                      (0x1 << 1)

drivers/i2c/ast_i2c.h new file +8 -1

+
+/* 0x20 : I2CD Transmit/Receive Byte Buffer Register   */
+#define AST_I2CD_GET_MODE(x)                           (((x) >> 8) & 0x1)
+
+#define AST_I2CD_RX_BYTE_BUFFER                                        (0xff << 8)
+#define AST_I2CD_TX_BYTE_BUFFER                                        (0xff)
+
+#endif                         /* __AST_I2C_H_ */
--

2.8.0.rc3.226.g39d4020

Hi Maxim,

Quoted 5 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

The driver is very limited: only single master mode is supported and only byte-by-byte synchronous reads and writes are supported, no Pool Buffers or DMA.

Please add that into the Kconfig help too.

Quoted 58 lines

Signed-off-by Maxim Sloyko <[email protected]>

---
 drivers/i2c/Kconfig   |   7 ++
 drivers/i2c/Makefile  |   1 +
 drivers/i2c/ast_i2c.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/ast_i2c.h | 143 +++++++++++++++++++++++
 4 files changed, 456 insertions(+)
 create mode 100644 drivers/i2c/ast_i2c.c
 create mode 100644 drivers/i2c/ast_i2c.h

drivers/i2c/Kconfig +7 -0

index 6e22bba..720c475 100644
@@ -90,6 +90,13 @@ config SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED
          enable status register. This config option can be enabled in such
          cases.
 
+config SYS_I2C_AST
+       bool "Aspeed I2C Controller"
+       depends on DM_I2C
+       help
+         Say yes here to select Aspeed I2C Host Controller. The driver
+         supports AST2500 and AST2400 controllers.
+
 config SYS_I2C_INTEL
        bool "Intel I2C/SMBUS driver"
        depends on DM_I2C

drivers/i2c/Makefile +1 -0

index 167424d..89e046e 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o
 obj-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
 obj-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o
 obj-$(CONFIG_SYS_I2C) += i2c_core.o
+obj-$(CONFIG_SYS_I2C_AST) += ast_i2c.o
 obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
 obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
 obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o

drivers/i2c/ast_i2c.c new file +11 -0

new file mode 100644
index 0000000..f2c132e
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>

drivers/i2c/ast_i2c.c line 21

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+#include <i2c.h>
+
+#include <asm/arch/ast_scu.h>
+#include <asm/arch/regs-scu.h>
+#include <asm/io.h>
+
+#include "ast_i2c.h"
+
+#define I2C_TIMEOUT_US (100000)
+#define I2C_SLEEP_STEP (20)

I2C_SLEEP_STEP_US

drivers/i2c/ast_i2c.c line 22

Quoted 1 line

drivers/i2c/ast_i2c.c new file +1 -0

+#define EI2C_TIMEOUT (1001)

EI2C_TIMEOUT_US

Please drop the () on those

drivers/i2c/ast_i2c.c line 26

Quoted 4 lines

drivers/i2c/ast_i2c.c new file +4 -0

+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct ast_i2c {

Please add a struct comment with member info.

drivers/i2c/ast_i2c.c line 32

Quoted 6 lines

drivers/i2c/ast_i2c.c new file +6 -0

+       u32 id;
+       struct ast_i2c_regs *regs;
+       int speed;
+};
+
+static u32 get_clk_reg_val(u32 divider_ratio)

Function comment. Consider returning ulong.

drivers/i2c/ast_i2c.c line 44

Quoted 12 lines

drivers/i2c/ast_i2c.c new file +12 -0

+{
+       unsigned int inc = 0, div;
+       u32 scl_low, scl_high, data;
+
+       for (div = 0; divider_ratio >= 16; div++) {
+               inc |= (divider_ratio & 1);
+               divider_ratio >>= 1;
+       }
+       divider_ratio += inc;
+       scl_low = (divider_ratio >> 1) - 1;
+       scl_high = divider_ratio - scl_low - 2;
+       data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;

blank link here. What is the magic 0x77700300 for? Should either have a comment or a #define

drivers/i2c/ast_i2c.c line 57

Quoted 13 lines

drivers/i2c/ast_i2c.c new file +13 -0

+       return data;
+}
+
+static inline void ast_i2c_clear_interrupts(struct ast_i2c_regs *i2c_base)
+{
+       writel(~0, &i2c_base->isr);
+}
+
+static void ast_i2c_init_bus(struct ast_i2c *i2c_bus)
+{
+       /* Reset device */
+       writel(0, &i2c_bus->regs->fcr);
+       /* Enable Master Mode. Assuming single-master. */

nit: Please avoid '.' at end of comments

Quoted 15 lines

drivers/i2c/ast_i2c.c new file +15 -0

+       debug("Enable Master for %p\n", i2c_bus->regs);
+       writel(AST_I2CD_MASTER_EN
+              | AST_I2CD_M_SDA_LOCK_EN
+              | AST_I2CD_MULTI_MASTER_DIS | AST_I2CD_M_SCL_DRIVE_EN,
+              &i2c_bus->regs->fcr);
+       debug("FCR: %p\n", &i2c_bus->regs->fcr);
+       /* Enable Interrupts */
+       writel(AST_I2CD_INTR_TX_ACK
+              | AST_I2CD_INTR_TX_NAK
+              | AST_I2CD_INTR_RX_DONE
+              | AST_I2CD_INTR_BUS_RECOVER_DONE
+              | AST_I2CD_INTR_NORMAL_STOP
+              | AST_I2CD_INTR_ABNORMAL, &i2c_bus->regs->icr);
+}
+

drivers/i2c/ast_i2c.c line 82

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+static int ast_i2c_probe(struct udevice *dev)
+{
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+
+       debug("Enabling I2C%u\n", dev->seq);
+       ast_scu_enable_i2c(dev->seq);
+
+       i2c_bus->id = dev->seq;
+       struct ast_i2c_regs *i2c_base =
+           (struct ast_i2c_regs *)dev_get_addr(dev);

Can you use dev_get_addr_ptr()? Need to check for NULL and return -EINVAL. Also, reading the device tree should really be in the ofdata_to_platdata() method, not probe().

drivers/i2c/ast_i2c.c line 91

Quoted 9 lines

drivers/i2c/ast_i2c.c new file +9 -0

+       i2c_bus->regs = i2c_base;
+
+       ast_i2c_init_bus(i2c_bus);
+       return 0;
+}
+
+static inline int ast_i2c_wait_isr(struct ast_i2c_regs *i2c_base, u32 flag)
+{
+       int timeout = I2C_TIMEOUT_US;

blank line here

drivers/i2c/ast_i2c.c line 99

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +8 -0

+       while (!(readl(&i2c_base->isr) & flag) && timeout > 0) {
+               udelay(I2C_SLEEP_STEP);
+               timeout -= I2C_SLEEP_STEP;
+       }
 
 
+
+       ast_i2c_clear_interrupts(i2c_base);
+       if (timeout <= 0)
+               return -EI2C_TIMEOUT;

return -ETIMEDOUT

drivers/i2c/ast_i2c.c line 105

Quoted 6 lines

drivers/i2c/ast_i2c.c new file +6 -0

+       return 0;
+}
+
+static inline int ast_i2c_send_stop(struct ast_i2c_regs *i2c_base)
+{
+       writel(AST_I2CD_M_STOP_CMD, &i2c_base->csr);

blank line (please do this before each return)

drivers/i2c/ast_i2c.c line 113

Quoted 8 lines

drivers/i2c/ast_i2c.c new file +8 -0

+       return ast_i2c_wait_isr(i2c_base, AST_I2CD_INTR_NORMAL_STOP);
+}
+
+static inline int ast_i2c_wait_tx(struct ast_i2c_regs *i2c_base)
+{
+       int timeout = I2C_TIMEOUT_US;
+       u32 flag = AST_I2CD_INTR_TX_ACK | AST_I2CD_INTR_TX_NAK;
+       u32 status = readl(&i2c_base->isr) & flag;

blank line (please do this after all decl blocks)

drivers/i2c/ast_i2c.c line 120

Quoted 7 lines

drivers/i2c/ast_i2c.c new file +7 -0

+       while (!status && timeout > 0) {
+               status = readl(&i2c_base->isr) & flag;
+               udelay(I2C_SLEEP_STEP);
+               timeout -= I2C_SLEEP_STEP;
+       }
+
+       int ret = 0;

Move this to top of function. Declarations should all be at the start of the block.

drivers/i2c/ast_i2c.c line 131

Quoted 11 lines

drivers/i2c/ast_i2c.c new file +11 -0

+       if (status == AST_I2CD_INTR_TX_NAK)
+               ret = -EREMOTEIO;
+
+       if (timeout <= 0)
+               ret = -EI2C_TIMEOUT;
+
+       ast_i2c_clear_interrupts(i2c_base);
+       return ret;
+}
+
+static inline int ast_i2c_start_txn(struct ast_i2c_regs *i2c_base, u8 devaddr)

Why inline? The compiler will do that anyway so I think you can drop it. Also please use int or uint for devaddr - particularly on ARM it isn't good to have non-register-sized parameters and return values.

Quoted 16 lines

drivers/i2c/ast_i2c.c new file +16 -0

+{
+       /* Start and Send Device Address */
+       writel(devaddr, &i2c_base->trbbr);
+       writel(AST_I2CD_M_START_CMD | AST_I2CD_M_TX_CMD, &i2c_base->csr);
+       return ast_i2c_wait_tx(i2c_base);
+}
+
+static int ast_i2c_read_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8 *buffer,
+                            size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error =
+           ast_i2c_start_txn(i2c_base, (chip_addr << 1) | I2C_M_RD);
+       if (i2c_error < 0)
+               return i2c_error;

drivers/i2c/ast_i2c.c line 157

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+
+       u32 i2c_cmd = AST_I2CD_M_RX_CMD;
+       for (; len > 0; len--, buffer++) {
+               if (len == 1)
+                       i2c_cmd |= AST_I2CD_M_S_RX_CMD_LAST;
+               writel(i2c_cmd, &i2c_base->csr);
+               i2c_error = ast_i2c_wait_isr(i2c_base, AST_I2CD_INTR_RX_DONE);
+               if (i2c_error < 0)
+                       return i2c_error;
+               *buffer = AST_I2CD_RX_DATA_BUF_GET(readl(&i2c_base->trbbr));

Instead of AST_I2CD_RX_DATA_BUF_GET(), please define something like AST_I2CD_RX_DATA_SHIFT and _MASK and use then here.

#define AST_I2CD_RX_DATA_SHIFT 8
#define AST_I2CD_RX_DATA_MASK (0xff << AST_I2CD_RX_DATA_SHIFT)

Quoted 30 lines

drivers/i2c/ast_i2c.c new file +30 -0

+       }
+       ast_i2c_clear_interrupts(i2c_base);
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;
+}
+
+static int ast_i2c_write_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8
+                             *buffer, size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error = ast_i2c_start_txn(i2c_base, (chip_addr << 1));
+       if (i2c_error < 0)
+               return i2c_error;
+
+       for (; len > 0; len--, buffer++) {
+               writel(*buffer, &i2c_base->trbbr);
+               writel(AST_I2CD_M_TX_CMD, &i2c_base->csr);
+               i2c_error = ast_i2c_wait_tx(i2c_base);
+               if (i2c_error < 0)
+                       return i2c_error;
+       }
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;

drivers/i2c/ast_i2c.c line 197

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+}
+
+static int ast_i2c_deblock(struct udevice *dev)
+{
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       u32 csr = readl(&i2c_base->csr);
+
+       int deblock_error = 0;

Just 'ret' is good enough.

Quoted 25 lines

drivers/i2c/ast_i2c.c new file +25 -0

+       bool sda_high = csr & AST_I2CD_SDA_LINE_STS;
+       bool scl_high = csr & AST_I2CD_SCL_LINE_STS;
+       if (sda_high && scl_high) {
+               /* Bus is idle, no deblocking needed. */
+               return 0;
+       } else if (sda_high) {
+               /* Send stop command */
+               debug("Unterminated TXN in (%x), sending stop\n", csr);
+               deblock_error = ast_i2c_send_stop(i2c_base);
+       } else if (scl_high) {
+               /* Possibly stuck slave */
+               debug("Bus stuck (%x), attempting recovery\n", csr);
+               writel(AST_I2CD_BUS_RECOVER_CMD, &i2c_base->csr);
+               deblock_error = ast_i2c_wait_isr(
+                       i2c_base, AST_I2CD_INTR_BUS_RECOVER_DONE);
+       } else {
+               /* Just try to reinit the device. */
+               ast_i2c_init_bus(i2c_bus);
+       }
+
+       return deblock_error;
+}
+
+static int ast_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
+{

drivers/i2c/ast_i2c.c line 232

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+       int ret;
+
+       (void)ast_i2c_deblock(dev);
+       debug("i2c_xfer: %d messages\n", nmsgs);
+       for (; nmsgs > 0; nmsgs--, msg++) {
+               if (msg->flags & I2C_M_RD) {
+                       debug("i2c_read: chip=0x%x, len=0x%x, flags=0x%x\n",
+                             msg->addr, msg->len, msg->flags);
+                       ret =

Can you break the line in the parameters instead of at '=' ?

Quoted 13 lines

drivers/i2c/ast_i2c.c new file +13 -0

+                           ast_i2c_read_data(i2c_bus, msg->addr, msg->buf,
+                                               msg->len, (nmsgs == 1));
+               } else {
+                       debug("i2c_write: chip=0x%x, len=0x%x, flags=0x%x\n",
+                             msg->addr, msg->len, msg->flags);
+                       ret =
+                           ast_i2c_write_data(i2c_bus, msg->addr, msg->buf,
+                                                msg->len, (nmsgs == 1));
+               }
+               if (ret) {
+                       debug("%s: error (%d)\n", __func__, ret);
+                       return -EREMOTEIO;
+               }

drivers/i2c/ast_i2c.c line 255

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+       }
+
+       return 0;
+}
+
+static int ast_i2c_set_speed(struct udevice *dev, unsigned int speed)
+{
+       debug("Setting speed ofr I2C%d to <%u>\n", dev->seq, speed);
+       if (!speed) {
+               debug("No valid speed specified.\n");

Drop '.'

Quoted 75 lines

drivers/i2c/ast_i2c.c new file +50 -0

+               return -EINVAL;
+       }
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+
+       i2c_bus->speed = speed;
+       /* TODO: get this from device tree */
+       u32 pclk = ast_get_apbclk();
+       u32 divider = pclk / speed;
+
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+       if (speed > 400000) {
+               debug("Enabling High Speed\n");
+               setbits_le32(&i2c_base->fcr, AST_I2CD_M_HIGH_SPEED_EN
+                            | AST_I2CD_M_SDA_DRIVE_1T_EN
+                            | AST_I2CD_SDA_DRIVE_1T_EN);
+               writel(0x3, &i2c_base->cactcr2);
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+       } else {
+               debug("Enabling Normal Speed\n");
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+               writel(AST_NO_TIMEOUT_CTRL, &i2c_base->cactcr2);
+       }
+
+       ast_i2c_clear_interrupts(i2c_base);
+       return 0;
+}
+
+static const struct dm_i2c_ops ast_i2c_ops = {
+       .xfer = ast_i2c_xfer,
+       .set_bus_speed = ast_i2c_set_speed,
+       .deblock = ast_i2c_deblock,
+};
+
+static const struct udevice_id ast_i2c_ids[] = {
+       {.compatible = "aspeed,ast2400-i2c-controller",},
+       {.compatible = "aspeed,ast2400-i2c-bus",},
+       {},
+};
+
+/* Tell GNU Indent to keep this as is: */
+/* *INDENT-OFF* */
+U_BOOT_DRIVER(i2c_aspeed) = {
+       .name = "i2c_aspeed",
+       .id = UCLASS_I2C,
+       .of_match = ast_i2c_ids,
+       .probe = ast_i2c_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_i2c),
+       .ops = &ast_i2c_ops,
+};
+/* *INDENT-ON* */

drivers/i2c/ast_i2c.h new file +19 -0

new file mode 100644
index 0000000..8926fd2
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef __AST_I2C_H_
+#define __AST_I2C_H_
+
+struct ast_i2c_regs {
+       uint32_t fcr;
+       uint32_t cactcr1;
+       uint32_t cactcr2;
+       uint32_t icr;
+       uint32_t isr;
+       uint32_t csr;
+       uint32_t sdar;
+       uint32_t pbcr;

drivers/i2c/ast_i2c.h line 29

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+       uint32_t trbbr;
+#ifdef CONFIG_TARGET_AST_G5
+       uint32_t dma_mbar;
+       uint32_t dma_tlr;
+#endif
+};
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register  */
+#define AST_I2CD_BUFF_SEL_MASK                         (0x7 << 20)

It's fine to have the AST_I2CD_ prefix if you want it. But it does make things longer, and this header will only be included in one file. So feel free to prune to AST_ if you like.

Quoted 4 lines

drivers/i2c/ast_i2c.h new file +4 -0

+#define AST_I2CD_BUFF_SEL(x)                           (x << 20)
+#define AST_I2CD_M_SDA_LOCK_EN                 (0x1 << 16)
+#define AST_I2CD_MULTI_MASTER_DIS                      (0x1 << 15)
+#define AST_I2CD_M_SCL_DRIVE_EN                (0x1 << 14)

drivers/i2c/ast_i2c.h line 43

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+#define AST_I2CD_MSB_STS                                       (0x1 << 9)
+#define AST_I2CD_SDA_DRIVE_1T_EN                       (0x1 << 8)
+#define AST_I2CD_M_SDA_DRIVE_1T_EN             (0x1 << 7)
+#define AST_I2CD_M_HIGH_SPEED_EN               (0x1 << 6)
+#define AST_I2CD_DEF_ADDR_EN                           (0x1 << 5)
+#define AST_I2CD_DEF_ALERT_EN                          (0x1 << 4)
+#define AST_I2CD_DEF_ARP_EN                                    (0x1 << 3)
+#define AST_I2CD_DEF_GCALL_EN                          (0x1 << 2)
+#define AST_I2CD_SLAVE_EN                                      (0x1 << 1)
+#define AST_I2CD_MASTER_EN                                     (0x1)

Avoid brackets around simple constants

Quoted 82 lines

drivers/i2c/ast_i2c.h new file +82 -0

+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define AST_I2CD_tBUF                                          (0x1 << 28)
+#define AST_I2CD_tHDSTA                                                (0x1 << 24)
+#define AST_I2CD_tACST                                         (0x1 << 20)
+#define AST_I2CD_tCKHIGH                                       (0x1 << 16)
+#define AST_I2CD_tCKLOW                                                (0x1 << 12)
+#define AST_I2CD_tHDDAT                                                (0x1 << 10)
+#define AST_I2CD_CLK_TO_BASE_DIV                       (0x1 << 8)
+#define AST_I2CD_CLK_BASE_DIV                          (0x1)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define AST_I2CD_tTIMEOUT                                      (0x1)
+#define AST_NO_TIMEOUT_CTRL                                    0x0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define AST_I2CD_INTR_SDA_DL_TIMEOUT                   (0x1 << 14)
+#define AST_I2CD_INTR_BUS_RECOVER_DONE                 (0x1 << 13)
+#define AST_I2CD_INTR_SMBUS_ALERT                      (0x1 << 12)
+#define AST_I2CD_INTR_SMBUS_ARP_ADDR                   (0x1 << 11)
+#define AST_I2CD_INTR_SMBUS_DEV_ALERT_ADDR             (0x1 << 10)
+#define AST_I2CD_INTR_SMBUS_DEF_ADDR                   (0x1 << 9)
+#define AST_I2CD_INTR_GCALL_ADDR                       (0x1 << 8)
+#define AST_I2CD_INTR_SLAVE_MATCH                      (0x1 << 7)
+#define AST_I2CD_INTR_SCL_TIMEOUT                      (0x1 << 6)
+#define AST_I2CD_INTR_ABNORMAL                         (0x1 << 5)
+#define AST_I2CD_INTR_NORMAL_STOP                      (0x1 << 4)
+#define AST_I2CD_INTR_ARBIT_LOSS                       (0x1 << 3)
+#define AST_I2CD_INTR_RX_DONE                          (0x1 << 2)
+#define AST_I2CD_INTR_TX_NAK                           (0x1 << 1)
+#define AST_I2CD_INTR_TX_ACK                           (0x1 << 0)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define AST_I2CD_SDA_OE                                        (0x1 << 28)
+#define AST_I2CD_SDA_O                                 (0x1 << 27)
+#define AST_I2CD_SCL_OE                                        (0x1 << 26)
+#define AST_I2CD_SCL_O                                 (0x1 << 25)
+#define AST_I2CD_TX_TIMING                             (0x1 << 24)
+#define AST_I2CD_TX_STATUS                             (0x1 << 23)
+
+/* Tx State Machine */
+#define AST_I2CD_IDLE                                  0x0
+#define AST_I2CD_MACTIVE                               0x8
+#define AST_I2CD_MSTART                                        0x9
+#define AST_I2CD_MSTARTR                               0xa
+#define AST_I2CD_MSTOP                                 0xb
+#define AST_I2CD_MTXD                                  0xc
+#define AST_I2CD_MRXACK                                        0xd
+#define AST_I2CD_MRXD                                  0xe
+#define AST_I2CD_MTXACK                                0xf
+#define AST_I2CD_SWAIT                                 0x1
+#define AST_I2CD_SRXD                                  0x4
+#define AST_I2CD_STXACK                                0x5
+#define AST_I2CD_STXD                                  0x6
+#define AST_I2CD_SRXACK                                0x7
+#define AST_I2CD_RECOVER                               0x3
+
+#define AST_I2CD_SCL_LINE_STS                          (0x1 << 18)
+#define AST_I2CD_SDA_LINE_STS                          (0x1 << 17)
+#define AST_I2CD_BUS_BUSY_STS                          (0x1 << 16)
+#define AST_I2CD_SDA_OE_OUT_DIR                                (0x1 << 15)
+#define AST_I2CD_SDA_O_OUT_DIR                         (0x1 << 14)
+#define AST_I2CD_SCL_OE_OUT_DIR                                (0x1 << 13)
+#define AST_I2CD_SCL_O_OUT_DIR                         (0x1 << 12)
+#define AST_I2CD_BUS_RECOVER_CMD                       (0x1 << 11)
+#define AST_I2CD_S_ALT_EN                              (0x1 << 10)
+#define AST_I2CD_RX_DMA_ENABLE                         (0x1 << 9)
+#define AST_I2CD_TX_DMA_ENABLE                         (0x1 << 8)
+
+/* Command Bit */
+#define AST_I2CD_RX_BUFF_ENABLE                                (0x1 << 7)
+#define AST_I2CD_TX_BUFF_ENABLE                                (0x1 << 6)
+#define AST_I2CD_M_STOP_CMD                                    (0x1 << 5)
+#define AST_I2CD_M_S_RX_CMD_LAST                       (0x1 << 4)
+#define AST_I2CD_M_RX_CMD                                      (0x1 << 3)
+#define AST_I2CD_S_TX_CMD                                      (0x1 << 2)
+#define AST_I2CD_M_TX_CMD                                      (0x1 << 1)

drivers/i2c/ast_i2c.h line 135

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+#define AST_I2CD_M_START_CMD                           (0x1)
+
+/* 0x18 : I2CD Slave Device Address Register   */
+
+/* 0x1C : I2CD Pool Buffer Control Register   */
+#define AST_I2CD_RX_BUF_ADDR_GET(x)                    (((x) >> 24) & 0xff)
+#define AST_I2CD_RX_BUF_END_ADDR_SET(x)                        ((x) << 16)
+#define AST_I2CD_TX_DATA_BUF_END_SET(x)                        (((x) & 0xff) << 8)
+#define AST_I2CD_RX_DATA_BUF_GET(x)                    (((x) >> 8) & 0xff)
+#define AST_I2CD_BUF_BASE_ADDR_SET(x)                  ((x) & 0x3f)

Just use simple shift and mask values, and put the logic in your C code.

Quoted 11 lines

drivers/i2c/ast_i2c.h new file +8 -1

+
+/* 0x20 : I2CD Transmit/Receive Byte Buffer Register   */
+#define AST_I2CD_GET_MODE(x)                           (((x) >> 8) & 0x1)
+
+#define AST_I2CD_RX_BYTE_BUFFER                                        (0xff << 8)
+#define AST_I2CD_TX_BYTE_BUFFER                                        (0xff)
+
+#endif                         /* __AST_I2C_H_ */
--

2.8.0.rc3.226.g39d4020

Regards,
Simon

[resend from correct address]

Quoted 10 lines · Simon Glass

Hi Maxim,

Quoted 5 lines · On 22 November 2016 at 16:56, <[email protected]> wrote:

From: Maxim Sloyko <[email protected]>

The driver is very limited: only single master mode is supported and only byte-by-byte synchronous reads and writes are supported, no Pool Buffers or DMA.

Please add that into the Kconfig help too.

Quoted 85 lines

Quoted 59 lines

Signed-off-by Maxim Sloyko <[email protected]>

---
 drivers/i2c/Kconfig   |   7 ++
 drivers/i2c/Makefile  |   1 +
 drivers/i2c/ast_i2c.c | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/ast_i2c.h | 143 +++++++++++++++++++++++
 4 files changed, 456 insertions(+)
 create mode 100644 drivers/i2c/ast_i2c.c
 create mode 100644 drivers/i2c/ast_i2c.h

drivers/i2c/Kconfig +7 -0

index 6e22bba..720c475 100644
@@ -90,6 +90,13 @@ config SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED
          enable status register. This config option can be enabled in such
          cases.
 
+config SYS_I2C_AST
+       bool "Aspeed I2C Controller"
+       depends on DM_I2C
+       help
+         Say yes here to select Aspeed I2C Host Controller. The driver
+         supports AST2500 and AST2400 controllers.
+
 config SYS_I2C_INTEL
        bool "Intel I2C/SMBUS driver"
        depends on DM_I2C

drivers/i2c/Makefile +1 -0

index 167424d..89e046e 100644
@@ -16,6 +16,7 @@ obj-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o
 obj-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
 obj-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o
 obj-$(CONFIG_SYS_I2C) += i2c_core.o
+obj-$(CONFIG_SYS_I2C_AST) += ast_i2c.o
 obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
 obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
 obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o

drivers/i2c/ast_i2c.c new file +11 -0

new file mode 100644
index 0000000..f2c132e
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>

drivers/i2c/ast_i2c.c line 21

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+#include <i2c.h>
+
+#include <asm/arch/ast_scu.h>
+#include <asm/arch/regs-scu.h>
+#include <asm/io.h>
+
+#include "ast_i2c.h"
+
+#define I2C_TIMEOUT_US (100000)
+#define I2C_SLEEP_STEP (20)

I2C_SLEEP_STEP_US

drivers/i2c/ast_i2c.c line 22

Quoted 1 line

drivers/i2c/ast_i2c.c new file +1 -0

+#define EI2C_TIMEOUT (1001)

EI2C_TIMEOUT_US

Please drop the () on those

drivers/i2c/ast_i2c.c line 26

Quoted 4 lines

drivers/i2c/ast_i2c.c new file +4 -0

+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct ast_i2c {

Please add a struct comment with member info.

Quoted 8 lines

drivers/i2c/ast_i2c.c line 32

Quoted 6 lines

drivers/i2c/ast_i2c.c new file +6 -0

+       u32 id;
+       struct ast_i2c_regs *regs;
+       int speed;
+};
+
+static u32 get_clk_reg_val(u32 divider_ratio)

Function comment. Consider returning ulong.

Quoted 492 lines

drivers/i2c/ast_i2c.c line 44

Quoted 13 lines

drivers/i2c/ast_i2c.c new file +12 -0

+{
+       unsigned int inc = 0, div;
+       u32 scl_low, scl_high, data;
+
+       for (div = 0; divider_ratio >= 16; div++) {
+               inc |= (divider_ratio & 1);
+               divider_ratio >>= 1;
+       }
+       divider_ratio += inc;
+       scl_low = (divider_ratio >> 1) - 1;
+       scl_high = divider_ratio - scl_low - 2;
+       data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;

blank link here. What is the magic 0x77700300 for? Should either have a comment or a #define

drivers/i2c/ast_i2c.c line 57

Quoted 13 lines

drivers/i2c/ast_i2c.c new file +13 -0

+       return data;
+}
+
+static inline void ast_i2c_clear_interrupts(struct ast_i2c_regs *i2c_base)
+{
+       writel(~0, &i2c_base->isr);
+}
+
+static void ast_i2c_init_bus(struct ast_i2c *i2c_bus)
+{
+       /* Reset device */
+       writel(0, &i2c_bus->regs->fcr);
+       /* Enable Master Mode. Assuming single-master. */

nit: Please avoid '.' at end of comments

Quoted 15 lines

drivers/i2c/ast_i2c.c new file +15 -0

+       debug("Enable Master for %p\n", i2c_bus->regs);
+       writel(AST_I2CD_MASTER_EN
+              | AST_I2CD_M_SDA_LOCK_EN
+              | AST_I2CD_MULTI_MASTER_DIS | AST_I2CD_M_SCL_DRIVE_EN,
+              &i2c_bus->regs->fcr);
+       debug("FCR: %p\n", &i2c_bus->regs->fcr);
+       /* Enable Interrupts */
+       writel(AST_I2CD_INTR_TX_ACK
+              | AST_I2CD_INTR_TX_NAK
+              | AST_I2CD_INTR_RX_DONE
+              | AST_I2CD_INTR_BUS_RECOVER_DONE
+              | AST_I2CD_INTR_NORMAL_STOP
+              | AST_I2CD_INTR_ABNORMAL, &i2c_bus->regs->icr);
+}
+

drivers/i2c/ast_i2c.c line 82

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+static int ast_i2c_probe(struct udevice *dev)
+{
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+
+       debug("Enabling I2C%u\n", dev->seq);
+       ast_scu_enable_i2c(dev->seq);
+
+       i2c_bus->id = dev->seq;
+       struct ast_i2c_regs *i2c_base =
+           (struct ast_i2c_regs *)dev_get_addr(dev);

Can you use dev_get_addr_ptr()? Need to check for NULL and return -EINVAL. Also, reading the device tree should really be in the ofdata_to_platdata() method, not probe().

drivers/i2c/ast_i2c.c line 91

Quoted 9 lines

drivers/i2c/ast_i2c.c new file +9 -0

+       i2c_bus->regs = i2c_base;
+
+       ast_i2c_init_bus(i2c_bus);
+       return 0;
+}
+
+static inline int ast_i2c_wait_isr(struct ast_i2c_regs *i2c_base, u32 flag)
+{
+       int timeout = I2C_TIMEOUT_US;

blank line here

drivers/i2c/ast_i2c.c line 99

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +8 -0

+       while (!(readl(&i2c_base->isr) & flag) && timeout > 0) {
+               udelay(I2C_SLEEP_STEP);
+               timeout -= I2C_SLEEP_STEP;
+       }
 
 
+
+       ast_i2c_clear_interrupts(i2c_base);
+       if (timeout <= 0)
+               return -EI2C_TIMEOUT;

return -ETIMEDOUT

drivers/i2c/ast_i2c.c line 105

Quoted 6 lines

drivers/i2c/ast_i2c.c new file +6 -0

+       return 0;
+}
+
+static inline int ast_i2c_send_stop(struct ast_i2c_regs *i2c_base)
+{
+       writel(AST_I2CD_M_STOP_CMD, &i2c_base->csr);

blank line (please do this before each return)

drivers/i2c/ast_i2c.c line 113

Quoted 8 lines

drivers/i2c/ast_i2c.c new file +8 -0

+       return ast_i2c_wait_isr(i2c_base, AST_I2CD_INTR_NORMAL_STOP);
+}
+
+static inline int ast_i2c_wait_tx(struct ast_i2c_regs *i2c_base)
+{
+       int timeout = I2C_TIMEOUT_US;
+       u32 flag = AST_I2CD_INTR_TX_ACK | AST_I2CD_INTR_TX_NAK;
+       u32 status = readl(&i2c_base->isr) & flag;

blank line (please do this after all decl blocks)

drivers/i2c/ast_i2c.c line 120

Quoted 7 lines

drivers/i2c/ast_i2c.c new file +7 -0

+       while (!status && timeout > 0) {
+               status = readl(&i2c_base->isr) & flag;
+               udelay(I2C_SLEEP_STEP);
+               timeout -= I2C_SLEEP_STEP;
+       }
+
+       int ret = 0;

Move this to top of function. Declarations should all be at the start of the block.

drivers/i2c/ast_i2c.c line 131

Quoted 11 lines

drivers/i2c/ast_i2c.c new file +11 -0

+       if (status == AST_I2CD_INTR_TX_NAK)
+               ret = -EREMOTEIO;
+
+       if (timeout <= 0)
+               ret = -EI2C_TIMEOUT;
+
+       ast_i2c_clear_interrupts(i2c_base);
+       return ret;
+}
+
+static inline int ast_i2c_start_txn(struct ast_i2c_regs *i2c_base, u8 devaddr)

Why inline? The compiler will do that anyway so I think you can drop it. Also please use int or uint for devaddr - particularly on ARM it isn't good to have non-register-sized parameters and return values.

Quoted 16 lines

drivers/i2c/ast_i2c.c new file +16 -0

+{
+       /* Start and Send Device Address */
+       writel(devaddr, &i2c_base->trbbr);
+       writel(AST_I2CD_M_START_CMD | AST_I2CD_M_TX_CMD, &i2c_base->csr);
+       return ast_i2c_wait_tx(i2c_base);
+}
+
+static int ast_i2c_read_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8 *buffer,
+                            size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error =
+           ast_i2c_start_txn(i2c_base, (chip_addr << 1) | I2C_M_RD);
+       if (i2c_error < 0)
+               return i2c_error;

drivers/i2c/ast_i2c.c line 157

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+
+       u32 i2c_cmd = AST_I2CD_M_RX_CMD;
+       for (; len > 0; len--, buffer++) {
+               if (len == 1)
+                       i2c_cmd |= AST_I2CD_M_S_RX_CMD_LAST;
+               writel(i2c_cmd, &i2c_base->csr);
+               i2c_error = ast_i2c_wait_isr(i2c_base, AST_I2CD_INTR_RX_DONE);
+               if (i2c_error < 0)
+                       return i2c_error;
+               *buffer = AST_I2CD_RX_DATA_BUF_GET(readl(&i2c_base->trbbr));

Instead of AST_I2CD_RX_DATA_BUF_GET(), please define something like AST_I2CD_RX_DATA_SHIFT and _MASK and use then here.

#define AST_I2CD_RX_DATA_SHIFT 8
#define AST_I2CD_RX_DATA_MASK (0xff << AST_I2CD_RX_DATA_SHIFT)

Quoted 30 lines

drivers/i2c/ast_i2c.c new file +30 -0

+       }
+       ast_i2c_clear_interrupts(i2c_base);
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;
+}
+
+static int ast_i2c_write_data(struct ast_i2c *i2c_bus, u8 chip_addr, u8
+                             *buffer, size_t len, bool send_stop)
+{
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       int i2c_error = ast_i2c_start_txn(i2c_base, (chip_addr << 1));
+       if (i2c_error < 0)
+               return i2c_error;
+
+       for (; len > 0; len--, buffer++) {
+               writel(*buffer, &i2c_base->trbbr);
+               writel(AST_I2CD_M_TX_CMD, &i2c_base->csr);
+               i2c_error = ast_i2c_wait_tx(i2c_base);
+               if (i2c_error < 0)
+                       return i2c_error;
+       }
+
+       if (send_stop)
+               return ast_i2c_send_stop(i2c_base);
+
+       return 0;

drivers/i2c/ast_i2c.c line 197

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+}
+
+static int ast_i2c_deblock(struct udevice *dev)
+{
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+
+       u32 csr = readl(&i2c_base->csr);
+
+       int deblock_error = 0;

Just 'ret' is good enough.

Quoted 25 lines

drivers/i2c/ast_i2c.c new file +25 -0

+       bool sda_high = csr & AST_I2CD_SDA_LINE_STS;
+       bool scl_high = csr & AST_I2CD_SCL_LINE_STS;
+       if (sda_high && scl_high) {
+               /* Bus is idle, no deblocking needed. */
+               return 0;
+       } else if (sda_high) {
+               /* Send stop command */
+               debug("Unterminated TXN in (%x), sending stop\n", csr);
+               deblock_error = ast_i2c_send_stop(i2c_base);
+       } else if (scl_high) {
+               /* Possibly stuck slave */
+               debug("Bus stuck (%x), attempting recovery\n", csr);
+               writel(AST_I2CD_BUS_RECOVER_CMD, &i2c_base->csr);
+               deblock_error = ast_i2c_wait_isr(
+                       i2c_base, AST_I2CD_INTR_BUS_RECOVER_DONE);
+       } else {
+               /* Just try to reinit the device. */
+               ast_i2c_init_bus(i2c_bus);
+       }
+
+       return deblock_error;
+}
+
+static int ast_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)
+{

drivers/i2c/ast_i2c.c line 232

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+       int ret;
+
+       (void)ast_i2c_deblock(dev);
+       debug("i2c_xfer: %d messages\n", nmsgs);
+       for (; nmsgs > 0; nmsgs--, msg++) {
+               if (msg->flags & I2C_M_RD) {
+                       debug("i2c_read: chip=0x%x, len=0x%x, flags=0x%x\n",
+                             msg->addr, msg->len, msg->flags);
+                       ret =

Can you break the line in the parameters instead of at '=' ?

Quoted 13 lines

drivers/i2c/ast_i2c.c new file +13 -0

+                           ast_i2c_read_data(i2c_bus, msg->addr, msg->buf,
+                                               msg->len, (nmsgs == 1));
+               } else {
+                       debug("i2c_write: chip=0x%x, len=0x%x, flags=0x%x\n",
+                             msg->addr, msg->len, msg->flags);
+                       ret =
+                           ast_i2c_write_data(i2c_bus, msg->addr, msg->buf,
+                                                msg->len, (nmsgs == 1));
+               }
+               if (ret) {
+                       debug("%s: error (%d)\n", __func__, ret);
+                       return -EREMOTEIO;
+               }

drivers/i2c/ast_i2c.c line 255

Quoted 10 lines

drivers/i2c/ast_i2c.c new file +10 -0

+       }
+
+       return 0;
+}
+
+static int ast_i2c_set_speed(struct udevice *dev, unsigned int speed)
+{
+       debug("Setting speed ofr I2C%d to <%u>\n", dev->seq, speed);
+       if (!speed) {
+               debug("No valid speed specified.\n");

Drop '.'

Quoted 75 lines

drivers/i2c/ast_i2c.c new file +50 -0

+               return -EINVAL;
+       }
+       struct ast_i2c *i2c_bus = dev_get_priv(dev);
+
+       i2c_bus->speed = speed;
+       /* TODO: get this from device tree */
+       u32 pclk = ast_get_apbclk();
+       u32 divider = pclk / speed;
+
+       struct ast_i2c_regs *i2c_base = i2c_bus->regs;
+       if (speed > 400000) {
+               debug("Enabling High Speed\n");
+               setbits_le32(&i2c_base->fcr, AST_I2CD_M_HIGH_SPEED_EN
+                            | AST_I2CD_M_SDA_DRIVE_1T_EN
+                            | AST_I2CD_SDA_DRIVE_1T_EN);
+               writel(0x3, &i2c_base->cactcr2);
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+       } else {
+               debug("Enabling Normal Speed\n");
+               writel(get_clk_reg_val(divider), &i2c_base->cactcr1);
+               writel(AST_NO_TIMEOUT_CTRL, &i2c_base->cactcr2);
+       }
+
+       ast_i2c_clear_interrupts(i2c_base);
+       return 0;
+}
+
+static const struct dm_i2c_ops ast_i2c_ops = {
+       .xfer = ast_i2c_xfer,
+       .set_bus_speed = ast_i2c_set_speed,
+       .deblock = ast_i2c_deblock,
+};
+
+static const struct udevice_id ast_i2c_ids[] = {
+       {.compatible = "aspeed,ast2400-i2c-controller",},
+       {.compatible = "aspeed,ast2400-i2c-bus",},
+       {},
+};
+
+/* Tell GNU Indent to keep this as is: */
+/* *INDENT-OFF* */
+U_BOOT_DRIVER(i2c_aspeed) = {
+       .name = "i2c_aspeed",
+       .id = UCLASS_I2C,
+       .of_match = ast_i2c_ids,
+       .probe = ast_i2c_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_i2c),
+       .ops = &ast_i2c_ops,
+};
+/* *INDENT-ON* */

drivers/i2c/ast_i2c.h new file +19 -0

new file mode 100644
index 0000000..8926fd2
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2012-2020  ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef __AST_I2C_H_
+#define __AST_I2C_H_
+
+struct ast_i2c_regs {
+       uint32_t fcr;
+       uint32_t cactcr1;
+       uint32_t cactcr2;
+       uint32_t icr;
+       uint32_t isr;
+       uint32_t csr;
+       uint32_t sdar;
+       uint32_t pbcr;

drivers/i2c/ast_i2c.h line 29

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+       uint32_t trbbr;
+#ifdef CONFIG_TARGET_AST_G5
+       uint32_t dma_mbar;
+       uint32_t dma_tlr;
+#endif
+};
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register  */
+#define AST_I2CD_BUFF_SEL_MASK                         (0x7 << 20)

It's fine to have the AST_I2CD_ prefix if you want it. But it does make things longer, and this header will only be included in one file. So feel free to prune to AST_ if you like.

Quoted 4 lines

drivers/i2c/ast_i2c.h new file +4 -0

+#define AST_I2CD_BUFF_SEL(x)                           (x << 20)
+#define AST_I2CD_M_SDA_LOCK_EN                 (0x1 << 16)
+#define AST_I2CD_MULTI_MASTER_DIS                      (0x1 << 15)
+#define AST_I2CD_M_SCL_DRIVE_EN                (0x1 << 14)

drivers/i2c/ast_i2c.h line 43

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+#define AST_I2CD_MSB_STS                                       (0x1 << 9)
+#define AST_I2CD_SDA_DRIVE_1T_EN                       (0x1 << 8)
+#define AST_I2CD_M_SDA_DRIVE_1T_EN             (0x1 << 7)
+#define AST_I2CD_M_HIGH_SPEED_EN               (0x1 << 6)
+#define AST_I2CD_DEF_ADDR_EN                           (0x1 << 5)
+#define AST_I2CD_DEF_ALERT_EN                          (0x1 << 4)
+#define AST_I2CD_DEF_ARP_EN                                    (0x1 << 3)
+#define AST_I2CD_DEF_GCALL_EN                          (0x1 << 2)
+#define AST_I2CD_SLAVE_EN                                      (0x1 << 1)
+#define AST_I2CD_MASTER_EN                                     (0x1)

Avoid brackets around simple constants

Quoted 82 lines

drivers/i2c/ast_i2c.h new file +82 -0

+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define AST_I2CD_tBUF                                          (0x1 << 28)
+#define AST_I2CD_tHDSTA                                                (0x1 << 24)
+#define AST_I2CD_tACST                                         (0x1 << 20)
+#define AST_I2CD_tCKHIGH                                       (0x1 << 16)
+#define AST_I2CD_tCKLOW                                                (0x1 << 12)
+#define AST_I2CD_tHDDAT                                                (0x1 << 10)
+#define AST_I2CD_CLK_TO_BASE_DIV                       (0x1 << 8)
+#define AST_I2CD_CLK_BASE_DIV                          (0x1)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define AST_I2CD_tTIMEOUT                                      (0x1)
+#define AST_NO_TIMEOUT_CTRL                                    0x0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define AST_I2CD_INTR_SDA_DL_TIMEOUT                   (0x1 << 14)
+#define AST_I2CD_INTR_BUS_RECOVER_DONE                 (0x1 << 13)
+#define AST_I2CD_INTR_SMBUS_ALERT                      (0x1 << 12)
+#define AST_I2CD_INTR_SMBUS_ARP_ADDR                   (0x1 << 11)
+#define AST_I2CD_INTR_SMBUS_DEV_ALERT_ADDR             (0x1 << 10)
+#define AST_I2CD_INTR_SMBUS_DEF_ADDR                   (0x1 << 9)
+#define AST_I2CD_INTR_GCALL_ADDR                       (0x1 << 8)
+#define AST_I2CD_INTR_SLAVE_MATCH                      (0x1 << 7)
+#define AST_I2CD_INTR_SCL_TIMEOUT                      (0x1 << 6)
+#define AST_I2CD_INTR_ABNORMAL                         (0x1 << 5)
+#define AST_I2CD_INTR_NORMAL_STOP                      (0x1 << 4)
+#define AST_I2CD_INTR_ARBIT_LOSS                       (0x1 << 3)
+#define AST_I2CD_INTR_RX_DONE                          (0x1 << 2)
+#define AST_I2CD_INTR_TX_NAK                           (0x1 << 1)
+#define AST_I2CD_INTR_TX_ACK                           (0x1 << 0)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define AST_I2CD_SDA_OE                                        (0x1 << 28)
+#define AST_I2CD_SDA_O                                 (0x1 << 27)
+#define AST_I2CD_SCL_OE                                        (0x1 << 26)
+#define AST_I2CD_SCL_O                                 (0x1 << 25)
+#define AST_I2CD_TX_TIMING                             (0x1 << 24)
+#define AST_I2CD_TX_STATUS                             (0x1 << 23)
+
+/* Tx State Machine */
+#define AST_I2CD_IDLE                                  0x0
+#define AST_I2CD_MACTIVE                               0x8
+#define AST_I2CD_MSTART                                        0x9
+#define AST_I2CD_MSTARTR                               0xa
+#define AST_I2CD_MSTOP                                 0xb
+#define AST_I2CD_MTXD                                  0xc
+#define AST_I2CD_MRXACK                                        0xd
+#define AST_I2CD_MRXD                                  0xe
+#define AST_I2CD_MTXACK                                0xf
+#define AST_I2CD_SWAIT                                 0x1
+#define AST_I2CD_SRXD                                  0x4
+#define AST_I2CD_STXACK                                0x5
+#define AST_I2CD_STXD                                  0x6
+#define AST_I2CD_SRXACK                                0x7
+#define AST_I2CD_RECOVER                               0x3
+
+#define AST_I2CD_SCL_LINE_STS                          (0x1 << 18)
+#define AST_I2CD_SDA_LINE_STS                          (0x1 << 17)
+#define AST_I2CD_BUS_BUSY_STS                          (0x1 << 16)
+#define AST_I2CD_SDA_OE_OUT_DIR                                (0x1 << 15)
+#define AST_I2CD_SDA_O_OUT_DIR                         (0x1 << 14)
+#define AST_I2CD_SCL_OE_OUT_DIR                                (0x1 << 13)
+#define AST_I2CD_SCL_O_OUT_DIR                         (0x1 << 12)
+#define AST_I2CD_BUS_RECOVER_CMD                       (0x1 << 11)
+#define AST_I2CD_S_ALT_EN                              (0x1 << 10)
+#define AST_I2CD_RX_DMA_ENABLE                         (0x1 << 9)
+#define AST_I2CD_TX_DMA_ENABLE                         (0x1 << 8)
+
+/* Command Bit */
+#define AST_I2CD_RX_BUFF_ENABLE                                (0x1 << 7)
+#define AST_I2CD_TX_BUFF_ENABLE                                (0x1 << 6)
+#define AST_I2CD_M_STOP_CMD                                    (0x1 << 5)
+#define AST_I2CD_M_S_RX_CMD_LAST                       (0x1 << 4)
+#define AST_I2CD_M_RX_CMD                                      (0x1 << 3)
+#define AST_I2CD_S_TX_CMD                                      (0x1 << 2)
+#define AST_I2CD_M_TX_CMD                                      (0x1 << 1)

drivers/i2c/ast_i2c.h line 135

Quoted 10 lines

drivers/i2c/ast_i2c.h new file +10 -0

+#define AST_I2CD_M_START_CMD                           (0x1)
+
+/* 0x18 : I2CD Slave Device Address Register   */
+
+/* 0x1C : I2CD Pool Buffer Control Register   */
+#define AST_I2CD_RX_BUF_ADDR_GET(x)                    (((x) >> 24) & 0xff)
+#define AST_I2CD_RX_BUF_END_ADDR_SET(x)                        ((x) << 16)
+#define AST_I2CD_TX_DATA_BUF_END_SET(x)                        (((x) & 0xff) << 8)
+#define AST_I2CD_RX_DATA_BUF_GET(x)                    (((x) >> 8) & 0xff)
+#define AST_I2CD_BUF_BASE_ADDR_SET(x)                  ((x) & 0x3f)

Just use simple shift and mask values, and put the logic in your C code.

Quoted 11 lines

drivers/i2c/ast_i2c.h new file +8 -1

+
+/* 0x20 : I2CD Transmit/Receive Byte Buffer Register   */
+#define AST_I2CD_GET_MODE(x)                           (((x) >> 8) & 0x1)
+
+#define AST_I2CD_RX_BYTE_BUFFER                                        (0xff << 8)
+#define AST_I2CD_TX_BYTE_BUFFER                                        (0xff)
+
+#endif                         /* __AST_I2C_H_ */
--

2.8.0.rc3.226.g39d4020

Regards,
Simon

From: Maxim Sloyko <[email protected]>

This series of patches is only meant for openbmc/u-boot tree.

It adds basic support for aspeed i2c. Only single master mode is supported with synchronous transfer.

The style is inconsistent with U-Boot style guide in few places, but follows local style in those files.

Maxim Sloyko (5):
aspeed/g5: Device Tree for ast2500, copied from openbmc/linux (include
file), plus minimal device tree configuration for ast2500 eval board.
aspeed: Fixed incosistency in some SCU registers naming.
aspeed: Added function to calculate APB Clock frequency.
aspeed: Added function to configure pins for I2C devices.
aspeed: I2C driver.

  arch/arm/dts/Makefile                       |    2 +
  arch/arm/dts/aspeed-g5-evb.dts              |   28 +
  arch/arm/dts/aspeed-g5.dtsi                 | 1278 +++++++++++++++++++++++++++
  arch/arm/include/asm/arch-aspeed/ast_scu.h  |    6 +
  arch/arm/include/asm/arch-aspeed/regs-scu.h |   73 +-
  arch/arm/mach-aspeed/ast-scu.c              |   41 +-
  drivers/i2c/Kconfig                         |    7 +
  drivers/i2c/Makefile                        |    1 +
  drivers/i2c/ast_i2c.c                       |  305 +++++++
  drivers/i2c/ast_i2c.h                       |  143 +++
  10 files changed, 1851 insertions(+), 33 deletions(-)

create mode 100644 arch/arm/dts/aspeed-g5-evb.dts create mode 100644 arch/arm/dts/aspeed-g5.dtsi create mode 100644 drivers/i2c/ast_i2c.c create mode 100644 drivers/i2c/ast_i2c.h

--
2.8.0.rc3.226.g39d4020

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hello Maxim,

Am 23.11.2016 um 00:56 schrieb maxims at google.com:

Quoted 4 lines

From: Maxim Sloyko <[email protected]>

This series of patches is only meant for openbmc/u-boot tree.

If this series is not for U-Boot mainline, why do you post this patches on the U-Boot Mailinglist?

bye,
Heiko

Quoted 32 lines

It adds basic support for aspeed i2c. Only single master mode is supported with synchronous transfer.

The style is inconsistent with U-Boot style guide in few places, but follows local style in those files.

Maxim Sloyko (5):
aspeed/g5: Device Tree for ast2500, copied from openbmc/linux (include
file), plus minimal device tree configuration for ast2500 eval board.
aspeed: Fixed incosistency in some SCU registers naming.
aspeed: Added function to calculate APB Clock frequency.
aspeed: Added function to configure pins for I2C devices.
aspeed: I2C driver.

  arch/arm/dts/Makefile                       |    2 +
  arch/arm/dts/aspeed-g5-evb.dts              |   28 +
  arch/arm/dts/aspeed-g5.dtsi                 | 1278
+++++++++++++++++++++++++++
  arch/arm/include/asm/arch-aspeed/ast_scu.h  |    6 +
  arch/arm/include/asm/arch-aspeed/regs-scu.h |   73 +-
  arch/arm/mach-aspeed/ast-scu.c              |   41 +-
  drivers/i2c/Kconfig                         |    7 +
  drivers/i2c/Makefile                        |    1 +
  drivers/i2c/ast_i2c.c                       |  305 +++++++
  drivers/i2c/ast_i2c.h                       |  143 +++
  10 files changed, 1851 insertions(+), 33 deletions(-)

create mode 100644 arch/arm/dts/aspeed-g5-evb.dts create mode 100644 arch/arm/dts/aspeed-g5.dtsi create mode 100644 drivers/i2c/ast_i2c.c create mode 100644 drivers/i2c/ast_i2c.h

Quoted 9 lines

--
2.8.0.rc3.226.g39d4020

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Sorry, this was by mistake, please ignore.

Hello Maxim,

Am 23.11.2016 um 00:56 schrieb maxims at google.com <mailto:[email protected]>:

From: Maxim Sloyko <maxims at google.com <mailto:[email protected]>>

This series of patches is only meant for openbmc/u-boot tree.

If this series is not for U-Boot mainline, why do you post this patches on the U-Boot Mailinglist?

bye,
Heiko

It adds basic support for aspeed i2c. Only single master mode is supported with synchronous transfer.

The style is inconsistent with U-Boot style guide in few places, but follows local style in those files.

Maxim Sloyko (5):
aspeed/g5: Device Tree for ast2500, copied from openbmc/linux (include
file), plus minimal device tree configuration for ast2500 eval board.
aspeed: Fixed incosistency in some SCU registers naming.
aspeed: Added function to calculate APB Clock frequency.
aspeed: Added function to configure pins for I2C devices.
aspeed: I2C driver.

arch/arm/dts/Makefile | 2 +
arch/arm/dts/aspeed-g5-evb.dts | 28 +
arch/arm/dts/aspeed-g5.dtsi | 1278 +++++++++++++++++++++++++++
arch/arm/include/asm/arch-aspeed/ast_scu.h | 6 +
arch/arm/include/asm/arch-aspeed/regs-scu.h | 73 +-
arch/arm/mach-aspeed/ast-scu.c | 41 +-
drivers/i2c/Kconfig | 7 +
drivers/i2c/Makefile | 1 +
drivers/i2c/ast_i2c.c | 305 +++++++
drivers/i2c/ast_i2c.h | 143 +++
10 files changed, 1851 insertions(+), 33 deletions(-) create mode 100644 arch/arm/dts/aspeed-g5-evb.dts create mode 100644 arch/arm/dts/aspeed-g5.dtsi create mode 100644 drivers/i2c/ast_i2c.c create mode 100644 drivers/i2c/ast_i2c.h

--
2.8.0.rc3.226.g39d4020

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de <mailto:[email protected]>
http://lists.denx.de/mailman/listinfo/u-boot <http://lists.denx.de/mailman/listinfo/u-boot>

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

--
*M*axim *S*loyko

Change FUC to FUN in SCU registers naming, which was a typo that go out of hand

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/regs-scu.h | 64 ++++++++++++++---------------
 arch/arm/mach-aspeed/ast-scu.c              |  2 +-
 2 files changed, 33 insertions(+), 33 deletions(-)

Add missing definitions for configuring I2C pins

Signed-off-by Maxim Sloyko <[email protected]>

---
 arch/arm/include/asm/arch-aspeed/regs-scu.h | 9 +++++++++
 1 file changed, 9 insertions(+)

arch/arm/include/asm/arch-aspeed/regs-scu.h +0 -0

index 5445023..6cb4d0d 100644
@@ -844,11 +844,15 @@
 #define SCU_FUN_PIN_I2C5               (0x1 << 18)

arch/arm/include/asm/arch-aspeed/regs-scu.h +6 -1

+
 #define SCU_FUN_PIN_MII2_TX_DRIV(x)    (x << 10)
 #define SCU_FUN_PIN_MII2_TX_DRIV_MASK  (0x3 << 10)
 #define SCU_FUN_PIN_MII1_TX_DRIV(x)    (x << 8)

@@ -914,6 +918,11 @@
 #define SCU_FUN_PIN_ROMA4              (0x1 << 18)
 #define SCU_FUN_PIN_ROMA3              (0x1 << 17)
 #define SCU_FUN_PIN_ROMA2              (0x1 << 16)
+/* AST2500 only */
+#define SCU_FUN_PIN_SDA2               (0x1 << 15)
+#define SCU_FUN_PIN_SCL2               (0x1 << 14)
+#define SCU_FUN_PIN_SDA1               (0x1 << 13)
+#define SCU_FUN_PIN_SCL1               (0x1 << 12)
 
 /* AST_SCU_FUN_PIN_CTRL9               0xA8 - Multi-function Pin Control#9 */
 #define SCU_FUN_PIN_ROMA21             (0x1 << 3)
--

2.8.0.rc3.226.g39d4020