LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Trent Piepho <tpiepho@freescale.com>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Richard Purdie <rpurdie@rpsys.net>,
	Sean MacLennan <smaclennan@pikatech.com>,
	Wolfram Sang <w.sang@pengutronix.de>,
	Grant Likely <grant.likely@secretlab.ca>
Subject: Re: [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()
Date: Tue, 28 Oct 2008 17:39:33 +0300	[thread overview]
Message-ID: <20081028143933.GA18453@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <1224889741-4167-1-git-send-email-tpiepho@freescale.com>

On Fri, Oct 24, 2008 at 04:08:58PM -0700, Trent Piepho wrote:
> The device binding spec for OF GPIOs defines a flags field, but there is
> currently no way to get it.  This patch adds a parameter to of_get_gpio()
> where the flags will be returned if non-NULL.  of_get_gpio() in turn passes
> the parameter to the of_gpio_chip's xlate method, which can extract any
> flags present from the gpio_spec.
> 
> The default (and currently only) of_gpio_chip xlate method,
> of_gpio_simple_xlate(), is modified to do this.

Looks good. Few comments below.

> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> ---
>  drivers/mtd/nand/fsl_upm.c              |    2 +-
>  drivers/net/fs_enet/fs_enet-main.c      |    2 +-
>  drivers/net/phy/mdio-ofgpio.c           |    4 ++--
>  drivers/of/gpio.c                       |   13 ++++++++++---
>  drivers/serial/cpm_uart/cpm_uart_core.c |    2 +-
>  include/linux/of_gpio.h                 |   17 +++++++++++++----
>  6 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
> index 024e3ff..a25d962 100644
> --- a/drivers/mtd/nand/fsl_upm.c
> +++ b/drivers/mtd/nand/fsl_upm.c

[...]
> @@ -59,7 +60,9 @@ int of_get_gpio(struct device_node *np, int index)
>  		goto err1;
>  	}
>  
> -	ret = of_gc->xlate(of_gc, np, gpio_spec);
> +	if (flags)
> +		*flags = 0;
> +	ret = of_gc->xlate(of_gc, np, gpio_spec, flags);
>  	if (ret < 0)
>  		goto err1;
>  
> @@ -77,19 +80,23 @@ EXPORT_SYMBOL(of_get_gpio);
>   * @of_gc:	pointer to the of_gpio_chip structure
>   * @np:		device node of the GPIO chip
>   * @gpio_spec:	gpio specifier as found in the device tree
> + * @flags:	if non-NUll flags are returned here

NULL, not NUll.

>   *
>   * This is simple translation function, suitable for the most 1:1 mapped
>   * gpio chips. This function performs only one sanity check: whether gpio
>   * is less than ngpios (that is specified in the gpio_chip).
>   */
>  int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
> -			 const void *gpio_spec)
> +			 const void *gpio_spec, unsigned int *flags)

Why you made it unsigned int? In my original patch, I used
named enum, which is self-documenting type.

