[OpenBIOS] [RFC] Switch OFMEM to use phys_addr_t for physical addresses
Andreas Färber
andreas.faerber at web.de
Sun Nov 7 13:02:18 CET 2010
Hi,
Am 07.11.2010 um 12:16 schrieb Mark Cave-Ayland:
> Here is my first attempt at switching over OFMEM to use phys_addr_t
> for physical addresses. It was mainly a case of going through all of
> the OFMEM APIs by hand and then updating the definitions used to
> hold physical addresses, along with swapping over range_t to use
> phys_addr_t on the assumption that sizeof(phys_addr_t) >=
> sizeof(ucell).
Looking good so far, except for one issue.
> Index: libopenbios/ofmem_common.c
> ===================================================================
> --- libopenbios/ofmem_common.c (revision 946)
> +++ libopenbios/ofmem_common.c (working copy)
> @@ -463,7 +463,7 @@
> }
>
> /* if align != 0, phys is ignored. Returns -1 on error */
> -ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
> +ucell ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
> {
> OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
> " align=" FMT_ucellx "\n",
This will break the trace code on sparc32 and ppc64. We'll need some
target-specific define to replace FMT_ucellx in the trace code for the
"phys" parameter since it may be larger.
Btw if we start deviating from ucell, we might as well use size_t size
in a second step, but I don't see a real use case yet.
> Index: arch/sparc64/ofmem_sparc64.c
> ===================================================================
> --- arch/sparc64/ofmem_sparc64.c (revision 946)
> +++ arch/sparc64/ofmem_sparc64.c (working copy)
> @@ -514,7 +516,8 @@
> static void
> mem_claim( void )
> {
> - ucell phys=-1UL, size, align;
> + ucell size, align;
> + phys_addr_t phys=-1UL;
I'd appreciate if you could enhance the code you refactored with
spaces around the assignment, i.e. phys_addr_t phys = -1UL;, for
readability.
> @@ -548,7 +552,8 @@
> static void
> mem_retain ( void )
> {
> - ucell phys=-1UL, size, align;
> + ucell size, align;
> + phys_addr_t phys=-1UL;
Dito.
> @@ -393,7 +394,7 @@
> }
> }
>
> -void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
> +void ofmem_map_pages(phys_addr_t phys, ucell virt, ucell size,
> ucell mode)
> {
> return map_pages(phys, virt, size, mode);
> }
Note that I find it slightly odd that there is a (trivial)
ofmem_map_ranges() function in arch/sparc64/lib.c, but that's
unrelated to phys_addr_t.
What's the difference to ofmem_map() in ofmem_common.c? We should
either share it, rename it or drop it mid-term.
Regards,
Andreas
More information about the OpenBIOS
mailing list