diff mbox series

[v4,2/4] irqchip/gic: common: add support to device tree based quirks

Message ID 20181210102309.8207-3-srinivas.kandagatla@linaro.org
State Superseded
Headers show
Series irqchip/gic-v3: Add support to DT based quirk for msm8996 | expand

Commit Message

Srinivas Kandagatla Dec. 10, 2018, 10:23 a.m. UTC 
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(+)

-- 
2.19.2

Comments

Marc Zyngier Dec. 10, 2018, 10:55 a.m. UTC | #1 
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.
-- 
Jazz is not dead. It just smells funny...
Srinivas Kandagatla Dec. 10, 2018, 1:19 p.m. UTC | #2 
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 mbox series

Patch

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)