答复: [PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-22 Thread 赵 思晨 
Hi Gedare Bloom, Hi Ben:


Latest news. I just modify the PV's i2c code these days, and now it can read 
the EEPROM preliminarily by the interrupt not the polling , it is the different 
way from my previously submitted work(the diff.patch). Without the read eeprom 
function and other not standardized funciton i created before.

And i will test and modify the code in the future to make sure it can works 
more normative.


Best Regards

Sichen Zhao




发自 Outlook

发件人: devel  代表 Gedare Bloom 
发送时间: 2017年3月22日 1:36:13
收件人: Ben Gras
抄送: devel@rtems.org
主题: Re: [PATCH] modify punitvara's works on BBB i2c, and now can read the 
eeprom info.

On Tue, Mar 21, 2017 at 12:12 AM, Ben Gras  wrote:
> Dear all,
>
> SZ, great initiative. Gedare and Sebastian have commented.
>
> For my part, can I just check - the implementation is supposed to be
> generic to i2c devices, correct?
>
> If so, can you explain what read_eeprom is for? Is it just as demo? Is
> the code generic otherwise?
>
I believe PV used EEPROM to test his i2c implementation.

> Cheers,
>
> Ben
>
>
> On 03/14/2017 10:05 AM, Sichen Zhao wrote:
>> ---
>>  c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 
>> --
>>  c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
>>  cpukit/dev/i2c/eeprom.c   |  24 +-
>>  testsuites/samples/i2c0/init.c|  98 -
>>  4 files changed, 777 insertions(+), 47 deletions(-)
>>
>> diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
>> b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> index 6b790e5..6a22125 100644
>> --- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> +++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> @@ -21,11 +21,23 @@
>>  #include 
>>  #include 
>>
>> +#define u16 unsigned int
>> +
>> +static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
>> +static void omap24_i2c_init(i2c_bus *base);
>> +static void flush_fifo(i2c_bus *base);
>> +static int wait_for_bb(i2c_bus *base);
>> +static int omap24_i2c_probe(i2c_bus *base);
>> +static u16 wait_for_event(i2c_bus *base);
>> +static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, 
>> int alen, unsigned char *buffer,
>> +   int len);
>> +static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
>> +static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint 
>> addr,int alen, unsigned char *buffer,
>> +int len);
>>  /*
>>  static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>>  {
>>bool status =true;
>> -
>>  // We will check i2c_bus_id in am335x_i2c_bus_register
>>  // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE 
>> ??
>>if (bus->i2c_bus_id == I2C1) {
>> @@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>>  /* ref. Table 21-4 I2C Clock Signals */
>>  /*
>>   For I2C1/2
>> -
>>   Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
>> -
>>   Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
>>  */
>>
>> @@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. 
>> As long as in
>>  this configuration, power domain sleep transition cannot happen.*/
>>   /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
>>  AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
>>AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
>>  */
>> @@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
>> happen.*/
>>if (bus->i2c_bus_id == I2C1) {
>>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
>>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
>>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
>>} else if (bus->i2c_bus_id == I2C2) {
>>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
>>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
>>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
>> -
>>while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
>> (AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
>>  AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
>> -
>>}
>>  }
>>  */
>>
>>  static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
>>  {
>> -  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
>> +  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
>> AM335X_CONF_I2C0_SDA);
>> +  printf("bus->regs:%x\n", bus->regs);
>> +
>> +  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
>>

