On Wed, Jan 13, 2016 at 04:13:07PM +0100, Andrew Jones wrote:
> > diff --git a/lib/pci-host-generic.h b/lib/pci-host-generic.h
> > new file mode 100644
> > index 0000000..097ac2d
> > --- /dev/null
> > +++ b/lib/pci-host-generic.h
> > @@ -0,0 +1,26 @@
> > +#ifndef PCI_HOST_GENERIC_H
> > +#define PCI_HOST_GENERIC_H
> > +
> > +struct pci_addr_range {
> > + phys_addr_t start;
> > + phys_addr_t size;
> > +};
> > +
> > +struct pci_addr_space {
> > + struct pci_addr_range cpu_range;
> > + struct pci_addr_range pci_range;
> > + phys_addr_t alloc;
>
> alloc isn't used in this patch, but guess it will be later
>
> > + u32 of_flags;
> > +};
> > +
> > +struct pci_host_bridge {
> > + struct pci_addr_range cpu_range;
> > + int bus;
> > + int bus_max;
> > + int nr_addr_spaces;
> > + struct pci_addr_space addr_space[];
> > +};
> > +
> > +#define PCI_ECAM_BUS_SIZE (1 << 20)
>
> Why not put this struct declaration (and supporting structs) in pci.h?
> Linux has struct pci_host_bridge in include/linux/pci.h
I think we do not need any of these structures to get public.
My initial attempt was also an assessment of an idea to
generalize PCI code across architectures. But my current
judgement - an outcome is not worth it. With x86 PCI access
so much different and only bus 0 function 0 "hierarchy" we
only need the simplest required routines.
Hence, I would leave these in pci-host-generic.h (just not
to blow pci-host-generic.c too big).
As result, just these routines would end up in pci.h:
bool pci_probe(void);
void pci_free(void); /* pci_shutdown() ? */
int pci_bus_scan(void);
int pci_testdev(void);
Thoughts?
> > +
> > +#endif
> > diff --git a/lib/pci.h b/lib/pci.h
> > new file mode 100644
> > index 0000000..22b8e31
> > --- /dev/null
> > +++ b/lib/pci.h
> > @@ -0,0 +1,13 @@
> > +#ifndef PCI_H
> > +#define PCI_H
>
> This file needs its copyright/gpl header. I also like to document
> the lib here in the header file, e.g. see lib/devicetree.h and
> lib/alloc.h
>
> > +
> > +#include "alloc.h"
> > +
> > +struct pci {
> > + void *sysdata;
>
> The choice of sysdata for the name appears to come from the Linux
> kernel. Why not also call 'struct pci' 'struct pci_dev' for full
> consistency?
>
> > +};
> > +
> > +extern struct pci *pci_dt_probe(void);
> > +extern void pci_shutdown(struct pci *pci);
> > +
> > +#endif
> > --
> > 1.8.3.1
> >
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
