Re: [RFC PATCH 4/4] arm64, acpi, pci: Provide arch-specific calls for PCI host bridge dirver (PNP0A03).

[Tomasz Nowicki]

On 07.11.2014 15:24, Arnd Bergmann wrote:
> On Friday 07 November 2014 14:27:56 Tomasz Nowicki wrote:
> >   #ifdef CONFIG_PCI
> > +struct pci_controller {
> > +	struct acpi_device *companion;
> > +	int segment;
> > +	int node;		/* nearest node with memory or NUMA_NO_NODE for global allocation */
> > +};
> > +
> > +#define PCI_CONTROLLER(busdev) ((struct pci_controller *) busdev->sysdata)
> > +
>
> Don't use busdev->sysdata in architecture specific code, it belongs to the
> host bridge driver with the new model. For ACPI you don't have a host bridge
> driver, but it's better to keep these separate.
>
> The segment is always the same as the domain number, so just use that.
> The node and companion members here can get added to struct pci_host_bridge.

The reason why I put segment field to struct pci_controller is to 
initialize domain_nr of struct pci_bus being in pci_create_root_bus(), 
domain_nr can be used later on though. Correct me I am wrong.

Honestly I do not see the way to create root bus without e.g. 
sysdata.segment here.

> > @@ -43,7 +53,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> >    */
> >   int pcibios_add_device(struct pci_dev *dev)
> >   {
> > -	dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > +	if (acpi_disabled)
> > +		dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> >
> >   	return 0;
> >   }
>
> How do you assign the irq number with ACPI? Do you only support MSI?

I missed that, will add in next ver.

> >   /*
> >    * raw_pci_read/write - Platform-specific PCI config space access.
> > - *
> > - * Default empty implementation.  Replace with an architecture-specific setup
> > - * routine, if necessary.
> >    */
> >   int raw_pci_read(unsigned int domain, unsigned int bus,
> >   		  unsigned int devfn, int reg, int len, u32 *val)
> >   {
> > -	return -EINVAL;
> > +	char __iomem *addr;
> > +
> > +	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
> > +err:		*val = -1;
> > +		return -EINVAL;
> > +	}
> > +
> > +	rcu_read_lock();
> > +	addr = pci_dev_base(domain, bus, devfn);
> > +	if (!addr) {
> > +		rcu_read_unlock();
> > +		goto err;
> > +	}
>
> The config space accessors should probably be shared with
> drivers/pci/host/pci-host-generic.c, e.g. by moving the rest of
> the new code in there as well, or by moving the config space
> accessors from that file to drivers/pci/mmconfig.c.

Tomasz
--
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/