Re: [PATCH v3] ARM: pxa: fix GPIO double shifts

Hi Joe,

Joe Perches [off-list ref] writes:

trivially:

quoted

diff --git a/arch/arm/mach-pxa/corgi_pm.c b/arch/arm/mach-pxa/corgi_pm.c

[]

quoted

@@ -131,15 +131,13 @@ static int corgi_should_wakeup(unsigned int resume_on_alarm)

?	return is_resume;
?}
?
-static unsigned long corgi_charger_wakeup(void)
+static bool corgi_charger_wakeup(void)
?{
-	unsigned long ret;
+	bool ret;
?
-	ret = (!gpio_get_value(CORGI_GPIO_AC_IN) << GPIO_bit(CORGI_GPIO_AC_IN))
-		| (!gpio_get_value(CORGI_GPIO_KEY_INT)
-		<< GPIO_bit(CORGI_GPIO_KEY_INT))
-		| (!gpio_get_value(CORGI_GPIO_WAKEUP)
-		<< GPIO_bit(CORGI_GPIO_WAKEUP));
+	ret = !gpio_get_value(CORGI_GPIO_AC_IN)
+		|| !gpio_get_value(CORGI_GPIO_KEY_INT)
+		|| !gpio_get_value(CORGI_GPIO_WAKEUP);

These might be better without the automatic use of ret

	return !gpio_get_value(CORGI_GPIO_AC_IN) ||
	? ? ? ?!gpio_get_value(CORGI_GPIO_KEY_INT) ||
	? ? ? ?!gpio_get_value(CORGI_GPIO_WAKEUP);

Yeah, I thought about this when I made the patch.

I supposed it was written this way so that a printk was easier to add, that's
why I didn't change the useless variable.

I have no strong opinion about this, so if you think it's worth it I can make
the additional change.

Cheers.

--
Robert

On Mon, 2016-08-01 at 00:04 +0200, Robert Jarzmik wrote:

Joe Perches [off-list ref] writes:

[]

quoted

These might be better without the automatic use of ret

	return !gpio_get_value(CORGI_GPIO_AC_IN) ||
	? ? ? ?!gpio_get_value(CORGI_GPIO_KEY_INT) ||
	? ? ? ?!gpio_get_value(CORGI_GPIO_WAKEUP);

Yeah, I thought about this when I made the patch.

I supposed it was written this way so that a printk was easier to add, that's
why I didn't change the useless variable.

I have no strong opinion about this, so if you think it's worth it I can make
the additional change.

Hi Robert. ?Your choice.

It was one of those, 'oh, bother' type of things but?
I think it's worth it for readability and consistency.

cheers, Joe