| Message ID | 20191113190520.305410-3-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] pinctrl: cherryview: Split out irq hw-init into a separate helper function | expand |
On Wed, Nov 13, 2019 at 08:05:20PM +0100, Hans de Goede wrote: > We need to convert all old gpio irqchips to pass the irqchip > setup along when adding the gpio_chip. For more info see > drivers/gpio/TODO. > > For chained irqchips this is a pretty straight-forward conversion. > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > struct irq_chip irqchip; > void __iomem *regs; > + unsigned int irq; > unsigned intr_lines[16]; This will conflict with our for-next. > + if (need_valid_mask) > + chip->irq.init_valid_mask = chv_init_irq_valid_mask; > + chip->irq.init_hw = chv_gpio_irq_init_hw; > + chip->irq.parent_handler = chv_gpio_irq_handler; > + chip->irq.num_parents = 1; > + chip->irq.parents = &pctrl->irq; > + chip->irq.default_type = IRQ_TYPE_NONE; > + chip->irq.handler = handle_bad_irq; > > if (!need_valid_mask) { > irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0, Perhaps now it makes sense to if (need_valid_mask) { chip->irq.init_valid_mask = chv_init_irq_valid_mask; } else { irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0, ... } ?
Hi, On 13-11-2019 20:27, Andy Shevchenko wrote: > On Wed, Nov 13, 2019 at 08:05:20PM +0100, Hans de Goede wrote: >> We need to convert all old gpio irqchips to pass the irqchip >> setup along when adding the gpio_chip. For more info see >> drivers/gpio/TODO. >> >> For chained irqchips this is a pretty straight-forward conversion. >> >> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> struct irq_chip irqchip; >> void __iomem *regs; >> + unsigned int irq; >> unsigned intr_lines[16]; > > This will conflict with our for-next. Ah, I did cherry-pick intel-pinctrl for-next into my tree a couple of days back, but I see there is a new "pinctrl: cherryview: Missed type change to unsigned int" commit there which causes this conflict. I have cherry picked this new commit into my tree and I will send out a v4 which should not conflict. > >> + if (need_valid_mask) >> + chip->irq.init_valid_mask = chv_init_irq_valid_mask; >> + chip->irq.init_hw = chv_gpio_irq_init_hw; >> + chip->irq.parent_handler = chv_gpio_irq_handler; >> + chip->irq.num_parents = 1; >> + chip->irq.parents = &pctrl->irq; >> + chip->irq.default_type = IRQ_TYPE_NONE; >> + chip->irq.handler = handle_bad_irq; >> >> if (!need_valid_mask) { >> irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0, > > > Perhaps now it makes sense to > > if (need_valid_mask) { > chip->irq.init_valid_mask = chv_init_irq_valid_mask; > } else { > irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0, > ... > } > > ? Ack good one, will also change this for v4. Regards, Hans p.s. About upstreaming this, I know this has a pre-requisite on the new add_ranges callback stuff, but how about Linus Walleij creating an immutable branch of his tree with the first series which adds the add_ranges callback in there and then you merge that branch into pinctrl-intel/for-next and then we just upstream all of this for 5.5 ? That seems easier then spreading it out over 2 cycles. Just my 2 cents.
diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index dd7d48614b7b..0d8a993f0cee 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -149,6 +149,7 @@ struct chv_pin_context { * @chip: GPIO chip in this pin controller * @irqchip: IRQ chip in this pin controller * @regs: MMIO registers + * @irq: Our parent irq * @intr_lines: Stores mapping between 16 HW interrupt wires and GPIO * offset (in GPIO number space) * @community: Community this pinctrl instance represents @@ -165,6 +166,7 @@ struct chv_pinctrl { struct gpio_chip chip; struct irq_chip irqchip; void __iomem *regs; + unsigned int irq; unsigned intr_lines[16]; const struct chv_community *community; u32 saved_intmask; @@ -1617,16 +1619,25 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) chip->add_pin_ranges = chv_gpio_add_pin_ranges; chip->parent = pctrl->dev; chip->base = -1; - if (need_valid_mask) - chip->irq.init_valid_mask = chv_init_irq_valid_mask; - ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl); - if (ret) { - dev_err(pctrl->dev, "Failed to register gpiochip\n"); - return ret; - } + pctrl->irq = irq; + pctrl->irqchip.name = "chv-gpio"; + pctrl->irqchip.irq_startup = chv_gpio_irq_startup; + pctrl->irqchip.irq_ack = chv_gpio_irq_ack; + pctrl->irqchip.irq_mask = chv_gpio_irq_mask; + pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask; + pctrl->irqchip.irq_set_type = chv_gpio_irq_type; + pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE; - chv_gpio_irq_init_hw(chip); + chip->irq.chip = &pctrl->irqchip; + if (need_valid_mask) + chip->irq.init_valid_mask = chv_init_irq_valid_mask; + chip->irq.init_hw = chv_gpio_irq_init_hw; + chip->irq.parent_handler = chv_gpio_irq_handler; + chip->irq.num_parents = 1; + chip->irq.parents = &pctrl->irq; + chip->irq.default_type = IRQ_TYPE_NONE; + chip->irq.handler = handle_bad_irq; if (!need_valid_mask) { irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0, @@ -1637,18 +1648,9 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) } } - pctrl->irqchip.name = "chv-gpio"; - pctrl->irqchip.irq_startup = chv_gpio_irq_startup; - pctrl->irqchip.irq_ack = chv_gpio_irq_ack; - pctrl->irqchip.irq_mask = chv_gpio_irq_mask; - pctrl->irqchip.irq_unmask = chv_gpio_irq_unmask; - pctrl->irqchip.irq_set_type = chv_gpio_irq_type; - pctrl->irqchip.flags = IRQCHIP_SKIP_SET_WAKE; - - ret = gpiochip_irqchip_add(chip, &pctrl->irqchip, 0, - handle_bad_irq, IRQ_TYPE_NONE); + ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl); if (ret) { - dev_err(pctrl->dev, "failed to add IRQ chip\n"); + dev_err(pctrl->dev, "Failed to register gpiochip\n"); return ret; } @@ -1662,8 +1664,6 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq) } } - gpiochip_set_chained_irqchip(chip, &pctrl->irqchip, irq, - chv_gpio_irq_handler); return 0; }