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

Christian Mauderer list at c-mauderer.de
Thu Sep 19 19:33:24 UTC 2019


Thanks for the patch. Now it's OK and I pushed it (with an indentation fix).

Best regards

Christian

On 19/09/2019 20:56, Vijay Kumar Banerjee wrote:
> 
> 
> 
> On Fri, Sep 20, 2019 at 12:08 AM Christian Mauderer <list at c-mauderer.de
> <mailto:list at c-mauderer.de>> wrote:
> 
>     On 19/09/2019 19:48, Vijay Kumar Banerjee wrote:
>     >
>     >
>     >
>     > On Thu, Sep 19, 2019 at 10:05 PM Christian Mauderer
>     <list at c-mauderer.de <mailto:list at c-mauderer.de>
>     > <mailto:list at c-mauderer.de <mailto:list at c-mauderer.de>>> wrote:
>     >
>     >     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.
>     >
>     > This is a very good point. 
>     >
>     >     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.
>     >
>     > Will it be sufficient to do this:
>     > ```
>     > static void bbb_i2c_0_initialize(void)
>     > {
>     >   int err;
>     >
>     >   err = am335x_i2c_bus_register(BBB_I2C_0_BUS_PATH,
>     >                                 AM335X_I2C0_BASE,
>     >                                 I2C_BUS_CLOCK_DEFAULT,
>     >                                 BBB_I2C0_IRQ);
>     >   if (err != 0){
>     >     printk("i2c-0 device could not be registered");
>     >   }
>     > }
>     > ```
> 
>     You might could add the value of `err` to the output (just at the end in
>     brackets or similar). Sometimes it is helpful to find the reason.
>     Otherwise I'm OK with it.
> 
> Fixed it in the attached v3 of the patch. 



More information about the devel mailing list