Convert resource to u64 from unsigned long
PATCH 1/5 Convert resource to u64 from unsigned long
Here's my first attempt of trying to convert the struct resource start/end to u64 per blessed by Linus =) This is to support >32bit physical address on 32bit archs such as some of the newer ARMv6 and XSC3 based platforms and perhaps IA32 PAE. I left the PCI stuff alone functionally. Supporting 64bit PCI BAR on 32bit archs is for another day. I fixed most of the core stuff I can think of, fixed ARM and i386 hopefully and a few of the device drivers as examples. I have tested on an IQ31244 XScale IOP (ARM) platform and a dual-xeon platform for i386. Matt Porter has graciously sent me PPC fixes that he tested on.
Signature · 11 lines
-= Dave =- Software Engineer - Advanced Development Engineering Team Storage Component Division - Intel Corp. mailto://dave.jiang @ intel http://sourceforge.net/projects/xscaleiop/ ---- The views expressed in this email are mine alone and do not necessarily reflect the views of my employer (Intel Corp.).
Quoted 10 lines · Dave
ere's my first attempt of trying to convert the struct resource start/end to u64 per blessed by Linus =) This is to support >32bit physical address on 32bit archs such as some of the newer ARMv6 and XSC3 based platforms and perhaps IA32 PAE. I left the PCI stuff alone functionally. Supporting 64bit PCI BAR on 32bit archs is for another day. I fixed most of the core stuff I can think of, fixed ARM and i386 hopefully and a few of the device drivers as examples. I have tested on an IQ31244 XScale IOP (ARM) platform and a dual-xeon platform for i386. Matt Porter has graciously sent me PPC fixes that he tested on.
OK, well Greg KH will be the main target of this work..
Can you do something a bit more friendly than application/octet-stream encoding, btw?
+#if BITS_PER_LONG == 64
+#define U64FMT "016lx"
+#else
+#define U64FMT "016Lx"
+#endif
We've avoided doing this. We prefer to do
printk("%llx", (unsigned long long)foo);which is tidier, although a little more runtime-costly.
Your approach assumes that all 64-bit architectures implement u64 as unsigned long (as opposed to unsigned long long, which I guess is legal?) I don't know if that's a problem or not.
Also, the patches introduce tons of ifdefs such as:
+#if BITS_PER_LONG == 64
return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE);
+#else
+ return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE);
+#endif
We really should find a way of avoiding this. Even if it is
#if BITS_PER_LONG == 64
#define resource_to_ptr(r) ((void *)(r))
#else
#define resource_to_ptr(r) ((void *)((u32)r))
#endif
in a header file somewhere. Open-coding the decision all over the place is unsightly.
Review
Quoted 8 lines · Andrew Morton
Also, the patches introduce tons of ifdefs such as:
+#if
BITS_PER_LONG== 64return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE);(quoted diff) +3
-0+#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endif
Ouch. Who does that, anyway? It's wrong to do that. It's not a pointer, not even an __iomem one. You'd need to do an ioremap() on it to turn it into a pointer.
Linus
Quoted 16 lines · Linus Torvalds
Review
Quoted 8 lines · Andrew Morton
Also, the patches introduce tons of ifdefs such as:
(quoted diff) +4
-0+#if BITS_PER_LONG == 64 return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endifOuch. Who does that, anyway? It's wrong to do that. It's not a pointer, not even an
__iomemone. You'd need to do anioremap()on it to turn it into a pointer.
Quoted 2 lines
Linus
It's in the PCI option ROM code and at first it thew me just a bit too. Apparently the resource.start is a kmalloc'd buffer and not really an actual bus address. Is that a gross abuse of the way struct resource is intended to be used?
Signature · 11 lines
-= Dave =- Software Engineer - Advanced Development Engineering Team Storage Component Division - Intel Corp. mailto://dave.jiang @ intel http://sourceforge.net/projects/xscaleiop/ ---- The views expressed in this email are mine alone and do not necessarily reflect the views of my employer (Intel Corp.).
Quoted 43 lines · Andrew Morton
Quoted 10 lines · Dave
ere's my first attempt of trying to convert the struct resource start/end to u64 per blessed by Linus =) This is to support >32bit physical address on 32bit archs such as some of the newer ARMv6 and XSC3 based platforms and perhaps IA32 PAE. I left the PCI stuff alone functionally. Supporting 64bit PCI BAR on 32bit archs is for another day. I fixed most of the core stuff I can think of, fixed ARM and i386 hopefully and a few of the device drivers as examples. I have tested on an IQ31244 XScale IOP (ARM) platform and a dual-xeon platform for i386. Matt Porter has graciously sent me PPC fixes that he tested on.
OK, well Greg KH will be the main target of this work..
Can you do something a bit more friendly than application/octet-stream encoding, btw?
(quoted diff) +5
-0+#if BITS_PER_LONG == 64 +#define U64FMT "016lx" +#else +#define U64FMT "016Lx" +#endifWe've avoided doing this. We prefer to do
printk("%llx", (unsigned long long)foo);which is tidier, although a little more runtime-costly.
Your approach assumes that all 64-bit architectures implement u64 as unsigned long (as opposed to unsigned long long, which I guess is legal?) I don't know if that's a problem or not.
Also, the patches introduce tons of ifdefs such as:
(quoted diff) +4
-0+#if BITS_PER_LONG == 64 return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endifWe really should find a way of avoiding this. Even if it is
Quoted 9 lines
#if
BITS_PER_LONG== 64
#defineresource_to_ptr(r)((void *)(r))
#else
#defineresource_to_ptr(r)((void *)((u32)r))
#endifin a header file somewhere. Open-coding the decision all over the place is unsightly.
Ok, I shall rework the patches w/ ull. I wasn't sure that ull would cause problems on 64bit archs or not for u64....thus the ugly workarounds....
BTW, anyone know how to inline patches via gmail?
Signature · 11 lines
-= Dave =- Software Engineer - Advanced Development Engineering Team Storage Component Division - Intel Corp. mailto://dave.jiang @ intel http://sourceforge.net/projects/xscaleiop/ ---- The views expressed in this email are mine alone and do not necessarily reflect the views of my employer (Intel Corp.).
Quoted 8 lines · Andrew Morton
+#if
BITS_PER_LONG== 64return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE);(quoted diff) +3
-0+#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endifWe really should find a way of avoiding this. Even if it is
Quoted 8 lines
#if
BITS_PER_LONG== 64
#defineresource_to_ptr(r)((void *)(r))
#else
#defineresource_to_ptr(r)((void *)((u32)r))
#endifin a header file somewhere. Open-coding the decision all over the place is unsightly.
This is wrong anyway - ioremap() on these will give you __iomem pointers, but cast like that looks very bogus.
Review
Quoted 7 lines · Andrew Morton
Also, the patches introduce tons of ifdefs such as:
+#if
BITS_PER_LONG== 64return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE);(quoted diff) +3
-0+#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endif
That looks rather illegal to me. What says that ROM resources are in a different memory space to MMIO resources? PCI internally treats them the same as MMIO, and as such they certainly require ioremapping on ARM.
Just because x86 has a broken architecture with respect to having holes in its memory map does not mean that's suitable for the rest of the world.
ISTR x86 ioremap knows about this, so maybe the solution to the above is to do it the Right Way(tm) and use ioremap() ?
Signature · 4 lines
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial coreReview
Quoted 5 lines · Andrew Morton
(quoted diff) +4
-0+#if BITS_PER_LONG == 64 return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endif
I just noticed that the PCI rom code also appears to be mixing other types, e.g. it delcares start as loff_t and stuffs a pci_resource_start into it, then uses it as the first argument to ioremap.
Jesse
Quoted 11 lines · Jesse Barnes
Review
Quoted 5 lines · Andrew Morton
(quoted diff) +4
-0+#if BITS_PER_LONG == 64 return (void __iomem *)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#else + return (void __iomem *)(u32)pci_resource_start(pdev, PCI_ROM_RESOURCE); +#endifI just noticed that the PCI rom code also appears to be mixing other types, e.g. it delcares start as
loff_tand stuffs apci_resource_startinto it, then uses it as the first argument to ioremap.
Quoted 2 lines
Jesse
Looks like the PCI rom code is in a "weird" state and I will just leave it alone until it is done the "right" way (unless it's not compiling). I don't believe I'll have to touch the code if it is corrected....
Signature · 11 lines
-= Dave =- Software Engineer - Advanced Development Engineering Team Storage Component Division - Intel Corp. mailto://dave.jiang @ intel http://sourceforge.net/projects/xscaleiop/ ---- The views expressed in this email are mine alone and do not necessarily reflect the views of my employer (Intel Corp.).
On Thu, Jan 13, 2005 at 04:26:57PM -0700, Dave wrote:
Ick, care to put the patches inline so we can comment on them?
thanks,
greg k-h