Hi Honghui & Bjorn,

On Tuesday, 8 August 2017 23:49:52 PDT Honghui Zhang wrote:

On Tue, 2017-08-08 at 15:19 -0500, Bjorn Helgaas wrote:

quoted

On Fri, Aug 04, 2017 at 08:18:09AM -0500, Bjorn Helgaas wrote:

quoted

On Fri, Aug 04, 2017 at 04:39:36PM +0800, Honghui Zhang wrote:

quoted

On Thu, 2017-08-03 at 17:42 -0500, Bjorn Helgaas wrote:

quoted

quoted

+	port->irq_domain = irq_domain_add_linear(pcie_intc_node,
INTX_NUM,
+						 &intx_domain_ops, port);

I think there's an issue here with a 4-element IRQ domain and the
hwirq numbers 1-4 from the of_irq_parse_and_map_pci() path, so INTD
may not work correctly.

See
http://lkml.kernel.org/r/20170801212931.GA26498 at bhelgaas-glaptop.roa
m.corp.google.com and related discussion.

Sorry, I did not get this,
I do some test with an intel E350T4 PCIe NICs, it's a x1 lane
multi-function device.
What I got from the log is below:
->of_irq_parse_and_map_pci

	->of_irq_parse_pci
	
		->irq_create_of_mapping
		
			->irq_create_fwspec_mapping
			
				->irq_domain_translate
				which will go through
				d->ops->translate #the hwirq really start from 0

And I tested every NIC port of the Intel E350T4 with tftp transfer
data,
seems all are OK with this code.

OK.  I don't know what d->ops->translate is involved here, but if it
works, I guess this is OK for now.  We're trying to clean this up and
make it consistent across all the drivers.  Many of them allocate a
5-element IRQ domain, some make a 4-element domain, and on some of
them INTD doesn't work.  It's a mess.

Paul Burton is cleaning this up.  Can you point out the d->ops->translate
function that's involved here?

Hi, Bjorn,

Sorry for my last reply, I was tracking the wrong logs. The real trick
is here:

->of_irq_parse_and_map_pci
  ->of_irq_parse_pci	#out_irq->args[0] start from 1(1 == INTA)
     ->of_irq_parse_raw

After of_irq_parse_raw finished it's work, the out_irq->args[0] will be
remapped as "interrupt-map" property defines[1], which in my case, it's
start from 0, and then fwspec->param[0] is start from 0 (0 == INTA).

My "interrupt-map" property is defined as below:
interrupt-map = <0 0 0 1 &pcie_intc0 0>,
		<0 0 0 2 &pcie_intc0 1>,
		<0 0 0 3 &pcie_intc0 2>,
		<0 0 0 4 &pcie_intc0 3>;

I do some test with the changes of property defined as below:
interrupt-map = <0 0 0 1 &pcie_intc0 1>,
		<0 0 0 2 &pcie_intc0 2>,
		<0 0 0 3 &pcie_intc0 3>,
		<0 0 0 4 &pcie_intc0 4>;
Then I got the same running complain as Paul have got[2]

So I guess it's the "interrupt-map" property defined in dtsi node play
the key role in this.

[1]http://elixir.free-electrons.com/linux/v4.13-rc4/source/drivers/of/irq.c#
L265 [2]https://patchwork.kernel.org/patch/9794355

thanks.

That seems like a possibly more sensible way to handle it, so long as you have 
control over all the device trees that the driver may be exposed to.

Bjorn: is this something you want to deal with on a driver by driver basis? 
ie. new drivers just use interrupt-map as above and older ones with existing 
DT bindings use the xlate function? Looking at the drivers & device trees we 
have in-tree it seems we already have a mix of 0-3 & 1-4 ranges in use, so 
we'd just need to use the xlate function for those which currently use 1-4.

I had originally done the same thing with interrupt-map on the MIPS Boston 
board[1] and the Xilinx PCIe driver[2], though that change would break any 
pre-existing device trees that use 1-4 in the interrupt-map property (which is 
sadly what the driver's binding document shows...).

Thanks,
    Paul

[1] https://www.linux-mips.org/archives/linux-mips/2016-08/msg00425.html
[2] https://patchwork.kernel.org/patch/9763191/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170809/cde7d45f/attachment.sig>