On Mon, Aug 18, 2014 at 03:25:50PM +0100, Catalin Marinas wrote:

On Fri, Aug 15, 2014 at 11:30:52AM +0100, Liviu Dudau wrote:

quoted

On Fri, Aug 15, 2014 at 09:56:32AM +0100, Wei Yang wrote:

quoted

On Thu, Aug 14, 2014 at 04:49:59PM +0100, Liviu Dudau wrote:

quoted

On Thu, Aug 14, 2014 at 03:58:04PM +0100, Wei Yang wrote:

quoted

On Tue, Aug 12, 2014 at 05:25:15PM +0100, Liviu Dudau wrote:

quoted

int __weak pcibios_add_device(struct pci_dev *dev)
{
+	dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+
	return 0;
}

For this, my suggestion is to add arch dependent function to setup the irq
line for pci devices. I can't find an obvious reason this won't work on other
archs, but maybe this will hurt some of them?

I'm not sure I understand your point. Architectures that support OF will obviously
benefit from this common approach, and for the other ones the function is empty
so it will not change existing behaviour. If you are suggesting that I should
create a new API that each architecture could go and implement for setting up the
IRQ line then I would agree that it would be nice to have that, but the question
is how many architectures are outside OF that need this?

My suggestion is to define the pcibios_add_device() for arm arch, like the one
in arch/powerpc/kernel/pci-common.c. If my understanding is correct, this
patch set address the pci bus setup mostly on arm arch.

And also arm64 at the least.

...

quoted

Well, it will become necessary as old code gets dismantled and converted towards
this patchset. To give you an example that I'm familiar with, for arch/arm the
host bridge drivers have moved into drivers/pci/host, but they still depend/use
the bios32 infrastructure that takes care of setting up the irq. When they switch
to my version they would have to go and debug the "irq not being assigned" issue
and it is quite likely that some of the people doing the conversion will complain
about my code rather than understanding the issue. What I'm trying to do is to
make switching to my patchset as painless as possible, with a cleanup to remove
redundant operations coming after the switchover.

While the goal is fine, until we see a common pattern for what needs to
go into pcibios_add_device() I think we should have an arm64-specific
implementation (and probably an arm32 specific one as well). I can see
powerpc uses it for setting the DMA ops. Would we have a similar need on
arm64 to choose between coherent and non-coherent dma_ops?

At this point I would like to hear more from people doing the conversion of
the drivers. I cannot answer fully for all arm or arm64 drivers.

Also at some point we'll get ACPI support, so I'm not sure what we do
with assigning the dev->irq here but definitely of_* functions won't
work.

Yes, maybe adding a check if the IRQ has not been assigned already?
I have no idea what the order will be between ACPI ops and of_* ones.

Best regards,
Liviu

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...