[OpenBIOS] [PATCH v3 1/2] ofmem: Use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Hi Andreas,

Just some general comments:

Andreas Färber wrote:

> @@ -257,7 +257,8 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
>  	}
>  
>  	/* inverse of phys_range list could take 2 more cells for the tail */
> -	prop_used = (ncells+1) * sizeof(ucell) * 2;
> +	prop_used = (ncells + 1) * sizeof(ucell) *
> +		(((ph == s_phandle_memory) ? ofmem_arch_get_physaddr_cellsize() : 1) + 1);

I think a comment here explaining that physical address size can be >= 
virtual address size would help readability here.

> @@ -291,7 +292,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
>  
>  		size = r->start - start;
>  		if (size) {
> -			prop[ncells++] = start;
> +			if (ph == s_phandle_memory)

Add some braces to the if() to emulate QEMU style (even if it's a single 
line), plus add a comment e.g. /* For physical addresses */
				
> +				ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
> +			else

Same here, except comment should read e.g. /* For virtual addresses */

> +				prop[ncells++] = start;
>  			prop[ncells++] = size;
>  		}
>  		start = r->start + r->size;
> @@ -299,7 +303,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
>  
>  	/* tail */
>  	if (start < top_address) {
> -		prop[ncells++] = start;
> +		if (ph == s_phandle_memory)

Same here.

> +			ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
> +		else

And here.

> +			prop[ncells++] = start;
>  		prop[ncells++] = top_address - start;
>  	}

Other than that, it looks good to me. As long as there are no 
regressions, I'm happy for commit.


ATB,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs