Messages in this thread Patch in this message | | | Date | Sun, 14 Jan 2018 10:56:56 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH 6/8] irqchip/gic-v3-its: add ability to resend MAPC on resume |
| |
On Fri, 12 Jan 2018 21:24:20 +0000, Derek Basehore wrote: > > This adds a DT-binding to resend the MAPC command to an ITS node on
This isn't a DT binding. That's the driver implementation. The binding is what you put in Documentation/device-tree...
> resume. If the ITS is powered down during suspend and the collections > are not backed by memory, the ITS will lose that state. This just sets > up the known state for the collections after the ITS is restored. > > Change-Id: I4fe7f3f16500e1d3b14368262f03a43509c0049b > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > --- > drivers/irqchip/irq-gic-v3-its.c | 85 ++++++++++++++++++++++------------------ > 1 file changed, 47 insertions(+), 38 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 5cb808e3d0bf..6d2688a2ee67 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -46,6 +46,7 @@ > #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0) > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1) > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2) > +#define ITS_FLAGS_RESEND_MAPC_ON_RESUME (1ULL << 3) > > #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0) > > @@ -1949,52 +1950,53 @@ static void its_cpu_init_lpis(void) > dsb(sy); > } > > -static void its_cpu_init_collection(void) > +static void its_cpu_init_collection(struct its_node *its) > { > - struct its_node *its; > - int cpu; > - > - spin_lock(&its_lock); > - cpu = smp_processor_id(); > - > - list_for_each_entry(its, &its_nodes, entry) { > - u64 target; > + int cpu = smp_processor_id(); > + u64 target; > > - /* avoid cross node collections and its mapping */ > - if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) { > - struct device_node *cpu_node; > + /* avoid cross node collections and its mapping */ > + if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) { > + struct device_node *cpu_node; > > - cpu_node = of_get_cpu_node(cpu, NULL); > - if (its->numa_node != NUMA_NO_NODE && > - its->numa_node != of_node_to_nid(cpu_node)) > - continue; > - } > + cpu_node = of_get_cpu_node(cpu, NULL); > + if (its->numa_node != NUMA_NO_NODE && > + its->numa_node != of_node_to_nid(cpu_node)) > + return; > + } > > + /* > + * We now have to bind each collection to its target > + * redistributor. > + */ > + if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) { > /* > - * We now have to bind each collection to its target > + * This ITS wants the physical address of the > * redistributor. > */ > - if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) { > - /* > - * This ITS wants the physical address of the > - * redistributor. > - */ > - target = gic_data_rdist()->phys_base; > - } else { > - /* > - * This ITS wants a linear CPU number. > - */ > - target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER); > - target = GICR_TYPER_CPU_NUMBER(target) << 16; > - } > + target = gic_data_rdist()->phys_base; > + } else { > + /* This ITS wants a linear CPU number. */ > + target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER); > + target = GICR_TYPER_CPU_NUMBER(target) << 16; > + } > > - /* Perform collection mapping */ > - its->collections[cpu].target_address = target; > - its->collections[cpu].col_id = cpu; > + /* Perform collection mapping */ > + its->collections[cpu].target_address = target; > + its->collections[cpu].col_id = cpu; > > - its_send_mapc(its, &its->collections[cpu], 1); > - its_send_invall(its, &its->collections[cpu]); > - } > + its_send_mapc(its, &its->collections[cpu], 1); > + its_send_invall(its, &its->collections[cpu]); > +} > + > +static void its_cpu_init_collections(void) > +{ > + struct its_node *its; > + > + spin_lock(&its_lock); > + > + list_for_each_entry(its, &its_nodes, entry) > + its_cpu_init_collection(its); > > spin_unlock(&its_lock); > } > @@ -3089,6 +3091,9 @@ void its_restore_enable(void) > its_write_baser(its, &its->tables[i], > its->tables[i].val); > writel_relaxed(ctx->ctlr, base + GITS_CTLR); > + > + if (its->flags & ITS_FLAGS_RESEND_MAPC_ON_RESUME) > + its_cpu_init_collection(its); > } > spin_unlock(&its_lock); > } > @@ -3312,6 +3317,10 @@ static int __init its_probe_one(struct resource *res, > ctlr |= GITS_CTLR_ImDe; > writel_relaxed(ctlr, its->base + GITS_CTLR); > > + if (fwnode_property_present(its->fwnode_handle, > + "resend-mapc-on-resume")) > + its->flags |= ITS_FLAGS_RESEND_MAPC_ON_RESUME;
This function is firmware agnostic, and I really don't want to make this thing a general purpose property.
Please set this as a quirk when you detect some particular combination of HW and firmware (in your case, GIC500 + this property). And again, the property name is totally backward. IT should be something like "collection-state-lost-on-suspend".
> + > err = its_init_domain(handle, its); > if (err) > goto out_free_tables; > @@ -3347,7 +3356,7 @@ int its_cpu_init(void) > return -ENXIO; > } > its_cpu_init_lpis(); > - its_cpu_init_collection(); > + its_cpu_init_collections(); > } > > return 0; > -- > 2.16.0.rc1.238.g530d649a79-goog >
Thanks,
M.
| |