Quoting Lina Iyer (2019-01-16 15:13:28)
> On Thu, Dec 20 2018 at 13:03 -0700, Stephen Boyd wrote:
> >Quoting Lina Iyer (2018-12-19 14:11:03)
> >> +
> >> +static int msm_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
> >> + unsigned int nr_irqs, void *arg)
> >> +{
> >> + int ret;
> >> + irq_hw_number_t hwirq;
> >> + struct gpio_chip *gc = domain->host_data;
> >> + struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
> >> + struct irq_fwspec *fwspec = arg;
> >> + struct qcom_irq_fwspec parent = { };
> >> + unsigned int type;
> >> +
> >> + ret = msm_gpio_domain_translate(domain, fwspec, &hwirq, &type);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> >> + &pctrl->irq_chip, gc);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + if (!domain->parent)
> >> + return 0;
> >> +
> >> + parent.fwspec.fwnode = domain->parent->fwnode;
> >> + parent.fwspec.param_count = 2;
> >> + parent.fwspec.param[0] = hwirq;
> >> + parent.fwspec.param[1] = type;
> >> +
> >> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (parent.mask)
> >> + set_bit(hwirq, pctrl->wakeup_masked_irqs);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/*
> >> + * TODO: Get rid of this and push it into gpiochip_to_irq()
> >
> >Hmm.. yeah we need to do this still. I think we can have a generic two
> >cell function similar to irq_domain_xlate_twocell() that does the fwspec
> >creation and uses some of the things that we pass to
> >gpiochip_irqchip_add(), like the default level type. This existing
> >function is not good to have, so there's work to do to get rid of this.
> >
>
> >I was also thinking that maybe we can make the alloc function above take
> >a struct gpio_irq_fwspec structure that tells the alloc function what
> >gpiochip the irq is for. That would mean that we need to change the
> >gpio_to_irq() function below to be generic and stuff the chip inside the
> >fwspec wrapper structure:
> >
> > struct gpio_irq_fwspec {
> > struct irq_fwspec fwspec;
> > struct gpio_chip *chip;
> > unsigned int offset;
> > };
> >
> >but I seem to recall that was not working for some reason.
> >
> I was thinking about this. If I understand you correctly, we want to
> generalize the .translate and .alloc functions. We could move the
> .translate to generic however, the alloc would still need to be specific
> for the parent.mask. But we can do this without the gpio_irq_fwspec. I
> presume you suggest this structure so we could pass the hwirq and type
> to the .alloc function. but we have that in the fwspec. What am I
> missing?
I'm trying to flatten the irq number space into a single integer while
letting it cross multiple gpio chips. Similar to how gpiolib flattens
the gpio number space across multiple gpio chips. The goal being to make
gpiochip_to_irq() do this all for us without having to implement it in
each gpio driver, but maybe that isn't needed or wise if the goal is to
move drivers away from taking a gpio number and converting it into an
irq with gpio_to_irq() to begin with.
FWIW, the SPMI PMIC gpio chip conversion patches didn't fix this problem
and Brian just called irq_create_fwspec_mapping() from the to_irq()
hook. So maybe this can be cleaned up later.
