[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
from Linus Walleij's patch too.

Barry Song (3):
  pinctrl: sirf: wrap all gpio banks into one gpio_chip
  gpio: make handler_data configurable while using gpiolib_irqchip
  pinctrl: sirf: move to use irq_get_handler_data
Linus Walleij (1):
  pinctrl: sirf: switch driver to use gpiolib irqchip helpers

 drivers/gpio/gpiolib.c              |   10 +-
 drivers/pinctrl/Kconfig             |    1 +
 drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
 include/linux/gpio/driver.h         |    2 +-
 4 files changed, 97 insertions(+), 180 deletions(-)

-- 
1.7.5.4

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
from Linus Walleij's patch too.

Barry Song (3):
  pinctrl: sirf: wrap all gpio banks into one gpio_chip
  gpio: make handler_data configurable while using gpiolib_irqchip
  pinctrl: sirf: move to use irq_get_handler_data
Linus Walleij (1):
  pinctrl: sirf: switch driver to use gpiolib irqchip helpers

 drivers/gpio/gpiolib.c              |   10 +-
 drivers/pinctrl/Kconfig             |    1 +
 drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
 include/linux/gpio/driver.h         |    2 +-
 4 files changed, 97 insertions(+), 180 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

all gpio banks are in one chip, that makes software clean in mapping
irq and gpio.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/pinctrl/sirf/pinctrl-sirf.c |  217 +++++++++++++++--------------------
 1 files changed, 91 insertions(+), 126 deletions(-)

diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index 2c3eb20..cb81b15 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -34,15 +34,19 @@
 #define DRIVER_NAME "pinmux-sirf"
 
 struct sirfsoc_gpio_bank {
-	struct of_mm_gpio_chip chip;
-	struct irq_domain *domain;
 	int id;
 	int parent_irq;
 	spinlock_t lock;
+};
+
+struct sirfsoc_gpio_chip {
+	struct of_mm_gpio_chip chip;
+	struct irq_domain *domain;
 	bool is_marco; /* for marco, some registers are different with prima2 */
+	struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
 };
 
-static struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
+static struct sirfsoc_gpio_chip sgpio_chip;
 static DEFINE_SPINLOCK(sgpio_lock);
 
 static struct sirfsoc_pin_group *sirfsoc_pin_groups;
@@ -258,32 +262,12 @@ static struct pinctrl_desc sirfsoc_pinmux_desc = {
 /*
  * Todo: bind irq_chip to every pinctrl_gpio_range
  */
-static struct pinctrl_gpio_range sirfsoc_gpio_ranges[] = {
-	{
-		.name = "sirfsoc-gpio*",
-		.id = 0,
-		.base = 0,
-		.pin_base = 0,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 1,
-		.base = 32,
-		.pin_base = 32,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 2,
-		.base = 64,
-		.pin_base = 64,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 3,
-		.base = 96,
-		.pin_base = 96,
-		.npins = 19,
-	},
+static struct pinctrl_gpio_range sirfsoc_gpio_ranges = {
+	.name = "sirfsoc-gpio*",
+	.id = 0,
+	.base = 0,
+	.pin_base = 0,
+	.npins = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
 };
 
 static void __iomem *sirfsoc_rsc_of_iomap(void)
@@ -303,19 +287,19 @@ static void __iomem *sirfsoc_rsc_of_iomap(void)
 }
 
 static int sirfsoc_gpio_of_xlate(struct gpio_chip *gc,
-       const struct of_phandle_args *gpiospec,
-       u32 *flags)
+	const struct of_phandle_args *gpiospec,
+	u32 *flags)
 {
-       if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE)
+	if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE)
 		return -EINVAL;
 
-       if (gc != &sgpio_bank[gpiospec->args[0] / SIRFSOC_GPIO_BANK_SIZE].chip.gc)
+	if (gc != &sgpio_chip.chip.gc)
 		return -EINVAL;
 
-       if (flags)
+	if (flags)
 		*flags = gpiospec->args[1];
 
-       return gpiospec->args[0] % SIRFSOC_GPIO_BANK_SIZE;
+	return gpiospec->args[0];
 }
 
 static const struct of_device_id pinmux_ids[] = {
@@ -331,7 +315,6 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev)
 	struct sirfsoc_pmx *spmx;
 	struct device_node *np = pdev->dev.of_node;
 	const struct sirfsoc_pinctrl_data *pdata;
-	int i;
 
 	/* Create state holders etc for this driver */
 	spmx = devm_kzalloc(&pdev->dev, sizeof(*spmx), GFP_KERNEL);
@@ -375,10 +358,8 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev)
 		goto out_no_pmx;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sirfsoc_gpio_ranges); i++) {
-		sirfsoc_gpio_ranges[i].gc = &sgpio_bank[i].chip.gc;
-		pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges[i]);
-	}
+	sirfsoc_gpio_ranges.gc = &sgpio_chip.chip.gc;
+	pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges);
 
 	dev_info(&pdev->dev, "initialized SIRFSOC pinmux driver\n");
 
@@ -464,33 +445,23 @@ static int __init sirfsoc_pinmux_init(void)
 }
 arch_initcall(sirfsoc_pinmux_init);
 
-static inline int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
 {
-	struct sirfsoc_gpio_bank *bank = container_of(to_of_mm_gpio_chip(chip),
-		struct sirfsoc_gpio_bank, chip);
-
-	return irq_create_mapping(bank->domain, offset + bank->id *
-		SIRFSOC_GPIO_BANK_SIZE);
+	return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
 }
 
-static inline int sirfsoc_gpio_to_offset(unsigned int gpio)
+static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
-	return gpio % SIRFSOC_GPIO_BANK_SIZE;
+	return irq_create_mapping(sgpio_chip.domain, offset);
 }
 
-static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
+static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio)
 {
-	return &sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
-}
-
-static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chip *chip)
-{
-	return container_of(to_of_mm_gpio_chip(chip), struct sirfsoc_gpio_bank, chip);
+	return gpio % SIRFSOC_GPIO_BANK_SIZE;
 }
-
 static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -499,9 +470,9 @@ static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
