Re: [PATCH 04/17] pci: Add arch_can_pci_mmap_wc() macro

[David Woodhouse]

On Tue, 2017-04-04 at 16:36 -0500, Bjorn Helgaas wrote:
> > --- a/arch/xtensa/kernel/pci.c
> > +++ b/arch/xtensa/kernel/pci.c
> > @@ -345,7 +345,7 @@ __pci_mmap_set_pgprot(struct pci_dev *dev, struct vm_area_struct *vma,
> > Â
> > Â	/* Set to write-through */
> > Â	prot = (prot & _PAGE_CA_MASK) | _PAGE_CA_WT;
> > -#if 0
> > +#ifdef arch_can_pci_mmap_wc
>
> This hunk seems like maybe it should be a separate patch.
> 
> ...
>
> This part seems different -- it changes the way pci_mmap_page_range()
> works.

It doesn't, because arch_can_pci_map_wc isn't defined on xtensa. So
it's just a trivial cleanup for future-proofing, turning that 'if 0'
into 'if <something that isn't set>'.

> > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> > index dc8912e..c49be71 100644
> > --- a/drivers/pci/proc.c
> > +++ b/drivers/pci/proc.c
> > @@ -209,15 +209,18 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
> > Â		fpriv->mmap_state = pci_mmap_mem;
> > Â		break;
> > Â
> > +#ifdef arch_can_pci_mmap_wc
> Can we get rid of these #ifdefs in the code by adding this to linux/pci.h?
> 
> Â #ifndef arch_can_pci_mmap_wc
> Â #define arch_can_pci_mmap_wc()	0
> Â #endif

Er.... at the time, that was non-trivial because there was something
that actually needed to be *removed* with the preprocessor instead of
just not being called. But after I settled on the incremental approach
of having pci_mmap_resource_range() be a wrapper for
pci_mmap_page_range() *and* vice versa I think that requirement went
away. I'll take another look and see if I can do that now; thanks.

Are you happy with the suggestion that we use HAVE_PCI_MMAP *only* to
control mmap on /proc/bus/pci, and we present mmap through sysfs on all
platforms via the generic code?

Attachment: smime.p7s
Description: S/MIME cryptographic signature