Re: [U-Boot] [PATCH v1 05/11] board: gdsys: IHS I2C master driver

Hello Dirk,

Am 30.04.2014 15:49, schrieb dirk.eibach@gdsys.cc:

From: Dirk Eibachdirk.eibach@gdsys.cc

IHS I2C master support was merely a hack in the osd driver. Now it is a proper u-boot I2C framework driver, supporting the v2.00 master features.

Signed-off-by: Dirk Eibachdirk.eibach@gdsys.cc

board/gdsys/common/Makefile | 4 +- board/gdsys/common/ihs_i2c.c | 196 +++++++++++++++++++++++++++++++++++++++++

Hmm... why you do not add this i2c driver to drivers/i2c ?

board/gdsys/common/osd.c | 1 - include/configs/dlvision-10g.h | 20 ++++- include/configs/iocon.h | 22 +++-- include/gdsys_fpga.h | 25 +++--- 6 files changed, 245 insertions(+), 23 deletions(-) create mode 100644 board/gdsys/common/ihs_i2c.c

diff --git a/board/gdsys/common/Makefile b/board/gdsys/common/Makefile index 7f8b427..a5ee016 100644 --- a/board/gdsys/common/Makefile +++ b/board/gdsys/common/Makefile @@ -8,6 +8,6 @@ obj-$(CONFIG_SYS_FPGA_COMMON) += fpga.o obj-$(CONFIG_IO) += miiphybb.o obj-$(CONFIG_IO64) += miiphybb.o -obj-$(CONFIG_IOCON) += osd.o mclink.o dp501.o -obj-$(CONFIG_DLVISION_10G) += osd.o +obj-$(CONFIG_IOCON) += osd.o mclink.o dp501.o ihs_i2c.o +obj-$(CONFIG_DLVISION_10G) += osd.o ihs_i2c.o

and add a proper CONFIG define for this i2c driver.

obj-$(CONFIG_CONTROLCENTERD) += dp501.o diff --git a/board/gdsys/common/ihs_i2c.c b/board/gdsys/common/ihs_i2c.c new file mode 100644 index 0000000..ba2f5dc --- /dev/null +++ b/board/gdsys/common/ihs_i2c.c @@ -0,0 +1,196 @@ +/*

• • (C) Copyright 2013
• • Dirk Eibach, Guntermann& Drunck GmbH, eibach@gdsys.de
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include<common.h> +#include<i2c.h> +#include<gdsys_fpga.h>

+DECLARE_GLOBAL_DATA_PTR;

+enum {

• I2CINT_ERROR_EV = 1<< 13,
• I2CINT_TRANSMIT_EV = 1<< 14,
• I2CINT_RECEIVE_EV = 1<< 15,

+};

+static int wait_for_int(bool read) +{

• u16 val;
• unsigned int ctr = 0;
• FPGA_GET_REG(I2C_ADAP_HWNR, i2c.interrupt_status,&val);
• while (!(val& (I2CINT_ERROR_EV

•       | (read ? I2CINT_RECEIVE_EV : I2CINT_TRANSMIT_EV)))) {
  

• udelay(10);
  

• if (ctr++>  5000) {
  

• 	printf("I2C timeout\n");
  

• 	return 1;
  

• }
  

• FPGA_GET_REG(I2C_ADAP_HWNR, i2c.interrupt_status,&val);
  

• }
• return (val& I2CINT_ERROR_EV) ? 1 : 0;

+}

+static int ihs_i2c_transfer(uchar chip, uchar *buffer, int len, bool read,

• 	    bool is_last)
  

+{

• u16 val;
• FPGA_SET_REG(I2C_ADAP_HWNR, i2c.interrupt_status, I2CINT_ERROR_EV

•      | I2CINT_RECEIVE_EV | I2CINT_TRANSMIT_EV);
  

• FPGA_GET_REG(I2C_ADAP_HWNR, i2c.interrupt_status,&val);
• if (!read&& len) {

Nitpick: please:

if (!read && len) {

• val = buffer[0];
  

• if (len>  1)
  

• 	val |= buffer[1]<<  8;
  

here too:

val |= buffer[1] << 8;

Please check globally.

• FPGA_SET_REG(I2C_ADAP_HWNR, i2c.write_mailbox_ext, val);
  

• }
• FPGA_SET_REG(I2C_ADAP_HWNR, i2c.write_mailbox,

•      (read ? 0x8000 : 0x8400)
  

Could you use defines for this magic constants?

•      | (chip<<  1)
  

•      | ((len>  1) ? (1<<  11) : 0)
  

•      | (is_last ? 0 : (1<<  13)));
  

and here too ...

• if (wait_for_int(read))

• return 1;
  

• if (read) {

• FPGA_GET_REG(I2C_ADAP_HWNR, i2c.read_mailbox_ext,&val);
  

• buffer[0] = val&  0xff;
  

• if (len>  1)
  

• 	buffer[1] = val>>  8;
  

• }
• return 0;

+}

+static int ihs_i2c_address(uchar chip, uint addr, int alen, bool hold_bus) +{

• int shift = (alen-1) * 8;
• while (alen) {

• int transfer = MIN(alen, 2);
  

• uchar buf[2];
  

• bool is_last = alen<= transfer;
  

• buf[0] = addr>>  shift;
  

• if (alen>  1)
  

• 	buf[1] = addr>>  (shift - 8);
  

• if (ihs_i2c_transfer(chip, buf, transfer, false,
  

• 		     hold_bus ? false : is_last))
  

• 	return 1;
  

• shift -= 16;
  

• alen -= transfer;
  

• }
• return 0;

+}

+static int ihs_i2c_access(struct i2c_adapter *adap, uchar chip, uint addr,

• 	  int alen, uchar *buffer, int len, bool read)
  

+{

• if (len<= 0)

• return 1;
  

• if (ihs_i2c_address(chip, addr, alen, !read))

• return 1;
  

• while (len) {

• int transfer = MIN(len, 2);
  

• if (ihs_i2c_transfer(chip, buffer, transfer, read,
  

• 		     len<= transfer))
  

• 	return 1;
  

• buffer += transfer;
  

• addr += transfer;
  

• len -= transfer;
  

• }
• return 0;

+}

+static void ihs_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) +{ +#ifdef CONFIG_SYS_I2C_INIT_BOARD

• /*

• * Call board specific i2c bus reset routine before accessing the
  

• * environment, which might be in a chip on that bus. For details
  

• * about this problem see doc/I2C_Edge_Conditions.
  

• */
  

