All of lore.kernel.org
 help / color / mirror / Atom feed
From: grant.likely@secretlab.ca (Grant Likely)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 5/9] gpio: pxa: parse gpio from DTS file
Date: Tue, 08 May 2012 13:40:27 -0600	[thread overview]
Message-ID: <20120508194027.5DC3A3E03E5@localhost> (raw)
In-Reply-To: <1336134626-12262-6-git-send-email-haojian.zhuang@gmail.com>

On Fri,  4 May 2012 20:30:22 +0800, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> Parse GPIO numbers from DTS file. Allocate interrupt according to
> GPIO numbers.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> ---
>  drivers/gpio/gpio-pxa.c |  116 +++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 98 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
> index fc3ace3..58a6a63 100644
> --- a/drivers/gpio/gpio-pxa.c
> +++ b/drivers/gpio/gpio-pxa.c
> @@ -11,13 +11,17 @@
>   *  it under the terms of the GNU General Public License version 2 as
>   *  published by the Free Software Foundation.
>   */
> +#include <linux/module.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
>  #include <linux/gpio-pxa.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
> +#include <linux/irqdomain.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/syscore_ops.h>
>  #include <linux/slab.h>
> @@ -56,6 +60,10 @@
>  
>  int pxa_last_gpio;
>  
> +#ifdef CONFIG_OF
> +static struct irq_domain *domain;
> +#endif
> +
>  struct pxa_gpio_chip {
>  	struct gpio_chip chip;
>  	void __iomem	*regbase;
> @@ -81,7 +89,6 @@ enum {
>  	PXA3XX_GPIO,
>  	PXA93X_GPIO,
>  	MMP_GPIO = 0x10,
> -	MMP2_GPIO,
>  };
>  
>  static DEFINE_SPINLOCK(gpio_lock);
> @@ -475,22 +482,92 @@ static int pxa_gpio_nums(void)
>  		gpio_type = MMP_GPIO;
>  	} else if (cpu_is_mmp2()) {
>  		count = 191;
> -		gpio_type = MMP2_GPIO;
> +		gpio_type = MMP_GPIO;
>  	}
>  #endif /* CONFIG_ARCH_MMP */
>  	return count;
>  }
>  
> +static struct of_device_id pxa_gpio_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-gpio" },
> +	{ .compatible = "mrvl,mmp-gpio", .data = (void *)MMP_GPIO },
> +	{}
> +};
> +
> +static int pxa_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +			      irq_hw_number_t hw)
> +{
> +	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +				 handle_edge_irq);
> +	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +	return 0;
> +}
> +
> +const struct irq_domain_ops pxa_irq_domain_ops = {
> +	.map	= pxa_irq_domain_map,
> +};
> +
> +#ifdef CONFIG_OF
> +static int __devinit pxa_gpio_probe_dt(struct platform_device *pdev)
> +{
> +	int ret, nr_banks, nr_gpios, irq_base;
> +	struct device_node *prev, *next, *np = pdev->dev.of_node;
> +	const struct of_device_id *of_id =
> +				of_match_device(pxa_gpio_dt_ids, &pdev->dev);
> +
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "Failed to find gpio controller\n");
> +		return -EFAULT;
> +	}
> +	gpio_type = (int)of_id->data;
> +
> +	next = of_get_next_child(np, NULL);
> +	prev = next;
> +	if (!next) {
> +		dev_err(&pdev->dev, "Failed to find child gpio node\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +	for (nr_banks = 1; ; nr_banks++) {
> +		next = of_get_next_child(np, prev);
> +		if (!next)
> +			break;
> +		prev = next;
> +	}
> +	of_node_put(prev);
> +	nr_gpios = nr_banks << 5;
> +	pxa_last_gpio = nr_gpios - 1;
> +
> +	irq_base = irq_alloc_descs(-1, 0, nr_gpios, 0);
> +	if (irq_base < 0) {
> +		dev_err(&pdev->dev, "Failed to allocate IRQ numbers\n");
> +		goto err;
> +	}
> +	domain = irq_domain_add_legacy(np, nr_gpios, irq_base, 0,
> +				       &pxa_irq_domain_ops, NULL);
> +	return 0;

Instead of putting the irq_domain_add_legacy() call into the DT-only
path, the entire driver should be converted over to use an irq_domain.
That will keep the driver simpler overall.

Also, as part of the conversion to irq_domain the irq handling
functions and the *_gpio_to_irq functions need to be refactored to use
irq_find_mapping() instead of a hard coded offset.  Also, the hwirq
number (which should always equal the gpio number) is now available in
irq_data via irq->hwirq.

So, for example, pxa_gpio_irq_type does:
	int gpio = pxa_irq_to_gpio(d->irq);

but now it can simply be:
	int gpio = d->irq;

> +err:
> +	iounmap(gpio_reg_base);
> +	return ret;
> +}
> +#else
> +#define pxa_gpio_probe_dt(pdev)		(-1)
> +#endif
> +
>  static int __devinit pxa_gpio_probe(struct platform_device *pdev)
>  {
>  	struct pxa_gpio_chip *c;
>  	struct resource *res;
>  	struct clk *clk;
>  	struct pxa_gpio_platform_data *info;
> -	int gpio, irq, ret;
> +	int gpio, irq, ret, use_of = 0;
>  	int irq0 = 0, irq1 = 0, irq_mux, gpio_offset = 0;
>  
> -	pxa_last_gpio = pxa_gpio_nums();
> +	ret = pxa_gpio_probe_dt(pdev);
> +	if (ret < 0)
> +		pxa_last_gpio = pxa_gpio_nums();
> +	else
> +		use_of = 1;
>  	if (!pxa_last_gpio)
>  		return -EINVAL;
>  
> @@ -545,25 +622,27 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)
>  			writel_relaxed(~0, c->regbase + ED_MASK_OFFSET);
>  	}
>  
> +	if (!use_of) {
>  #ifdef CONFIG_ARCH_PXA
> -	irq = gpio_to_irq(0);
> -	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> -				 handle_edge_irq);
> -	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> -	irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler);
> -
> -	irq = gpio_to_irq(1);
> -	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> -				 handle_edge_irq);
> -	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> -	irq_set_chained_handler(IRQ_GPIO1, pxa_gpio_demux_handler);
> -#endif
> +		irq = gpio_to_irq(0);
> +		irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +					 handle_edge_irq);
> +		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler);
>  
> -	for (irq  = gpio_to_irq(gpio_offset);
> -		irq <= gpio_to_irq(pxa_last_gpio); irq++) {
> +		irq = gpio_to_irq(1);
>  		irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
>  					 handle_edge_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		irq_set_chained_handler(IRQ_GPIO1, pxa_gpio_demux_handler);
> +#endif

See?  With this patch the mapping mechanism is duplicated for DT and
non-DT.  If the legacy mapping was registered here, then there the
duplicate code path goes away.

> +
> +		for (irq  = gpio_to_irq(gpio_offset);
> +			irq <= gpio_to_irq(pxa_last_gpio); irq++) {
> +			irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +						 handle_edge_irq);
> +			set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		}
>  	}
>  
>  	irq_set_chained_handler(irq_mux, pxa_gpio_demux_handler);
> @@ -574,6 +653,7 @@ static struct platform_driver pxa_gpio_driver = {
>  	.probe		= pxa_gpio_probe,
>  	.driver		= {
>  		.name	= "pxa-gpio",
> +		.of_match_table = pxa_gpio_dt_ids,
>  	},
>  };
>  
> -- 
> 1.7.5.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: Haojian Zhuang
	<haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	arnd-r2nGTMty4D4@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Subject: Re: [PATCH v2 5/9] gpio: pxa: parse gpio from DTS file
