diff mbox

[2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names

Message ID 1895563.95T8SxMegu@vostro.rjw.lan
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Rafael J. Wysocki July 27, 2013, 1:11 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify acpi_device_set_power() so that diagnostic messages printed by
it to the kernel log always contain the name of the device concerned
to make it possible to identify the device that triggered the message
if need be.

Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that
function.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Aaron Lu July 29, 2013, 2:29 a.m. UTC | #1 
On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Modify acpi_device_set_power() so that diagnostic messages printed by
> it to the kernel log always contain the name of the device concerned
> to make it possible to identify the device that triggered the message
> if need be.
> 
> Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that
> function.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/device_pm.c |   22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
>  	/* Make sure this is a valid target state */
>  
>  	if (state == device->power.state) {
> -		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n",
> +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
> +				  device->pnp.bus_id,
>  				  acpi_power_state_string(state)));
>  		return 0;
>  	}
>  
>  	if (!device->power.states[state].flags.valid) {
> -		printk(KERN_WARNING PREFIX "Device does not support %s\n",
> -		       acpi_power_state_string(state));
> +		dev_warn(&device->dev, "Power state %s not supported\n",
> +			 acpi_power_state_string(state));
>  		return -ENODEV;
>  	}
>  	if (device->parent && (state < device->parent->power.state)) {
> -		printk(KERN_WARNING PREFIX
> -			      "Cannot set device to a higher-powered"
> -			      " state than parent\n");
> +		dev_warn(&device->dev, "Cannot transition to a higher-powered "
> +			 "state than parent\n");

I think the state information would also be useful here:

		dev_warn(&device->dev, "Cannot transition to a higher-powereed "
		         "state %d than paeren's state %d\n", state,
			 device->parent->power.state);

Thanks,
Aaron

>  		return -ENODEV;
>  	}
>  
> @@ -192,8 +192,8 @@ int acpi_device_set_power(struct acpi_de
>  
>  	if (state < device->power.state && state != ACPI_STATE_D0
>  	    && device->power.state >= ACPI_STATE_D3_HOT) {
> -		printk(KERN_WARNING PREFIX
> -			"Cannot transition to non-D0 state from D3\n");
> +		dev_warn(&device->dev,
> +			 "Cannot transition to non-D0 state from D3\n");
>  		return -ENODEV;
>  	}
>  
> @@ -220,10 +220,8 @@ int acpi_device_set_power(struct acpi_de
>  
>   end:
>  	if (result) {
> -		printk(KERN_WARNING PREFIX
> -			      "Device [%s] failed to transition to %s\n",
> -			      device->pnp.bus_id,
> -			      acpi_power_state_string(state));
> +		dev_warn(&device->dev, "Failed to change power state to %s\n",
> +			 acpi_power_state_string(state));
>  	} else {
>  		device->power.state = state;
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tianyu Lan July 29, 2013, 3:06 a.m. UTC | #2 
2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Modify acpi_device_set_power() so that diagnostic messages printed by
> it to the kernel log always contain the name of the device concerned
> to make it possible to identify the device that triggered the message
> if need be.
>
> Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that
> function.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/device_pm.c |   22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
>         /* Make sure this is a valid target state */
>
>         if (state == device->power.state) {
> -               ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n",
> +               ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
Missing "is" or it should be omitted?

> +                                 device->pnp.bus_id,
>                                   acpi_power_state_string(state)));
>                 return 0;
>         }
>
>         if (!device->power.states[state].flags.valid) {
> -               printk(KERN_WARNING PREFIX "Device does not support %s\n",
> -                      acpi_power_state_string(state));
> +               dev_warn(&device->dev, "Power state %s not supported\n",
> +                        acpi_power_state_string(state));
>                 return -ENODEV;
>         }
>         if (device->parent && (state < device->parent->power.state)) {
> -               printk(KERN_WARNING PREFIX
> -                             "Cannot set device to a higher-powered"
> -                             " state than parent\n");
> +               dev_warn(&device->dev, "Cannot transition to a higher-powered "
> +                        "state than parent\n");
>                 return -ENODEV;
>         }
>
> @@ -192,8 +192,8 @@ int acpi_device_set_power(struct acpi_de
>
>         if (state < device->power.state && state != ACPI_STATE_D0
>             && device->power.state >= ACPI_STATE_D3_HOT) {
> -               printk(KERN_WARNING PREFIX
> -                       "Cannot transition to non-D0 state from D3\n");
> +               dev_warn(&device->dev,
> +                        "Cannot transition to non-D0 state from D3\n");
>                 return -ENODEV;
>         }
>
> @@ -220,10 +220,8 @@ int acpi_device_set_power(struct acpi_de
>
>   end:
>         if (result) {
> -               printk(KERN_WARNING PREFIX
> -                             "Device [%s] failed to transition to %s\n",
> -                             device->pnp.bus_id,
> -                             acpi_power_state_string(state));
> +               dev_warn(&device->dev, "Failed to change power state to %s\n",
> +                        acpi_power_state_string(state));
>         } else {
>                 device->power.state = state;
>                 ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches July 29, 2013, 3:11 a.m. UTC | #3 
On Mon, 2013-07-29 at 11:06 +0800, Lan Tianyu wrote:
> 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[]
> >  drivers/acpi/device_pm.c |   22 ++++++++++------------
[]
> > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
[]
> >         if (device->parent && (state < device->parent->power.state)) {
> > -               printk(KERN_WARNING PREFIX
> > -                             "Cannot set device to a higher-powered"
> > -                             " state than parent\n");
> > +               dev_warn(&device->dev, "Cannot transition to a higher-powered "
> > +                        "state than parent\n");

coalesce format please.


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 29, 2013, 12:11 p.m. UTC | #4 
On Monday, July 29, 2013 11:06:36 AM Lan Tianyu wrote:
> 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Modify acpi_device_set_power() so that diagnostic messages printed by
> > it to the kernel log always contain the name of the device concerned
> > to make it possible to identify the device that triggered the message
> > if need be.
> >
> > Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that
> > function.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/device_pm.c |   22 ++++++++++------------
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/device_pm.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/device_pm.c
> > +++ linux-pm/drivers/acpi/device_pm.c
> > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
> >         /* Make sure this is a valid target state */
> >
> >         if (state == device->power.state) {
> > -               ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n",
> > +               ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
> Missing "is" or it should be omitted?

The "is" is not necessary here (in my opinion).

Kind of in analogy with "This has been done already" and "Already done".

Thanks,
Rafael
Sergei Shtylyov July 29, 2013, 12:16 p.m. UTC | #5 
Hello.

On 29-07-2013 16:17, Rafael J. Wysocki wrote:

[...]

>>>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
>> []
>>>>          if (device->parent && (state < device->parent->power.state)) {
>>>> -               printk(KERN_WARNING PREFIX
>>>> -                             "Cannot set device to a higher-powered"
>>>> -                             " state than parent\n");
>>>> +               dev_warn(&device->dev, "Cannot transition to a higher-powered "
>>>> +                        "state than parent\n");

>> coalesce format please.

> I can, but then it'll cross the 80 columns boundary.

   It's not a problem with checkpatch.pl anymore. Contrariwise, it whines 
about the broken up string literals, AFAIR.

> Thanks,
> Rafael

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 29, 2013, 12:17 p.m. UTC | #6 
On Sunday, July 28, 2013 08:11:53 PM Joe Perches wrote:
> On Mon, 2013-07-29 at 11:06 +0800, Lan Tianyu wrote:
> > 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> []
> > >  drivers/acpi/device_pm.c |   22 ++++++++++------------
> []
> > > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
> []
> > >         if (device->parent && (state < device->parent->power.state)) {
> > > -               printk(KERN_WARNING PREFIX
> > > -                             "Cannot set device to a higher-powered"
> > > -                             " state than parent\n");
> > > +               dev_warn(&device->dev, "Cannot transition to a higher-powered "
> > > +                        "state than parent\n");
> 
> coalesce format please.

I can, but then it'll cross the 80 columns boundary.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki July 29, 2013, 12:20 p.m. UTC | #7 
On Monday, July 29, 2013 10:29:36 AM Aaron Lu wrote:
> On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Modify acpi_device_set_power() so that diagnostic messages printed by
> > it to the kernel log always contain the name of the device concerned
> > to make it possible to identify the device that triggered the message
> > if need be.
> > 
> > Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that
> > function.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/device_pm.c |   22 ++++++++++------------
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > Index: linux-pm/drivers/acpi/device_pm.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/device_pm.c
> > +++ linux-pm/drivers/acpi/device_pm.c
> > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
> >  	/* Make sure this is a valid target state */
> >  
> >  	if (state == device->power.state) {
> > -		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n",
> > +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
> > +				  device->pnp.bus_id,
> >  				  acpi_power_state_string(state)));
> >  		return 0;
> >  	}
> >  
> >  	if (!device->power.states[state].flags.valid) {
> > -		printk(KERN_WARNING PREFIX "Device does not support %s\n",
> > -		       acpi_power_state_string(state));
> > +		dev_warn(&device->dev, "Power state %s not supported\n",
> > +			 acpi_power_state_string(state));
> >  		return -ENODEV;
> >  	}
> >  	if (device->parent && (state < device->parent->power.state)) {
> > -		printk(KERN_WARNING PREFIX
> > -			      "Cannot set device to a higher-powered"
> > -			      " state than parent\n");
> > +		dev_warn(&device->dev, "Cannot transition to a higher-powered "
> > +			 "state than parent\n");
> 
> I think the state information would also be useful here:
> 
> 		dev_warn(&device->dev, "Cannot transition to a higher-powereed "
> 		         "state %d than paeren's state %d\n", state,
> 			 device->parent->power.state);

This is not the scope of this patch, please send another one on top of it.

Thanks,
Rafael
diff mbox

Patch

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -166,20 +166,20 @@  int acpi_device_set_power(struct acpi_de
 	/* Make sure this is a valid target state */
 
 	if (state == device->power.state) {
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n",
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n",
+				  device->pnp.bus_id,
 				  acpi_power_state_string(state)));
 		return 0;
 	}
 
 	if (!device->power.states[state].flags.valid) {
-		printk(KERN_WARNING PREFIX "Device does not support %s\n",
-		       acpi_power_state_string(state));
+		dev_warn(&device->dev, "Power state %s not supported\n",
+			 acpi_power_state_string(state));
 		return -ENODEV;
 	}
 	if (device->parent && (state < device->parent->power.state)) {
-		printk(KERN_WARNING PREFIX
-			      "Cannot set device to a higher-powered"
-			      " state than parent\n");
+		dev_warn(&device->dev, "Cannot transition to a higher-powered "
+			 "state than parent\n");
 		return -ENODEV;
 	}
 
@@ -192,8 +192,8 @@  int acpi_device_set_power(struct acpi_de
 
 	if (state < device->power.state && state != ACPI_STATE_D0
 	    && device->power.state >= ACPI_STATE_D3_HOT) {
-		printk(KERN_WARNING PREFIX
-			"Cannot transition to non-D0 state from D3\n");
+		dev_warn(&device->dev,
+			 "Cannot transition to non-D0 state from D3\n");
 		return -ENODEV;
 	}
 
@@ -220,10 +220,8 @@  int acpi_device_set_power(struct acpi_de
 
  end:
 	if (result) {
-		printk(KERN_WARNING PREFIX
-			      "Device [%s] failed to transition to %s\n",
-			      device->pnp.bus_id,
-			      acpi_power_state_string(state));
+		dev_warn(&device->dev, "Failed to change power state to %s\n",
+			 acpi_power_state_string(state));
 	} else {
 		device->power.state = state;
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,