Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Rhyland Klein <rklein@nvidia.com>]

On 11/26/2013 5:05 AM, Mika Westerberg wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/mach-tegra/board-paz00.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
> index 06f024070dab..a309795da665 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <linux/platform_device.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/rfkill-gpio.h>
>  #include "board.h"
>  
> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
>  	},
>  };
>  
> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> +	GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
> +	GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
> +};

I wouldn't think this table would match for the gpios as the driver
currently is. From what I see, the driver calls into gpiod_get_index,
which will try 1 of 3 ways of getting the gpios:

of-enabled: of_find_gpio
	- which I believe wouldn't work for paz00, since rfkill
	  doesn't support dt?
acpi: acpi_find_gpio
	- I assume this does work, but I didn't dive into it
gpiod lookup table: gpiod_find
	- I think this is the path we expect to be taken, given the addition of
the lookup table here, but I don't think it would actually match.
Looking at the code for gpiod_find, it seems like it would try to match
the con_id, but would fail. Patch 2/6 is passing the reset_name and
shutdown_name for the con_ids, which isn't what is registered in this
table.

Shouldn't it look more like this?

+static struct gpiod_lookup wifi_gpio_lookup[] = {
+	GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, 0),
+	GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, 0),
+};

Sorry if I am missing something...

-rhyland

-- 
nvpublic