Re: [PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-21 Thread Gedare Bloom 
On Tue, Mar 21, 2017 at 12:12 AM, Ben Gras  wrote:
> Dear all,
>
> SZ, great initiative. Gedare and Sebastian have commented.
>
> For my part, can I just check - the implementation is supposed to be
> generic to i2c devices, correct?
>
> If so, can you explain what read_eeprom is for? Is it just as demo? Is
> the code generic otherwise?
>
I believe PV used EEPROM to test his i2c implementation.

> Cheers,
>
> Ben
>
>
> On 03/14/2017 10:05 AM, Sichen Zhao wrote:
>> ---
>>  c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 
>> --
>>  c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
>>  cpukit/dev/i2c/eeprom.c   |  24 +-
>>  testsuites/samples/i2c0/init.c|  98 -
>>  4 files changed, 777 insertions(+), 47 deletions(-)
>>
>> diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
>> b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> index 6b790e5..6a22125 100644
>> --- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> +++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
>> @@ -21,11 +21,23 @@
>>  #include 
>>  #include 
>>
>> +#define u16 unsigned int
>> +
>> +static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
>> +static void omap24_i2c_init(i2c_bus *base);
>> +static void flush_fifo(i2c_bus *base);
>> +static int wait_for_bb(i2c_bus *base);
>> +static int omap24_i2c_probe(i2c_bus *base);
>> +static u16 wait_for_event(i2c_bus *base);
>> +static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, 
>> int alen, unsigned char *buffer,
>> +   int len);
>> +static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
>> +static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint 
>> addr,int alen, unsigned char *buffer,
>> +int len);
>>  /*
>>  static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>>  {
>>bool status =true;
>> -
>>  // We will check i2c_bus_id in am335x_i2c_bus_register
>>  // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE 
>> ??
>>if (bus->i2c_bus_id == I2C1) {
>> @@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>>  /* ref. Table 21-4 I2C Clock Signals */
>>  /*
>>   For I2C1/2
>> -
>>   Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
>> -
>>   Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
>>  */
>>
>> @@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. 
>> As long as in
>>  this configuration, power domain sleep transition cannot happen.*/
>>   /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
>>  AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
>>AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
>>  */
>> @@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
>> happen.*/
>>if (bus->i2c_bus_id == I2C1) {
>>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
>>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
>>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
>>} else if (bus->i2c_bus_id == I2C2) {
>>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
>>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
>> -
>>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
>>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
>> AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
>> -
>>while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
>> (AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
>>  AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
>> -
>>}
>>  }
>>  */
>>
>>  static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
>>  {
>> -  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
>> +  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
>> AM335X_CONF_I2C0_SDA);
>> +  printf("bus->regs:%x\n", bus->regs);
>> +
>> +  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
>>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>>
>> -  REG(bus->regs + AM335X_CONF_I2C0_SCL) =
>> +  REG(0x44e1 + AM335X_CONF_I2C0_SCL) =
>>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>>  }
>>
>> @@ -314,14 +322,29 @@ void am335x_i2c_init(bbb_i2c_bus *bus, uint32_t 
>> input_clock)
>>  static bool am335x_i2c_busbusy(volatile bbb_i2c_regs *regs)
>>  {
>>bool status;
>> -
>> -  if (REG(>BBB_I2C_IRQSTATUS_RAW) & AM335X_I2C_IRQSTATUS_RAW_BB)
>> +  unsigned short stat;
>> +  int timeout = I2C_TIMEOUT;
>> +
>> +  REG(>BBB_I2C_IRQSTATUS)=0x;
>> +  
>> printf("REG(>BBB_I2C_IRQSTATUS_RAW):%x\n",REG(>BBB_I2C_IRQSTATUS_RAW)
>>  );
>> + // printf("%x\n",0x1400 & 0x1000 );
>> + printf("REG(>BBB_I2C_IRQSTATUS_RAW) & 
>>

答复: [PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-20 Thread 赵 思晨 
Hi all:
I think it can use in i2c devices, but i don't test it.
and the read_eeprom is using reading the eeprom. and i will modify it in the 
future, conform my code to the RTEMS code specification.


Best regrads

Sichen Zhao


发自 Outlook

发件人: devel  代表 Ben Gras 
发送时间: 2017年3月21日 12:12:42
收件人: devel@rtems.org; 1473996...@qq.com
主题: Re: [PATCH] modify punitvara's works on BBB i2c, and now can read the 
eeprom info.

Dear all,

SZ, great initiative. Gedare and Sebastian have commented.

For my part, can I just check - the implementation is supposed to be
generic to i2c devices, correct?

If so, can you explain what read_eeprom is for? Is it just as demo? Is
the code generic otherwise?

Cheers,

Ben


On 03/14/2017 10:05 AM, Sichen Zhao wrote:
> ---
>  c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 
> --
>  c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
>  cpukit/dev/i2c/eeprom.c   |  24 +-
>  testsuites/samples/i2c0/init.c|  98 -
>  4 files changed, 777 insertions(+), 47 deletions(-)
>
> diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
> b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> index 6b790e5..6a22125 100644
> --- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> +++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> @@ -21,11 +21,23 @@
>  #include 
>  #include 
>
> +#define u16 unsigned int
> +
> +static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
> +static void omap24_i2c_init(i2c_bus *base);
> +static void flush_fifo(i2c_bus *base);
> +static int wait_for_bb(i2c_bus *base);
> +static int omap24_i2c_probe(i2c_bus *base);
> +static u16 wait_for_event(i2c_bus *base);
> +static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, int 
> alen, unsigned char *buffer,
> +   int len);
> +static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
> +static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint addr,int 
> alen, unsigned char *buffer,
> +int len);
>  /*
>  static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  {
>bool status =true;
> -
>  // We will check i2c_bus_id in am335x_i2c_bus_register
>  // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE 
> ??
>if (bus->i2c_bus_id == I2C1) {
> @@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  /* ref. Table 21-4 I2C Clock Signals */
>  /*
>   For I2C1/2
> -
>   Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
> -
>   Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
>  */
>
> @@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. As 
> long as in
>  this configuration, power domain sleep transition cannot happen.*/
>   /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
>  AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
> -
>while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
>AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
>  */
> @@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
> happen.*/
>if (bus->i2c_bus_id == I2C1) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
>} else if (bus->i2c_bus_id == I2C2) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
> -
>while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
> (AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
>  AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
> -
>}
>  }
>  */
>
>  static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
>  {
> -  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
> +  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
> AM335X_CONF_I2C0_SDA);
> +  printf("bus->regs:%x\n", bus->regs);
> +
> +  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>
> -  REG(bus->regs + AM335X_CONF_I2C0_SCL) =
> +  REG(0x44e1 + AM335X_CONF_I2C0_SCL) =
>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>  }
>
> @@ -314,14 +322,29 @@ void am335x_i2c_init(bbb_i2c_bus *bus, uint32_t 
> input_clock)
>  static bool am335x_i2c_busbusy(volatile bbb_i2c_regs *regs)
>  {
>bool status;
> -
> -  if (REG(>BBB_I2C_IRQSTATUS_RAW) & AM335X_I2C_IRQSTATUS_RAW_BB)
> +  unsigned short stat;
> +  int timeout

Re: [PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-20 Thread Ben Gras 
Dear all,

SZ, great initiative. Gedare and Sebastian have commented.

For my part, can I just check - the implementation is supposed to be
generic to i2c devices, correct?

If so, can you explain what read_eeprom is for? Is it just as demo? Is
the code generic otherwise?

Cheers,

Ben


On 03/14/2017 10:05 AM, Sichen Zhao wrote:
> ---
>  c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 
> --
>  c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
>  cpukit/dev/i2c/eeprom.c   |  24 +-
>  testsuites/samples/i2c0/init.c|  98 -
>  4 files changed, 777 insertions(+), 47 deletions(-)
>
> diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
> b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> index 6b790e5..6a22125 100644
> --- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> +++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> @@ -21,11 +21,23 @@
>  #include 
>  #include 
>  
> +#define u16 unsigned int
> +
> +static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
> +static void omap24_i2c_init(i2c_bus *base);
> +static void flush_fifo(i2c_bus *base);
> +static int wait_for_bb(i2c_bus *base);
> +static int omap24_i2c_probe(i2c_bus *base);
> +static u16 wait_for_event(i2c_bus *base);
> +static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, int 
> alen, unsigned char *buffer, 
> +   int len);
> +static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
> +static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint addr,int 
> alen, unsigned char *buffer, 
> +int len);
>  /*
>  static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  {
>bool status =true;
> -
>  // We will check i2c_bus_id in am335x_i2c_bus_register
>  // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE 
> ??
>if (bus->i2c_bus_id == I2C1) {
> @@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  /* ref. Table 21-4 I2C Clock Signals */
>  /* 
>   For I2C1/2
> -
>   Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
> -
>   Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
>  */
>  
> @@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. As 
> long as in
>  this configuration, power domain sleep transition cannot happen.*/
>   /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
>  AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
> -
>while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
>AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
>  */
> @@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
> happen.*/
>if (bus->i2c_bus_id == I2C1) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
>} else if (bus->i2c_bus_id == I2C2) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
> -
>while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
> (AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
>  AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
> -
>}
>  }
>  */
>  
>  static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
>  {
> -  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
> +  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
> AM335X_CONF_I2C0_SDA);
> +  printf("bus->regs:%x\n", bus->regs);
> + 
> +  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>  
> -  REG(bus->regs + AM335X_CONF_I2C0_SCL) =
> +  REG(0x44e1 + AM335X_CONF_I2C0_SCL) =
>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN); 
>  }
>  
> @@ -314,14 +322,29 @@ void am335x_i2c_init(bbb_i2c_bus *bus, uint32_t 
> input_clock)
>  static bool am335x_i2c_busbusy(volatile bbb_i2c_regs *regs)
>  {
>bool status;
> -
> -  if (REG(>BBB_I2C_IRQSTATUS_RAW) & AM335X_I2C_IRQSTATUS_RAW_BB)
> +  unsigned short stat;
> +  int timeout = I2C_TIMEOUT;
> +
> +  REG(>BBB_I2C_IRQSTATUS)=0x;
> +  
> printf("REG(>BBB_I2C_IRQSTATUS_RAW):%x\n",REG(>BBB_I2C_IRQSTATUS_RAW)
>  );
> + // printf("%x\n",0x1400 & 0x1000 );
> + printf("REG(>BBB_I2C_IRQSTATUS_RAW) & 
> AM335X_I2C_IRQSTATUS_RAW_BB:%x\n",(REG(>BBB_I2C_IRQSTATUS_RAW) & 
> AM335X_I2C_IRQSTATUS_RAW_BB));
> +while(stat =( REG(>BBB_I2C_IRQSTATUS_RAW) & 
> AM335X_I2C_IRQSTATUS_RAW_BB) && timeout--)
>{
> -status = true; 
> -  } else {
> -status = false;
> +
>

Re: [PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-14 Thread Gedare Bloom 
We eventually need all of the combined patches at once in order to
merge. If this is functionally working, then it is time to focus on
cleaning up.

It would be good to separate the testsuite change from the others. I'm
not sure the testsuite is mergeable.

It would also be best to separate the eeprom.c change from the others,
since it is modifying more shared code and should be reviewed/merged
separately from the BB changes.

I have a few comments for you to improve below.

On Tue, Mar 14, 2017 at 10:05 AM, Sichen Zhao <1473996...@qq.com> wrote:
> ---
>  c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 
> --
>  c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
>  cpukit/dev/i2c/eeprom.c   |  24 +-
>  testsuites/samples/i2c0/init.c|  98 -
>  4 files changed, 777 insertions(+), 47 deletions(-)
>
> diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
> b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> index 6b790e5..6a22125 100644
> --- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> +++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
> @@ -21,11 +21,23 @@
>  #include 
>  #include 
>
> +#define u16 unsigned int
If this is just for your convenience, prefer to use the standard and
available uint16_t instead. Is there an advantage to use the 16 bit
versus 32 bit? (As you have it, the u16 is going to be 32 bit.)

> +
> +static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
> +static void omap24_i2c_init(i2c_bus *base);
> +static void flush_fifo(i2c_bus *base);
> +static int wait_for_bb(i2c_bus *base);
> +static int omap24_i2c_probe(i2c_bus *base);
> +static u16 wait_for_event(i2c_bus *base);
> +static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, int 
> alen, unsigned char *buffer,
> +   int len);
> +static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
> +static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint addr,int 
> alen, unsigned char *buffer,
> +int len);
>  /*
Eventually need to remove/fix the commented-out code.

>  static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  {
>bool status =true;
> -
>  // We will check i2c_bus_id in am335x_i2c_bus_register
>  // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE 
> ??
>if (bus->i2c_bus_id == I2C1) {
> @@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
>  /* ref. Table 21-4 I2C Clock Signals */
>  /*
>   For I2C1/2
> -
>   Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
> -
>   Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
>  */
>
> @@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. As 
> long as in
>  this configuration, power domain sleep transition cannot happen.*/
>   /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
>  AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
> -
>while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
>AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
>  */
> @@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
> happen.*/
>if (bus->i2c_bus_id == I2C1) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
>   AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
>} else if (bus->i2c_bus_id == I2C2) {
>REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
> -
>while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
>   AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
> AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
> -
>while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
> (AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
>  AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
> -
>}
>  }
>  */
>
>  static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
>  {
> -  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
> +  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
> AM335X_CONF_I2C0_SDA);
This line exceeds 80 char. Please have a read through
https://devel.rtems.org/wiki/Developer/Coding/Conventions

