On 03/11/15 20:09, Michael Ellerman wrote:
Part of your problem is you're storing afu->crs_len which is not __iomem in
cfg_data which is, and so that's leading to some of your casts.
I don't really see why you're using cfg_data like that, you have the afu in
phb->private_data. But maybe cfg_data needs to hold that value for some other
code I'm not seeing.
I can't see any obvious reason why we need to use cfg_data either.
Regardless, in cxl_pcie_config_info() you have the afu pointer, so you can just
look at afu->crs_len directly can't you?
That means you can drop one cast of cfg_data to unsigned long in there.
Then I see that cxl_pcie_cfg_addr() is only used in cxl_pcie_config_info(), and
doesn't abstract much. So I'd drop it and inline the logic. That leads to the
realisation that we're calling cxl_pcie_cfg_record() twice, so we can save the
value and only call it once.
You're then left with:
addr = phb->cfg_addr + (afu->crs_len * record) + offset;
*ioaddr = (void *)(addr & ~0x3ULL);
*shift = ((addr & 0x3) * 8);
Will do, that's nicer.
Ideally we'd be able to say that cfg_addr is always aligned, and so it doesn't
need to be part of the calculation. I suspect that is true but you'll have to
check. If it is you can then leave cfg_addr out of the logic and you have:
addr = (afu->crs_len * record) + offset;
*ioaddr = phb->cfg_addr + (addr & ~0x3ull);
*shift = (addr & 0x3) * 8;
Which hopefully still gives you the right result! :)
Will check.
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
