[PATCH 3/4]: Change arm-gic-its to support the Mbigen interrupt

[PATCH 3/4]: Change arm-gic-its to support the Mbigen interrupt

[majun (F)]

This patch is applied to support the mbigen interrupt.

Change log:
--For irq_mbigen.c using,move some struct and function definition
  to a new head file arm-gic-its.h
--Add a irq_write_mbi_msg member for mbi interrupt using
--For mbi interrupt, the event id depends on the Hardware pin number on mbigen,
  so, the its_get_event_id is changed to calclulate the event id of mbi interrupt
--For mbigen, the device id information need to be carried with msg. So,
  its_irq_compose_msi_msg is changed.
--its_mask_msi_irq and its_unmaks_msi_irq are modifid for mbi interrupt using.
--before the irq_ack callback, check the irq_ack first(chip.c)


Signed-off-by: Ma Jun <majun258@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c    |   71 +++++++++++-----------------------
 include/linux/irq.h                 |    1 +
 include/linux/irqchip/arm-gic-its.h |   68 +++++++++++++++++++++++++++++++++
 kernel/irq/chip.c                   |   12 ++++--
 4 files changed, 100 insertions(+), 52 deletions(-)
 mode change 100644 => 100755 drivers/irqchip/irq-gic-v3-its.c
 create mode 100755 include/linux/irqchip/arm-gic-its.h

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
old mode 100644
new mode 100755
index 9687f8a..21c36bf
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -22,6 +22,7 @@
 #include <linux/log2.h>
 #include <linux/mm.h>
 #include <linux/msi.h>
+#include <linux/mbi.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -30,7 +31,7 @@
 #include <linux/percpu.h>
 #include <linux/slab.h>

-#include <linux/irqchip/arm-gic-v3.h>
+#include <linux/irqchip/arm-gic-its.h>

 #include <asm/cacheflush.h>
 #include <asm/cputype.h>
@@ -42,36 +43,6 @@

 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)

-/*
- * Collection structure - just an ID, and a redistributor address to
- * ping. We use one per CPU as a bag of interrupts assigned to this
- * CPU.
- */
-struct its_collection {
-	u64			target_address;
-	u16			col_id;
-};
-
-/*
- * The ITS structure - contains most of the infrastructure, with the
- * msi_controller, the command queue, the collections, and the list of
- * devices writing to it.
- */
-struct its_node {
-	raw_spinlock_t		lock;
-	struct list_head	entry;
-	struct msi_controller	msi_chip;
-	struct irq_domain	*domain;
-	void __iomem		*base;
-	unsigned long		phys_base;
-	struct its_cmd_block	*cmd_base;
-	struct its_cmd_block	*cmd_write;
-	void			*tables[GITS_BASER_NR_REGS];
-	struct its_collection	*collections;
-	struct list_head	its_device_list;
-	u64			flags;
-	u32			ite_size;
-};

 #define ITS_ITT_ALIGN		SZ_256

@@ -79,17 +50,6 @@ struct its_node {
  * The ITS view of a device - belongs to an ITS, a collection, owns an
  * interrupt translation table, and a list of interrupts.
  */
-struct its_device {
-	struct list_head	entry;
-	struct its_node		*its;
-	struct its_collection	*collection;
-	void			*itt;
-	unsigned long		*lpi_map;
-	irq_hw_number_t		lpi_base;
-	int			nr_lpis;
-	u32			nr_ites;
-	u32			device_id;
-};

 static LIST_HEAD(its_nodes);
 static DEFINE_SPINLOCK(its_lock);
@@ -528,7 +488,11 @@ static void its_send_invall(struct its_node *its, struct its_collection *col)
 static inline u32 its_get_event_id(struct irq_data *d)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-	return d->hwirq - its_dev->lpi_base;
+
+	if (!is_mbigen_domain(d))
+		return d->hwirq - its_dev->lpi_base;
+	else
+		return get_mbi_offset(d->irq);
 }

 static void lpi_set_config(struct irq_data *d, bool enable)
