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

[Vijay Kumar Banerjee]

On Wed, Sep 18, 2019 at 11:29 PM Christian Mauderer <list at c-mauderer.de>
wrote:

> 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.
>
> Hi,

Thanks for the review. I have posted a v2 [1] of the patch following the
first
alternative as you suggested. I'll also try to send a docs patch to mention
that i2c-0 is registered at init and i2c-1 and i2c-2 can be registered with
the
respective wrappers.

[1]: https://lists.rtems.org/pipermail/devel/2019-September/027765.html

Best regards,
Vijay

> 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
> > +);
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20190919/414398e3/attachment-0002.html>