On Wed, Apr 13, 2016 at 08:39:35PM +0200, Alexander Gordeev wrote:
> On Wed, Apr 13, 2016 at 06:38:43PM +0200, Andrew Jones wrote:
> > On Mon, Apr 11, 2016 at 01:04:21PM +0200, Alexander Gordeev wrote:
> > > Cc: Thomas Huth <thuth@xxxxxxxxxx>
> > > Cc: Andrew Jones <drjones@xxxxxxxxxx>
> > > Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> > > ---
> > > lib/pci.c | 8 ++++++++
> > > lib/pci.h | 1 +
> > > 2 files changed, 9 insertions(+)
> > >
> > > diff --git a/lib/pci.c b/lib/pci.c
> > > index 43e9c0c38434..46aee60e0f90 100644
> > > --- a/lib/pci.c
> > > +++ b/lib/pci.c
> > > @@ -84,6 +84,14 @@ phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num)
> > > }
> > > }
> > >
> > > +void pci_bar_set(pcidevaddr_t dev, int bar_num, uint64_t addr)
> > > +{
> > > + int off = PCI_BASE_ADDRESS_0 + bar_num * 4;
> >
> > Blank line here please. Please run the kernel's checkpatch script on all
> > your patches. We strive to follow the kernel coding style. (Well, don't
> > look at the legacy code :-)
> >
> > > + pci_config_writel(dev, off, (uint32_t)addr);
> > > + if (pci_bar_is64(dev, bar_num))
> > > + pci_config_writel(dev, (uint32_t)(addr >> 32), off + 4);
> >
> > This pci_config_writel() call is using offset as the 3rd parameter, but
> > it should be the 2nd.
>
> Bummer.
>
> > Also, where do you introduce pci_config_writel()? I haven't closely
> > reviewed all the previous patches, but so far I only see uses of it, no
> > introduction of it. Does this series compile one patch at a time? We
> > should ensure it does to maintain bisectability.
>
> 06/15 "pci/x86: Add remaining PCI configuration space accessors"
>
> pci_config_read*/write* accessors are not expected implemented
> on archs that do not include lib/pci.c. Once lib/pci.c added
> to an arch accessors must be introduced to lib/asm/pci.h as well.
>
> Also, no generic implementation introduced as there is no such
> thing as generic PCI access.
>
> If it makes sense?
It does. And now I see why I didn't see it. You didn't CC me on 06/15,
so that message didn't land in my "to review" virtual folder, and I
didn't notice that the series jumped from 5 to 7 when I was looking it
over.
drew
>
> > Thanks,
> > drew
> >
> > > +}
> > > +
> > > bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
> > > {
> > > uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
> > > diff --git a/lib/pci.h b/lib/pci.h
> > > index 09b500ea19f0..69d2a62f1b32 100644
> > > --- a/lib/pci.h
> > > +++ b/lib/pci.h
> > > @@ -15,6 +15,7 @@ enum {
> > > PCIDEVADDR_INVALID = 0xffff,
> > > };
> > > pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> > > +void pci_bar_set(pcidevaddr_t dev, int bar_num, uint64_t addr);
> > > phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num);
> > > phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
> > > bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
> > > --
> > > 1.8.3.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
