Message ID | 1454769447-785-2-git-send-email-ldewangan@nvidia.com |
---|---|
State | Superseded |
Headers | show |
On Sat, Feb 06, 2016 at 08:07:22PM +0530, Laxman Dewangan wrote: > Before removing irq domains, it is require to unmap all > mapped interrupt from that domain. Currently there is API > to map the interrupt on chip as regmap_irq_get_virq() for > creating mapping. Add equivalent API to dispose the mapped > irq in irq domains. This makes no sense to me. Why would you ever want to unmap the interrupts separately to destroying the domain and why would you ever want to destroy the domain without unmapping the interrupts?
On Monday 08 February 2016 08:25 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Sat, Feb 06, 2016 at 08:07:22PM +0530, Laxman Dewangan wrote: > >> Before removing irq domains, it is require to unmap all >> mapped interrupt from that domain. Currently there is API >> to map the interrupt on chip as regmap_irq_get_virq() for >> creating mapping. Add equivalent API to dispose the mapped >> irq in irq domains. > This makes no sense to me. Why would you ever want to unmap the > interrupts separately to destroying the domain This is the requirement from irq_domain_remove(). This is what we have in irq_domain_remove(): kernel/irq/irqdomain.c /* * This routine is used to remove an irq domain. The caller must ensure * that all mappings within the domain have been disposed of prior to * use, depending on the revmap type. */ void irq_domain_remove(struct irq_domain *domain) I am adding the API equivalent to regmap_irq_get_virq() to unmap virtual irq here. > and why would you ever > want to destroy the domain without unmapping the interrupts? > That's exactlly we are trying to do, unmap interrupt in client level before destroying domain.
Adding Thomas in the discussion. On Monday 08 February 2016 10:29 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Mon, Feb 08, 2016 at 10:08:28PM +0530, Laxman Dewangan wrote: >> On Monday 08 February 2016 08:25 PM, Mark Brown wrote: >>> On Sat, Feb 06, 2016 at 08:07:22PM +0530, Laxman Dewangan wrote: >>>> Before removing irq domains, it is require to unmap all >>>> mapped interrupt from that domain. Currently there is API >>>> to map the interrupt on chip as regmap_irq_get_virq() for >>>> creating mapping. Add equivalent API to dispose the mapped >>>> irq in irq domains. >>> This makes no sense to me. Why would you ever want to unmap the >>> interrupts separately to destroying the domain >> This is the requirement from irq_domain_remove(). This is what we have in >> irq_domain_remove(): >> kernel/irq/irqdomain.c >> I am adding the API equivalent to regmap_irq_get_virq() to unmap virtual irq >> here. > This does not explain why anyone would ever want to use this interface > (which was my question), why would anyone ever want to do this as a > separate step? OK, so you want to say that irq_domain_remove() should take care of doing unmap also? > >>> and why would you ever >>> want to destroy the domain without unmapping the interrupts? >> That's exactlly we are trying to do, unmap interrupt in client level before >> destroying domain. > Again, why would any client ever want to skip this step? If above is yes then we will not need for unmap virtual irq. So fix need to go in the irq_domain_remove() to unamp before actually destroying the irq domain?
On Mon, Feb 08, 2016 at 10:08:28PM +0530, Laxman Dewangan wrote: > On Monday 08 February 2016 08:25 PM, Mark Brown wrote: > >On Sat, Feb 06, 2016 at 08:07:22PM +0530, Laxman Dewangan wrote: > >>Before removing irq domains, it is require to unmap all > >>mapped interrupt from that domain. Currently there is API > >>to map the interrupt on chip as regmap_irq_get_virq() for > >>creating mapping. Add equivalent API to dispose the mapped > >>irq in irq domains. > >This makes no sense to me. Why would you ever want to unmap the > >interrupts separately to destroying the domain > This is the requirement from irq_domain_remove(). This is what we have in > irq_domain_remove(): > kernel/irq/irqdomain.c > I am adding the API equivalent to regmap_irq_get_virq() to unmap virtual irq > here. This does not explain why anyone would ever want to use this interface (which was my question), why would anyone ever want to do this as a separate step? > >and why would you ever > >want to destroy the domain without unmapping the interrupts? > That's exactlly we are trying to do, unmap interrupt in client level before > destroying domain. Again, why would any client ever want to skip this step?
On Mon, Feb 08, 2016 at 10:26:20PM +0530, Laxman Dewangan wrote: > Adding Thomas in the discussion. > >This does not explain why anyone would ever want to use this interface > >(which was my question), why would anyone ever want to do this as a > >separate step? > OK, so you want to say that irq_domain_remove() should take care of doing > unmap also? My main focus is on the regmap-irq interface you are trying to add - why would a user of this interface want to do this as a separate step (which would require changing all the existing users...)? We're providing an interface to get the virq mapped to a particular interrupt in the device, it's not obvious that that is an allocation and it certainly wasn't the intention (this code predates us using domains). > So fix need to go in the irq_domain_remove() to unamp before actually > destroying the irq domain? That's one option, but you could also do this at the regmap-irq level.
On Monday 08 February 2016 11:49 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Mon, Feb 08, 2016 at 10:26:20PM +0530, Laxman Dewangan wrote: > >> So fix need to go in the irq_domain_remove() to unamp before actually >> destroying the irq domain? > That's one option, but you could also do this at the regmap-irq level. > Do we need to dispose all virtual irq in regmap_del_irq_chip() before actually removing domain? This means we need to store the created virq in regmap_irq_chip_data for disposing it when removing the irq domain.
On Tuesday 09 February 2016 10:46 AM, Laxman Dewangan wrote: > > On Monday 08 February 2016 11:49 PM, Mark Brown wrote: >> * PGP Signed by an unknown key >> >> On Mon, Feb 08, 2016 at 10:26:20PM +0530, Laxman Dewangan wrote: >> >>> So fix need to go in the irq_domain_remove() to unamp before actually >>> destroying the irq domain? >> That's one option, but you could also do this at the regmap-irq level. >> > > Do we need to dispose all virtual irq in regmap_del_irq_chip() before > actually removing domain? > This means we need to store the created virq in regmap_irq_chip_data > for disposing it when removing the irq domain. It is easy for me to communicate through code to avoid any confusion. So do you want to say as follows? In this case, there is no need of any new API. void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *d) { + unsigned int virq; + int hwirq; + if (!d) return; free_irq(irq, d); + + /* Unmap all virtual irq from this domain */ + for (hwirq = 0; hwirq < d->chip->num_irqs; hwirq++) { + if (!d->chip->irqs[irq].mask) + continue; + + /* Get virtual irq of hwirq on chip if already mapped */ + virq = irq_find_mapping(d->domain, hwirq); + if (virq) + irq_dispose_mapping(virq); + } +
On Tuesday 09 February 2016 04:57 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Tue, Feb 09, 2016 at 02:38:28PM +0530, Laxman Dewangan wrote: >> On Tuesday 09 February 2016 10:46 AM, Laxman Dewangan wrote: >>> Do we need to dispose all virtual irq in regmap_del_irq_chip() before >>> actually removing domain? >>> This means we need to store the created virq in regmap_irq_chip_data for >>> disposing it when removing the irq domain. >> It is easy for me to communicate through code to avoid any confusion. So do >> you want to say as follows? In this case, there is no need of any new API. > Something like that, yes. Documentation/SubmittingPatches please... Thanks for suggestion and review. Sure, I will do this. BTW, I like to add devm_ version of regmap_add_irq_chip() and regmap_del_irq_chip() so that error path and .remove callback is much cleaner. If you are fine then I can make part of this series so that rtc driver will look more simple on probe/remove callback.
On Tue, Feb 09, 2016 at 02:38:28PM +0530, Laxman Dewangan wrote: > On Tuesday 09 February 2016 10:46 AM, Laxman Dewangan wrote: > >Do we need to dispose all virtual irq in regmap_del_irq_chip() before > >actually removing domain? > >This means we need to store the created virq in regmap_irq_chip_data for > >disposing it when removing the irq domain. > It is easy for me to communicate through code to avoid any confusion. So do > you want to say as follows? In this case, there is no need of any new API. Something like that, yes. Documentation/SubmittingPatches please...
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 9b0d202..9b36fc8 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -707,6 +707,20 @@ int regmap_irq_get_virq(struct regmap_irq_chip_data *data, int irq) EXPORT_SYMBOL_GPL(regmap_irq_get_virq); /** + * regmap_irq_put_virq(): Unmap mapped interrupt of a chip in irq-domain. + * + * This routine is used to dispose the created mapping from irq domain + * which must be call before removing irq domain. + * + * @irq: virtual irq number which was mapped with irq domains. + */ +void regmap_irq_put_virq(int virq) +{ + irq_dispose_mapping(virq); +} +EXPORT_SYMBOL_GPL(regmap_irq_put_virq); + +/** * regmap_irq_get_domain(): Retrieve the irq_domain for the chip * * Useful for drivers to request their own IRQs and for integration diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 27aaac9..546eab5 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -870,6 +870,7 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *data); int regmap_irq_chip_get_base(struct regmap_irq_chip_data *data); int regmap_irq_get_virq(struct regmap_irq_chip_data *data, int irq); +void regmap_irq_put_virq(int virq); struct irq_domain *regmap_irq_get_domain(struct regmap_irq_chip_data *data); #else
Before removing irq domains, it is require to unmap all mapped interrupt from that domain. Currently there is API to map the interrupt on chip as regmap_irq_get_virq() for creating mapping. Add equivalent API to dispose the mapped irq in irq domains. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com> CC: Javier Martinez Canillas <javier@osg.samsung.com> --- This is new in the series which was identified when testing V3 series that it is require to unmap virq before deleting irq chip domain. Adding this APIs in regmap irq framework. drivers/base/regmap/regmap-irq.c | 14 ++++++++++++++ include/linux/regmap.h | 1 + 2 files changed, 15 insertions(+)