Re: [RFC 2/4] PCI: generic: Add support for ARM64 and MSI(x)

[Lorenzo Pieralisi]

On Thu, Oct 23, 2014 at 10:13:09AM +0100, Liviu Dudau wrote:
> On Wed, Oct 22, 2014 at 09:52:19PM +0100, Arnd Bergmann wrote:
> > On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > > 
> > > [...]
> > > 
> > > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > > interfaces that set the domain number.
> > > 
> > > I wished, but it is a bit more complicated than I thought unfortunately,
> > > mostly because some drivers, eg cns3xxx set the domain numbers
> > > statically in pci_sys_data and this sets a chain of dependency that is
> > > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > > the domain number (in pci_sys_data) in a way that clashes with the
> > > generic domain_nr implementation, I need to give it more thought.
> > 
> > Just had a look at that driver, shouldn't be too hard to change, see below.
> 
> I like this!
> 
> One thing though ...

I like it too, it is one way of removing the artificial domain dependency
from this driver.

I think that by removing that, we could switch to CONFIG_PCI_DOMAINS_GENERIC
on ARM32. I will remove the dependency in drivers/pci/host/pci-mvebu.c
introduced by commit 2613ba48. pci_sys_data.domain is always 0 in that
driver so its usefulness is doubtful, comments welcome, copied Jason in
if he has comments.

[...]

> > @@ -323,6 +309,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> >  void __init cns3xxx_pcie_init_late(void)
> >  {
> >  	int i;
> > +	void *private_data;
> > +	struct hw_pci hw_pci = {
> > +		.nr_controllers = 1,
> > +		.ops = &cns3xxx_pcie_ops,
> > +		.setup = cns3xxx_pci_setup,
> > +		.map_irq = cns3xxx_pcie_map_irq,
> > +		.private_data = &private_data,
> > +	};
> >  
> >  	pcibios_min_io = 0;
> >  	pcibios_min_mem = 0;
> > @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void)
> >  		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
> >  		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
> >  		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> > -		pci_common_init(&cns3xxx_pcie[i].hw_pci);
> > +		hw_pci->domain = i;

+		hw_pci.domain = i;

I will remove this since if we move to generic domains it is useless to
pass the value through hw_pci.

> > +		private_data = &cns3xxx_pcie[i];
> 
> Is this dance with pointers absolutely necessary? Does gcc though dishes at you
> for doing hw_pci->private_data = &cns3xxx_pcie[i] directly?

You can't, hw_pci.private_data is void **.

Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/