Hi,
Just a few nitpicks, no comments on the functional parts:
On Mon, Apr 07, 2008 at 06:45:37PM -0500, Manish Ahuja wrote:
> A small proposed change in the amount of reserve space we allocate during
> boot.
> Currently we reserve 256MB only.
> The proposed change does one of the 3 things.
>
> A. It checks to see if there is cmdline variable set and if found sets the
> value to it. OR
> B. It computes 5% of total ram and rounds it down to multiples of 256MB. AND
> C. Compares the rounded down value and returns larger of two values, the new
> computed value or 256MB.
...
> +/* Look for phyp_dump_reserve_size= cmdline option */
> +static int __init early_phyp_dump_reserve_size(char *p)
> +{
> + if (p)
> + phyp_dump_info->phyp_dump_reserve_bootvar = memparse(p, &p);
[...]
> @@ -24,8 +24,10 @@ struct phyp_dump {
> /* Memory that is reserved during very early boot. */
> unsigned long init_reserve_start;
> unsigned long init_reserve_size;
> - /* Check status during boot if dump supported, active & present*/
> + /* cmd line options during boot */
> + unsigned long phyp_dump_reserve_bootvar;
> unsigned long phyp_dump_at_boot;
> + /* Check status during boot if dump supported, active & present*/
> unsigned long phyp_dump_configured;
> unsigned long phyp_dump_is_active;
> /* store cpu & hpte size */
These make for some really long variable names and lines. I know from
experience, since I've picked unneccessary long driver names in the past
myself. :)
How about just naming the new variables reserve_bootvar, etc? The name
of the struct they're in makes it obvious what they're for.
> +static inline unsigned long phyp_dump_calculate_reserve_size(void)
> +{
> + unsigned long tmp;
> +
> + if (phyp_dump_info->phyp_dump_reserve_bootvar)
> + return phyp_dump_info->phyp_dump_reserve_bootvar;
> +
> + /* divide by 20 to get 5% of value */
> + tmp = lmb_end_of_DRAM();
> + do_div(tmp, 20);
> +
> + /* round it down in multiples of 256 */
> + tmp = tmp & ~0x000000001FFFFFFF;
That's 512MB, isn't it?
-Olof
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