@@ -515,24 +486,24 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK;
 	val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK;
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
 
 static void sirfsoc_gpio_irq_mask(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 
 	__sirfsoc_gpio_irq_mask(bank, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
 }
 
 static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -541,17 +512,17 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK;
 	val |= SIRFSOC_GPIO_CTL_INTR_EN_MASK;
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
 
 static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -560,7 +531,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~(SIRFSOC_GPIO_CTL_INTR_STS_MASK | SIRFSOC_GPIO_CTL_OUT_EN_MASK);
 
 	switch (type) {
@@ -588,7 +559,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 		break;
 	}
 
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 
@@ -597,10 +568,8 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 
 static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
-	if (gpio_lock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE)) {
-		dev_err(bank->chip.gc.dev,
+	if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) {
+		dev_err(sgpio_chip.chip.gc.dev,
 			"unable to lock HW IRQ %lu for IRQ\n",
 			d->hwirq);
 		return -EINVAL;
@@ -610,9 +579,7 @@ static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
 
 static void sirfsoc_gpio_irq_relres(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
-	gpio_unlock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
+	gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq);
 }
 
 static struct irq_chip sirfsoc_irq_chip = {
@@ -634,7 +601,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 
 	chained_irq_enter(chip, desc);
 
-	status = readl(bank->chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id));
+	status = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id));
 	if (!status) {
 		printk(KERN_WARNING
 			"%s: gpio id %d status %#x no interrupt is flaged\n",
@@ -644,7 +611,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 	}
 
 	while (status) {
-		ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx));
+		ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx));
 
 		/*
 		 * Here we must check whether the corresponding GPIO's interrupt
@@ -653,7 +620,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 		if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
 			pr_debug("%s: gpio id %d idx %d happens\n",
 				__func__, bank->id, idx);
-			generic_handle_irq(irq_find_mapping(bank->domain, idx +
+			generic_handle_irq(irq_find_mapping(sgpio_chip.domain, idx +
 					bank->id * SIRFSOC_GPIO_BANK_SIZE));
 		}
 
@@ -668,14 +635,14 @@ static inline void sirfsoc_gpio_set_input(struct sirfsoc_gpio_bank *bank, unsign
 {
 	u32 val;
 
-	val = readl(bank->chip.regs + ctrl_offset);
+	val = readl(sgpio_chip.chip.regs + ctrl_offset);
 	val &= ~SIRFSOC_GPIO_CTL_OUT_EN_MASK;
-	writel(val, bank->chip.regs + ctrl_offset);
+	writel(val, sgpio_chip.chip.regs + ctrl_offset);
 }
 
 static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	unsigned long flags;
 
 	if (pinctrl_request_gpio(chip->base + offset))
@@ -697,7 +664,7 @@ static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
@@ -712,8 +679,8 @@ static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 static int sirfsoc_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
-	int idx = sirfsoc_gpio_to_offset(gpio);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio);
+	int idx = sirfsoc_gpio_to_bankoff(gpio);
 	unsigned long flags;
 	unsigned offset;
 
@@ -736,7 +703,7 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	out_ctrl = readl(bank->chip.regs + offset);
+	out_ctrl = readl(sgpio_chip.chip.regs + offset);
 	if (value)
 		out_ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK;
 	else
@@ -744,15 +711,15 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig
 
 	out_ctrl &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK;
 	out_ctrl |= SIRFSOC_GPIO_CTL_OUT_EN_MASK;
-	writel(out_ctrl, bank->chip.regs + offset);
+	writel(out_ctrl, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int value)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
-	int idx = sirfsoc_gpio_to_offset(gpio);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio);
+	int idx = sirfsoc_gpio_to_bankoff(gpio);
 	u32 offset;
 	unsigned long flags;
 
@@ -769,13 +736,13 @@ static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio,
 
 static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	u32 val;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	val = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	val = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 
@@ -785,18 +752,18 @@ static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset)
 static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
 	int value)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	u32 ctrl;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 	if (value)
 		ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK;
 	else
 		ctrl &= ~SIRFSOC_GPIO_CTL_DATAOUT_MASK;
-	writel(ctrl, bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	writel(ctrl, sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
@@ -811,7 +778,6 @@ static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 
 	irq_set_chip(irq, &sirfsoc_irq_chip);
 	irq_set_handler(irq, handle_level_irq);
-	irq_set_chip_data(irq, bank + hwirq / SIRFSOC_GPIO_BANK_SIZE);
 	set_irq_flags(irq, IRQF_VALID);
 
 	return 0;
@@ -830,10 +796,10 @@ static void sirfsoc_gpio_set_pullup(const u32 *pullups)
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		for_each_set_bit(n, p + i, BITS_PER_LONG) {
 			u32 offset = SIRFSOC_GPIO_CTRL(i, n);
-			u32 val = readl(sgpio_bank[i].chip.regs + offset);
+			u32 val = readl(sgpio_chip.chip.regs + offset);
 			val |= SIRFSOC_GPIO_CTL_PULL_MASK;
 			val |= SIRFSOC_GPIO_CTL_PULL_HIGH;
-			writel(val, sgpio_bank[i].chip.regs + offset);
+			writel(val, sgpio_chip.chip.regs + offset);
 		}
 	}
 }
@@ -846,10 +812,10 @@ static void sirfsoc_gpio_set_pulldown(const u32 *pulldowns)
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		for_each_set_bit(n, p + i, BITS_PER_LONG) {
 			u32 offset = SIRFSOC_GPIO_CTRL(i, n);
-			u32 val = readl(sgpio_bank[i].chip.regs + offset);
+			u32 val = readl(sgpio_chip.chip.regs + offset);
 			val |= SIRFSOC_GPIO_CTL_PULL_MASK;
 			val &= ~SIRFSOC_GPIO_CTL_PULL_HIGH;
-			writel(val, sgpio_bank[i].chip.regs + offset);
+			writel(val, sgpio_chip.chip.regs + offset);
 		}
 	}
 }
@@ -877,48 +843,47 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 		is_marco = 1;
 
 	domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
-		&sirfsoc_gpio_irq_simple_ops, sgpio_bank);
+		&sirfsoc_gpio_irq_simple_ops, &sgpio_chip);
 	if (!domain) {
 		pr_err("%s: Failed to create irqdomain\n", np->full_name);
 			err = -ENOSYS;
 		goto out;
 	}
 
+	sgpio_chip.chip.gc.request = sirfsoc_gpio_request;
+	sgpio_chip.chip.gc.free = sirfsoc_gpio_free;
+	sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input;
+	sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value;
+	sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output;
+	sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value;
+	sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq;
+	sgpio_chip.chip.gc.base = 0;
+	sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS;
+	sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
+	sgpio_chip.chip.gc.of_node = np;
+	sgpio_chip.chip.gc.of_xlate = sirfsoc_gpio_of_xlate;
+	sgpio_chip.chip.gc.of_gpio_n_cells = 2;
+	sgpio_chip.chip.gc.dev = &pdev->dev;
+	sgpio_chip.chip.regs = regs;
+	sgpio_chip.is_marco = is_marco;
+	sgpio_chip.domain = domain;
+
+	err = gpiochip_add(&sgpio_chip.chip.gc);
+	if (err) {
+		pr_err("%s: error in probe function with status %d\n",
+			np->full_name, err);
+		goto out;
+	}
+
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
-		bank = &sgpio_bank[i];
+		bank = &sgpio_chip.sgpio_bank[i];
 		spin_lock_init(&bank->lock);
-		bank->chip.gc.request = sirfsoc_gpio_request;
-		bank->chip.gc.free = sirfsoc_gpio_free;
-		bank->chip.gc.direction_input = sirfsoc_gpio_direction_input;
-		bank->chip.gc.get = sirfsoc_gpio_get_value;
-		bank->chip.gc.direction_output = sirfsoc_gpio_direction_output;
-		bank->chip.gc.set = sirfsoc_gpio_set_value;
-		bank->chip.gc.to_irq = sirfsoc_gpio_to_irq;
-		bank->chip.gc.base = i * SIRFSOC_GPIO_BANK_SIZE;
-		bank->chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE;
-		bank->chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
-		bank->chip.gc.of_node = np;
-		bank->chip.gc.of_xlate = sirfsoc_gpio_of_xlate;
-		bank->chip.gc.of_gpio_n_cells = 2;
-		bank->chip.gc.dev = &pdev->dev;
-		bank->chip.regs = regs;
-		bank->id = i;
-		bank->is_marco = is_marco;
 		bank->parent_irq = platform_get_irq(pdev, i);
 		if (bank->parent_irq < 0) {
 			err = bank->parent_irq;
 			goto out;
 		}
 
-		err = gpiochip_add(&bank->chip.gc);
-		if (err) {
-			pr_err("%s: error in probe function with status %d\n",
-				np->full_name, err);
-			goto out;
-		}
-
-		bank->domain = domain;
-
 		irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
 		irq_set_handler_data(bank->parent_irq, bank);
 	}
-- 
1.7.5.4

[PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

all gpio banks are in one chip, that makes software clean in mapping
irq and gpio.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/pinctrl/sirf/pinctrl-sirf.c |  217 +++++++++++++++--------------------
 1 files changed, 91 insertions(+), 126 deletions(-)

diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index 2c3eb20..cb81b15 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -34,15 +34,19 @@
 #define DRIVER_NAME "pinmux-sirf"
 
 struct sirfsoc_gpio_bank {
-	struct of_mm_gpio_chip chip;
-	struct irq_domain *domain;
 	int id;
 	int parent_irq;
 	spinlock_t lock;
+};
+
+struct sirfsoc_gpio_chip {
+	struct of_mm_gpio_chip chip;
+	struct irq_domain *domain;
 	bool is_marco; /* for marco, some registers are different with prima2 */
+	struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
 };
 
-static struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
+static struct sirfsoc_gpio_chip sgpio_chip;
 static DEFINE_SPINLOCK(sgpio_lock);
 
 static struct sirfsoc_pin_group *sirfsoc_pin_groups;
@@ -258,32 +262,12 @@ static struct pinctrl_desc sirfsoc_pinmux_desc = {
 /*
  * Todo: bind irq_chip to every pinctrl_gpio_range
  */
-static struct pinctrl_gpio_range sirfsoc_gpio_ranges[] = {
-	{
-		.name = "sirfsoc-gpio*",
-		.id = 0,
-		.base = 0,
-		.pin_base = 0,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 1,
-		.base = 32,
-		.pin_base = 32,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 2,
-		.base = 64,
-		.pin_base = 64,
-		.npins = 32,
-	}, {
-		.name = "sirfsoc-gpio*",
-		.id = 3,
-		.base = 96,
-		.pin_base = 96,
-		.npins = 19,
-	},
+static struct pinctrl_gpio_range sirfsoc_gpio_ranges = {
+	.name = "sirfsoc-gpio*",
+	.id = 0,
+	.base = 0,
+	.pin_base = 0,
+	.npins = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
 };
 
 static void __iomem *sirfsoc_rsc_of_iomap(void)
@@ -303,19 +287,19 @@ static void __iomem *sirfsoc_rsc_of_iomap(void)
 }
 
 static int sirfsoc_gpio_of_xlate(struct gpio_chip *gc,
-       const struct of_phandle_args *gpiospec,
-       u32 *flags)
+	const struct of_phandle_args *gpiospec,
+	u32 *flags)
 {
-       if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE)
+	if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE)
 		return -EINVAL;
 
-       if (gc != &sgpio_bank[gpiospec->args[0] / SIRFSOC_GPIO_BANK_SIZE].chip.gc)
+	if (gc != &sgpio_chip.chip.gc)
 		return -EINVAL;
 
-       if (flags)
+	if (flags)
 		*flags = gpiospec->args[1];
 
-       return gpiospec->args[0] % SIRFSOC_GPIO_BANK_SIZE;
+	return gpiospec->args[0];
 }
 
 static const struct of_device_id pinmux_ids[] = {
@@ -331,7 +315,6 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev)
 	struct sirfsoc_pmx *spmx;
 	struct device_node *np = pdev->dev.of_node;
 	const struct sirfsoc_pinctrl_data *pdata;
-	int i;
 
 	/* Create state holders etc for this driver */
 	spmx = devm_kzalloc(&pdev->dev, sizeof(*spmx), GFP_KERNEL);
@@ -375,10 +358,8 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev)
 		goto out_no_pmx;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sirfsoc_gpio_ranges); i++) {
-		sirfsoc_gpio_ranges[i].gc = &sgpio_bank[i].chip.gc;
-		pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges[i]);
-	}
+	sirfsoc_gpio_ranges.gc = &sgpio_chip.chip.gc;
+	pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges);
 
 	dev_info(&pdev->dev, "initialized SIRFSOC pinmux driver\n");
 