debug prints should be wrapped in an ifdef controlled by a define flag
either at the top of the file (preferred, to localize debug output) or
from RTEMS_DEBUG.

> +  printf("bus->regs:%x\n", bus->regs);
> +
> +  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
Replacing a variable with a hard-coded value (here, 0x44e1) is not
good for maintainability or readability. Please provide/use a proper
variable or macro.

>(BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
>
> -  REG(bus->regs + AM335X_CONF_I2C0_SCL) =
> +

[PATCH] modify punitvara's works on BBB i2c, and now can read the eeprom info.

2017-03-14 Thread Sichen Zhao 
---
 c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c | 684 --
 c/src/lib/libbsp/arm/beagle/include/i2c.h |  18 +-
 cpukit/dev/i2c/eeprom.c   |  24 +-
 testsuites/samples/i2c0/init.c|  98 -
 4 files changed, 777 insertions(+), 47 deletions(-)

diff --git a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c 
b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
index 6b790e5..6a22125 100644
--- a/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
+++ b/c/src/lib/libbsp/arm/beagle/i2c/bbb-i2c.c
@@ -21,11 +21,23 @@
 #include 
 #include 
 
+#define u16 unsigned int
+
+static int am335x_i2c_set_clock(i2c_bus *base, unsigned long clock);
+static void omap24_i2c_init(i2c_bus *base);
+static void flush_fifo(i2c_bus *base);
+static int wait_for_bb(i2c_bus *base);
+static int omap24_i2c_probe(i2c_bus *base);
+static u16 wait_for_event(i2c_bus *base);
+static int am335x_i2c_read(i2c_bus *base, unsigned char chip, uint addr, int 
alen, unsigned char *buffer, 
+   int len);
+static int read_eeprom(i2c_bus *base,struct am335x_baseboard_id *header);
+static int am335x_i2c_write(i2c_bus *base, unsigned char chip, uint addr,int 
alen, unsigned char *buffer, 
+int len);
 /*
 static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
 {
   bool status =true;
-
 // We will check i2c_bus_id in am335x_i2c_bus_register
 // Apart from mode and pull_up register what about SCREWCTRL & RXACTIVE ??
   if (bus->i2c_bus_id == I2C1) {
@@ -48,9 +60,7 @@ static bool am335x_i2c_pinmux(bbb_i2c_bus *bus)
 /* ref. Table 21-4 I2C Clock Signals */
 /* 
  For I2C1/2
-
  Interface clock - 100MHz - CORE_LKOUTM4 / 2 - pd_per_l4ls_gclk
-
  Functional clock - 48MHz - PER_CLKOUTM2 / 4 - pd_per_ic2_fclk
 */
 
@@ -74,7 +84,6 @@ state. Functional clocks are guarantied to stay present. As 
long as in
 this configuration, power domain sleep transition cannot happen.*/
  /* REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) |=
 AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE;
-
   while((REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKCTRL) &
   AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE) != 
AM335X_CM_PER_L4LS_CLKCTRL_MODULEMODE_ENABLE);
 */
@@ -86,30 +95,29 @@ this configuration, power domain sleep transition cannot 
happen.*/
   if (bus->i2c_bus_id == I2C1) {
   REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) |=
  AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE;
-
   while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C1_CLKCTRL) &
  AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE) != 
