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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
