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.
Signed-off-by: Dave Jiang ([email protected])
--
-= 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, 13 Jan 2005, Andrew Morton wrote:
>
> 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
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
Dave <[email protected]> wrote:
>
> 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.
On Thu, 13 Jan 2005 16:23:09 -0800, Andrew Morton <[email protected]> wrote:
> Dave <[email protected]> wrote:
> >
> > 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.
>
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?
--
-= 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, 13 Jan 2005 16:43:58 -0800 (PST), Linus Torvalds
<[email protected]> wrote:
>
>
> On Thu, 13 Jan 2005, Andrew Morton wrote:
> >
> > 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
>
> 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
>
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?
--
-= 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:23:09PM -0800, Andrew Morton wrote:
> 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
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() ?
--
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 core
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
On Thu, Jan 13, 2005 at 04:23:09PM -0800, Andrew Morton wrote:
> +#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.
This is wrong anyway - ioremap() on these will give you __iomem pointers,
but cast like that looks very bogus.
On Thursday, January 13, 2005 4:23 pm, Andrew Morton wrote:
> +#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
On Thu, 13 Jan 2005 17:35:04 -0800, Jesse Barnes <[email protected]> wrote:
> On Thursday, January 13, 2005 4:23 pm, Andrew Morton wrote:
> > +#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
>
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....
--
-= 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.).