On 08.07.2022 16:54, Wei Chen wrote:
> @@ -82,3 +83,25 @@ unsigned int __init arch_get_dma_bitsize(void)
>                   flsl(node_start_pfn(node) + node_spanned_pages(node) / 4 - 
> 1)
>                   + PAGE_SHIFT, 32);
>  }
> +
> +/*
> + * This function provides the ability for caller to get one RAM entry
> + * from architectural memory map by index.
> + *
> + * This function will return zero if it can return a proper RAM entry.
> + * otherwise it will return -ENODEV for out of scope index, or return
> + * -EINVAL for non-RAM type memory entry.
> + */

I think the comment also wants to spell out that the range is
exclusive at the end (assuming that's suitable for Arm; else now
would perhaps be the time to change it).

> +int __init arch_get_memory_map(unsigned int idx, paddr_t *start, paddr_t 
> *end)
> +{
> +    if ( idx >= e820.nr_map )
> +        return -ENODEV;

Perhaps better -ENOENT?

> +    if ( e820.map[idx].type != E820_RAM )
> +        return -EINVAL;

I'm sorry, this should have occurred to me already when commenting on
v1: "get_memory_map" doesn't really fit this "RAM only" restriction.
Maybe arch_get_ram_range()? Or maybe others have some good naming
suggestion?

> +    *start = e820.map[idx].addr;
> +    *end = e820.map[idx].addr + e820.map[idx].size;

Nit: Would be shorter to read if you (re)used *start.

Jan

Reply via email to