@@ -464,33 +445,23 @@ static int __init sirfsoc_pinmux_init(void)
 }
 arch_initcall(sirfsoc_pinmux_init);
 
-static inline int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
 {
-	struct sirfsoc_gpio_bank *bank = container_of(to_of_mm_gpio_chip(chip),
-		struct sirfsoc_gpio_bank, chip);
-
-	return irq_create_mapping(bank->domain, offset + bank->id *
-		SIRFSOC_GPIO_BANK_SIZE);
+	return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
 }
 
-static inline int sirfsoc_gpio_to_offset(unsigned int gpio)
+static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
-	return gpio % SIRFSOC_GPIO_BANK_SIZE;
+	return irq_create_mapping(sgpio_chip.domain, offset);
 }
 
-static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
+static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio)
 {
-	return &sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
-}
-
-static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chip *chip)
-{
-	return container_of(to_of_mm_gpio_chip(chip), struct sirfsoc_gpio_bank, chip);
+	return gpio % SIRFSOC_GPIO_BANK_SIZE;
 }
-
 static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -499,9 +470,9 @@ static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
@@ -515,24 +486,24 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK;
 	val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK;
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
 
 static void sirfsoc_gpio_irq_mask(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 
 	__sirfsoc_gpio_irq_mask(bank, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
 }
 
 static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -541,17 +512,17 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK;
 	val |= SIRFSOC_GPIO_CTL_INTR_EN_MASK;
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 }
 
 static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
 	int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
 	u32 val, offset;
 	unsigned long flags;
@@ -560,7 +531,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 
 	spin_lock_irqsave(&sgpio_lock, flags);
 
-	val = readl(bank->chip.regs + offset);
+	val = readl(sgpio_chip.chip.regs + offset);
 	val &= ~(SIRFSOC_GPIO_CTL_INTR_STS_MASK | SIRFSOC_GPIO_CTL_OUT_EN_MASK);
 
 	switch (type) {
@@ -588,7 +559,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 		break;
 	}
 
-	writel(val, bank->chip.regs + offset);
+	writel(val, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&sgpio_lock, flags);
 
@@ -597,10 +568,8 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 
 static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
-	if (gpio_lock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE)) {
-		dev_err(bank->chip.gc.dev,
+	if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) {
+		dev_err(sgpio_chip.chip.gc.dev,
 			"unable to lock HW IRQ %lu for IRQ\n",
 			d->hwirq);
 		return -EINVAL;
@@ -610,9 +579,7 @@ static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
 
 static void sirfsoc_gpio_irq_relres(struct irq_data *d)
 {
-	struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
-	gpio_unlock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
+	gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq);
 }
 
 static struct irq_chip sirfsoc_irq_chip = {
@@ -634,7 +601,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 
 	chained_irq_enter(chip, desc);
 
-	status = readl(bank->chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id));
+	status = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id));
 	if (!status) {
 		printk(KERN_WARNING
 			"%s: gpio id %d status %#x no interrupt is flaged\n",
@@ -644,7 +611,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 	}
 
 	while (status) {
-		ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx));
+		ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx));
 
 		/*
 		 * Here we must check whether the corresponding GPIO's interrupt
@@ -653,7 +620,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 		if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
 			pr_debug("%s: gpio id %d idx %d happens\n",
 				__func__, bank->id, idx);
-			generic_handle_irq(irq_find_mapping(bank->domain, idx +
+			generic_handle_irq(irq_find_mapping(sgpio_chip.domain, idx +
 					bank->id * SIRFSOC_GPIO_BANK_SIZE));
 		}
 
@@ -668,14 +635,14 @@ static inline void sirfsoc_gpio_set_input(struct sirfsoc_gpio_bank *bank, unsign
 {
 	u32 val;
 
-	val = readl(bank->chip.regs + ctrl_offset);
+	val = readl(sgpio_chip.chip.regs + ctrl_offset);
 	val &= ~SIRFSOC_GPIO_CTL_OUT_EN_MASK;
-	writel(val, bank->chip.regs + ctrl_offset);
+	writel(val, sgpio_chip.chip.regs + ctrl_offset);
 }
 
 static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	unsigned long flags;
 
 	if (pinctrl_request_gpio(chip->base + offset))
@@ -697,7 +664,7 @@ static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
@@ -712,8 +679,8 @@ static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 static int sirfsoc_gpio_direction_input(struct gpio_chip *chip, unsigned gpio)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
-	int idx = sirfsoc_gpio_to_offset(gpio);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio);
+	int idx = sirfsoc_gpio_to_bankoff(gpio);
 	unsigned long flags;
 	unsigned offset;
 
@@ -736,7 +703,7 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	out_ctrl = readl(bank->chip.regs + offset);
+	out_ctrl = readl(sgpio_chip.chip.regs + offset);
 	if (value)
 		out_ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK;
 	else
@@ -744,15 +711,15 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig
 
 	out_ctrl &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK;
 	out_ctrl |= SIRFSOC_GPIO_CTL_OUT_EN_MASK;
-	writel(out_ctrl, bank->chip.regs + offset);
+	writel(out_ctrl, sgpio_chip.chip.regs + offset);
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int value)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
-	int idx = sirfsoc_gpio_to_offset(gpio);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio);
+	int idx = sirfsoc_gpio_to_bankoff(gpio);
 	u32 offset;
 	unsigned long flags;
 
@@ -769,13 +736,13 @@ static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio,
 
 static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	u32 val;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	val = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	val = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 
@@ -785,18 +752,18 @@ static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset)
 static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
 	int value)
 {
-	struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip);
+	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset);
 	u32 ctrl;
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 	if (value)
 		ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK;
 	else
 		ctrl &= ~SIRFSOC_GPIO_CTL_DATAOUT_MASK;
-	writel(ctrl, bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
+	writel(ctrl, sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset));
 
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
@@ -811,7 +778,6 @@ static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
 
 	irq_set_chip(irq, &sirfsoc_irq_chip);
 	irq_set_handler(irq, handle_level_irq);
-	irq_set_chip_data(irq, bank + hwirq / SIRFSOC_GPIO_BANK_SIZE);
 	set_irq_flags(irq, IRQF_VALID);
 
 	return 0;
@@ -830,10 +796,10 @@ static void sirfsoc_gpio_set_pullup(const u32 *pullups)
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		for_each_set_bit(n, p + i, BITS_PER_LONG) {
 			u32 offset = SIRFSOC_GPIO_CTRL(i, n);
-			u32 val = readl(sgpio_bank[i].chip.regs + offset);
+			u32 val = readl(sgpio_chip.chip.regs + offset);
 			val |= SIRFSOC_GPIO_CTL_PULL_MASK;
 			val |= SIRFSOC_GPIO_CTL_PULL_HIGH;
-			writel(val, sgpio_bank[i].chip.regs + offset);
+			writel(val, sgpio_chip.chip.regs + offset);
 		}
 	}
 }
@@ -846,10 +812,10 @@ static void sirfsoc_gpio_set_pulldown(const u32 *pulldowns)
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		for_each_set_bit(n, p + i, BITS_PER_LONG) {
 			u32 offset = SIRFSOC_GPIO_CTRL(i, n);
-			u32 val = readl(sgpio_bank[i].chip.regs + offset);
+			u32 val = readl(sgpio_chip.chip.regs + offset);
 			val |= SIRFSOC_GPIO_CTL_PULL_MASK;
 			val &= ~SIRFSOC_GPIO_CTL_PULL_HIGH;
-			writel(val, sgpio_bank[i].chip.regs + offset);
+			writel(val, sgpio_chip.chip.regs + offset);
 		}
 	}
 }
@@ -877,48 +843,47 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 		is_marco = 1;
 
 	domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
-		&sirfsoc_gpio_irq_simple_ops, sgpio_bank);
+		&sirfsoc_gpio_irq_simple_ops, &sgpio_chip);
 	if (!domain) {
 		pr_err("%s: Failed to create irqdomain\n", np->full_name);
 			err = -ENOSYS;
 		goto out;
 	}
 
+	sgpio_chip.chip.gc.request = sirfsoc_gpio_request;
+	sgpio_chip.chip.gc.free = sirfsoc_gpio_free;
+	sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input;
+	sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value;
+	sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output;
+	sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value;
+	sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq;
+	sgpio_chip.chip.gc.base = 0;
+	sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS;
+	sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
+	sgpio_chip.chip.gc.of_node = np;
+	sgpio_chip.chip.gc.of_xlate = sirfsoc_gpio_of_xlate;
+	sgpio_chip.chip.gc.of_gpio_n_cells = 2;
+	sgpio_chip.chip.gc.dev = &pdev->dev;
+	sgpio_chip.chip.regs = regs;
+	sgpio_chip.is_marco = is_marco;
+	sgpio_chip.domain = domain;
+
+	err = gpiochip_add(&sgpio_chip.chip.gc);
+	if (err) {
+		pr_err("%s: error in probe function with status %d\n",
+			np->full_name, err);
+		goto out;
+	}
+
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
-		bank = &sgpio_bank[i];
+		bank = &sgpio_chip.sgpio_bank[i];
 		spin_lock_init(&bank->lock);
-		bank->chip.gc.request = sirfsoc_gpio_request;
-		bank->chip.gc.free = sirfsoc_gpio_free;
-		bank->chip.gc.direction_input = sirfsoc_gpio_direction_input;
-		bank->chip.gc.get = sirfsoc_gpio_get_value;
-		bank->chip.gc.direction_output = sirfsoc_gpio_direction_output;
-		bank->chip.gc.set = sirfsoc_gpio_set_value;
-		bank->chip.gc.to_irq = sirfsoc_gpio_to_irq;
-		bank->chip.gc.base = i * SIRFSOC_GPIO_BANK_SIZE;
-		bank->chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE;
-		bank->chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
-		bank->chip.gc.of_node = np;
-		bank->chip.gc.of_xlate = sirfsoc_gpio_of_xlate;
-		bank->chip.gc.of_gpio_n_cells = 2;
-		bank->chip.gc.dev = &pdev->dev;
-		bank->chip.regs = regs;
-		bank->id = i;
-		bank->is_marco = is_marco;
 		bank->parent_irq = platform_get_irq(pdev, i);
 		if (bank->parent_irq < 0) {
 			err = bank->parent_irq;
 			goto out;
 		}
 
-		err = gpiochip_add(&bank->chip.gc);
-		if (err) {
-			pr_err("%s: error in probe function with status %d\n",
-				np->full_name, err);
-			goto out;
-		}
-
-		bank->domain = domain;
-
 		irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
 		irq_set_handler_data(bank->parent_irq, bank);
 	}
-- 
1.7.5.4

[PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers

[Barry Song]

From: Linus Walleij <linus.walleij@linaro.org>

This switches the SiRF pinctrl driver over to using the gpiolib
irqchip helpers simplifying some of the code.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 came from Linus Walleij's patch.

 drivers/pinctrl/Kconfig             |    1 +
 drivers/pinctrl/sirf/pinctrl-sirf.c |   89 ++++++++++------------------------
 2 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index e493240..ddd4e6e 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -270,6 +270,7 @@ config PINCTRL_SIRF
 	bool "CSR SiRFprimaII/SiRFmarco pin controller driver"
 	depends on ARCH_SIRF
 	select PINMUX
+	select GPIOLIB_IRQCHIP
 
 config PINCTRL_SUNXI
 	bool
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index cb81b15..c03dcc7 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -14,8 +14,6 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <linux/irqdomain.h>
-#include <linux/irqchip/chained_irq.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/pinctrl/consumer.h>
@@ -27,7 +25,6 @@
 #include <linux/bitops.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
-#include <asm/mach/irq.h>
 
 #include "pinctrl-sirf.h"
 
@@ -41,7 +38,6 @@ struct sirfsoc_gpio_bank {
 
 struct sirfsoc_gpio_chip {
 	struct of_mm_gpio_chip chip;
-	struct irq_domain *domain;
 	bool is_marco; /* for marco, some registers are different with prima2 */
 	struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
 };
