| Message ID | 20181210102309.8207-3-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | irqchip/gic-v3: Add support to DT based quirk for msm8996 | expand |
On 10/12/2018 10:23, Srinivas Kandagatla wrote: > This patch adds support to device tree based quirks based on > device tree compatible string. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/irqchip/irq-gic-common.c | 12 ++++++++++++ > drivers/irqchip/irq-gic-common.h | 3 +++ > drivers/irqchip/irq-gic-v3.c | 5 +++++ > 3 files changed, 20 insertions(+) > > diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c > index 01e673c680cd..3c93c6f4d1f1 100644 > --- a/drivers/irqchip/irq-gic-common.c > +++ b/drivers/irqchip/irq-gic-common.c > @@ -36,6 +36,18 @@ void gic_set_kvm_info(const struct gic_kvm_info *info) > gic_kvm_info = info; > } > > +void gic_enable_of_quirks(const struct device_node *np, > + const struct gic_quirk *quirks, void *data) > +{ > + for (; quirks->desc; quirks++) { So you expect quirks->desc to be NULL at some point, just like for any other quirk table we have. > + if (!of_device_is_compatible(np, quirks->compatible)) > + continue; > + if (quirks->init(data)) > + pr_info("GIC: enabling workaround for %s\n", > + quirks->desc); > + } > +} > + > void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, > void *data) > { > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h > index 3919cd7c5285..97e58fb6c232 100644 > --- a/drivers/irqchip/irq-gic-common.h > +++ b/drivers/irqchip/irq-gic-common.h > @@ -23,6 +23,7 @@ > > struct gic_quirk { > const char *desc; > + const char *compatible; > bool (*init)(void *data); > u32 iidr; > u32 mask; > @@ -35,6 +36,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs, > void gic_cpu_config(void __iomem *base, void (*sync_access)(void)); > void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, > void *data); > +void gic_enable_of_quirks(const struct device_node *np, > + const struct gic_quirk *quirks, void *data); > > void gic_set_kvm_info(const struct gic_kvm_info *info); > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 29e9d47be97d..c95796fa4de6 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -1271,6 +1271,9 @@ static void __init gic_of_setup_kvm_info(struct device_node *node) > gic_set_kvm_info(&gic_v3_kvm_info); > } > > +static const struct gic_quirk gic_quirks[] = { > +}; ... and yet here you provide an empty table. That's not going to work very well. You definitely need to have an empty entry at the end of the array, always. I guess you want to test your code on a non affected platform, and I'm pretty sure you'll see it exploding. > + > static int __init gic_of_init(struct device_node *node, struct device_node *parent) > { > void __iomem *dist_base; > @@ -1318,6 +1321,8 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare > if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) > redist_stride = 0; > > + gic_enable_of_quirks(node, gic_quirks, &gic_data); > + > err = gic_init_bases(dist_base, rdist_regs, nr_redist_regions, > redist_stride, &node->fwnode); > if (err) > Thanks, M.
On 10/12/18 10:55, Marc Zyngier wrote: >> >> +void gic_enable_of_quirks(const struct device_node *np, >> + const struct gic_quirk *quirks, void *data) >> +{ >> + for (; quirks->desc; quirks++) { > So you expect quirks->desc to be NULL at some point, just like for any > other quirk table we have. > >> + if (!of_device_is_compatible(np, quirks->compatible)) >> + continue; >> + if (quirks->init(data)) >> + pr_info("GIC: enabling workaround for %s\n", >> + quirks->desc); >> + } >> +} >> + >> void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, >> void *data) >> { >> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h >> index 3919cd7c5285..97e58fb6c232 100644 >> --- a/drivers/irqchip/irq-gic-common.h >> +++ b/drivers/irqchip/irq-gic-common.h >> @@ -23,6 +23,7 @@ >> >> struct gic_quirk { >> const char *desc; >> + const char *compatible; >> bool (*init)(void *data); >> u32 iidr; >> u32 mask; >> @@ -35,6 +36,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs, >> void gic_cpu_config(void __iomem *base, void (*sync_access)(void)); >> void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, >> void *data); >> +void gic_enable_of_quirks(const struct device_node *np, >> + const struct gic_quirk *quirks, void *data); >> >> void gic_set_kvm_info(const struct gic_kvm_info *info); >> >> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >> index 29e9d47be97d..c95796fa4de6 100644 >> --- a/drivers/irqchip/irq-gic-v3.c >> +++ b/drivers/irqchip/irq-gic-v3.c >> @@ -1271,6 +1271,9 @@ static void __init gic_of_setup_kvm_info(struct device_node *node) >> gic_set_kvm_info(&gic_v3_kvm_info); >> } >> >> +static const struct gic_quirk gic_quirks[] = { >> +}; > ... and yet here you provide an empty table. That's not going to work > very well. You definitely need to have an empty entry at the end of the > array, always. > > I guess you want to test your code on a non affected platform, and I'm > pretty sure you'll see it exploding. Yes, Should have carefully looked at this!! We need an empty entry at the end! I will fix it and send a new version! Thanks, srini >
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c index 01e673c680cd..3c93c6f4d1f1 100644 --- a/drivers/irqchip/irq-gic-common.c +++ b/drivers/irqchip/irq-gic-common.c @@ -36,6 +36,18 @@ void gic_set_kvm_info(const struct gic_kvm_info *info) gic_kvm_info = info; } +void gic_enable_of_quirks(const struct device_node *np, + const struct gic_quirk *quirks, void *data) +{ + for (; quirks->desc; quirks++) { + if (!of_device_is_compatible(np, quirks->compatible)) + continue; + if (quirks->init(data)) + pr_info("GIC: enabling workaround for %s\n", + quirks->desc); + } +} + void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, void *data) { diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h index 3919cd7c5285..97e58fb6c232 100644 --- a/drivers/irqchip/irq-gic-common.h +++ b/drivers/irqchip/irq-gic-common.h @@ -23,6 +23,7 @@ struct gic_quirk { const char *desc; + const char *compatible; bool (*init)(void *data); u32 iidr; u32 mask; @@ -35,6 +36,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs, void gic_cpu_config(void __iomem *base, void (*sync_access)(void)); void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, void *data); +void gic_enable_of_quirks(const struct device_node *np, + const struct gic_quirk *quirks, void *data); void gic_set_kvm_info(const struct gic_kvm_info *info); diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 29e9d47be97d..c95796fa4de6 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -1271,6 +1271,9 @@ static void __init gic_of_setup_kvm_info(struct device_node *node) gic_set_kvm_info(&gic_v3_kvm_info); } +static const struct gic_quirk gic_quirks[] = { +}; + static int __init gic_of_init(struct device_node *node, struct device_node *parent) { void __iomem *dist_base; @@ -1318,6 +1321,8 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) redist_stride = 0; + gic_enable_of_quirks(node, gic_quirks, &gic_data); + err = gic_init_bases(dist_base, rdist_regs, nr_redist_regions, redist_stride, &node->fwnode); if (err)
This patch adds support to device tree based quirks based on device tree compatible string. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/irqchip/irq-gic-common.c | 12 ++++++++++++ drivers/irqchip/irq-gic-common.h | 3 +++ drivers/irqchip/irq-gic-v3.c | 5 +++++ 3 files changed, 20 insertions(+)