linguini1 commented on code in PR #18499:
URL: https://github.com/apache/nuttx/pull/18499#discussion_r2896971705
##########
arch/arm64/src/bcm2711/bcm2711_gpio.c:
##########
@@ -370,35 +372,50 @@ void bcm2711_gpio_set_func(uint32_t gpio, enum
bcm2711_gpio_func_e func)
DEBUGASSERT(gpio < BCM_GPIO_NUM);
uint32_t value = 0;
+ uint32_t mask = 0;
+ uint32_t shift = 0;
+
if (gpio <= 9)
{
- value = (g_fsel_map[func] << (gpio * 3));
- modreg32(value, value, BCM_GPIO_GPFSEL0);
+ shift = (gpio * 3);
+ mask = (0x7 << shift);
+ value = (g_fsel_map[func] << shift);
+ modreg32(value, mask, BCM_GPIO_GPFSEL0);
}
- else if (gpio <= 19 && gpio > 9)
+ else if (gpio <= 19 && gpio >= 10)
{
- value = (g_fsel_map[func] << ((gpio - 10) * 3));
- modreg32(value, value, BCM_GPIO_GPFSEL1);
+ shift = ((gpio - 10) * 3);
+ mask = (0x7 << shift);
+ value = (g_fsel_map[func] << shift);
+ modreg32(value, mask, BCM_GPIO_GPFSEL1);
}
- else if (gpio <= 29 && gpio > 20)
+ else if (gpio <= 29 && gpio >= 20)
{
- value = (g_fsel_map[func] << ((gpio - 20) * 3));
- modreg32(value, value, BCM_GPIO_GPFSEL2);
+ shift = ((gpio - 20) * 3);
+ mask = (0x7 << shift);
+ value = (g_fsel_map[func] << shift);
+ modreg32(value, mask, BCM_GPIO_GPFSEL2);
Review Comment:
It is (at least in my experience) pretty common in the codebase to use
`value` as the mask as well when calling `modreg32`. I think I would revert
this since it's not part of your GPIO change.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
