On 31/10/15 00:07, Arnd Bergmann wrote:
On Wednesday 28 October 2015 14:29:39 Andrew Donnellan wrote:
--- a/drivers/misc/cxl/vphb.c
+++ b/drivers/misc/cxl/vphb.c
@@ -128,7 +128,7 @@ static int cxl_pcie_config_info(struct pci_bus *bus,
unsigned int devfn,
return PCIBIOS_BAD_REGISTER_NUMBER;
addr = cxl_pcie_cfg_addr(phb, bus->number, devfn, offset);
- *ioaddr = (void *)(addr & ~0x3ULL);
+ *ioaddr = (void __iomem *)(addr & ~0x3ULL);
*shift = ((addr & 0x3) * 8);
switch (len) {
case 1:
It would be nice to change the return value of cxl_pcie_cfg_addr to
'void __iomem *' as well, and only do the cast (back and forth) inside
the calculation, to make it clear that the input type is the same as the
output. Perhaps use a static inline function to wrap it.
That would work, not sure if I'd bother with a wrapper function.
@@ -249,7 +249,7 @@ int cxl_pci_vphb_add(struct cxl_afu *afu)
/* Setup the PHB using arch provided callback */
phb->ops = &cxl_pcie_pci_ops;
phb->cfg_addr = afu->afu_desc_mmio + afu->crs_offset;
- phb->cfg_data = (void *)(u64)afu->crs_len;
+ phb->cfg_data = (void __iomem *)afu->crs_len;
phb->private_data = afu;
phb->controller_ops = cxl_pci_controller_ops;
This needs a comment to explain why this is correct. I've tried to find it
out by reading the code and could not find any explanation. Also, you
need to cast to an intermediate (uintptr_t) type, as directly converting
a u64 to a pointer of any sort is nonportable, and it would be good to
at least allow compile-testing this code on other architectures.
It's impossible to compile cxl on other architectures given that we
depend on powerpc- and powernv-specific functions... but in any case, I
suppose using uintptr_t is more correct.
I suspect that 'phb->cfg_data' is used in this driver in a way that is
incompatible with the other uses of the same variable. Maybe you can
replace it with something like
union {
void __iomem *cfg_data;
u64 cxl_cfg_offset;
};
to make it clear that in this driver it is used as an offset rather than
a pointer.
You're right, I hadn't looked closely at exactly how it was been used.
Ian, thoughts?
Andrew
--
Andrew Donnellan Software Engineer, OzLabs
andrew.donnel...@au1.ibm.com Australia Development Lab, Canberra
+61 2 6201 8874 (work) IBM Australia Limited
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