@@ -599,7 +563,13 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)

 	msg->address_lo		= addr & ((1UL << 32) - 1);
 	msg->address_hi		= addr >> 32;
-	msg->data		= its_get_event_id(d);
+
+	/*for devices connect to mbigen, device id
+	 *information is needed*/
+	if (!is_mbigen_domain(d))
+		msg->data = its_get_event_id(d);
+	else
+		msg->data = (its_dev->device_id << 16) | its_get_event_id(d);
 }

 static struct irq_chip its_irq_chip = {
@@ -613,13 +583,15 @@ static struct irq_chip its_irq_chip = {

 static void its_mask_msi_irq(struct irq_data *d)
 {
-	pci_msi_mask_irq(d);
+	if (!is_mbigen_domain(d))
+		pci_msi_mask_irq(d);
 	irq_chip_mask_parent(d);
 }

 static void its_unmask_msi_irq(struct irq_data *d)
 {
-	pci_msi_unmask_irq(d);
+	if (!is_mbigen_domain(d))
+		pci_msi_unmask_irq(d);
 	irq_chip_unmask_parent(d);
 }

@@ -629,6 +601,8 @@ static struct irq_chip its_msi_irq_chip = {
 	.irq_mask		= its_mask_msi_irq,
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_write_msi_msg	= pci_msi_domain_write_msg,
+	.irq_write_mbi_msg 	= mbigen_write_msg,
+	.irq_ack		= irq_chip_ack_parent,
 };

 /*
@@ -721,6 +695,7 @@ static void its_lpi_free(unsigned long *bitmap, int base, int nr_ids)

 	for (lpi = base; lpi < (base + nr_ids); lpi += IRQS_PER_CHUNK) {
 		int chunk = its_lpi_to_chunk(lpi);
+
 		BUG_ON(chunk > lpi_chunks);
 		if (test_bit(chunk, lpi_bitmap)) {
 			clear_bit(chunk, lpi_bitmap);
@@ -1067,7 +1042,7 @@ static void its_cpu_init_collection(void)
 	spin_unlock(&its_lock);
 }

-static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
+struct its_device *its_find_device(struct its_node *its, u32 dev_id)
 {
 	struct its_device *its_dev = NULL, *tmp;
 	unsigned long flags;
@@ -1086,7 +1061,7 @@ static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
 	return its_dev;
 }

-static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
+struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 					    int nvecs)
 {
 	struct its_device *dev;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 62c6901..9a745a8 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -365,6 +365,7 @@ struct irq_chip {

 	void		(*irq_compose_msi_msg)(struct irq_data *data, struct msi_msg *msg);
 	void		(*irq_write_msi_msg)(struct irq_data *data, struct msi_msg *msg);
+	void		(*irq_write_mbi_msg)(struct irq_data *irq_data, struct msi_msg *msg);

 	int		(*irq_get_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool *state);
 	int		(*irq_set_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool state);
diff --git a/include/linux/irqchip/arm-gic-its.h b/include/linux/irqchip/arm-gic-its.h
new file mode 100755
index 0000000..5677114
--- /dev/null
+++ b/include/linux/irqchip/arm-gic-its.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2013, 2014 ARM Limited, All Rights Reserved.
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __LINUX_IRQCHIP_ARM_GIC_ITS_H
+#define __LINUX_IRQCHIP_ARM_GIC_ITS_H
+
+#include <linux/irqchip/arm-gic-v3.h>
+
+struct its_device {
+	struct list_head	entry;
+	struct its_node		*its;
+	struct its_collection	*collection;
+	void			*itt;
+	unsigned long		*lpi_map;
+	irq_hw_number_t		lpi_base;
+	int			nr_lpis;
+	u32			nr_ites;
+	u32			device_id;
+};
+/*
+ * Collection structure - just an ID, and a redistributor address to
+ * ping. We use one per CPU as a bag of interrupts assigned to this
+ * CPU.
+ */
+struct its_collection {
+	u64			target_address;
+	u16			col_id;
+};
+
+/*
+ * The ITS structure - contains most of the infrastructure, with the
+ * msi_controller, the command queue, the collections, and the list of
+ * devices writing to it.
+ */
+struct its_node {
+	raw_spinlock_t		lock;
+	struct list_head	entry;
+	struct msi_controller	msi_chip;
+	struct irq_domain	*domain;
+	void __iomem		*base;
+	unsigned long		phys_base;
+	struct its_cmd_block	*cmd_base;
+	struct its_cmd_block	*cmd_write;
+	void			*tables[GITS_BASER_NR_REGS];
+	struct its_collection	*collections;
+	struct list_head	its_device_list;
+	u64			flags;
+	u32			ite_size;
+};
+
+struct its_device *its_find_device(struct its_node *its, u32 dev_id);
+struct its_device *its_create_device(struct its_node *its, u32 dev_id,
+					    int nvecs);
+#endif
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index eb9a4ea..3020efc 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -882,7 +882,8 @@ void irq_cpu_offline(void)
 void irq_chip_ack_parent(struct irq_data *data)
 {
 	data = data->parent_data;
-	data->chip->irq_ack(data);
+	if (data->chip->irq_ack)
+		data->chip->irq_ack(data);
 }

 /**
@@ -892,7 +893,8 @@ void irq_chip_ack_parent(struct irq_data *data)
 void irq_chip_mask_parent(struct irq_data *data)
 {
 	data = data->parent_data;
-	data->chip->irq_mask(data);
+	if (data->chip->irq_mask)
+		data->chip->irq_mask(data);
 }

 /**
@@ -902,7 +904,8 @@ void irq_chip_mask_parent(struct irq_data *data)
 void irq_chip_unmask_parent(struct irq_data *data)
 {
 	data = data->parent_data;
-	data->chip->irq_unmask(data);
+	if (data->chip->irq_unmask)
+		data->chip->irq_unmask(data);
 }

 /**
@@ -912,7 +915,8 @@ void irq_chip_unmask_parent(struct irq_data *data)
 void irq_chip_eoi_parent(struct irq_data *data)
 {
 	data = data->parent_data;
-	data->chip->irq_eoi(data);
+	if (data->chip->irq_eoi)
+		data->chip->irq_eoi(data);
 }

 /**
-- 
1.7.1

Re: [PATCH 3/4]: Change arm-gic-its to support the Mbigen interrupt

[Marc Zyngier]

On 30/05/15 04:19, majun (F) wrote:
> This patch is applied to support the mbigen interrupt.
> 
> Change log:
> --For irq_mbigen.c using,move some struct and function definition
>   to a new head file arm-gic-its.h
> --Add a irq_write_mbi_msg member for mbi interrupt using
> --For mbi interrupt, the event id depends on the Hardware pin number on mbigen,
>   so, the its_get_event_id is changed to calclulate the event id of mbi interrupt
> --For mbigen, the device id information need to be carried with msg. So,
>   its_irq_compose_msi_msg is changed.
> --its_mask_msi_irq and its_unmaks_msi_irq are modifid for mbi interrupt using.
> --before the irq_ack callback, check the irq_ack first(chip.c)
> 
> 
> Signed-off-by: Ma Jun <majun258@huawei.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c    |   71 +++++++++++-----------------------
>  include/linux/irq.h                 |    1 +
>  include/linux/irqchip/arm-gic-its.h |   68 +++++++++++++++++++++++++++++++++
>  kernel/irq/chip.c                   |   12 ++++--
>  4 files changed, 100 insertions(+), 52 deletions(-)
>  mode change 100644 => 100755 drivers/irqchip/irq-gic-v3-its.c
>  create mode 100755 include/linux/irqchip/arm-gic-its.h
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> old mode 100644
> new mode 100755
> index 9687f8a..21c36bf
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -22,6 +22,7 @@
>  #include <linux/log2.h>
>  #include <linux/mm.h>
>  #include <linux/msi.h>
> +#include <linux/mbi.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> @@ -30,7 +31,7 @@
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
> 
> -#include <linux/irqchip/arm-gic-v3.h>
> +#include <linux/irqchip/arm-gic-its.h>
> 
>  #include <asm/cacheflush.h>
>  #include <asm/cputype.h>
> @@ -42,36 +43,6 @@
> 
>  #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
> 
> -/*
> - * Collection structure - just an ID, and a redistributor address to
> - * ping. We use one per CPU as a bag of interrupts assigned to this
> - * CPU.
> - */
> -struct its_collection {
> -	u64			target_address;
> -	u16			col_id;
> -};
> -
> -/*
> - * The ITS structure - contains most of the infrastructure, with the
> - * msi_controller, the command queue, the collections, and the list of
> - * devices writing to it.
> - */
> -struct its_node {
> -	raw_spinlock_t		lock;
> -	struct list_head	entry;
> -	struct msi_controller	msi_chip;
> -	struct irq_domain	*domain;
> -	void __iomem		*base;
> -	unsigned long		phys_base;
> -	struct its_cmd_block	*cmd_base;
> -	struct its_cmd_block	*cmd_write;
> -	void			*tables[GITS_BASER_NR_REGS];
> -	struct its_collection	*collections;
> -	struct list_head	its_device_list;
> -	u64			flags;
> -	u32			ite_size;
> -};
> 
>  #define ITS_ITT_ALIGN		SZ_256
> 
> @@ -79,17 +50,6 @@ struct its_node {
>   * The ITS view of a device - belongs to an ITS, a collection, owns an
>   * interrupt translation table, and a list of interrupts.
>   */
> -struct its_device {
> -	struct list_head	entry;
> -	struct its_node		*its;
> -	struct its_collection	*collection;
> -	void			*itt;
> -	unsigned long		*lpi_map;
> -	irq_hw_number_t		lpi_base;
> -	int			nr_lpis;
> -	u32			nr_ites;
> -	u32			device_id;
> -};

As I said before, there is no way an external driver is going to touch
these. These structure are private to the ITS, and are definitely going
to stay that way.

> 
>  static LIST_HEAD(its_nodes);
>  static DEFINE_SPINLOCK(its_lock);
> @@ -528,7 +488,11 @@ static void its_send_invall(struct its_node *its, struct its_collection *col)
>  static inline u32 its_get_event_id(struct irq_data *d)
>  {
>  	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
> -	return d->hwirq - its_dev->lpi_base;
> +
> +	if (!is_mbigen_domain(d))
> +		return d->hwirq - its_dev->lpi_base;
> +	else
> +		return get_mbi_offset(d->irq);
>  }

So on top of exposing the guts of the ITS code to the outside world, you
are creating a compile-time dependency between the ITS and your MBI
thing. No way.

> 
>  static void lpi_set_config(struct irq_data *d, bool enable)
> @@ -599,7 +563,13 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
> 
>  	msg->address_lo		= addr & ((1UL << 32) - 1);
>  	msg->address_hi		= addr >> 32;
> -	msg->data		= its_get_event_id(d);
> +
> +	/*for devices connect to mbigen, device id
> +	 *information is needed*/
> +	if (!is_mbigen_domain(d))
> +		msg->data = its_get_event_id(d);
> +	else
> +		msg->data = (its_dev->device_id << 16) | its_get_event_id(d);
>  }
> 
>  static struct irq_chip its_irq_chip = {
> @@ -613,13 +583,15 @@ static struct irq_chip its_irq_chip = {
> 
>  static void its_mask_msi_irq(struct irq_data *d)
>  {
> -	pci_msi_mask_irq(d);
> +	if (!is_mbigen_domain(d))
> +		pci_msi_mask_irq(d);
>  	irq_chip_mask_parent(d);
>  }
> 
>  static void its_unmask_msi_irq(struct irq_data *d)
>  {
> -	pci_msi_unmask_irq(d);
> +	if (!is_mbigen_domain(d))
> +		pci_msi_unmask_irq(d);
>  	irq_chip_unmask_parent(d);
>  }
> 
> @@ -629,6 +601,8 @@ static struct irq_chip its_msi_irq_chip = {
>  	.irq_mask		= its_mask_msi_irq,
>  	.irq_eoi		= irq_chip_eoi_parent,
>  	.irq_write_msi_msg	= pci_msi_domain_write_msg,
> +	.irq_write_mbi_msg 	= mbigen_write_msg,
> +	.irq_ack		= irq_chip_ack_parent,
>  };
> 

Do you realize that you're hacking into code that is PCI specific, and
that most of your changes could be solved elegantly by just having an
MBI specific irqchip/domain?

>  /*
> @@ -721,6 +695,7 @@ static void its_lpi_free(unsigned long *bitmap, int base, int nr_ids)
> 
>  	for (lpi = base; lpi < (base + nr_ids); lpi += IRQS_PER_CHUNK) {
>  		int chunk = its_lpi_to_chunk(lpi);
> +
>  		BUG_ON(chunk > lpi_chunks);
>  		if (test_bit(chunk, lpi_bitmap)) {
>  			clear_bit(chunk, lpi_bitmap);
> @@ -1067,7 +1042,7 @@ static void its_cpu_init_collection(void)
>  	spin_unlock(&its_lock);
>  }
> 
> -static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
> +struct its_device *its_find_device(struct its_node *its, u32 dev_id)
>  {
>  	struct its_device *its_dev = NULL, *tmp;
>  	unsigned long flags;
> @@ -1086,7 +1061,7 @@ static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
>  	return its_dev;
>  }
> 
> -static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
> +struct its_device *its_create_device(struct its_node *its, u32 dev_id,
>  					    int nvecs)
>  {
>  	struct its_device *dev;
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 62c6901..9a745a8 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -365,6 +365,7 @@ struct irq_chip {
> 
>  	void		(*irq_compose_msi_msg)(struct irq_data *data, struct msi_msg *msg);
>  	void		(*irq_write_msi_msg)(struct irq_data *data, struct msi_msg *msg);
> +	void		(*irq_write_mbi_msg)(struct irq_data *irq_data, struct msi_msg *msg);
> 
>  	int		(*irq_get_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool *state);
>  	int		(*irq_set_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool state);
> diff --git a/include/linux/irqchip/arm-gic-its.h b/include/linux/irqchip/arm-gic-its.h
> new file mode 100755
> index 0000000..5677114
> --- /dev/null
> +++ b/include/linux/irqchip/arm-gic-its.h
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (C) 2013, 2014 ARM Limited, All Rights Reserved.
> + * Author: Marc Zyngier <marc.zyngier@arm.com>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __LINUX_IRQCHIP_ARM_GIC_ITS_H
> +#define __LINUX_IRQCHIP_ARM_GIC_ITS_H
> +
> +#include <linux/irqchip/arm-gic-v3.h>
> +
> +struct its_device {
> +	struct list_head	entry;
> +	struct its_node		*its;
> +	struct its_collection	*collection;
> +	void			*itt;
> +	unsigned long		*lpi_map;
> +	irq_hw_number_t		lpi_base;
> +	int			nr_lpis;
> +	u32			nr_ites;
> +	u32			device_id;
> +};
> +/*
> + * Collection structure - just an ID, and a redistributor address to
> + * ping. We use one per CPU as a bag of interrupts assigned to this
> + * CPU.
> + */
> +struct its_collection {
> +	u64			target_address;
> +	u16			col_id;
> +};
> +
> +/*
> + * The ITS structure - contains most of the infrastructure, with the
> + * msi_controller, the command queue, the collections, and the list of
> + * devices writing to it.
> + */
> +struct its_node {
> +	raw_spinlock_t		lock;
> +	struct list_head	entry;
> +	struct msi_controller	msi_chip;
> +	struct irq_domain	*domain;
> +	void __iomem		*base;
> +	unsigned long		phys_base;
> +	struct its_cmd_block	*cmd_base;
> +	struct its_cmd_block	*cmd_write;
> +	void			*tables[GITS_BASER_NR_REGS];
> +	struct its_collection	*collections;
> +	struct list_head	its_device_list;
> +	u64			flags;
> +	u32			ite_size;
> +};
> +
> +struct its_device *its_find_device(struct its_node *its, u32 dev_id);
> +struct its_device *its_create_device(struct its_node *its, u32 dev_id,
> +					    int nvecs);
> +#endif
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index eb9a4ea..3020efc 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -882,7 +882,8 @@ void irq_cpu_offline(void)
>  void irq_chip_ack_parent(struct irq_data *data)
>  {
>  	data = data->parent_data;
> -	data->chip->irq_ack(data);
> +	if (data->chip->irq_ack)
> +		data->chip->irq_ack(data);
>  }
> 
>  /**
> @@ -892,7 +893,8 @@ void irq_chip_ack_parent(struct irq_data *data)
>  void irq_chip_mask_parent(struct irq_data *data)
>  {
>  	data = data->parent_data;
> -	data->chip->irq_mask(data);
> +	if (data->chip->irq_mask)
> +		data->chip->irq_mask(data);
>  }
> 
>  /**
> @@ -902,7 +904,8 @@ void irq_chip_mask_parent(struct irq_data *data)
>  void irq_chip_unmask_parent(struct irq_data *data)
>  {
>  	data = data->parent_data;
> -	data->chip->irq_unmask(data);
> +	if (data->chip->irq_unmask)
> +		data->chip->irq_unmask(data);
>  }
> 
>  /**
> @@ -912,7 +915,8 @@ void irq_chip_unmask_parent(struct irq_data *data)
>  void irq_chip_eoi_parent(struct irq_data *data)
>  {
>  	data = data->parent_data;
> -	data->chip->irq_eoi(data);
> +	if (data->chip->irq_eoi)
> +		data->chip->irq_eoi(data);
>  }
> 
>  /**
> 

Like I said in patch #1, you need to come up with a different approach.
Use the LPI layer of the ITS as the parent for your MBI, stop messing
with the PCI stuff, stop poking the ITS internals.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 3/4]: Change arm-gic-its to support the Mbigen interrupt

[majun (F)]

Hi Marc:
	Thanks for your review.

Accroding to my initial scheme, mbigen and pci devices uses the
same parent domain--MSI domain.

But accroding to your opinion, it seems use the ITS domain as parent domain
of Mbigne is a better way. Am i right ?

Thanks
Ma Jun


在 2015/6/1 17:13, Marc Zyngier 写道:
> Like I said in patch #1, you need to come up with a different approach.
> Use the LPI layer of the ITS as the parent for your MBI, stop messing
> with the PCI stuff, stop poking the ITS internals.
> 
> Thanks,
> 
> 	M.

Re: [PATCH 3/4]: Change arm-gic-its to support the Mbigen interrupt

[Marc Zyngier]

On 02/06/15 12:34, majun (F) wrote:
> Hi Marc: Thanks for your review.
> 
> Accroding to my initial scheme, mbigen and pci devices uses the same
> parent domain--MSI domain.
> 
> But accroding to your opinion, it seems use the ITS domain as parent
> domain of Mbigne is a better way. Am i right ?

Let's face it, your MBIGEN is not a PCI device at all. It is something
else entirely, and you're just adding complexity to a subsystem that
really doesn't need it.

So why would you hack the PCI domain implementation at all? The kernel
now has the right level of abstraction, where you can provide interrupts
to different implementations of MSIs.

Even better, you can base your MBIGEN domain on the core code that lives
in kernel/irq/msi.c, just like PCI does (drivers/pci/msi.c). You will
end up with something like:

PCI/MSI --> ITS --> GIC
MBIGEN  _/

where both domains can coexist happily, and still share a common code
base. This shouldn't require any hack either. The only possible snag is
to be able to obtain a pointer to the ITS domain (there is currently no
provision for that), but that should be easily solved.

My point here is that if you need to do touch the core code, you need to
be able to justify it. So far, I haven't seen anything in your HW that
would require the kind of redesign you've posted.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...