Re: [PATCH 1/2] ARM: boards: protonic-imx6: prtvt7: Don't use fixed numbers for dynamic GPIOs

[Robin van der Gracht <robin@protonic.nl>]

Hi Sacha,

On 31-03-2025 10:10, Sascha Hauer wrote:
> On Thu, Mar 27, 2025 at 11:06:34AM +0100, Robin van der Gracht wrote:
>> The GPIO indexing differs from the pin indexing.
>>
>> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
>> ---
>>   arch/arm/boards/protonic-imx6/board.c | 32 +++++++++++++++++++++------
>>   1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/boards/protonic-imx6/board.c b/arch/arm/boards/protonic-imx6/board.c
>> index 9e62dc1544..4585ca4730 100644
>> --- a/arch/arm/boards/protonic-imx6/board.c
>> +++ b/arch/arm/boards/protonic-imx6/board.c
>> @@ -21,6 +21,7 @@
>>   #include <net.h>
>>   #include <of_device.h>
>>   #include <linux/regmap.h>
>> +#include <of_gpio.h>
>>   #include <sys/mount.h>
>>   #include <sys/stat.h>
>>   #include <unistd.h>
>> @@ -740,17 +741,34 @@ static int prt_imx6_init_kvg_yaco(struct prt_imx6_priv *priv)
>>   	return prt_imx6_init_kvg_power(priv, PW_MODE_KVG_WITH_YACO);
>>   }
>>   
>> -#define GPIO_KEY_F6     (0xe0 + 5)
>> -#define GPIO_KEY_CYCLE  (0xe0 + 2)
>> -
>>   static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv)
>>   {
>> -	/* This function relies heavely on the gpio-pca9539 driver */
>> +	int gpio_cycle, gpio_f6;
>> +	struct device_node *keys, *key;
>> +
>> +	keys = of_find_compatible_node(NULL, NULL, "gpio-keys");
>> +	if (!keys) {
>> +		dev_err(priv->dev, "Can't find gpio-keys of node!\n");
>> +		return -ENODEV;
>> +	}
>>   
>> -	gpio_direction_input(GPIO_KEY_F6);
>> -	gpio_direction_input(GPIO_KEY_CYCLE);
>> +	key = of_find_node_by_name(keys, "key-cycle");
>> +	gpio_cycle = of_get_named_gpio_flags(key, "gpios", 0, NULL);
>> +	if (gpio_cycle < 0) {
>> +		dev_err(priv->dev, "Unable to get cycle key gpio\n");
>> +		return gpio_cycle;
>> +	}
>> +	gpio_request_one(gpio_cycle, GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "Cycle");
>> +
>> +	key = of_find_node_by_name(keys, "key-f6");
>> +	gpio_f6 = of_get_named_gpio_flags(key, "gpios", 0, NULL);
>> +	if (gpio_f6 < 0) {
>> +		dev_err(priv->dev, "Unable to get F6 key gpio\n");
>> +		return gpio_f6;
>> +	}
>> +	gpio_request_one(gpio_f6, GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "F6");
>>   
>> -	if (gpio_get_value(GPIO_KEY_CYCLE) && gpio_get_value(GPIO_KEY_F6))
>> +	if (!(gpio_is_active(gpio_cycle) && gpio_is_active(gpio_f6)))
>>   		priv->no_usb_check = 1;
> Given that you parse the gpio-keys node here, have you thought about
> using the gpio-keys driver directly instead?
>
> input_register_notfier() should give you the necessary events when KEY_CYCLEWINDOWS
> and KEY_F6 are pressed.
We tried that in the past. Building Barebox with the gpio-keys driver 
(and thus enabling the keyboard),
had the side effect that Barebox autoboot was interruptible when a user 
pressed one of the buttons
a at the right time during boot.

We reverted to this option at the time to circumvent that. But now days 
we require ctrl-c to be pressed
on almost all of our boards to interrupt autoboot. So we could give it 
another try with the gpio-keys driver.

Robin