Date: Tue, 08 May 2012 13:40:27 -0600	[thread overview]
Message-ID: <20120508194027.5DC3A3E03E5@localhost> (raw)
In-Reply-To: <1336134626-12262-6-git-send-email-haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri,  4 May 2012 20:30:22 +0800, Haojian Zhuang <haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Parse GPIO numbers from DTS file. Allocate interrupt according to
> GPIO numbers.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpio/gpio-pxa.c |  116 +++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 98 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
> index fc3ace3..58a6a63 100644
> --- a/drivers/gpio/gpio-pxa.c
> +++ b/drivers/gpio/gpio-pxa.c
> @@ -11,13 +11,17 @@
>   *  it under the terms of the GNU General Public License version 2 as
>   *  published by the Free Software Foundation.
>   */
> +#include <linux/module.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
>  #include <linux/gpio-pxa.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
> +#include <linux/irqdomain.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/syscore_ops.h>
>  #include <linux/slab.h>
> @@ -56,6 +60,10 @@
>  
>  int pxa_last_gpio;
>  
> +#ifdef CONFIG_OF
> +static struct irq_domain *domain;
> +#endif
> +
>  struct pxa_gpio_chip {
>  	struct gpio_chip chip;
>  	void __iomem	*regbase;
> @@ -81,7 +89,6 @@ enum {
>  	PXA3XX_GPIO,
>  	PXA93X_GPIO,
>  	MMP_GPIO = 0x10,
> -	MMP2_GPIO,
>  };
>  
>  static DEFINE_SPINLOCK(gpio_lock);
> @@ -475,22 +482,92 @@ static int pxa_gpio_nums(void)
>  		gpio_type = MMP_GPIO;
>  	} else if (cpu_is_mmp2()) {
>  		count = 191;
> -		gpio_type = MMP2_GPIO;
> +		gpio_type = MMP_GPIO;
>  	}
>  #endif /* CONFIG_ARCH_MMP */
>  	return count;
>  }
>  
> +static struct of_device_id pxa_gpio_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-gpio" },
> +	{ .compatible = "mrvl,mmp-gpio", .data = (void *)MMP_GPIO },
> +	{}
> +};
> +
> +static int pxa_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +			      irq_hw_number_t hw)
> +{
> +	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +				 handle_edge_irq);
> +	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +	return 0;
> +}
> +
> +const struct irq_domain_ops pxa_irq_domain_ops = {
> +	.map	= pxa_irq_domain_map,
> +};
> +
> +#ifdef CONFIG_OF
> +static int __devinit pxa_gpio_probe_dt(struct platform_device *pdev)
> +{
> +	int ret, nr_banks, nr_gpios, irq_base;
> +	struct device_node *prev, *next, *np = pdev->dev.of_node;
> +	const struct of_device_id *of_id =
> +				of_match_device(pxa_gpio_dt_ids, &pdev->dev);
> +
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "Failed to find gpio controller\n");
> +		return -EFAULT;
> +	}
> +	gpio_type = (int)of_id->data;
> +
> +	next = of_get_next_child(np, NULL);
> +	prev = next;
> +	if (!next) {
> +		dev_err(&pdev->dev, "Failed to find child gpio node\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +	for (nr_banks = 1; ; nr_banks++) {
> +		next = of_get_next_child(np, prev);
> +		if (!next)
> +			break;
> +		prev = next;
> +	}
> +	of_node_put(prev);
> +	nr_gpios = nr_banks << 5;
> +	pxa_last_gpio = nr_gpios - 1;
> +
> +	irq_base = irq_alloc_descs(-1, 0, nr_gpios, 0);
> +	if (irq_base < 0) {
> +		dev_err(&pdev->dev, "Failed to allocate IRQ numbers\n");
> +		goto err;
> +	}
> +	domain = irq_domain_add_legacy(np, nr_gpios, irq_base, 0,
> +				       &pxa_irq_domain_ops, NULL);
> +	return 0;

Instead of putting the irq_domain_add_legacy() call into the DT-only
path, the entire driver should be converted over to use an irq_domain.
That will keep the driver simpler overall.

Also, as part of the conversion to irq_domain the irq handling
functions and the *_gpio_to_irq functions need to be refactored to use
irq_find_mapping() instead of a hard coded offset.  Also, the hwirq
number (which should always equal the gpio number) is now available in
irq_data via irq->hwirq.

So, for example, pxa_gpio_irq_type does:
	int gpio = pxa_irq_to_gpio(d->irq);

but now it can simply be:
	int gpio = d->irq;

> +err:
> +	iounmap(gpio_reg_base);
> +	return ret;
> +}
> +#else
> +#define pxa_gpio_probe_dt(pdev)		(-1)
> +#endif
> +
>  static int __devinit pxa_gpio_probe(struct platform_device *pdev)
>  {
>  	struct pxa_gpio_chip *c;
>  	struct resource *res;
>  	struct clk *clk;
>  	struct pxa_gpio_platform_data *info;
> -	int gpio, irq, ret;
> +	int gpio, irq, ret, use_of = 0;
>  	int irq0 = 0, irq1 = 0, irq_mux, gpio_offset = 0;
>  
> -	pxa_last_gpio = pxa_gpio_nums();
> +	ret = pxa_gpio_probe_dt(pdev);
> +	if (ret < 0)
> +		pxa_last_gpio = pxa_gpio_nums();
> +	else
> +		use_of = 1;
>  	if (!pxa_last_gpio)
>  		return -EINVAL;
>  
> @@ -545,25 +622,27 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)
>  			writel_relaxed(~0, c->regbase + ED_MASK_OFFSET);
>  	}
>  
> +	if (!use_of) {
>  #ifdef CONFIG_ARCH_PXA
> -	irq = gpio_to_irq(0);
> -	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> -				 handle_edge_irq);
> -	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> -	irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler);
> -
> -	irq = gpio_to_irq(1);
> -	irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> -				 handle_edge_irq);
> -	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> -	irq_set_chained_handler(IRQ_GPIO1, pxa_gpio_demux_handler);
> -#endif
> +		irq = gpio_to_irq(0);
> +		irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +					 handle_edge_irq);
> +		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		irq_set_chained_handler(IRQ_GPIO0, pxa_gpio_demux_handler);
>  
> -	for (irq  = gpio_to_irq(gpio_offset);
> -		irq <= gpio_to_irq(pxa_last_gpio); irq++) {
> +		irq = gpio_to_irq(1);
>  		irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
>  					 handle_edge_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		irq_set_chained_handler(IRQ_GPIO1, pxa_gpio_demux_handler);
> +#endif

See?  With this patch the mapping mechanism is duplicated for DT and
non-DT.  If the legacy mapping was registered here, then there the
duplicate code path goes away.

> +
> +		for (irq  = gpio_to_irq(gpio_offset);
> +			irq <= gpio_to_irq(pxa_last_gpio); irq++) {
> +			irq_set_chip_and_handler(irq, &pxa_muxed_gpio_chip,
> +						 handle_edge_irq);
> +			set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> +		}
>  	}
>  
>  	irq_set_chained_handler(irq_mux, pxa_gpio_demux_handler);
> @@ -574,6 +653,7 @@ static struct platform_driver pxa_gpio_driver = {
>  	.probe		= pxa_gpio_probe,
>  	.driver		= {
>  		.name	= "pxa-gpio",
> +		.of_match_table = pxa_gpio_dt_ids,
>  	},
>  };
>  
> -- 
> 1.7.5.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

  reply	other threads:[~2012-05-08 19:40 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-04 12:30 [PATCH v2 0/9] support dt for mmp2 Haojian Zhuang
2012-05-04 12:30 ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 1/9] ARM: mmp: fix build issue on mmp with device tree Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 2/9] ARM: mmp: append CONFIG_MACH_MMP2_DT Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 3/9] ARM: mmp: support DT in irq Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-08 17:56   ` Grant Likely
2012-05-08 17:56     ` Grant Likely
2012-05-04 12:30 ` [PATCH v2 4/9] ARM: mmp: support DT in timer Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 5/9] gpio: pxa: parse gpio from DTS file Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-08 19:40   ` Grant Likely [this message]
2012-05-08 19:40     ` Grant Likely
2012-05-04 12:30 ` [PATCH v2 6/9] ARM: mmp: support mmp2 with device tree Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 7/9] ARM: mmp: support pxa910 " Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 12:30 ` [PATCH v2 8/9] ARM: dts: refresh dts file for arch mmp Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-06-05  0:08   ` Chris Ball
2012-06-05  0:08     ` Chris Ball
2012-06-05  2:03     ` Haojian Zhuang
2012-06-05  2:03       ` Haojian Zhuang
2012-06-06  1:28     ` Arnd Bergmann
2012-06-06  1:28       ` Arnd Bergmann
2012-06-06  1:47       ` Mitch Bradley
2012-06-06  1:47         ` Mitch Bradley
2012-06-06  2:35         ` Haojian Zhuang
2012-06-06  2:35           ` Haojian Zhuang
2012-06-06  5:22           ` Mitch Bradley
2012-06-06  5:22             ` Mitch Bradley
2012-06-06  5:25             ` Haojian Zhuang
2012-06-06  5:25               ` Haojian Zhuang
2012-06-06 16:22               ` Mitch Bradley
2012-06-06 16:22                 ` Mitch Bradley
2012-06-06  3:05         ` Arnd Bergmann
2012-06-06  3:05           ` Arnd Bergmann
2012-05-04 12:30 ` [PATCH v2 9/9] Documentation: update docs for mmp dt Haojian Zhuang
2012-05-04 12:30   ` Haojian Zhuang
2012-05-04 14:27 ` [PATCH v2 0/9] support dt for mmp2 Arnd Bergmann
2012-05-04 14:27   ` Arnd Bergmann
2012-05-17 23:57 ` Rob Herring
2012-05-17 23:57   ` Rob Herring
2012-05-18  2:15   ` Haojian Zhuang
2012-05-18  2:15     ` Haojian Zhuang

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=20120508194027.5DC3A3E03E5@localhost \
    --to=grant.likely@secretlab.ca \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.