On Fri, Feb 05, 2016 at 12:48:50PM +0100, Alexander Gordeev wrote:
> 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?
Whichever you prefer. We either "bloat" pci.h or we "bloat" the lib
directory with another file :-) Actually, I still prefer pci.h, because
if arm unit tests will always need both of them, then they'll always
need two #include lines... OTOH, if you don't think unit tests will
need these structures, then they could just be defined right in
pci-host-generic.c (or pci.c if you consolidate the source files).
Thanks,
drew
>
> > > +
> > > +#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