@@ -450,15 +446,11 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
 	return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
 }
 
-static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
-	return irq_create_mapping(sgpio_chip.domain, offset);
-}
-
 static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio)
 {
 	return gpio % SIRFSOC_GPIO_BANK_SIZE;
 }
+
 static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 {
 	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
@@ -566,38 +558,28 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 	return 0;
 }
 
-static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
-{
-	if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) {
-		dev_err(sgpio_chip.chip.gc.dev,
-			"unable to lock HW IRQ %lu for IRQ\n",
-			d->hwirq);
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static void sirfsoc_gpio_irq_relres(struct irq_data *d)
-{
-	gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq);
-}
-
 static struct irq_chip sirfsoc_irq_chip = {
 	.name = "sirf-gpio-irq",
 	.irq_ack = sirfsoc_gpio_irq_ack,
 	.irq_mask = sirfsoc_gpio_irq_mask,
 	.irq_unmask = sirfsoc_gpio_irq_unmask,
 	.irq_set_type = sirfsoc_gpio_irq_type,
-	.irq_request_resources = sirfsoc_gpio_irq_reqres,
-	.irq_release_resources = sirfsoc_gpio_irq_relres,
 };
 
 static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 {
-	struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
+	struct sirfsoc_gpio_bank *bank;
 	u32 status, ctrl;
 	int idx = 0;
 	struct irq_chip *chip = irq_get_chip(irq);
+	int i;
+
+	for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
+		bank = &sgpio_chip.sgpio_bank[i];
+		if (bank->parent_irq == irq)
+			break;
+	}
+	BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
 
 	chained_irq_enter(chip, desc);
 
@@ -620,7 +602,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 		if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
 			pr_debug("%s: gpio id %d idx %d happens\n",
 				__func__, bank->id, idx);
-			generic_handle_irq(irq_find_mapping(sgpio_chip.domain, idx +
+			generic_handle_irq(irq_find_mapping(sgpio_chip.chip.gc.irqdomain, idx +
 					bank->id * SIRFSOC_GPIO_BANK_SIZE));
 		}
 
@@ -768,26 +750,6 @@ static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
-				irq_hw_number_t hwirq)
-{
-	struct sirfsoc_gpio_bank *bank = d->host_data;
-
-	if (!bank)
-		return -EINVAL;
-
-	irq_set_chip(irq, &sirfsoc_irq_chip);
-	irq_set_handler(irq, handle_level_irq);
-	set_irq_flags(irq, IRQF_VALID);
-
-	return 0;
-}
-
-static const struct irq_domain_ops sirfsoc_gpio_irq_simple_ops = {
-	.map = sirfsoc_gpio_irq_map,
-	.xlate = irq_domain_xlate_twocell,
-};
-
 static void sirfsoc_gpio_set_pullup(const u32 *pullups)
 {
 	int i, n;
@@ -826,7 +788,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	struct sirfsoc_gpio_bank *bank;
 	void __iomem *regs;
 	struct platform_device *pdev;
-	struct irq_domain *domain;
 	bool is_marco = false;
 
 	u32 pullups[SIRFSOC_GPIO_NO_OF_BANKS], pulldowns[SIRFSOC_GPIO_NO_OF_BANKS];
@@ -842,21 +803,12 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	if (of_device_is_compatible(np, "sirf,marco-pinctrl"))
 		is_marco = 1;
 
-	domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
-		&sirfsoc_gpio_irq_simple_ops, &sgpio_chip);
-	if (!domain) {
-		pr_err("%s: Failed to create irqdomain\n", np->full_name);
-			err = -ENOSYS;
-		goto out;
-	}
-
 	sgpio_chip.chip.gc.request = sirfsoc_gpio_request;
 	sgpio_chip.chip.gc.free = sirfsoc_gpio_free;
 	sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input;
 	sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value;
 	sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output;
 	sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value;
-	sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq;
 	sgpio_chip.chip.gc.base = 0;
 	sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS;
 	sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
@@ -866,15 +818,24 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	sgpio_chip.chip.gc.dev = &pdev->dev;
 	sgpio_chip.chip.regs = regs;
 	sgpio_chip.is_marco = is_marco;
-	sgpio_chip.domain = domain;
 
 	err = gpiochip_add(&sgpio_chip.chip.gc);
 	if (err) {
-		pr_err("%s: error in probe function with status %d\n",
+		dev_err(&pdev->dev, "%s: error in probe function with status %d\n",
 			np->full_name, err);
 		goto out;
 	}
 
+	err =  gpiochip_irqchip_add(&sgpio_chip.chip.gc,
+		&sirfsoc_irq_chip,
+		0, handle_level_irq,
+		IRQ_TYPE_NONE);
+	if (err) {
+		dev_err(&pdev->dev,
+			"could not connect irqchip to gpiochip\n");
+		goto out;
+	}
+
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		bank = &sgpio_chip.sgpio_bank[i];
 		spin_lock_init(&bank->lock);
@@ -884,8 +845,10 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 			goto out;
 		}
 
-		irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
-		irq_set_handler_data(bank->parent_irq, bank);
+		gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc,
+			&sirfsoc_irq_chip,
+			bank->parent_irq,
+			sirfsoc_gpio_handle_irq);
 	}
 
 	if (!of_property_read_u32_array(np, "sirf,pullups", pullups,
-- 
1.7.5.4

[PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers

[Barry Song]

From: Linus Walleij <linus.walleij@linaro.org>

This switches the SiRF pinctrl driver over to using the gpiolib
irqchip helpers simplifying some of the code.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 came from Linus Walleij's patch.

 drivers/pinctrl/Kconfig             |    1 +
 drivers/pinctrl/sirf/pinctrl-sirf.c |   89 ++++++++++------------------------
 2 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index e493240..ddd4e6e 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -270,6 +270,7 @@ config PINCTRL_SIRF
 	bool "CSR SiRFprimaII/SiRFmarco pin controller driver"
 	depends on ARCH_SIRF
 	select PINMUX
+	select GPIOLIB_IRQCHIP
 
 config PINCTRL_SUNXI
 	bool
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index cb81b15..c03dcc7 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -14,8 +14,6 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <linux/irqdomain.h>
-#include <linux/irqchip/chained_irq.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/pinctrl/consumer.h>
@@ -27,7 +25,6 @@
 #include <linux/bitops.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
-#include <asm/mach/irq.h>
 
 #include "pinctrl-sirf.h"
 
@@ -41,7 +38,6 @@ struct sirfsoc_gpio_bank {
 
 struct sirfsoc_gpio_chip {
 	struct of_mm_gpio_chip chip;
-	struct irq_domain *domain;
 	bool is_marco; /* for marco, some registers are different with prima2 */
 	struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS];
 };
@@ -450,15 +446,11 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio)
 	return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE];
 }
 
-static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
-	return irq_create_mapping(sgpio_chip.domain, offset);
-}
-
 static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio)
 {
 	return gpio % SIRFSOC_GPIO_BANK_SIZE;
 }
