On Wed, Sep 18, 2019 at 11:29 PM Christian Mauderer <l...@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 > > +); > > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
