From: Claudiu Beznea <[email protected]>
Hi,
Series adds support for IA55 available on RZ/G3S SoC.
Patches are split as follows:
- 1/9 updates documentation
- 2/9 adds IA55 clock
- 3-5/9 minor cleanups to align with the suggestions at [1] and
coding style recommendations
- 6/9 implement restriction described in HW manual for ISCR register
- 7/9 add a macro to retrieve TITSR base address based on it's index
- 8/9 add suspend to RAM support
- 9/9 adds IA55 device tree node
Thank you,
Claudiu Beznea
[1] https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Changes in v2:
- collected Conor's tag
- updated commit description according to code review comments
- added patches 4, 5 according to review recommendations
- updated patch 7/9 to retrieve only TITSR base address; dropped the rest
of the changes for the moment
- in patch 8/9 use local variable in suspend/resume functions for controller's
base address, indent initialized structures members to tabs, updated
private driver data structure name
- patch 3/7 from v1 was replaced by patch 7/9 in v2
- patch 5/7 from v1 was renamed "Add support for suspend to RAM"
- cleanup patches were kept at the beginning of the series and features at the end
Claudiu Beznea (9):
dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document RZ/G3S
clk: renesas: r9a08g045: Add IA55 pclk and its reset
irqchip/renesas-rzg2l: Use tabs instead of spaces
irqchip/renesas-rzg2l: Align struct member names to tabs
irqchip/renesas-rzg2l: Document structure members
irqchip/renesas-rzg2l: Implement restriction when writing ISCR
register
irqchip/renesas-rzg2l: Add macro to retrieve TITSR register offset
based on register index
irqchip/renesas-rzg2l: Add support for suspend to RAM
arm64: dts: renesas: r9108g045: Add IA55 interrupt controller node
.../renesas,rzg2l-irqc.yaml | 5 +-
arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 68 +++++++++++
drivers/clk/renesas/r9a08g045-cpg.c | 3 +
drivers/irqchip/irq-renesas-rzg2l.c | 110 +++++++++++++-----
4 files changed, 156 insertions(+), 30 deletions(-)
--
2.39.2
From: Claudiu Beznea <[email protected]>
There are 2 TITSR registers available on IA55 interrupt controller. A
single macro could be used to access both of them. Add a macro that
retrieves TITSR register offset based on it's index. This macro is
useful in commit that adds suspend/resume support to access both TITSR
registers in a for loop.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- improved commit description
- used uppercase letter after ":" in patch title
- kept only the macro that returns the TITSR offset
drivers/irqchip/irq-renesas-rzg2l.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index a77ac6e1606f..45b696db220f 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -28,8 +28,7 @@
#define ISCR 0x10
#define IITSR 0x14
#define TSCR 0x20
-#define TITSR0 0x24
-#define TITSR1 0x28
+#define TITSR(n) (0x24 + (n) * 4)
#define TITSR0_MAX_INT 16
#define TITSEL_WIDTH 0x2
#define TSSR(n) (0x30 + ((n) * 4))
@@ -200,8 +199,7 @@ static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
unsigned int hwirq = irqd_to_hwirq(d);
u32 titseln = hwirq - IRQC_TINT_START;
- u32 offset;
- u8 sense;
+ u8 index, sense;
u32 reg;
switch (type & IRQ_TYPE_SENSE_MASK) {
@@ -217,17 +215,17 @@ static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
return -EINVAL;
}
- offset = TITSR0;
+ index = 0;
if (titseln >= TITSR0_MAX_INT) {
titseln -= TITSR0_MAX_INT;
- offset = TITSR1;
+ index = 1;
}
raw_spin_lock(&priv->lock);
- reg = readl_relaxed(priv->base + offset);
+ reg = readl_relaxed(priv->base + TITSR(index));
reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
reg |= sense << (titseln * TITSEL_WIDTH);
- writel_relaxed(reg, priv->base + offset);
+ writel_relaxed(reg, priv->base + TITSR(index));
raw_spin_unlock(&priv->lock);
return 0;
--
2.39.2
From: Claudiu Beznea <[email protected]>
The RZ/G2L manual (chapter "IRQ Status Control Register (ISCR)") describes
the operation to clear interrupts through the ISCR register as follows:
[Write operation]
When "Falling-edge detection", "Rising-edge detection" or
"Falling/Rising-edge detection" is set in IITSR:
- In case ISTAT is 1
0: IRQn interrupt detection status is cleared.
1: Invalid to write.
- In case ISTAT is 0
Invalid to write.
When “Low-level detection” is set in IITSR.:
Invalid to write.
Take the interrupt type into account when clearing interrupts through
the ISCR register to avoid writing the ISCR when interrupt type is
level.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- adapted according to review comments
- improved commit description
- used uppercase letter after ":" in patch title
drivers/irqchip/irq-renesas-rzg2l.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index d666912adc74..a77ac6e1606f 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -78,11 +78,17 @@ static void rzg2l_irq_eoi(struct irq_data *d)
unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
u32 bit = BIT(hw_irq);
- u32 reg;
+ u32 iitsr, iscr;
- reg = readl_relaxed(priv->base + ISCR);
- if (reg & bit)
- writel_relaxed(reg & ~bit, priv->base + ISCR);
+ iscr = readl_relaxed(priv->base + ISCR);
+ iitsr = readl_relaxed(priv->base + IITSR);
+
+ /*
+ * ISCR can only be cleared if the type is falling-edge, rising-edge or
+ * falling/rising-edge.
+ */
+ if ((iscr & bit) && (iitsr & IITSR_IITSEL_MASK(hw_irq)))
+ writel_relaxed(iscr & ~bit, priv->base + ISCR);
}
static void rzg2l_tint_eoi(struct irq_data *d)
--
2.39.2
From: Claudiu Beznea <[email protected]>
Add IA55 interrupt controller node and set it as interrupt parent for pin
controller.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none
arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 68 ++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 02a5dc9a0a3e..793512c4b31c 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -101,6 +101,7 @@ pinctrl: pinctrl@11030000 {
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ interrupt-parent = <&irqc>;
gpio-ranges = <&pinctrl 0 0 152>;
clocks = <&cpg CPG_MOD R9A08G045_GPIO_HCLK>;
power-domains = <&cpg>;
@@ -109,6 +110,73 @@ pinctrl: pinctrl@11030000 {
<&cpg R9A08G045_GPIO_SPARE_RESETN>;
};
+ irqc: interrupt-controller@11050000 {
+ compatible = "renesas,r9a08g045-irqc", "renesas,rzg2l-irqc";
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0 0x11050000 0 0x10000>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 430 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 431 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 432 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 437 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 440 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 441 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 443 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "nmi",
+ "irq0", "irq1", "irq2", "irq3",
+ "irq4", "irq5", "irq6", "irq7",
+ "tint0", "tint1", "tint2", "tint3",
+ "tint4", "tint5", "tint6", "tint7",
+ "tint8", "tint9", "tint10", "tint11",
+ "tint12", "tint13", "tint14", "tint15",
+ "tint16", "tint17", "tint18", "tint19",
+ "tint20", "tint21", "tint22", "tint23",
+ "tint24", "tint25", "tint26", "tint27",
+ "tint28", "tint29", "tint30", "tint31",
+ "bus-err";
+ clocks = <&cpg CPG_MOD R9A08G045_IA55_CLK>,
+ <&cpg CPG_MOD R9A08G045_IA55_PCLK>;
+ clock-names = "clk", "pclk";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A08G045_IA55_RESETN>;
+ };
+
sdhi0: mmc@11c00000 {
compatible = "renesas,sdhi-r9a08g045", "renesas,rcar-gen3-sdhi";
reg = <0x0 0x11c00000 0 0x10000>;
--
2.39.2
From: Claudiu Beznea <[email protected]>
irqchip-renesas-rzg2l driver is used on RZ/G3S SoC. RZ/G3S could go to deep
sleep states where power to different SoC's parts are cut off and RAM is
switched to self-refresh. The resume from these states is done with the
help of bootloader.
IA55 IRQ controller needs to be reconfigured when resuming from deep sleep
state. For this the IA55 registers are cached in suspend and restored in
resume.
The IA55 IRQ controller is connected to GPIO controller and GIC as follows:
┌──────────┐ ┌──────────┐
│ │ SPIX │ │
│ ├─────────►│ │
│ │ │ │
│ │ │ │
┌────────┐IRQ0-7 │ IA55 │ │ GIC │
Pin0 ───────►│ ├─────────────►│ │ │ │
│ │ │ │ PPIY │ │
... │ GPIO │ │ ├─────────►│ │
│ │GPIOINT0-127 │ │ │ │
PinN ───────►│ ├─────────────►│ │ │ │
└────────┘ └──────────┘ └──────────┘
where:
- Pin0 is the first GPIO controller pin
- PinN is the last GPIO controller pin
- SPIX is the SPI interrupt with identifier X
- PPIY is the PPI interrupt with identifier Y
Suspend/resume functionality was implemented with syscore_ops to be able
to cache/restore the registers after/before GPIO controller suspend/resume
was called. As suspend/resume function members of syscore_ops doesn't take
any argument, to be able to access the cache data structure and
controller's base address from within suspend/resume functions, the driver
private data structure was declared as static in file, named
rzg2l_irqc_data and driver has been adjusted accordingly for this.
Because IA55 IRQC is resumed before GPIO controller and different GPIO
pins could be in unwanted state for IA55 IRQC (e.g. HiZ) when IA55
reconfiguration is done on resume path, to avoid spurious interrupts
the IA55 resume configures only interrupt type on resume. The interrupt
enable operation will be done at the end of GPIO controller resume.
The interrupt type reconfiguration was kept in IA55 driver to minimize
the number of subsystems interactions on suspend/resume b/w GPIO and
IA55 drivers (as the IRQ reconfiguration from GPIO driver is done with
IRQ specific APIs).
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- improved commit description
- use uppercase letter after ":" in patch title
- implemented review comments: used tabs to align initialized structures
members, use proper naming for driver's private data structure
- use local variable for controller's base address in suspend/resume
functions
drivers/irqchip/irq-renesas-rzg2l.c | 68 +++++++++++++++++++++++------
1 file changed, 55 insertions(+), 13 deletions(-)
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 45b696db220f..bd0dd9fcd68a 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -18,6 +18,7 @@
#include <linux/pm_runtime.h>
#include <linux/reset.h>
#include <linux/spinlock.h>
+#include <linux/syscore_ops.h>
#define IRQC_IRQ_START 1
#define IRQC_IRQ_COUNT 8
@@ -55,17 +56,29 @@
#define TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
#define TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
+/**
+ * struct rzg2l_irqc_reg_cache - registers cache (necessary for suspend/resume)
+ * @iitsr: IITSR register
+ * @titsr: TITSR registers
+ */
+struct rzg2l_irqc_reg_cache {
+ u32 iitsr;
+ u32 titsr[2];
+};
+
/**
* struct rzg2l_irqc_priv - IRQ controller private data structure
* @base: controller's base address
* @fwspec: IRQ firmware specific data
* @lock: lock to protect concurrent access to hardware registers
+ * @cache: registers cache (necessary for suspend/resume)
*/
-struct rzg2l_irqc_priv {
+static struct rzg2l_irqc_priv {
void __iomem *base;
struct irq_fwspec fwspec[IRQC_NUM_IRQ];
raw_spinlock_t lock;
-};
+ struct rzg2l_irqc_reg_cache cache;
+} rzg2l_irqc_data;
static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
{
@@ -246,6 +259,38 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH);
}
+static int rzg2l_irqc_irq_suspend(void)
+{
+ struct rzg2l_irqc_reg_cache *cache = &rzg2l_irqc_data.cache;
+ void __iomem *base = rzg2l_irqc_data.base;
+
+ cache->iitsr = readl_relaxed(base + IITSR);
+ for (u8 i = 0; i < 2; i++)
+ cache->titsr[i] = readl_relaxed(base + TITSR(i));
+
+ return 0;
+}
+
+static void rzg2l_irqc_irq_resume(void)
+{
+ struct rzg2l_irqc_reg_cache *cache = &rzg2l_irqc_data.cache;
+ void __iomem *base = rzg2l_irqc_data.base;
+
+ /*
+ * Restore only interrupt type. TSSRx will be restored at the
+ * request of pin controller to avoid spurious interrupts due
+ * to invalid PIN states.
+ */
+ for (u8 i = 0; i < 2; i++)
+ writel_relaxed(cache->titsr[i], base + TITSR(i));
+ writel_relaxed(cache->iitsr, base + IITSR);
+}
+
+static struct syscore_ops rzg2l_irqc_syscore_ops = {
+ .suspend = rzg2l_irqc_irq_suspend,
+ .resume = rzg2l_irqc_irq_resume,
+};
+
static const struct irq_chip irqc_chip = {
.name = "rzg2l-irqc",
.irq_eoi = rzg2l_irqc_eoi,
@@ -331,7 +376,6 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
struct irq_domain *irq_domain, *parent_domain;
struct platform_device *pdev;
struct reset_control *resetn;
- struct rzg2l_irqc_priv *priv;
int ret;
pdev = of_find_device_by_node(node);
@@ -344,15 +388,11 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
return -ENODEV;
}
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
+ rzg2l_irqc_data.base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
+ if (IS_ERR(rzg2l_irqc_data.base))
+ return PTR_ERR(rzg2l_irqc_data.base);
- priv->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
- if (IS_ERR(priv->base))
- return PTR_ERR(priv->base);
-
- ret = rzg2l_irqc_parse_interrupts(priv, node);
+ ret = rzg2l_irqc_parse_interrupts(&rzg2l_irqc_data, node);
if (ret) {
dev_err(&pdev->dev, "cannot parse interrupts: %d\n", ret);
return ret;
@@ -375,17 +415,19 @@ static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
goto pm_disable;
}
- raw_spin_lock_init(&priv->lock);
+ raw_spin_lock_init(&rzg2l_irqc_data.lock);
irq_domain = irq_domain_add_hierarchy(parent_domain, 0, IRQC_NUM_IRQ,
node, &rzg2l_irqc_domain_ops,
- priv);
+ &rzg2l_irqc_data);
if (!irq_domain) {
dev_err(&pdev->dev, "failed to add irq domain\n");
ret = -ENOMEM;
goto pm_put;
}
+ register_syscore_ops(&rzg2l_irqc_syscore_ops);
+
return 0;
pm_put:
--
2.39.2
From: Claudiu Beznea <[email protected]>
Align struct member names to tabs to follow the requirements from
maintainer-tip file. 3 tabs were used at the moment as the next commits
will add a new member which requires 3 tabs for a better view.
Link: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- this patch is new
drivers/irqchip/irq-renesas-rzg2l.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index cc42cbd05762..90971ab06f0c 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -57,9 +57,9 @@
#define TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
struct rzg2l_irqc_priv {
- void __iomem *base;
- struct irq_fwspec fwspec[IRQC_NUM_IRQ];
- raw_spinlock_t lock;
+ void __iomem *base;
+ struct irq_fwspec fwspec[IRQC_NUM_IRQ];
+ raw_spinlock_t lock;
};
static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
--
2.39.2
From: Claudiu Beznea <[email protected]>
Document structure members to follow the requirements specified in
maintainer-tip, section 4.3.7. Struct declarations and initializers.
Link: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- this patch is new
drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
index 90971ab06f0c..d666912adc74 100644
--- a/drivers/irqchip/irq-renesas-rzg2l.c
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -56,6 +56,12 @@
#define TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
#define TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
+/**
+ * struct rzg2l_irqc_priv - IRQ controller private data structure
+ * @base: controller's base address
+ * @fwspec: IRQ firmware specific data
+ * @lock: lock to protect concurrent access to hardware registers
+ */
struct rzg2l_irqc_priv {
void __iomem *base;
struct irq_fwspec fwspec[IRQC_NUM_IRQ];
--
2.39.2
Hi, Biju,
On 15.11.2023 16:45, Biju Das wrote:
> Hi Claudiu,
>
> Thanks for the patch.
>
>> Subject: [PATCH v2 8/9] irqchip/renesas-rzg2l: Add support for suspend to
>> RAM
>>
>> From: Claudiu Beznea <[email protected]>
>>
>> irqchip-renesas-rzg2l driver is used on RZ/G3S SoC. RZ/G3S could go to
>> deep sleep states where power to different SoC's parts are cut off and RAM
>> is switched to self-refresh. The resume from these states is done with the
>> help of bootloader.
>>
>> IA55 IRQ controller needs to be reconfigured when resuming from deep sleep
>> state. For this the IA55 registers are cached in suspend and restored in
>> resume.
>>
>> The IA55 IRQ controller is connected to GPIO controller and GIC as
>> follows:
>>
>> ┌──────────┐ ┌──────────┐
>> │ │ SPIX │ │
>> │ ├─────────►│ │
>> │ │ │ │
>> │ │ │ │
>> ┌────────┐IRQ0-7 │ IA55 │ │ GIC │
>> Pin0 ───────►│ ├─────────────►│ │ │ │
>> │ │ │ │ PPIY │ │
>> ... │ GPIO │ │ ├─────────►│ │
>> │ │GPIOINT0-127 │ │ │ │
>> PinN ───────►│ ├─────────────►│ │ │ │
>> └────────┘ └──────────┘ └──────────┘
>>
>> where:
>> - Pin0 is the first GPIO controller pin
>> - PinN is the last GPIO controller pin
>> - SPIX is the SPI interrupt with identifier X
>> - PPIY is the PPI interrupt with identifier Y
>>
>> Suspend/resume functionality was implemented with syscore_ops to be able
>> to cache/restore the registers after/before GPIO controller suspend/resume
>> was called. As suspend/resume function members of syscore_ops doesn't take
>> any argument, to be able to access the cache data structure and
>> controller's base address from within suspend/resume functions, the driver
>> private data structure was declared as static in file, named
>> rzg2l_irqc_data and driver has been adjusted accordingly for this.
>>
>> Because IA55 IRQC is resumed before GPIO controller and different GPIO
>> pins could be in unwanted state for IA55 IRQC (e.g. HiZ) when IA55
>> reconfiguration is done on resume path, to avoid spurious interrupts the
>> IA55 resume configures only interrupt type on resume. The interrupt enable
>> operation will be done at the end of GPIO controller resume.
>> The interrupt type reconfiguration was kept in IA55 driver to minimize the
>> number of subsystems interactions on suspend/resume b/w GPIO and
>> IA55 drivers (as the IRQ reconfiguration from GPIO driver is done with IRQ
>> specific APIs).
>>
>> Signed-off-by: Claudiu Beznea <[email protected]>
>> ---
>>
>> Changes in v2:
>> - improved commit description
>> - use uppercase letter after ":" in patch title
>> - implemented review comments: used tabs to align initialized structures
>> members, use proper naming for driver's private data structure
>> - use local variable for controller's base address in suspend/resume
>> functions
>>
>> drivers/irqchip/irq-renesas-rzg2l.c | 68 +++++++++++++++++++++++------
>> 1 file changed, 55 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-
>> renesas-rzg2l.c
>> index 45b696db220f..bd0dd9fcd68a 100644
>> --- a/drivers/irqchip/irq-renesas-rzg2l.c
>> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
>> @@ -18,6 +18,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/reset.h>
>> #include <linux/spinlock.h>
>> +#include <linux/syscore_ops.h>
>>
>> #define IRQC_IRQ_START 1
>> #define IRQC_IRQ_COUNT 8
>> @@ -55,17 +56,29 @@
>> #define TINT_EXTRACT_HWIRQ(x) FIELD_GET(GENMASK(15, 0), (x))
>> #define TINT_EXTRACT_GPIOINT(x) FIELD_GET(GENMASK(31, 16), (x))
>>
>> +/**
>> + * struct rzg2l_irqc_reg_cache - registers cache (necessary for
>> +suspend/resume)
>> + * @iitsr: IITSR register
>> + * @titsr: TITSR registers
>> + */
>> +struct rzg2l_irqc_reg_cache {
>> + u32 iitsr;
>> + u32 titsr[2];
>> +};
>> +
>> /**
>> * struct rzg2l_irqc_priv - IRQ controller private data structure
>> * @base: controller's base address
>> * @fwspec: IRQ firmware specific data
>> * @lock: lock to protect concurrent access to hardware registers
>> + * @cache: registers cache (necessary for suspend/resume)
>> */
>> -struct rzg2l_irqc_priv {
>> +static struct rzg2l_irqc_priv {
>> void __iomem *base;
>> struct irq_fwspec fwspec[IRQC_NUM_IRQ];
>> raw_spinlock_t lock;
>> -};
>> + struct rzg2l_irqc_reg_cache cache;
>> +} rzg2l_irqc_data;
>
> Why can't you use a static pointer here and fill it in probe()
> and use this pointer in suspend()/resume()?
I can do that. I think I wrongly understood previous review comment on
this. I'll update and resend.
Thank you,
Claudiu Beznea
>
> Cheers,
> Biju
>
>>
>> static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
>> { @@ -246,6 +259,38 @@ static int rzg2l_irqc_set_type(struct irq_data *d,
>> unsigned int type)
>> return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH); }
>>
>> +static int rzg2l_irqc_irq_suspend(void) {
>> + struct rzg2l_irqc_reg_cache *cache = &rzg2l_irqc_data.cache;
>> + void __iomem *base = rzg2l_irqc_data.base;
>> +
>> + cache->iitsr = readl_relaxed(base + IITSR);
>> + for (u8 i = 0; i < 2; i++)
>> + cache->titsr[i] = readl_relaxed(base + TITSR(i));
>> +
>> + return 0;
>> +}
>> +
>> +static void rzg2l_irqc_irq_resume(void) {
>> + struct rzg2l_irqc_reg_cache *cache = &rzg2l_irqc_data.cache;
>> + void __iomem *base = rzg2l_irqc_data.base;
>> +
>> + /*
>> + * Restore only interrupt type. TSSRx will be restored at the
>> + * request of pin controller to avoid spurious interrupts due
>> + * to invalid PIN states.
>> + */
>> + for (u8 i = 0; i < 2; i++)
>> + writel_relaxed(cache->titsr[i], base + TITSR(i));
>> + writel_relaxed(cache->iitsr, base + IITSR); }
>> +
>> +static struct syscore_ops rzg2l_irqc_syscore_ops = {
>> + .suspend = rzg2l_irqc_irq_suspend,
>> + .resume = rzg2l_irqc_irq_resume,
>> +};
>> +
>> static const struct irq_chip irqc_chip = {
>> .name = "rzg2l-irqc",
>> .irq_eoi = rzg2l_irqc_eoi,
>> @@ -331,7 +376,6 @@ static int rzg2l_irqc_init(struct device_node *node,
>> struct device_node *parent)
>> struct irq_domain *irq_domain, *parent_domain;
>> struct platform_device *pdev;
>> struct reset_control *resetn;
>> - struct rzg2l_irqc_priv *priv;
>> int ret;
>>
>> pdev = of_find_device_by_node(node);
>> @@ -344,15 +388,11 @@ static int rzg2l_irqc_init(struct device_node *node,
>> struct device_node *parent)
>> return -ENODEV;
>> }
>>
>> - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> - if (!priv)
>> - return -ENOMEM;
>> + rzg2l_irqc_data.base = devm_of_iomap(&pdev->dev, pdev->dev.of_node,
>> 0, NULL);
>> + if (IS_ERR(rzg2l_irqc_data.base))
>> + return PTR_ERR(rzg2l_irqc_data.base);
>>
>> - priv->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
>> - if (IS_ERR(priv->base))
>> - return PTR_ERR(priv->base);
>> -
>> - ret = rzg2l_irqc_parse_interrupts(priv, node);
>> + ret = rzg2l_irqc_parse_interrupts(&rzg2l_irqc_data, node);
>> if (ret) {
>> dev_err(&pdev->dev, "cannot parse interrupts: %d\n", ret);
>> return ret;
>> @@ -375,17 +415,19 @@ static int rzg2l_irqc_init(struct device_node *node,
>> struct device_node *parent)
>> goto pm_disable;
>> }
>>
>> - raw_spin_lock_init(&priv->lock);
>> + raw_spin_lock_init(&rzg2l_irqc_data.lock);
>>
>> irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
>> IRQC_NUM_IRQ,
>> node, &rzg2l_irqc_domain_ops,
>> - priv);
>> + &rzg2l_irqc_data);
>> if (!irq_domain) {
>> dev_err(&pdev->dev, "failed to add irq domain\n");
>> ret = -ENOMEM;
>> goto pm_put;
>> }
>>
>> + register_syscore_ops(&rzg2l_irqc_syscore_ops);
>> +
>> return 0;
>>
>> pm_put:
>> --
>> 2.39.2
>