+
 static void sirfsoc_gpio_irq_ack(struct irq_data *d)
 {
 	struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq);
@@ -566,38 +558,28 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
 	return 0;
 }
 
-static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
-{
-	if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) {
-		dev_err(sgpio_chip.chip.gc.dev,
-			"unable to lock HW IRQ %lu for IRQ\n",
-			d->hwirq);
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static void sirfsoc_gpio_irq_relres(struct irq_data *d)
-{
-	gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq);
-}
-
 static struct irq_chip sirfsoc_irq_chip = {
 	.name = "sirf-gpio-irq",
 	.irq_ack = sirfsoc_gpio_irq_ack,
 	.irq_mask = sirfsoc_gpio_irq_mask,
 	.irq_unmask = sirfsoc_gpio_irq_unmask,
 	.irq_set_type = sirfsoc_gpio_irq_type,
-	.irq_request_resources = sirfsoc_gpio_irq_reqres,
-	.irq_release_resources = sirfsoc_gpio_irq_relres,
 };
 
 static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 {
-	struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
+	struct sirfsoc_gpio_bank *bank;
 	u32 status, ctrl;
 	int idx = 0;
 	struct irq_chip *chip = irq_get_chip(irq);
+	int i;
+
+	for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
+		bank = &sgpio_chip.sgpio_bank[i];
+		if (bank->parent_irq == irq)
+			break;
+	}
+	BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
 
 	chained_irq_enter(chip, desc);
 
@@ -620,7 +602,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 		if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
 			pr_debug("%s: gpio id %d idx %d happens\n",
 				__func__, bank->id, idx);
-			generic_handle_irq(irq_find_mapping(sgpio_chip.domain, idx +
+			generic_handle_irq(irq_find_mapping(sgpio_chip.chip.gc.irqdomain, idx +
 					bank->id * SIRFSOC_GPIO_BANK_SIZE));
 		}
 
@@ -768,26 +750,6 @@ static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
-static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
-				irq_hw_number_t hwirq)
-{
-	struct sirfsoc_gpio_bank *bank = d->host_data;
-
-	if (!bank)
-		return -EINVAL;
-
-	irq_set_chip(irq, &sirfsoc_irq_chip);
-	irq_set_handler(irq, handle_level_irq);
-	set_irq_flags(irq, IRQF_VALID);
-
-	return 0;
-}
-
-static const struct irq_domain_ops sirfsoc_gpio_irq_simple_ops = {
-	.map = sirfsoc_gpio_irq_map,
-	.xlate = irq_domain_xlate_twocell,
-};
-
 static void sirfsoc_gpio_set_pullup(const u32 *pullups)
 {
 	int i, n;
@@ -826,7 +788,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	struct sirfsoc_gpio_bank *bank;
 	void __iomem *regs;
 	struct platform_device *pdev;
-	struct irq_domain *domain;
 	bool is_marco = false;
 
 	u32 pullups[SIRFSOC_GPIO_NO_OF_BANKS], pulldowns[SIRFSOC_GPIO_NO_OF_BANKS];
@@ -842,21 +803,12 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	if (of_device_is_compatible(np, "sirf,marco-pinctrl"))
 		is_marco = 1;
 
-	domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
-		&sirfsoc_gpio_irq_simple_ops, &sgpio_chip);
-	if (!domain) {
-		pr_err("%s: Failed to create irqdomain\n", np->full_name);
-			err = -ENOSYS;
-		goto out;
-	}
-
 	sgpio_chip.chip.gc.request = sirfsoc_gpio_request;
 	sgpio_chip.chip.gc.free = sirfsoc_gpio_free;
 	sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input;
 	sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value;
 	sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output;
 	sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value;
-	sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq;
 	sgpio_chip.chip.gc.base = 0;
 	sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS;
 	sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
@@ -866,15 +818,24 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 	sgpio_chip.chip.gc.dev = &pdev->dev;
 	sgpio_chip.chip.regs = regs;
 	sgpio_chip.is_marco = is_marco;
-	sgpio_chip.domain = domain;
 
 	err = gpiochip_add(&sgpio_chip.chip.gc);
 	if (err) {
-		pr_err("%s: error in probe function with status %d\n",
+		dev_err(&pdev->dev, "%s: error in probe function with status %d\n",
 			np->full_name, err);
 		goto out;
 	}
 
+	err =  gpiochip_irqchip_add(&sgpio_chip.chip.gc,
+		&sirfsoc_irq_chip,
+		0, handle_level_irq,
+		IRQ_TYPE_NONE);
+	if (err) {
+		dev_err(&pdev->dev,
+			"could not connect irqchip to gpiochip\n");
+		goto out;
+	}
+
 	for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
 		bank = &sgpio_chip.sgpio_bank[i];
 		spin_lock_init(&bank->lock);
@@ -884,8 +845,10 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 			goto out;
 		}
 
-		irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
-		irq_set_handler_data(bank->parent_irq, bank);
+		gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc,
+			&sirfsoc_irq_chip,
+			bank->parent_irq,
+			sirfsoc_gpio_handle_irq);
 	}
 
 	if (!of_property_read_u32_array(np, "sirf,pullups", pullups,
-- 
1.7.5.4

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

since it is called handler_data, drivers should have the ability to
set handler_data based on real hardware.
GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
break GPIO core.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/gpio/gpiolib.c      |   10 +++-------
 include/linux/gpio/driver.h |    2 +-
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 761013f..4b90d14 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1351,24 +1351,20 @@ static struct gpio_chip *find_chip_by_name(const char *name)
 
 /**
  * gpiochip_add_chained_irqchip() - adds a chained irqchip to a gpiochip
- * @gpiochip: the gpiochip to add the irqchip to
+ * @handler_data: handler_data which will be used by ISR of interrupt parent
  * @irqchip: the irqchip to add to the gpiochip
  * @parent_irq: the irq number corresponding to the parent IRQ for this
  * chained irqchip
  * @parent_handler: the parent interrupt handler for the accumulated IRQ
  * coming out of the gpiochip
  */
-void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
+void gpiochip_set_chained_irqchip(void *handler_data,
 				  struct irq_chip *irqchip,
 				  int parent_irq,
 				  irq_flow_handler_t parent_handler)
 {
 	irq_set_chained_handler(parent_irq, parent_handler);
-	/*
-	 * The parent irqchip is already using the chip_data for this
-	 * irqchip, so our callbacks simply use the handler_data.
-	 */
-	irq_set_handler_data(parent_irq, gpiochip);
+	irq_set_handler_data(parent_irq, handler_data);
 }
 EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip);
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 1827b43..52a0758 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -207,7 +207,7 @@ void gpiod_add_lookup_table(struct gpiod_lookup_table *table);
 
 #ifdef CONFIG_GPIOLIB_IRQCHIP
 
-void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
+void gpiochip_set_chained_irqchip(void *handler_data,
 		struct irq_chip *irqchip,
 		int parent_irq,
 		irq_flow_handler_t parent_handler);
-- 
1.7.5.4

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

since it is called handler_data, drivers should have the ability to
set handler_data based on real hardware.
GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
break GPIO core.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/gpio/gpiolib.c      |   10 +++-------
 include/linux/gpio/driver.h |    2 +-
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 761013f..4b90d14 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1351,24 +1351,20 @@ static struct gpio_chip *find_chip_by_name(const char *name)
 
 /**
  * gpiochip_add_chained_irqchip() - adds a chained irqchip to a gpiochip
- * @gpiochip: the gpiochip to add the irqchip to
+ * @handler_data: handler_data which will be used by ISR of interrupt parent
  * @irqchip: the irqchip to add to the gpiochip
  * @parent_irq: the irq number corresponding to the parent IRQ for this
  * chained irqchip
  * @parent_handler: the parent interrupt handler for the accumulated IRQ
  * coming out of the gpiochip
  */
-void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
+void gpiochip_set_chained_irqchip(void *handler_data,
 				  struct irq_chip *irqchip,
 				  int parent_irq,
 				  irq_flow_handler_t parent_handler)
 {
 	irq_set_chained_handler(parent_irq, parent_handler);
-	/*
-	 * The parent irqchip is already using the chip_data for this
-	 * irqchip, so our callbacks simply use the handler_data.
-	 */
-	irq_set_handler_data(parent_irq, gpiochip);
+	irq_set_handler_data(parent_irq, handler_data);
 }
 EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip);
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 1827b43..52a0758 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -207,7 +207,7 @@ void gpiod_add_lookup_table(struct gpiod_lookup_table *table);
 
 #ifdef CONFIG_GPIOLIB_IRQCHIP
 
-void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
+void gpiochip_set_chained_irqchip(void *handler_data,
 		struct irq_chip *irqchip,
 		int parent_irq,
 		irq_flow_handler_t parent_handler);
-- 
1.7.5.4

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

we set bank as handler_data, now we can get bank in ISR directly
instead of looking-up.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/pinctrl/sirf/pinctrl-sirf.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index c03dcc7..36cc678 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -568,18 +568,10 @@ static struct irq_chip sirfsoc_irq_chip = {
 
 static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 {
-	struct sirfsoc_gpio_bank *bank;
+	struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
 	u32 status, ctrl;
 	int idx = 0;
 	struct irq_chip *chip = irq_get_chip(irq);
-	int i;
-
-	for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
-		bank = &sgpio_chip.sgpio_bank[i];
-		if (bank->parent_irq == irq)
-			break;
-	}
-	BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
 
 	chained_irq_enter(chip, desc);
 
@@ -845,7 +837,7 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 			goto out;
 		}
 