>  {
>  	const u32 *gpio = gpio_spec;
>  
>  	if (*gpio > of_gc->gc.ngpio)
>  		return -EINVAL;
>  
> +	if (flags && of_gc->gpio_cells > 1)
> +		*flags = gpio[1];
> +
>  	return *gpio;
>  }
>  EXPORT_SYMBOL(of_gpio_simple_xlate);
> diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c
> index bde4b4b..7835cd4 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -1094,7 +1094,7 @@ static int cpm_uart_init_port(struct device_node *np,
>  	}
>  
>  	for (i = 0; i < NUM_GPIOS; i++)
> -		pinfo->gpios[i] = of_get_gpio(np, i);
> +		pinfo->gpios[i] = of_get_gpio(np, i, NULL);
>  
>  	return cpm_uart_request_port(&pinfo->port);
>  
> diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
> index 67db101..0d332bf 100644
> --- a/include/linux/of_gpio.h
> +++ b/include/linux/of_gpio.h
> @@ -26,7 +26,7 @@ struct of_gpio_chip {
>  	struct gpio_chip gc;
>  	int gpio_cells;
>  	int (*xlate)(struct of_gpio_chip *of_gc, struct device_node *np,
> -		     const void *gpio_spec);
> +		     const void *gpio_spec, unsigned int *flags);
>  };
>  
>  static inline struct of_gpio_chip *to_of_gpio_chip(struct gpio_chip *gc)
> @@ -35,6 +35,14 @@ static inline struct of_gpio_chip *to_of_gpio_chip(struct gpio_chip *gc)
>  }
>  
>  /*
> + * Flags as returned by OF GPIO chip's xlate function.
> + * These do not need to be the same as the flags in the GPIO specifier in the
> + * OF device tree, but it's convenient if they are.  The mm chip OF GPIO
> + * driver works this way.

This is not of_mm_gpio_chip specific.

> + */
> +#define OF_GPIO_ACTIVE_LOW	1
> +
> +/*
>   * OF GPIO chip for memory mapped banks
>   */
>  struct of_mm_gpio_chip {
> @@ -50,16 +58,17 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc)
>  	return container_of(of_gc, struct of_mm_gpio_chip, of_gc);
>  }
>  
> -extern int of_get_gpio(struct device_node *np, int index);
> +extern int of_get_gpio(struct device_node *np, int index, unsigned int *flags);
>  extern int of_mm_gpiochip_add(struct device_node *np,
>  			      struct of_mm_gpio_chip *mm_gc);
>  extern int of_gpio_simple_xlate(struct of_gpio_chip *of_gc,
>  				struct device_node *np,
> -				const void *gpio_spec);
> +				const void *gpio_spec, unsigned int *flags);
>  #else
>  
>  /* Drivers may not strictly depend on the GPIO support, so let them link. */
> -static inline int of_get_gpio(struct device_node *np, int index)
> +static inline int of_get_gpio(struct device_node *np, int index,
> +			      unsigned int *flags)
>  {
>  	return -ENOSYS;
>  }
> -- 
> 1.5.4.3

Can you repost a fixed version with my Ack and Cc: Andrew Morton,
Benjamin Herrenschmidt?

I think this change should go into the 2.6.28, so that we can
write new code on top of new API. Otherwise this change will cause
issues in the next merge window.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

  parent reply	other threads:[~2008-10-28 14:39 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-24 23:04 OpenFirmware GPIO LED driver Trent Piepho
2008-10-24 23:08 ` [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio() Trent Piepho
2008-10-24 23:32   ` Grant Likely
2008-10-28 14:39   ` Anton Vorontsov [this message]
2008-10-28 14:53     ` Grant Likely
2008-10-28 15:16       ` Anton Vorontsov
2008-10-28 15:42         ` Grant Likely
2008-10-28 16:56           ` Anton Vorontsov
2008-10-28 17:40             ` Grant Likely
2008-10-30  2:21     ` [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()A Trent Piepho
2008-10-30 11:15       ` Anton Vorontsov
2008-10-31  2:03         ` [PATCH v2] of_gpio: Return GPIO flags from of_get_gpio() Trent Piepho
2008-11-26 16:20           ` Anton Vorontsov
2008-11-26 21:38             ` Paul Mackerras
2008-11-26 22:31               ` Anton Vorontsov
2008-11-26 22:35               ` Trent Piepho
2008-11-26 22:58                 ` Anton Vorontsov
2008-11-26 23:32                 ` Paul Mackerras
2008-10-24 23:08 ` [PATCH 2/4] leds: Support OpenFirmware led bindings Trent Piepho
2008-10-24 23:50   ` Grant Likely
2008-10-24 23:09 ` [PATCH 3/4] leds: Add option to have GPIO LEDs start on Trent Piepho
2008-10-24 23:59   ` Grant Likely
2008-10-24 23:09 ` [PATCH 4/4] leds: Let GPIO LEDs keep their current state Trent Piepho
2008-10-25  0:04   ` Grant Likely
2008-11-17 14:50   ` Richard Purdie
2008-11-21  1:05     ` Trent Piepho
2008-11-23 12:31       ` Pavel Machek
2008-12-03 10:04         ` Richard Purdie
2008-12-10  4:33           ` Trent Piepho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081028143933.GA18453@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=grant.likely@secretlab.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=rpurdie@rpsys.net \
    --cc=smaclennan@pikatech.com \
    --cc=tpiepho@freescale.com \
    --cc=w.sang@pengutronix.de \
    --subject='Re: [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).