acassis commented on code in PR #17012: URL: https://github.com/apache/nuttx/pull/17012#discussion_r2346558368
########## arch/arm64/src/bcm2711/bcm2711_i2c.c: ########## @@ -77,6 +73,86 @@ #define FIFO_DEPTH 16 +/* Logic for determining which alternate function should be used to achieve + * the user selected I2C pins for each interface + */ + +#ifdef CONFIG_BCM2711_I2C0 + +#if CONFIG_BCM2711_I2C0_SDA == 0 && CONFIG_BCM2711_I2C0_SCL == 1 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 28 && CONFIG_BCM2711_I2C0_SCL == 29 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 44 && CONFIG_BCM2711_I2C0_SCL == 45 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC1 +#elif CONFIG_BCM2711_I2C0_SDA == 46 && CONFIG_BCM2711_I2C0_SCL == 47 +#error "BCM2711 datasheet does not declare the alternate function needed for this pin pair." +#else +#error "Invalid I2C0 pin selections." +#endif + +#endif /* CONFIG_BCM2711_I2C0 */ + +#ifdef CONFIG_BCM2711_I2C1 + +#if CONFIG_BCM2711_I2C1_SDA == 2 && CONFIG_BCM2711_I2C1_SCL == 3 +#define BCM2711_I2C1_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C1_SDA == 44 && CONFIG_BCM2711_I2C1_SCL == 45 +#define BCM2711_I2C1_ALT BCM_GPIO_FUNC2 +#else +#error "Invalid I2C1 pin selections." Review Comment: Same here and for all other invalid pin selections below ########## arch/arm64/src/bcm2711/bcm2711_i2c.c: ########## @@ -77,6 +73,86 @@ #define FIFO_DEPTH 16 +/* Logic for determining which alternate function should be used to achieve + * the user selected I2C pins for each interface + */ + +#ifdef CONFIG_BCM2711_I2C0 + +#if CONFIG_BCM2711_I2C0_SDA == 0 && CONFIG_BCM2711_I2C0_SCL == 1 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 28 && CONFIG_BCM2711_I2C0_SCL == 29 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 44 && CONFIG_BCM2711_I2C0_SCL == 45 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC1 +#elif CONFIG_BCM2711_I2C0_SDA == 46 && CONFIG_BCM2711_I2C0_SCL == 47 +#error "BCM2711 datasheet does not declare the alternate function needed for this pin pair." +#else +#error "Invalid I2C0 pin selections." Review Comment: Maybe include here the valid pins to help user selecting the pins: #error "Invalid I2C0 pin selections, valid SDA/SCL pair pins combination: 0,1; 28,29; 44,45; 46,47" ########## arch/arm64/src/bcm2711/bcm2711_i2c.c: ########## @@ -77,6 +73,86 @@ #define FIFO_DEPTH 16 +/* Logic for determining which alternate function should be used to achieve + * the user selected I2C pins for each interface + */ + +#ifdef CONFIG_BCM2711_I2C0 + +#if CONFIG_BCM2711_I2C0_SDA == 0 && CONFIG_BCM2711_I2C0_SCL == 1 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 28 && CONFIG_BCM2711_I2C0_SCL == 29 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C0_SDA == 44 && CONFIG_BCM2711_I2C0_SCL == 45 +#define BCM2711_I2C0_ALT BCM_GPIO_FUNC1 +#elif CONFIG_BCM2711_I2C0_SDA == 46 && CONFIG_BCM2711_I2C0_SCL == 47 +#error "BCM2711 datasheet does not declare the alternate function needed for this pin pair." +#else +#error "Invalid I2C0 pin selections." +#endif + +#endif /* CONFIG_BCM2711_I2C0 */ + +#ifdef CONFIG_BCM2711_I2C1 + +#if CONFIG_BCM2711_I2C1_SDA == 2 && CONFIG_BCM2711_I2C1_SCL == 3 +#define BCM2711_I2C1_ALT BCM_GPIO_FUNC0 +#elif CONFIG_BCM2711_I2C1_SDA == 44 && CONFIG_BCM2711_I2C1_SCL == 45 +#define BCM2711_I2C1_ALT BCM_GPIO_FUNC2 +#else +#error "Invalid I2C1 pin selections." +#endif + +#endif /* CONFIG_BCM2711_I2C1 */ + +#ifdef CONFIG_BCM2711_I2C3 + +#if CONFIG_BCM2711_I2C3_SDA == 4 && CONFIG_BCM2711_I2C3_SCL == 5 +#define BCM2711_I2C3_ALT BCM_GPIO_FUNC5 +#elif CONFIG_BCM2711_I2C3_SDA == 2 && CONFIG_BCM2711_I2C3_SCL == 3 Review Comment: What if these pins are already used for I2C1? You can define a macro to indicate when a pair is already in use -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
