Re: [PATCH]Enabling Auto poweron after power is restored.

[Will Schmidt <will_schmidt@vnet.ibm.com>]

On Fri, 2006-01-12 at 12:28 -0600, Manish Ahuja wrote:
> During power outages, the ups notifies the system for a shutdown. In the
> current setup, it isn't possible to poweron when power is restored. This
> patch fixes the issue by calling the right ibm,power-off-ups token
> during such events. It also adds a proc interface so that rc.powerfail
> can parse the epow events and modify the power-off behavior accordingly
> to enable the right token to be called.
> 
> Signed-off-by: Manish Ahuja <ahuja@austin.ibm.com>
> 


> plain text document attachment (power-off-ups.patch)
> Index: 2.6-git2/arch/powerpc/kernel/rtas.c
> ===================================================================
> --- 2.6-git2.orig/arch/powerpc/kernel/rtas.c	2006-11-29 12:00:26.000000000 -0800
> +++ 2.6-git2/arch/powerpc/kernel/rtas.c	2006-11-30 10:56:12.000000000 -0800
> @@ -603,11 +603,30 @@
> 
>  void rtas_power_off(void)
>  {
> +	int rc = 0;
> +	int rtas_poweron_auto_token;
> +
>  	if (rtas_flash_term_hook)
>  		rtas_flash_term_hook(SYS_POWER_OFF);
> -	/* allow power on only with power button press */
> -	printk("RTAS power-off returned %d\n",
> -	       rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1));
> +
> +	if (rtas_poweron_auto == 0) {
> +		/* allow power on only with power button press */

I expect someone will agree or strongly disagree with me here on
cosmetics.. :-)  I'd be tempted to pull the one-liner comments out of
the code and replace them with a short paragraph above the function
name.

And.. 
Do we have a preference for  "(rtas_poweron_auto == 0)" versus "(!rtas_poweron_auto)" 
notation?    rtas.c has some of each, so I'm not sure.


> +		printk(KERN_INFO "RTAS power-off returned %d\n",
> +			rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1));
> +	} else {
> +		rtas_poweron_auto_token = rtas_token("ibm,power-off-ups");
> +
> +		if (rtas_poweron_auto_token == RTAS_UNKNOWN_SERVICE) {
> +			/* ibm,power-off-ups failed or token does not exist */
> +			rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
> +	 		printk(KERN_EMERG "Power-off called instead %d\n", rc );
> +		} else {
> +			/* Enable the system to reboot if power comes back on */
> +			rc = rtas_call(rtas_token("ibm,power-off-ups"), 0, 1, NULL);
> +			printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
> +		}

You could eliminate the temp variable rtas_poweron_auto_token, and
rewrite as something like:

               if (rtas_token("ibm,power-off-ups") == RTAS_UNKNOWN_SERVICE) {
                       rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
                       printk(KERN_EMERG "Power-off called instead %d\n", rc );
               } else {
                       rc = rtas_call(rtas_token("ibm,power-off-ups"),0, 1, NULL);
                       printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
               }


> +
> +	}
>  	for (;;);
>  }

<snippage> 
> +static ssize_t ppc_rtas_poweron_auto_write(struct file *file,
> +		const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	unsigned long ups_restart;
> +	int error = parse_number(buf, count, &ups_restart);
> +	if (error)
> +		return error;
> +
> +	if (ups_restart != 0)
> +		rtas_poweron_auto = 1;

the !=0 is redundant here..  just "if (ups_restart) " should be
sufficient. 

> +
> +	return count;
> +}

>  /**
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev