[permalink] [raw]

Subject: [PATCH V8 03/10] irqchip/loongson-pch-pic: Add suspend/resume support

Add suspend/resume support for PCH-PIC irqchip, which is needed for
suspend/hibernation.

Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/irq-loongson-pch-pic.c | 47 ++++++++++++++++++++++++++
1 file changed, 47 insertions(+)

diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index b0644d595789..5047ba96f7d7 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/syscore_ops.h>

/* Registers */
#define PCH_PIC_MASK 0x20
@@ -41,6 +42,9 @@ struct pch_pic {
struct fwnode_handle *domain_handle;
u32 ht_vec_base;
raw_spinlock_t pic_lock;
+ u32 saved_vec_en[PIC_REG_COUNT];
+ u32 saved_vec_pol[PIC_REG_COUNT];
+ u32 saved_vec_edge[PIC_REG_COUNT];
};

static struct pch_pic *pch_pic_priv[2];
@@ -142,6 +146,7 @@ static struct irq_chip pch_pic_irq_chip = {
.irq_ack = pch_pic_ack_irq,
.irq_set_affinity = irq_chip_set_affinity_parent,
.irq_set_type = pch_pic_set_type,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
};

static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
@@ -203,6 +208,46 @@ static void pch_pic_reset(struct pch_pic *priv)
}
}

+static int pch_pic_suspend(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pics; i++) {
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ pch_pic_priv[i]->saved_vec_pol[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ pch_pic_priv[i]->saved_vec_edge[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ pch_pic_priv[i]->saved_vec_en[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+
+ return 0;
+}
+
+static void pch_pic_resume(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pics; i++) {
+ pch_pic_reset(pch_pic_priv[i]);
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ writel(pch_pic_priv[i]->saved_vec_pol[j],
+ pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_edge[j],
+ pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_en[j],
+ pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+}
+
+static struct syscore_ops pch_pic_syscore_ops = {
+ .suspend = pch_pic_suspend,
+ .resume = pch_pic_resume,
+};
+
static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
struct irq_domain *parent_domain, struct fwnode_handle *domain_handle)
{
@@ -235,6 +280,8 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
pch_pic_reset(priv);
pch_pic_priv[nr_pics++] = priv;

+ register_syscore_ops(&pch_pic_syscore_ops);
+
return 0;

iounmap_base:
--
2.27.0

2021-12-16 12:58:52

by Huacai Chen

[permalink] [raw]

Subject: [PATCH V8 04/10] irqchip/loongson-pch-msi: Add ACPI init support

2021-12-16 12:59:42

by Huacai Chen

[permalink] [raw]

On Fri, 24 Dec 2021 07:31:53 +0000,
Huacai Chen <[email protected]> wrote:
>
> Hi, Marc,
>
> On Mon, Dec 20, 2021 at 8:13 PM Marc Zyngier <[email protected]> wrote:
> >
> > On Fri, 17 Dec 2021 04:45:24 +0000,
> > Huacai Chen <[email protected]> wrote:
> > >
> > > Hi, Marc,
> > >
> > > On Thu, Dec 16, 2021 at 11:06 PM Marc Zyngier <[email protected]> wrote:
> > > >
> > > > On Thu, 16 Dec 2021 12:53:48 +0000,
> > > > Huacai Chen <[email protected]> wrote:
> > > > >
> > > > > We are preparing to add new Loongson (based on LoongArch, not compatible
> > > > > with old MIPS-based Loongson) support. LoongArch use ACPI other than DT
> > > > > as its boot protocol, so add ACPI init support.
> > > > >
> > > > > PCH-PIC/PCH-MSI stands for "Interrupt Controller" that described in
> > > > > Section 5 of "Loongson 7A1000 Bridge User Manual". For more information
> > > > > please refer Documentation/loongarch/irq-chip-model.rst.
> > > > >
> > > > > Signed-off-by: Huacai Chen <[email protected]>
> > > > > ---
> > > > > drivers/irqchip/irq-loongson-pch-pic.c | 108 ++++++++++++++++++-------
> > > > > 1 file changed, 81 insertions(+), 27 deletions(-)
> > > >
> > > > [...]
> > > >
> > > > >
> > > > > +#ifdef CONFIG_ACPI
> > > > > +
> > > > > +struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
> > > > > + struct acpi_madt_bio_pic *acpi_pchpic)
> > > >
> > > > Who is calling this? This works the opposite way from what the arm64
> > > > irqchips are doing. Why? I have the ugly feeling that this is called
> > > > from the arch code, bypassing the existing infrastructure...
> > > Yes, this is called from the arch code and a bit ugly, but I can't
> > > find a better way to do this.
> > >
> > > Is the "existing infrastructure" declare the irqchip init function
> > > with IRQCHIP_ACPI_DECLARE and the arch code only need to call
> > > irqchip_init()? Then we have a problem: our irqchips have a 4 level
> > > hierachy and the parent should be initialized before its children. In
> > > FDT world this is not a problem, because of_irq_init() will sort
> > > irqchip drivers to ensure the right order. But in ACPI world,
> > > acpi_probe_device_table just call init functions in the linking order.
> > > If we want to control the order, it seems we can only sort the drivers
> > > in drivers/irq/Makefile. But I don't think this is a good idea...
> > >
> > > If there are better solutions, please let me know. Thanks.
> >
> > We have the exact same thing on the arm64 side, and we don't need of
> > this to be arch specific:
> >
> > - The MADT table describes the root interrupt controller, and it is
> > probed via IRQCHIP_ACPI_DECLARE().
> >
> > - Each children controller is declared in ACPI as a *device*, and is
> > both an interrupt producer and an interrupt consumer. Normal probe
> > deferral rules apply. See irq-mbigen.c for an example of how this is
> > done.
> Thank you for your suggestions, I have tried but failed. It seems
> there are some differences between irq-mbigen.c and our irqchips.
> Because our irqchips are mandatory while mbigen is optional.

The fact that this is optional has nothing to do with it. On a system
that requires mbigen to boot (both mass storage and networking are
hanging off it), there is a guarantee that the probe order will
respect the resource dependency.

And if that's not enough, -EPROBE_DEFER is your friend, always.

> If we declare our irqchips as devices, they are initialized in the
> initcall phase, which is too late for pci devices.

This suggests that your PCIe driver is either not enforcing the
dependencies on the interrupt controller (bad), or that the core code
is not made aware of the dependencies (equally bad).

In any case, this needs sorting, because a new architecture should be
able to boot without resorting to handcrafted dependencies that will
inevitably result in a larger pile of hacks over time. It is much
easier to solve it before day-1.

M.

--
Without deviation from the norm, progress is not possible.

2022-01-25 08:58:44

by Huacai Chen

[permalink] [raw]

Subject: Re: [PATCH V8 02/10] irqchip/loongson-pch-pic: Add ACPI init support

Hi, Marc,

On Fri, Dec 24, 2021 at 5:51 PM Marc Zyngier <[email protected]> wrote:
>
> On Fri, 24 Dec 2021 07:31:53 +0000,
> Huacai Chen <[email protected]> wrote:
> >
> > Hi, Marc,
> >
> > On Mon, Dec 20, 2021 at 8:13 PM Marc Zyngier <[email protected]> wrote:
> > >
> > > On Fri, 17 Dec 2021 04:45:24 +0000,
> > > Huacai Chen <[email protected]> wrote:
> > > >
> > > > Hi, Marc,
> > > >
> > > > On Thu, Dec 16, 2021 at 11:06 PM Marc Zyngier <[email protected]> wrote:
> > > > >
> > > > > On Thu, 16 Dec 2021 12:53:48 +0000,
> > > > > Huacai Chen <[email protected]> wrote:
> > > > > >
> > > > > > We are preparing to add new Loongson (based on LoongArch, not compatible
> > > > > > with old MIPS-based Loongson) support. LoongArch use ACPI other than DT
> > > > > > as its boot protocol, so add ACPI init support.
> > > > > >
> > > > > > PCH-PIC/PCH-MSI stands for "Interrupt Controller" that described in
> > > > > > Section 5 of "Loongson 7A1000 Bridge User Manual". For more information
> > > > > > please refer Documentation/loongarch/irq-chip-model.rst.
> > > > > >
> > > > > > Signed-off-by: Huacai Chen <[email protected]>
> > > > > > ---
> > > > > > drivers/irqchip/irq-loongson-pch-pic.c | 108 ++++++++++++++++++-------
> > > > > > 1 file changed, 81 insertions(+), 27 deletions(-)
> > > > >
> > > > > [...]
> > > > >
> > > > > >
> > > > > > +#ifdef CONFIG_ACPI
> > > > > > +
> > > > > > +struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
> > > > > > + struct acpi_madt_bio_pic *acpi_pchpic)
> > > > >
> > > > > Who is calling this? This works the opposite way from what the arm64
> > > > > irqchips are doing. Why? I have the ugly feeling that this is called
> > > > > from the arch code, bypassing the existing infrastructure...
> > > > Yes, this is called from the arch code and a bit ugly, but I can't
> > > > find a better way to do this.
> > > >
> > > > Is the "existing infrastructure" declare the irqchip init function
> > > > with IRQCHIP_ACPI_DECLARE and the arch code only need to call
> > > > irqchip_init()? Then we have a problem: our irqchips have a 4 level
> > > > hierachy and the parent should be initialized before its children. In
> > > > FDT world this is not a problem, because of_irq_init() will sort
> > > > irqchip drivers to ensure the right order. But in ACPI world,
> > > > acpi_probe_device_table just call init functions in the linking order.
> > > > If we want to control the order, it seems we can only sort the drivers
> > > > in drivers/irq/Makefile. But I don't think this is a good idea...
> > > >
> > > > If there are better solutions, please let me know. Thanks.
> > >
> > > We have the exact same thing on the arm64 side, and we don't need of
> > > this to be arch specific:
> > >
> > > - The MADT table describes the root interrupt controller, and it is
> > > probed via IRQCHIP_ACPI_DECLARE().
> > >
> > > - Each children controller is declared in ACPI as a *device*, and is
> > > both an interrupt producer and an interrupt consumer. Normal probe
> > > deferral rules apply. See irq-mbigen.c for an example of how this is
> > > done.
> > Thank you for your suggestions, I have tried but failed. It seems
> > there are some differences between irq-mbigen.c and our irqchips.
> > Because our irqchips are mandatory while mbigen is optional.
>
> The fact that this is optional has nothing to do with it. On a system
> that requires mbigen to boot (both mass storage and networking are
> hanging off it), there is a guarantee that the probe order will
> respect the resource dependency.
>
> And if that's not enough, -EPROBE_DEFER is your friend, always.
>
> > If we declare our irqchips as devices, they are initialized in the
> > initcall phase, which is too late for pci devices.
>
> This suggests that your PCIe driver is either not enforcing the
> dependencies on the interrupt controller (bad), or that the core code
> is not made aware of the dependencies (equally bad).
>
> In any case, this needs sorting, because a new architecture should be
> able to boot without resorting to handcrafted dependencies that will
> inevitably result in a larger pile of hacks over time. It is much
> easier to solve it before day-1.

Sorry for the long delay, but we still failed.

- There are several kinds of irq chips(e.g. pchpic、eiointc、cpuintc)
for LoongArch. SCI interrupt (Fixed hardware is implemented for
LoongArch in pch such as LS7A1000, and SCI interrupt is used for fixed
event handling.) is an irq input of pch irq chip which routes
interrupts to cpu as following irq chips path:

sci interrupt->|pchpic| ->|eiointc|->|cpuintc|

sci_interrupt will be transferred from gsi to irq through
acpi_gsi_to_irq in acpi_enable_subsystem called from acpi_bus_init
before acpi_scan_init where acpi device namespace is created, so we
should build pch irq domain and related upstream irq domains before
acpi_bus_init.

- PCI bus enumeration is executed from acpi_scan_init, and
pci_set_msi_domain will be called for setting msi_domain of enumerated
pci device. In pci_set_msi_domain, msi domain may be got through
pcibios_device_add, fdt, iort(used for arm64) or inheriting from host
bridge domain. And in each way, the msi domain needs to be found by
calling irq_find_matching_fwnode(fwnode, DOMAIN_BUS_PCI_MSI) to match
one from the registered msi domain before. So we build the msi domain
as x86 and arm64 before acpi_scan_init. The msi domain is hierarchic
as following:

msi interrupt->|msipic| ->|eiointc|->|cpuintc|

- Yes, a driver can be deferred probed when get -EPROBE_DEFER on
probing, but both sci interrupt transfer and pci bus enumeration are
common code (not private driver for LoongArch).

So, declaring pic devices in DSDT for seems not suitable, we can only
select the X86-like way which is a bit ugly.

Huacai

>
> M.
>
> --
> Without deviation from the norm, progress is not possible.