<div dir="ltr"><div dir="ltr"><br><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 19, 2019 at 10:05 PM Christian Mauderer <<a href="mailto:list@c-mauderer.de">list@c-mauderer.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Vijay,<br>
<br>
sorry for being picky: What happens if something goes wrong during that<br>
initialization? For example: If the system has a configuration where<br>
/dev/i2c0 can't be created. In that case it will just silently fail to<br>
create the i2c-0 which maybe leads to odd errors later.<br>
<br></blockquote><div>This is a very good point. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
There's already one precedent case how that could be handled: The<br>
powerpc/mvme3100 BSP. That BSP also initializes an I2C driver with a<br>
RTEMS_SYSINIT_ITEM. The return value is ignored too. But the<br>
BSP_i2c_initialize does print error messages.<br>
<br>
I would suggest that here too: Check the return value of<br>
am335x_i2c_bus_register in your new bbb_i2c_0_initialize and at least<br>
print an error message that i2c0 couldn't be initialized. Most likely<br>
you should use printk here because it's not sure whether printf would<br>
work already.<br>
<br></blockquote><div>Will it be sufficient to do this:</div><div>```</div><div>static void bbb_i2c_0_initialize(void)<br>{<br>  int err;<br><br>  err = am335x_i2c_bus_register(BBB_I2C_0_BUS_PATH,<br>                                AM335X_I2C0_BASE,<br>                                I2C_BUS_CLOCK_DEFAULT,<br>                                BBB_I2C0_IRQ);<br>  if (err != 0){<br>    printk("i2c-0 device could not be registered");<br>  }<br>}<br></div><div>``` </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Best regards<br>
<br>
Christian<br>
<br>
On 18/09/2019 21:18, Vijay Kumar Banerjee wrote:<br>
> ---<br>
>  bsps/arm/beagle/i2c/bbb-i2c.c     |  6 +++---<br>
>  bsps/arm/beagle/include/bsp/i2c.h | 10 ----------<br>
>  bsps/arm/beagle/start/bspstart.c  | 16 ++++++++++++++++<br>
>  3 files changed, 19 insertions(+), 13 deletions(-)<br>
> <br>
> diff --git a/bsps/arm/beagle/i2c/bbb-i2c.c b/bsps/arm/beagle/i2c/bbb-i2c.c<br>
> index 37b88864b9..f705078085 100644<br>
> --- a/bsps/arm/beagle/i2c/bbb-i2c.c<br>
> +++ b/bsps/arm/beagle/i2c/bbb-i2c.c<br>
> @@ -186,16 +186,16 @@ static int am335x_i2c_reset( bbb_i2c_bus *bus )<br>
>  <br>
>    bus->con_reg = 0;<br>
>    regs->BBB_I2C_CON = bus->con_reg;<br>
> -  udelay( 50000 );<br>
> +  rtems_counter_delay_nanoseconds(50000000);<br>
>  <br>
>    regs->BBB_I2C_SYSC = AM335X_I2C_SYSC_SRST;<br>
> -  udelay( 1000 );<br>
> +  rtems_counter_delay_nanoseconds(1000000);<br>
>    regs->BBB_I2C_CON = AM335X_I2C_CON_I2C_EN;<br>
>  <br>
>    while ( !( regs->BBB_I2C_SYSS & AM335X_I2C_SYSS_RDONE )<br>
>            && timeout >= 0 ) {<br>
>      --timeout;<br>
> -    udelay( 100 );<br>
> +  rtems_counter_delay_nanoseconds(100000);<br>
>    }<br>
>  <br>
>    if ( timeout <= 0 ) {<br>
> diff --git a/bsps/arm/beagle/include/bsp/i2c.h b/bsps/arm/beagle/include/bsp/i2c.h<br>
> index 9d253406bf..60f71194bf 100644<br>
> --- a/bsps/arm/beagle/include/bsp/i2c.h<br>
> +++ b/bsps/arm/beagle/include/bsp/i2c.h<br>
> @@ -94,16 +94,6 @@ int am335x_i2c_bus_register(<br>
>    rtems_vector_number irq<br>
>  );<br>
>  <br>
> -static inline int bbb_register_i2c_0( void )<br>
> -{<br>
> -  return am335x_i2c_bus_register(<br>
> -    BBB_I2C_0_BUS_PATH,<br>
> -    AM335X_I2C0_BASE,<br>
> -    I2C_BUS_CLOCK_DEFAULT,<br>
> -    BBB_I2C0_IRQ<br>
> -  );<br>
> -}<br>
> -<br>
>  static inline int bbb_register_i2c_1( void )<br>
>  {<br>
>    return am335x_i2c_bus_register(<br>
> diff --git a/bsps/arm/beagle/start/bspstart.c b/bsps/arm/beagle/start/bspstart.c<br>
> index 224f9ecf3b..8112e0867d 100644<br>
> --- a/bsps/arm/beagle/start/bspstart.c<br>
> +++ b/bsps/arm/beagle/start/bspstart.c<br>
> @@ -17,6 +17,8 @@<br>
>  #include <bsp/irq-generic.h><br>
>  #include <bsp/fdt.h><br>
>  #include <bsp/linker-symbols.h><br>
> +#include <bsp/i2c.h><br>
> +#include <rtems/sysinit.h><br>
>  <br>
>  #include "bspdebug.h"<br>
>  <br>
> @@ -41,3 +43,17 @@ uint32_t bsp_fdt_map_intr(const uint32_t *intr, size_t icells)<br>
>  {<br>
>    return intr[0];<br>
>  }<br>
> +<br>
> +static void bbb_i2c_0_initialize(void)<br>
> +{<br>
> +  am335x_i2c_bus_register(BBB_I2C_0_BUS_PATH,<br>
> +                          AM335X_I2C0_BASE,<br>
> +                          I2C_BUS_CLOCK_DEFAULT,<br>
> +                          BBB_I2C0_IRQ);<br>
> +}<br>
> +<br>
> +RTEMS_SYSINIT_ITEM(<br>
> +  bbb_i2c_0_initialize,<br>
> +  RTEMS_SYSINIT_LAST,<br>
> +  RTEMS_SYSINIT_ORDER_LAST<br>
> +);<br>
> <br>
<br>
</blockquote></div></div>