• i2c_init_board();

+#endif +}

+static int ihs_i2c_probe(struct i2c_adapter *adap, uchar chip) +{

• uchar buffer[2];
• if (ihs_i2c_transfer(chip, buffer, 0, true, true))

• return 1;
  

• return 0;

+}

+static int ihs_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,

• 	int alen, uchar *buffer, int len)
  

+{

• return ihs_i2c_access(adap, chip, addr, alen, buffer, len, true);

+}

+static int ihs_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,

• 	 int alen, uchar *buffer, int len)
  

+{

• return ihs_i2c_access(adap, chip, addr, alen, buffer, len, false);

+}

+static unsigned int ihs_i2c_set_bus_speed(struct i2c_adapter *adap,

• 			     unsigned int speed)
  

+{

• if (speed != adap->speed)

• return 1;
  

• return speed;

+}

+/*

• • Register IHS i2c adapters
• */

+#ifdef CONFIG_SYS_I2C_IHS_CH0 +U_BOOT_I2C_ADAP_COMPLETE(ihs0, ihs_i2c_init, ihs_i2c_probe,

• 	 ihs_i2c_read, ihs_i2c_write,
  

• 	 ihs_i2c_set_bus_speed,
  

• 	 CONFIG_SYS_I2C_IHS_SPEED_0,
  

• 	 CONFIG_SYS_I2C_IHS_SLAVE_0, 0)
  

+#endif +#ifdef CONFIG_SYS_I2C_IHS_CH1 +U_BOOT_I2C_ADAP_COMPLETE(ihs1, ihs_i2c_init, ihs_i2c_probe,

• 	 ihs_i2c_read, ihs_i2c_write,
  

• 	 ihs_i2c_set_bus_speed,
  

• 	 CONFIG_SYS_I2C_IHS_SPEED_1,
  

• 	 CONFIG_SYS_I2C_IHS_SLAVE_1, 1)
  

+#endif +#ifdef CONFIG_SYS_I2C_IHS_CH2 +U_BOOT_I2C_ADAP_COMPLETE(ihs2, ihs_i2c_init, ihs_i2c_probe,

• 	 ihs_i2c_read, ihs_i2c_write,
  

• 	 ihs_i2c_set_bus_speed,
  

• 	 CONFIG_SYS_I2C_IHS_SPEED_2,
  

• 	 CONFIG_SYS_I2C_IHS_SLAVE_2, 2)
  

