[PATCH v2] bsps/beagle: register i2c device at initialization

[Christian Mauderer]

Hello Vijay,

sorry for being picky: What happens if something goes wrong during that
initialization? For example: If the system has a configuration where
/dev/i2c0 can't be created. In that case it will just silently fail to
create the i2c-0 which maybe leads to odd errors later.

There's already one precedent case how that could be handled: The
powerpc/mvme3100 BSP. That BSP also initializes an I2C driver with a
RTEMS_SYSINIT_ITEM. The return value is ignored too. But the
BSP_i2c_initialize does print error messages.

I would suggest that here too: Check the return value of
am335x_i2c_bus_register in your new bbb_i2c_0_initialize and at least
print an error message that i2c0 couldn't be initialized. Most likely
you should use printk here because it's not sure whether printf would
work already.

Best regards

Christian

On 18/09/2019 21:18, Vijay Kumar Banerjee wrote:
> ---
>  bsps/arm/beagle/i2c/bbb-i2c.c     |  6 +++---
>  bsps/arm/beagle/include/bsp/i2c.h | 10 ----------
>  bsps/arm/beagle/start/bspstart.c  | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/bsps/arm/beagle/i2c/bbb-i2c.c b/bsps/arm/beagle/i2c/bbb-i2c.c
> index 37b88864b9..f705078085 100644
> --- a/bsps/arm/beagle/i2c/bbb-i2c.c
> +++ b/bsps/arm/beagle/i2c/bbb-i2c.c
> @@ -186,16 +186,16 @@ static int am335x_i2c_reset( bbb_i2c_bus *bus )
>  
>    bus->con_reg = 0;
>    regs->BBB_I2C_CON = bus->con_reg;
> -  udelay( 50000 );
> +  rtems_counter_delay_nanoseconds(50000000);
>  
>    regs->BBB_I2C_SYSC = AM335X_I2C_SYSC_SRST;
> -  udelay( 1000 );
> +  rtems_counter_delay_nanoseconds(1000000);
>    regs->BBB_I2C_CON = AM335X_I2C_CON_I2C_EN;
>  
>    while ( !( regs->BBB_I2C_SYSS & AM335X_I2C_SYSS_RDONE )
>            && timeout >= 0 ) {
>      --timeout;
> -    udelay( 100 );
> +  rtems_counter_delay_nanoseconds(100000);
>    }
>  
>    if ( timeout <= 0 ) {
> diff --git a/bsps/arm/beagle/include/bsp/i2c.h b/bsps/arm/beagle/include/bsp/i2c.h
> index 9d253406bf..60f71194bf 100644
> --- a/bsps/arm/beagle/include/bsp/i2c.h
> +++ b/bsps/arm/beagle/include/bsp/i2c.h
> @@ -94,16 +94,6 @@ int am335x_i2c_bus_register(
>    rtems_vector_number irq
>  );
>  
> -static inline int bbb_register_i2c_0( void )
> -{
> -  return am335x_i2c_bus_register(
> -    BBB_I2C_0_BUS_PATH,
> -    AM335X_I2C0_BASE,
> -    I2C_BUS_CLOCK_DEFAULT,
> -    BBB_I2C0_IRQ
> -  );
> -}
> -
>  static inline int bbb_register_i2c_1( void )
>  {
>    return am335x_i2c_bus_register(
> diff --git a/bsps/arm/beagle/start/bspstart.c b/bsps/arm/beagle/start/bspstart.c
> index 224f9ecf3b..8112e0867d 100644
> --- a/bsps/arm/beagle/start/bspstart.c
> +++ b/bsps/arm/beagle/start/bspstart.c
> @@ -17,6 +17,8 @@
>  #include <bsp/irq-generic.h>
>  #include <bsp/fdt.h>
>  #include <bsp/linker-symbols.h>
> +#include <bsp/i2c.h>
> +#include <rtems/sysinit.h>
>  
>  #include "bspdebug.h"
>  
> @@ -41,3 +43,17 @@ uint32_t bsp_fdt_map_intr(const uint32_t *intr, size_t icells)
>  {
>    return intr[0];
>  }
> +
> +static void bbb_i2c_0_initialize(void)
> +{
> +  am335x_i2c_bus_register(BBB_I2C_0_BUS_PATH,
> +                          AM335X_I2C0_BASE,
> +                          I2C_BUS_CLOCK_DEFAULT,
> +                          BBB_I2C0_IRQ);
> +}
> +
> +RTEMS_SYSINIT_ITEM(
> +  bbb_i2c_0_initialize,
> +  RTEMS_SYSINIT_LAST,
> +  RTEMS_SYSINIT_ORDER_LAST
> +);
>