-		gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc,
+		gpiochip_set_chained_irqchip(bank,
 			&sirfsoc_irq_chip,
 			bank->parent_irq,
 			sirfsoc_gpio_handle_irq);
-- 
1.7.5.4

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

we set bank as handler_data, now we can get bank in ISR directly
instead of looking-up.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/pinctrl/sirf/pinctrl-sirf.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index c03dcc7..36cc678 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -568,18 +568,10 @@ static struct irq_chip sirfsoc_irq_chip = {
 
 static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
 {
-	struct sirfsoc_gpio_bank *bank;
+	struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
 	u32 status, ctrl;
 	int idx = 0;
 	struct irq_chip *chip = irq_get_chip(irq);
-	int i;
-
-	for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
-		bank = &sgpio_chip.sgpio_bank[i];
-		if (bank->parent_irq == irq)
-			break;
-	}
-	BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
 
 	chained_irq_enter(chip, desc);
 
@@ -845,7 +837,7 @@ static int sirfsoc_gpio_probe(struct device_node *np)
 			goto out;
 		}
 
-		gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc,
+		gpiochip_set_chained_irqchip(bank,
 			&sirfsoc_irq_chip,
 			bank->parent_irq,
 			sirfsoc_gpio_handle_irq);
-- 
1.7.5.4

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Javier Martinez Canillas]

Hello Barry,

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
> From: Barry Song <Baohua.Song@csr.com>
>
> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
> from Linus Walleij's patch too.
>
> Barry Song (3):
>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>   gpio: make handler_data configurable while using gpiolib_irqchip
>   pinctrl: sirf: move to use irq_get_handler_data
> Linus Walleij (1):
>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>
>  drivers/gpio/gpiolib.c              |   10 +-
>  drivers/pinctrl/Kconfig             |    1 +
>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>  include/linux/gpio/driver.h         |    2 +-
>  4 files changed, 97 insertions(+), 180 deletions(-)
>
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Maybe you can first do the parameter change to
gpiochip_set_chained_irqchip() and then just convert to gpiolib
irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
4 but it is up to you.

The series looks good to me:

Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Javier Martinez Canillas]

Hello Barry,

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
> From: Barry Song <Baohua.Song@csr.com>
>
> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
> from Linus Walleij's patch too.
>
> Barry Song (3):
>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>   gpio: make handler_data configurable while using gpiolib_irqchip
>   pinctrl: sirf: move to use irq_get_handler_data
> Linus Walleij (1):
>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>
>  drivers/gpio/gpiolib.c              |   10 +-
>  drivers/pinctrl/Kconfig             |    1 +
>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>  include/linux/gpio/driver.h         |    2 +-
>  4 files changed, 97 insertions(+), 180 deletions(-)
>
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Maybe you can first do the parameter change to
gpiochip_set_chained_irqchip() and then just convert to gpiolib
irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
4 but it is up to you.

The series looks good to me:

Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>:
> Hello Barry,
>
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>> from Linus Walleij's patch too.
>>
>> Barry Song (3):
>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>   pinctrl: sirf: move to use irq_get_handler_data
>> Linus Walleij (1):
>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>>
>>  drivers/gpio/gpiolib.c              |   10 +-
>>  drivers/pinctrl/Kconfig             |    1 +
>>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>>  include/linux/gpio/driver.h         |    2 +-
>>  4 files changed, 97 insertions(+), 180 deletions(-)
>>
>> --
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> Maybe you can first do the parameter change to
> gpiochip_set_chained_irqchip() and then just convert to gpiolib
> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
> 4 but it is up to you.
>

Javier, thanks. the reason for splitting 2 and 4 is that i don't want
to lose the contribution of Linus for "gpiolib irqchip helpers".
otherwise, i will be the author of the merged patch.


> The series looks good to me:
>
> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>

-barry

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>:
> Hello Barry,
>
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>> from Linus Walleij's patch too.
>>
>> Barry Song (3):
>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>   pinctrl: sirf: move to use irq_get_handler_data
>> Linus Walleij (1):
>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>>
>>  drivers/gpio/gpiolib.c              |   10 +-
>>  drivers/pinctrl/Kconfig             |    1 +
>>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>>  include/linux/gpio/driver.h         |    2 +-
>>  4 files changed, 97 insertions(+), 180 deletions(-)
>>
>> --
>> 1.7.5.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> Maybe you can first do the parameter change to
> gpiochip_set_chained_irqchip() and then just convert to gpiolib
> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
> 4 but it is up to you.
>

Javier, thanks. the reason for splitting 2 and 4 is that i don't want
to lose the contribution of Linus for "gpiolib irqchip helpers".
otherwise, i will be the author of the merged patch.


> The series looks good to me:
>
> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>

-barry

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Javier Martinez Canillas]

On Wed, Apr 16, 2014 at 12:05 PM, Barry Song <21cnbao@gmail.com> wrote:
> 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>:
>> Hello Barry,
>>
>> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>>> From: Barry Song <Baohua.Song@csr.com>
>>>
>>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>>> from Linus Walleij's patch too.
>>>
>>> Barry Song (3):
>>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>>   pinctrl: sirf: move to use irq_get_handler_data
>>> Linus Walleij (1):
>>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>>>
>>>  drivers/gpio/gpiolib.c              |   10 +-
>>>  drivers/pinctrl/Kconfig             |    1 +
>>>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>>>  include/linux/gpio/driver.h         |    2 +-
>>>  4 files changed, 97 insertions(+), 180 deletions(-)
>>>
>>> --
>>> 1.7.5.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> Maybe you can first do the parameter change to
>> gpiochip_set_chained_irqchip() and then just convert to gpiolib
>> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
>> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
>> 4 but it is up to you.
>>
>
> Javier, thanks. the reason for splitting 2 and 4 is that i don't want
> to lose the contribution of Linus for "gpiolib irqchip helpers".
> otherwise, i will be the author of the merged patch.
>

Right, I didn't notice PATCH 2 was from Linus because it didn't have
his Signed-off-by but yours (although now I notice that it was sent
with him as the author).

AFAIK you can still squash both patches and in that case the
convention is to add the original author s-o-b and then explain your
contributions on top. i.e:

From: Linus Walleij <linus.walleij@linaro.org>

This switches the SiRF pinctrl driver over to using the gpiolib
irqchip helpers simplifying some of the code.

[Baohua.Song@csr.com: move to use irq_get_handler_data]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

But it's also OK to have split in two patches, I just asked out of curiosity :-)

Best regards,
Javier

>
>> The series looks good to me:
>>
>> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>
>
> -barry

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Javier Martinez Canillas]

On Wed, Apr 16, 2014 at 12:05 PM, Barry Song <21cnbao@gmail.com> wrote:
> 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>:
>> Hello Barry,
>>
>> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>>> From: Barry Song <Baohua.Song@csr.com>
>>>
>>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>>> from Linus Walleij's patch too.
>>>
>>> Barry Song (3):
>>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>>   pinctrl: sirf: move to use irq_get_handler_data
>>> Linus Walleij (1):
>>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>>>
>>>  drivers/gpio/gpiolib.c              |   10 +-
>>>  drivers/pinctrl/Kconfig             |    1 +
>>>  drivers/pinctrl/sirf/pinctrl-sirf.c |  264 ++++++++++++-----------------------
>>>  include/linux/gpio/driver.h         |    2 +-
>>>  4 files changed, 97 insertions(+), 180 deletions(-)
>>>
>>> --
>>> 1.7.5.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> Maybe you can first do the parameter change to
>> gpiochip_set_chained_irqchip() and then just convert to gpiolib
>> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a
>> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH
>> 4 but it is up to you.
>>
>
> Javier, thanks. the reason for splitting 2 and 4 is that i don't want
> to lose the contribution of Linus for "gpiolib irqchip helpers".
> otherwise, i will be the author of the merged patch.
>

Right, I didn't notice PATCH 2 was from Linus because it didn't have
his Signed-off-by but yours (although now I notice that it was sent
with him as the author).

AFAIK you can still squash both patches and in that case the
convention is to add the original author s-o-b and then explain your
contributions on top. i.e:

From: Linus Walleij <linus.walleij@linaro.org>

This switches the SiRF pinctrl driver over to using the gpiolib
irqchip helpers simplifying some of the code.

[Baohua.Song at csr.com: move to use irq_get_handler_data]
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

But it's also OK to have split in two patches, I just asked out of curiosity :-)

Best regards,
Javier

>
>> The series looks good to me:
>>
>> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>
>
> -barry

