Christian Mauderer commented: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/363#note_123059 The code looks OK for me. One point regarding the code: In the transfer function, you use some of the flags (like `I2C_M_TEN`). But not everything is supported (like `I2C_M_IGNORE_NACK`). I would suggest to somewhere check whether only flags that are supported are set. For example, I usually try to do that at the start of a transfer here: https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/bsps/arm/imxrt/i2c/imxrt-lpi2c.c?ref_type=heads#L305 But doing that on the fly is also OK. It just should happen at some point so that an application programmer notes if he tries to use something that is not supported. A functional edge case that you might want to test: Some I2C controllers support an addressing phase with no data. That can be (for example) used to detect I2C devices connected to the bus. We have a `i2cdetect` command for that: https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/cpukit/libmisc/shell/main_i2cdetect.c?ref_type=heads Note that not all drivers and all hardware support it and that's OK. But you should make sure that your driver at least doesn't crash or hangs with these settings. I have roughly looked through your code and I would expect that it at least doesn't crash and maybe even work without changes. But it would be good if you could confirm that you tested it. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/363#note_123059 You're receiving this email because of your account on gitlab.rtems.org.
_______________________________________________ bugs mailing list bugs@rtems.org http://lists.rtems.org/mailman/listinfo/bugs
