Re: [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.