Re: [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> all gpio banks are in one chip, that makes software clean in mapping
> irq and gpio.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

These all look good, but for some reason does not apply on top of
the pinctrl "devel" branch based on v3.15-rc2. Can you please rebase
this set onto my branch or atleast v3.15-rc2 and resend?

Yours,
Linus Walleij

[PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> all gpio banks are in one chip, that makes software clean in mapping
> irq and gpio.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

These all look good, but for some reason does not apply on top of
the pinctrl "devel" branch based on v3.15-rc2. Can you please rebase
this set onto my branch or atleast v3.15-rc2 and resend?

Yours,
Linus Walleij

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
> from Linus Walleij's patch too.
>
> Barry Song (3):
>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>   gpio: make handler_data configurable while using gpiolib_irqchip
>   pinctrl: sirf: move to use irq_get_handler_data
> Linus Walleij (1):
>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers

As mentioned I can't get this series to apply so please rebase,
resend and take this opportunity to also add relevant ACKs.

Yours,
Linus Walleij

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
> from Linus Walleij's patch too.
>
> Barry Song (3):
>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>   gpio: make handler_data configurable while using gpiolib_irqchip
>   pinctrl: sirf: move to use irq_get_handler_data
> Linus Walleij (1):
>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers

As mentioned I can't get this series to apply so please rebase,
resend and take this opportunity to also add relevant ACKs.

Yours,
Linus Walleij

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

2014-04-23 5:47 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>> from Linus Walleij's patch too.
>>
>> Barry Song (3):
>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>   pinctrl: sirf: move to use irq_get_handler_data
>> Linus Walleij (1):
>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>
> As mentioned I can't get this series to apply so please rebase,
> resend and take this opportunity to also add relevant ACKs.

linus, this patchset depends on your "pinctrl: sirf: rename inlined
accessor" you sent to me. it seems it is in neither your devel nor
3.5-rc2.

https://lkml.org/lkml/2014/4/8/156

>
> Yours,
> Linus Walleij

-barry

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Barry Song]

2014-04-23 5:47 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor".
>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased
>> from Linus Walleij's patch too.
>>
>> Barry Song (3):
>>   pinctrl: sirf: wrap all gpio banks into one gpio_chip
>>   gpio: make handler_data configurable while using gpiolib_irqchip
>>   pinctrl: sirf: move to use irq_get_handler_data
>> Linus Walleij (1):
>>   pinctrl: sirf: switch driver to use gpiolib irqchip helpers
>
> As mentioned I can't get this series to apply so please rebase,
> resend and take this opportunity to also add relevant ACKs.

linus, this patchset depends on your "pinctrl: sirf: rename inlined
accessor" you sent to me. it seems it is in neither your devel nor
3.5-rc2.

https://lkml.org/lkml/2014/4/8/156

>
> Yours,
> Linus Walleij

-barry

Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Linus Walleij]

On Wed, Apr 23, 2014 at 2:09 PM, Barry Song <21cnbao@gmail.com> wrote:

> linus, this patchset depends on your "pinctrl: sirf: rename inlined
> accessor" you sent to me. it seems it is in neither your devel nor
> 3.5-rc2.

Ah, sorry I had that on a separate branch.
Retrying...

Yours,
Linus Walleij

[PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers

[Linus Walleij]

On Wed, Apr 23, 2014 at 2:09 PM, Barry Song <21cnbao@gmail.com> wrote:

> linus, this patchset depends on your "pinctrl: sirf: rename inlined
> accessor" you sent to me. it seems it is in neither your devel nor
> 3.5-rc2.

Ah, sorry I had that on a separate branch.
Retrying...

Yours,
Linus Walleij

Re: [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> all gpio banks are in one chip, that makes software clean in mapping
> irq and gpio.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Patch applied!

Yours,
Linus Walleij

[PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> all gpio banks are in one chip, that makes software clean in mapping
> irq and gpio.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Patch applied!

Yours,
Linus Walleij

Re: [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Linus Walleij <linus.walleij@linaro.org>
>
> This switches the SiRF pinctrl driver over to using the gpiolib
> irqchip helpers simplifying some of the code.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Patch applied!

Yours,
Linus Walleij

[PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Linus Walleij <linus.walleij@linaro.org>
>
> This switches the SiRF pinctrl driver over to using the gpiolib
> irqchip helpers simplifying some of the code.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Patch applied!

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> since it is called handler_data, drivers should have the ability to
> set handler_data based on real hardware.
> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
> break GPIO core.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

NAK, the whole idea with the function gpiochip_set_chained_irqchip()
is that its sets things up so that the gpiochip is passed as handler
data on the parent IRQ, akin to how the gpiochip is passed as
chip data on the cascaded IRQs.

Yours,
Linus Walleij

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> since it is called handler_data, drivers should have the ability to
> set handler_data based on real hardware.
> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
> break GPIO core.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

NAK, the whole idea with the function gpiochip_set_chained_irqchip()
is that its sets things up so that the gpiochip is passed as handler
data on the parent IRQ, akin to how the gpiochip is passed as
chip data on the cascaded IRQs.

Yours,
Linus Walleij

Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> we set bank as handler_data, now we can get bank in ISR directly
> instead of looking-up.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

If you want to use some other handler data than the gpiochip,
you should just use irq_set_chained_handler() and
irq_set_handler_data() directly.

Yours,
Linus Walleij

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Linus Walleij]

On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:

> From: Barry Song <Baohua.Song@csr.com>
>
> we set bank as handler_data, now we can get bank in ISR directly
> instead of looking-up.
>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

If you want to use some other handler data than the gpiochip,
you should just use irq_set_chained_handler() and
irq_set_handler_data() directly.

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 4:13 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> since it is called handler_data, drivers should have the ability to
>> set handler_data based on real hardware.
>> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
>> break GPIO core.
>>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
> is that its sets things up so that the gpiochip is passed as handler
> data on the parent IRQ, akin to how the gpiochip is passed as
> chip data on the cascaded IRQs.

i think it is a really bad idea as the parent handler might not want
the whole chip if the chip has several parents and each parent want a
separate handler_data.
this patch doesn't break your way as the parameter is void *, drivers
which use your way don't need any change. but for drivers which want
more special handler_data, it supports better.

>
> Yours,
> Linus Walleij
-barry

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 4:13 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> since it is called handler_data, drivers should have the ability to
>> set handler_data based on real hardware.
>> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not
>> break GPIO core.
>>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
> is that its sets things up so that the gpiochip is passed as handler
> data on the parent IRQ, akin to how the gpiochip is passed as
> chip data on the cascaded IRQs.

i think it is a really bad idea as the parent handler might not want
the whole chip if the chip has several parents and each parent want a
separate handler_data.
this patch doesn't break your way as the parameter is void *, drivers
which use your way don't need any change. but for drivers which want
more special handler_data, it supports better.

>
> Yours,
> Linus Walleij
-barry

Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

2014-04-24 4:17 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> we set bank as handler_data, now we can get bank in ISR directly
>> instead of looking-up.
>>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> If you want to use some other handler data than the gpiochip,
> you should just use irq_set_chained_handler() and
> irq_set_handler_data() directly.

i think this should be fixed in the general API but not use one more
function call to over-write the handler_data which has been filled in
the API.
since we have the chance for drivers to set either the whole chip for
a simple chip,  or bank-specific data for a chip which has multiple
parent IRQs.

>
> Yours,
> Linus Walleij

-barry

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

2014-04-24 4:17 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> we set bank as handler_data, now we can get bank in ISR directly
>> instead of looking-up.
>>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> If you want to use some other handler data than the gpiochip,
> you should just use irq_set_chained_handler() and
> irq_set_handler_data() directly.

i think this should be fixed in the general API but not use one more
function call to over-write the handler_data which has been filled in
the API.
since we have the chance for drivers to set either the whole chip for
a simple chip,  or bank-specific data for a chip which has multiple
parent IRQs.

>
> Yours,
> Linus Walleij

-barry

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote:

>> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
>> is that its sets things up so that the gpiochip is passed as handler
>> data on the parent IRQ, akin to how the gpiochip is passed as
>> chip data on the cascaded IRQs.
>
> i think it is a really bad idea as the parent handler might not want
> the whole chip if the chip has several parents and each parent want a
> separate handler_data.
> this patch doesn't break your way as the parameter is void *, drivers
> which use your way don't need any change. but for drivers which want
> more special handler_data, it supports better.

This is perfectly possible, just don't use
gpiochip_set_chained_irqchip() which has this semantic
effect.

Use irq_set_chained_handler() and irq_set_handler_data()
directly instead.

I do see that this is maybe easier for the handler to get the bank
passed in.

However when I in the RFT patches change the driver to allocate
and pass a struct for the gpio chip, it all looks a bit different.
You would have to get the sgpio from the bank instead, which
may require to introduce a special pointer for that.

What we want to get rid of is this:

for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
                bank = &sgpio->sgpio_bank[i];
                if (bank->parent_irq == irq)
                        break;
}
BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);

So what about passing something like that to the handler:

struct sirf_irq_handler_data {
    struct sirfsoc_gpio_chip *sgpio;
    struct sirfsoc_gpio_bank *bank;
};

Then the irq handler instantly have pointers to all it needs.
But maybe it's better to just pass the bank, whatdoIknow.

Yours,
Linus Walleij

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote:

>> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
>> is that its sets things up so that the gpiochip is passed as handler
>> data on the parent IRQ, akin to how the gpiochip is passed as
>> chip data on the cascaded IRQs.
>
> i think it is a really bad idea as the parent handler might not want
> the whole chip if the chip has several parents and each parent want a
> separate handler_data.
> this patch doesn't break your way as the parameter is void *, drivers
> which use your way don't need any change. but for drivers which want
> more special handler_data, it supports better.

This is perfectly possible, just don't use
gpiochip_set_chained_irqchip() which has this semantic
effect.

Use irq_set_chained_handler() and irq_set_handler_data()
directly instead.

I do see that this is maybe easier for the handler to get the bank
passed in.

However when I in the RFT patches change the driver to allocate
and pass a struct for the gpio chip, it all looks a bit different.
You would have to get the sgpio from the bank instead, which
may require to introduce a special pointer for that.

What we want to get rid of is this:

for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
                bank = &sgpio->sgpio_bank[i];
                if (bank->parent_irq == irq)
                        break;
}
BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);