+#endif +#ifdef CONFIG_SYS_I2C_IHS_CH3 +U_BOOT_I2C_ADAP_COMPLETE(ihs3, ihs_i2c_init, ihs_i2c_probe,

• 	 ihs_i2c_read, ihs_i2c_write,
  

• 	 ihs_i2c_set_bus_speed,
  

• 	 CONFIG_SYS_I2C_IHS_SPEED_3,
  

• 	 CONFIG_SYS_I2C_IHS_SLAVE_3, 3)
  

+#endif diff --git a/board/gdsys/common/osd.c b/board/gdsys/common/osd.c index 0290949..a839a4e 100644 --- a/board/gdsys/common/osd.c +++ b/board/gdsys/common/osd.c @@ -342,7 +342,6 @@ int osd_probe(unsigned screen) i2c_reg_write(SIL1178_MASTER_I2C_ADDRESS, 0x08, 0x37); #endif

• FPGA_SET_REG(screen, videocontrol, 0x0002);

What has this change to do with the i2c driver? Please move this to a seperate patch, thanks!

FPGA_SET_REG(screen, osd.control, 0x0049);

FPGA_SET_REG(screen, osd.xy_size, ((32 - 1)<< 8) | (16 - 1));

Hmm... a lot of magic constants ...

diff --git a/include/configs/dlvision-10g.h b/include/configs/dlvision-10g.h index 7877897..d886740 100644 --- a/include/configs/dlvision-10g.h +++ b/include/configs/dlvision-10g.h @@ -97,9 +97,22 @@ /*

• I2C stuff

*/ +#define CONFIG_SYS_I2C_PPC4XX +#define CONFIG_SYS_I2C_PPC4XX_CH0 #define CONFIG_SYS_I2C_PPC4XX_SPEED_0 100000 +#define CONFIG_SYS_I2C_PPC4XX_SLAVE_0 0x7F

Here again, this change has nothing todo with adding this new i2c driver ...

Please split this in another patch, thanks.

+#define CONFIG_SYS_I2C_IHS_CH0 +#define CONFIG_SYS_I2C_IHS_SPEED_0 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_0 0x7F +#define CONFIG_SYS_I2C_IHS_CH1 +#define CONFIG_SYS_I2C_IHS_SPEED_1 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_1 0x7F

+#define CONFIG_SYS_SPD_BUS_NUM 2

/* Temp sensor/hwmon/dtt */ +#define CONFIG_SYS_DTT_BUS_NUM 2 #define CONFIG_DTT_LM63 1 /* National LM63 */ #define CONFIG_DTT_SENSORS { 0x4c, 0x4e, 0x18 } /* Sensor addresses */ #define CONFIG_DTT_PWM_LOOKUPTABLE \ @@ -107,6 +120,11 @@ { 54, 27 }, { 56, 31 }, { 58, 36 }, { 60, 40 } } #define CONFIG_DTT_TACH_LIMIT 0xa10

+#define CONFIG_SYS_ICS8N3QV01 +#define CONFIG_SYS_ICS8N3QV01_I2C {0, 1} +#define CONFIG_SYS_SIL1178 +#define CONFIG_SYS_SIL1178_I2C {0, 1}

and this ...

• /* EBC peripherals */

  #define CONFIG_SYS_FLASH_BASE 0xFC000000

@@ -306,9 +324,7 @@ /*

• OSD Setup

*/ -#define CONFIG_SYS_ICS8N3QV01 #define CONFIG_SYS_MPC92469AC -#define CONFIG_SYS_SIL1178 #define CONFIG_SYS_OSD_SCREENS CONFIG_SYS_FPGA_COUNT

#endif /* __CONFIG_H */ diff --git a/include/configs/iocon.h b/include/configs/iocon.h index 5636f38..7fa41ee 100644 --- a/include/configs/iocon.h +++ b/include/configs/iocon.h @@ -101,10 +101,24 @@ #define CONFIG_SYS_I2C_PPC4XX_SLAVE_0 0x7F

#define CONFIG_SYS_I2C_SPEED 400000 +#define CONFIG_SYS_SPD_BUS_NUM 4

#define CONFIG_PCA953X /* NXP PCA9554 */ #define CONFIG_PCA9698 /* NXP PCA9698 */

