resyfer commented on code in PR #18499:
URL: https://github.com/apache/nuttx/pull/18499#discussion_r2893575295
##########
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 might be because I've not seen many usages of the number itself being
used as a mask, but readability definitely took a small hit there. The compiler
and even some processors optimized this sort of stuff anyway. Do you want me to
revert this 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]
