[PATCH v3 1/3] IRQ/Gic-V3: Add mbigen driver to support mbigen interrupt controller

[marc.zyngier@arm.com (Marc Zyngier)]

On 08/07/15 05:21, majun (F) wrote:
> Hi Thomas:
> 
> ? 2015/7/6 20:33, Thomas Gleixner ??:
>> On Mon, 6 Jul 2015, Ma Jun wrote:
>>
> 
>>> +/**
>>> + * get_mbigen_node_type: get the mbigen node type
>>> + * @nid: the mbigen node value
>>> + * return 0: evnent id of interrupt connected to this node can be changed.
>>> + * return 1: evnent id of interrupt connected to this node cant be changed.
>>> + */
>>> +static int get_mbigen_node_type(int nid)
>>> +{
>>> +	if (nid > MG_NR) {
>>> +		pr_warn("MBIGEN: Device ID exceeds max number!\n");
>>> +		return 1;
>>> +	}
>>> +	if ((nid == 0) || (nid == 5) || (nid > 7))
>>> +		return 0;
>>> +	else
>>> +		return 1;
>>
>> Oh no. We do not hardcode such properties into a driver. That wants to
>> be in the device tree and set as a property in the node data structure.
>>
> Ok,I will move this to device tree
> 
>>> +static int mbigen_write_msg(struct irq_data *d, struct msi_msg *msg)
>>> +{
>>> +	struct mbigen_chip *chip = d->domain->host_data;
>>> +	void __iomem *addr;
>>> +	u32 nid, val, offset;
>>> +	int ret = 0;
>>> +
>>> +	nid = GET_NODE_NUM(d->hwirq);
>>> +	ret = get_mbigen_node_type(nid);
>>> +	if (ret)
>>> +		return 0;
>>
>> Care to explain what this does? It seems for some nodes you cannot
>> write the msi message. So how is that supposed to work? How is that
>> interrupt controlled (mask/unmask ...) ?
>>
> This function is used to write irq event id into vector register.Depends on
> hardware design, write operation is permitted in some mbigen node(nid=0,5,and >7),
> For other mbigen node, this register is read only.

So how do you expect this to work? You cannot program the event
generated by the mbigen, and the ITS has an ITT that probably doesn't
match your HW.

Best case, the interrupt is simply dropped, worse case you end up in an
interrupt storm because you can't figure out which device is screaming.

I'm a bit puzzled.

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