+#define CONFIG_SYS_I2C_IHS_CH0 +#define CONFIG_SYS_I2C_IHS_SPEED_0 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_0 0x7F +#define CONFIG_SYS_I2C_IHS_CH1 +#define CONFIG_SYS_I2C_IHS_SPEED_1 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_1 0x7F +#define CONFIG_SYS_I2C_IHS_CH2 +#define CONFIG_SYS_I2C_IHS_SPEED_2 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_2 0x7F +#define CONFIG_SYS_I2C_IHS_CH3 +#define CONFIG_SYS_I2C_IHS_SPEED_3 50000 +#define CONFIG_SYS_I2C_IHS_SLAVE_3 0x7F

• /*
  • Software (bit-bang) I2C driver configuration
  */

@@ -121,9 +135,9 @@ #define CONFIG_SYS_I2C_SOFT_SPEED_4 50000 #define CONFIG_SYS_I2C_SOFT_SLAVE_4 0x7F

-#define CONFIG_SYS_ICS8N3QV01_I2C {1, 2, 3, 4} -#define CONFIG_SYS_CH7301_I2C {1, 2, 3, 4} -#define CONFIG_SYS_DP501_I2C {1, 2, 3, 4} +#define CONFIG_SYS_ICS8N3QV01_I2C {5, 6, 7, 8} +#define CONFIG_SYS_CH7301_I2C {5, 6, 7, 8} +#define CONFIG_SYS_DP501_I2C {0, 1, 2, 3}

#ifndef __ASSEMBLY__ void fpga_gpio_set(unsigned int bus, int pin); @@ -148,8 +162,6 @@ int fpga_gpio_get(unsigned int bus, int pin); fpga_gpio_set(I2C_ADAP_HWNR, 0x0020); \ else \ fpga_gpio_clear(I2C_ADAP_HWNR, 0x0020); \

• while (!!fpga_gpio_get(I2C_ADAP_HWNR, 0x0020) != !!bit) \
  

• 	; \
  

  } while (0) #define I2C_DELAY udelay(25) /* 1/4 I2C clock duration */

diff --git a/include/gdsys_fpga.h b/include/gdsys_fpga.h index 85ddbcb..276a01e 100644 --- a/include/gdsys_fpga.h +++ b/include/gdsys_fpga.h @@ -43,10 +43,12 @@ struct ihs_gpio { };

struct ihs_i2c {

• u16 write_mailbox;

• u16 interrupt_status;
• u16 interrupt_enable; u16 write_mailbox_ext;

• u16 read_mailbox;

• u16 write_mailbox; u16 read_mailbox_ext;

• u16 read_mailbox; };

  struct ihs_osd {

@@ -84,7 +86,6 @@ struct ihs_fpga { #endif

#ifdef CONFIG_IO64

• struct ihs_fpga_channel { u16 status_int; u16 config_int;

@@ -121,9 +122,9 @@ struct ihs_fpga { u16 reserved_0[6]; /* 0x0008 */ struct ihs_gpio gpio; /* 0x0014 */ u16 mpc3w_control; /* 0x001a */

• u16 reserved_1[19]; /* 0x001c */
• u16 videocontrol; /* 0x0042 */
• u16 reserved_2[14]; /* 0x0044 */

• u16 reserved_1[18]; /* 0x001c */
• struct ihs_i2c i2c; /* 0x0040 */
• u16 reserved_2[10]; /* 0x004c */ u16 mc_int; /* 0x0060 */ u16 mc_int_en; /* 0x0062 */ u16 mc_status; /* 0x0064 */

@@ -150,15 +151,13 @@ struct ihs_fpga { u16 fpga_features; /* 0x0006 */ u16 reserved_0[10]; /* 0x0008 */ u16 extended_interrupt; /* 0x001c */

• u16 reserved_1[9]; /* 0x001e */
• struct ihs_i2c i2c; /* 0x0030 */
• u16 reserved_2[16]; /* 0x0038 */

• u16 reserved_1[29]; /* 0x001e */ u16 mpc3w_control; /* 0x0058 */

• u16 reserved_3[34]; /* 0x005a */
• u16 videocontrol; /* 0x009e */
• u16 reserved_4[176]; /* 0x00a0 */

• u16 reserved_2[3]; /* 0x005a */
• struct ihs_i2c i2c; /* 0x0060 */
• u16 reserved_3[205]; /* 0x0066 */ struct ihs_osd osd; /* 0x0200 */

• u16 reserved_5[761]; /* 0x020e */

• u16 reserved_4[761]; /* 0x020e */ u16 videomem[31736]; /* 0x0800 */ }; #endif

bye, Heiko