AM335X_CM_PER_I2C1_CLKCTRL_MODULEMODE_ENABLE);
   } else if (bus->i2c_bus_id == I2C2) {
   REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) |=
  AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE;
-
   while(REG((AM335X_CM_PER_ADDR + AM335X_CM_PER_I2C2_CLKCTRL) &
  AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE) != 
AM335X_CM_PER_I2C2_CLKCTRL_MODULEMODE_ENABLE);
-
   while(!(REG(AM335X_CM_PER_ADDR + AM335X_CM_PER_L4LS_CLKSTCTRL) &
(AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_L4LS_GCLK |
 AM335X_CM_PER_L4LS_CLKSTCTRL_CLKACTIVITY_I2C_FCLK)));
-
   }
 }
 */
 
 static void am335x_i2c0_pinmux(bbb_i2c_bus *bus)
 {
-  REG(bus->regs + AM335X_CONF_I2C0_SDA) =
+  printf("0x44e1 + AM335X_CONF_I2C0_SDA:%x\n",0x44e1 + 
AM335X_CONF_I2C0_SDA);
+  printf("bus->regs:%x\n", bus->regs);
+ 
+  REG(0x44e1 + AM335X_CONF_I2C0_SDA) =
   (BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN);
 
-  REG(bus->regs + AM335X_CONF_I2C0_SCL) =
+  REG(0x44e1 + AM335X_CONF_I2C0_SCL) =
   (BBB_RXACTIVE | BBB_SLEWCTRL | BBB_PU_EN); 
 }
 
