<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 18, 2019 at 11:29 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>
I tried it and the patch works. But there is a point that might could be<br>
improved:<br>
<br>
My application can still call bbb_register_i2c_0(). If I do that it<br>
returns an error code. That's better than if it would just initialize<br>
the bus twice. But it's not very clear from a user perspective. I could<br>
think of multiple alternatives:<br>
<br>
1. Remove / rename the function so that it is clear at link time that it<br>
isn't there any more. In the ideal case, it's not longer visible in a<br>
header that is thought for the application developer.<br>
<br>
2. Use 1 but make the function an empty one (returning success) so it<br>
doesn't fail.<br>
<br>
3. Remember that the function has already been called in some<br>
function-static flag and don't execute it a second time.<br>
<br>
I think I would use 1. The function isn't useful any more if it is<br>
called during system init. And it should be very clear for a developer<br>
that something changed if there is a linker error with that function.<br>
<br></blockquote><div>Hi,</div><div><br></div><div>Thanks for the review. I have posted a v2 [1] of the patch following the first</div><div>alternative as you suggested. I'll also try to send a docs patch to mention</div><div>that i2c-0 is registered at init and i2c-1 and i2c-2 can be registered with the</div><div>respective wrappers.</div><div><br></div><div>[1]: <a href="https://lists.rtems.org/pipermail/devel/2019-September/027765.html">https://lists.rtems.org/pipermail/devel/2019-September/027765.html</a></div><div><br></div><div>Best regards,</div><div>Vijay </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 00:03, Vijay Kumar Banerjee wrote:<br>
> ---<br>
>  bsps/arm/beagle/i2c/bbb-i2c.c    |  6 +++---<br>
>  bsps/arm/beagle/start/bspstart.c | 13 +++++++++++++<br>
>  2 files changed, 16 insertions(+), 3 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/start/bspstart.c b/bsps/arm/beagle/start/bspstart.c<br>
> index 224f9ecf3b..aadb9e826f 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,14 @@ 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>
> +  bbb_register_i2c_0();<br>
> +}<br>
> +<br>
> +RTEMS_SYSINIT_ITEM(<br>
> +  bbb_i2c_0_initialize,<br>
> +  RTEMS_SYSINIT_LAST,<br>
> +  RTEMS_SYSINIT_ORDER_LAST<br>
> +);<br>
> <br>
</blockquote></div></div>