So what about passing something like that to the handler:

struct sirf_irq_handler_data {
    struct sirfsoc_gpio_chip *sgpio;
    struct sirfsoc_gpio_bank *bank;
};

Then the irq handler instantly have pointers to all it needs.
But maybe it's better to just pass the bank, whatdoIknow.

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 7:09 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>>> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
>>> is that its sets things up so that the gpiochip is passed as handler
>>> data on the parent IRQ, akin to how the gpiochip is passed as
>>> chip data on the cascaded IRQs.
>>
>> i think it is a really bad idea as the parent handler might not want
>> the whole chip if the chip has several parents and each parent want a
>> separate handler_data.
>> this patch doesn't break your way as the parameter is void *, drivers
>> which use your way don't need any change. but for drivers which want
>> more special handler_data, it supports better.
>
> This is perfectly possible, just don't use
> gpiochip_set_chained_irqchip() which has this semantic
> effect.
>
> Use irq_set_chained_handler() and irq_set_handler_data()
> directly instead.
>
> I do see that this is maybe easier for the handler to get the bank
> passed in.
>
> However when I in the RFT patches change the driver to allocate
> and pass a struct for the gpio chip, it all looks a bit different.
> You would have to get the sgpio from the bank instead, which
> may require to introduce a special pointer for that.
>
> What we want to get rid of is this:
>
> for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
>                 bank = &sgpio->sgpio_bank[i];
>                 if (bank->parent_irq == irq)
>                         break;
> }
> BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
>
> So what about passing something like that to the handler:
>
> struct sirf_irq_handler_data {
>     struct sirfsoc_gpio_chip *sgpio;
>     struct sirfsoc_gpio_bank *bank;
> };
>
> Then the irq handler instantly have pointers to all it needs.
> But maybe it's better to just pass the bank, whatdoIknow.

only if we move to irq_set_chained_handler() and
irq_set_handler_data() directly and set bank-specific handler_data
manually, it works.
my point is that will we make the gpiochip_set_chained_irqchip() more
general for this kind of cases too since you have had a good API to
wrap things?

>
> Yours,
> Linus Walleij

-barry

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 7:09 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>>> NAK, the whole idea with the function gpiochip_set_chained_irqchip()
>>> is that its sets things up so that the gpiochip is passed as handler
>>> data on the parent IRQ, akin to how the gpiochip is passed as
>>> chip data on the cascaded IRQs.
>>
>> i think it is a really bad idea as the parent handler might not want
>> the whole chip if the chip has several parents and each parent want a
>> separate handler_data.
>> this patch doesn't break your way as the parameter is void *, drivers
>> which use your way don't need any change. but for drivers which want
>> more special handler_data, it supports better.
>
> This is perfectly possible, just don't use
> gpiochip_set_chained_irqchip() which has this semantic
> effect.
>
> Use irq_set_chained_handler() and irq_set_handler_data()
> directly instead.
>
> I do see that this is maybe easier for the handler to get the bank
> passed in.
>
> However when I in the RFT patches change the driver to allocate
> and pass a struct for the gpio chip, it all looks a bit different.
> You would have to get the sgpio from the bank instead, which
> may require to introduce a special pointer for that.
>
> What we want to get rid of is this:
>
> for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) {
>                 bank = &sgpio->sgpio_bank[i];
>                 if (bank->parent_irq == irq)
>                         break;
> }
> BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE);
>
> So what about passing something like that to the handler:
>
> struct sirf_irq_handler_data {
>     struct sirfsoc_gpio_chip *sgpio;
>     struct sirfsoc_gpio_bank *bank;
> };
>
> Then the irq handler instantly have pointers to all it needs.
> But maybe it's better to just pass the bank, whatdoIknow.

only if we move to irq_set_chained_handler() and
irq_set_handler_data() directly and set bank-specific handler_data
manually, it works.
my point is that will we make the gpiochip_set_chained_irqchip() more
general for this kind of cases too since you have had a good API to
wrap things?

>
> Yours,
> Linus Walleij

-barry

Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Linus Walleij]

On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote:

>> If you want to use some other handler data than the gpiochip,
>> you should just use irq_set_chained_handler() and
>> irq_set_handler_data() directly.
>
> i think this should be fixed in the general API but not use one more
> function call to over-write the handler_data which has been filled in
> the API.
> since we have the chance for drivers to set either the whole chip for
> a simple chip,  or bank-specific data for a chip which has multiple
> parent IRQs.

I don't think it's worth it for saving one line. The helper is intended
for the simple case, i.e. where it's enough to just get the gpio_chip
as handler data. Other alternatives need to be open coded.

I'm even considering removing this helper if it's confusing, it doesn't
really add much, gpiochip_irqchip_add() is the important function
to use, not gpiochip_set_chained_irqchip().

Yours,
Linus Walleij

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Linus Walleij]

On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote:

>> If you want to use some other handler data than the gpiochip,
>> you should just use irq_set_chained_handler() and
>> irq_set_handler_data() directly.
>
> i think this should be fixed in the general API but not use one more
> function call to over-write the handler_data which has been filled in
> the API.
> since we have the chance for drivers to set either the whole chip for
> a simple chip,  or bank-specific data for a chip which has multiple
> parent IRQs.

I don't think it's worth it for saving one line. The helper is intended
for the simple case, i.e. where it's enough to just get the gpio_chip
as handler data. Other alternatives need to be open coded.

I'm even considering removing this helper if it's confusing, it doesn't
really add much, gpiochip_irqchip_add() is the important function
to use, not gpiochip_set_chained_irqchip().

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote:

> my point is that will we make the gpiochip_set_chained_irqchip() more
> general for this kind of cases too since you have had a good API to
> wrap things?

As it only wraps two function calls I'm leaning toward deleting it as it
might create more complexity than it removes. Not sure.

We don't have a similar handler for registering nested threaded
irq handlers so it's not really helpful.

Yours,
Linus Walleij

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Linus Walleij]

On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote:

> my point is that will we make the gpiochip_set_chained_irqchip() more
> general for this kind of cases too since you have had a good API to
> wrap things?

As it only wraps two function calls I'm leaning toward deleting it as it
might create more complexity than it removes. Not sure.

We don't have a similar handler for registering nested threaded
irq handlers so it's not really helpful.

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 21:06 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> my point is that will we make the gpiochip_set_chained_irqchip() more
>> general for this kind of cases too since you have had a good API to
>> wrap things?
>
> As it only wraps two function calls I'm leaning toward deleting it as it
> might create more complexity than it removes. Not sure.

i do agree that we can remove it if it doesn't really support general
gpiochip. i think it is popular that some gpiochip has several irq
parents. so people might get confused by this API as it actually
doesn't really help many.

>
> We don't have a similar handler for registering nested threaded
> irq handlers so it's not really helpful.
>
> Yours,
> Linus Walleij
>

-barry

-barry

[PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip

[Barry Song]

2014-04-24 21:06 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>> my point is that will we make the gpiochip_set_chained_irqchip() more
>> general for this kind of cases too since you have had a good API to
>> wrap things?
>
> As it only wraps two function calls I'm leaning toward deleting it as it
> might create more complexity than it removes. Not sure.

i do agree that we can remove it if it doesn't really support general
gpiochip. i think it is popular that some gpiochip has several irq
parents. so people might get confused by this API as it actually
doesn't really help many.

>
> We don't have a similar handler for registering nested threaded
> irq handlers so it's not really helpful.
>
> Yours,
> Linus Walleij
>

-barry

-barry

Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

2014-04-24 20:59 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>>> If you want to use some other handler data than the gpiochip,
>>> you should just use irq_set_chained_handler() and
>>> irq_set_handler_data() directly.
>>
>> i think this should be fixed in the general API but not use one more
>> function call to over-write the handler_data which has been filled in
>> the API.
>> since we have the chance for drivers to set either the whole chip for
>> a simple chip,  or bank-specific data for a chip which has multiple
>> parent IRQs.
>
> I don't think it's worth it for saving one line. The helper is intended
> for the simple case, i.e. where it's enough to just get the gpio_chip
> as handler data. Other alternatives need to be open coded.
>

gpiochip_set_chained_irqchip() only save one line for simple gpiochip too.
it is not worth as well. people can realize what he should set as irq_handler.

> I'm even considering removing this helper if it's confusing, it doesn't
> really add much, gpiochip_irqchip_add() is the important function
> to use, not gpiochip_set_chained_irqchip().
>
> Yours,
> Linus Walleij

-barry

[PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data

[Barry Song]

2014-04-24 20:59 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote:
>
>>> If you want to use some other handler data than the gpiochip,
>>> you should just use irq_set_chained_handler() and
>>> irq_set_handler_data() directly.
>>
>> i think this should be fixed in the general API but not use one more
>> function call to over-write the handler_data which has been filled in
>> the API.
>> since we have the chance for drivers to set either the whole chip for
>> a simple chip,  or bank-specific data for a chip which has multiple
>> parent IRQs.
>
> I don't think it's worth it for saving one line. The helper is intended
> for the simple case, i.e. where it's enough to just get the gpio_chip
> as handler data. Other alternatives need to be open coded.
>

gpiochip_set_chained_irqchip() only save one line for simple gpiochip too.
it is not worth as well. people can realize what he should set as irq_handler.

> I'm even considering removing this helper if it's confusing, it doesn't
> really add much, gpiochip_irqchip_add() is the important function
> to use, not gpiochip_set_chained_irqchip().
>
> Yours,
> Linus Walleij

-barry