@@ -314,14 +322,29 @@ void am335x_i2c_init(bbb_i2c_bus *bus, uint32_t 
input_clock)
 static bool am335x_i2c_busbusy(volatile bbb_i2c_regs *regs)
 {
   bool status;
-
-  if (REG(>BBB_I2C_IRQSTATUS_RAW) & AM335X_I2C_IRQSTATUS_RAW_BB)
+  unsigned short stat;
+  int timeout = I2C_TIMEOUT;
+
+  REG(>BBB_I2C_IRQSTATUS)=0x;
+  
printf("REG(>BBB_I2C_IRQSTATUS_RAW):%x\n",REG(>BBB_I2C_IRQSTATUS_RAW)
 );
+ // printf("%x\n",0x1400 & 0x1000 );
+ printf("REG(>BBB_I2C_IRQSTATUS_RAW) & 
AM335X_I2C_IRQSTATUS_RAW_BB:%x\n",(REG(>BBB_I2C_IRQSTATUS_RAW) & 
AM335X_I2C_IRQSTATUS_RAW_BB));
+while(stat =( REG(>BBB_I2C_IRQSTATUS_RAW) & AM335X_I2C_IRQSTATUS_RAW_BB) 
&& timeout--)
   {
-status = true; 
-  } else {
-status = false;
+
+REG(>BBB_I2C_IRQSTATUS)=stat;
+udelay(20);
+
+  }
+
+  if (timeout <= 0) {
+printf("Timed out in wait_for_bb: status=%04x\n",
+   stat);
+return 1;
   }
-  return status; 
+  REG(>BBB_I2C_IRQSTATUS)=0x;   /* clear delayed stuff*/
+  return 0;
+
 }
 
 static void am335x_i2c_reset(bbb_i2c_bus *bus)
@@ -330,9 +353,27 @@ static void am335x_i2c_reset(bbb_i2c_bus *bus)
   printk("reset bus->reg is %x \n",bus->regs);
   /* Disable I2C module at the time of initialization*/
   /*Should I use write32 ?? I guess mmio_clear is correct choice here*/
+  REG(>BBB_I2C_CON)=0x00;