Skip to contentSkip to conversation

mfd: rtc: bd7xxxx Drop IC name from IRQ

PATCH Applied Matti Vaittinen 3 participants 9 messages Sep 26, 2024 – Nov 1, 2024

Subsystems (3)
lore
Share & tools
lore

Matti Vaittinen

author maintainer
#0 lore raw

PATCH 1/1 mfd: rtc: bd7xxxx Drop IC name from IRQ

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

From the RTC driver point of view it is irrelevant what the PMIC name is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Simplify this slightly by always using the RTC driver name 'bd70528' as the prefix for alarm interrupts, no matter what the exact PMIC model is, and always request the alarm interrupts of same name no matter what the PMIC model is.

---
This contains both the RTC and MFD changes in order to not break the functionality between commits to different subsystems.

Changes are based to stuff being merged in for v6.12-rc1. I can rebase and re-spin when 6.12-rc1 is out if needed.

---
 drivers/mfd/rohm-bd71828.c | 12 ++++++------
 drivers/rtc/rtc-bd70528.c  |  5 +----
 2 files changed, 7 insertions(+), 10 deletions(-)
drivers/mfd/rohm-bd71828.c +6 -6
index 39f7514aa3d8..738d8b3b9ffe 100644
@@ -32,15 +32,15 @@ static struct gpio_keys_platform_data bd71828_powerkey_data = {
 };
 
 static const struct resource bd71815_rtc_irqs[] = {
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd70528-rtc-alm-0"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd70528-rtc-alm-1"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd70528-rtc-alm-2"),
 };
 
 static const struct resource bd71828_rtc_irqs[] = {
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd70528-rtc-alm-0"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd70528-rtc-alm-1"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd70528-rtc-alm-2"),
 };
 
 static struct resource bd71815_power_irqs[] = {
drivers/rtc/rtc-bd70528.c +1 -4
index 59b627fc1ecf..954ac4ef53e8 100644
@@ -236,7 +236,6 @@ static int bd70528_probe(struct platform_device *pdev)
 {
 	struct bd70528_rtc *bd_rtc;
 	const struct rtc_class_ops *rtc_ops;
-	const char *irq_name;
 	int ret;
 	struct rtc_device *rtc;
 	int irq;
@@ -259,7 +258,6 @@ static int bd70528_probe(struct platform_device *pdev)
 
 	switch (chip) {
 	case ROHM_CHIP_TYPE_BD71815:
-		irq_name = "bd71815-rtc-alm-0";
 		bd_rtc->reg_time_start = BD71815_REG_RTC_START;
 
 		/*
@@ -276,7 +274,6 @@ static int bd70528_probe(struct platform_device *pdev)
 		hour_reg = BD71815_REG_HOUR;
 		break;
 	case ROHM_CHIP_TYPE_BD71828:
-		irq_name = "bd71828-rtc-alm-0";
 		bd_rtc->reg_time_start = BD71828_REG_RTC_START;
 		bd_rtc->bd718xx_alm_block_start = BD71828_REG_RTC_ALM_START;
 		hour_reg = BD71828_REG_RTC_HOUR;
@@ -286,7 +283,7 @@ static int bd70528_probe(struct platform_device *pdev)
 		return -ENOENT;
 	}
 
-	irq = platform_get_irq_byname(pdev, irq_name);
+	irq = platform_get_irq_byname(pdev, "bd70528-rtc-alm-0");
 
 	if (irq < 0)
 		return irq;

base-commit: abf2050f51fdca0fd146388f83cddd95a57a008d

Signature · 14 lines
2.45.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
maintainer
#1 lore raw
Quoted 18 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

From the RTC driver point of view it is irrelevant what the PMIC name is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Simplify this slightly by always using the RTC driver name 'bd70528' as the prefix for alarm interrupts, no matter what the exact PMIC model is, and always request the alarm interrupts of same name no matter what the PMIC model is.

Quoted 5 lines

---
This contains both the RTC and MFD changes in order to not break the functionality between commits to different subsystems.

I can take it with an RTC Ack and an indication whether an immutable branch should be created and shared.

Signature · 1 line
Lee Jones [李琼斯]
maintainer
#2 lore raw
Quoted 20 lines · Lee Jones
Quoted 18 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

From the RTC driver point of view it is irrelevant what the PMIC name is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Simplify this slightly by always using the RTC driver name 'bd70528' as the prefix for alarm interrupts, no matter what the exact PMIC model is, and always request the alarm interrupts of same name no matter what the PMIC model is.

Quoted 8 lines
Quoted 5 lines

---
This contains both the RTC and MFD changes in order to not break the functionality between commits to different subsystems.

I can take it with an RTC Ack and an indication whether an immutable branch should be created and shared.

I fear this may have fallen from Alexandre's queue.

Please submit as a RESEND.

Signature · 1 line
Lee Jones [李琼斯]
maintainer Applied
#3 lore raw

Re: (subset) [PATCH] mfd: rtc: bd7xxxx Drop IC name from IRQ

In reply to Matti Vaittinen
Quoted 6 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

Quoted 8 lines
Quoted 1 line

From the RTC driver point of view it is irrelevant what the PMIC name

is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

[...]

Applied, thanks!

[1/1] mfd: rtc: bd7xxxx Drop IC name from IRQ

      commit: cd49b605779b4fea8224650eeba70b258c5cc8cc

--
Lee Jones [李琼斯]

author maintainer
#4 lore raw

Re: (subset) [PATCH] mfd: rtc: bd7xxxx Drop IC name from IRQ

Quoted 14 lines · Lee Jones
Quoted 13 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

Quoted 1 line

From the RTC driver point of view it is irrelevant what the PMIC name

is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Quoted 6 lines
Quoted 1 line

[...]

Applied, thanks!

[1/1] mfd: rtc: bd7xxxx Drop IC name from IRQ

       commit: cd49b605779b4fea8224650eeba70b258c5cc8cc

Hello Lee, Alexandre,

Nothing pleases me more than having this quickly merged but...
... I don't think I saw ack from Alexandre yet. Furthermore, the (subset) makes me wonder because I sent RTC and MFD changes in a single patch - which might've been a mistake...

I tried finding the cd49b605779b4fea8224650eeba70b258c5cc8cc from MFD tree and failed. Hence I'm a bit unsure where we are going.

Yours,
-- Matti

maintainer
#5 lore raw

Re: (subset) [PATCH] mfd: rtc: bd7xxxx Drop IC name from IRQ

Quoted 24 lines · Matti Vaittinen
Quoted 14 lines · Lee Jones
Quoted 13 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

Quoted 1 line

From the RTC driver point of view it is irrelevant what the PMIC name

is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Quoted 6 lines
Quoted 1 line

[...]

Applied, thanks!

[1/1] mfd: rtc: bd7xxxx Drop IC name from IRQ

       commit: cd49b605779b4fea8224650eeba70b258c5cc8cc

Hello Lee, Alexandre,

Quoted 7 lines

Nothing pleases me more than having this quickly merged but... ... I don't think I saw ack from Alexandre yet. Furthermore, the (subset) makes me wonder because I sent RTC and MFD changes in a single patch - which might've been a mistake...

I tried finding the cd49b605779b4fea8224650eeba70b258c5cc8cc from MFD tree and failed. Hence I'm a bit unsure where we are going.

Applying this was a key-binding mistake.

This was my real intention:

https://lore.kernel.org/all/[email protected]/

Signature · 1 line
Lee Jones [李琼斯]
maintainer
#6 lore raw

Re: (subset) [PATCH] mfd: rtc: bd7xxxx Drop IC name from IRQ

Quoted 17 lines · Lee Jones
Quoted 15 lines · Matti Vaittinen
Quoted 14 lines · Lee Jones
Quoted 13 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

Quoted 1 line

From the RTC driver point of view it is irrelevant what the PMIC name

is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line

[...]

Applied, thanks!

Unapplied.

Signature · 1 line
Lee Jones [李琼斯]
maintainer Acked-by on v1 1/1
#7 lore raw
In reply to Matti Vaittinen
Quoted 13 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

From the RTC driver point of view it is irrelevant what the PMIC name is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

Quoted 6 lines

Simplify this slightly by always using the RTC driver name 'bd70528' as the prefix for alarm interrupts, no matter what the exact PMIC model is, and always request the alarm interrupts of same name no matter what the PMIC model is.

Quoted 92 lines

---
This contains both the RTC and MFD changes in order to not break the functionality between commits to different subsystems.

Changes are based to stuff being merged in for v6.12-rc1. I can rebase and re-spin when 6.12-rc1 is out if needed.

---
 drivers/mfd/rohm-bd71828.c | 12 ++++++------
 drivers/rtc/rtc-bd70528.c  |  5 +----
 2 files changed, 7 insertions(+), 10 deletions(-)
drivers/mfd/rohm-bd71828.c +6 -6
index 39f7514aa3d8..738d8b3b9ffe 100644
@@ -32,15 +32,15 @@ static struct gpio_keys_platform_data bd71828_powerkey_data = {
 };
 
 static const struct resource bd71815_rtc_irqs[] = {
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd70528-rtc-alm-0"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd70528-rtc-alm-1"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd70528-rtc-alm-2"),
 };
 
 static const struct resource bd71828_rtc_irqs[] = {
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
-	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd70528-rtc-alm-0"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd70528-rtc-alm-1"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd70528-rtc-alm-2"),
 };
 
 static struct resource bd71815_power_irqs[] = {
drivers/rtc/rtc-bd70528.c +1 -4
index 59b627fc1ecf..954ac4ef53e8 100644
@@ -236,7 +236,6 @@ static int bd70528_probe(struct platform_device *pdev)
 {
 	struct bd70528_rtc *bd_rtc;
 	const struct rtc_class_ops *rtc_ops;
-	const char *irq_name;
 	int ret;
 	struct rtc_device *rtc;
 	int irq;
@@ -259,7 +258,6 @@ static int bd70528_probe(struct platform_device *pdev)
 
 	switch (chip) {
 	case ROHM_CHIP_TYPE_BD71815:
-		irq_name = "bd71815-rtc-alm-0";
 		bd_rtc->reg_time_start = BD71815_REG_RTC_START;
 
 		/*
@@ -276,7 +274,6 @@ static int bd70528_probe(struct platform_device *pdev)
 		hour_reg = BD71815_REG_HOUR;
 		break;
 	case ROHM_CHIP_TYPE_BD71828:
-		irq_name = "bd71828-rtc-alm-0";
 		bd_rtc->reg_time_start = BD71828_REG_RTC_START;
 		bd_rtc->bd718xx_alm_block_start = BD71828_REG_RTC_ALM_START;
 		hour_reg = BD71828_REG_RTC_HOUR;
@@ -286,7 +283,7 @@ static int bd70528_probe(struct platform_device *pdev)
 		return -ENOENT;
 	}
 
-	irq = platform_get_irq_byname(pdev, irq_name);
+	irq = platform_get_irq_byname(pdev, "bd70528-rtc-alm-0");
 
 	if (irq < 0)
 		return irq;

base-commit: abf2050f51fdca0fd146388f83cddd95a57a008d

Signature · 14 lines
2.45.2


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer Applied
#8 lore raw

Re: (subset) [PATCH] mfd: rtc: bd7xxxx Drop IC name from IRQ

In reply to Matti Vaittinen
Quoted 6 lines · Matti Vaittinen

A few ROHM PMICs have an RTC block which can be controlled by the rtc-bd70528 driver. The RTC driver needs the alarm interrupt information from the parent MFD driver. The MFD driver provides the interrupt information as a set of named interrupts, where the name is of form:
<PMIC model>-rtc-alm-<x>, where x is an alarm block number.

Quoted 8 lines
Quoted 1 line

From the RTC driver point of view it is irrelevant what the PMIC name

is. It is sufficient to know this is alarm interrupt for a block X. The PMIC model information is carried to RTC via the platform device ID. Hence, having the PMIC model in the interrupt name is only making things more complex because the RTC driver needs to request differently named interrupts on different PMICs, making code unnecessary complicated.

[...]

Applied, thanks!

[1/1] mfd: rtc: bd7xxxx Drop IC name from IRQ

      commit: 9b15062cc05dbfec8092f5f08ac734fd29ed7b5a

--
Lee Jones [李琼斯]