| Message ID | 1455020907-4564-2-git-send-email-ldewangan@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello Laxman, On 02/09/2016 09:28 AM, Laxman Dewangan wrote: > It is require to dispose all virtual irq of hwirq on chip > created on given irq domain before removing this irq domain. > Hence dispose all mapped irqs before deleting the irq domains > in regmap_del_irq_chip(); > > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> > I believe this patch could be picked separately and not made part of this series since is fixing a bug that happens on most drivers using the regmap-irq API. This will avoid cross-subsystem churn for people. Your patch 6/6 does not introduce a regression since the bug already exists in the MFD driver, it just makes it more noticeable since it is easier to unbind the max77686 RTC driver than the MFD one. Best regards,
On Tuesday 09 February 2016 06:57 PM, Javier Martinez Canillas wrote: > Hello Laxman, > > On 02/09/2016 09:28 AM, Laxman Dewangan wrote: >> It is require to dispose all virtual irq of hwirq on chip >> created on given irq domain before removing this irq domain. >> Hence dispose all mapped irqs before deleting the irq domains >> in regmap_del_irq_chip(); >> >> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> >> > > I believe this patch could be picked separately and not made part of > this series since is fixing a bug that happens on most drivers using > the regmap-irq API. This will avoid cross-subsystem churn for people. > > Your patch 6/6 does not introduce a regression since the bug already > exists in the MFD driver, it just makes it more noticeable since it > is easier to unbind the max77686 RTC driver than the MFD one. If we dont have fix then rtc unbind/bind creates issue on S2R. Although it was issue on tot but the issue visible with my patch only. So if you test my 2 to 6 without 1, you will see issue. So to avoid bisect issue in functionality wise, this should go on sequence. This is my view.
Hello Laxman, On 02/09/2016 10:58 AM, Laxman Dewangan wrote: > > On Tuesday 09 February 2016 06:57 PM, Javier Martinez Canillas wrote: >> Hello Laxman, >> >> On 02/09/2016 09:28 AM, Laxman Dewangan wrote: >>> It is require to dispose all virtual irq of hwirq on chip >>> created on given irq domain before removing this irq domain. >>> Hence dispose all mapped irqs before deleting the irq domains >>> in regmap_del_irq_chip(); >>> >>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> >>> >> >> I believe this patch could be picked separately and not made part of >> this series since is fixing a bug that happens on most drivers using >> the regmap-irq API. This will avoid cross-subsystem churn for people. >> >> Your patch 6/6 does not introduce a regression since the bug already >> exists in the MFD driver, it just makes it more noticeable since it >> is easier to unbind the max77686 RTC driver than the MFD one. > > > If we dont have fix then rtc unbind/bind creates issue on S2R. Although it was issue on tot but the issue visible with my patch only. > > So if you test my 2 to 6 without 1, you will see issue. > Yes I know that but my point is that this will also happen in mainline if the MFD device is unbind. Is just that it's harder to do so since at least in the Chromebooks the regulators are used for the panel and backlight so you will need a serial console. > So to avoid bisect issue in functionality wise, this should go on sequence. This is my view. > The problem with patch series touching different subsystems is that the maintainers have to agree on how to handle the possible conflicts so it is easier for them if you split the patches and post them separately if the are really no dependencies. My view is that this fixes a regmap-irq core bug that is present in all the drivers using regmap-irq that remove the IRQ chip after mapping IRQs so it's really not related to your RTC series. But of course I'm not a subsystem maintainer so is up to Alexandre and Mark to decide that. I was just giving my opinion. Best regards,
Hello Laxman, On 02/09/2016 09:28 AM, Laxman Dewangan wrote: > It is require to dispose all virtual irq of hwirq on chip > created on given irq domain before removing this irq domain. > Hence dispose all mapped irqs before deleting the irq domains > in regmap_del_irq_chip(); > > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> > The patch looks good to me and I've also tested on an Exynos5800 Peach Pi Chromebook that S2R works correctly after the RTC dev was unbind so the bug is gone with this version of your series. Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Tested-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards,
On Tuesday 09 February 2016 08:44 PM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Tue, Feb 09, 2016 at 10:27:22AM -0300, Javier Martinez Canillas wrote: > >> I believe this patch could be picked separately and not made part of >> this series since is fixing a bug that happens on most drivers using >> the regmap-irq API. This will avoid cross-subsystem churn for people. > Yes, please don't mix things in when there are no dependencies. > Do I need to resend the patches on which 1/6 is not in series?
On Tue, Feb 09, 2016 at 10:27:22AM -0300, Javier Martinez Canillas wrote: > I believe this patch could be picked separately and not made part of > this series since is fixing a bug that happens on most drivers using > the regmap-irq API. This will avoid cross-subsystem churn for people. Yes, please don't mix things in when there are no dependencies.
Hello Laxman, On 02/09/2016 12:05 PM, Laxman Dewangan wrote: > > On Tuesday 09 February 2016 08:44 PM, Mark Brown wrote: >> * PGP Signed by an unknown key >> >> On Tue, Feb 09, 2016 at 10:27:22AM -0300, Javier Martinez Canillas wrote: >> >>> I believe this patch could be picked separately and not made part of >>> this series since is fixing a bug that happens on most drivers using >>> the regmap-irq API. This will avoid cross-subsystem churn for people. >> Yes, please don't mix things in when there are no dependencies. >> > > Do I need to resend the patches on which 1/6 is not in series? > You will need to re-spin anyways since patch 6/6 does not apply cleanly anymore on top of rtc-next as we talked before. So make your v7 to only include patches 2-6 and post 1/6 as a separate patch so Mark can pick it through the regmap tree. Best regards,
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 9b0d202..7e1e9e8 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -655,13 +655,34 @@ EXPORT_SYMBOL_GPL(regmap_add_irq_chip); * * @irq: Primary IRQ for the device * @d: regmap_irq_chip_data allocated by regmap_add_irq_chip() + * + * This function also dispose all mapped irq on chip. */ 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); + + /* Dispose all virtual irq from irq domain before removing it */ + for (hwirq = 0; hwirq < d->chip->num_irqs; hwirq++) { + /* Ignore hwirq if holes in the IRQ list */ + if (!d->chip->irqs[hwirq].mask) + continue; + + /* + * Find the virtual irq of hwirq on chip and if it is + * there then dispose it + */ + virq = irq_find_mapping(d->domain, hwirq); + if (virq) + irq_dispose_mapping(virq); + } + irq_domain_remove(d->domain); kfree(d->type_buf); kfree(d->type_buf_def);
It is require to dispose all virtual irq of hwirq on chip created on given irq domain before removing this irq domain. Hence dispose all mapped irqs before deleting the irq domains in regmap_del_irq_chip(); Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- This is new in series. Earlier patch for adding APIs to dispose virq via regmap is abandon and this patch took place. The dispose of virw is added in regmap_del_irq_chip(). drivers/base/regmap/regmap-irq.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)