Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1330211 > unrolled thread

[PATCH V6 0/6] rtc: max77686: make max77686 rtc driver as IP driver

Back to article view | Back to linux.kernel

Contents

  [PATCH V6 0/6] rtc: max77686: make max77686 rtc driver as IP driver Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-09 13:50 +0100
    [PATCH V6 3/6] rtc: max77686: use rtc regmap to access RTC registers Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-09 13:50 +0100
    [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-09 13:50 +0100
      Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before  removing domain Javier Martinez Canillas <javier@osg.samsung.com> - 2016-02-09 14:30 +0100
        Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing  domain Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-09 15:20 +0100
          Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before  removing domain Javier Martinez Canillas <javier@osg.samsung.com> - 2016-02-09 15:30 +0100
        Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing  domain Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-09 16:20 +0100
          Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before  removing domain Javier Martinez Canillas <javier@osg.samsung.com> - 2016-02-09 16:30 +0100
        Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before  removing domain Mark Brown <broonie@kernel.org> - 2016-02-09 16:20 +0100
      Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before  removing domain Javier Martinez Canillas <javier@osg.samsung.com> - 2016-02-09 16:00 +0100
      Applied "regmap: irq: dispose all virtual irq before removing domain" to the regmap tree Mark Brown <broonie@kernel.org> - 2016-02-09 17:20 +0100

#1330211 — [PATCH V6 0/6] rtc: max77686: make max77686 rtc driver as IP driver

Based on discussion on patch series of MAX77620 when adding separate
driver for max77620 RTC, it is discussed to reuse the max77686 driver
for all CHips MAX77802, MAX77686 and MAX77620. For this, the rtc-max77686
need to make as IP driver independent of their MFD parent driver.

This series makes the rtc-max77686 as independent driver from its 
parent. Required information is passed through the device parent
which is generic and does not depends on any max77686 specific
header ifnromation.

CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>

Changes from V1: 
- Added reviewed/tested by tag which we got from V1.
- Remove changes from Kconfig.
- Maintain all register definition in max77686 private header and remove
  the movement to rtc driver.
- Taken care of all comments on V1 from Krzysztof and Javier.

Changes from V2: 
- Fix the issue of crash in unbind.

Changes from V3: 
- Fix suspend-resume after unbind/bind by unmapping virq in remove callback.

Changes from V5: 
- Abandon the addition of new API in regmap-irq and add dispose of virq
  in regmap_del_irq_chip().

Laxman Dewangan (6):
  regmap: irq: dispose all virtual irq before removing domain
  rtc: max77686: fix checkpatch error
  rtc: max77686: use rtc regmap to access RTC registers
  rtc: max77686: avoid reference of parent device info multiple places
  mfd: max77686: do not set i2c client data for rtc i2c client
  rtc: max77686: move initialisation of rtc regmap, irq chip locally

 drivers/base/regmap/regmap-irq.c     |  21 ++++
 drivers/mfd/max77686.c               |  86 +-------------
 drivers/rtc/rtc-max77686.c           | 214 ++++++++++++++++++++++++++---------
 include/linux/mfd/max77686-private.h |   3 -
 4 files changed, 182 insertions(+), 142 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1330212 — [PATCH V6 3/6] rtc: max77686: use rtc regmap to access RTC registers

rtc_regmap should be used to access all RTC registers instead
of parent regmap regardless of what chip or property have it.

This makes the register access uniform and extendible for other
chips.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Krzysztof Kozlowski <k.kozlowski@samsung.com>
CC: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Changes from V1:
None, added reviewed/tested by.

Changes from V2:
None

Changes from V3:
None, become 3rd on series.

Changes from V5:
None

 drivers/rtc/rtc-max77686.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 253cf12..11f74ed 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -370,7 +370,7 @@ static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			goto out;
 		}
 
-		ret = regmap_read(info->max77686->regmap,
+		ret = regmap_read(info->max77686->rtc_regmap,
 				  map[REG_RTC_AE1], &val);
 		if (ret < 0) {
 			dev_err(info->dev,
@@ -426,7 +426,8 @@ static int max77686_rtc_stop_alarm(struct max77686_rtc_info *info)
 			goto out;
 		}
 
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1], 0);
+		ret = regmap_write(info->max77686->rtc_regmap,
+				   map[REG_RTC_AE1], 0);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
 				       map[REG_ALARM1_SEC], data,
@@ -471,7 +472,7 @@ static int max77686_rtc_start_alarm(struct max77686_rtc_info *info)
 		goto out;
 
 	if (info->drv_data->alarm_enable_reg) {
-		ret = regmap_write(info->max77686->regmap, map[REG_RTC_AE1],
+		ret = regmap_write(info->max77686->rtc_regmap, map[REG_RTC_AE1],
 				   MAX77802_ALARM_ENABLE_VALUE);
 	} else {
 		ret = regmap_bulk_read(info->max77686->rtc_regmap,
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1330213 — [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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(+)

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);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1330230 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1330276 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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.

[toc] | [prev] | [next] | [standalone]

#1330283 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1330325 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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?

[toc] | [prev] | [next] | [standalone]

#1330348 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1330335 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1330305 — Re: [PATCH V6 1/6] regmap: irq: dispose all virtual irq before removing domain

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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1330435 — Applied "regmap: irq: dispose all virtual irq before removing domain" to the regmap tree

The patch

   regmap: irq: dispose all virtual irq before removing domain

has been applied to the regmap tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 46189518d73080f4e4ea269a3b0f4b8583f486bd Mon Sep 17 00:00:00 2001
From: Laxman Dewangan <ldewangan@nvidia.com>
Date: Tue, 9 Feb 2016 17:58:22 +0530
Subject: [PATCH] regmap: irq: dispose all virtual irq before removing domain

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>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap-irq.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 9b0d202414d0..7e1e9e86c70b 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);
-- 
2.7.0.rc3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web