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

[Christian Mauderer]

Hello Vijay,

I tried it and the patch works. But there is a point that might could be
improved:

My application can still call bbb_register_i2c_0(). If I do that it
returns an error code. That's better than if it would just initialize
the bus twice. But it's not very clear from a user perspective. I could
think of multiple alternatives:

1. Remove / rename the function so that it is clear at link time that it
isn't there any more. In the ideal case, it's not longer visible in a
header that is thought for the application developer.

2. Use 1 but make the function an empty one (returning success) so it
doesn't fail.

3. Remember that the function has already been called in some
function-static flag and don't execute it a second time.

I think I would use 1. The function isn't useful any more if it is
called during system init. And it should be very clear for a developer
that something changed if there is a linker error with that function.

Best regards

Christian

On 18/09/2019 00:03, Vijay Kumar Banerjee wrote:
> ---
>  bsps/arm/beagle/i2c/bbb-i2c.c    |  6 +++---
>  bsps/arm/beagle/start/bspstart.c | 13 +++++++++++++
>  2 files changed, 16 insertions(+), 3 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/start/bspstart.c b/bsps/arm/beagle/start/bspstart.c
> index 224f9ecf3b..aadb9e826f 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,14 @@ uint32_t bsp_fdt_map_intr(const uint32_t *intr, size_t icells)
>  {
>    return intr[0];
>  }
> +
> +static void bbb_i2c_0_initialize(void)
> +{
> +  bbb_register_i2c_0();
> +}
> +
> +RTEMS_SYSINIT_ITEM(
> +  bbb_i2c_0_initialize,
> +  RTEMS_SYSINIT_LAST,
> +  RTEMS_SYSINIT_ORDER_LAST
> +);
>