[PATCH] sunxi: Properly check for SATAPWR and MACPWR
The #ifdef CONFIG_xxxPWR conditionals were not working as expected, as string Kconfig symbols are always "defined" from the preprocessor's perspective. This lead to unnecessary calls to the GPIO routines, but also always added a half a second delay to wait for a SATA disk to power up. Many thanks to Peter for pointing this out!
Fix this by properly comparing the Kconfig symbols against the empty string. strcmp() would be nicer for this, but GCC does not optimise this away, probably due to our standalone compiler switches.
Reported-by: Peter Robinson pbrobinson@gmail.com Signed-off-by: Andre Przywara andre.przywara@arm.com --- board/sunxi/board.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 4f058952b5b..a0b5778b3bc 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -265,18 +265,28 @@ int board_init(void) if (ret) return ret;
-#ifdef CONFIG_SATAPWR - satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR); - gpio_request(satapwr_pin, "satapwr"); - gpio_direction_output(satapwr_pin, 1); - /* Give attached sata device time to power-up to avoid link timeouts */ - mdelay(500); -#endif -#ifdef CONFIG_MACPWR - macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR); - gpio_request(macpwr_pin, "macpwr"); - gpio_direction_output(macpwr_pin, 1); -#endif + /* strcmp() would look better, but doesn't get optimised away. */ + if (CONFIG_SATAPWR[0]) { + satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR); + if (satapwr_pin >= 0) { + gpio_request(satapwr_pin, "satapwr"); + gpio_direction_output(satapwr_pin, 1); + + /* + * Give the attached SATA device time to power-up + * to avoid link timeouts + */ + mdelay(500); + } + } + + if (CONFIG_MACPWR[0]) { + macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR); + if (macpwr_pin >= 0) { + gpio_request(macpwr_pin, "macpwr"); + gpio_direction_output(macpwr_pin, 1); + } + }
#ifdef CONFIG_DM_I2C /*
On 1/18/21 7:05 PM, Andre Przywara wrote:
The #ifdef CONFIG_xxxPWR conditionals were not working as expected, as string Kconfig symbols are always "defined" from the preprocessor's perspective. This lead to unnecessary calls to the GPIO routines, but also always added a half a second delay to wait for a SATA disk to power up. Many thanks to Peter for pointing this out!
Fix this by properly comparing the Kconfig symbols against the empty string. strcmp() would be nicer for this, but GCC does not optimise this away, probably due to our standalone compiler switches.
Ethernet still works, and the speed-up is welcome.
Tested-by: Samuel Holland samuel@sholland.org # Orange Pi WinPlus
Cheers, Samuel
On Tue, Jan 19, 2021 at 1:06 AM Andre Przywara andre.przywara@arm.com wrote:
The #ifdef CONFIG_xxxPWR conditionals were not working as expected, as string Kconfig symbols are always "defined" from the preprocessor's perspective. This lead to unnecessary calls to the GPIO routines, but also always added a half a second delay to wait for a SATA disk to power up. Many thanks to Peter for pointing this out!
Fix this by properly comparing the Kconfig symbols against the empty string. strcmp() would be nicer for this, but GCC does not optimise this away, probably due to our standalone compiler switches.
Reported-by: Peter Robinson pbrobinson@gmail.com Signed-off-by: Andre Przywara andre.przywara@arm.com
Tested-by: Peter Robinson pbrobinson@gmail.com
Tested on Pine64, Cubietruck with root fs on SATA SSD, and an orangepi_pc
board/sunxi/board.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 4f058952b5b..a0b5778b3bc 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -265,18 +265,28 @@ int board_init(void) if (ret) return ret;
-#ifdef CONFIG_SATAPWR
satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR);gpio_request(satapwr_pin, "satapwr");gpio_direction_output(satapwr_pin, 1);/* Give attached sata device time to power-up to avoid link timeouts */mdelay(500);-#endif -#ifdef CONFIG_MACPWR
macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);gpio_request(macpwr_pin, "macpwr");gpio_direction_output(macpwr_pin, 1);-#endif
/* strcmp() would look better, but doesn't get optimised away. */if (CONFIG_SATAPWR[0]) {satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR);if (satapwr_pin >= 0) {gpio_request(satapwr_pin, "satapwr");gpio_direction_output(satapwr_pin, 1);/** Give the attached SATA device time to power-up* to avoid link timeouts*/mdelay(500);}}if (CONFIG_MACPWR[0]) {macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);if (macpwr_pin >= 0) {gpio_request(macpwr_pin, "macpwr");gpio_direction_output(macpwr_pin, 1);}}#ifdef CONFIG_DM_I2C /* -- 2.17.5
Dne torek, 19. januar 2021 ob 02:05:20 CET je Andre Przywara napisal(a):
The #ifdef CONFIG_xxxPWR conditionals were not working as expected, as string Kconfig symbols are always "defined" from the preprocessor's perspective. This lead to unnecessary calls to the GPIO routines, but also always added a half a second delay to wait for a SATA disk to power up. Many thanks to Peter for pointing this out!
Fix this by properly comparing the Kconfig symbols against the empty string. strcmp() would be nicer for this, but GCC does not optimise this away, probably due to our standalone compiler switches.
Reported-by: Peter Robinson pbrobinson@gmail.com Signed-off-by: Andre Przywara andre.przywara@arm.com
Nice find!
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
participants (4)
-
Andre Przywara -
Jernej Škrabec -
Peter Robinson -
Samuel Holland