[PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Seth Forshee]

The debug statements reporting unkown WMI and hotkey events are
polluting the log in some systems and generating unnecessary
concern from users. Change these to pr_debug to hide them normally
while keeping them available for when they're needed.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/dell-wmi.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index fa9a217..1881e28 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -165,8 +165,8 @@ static void dell_wmi_notify(u32 value, void *context)
 		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
 
 		if (dell_new_hk_type && (buffer_entry[1] != 0x10)) {
-			pr_info("Received unknown WMI event (0x%x)\n",
-				buffer_entry[1]);
+			pr_debug("Received unknown WMI event (0x%x)\n",
+				 buffer_entry[1]);
 			kfree(obj);
 			return;
 		}
@@ -179,7 +179,7 @@ static void dell_wmi_notify(u32 value, void *context)
 		key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
 							reported_key);
 		if (!key) {
-			pr_info("Unknown key %x pressed\n", reported_key);
+			pr_debug("Unknown key %x pressed\n", reported_key);
 		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
 			    key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video) {
 			/* Don't report brightness notifications that will also
-- 
1.7.5.4

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Matthew Garrett]

They're intended to be there, in the same way that the keyboard code 
generates unknown scancode errors. We want to know when users have keys 
that don't work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Corentin Chary]

On Mon, Nov 21, 2011 at 9:52 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> They're intended to be there, in the same way that the keyboard code
> generates unknown scancode errors. We want to know when users have keys
> that don't work.

Agree, and if specific known key should be ignored, they should be
present in the keymap.

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Seth Forshee]

On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> They're intended to be there, in the same way that the keyboard code 
> generates unknown scancode errors. We want to know when users have keys 
> that don't work.

All right, but what about the unknown WMI events? Those are the bigger
problem anyway, as at least one machine (the Latitude XT2) can generate
a lot of these messages for unknown reasons. It doesn't appear that
anyone has been making use of these messages since we still only support
one event. Would you object to demoting that one?

Seth

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Matthew Garrett]

On Mon, Nov 21, 2011 at 03:14:38PM -0600, Seth Forshee wrote:
> On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> > They're intended to be there, in the same way that the keyboard code 
> > generates unknown scancode errors. We want to know when users have keys 
> > that don't work.
> 
> All right, but what about the unknown WMI events? Those are the bigger
> problem anyway, as at least one machine (the Latitude XT2) can generate
> a lot of these messages for unknown reasons. It doesn't appear that
> anyone has been making use of these messages since we still only support
> one event. Would you object to demoting that one?

Those can probably be ignored, but again it would be nice to find out 
what they actually mean so we know whether we can do anything with them. 
Any chance of you being able to work with Dell on that?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Seth Forshee]

On Mon, Nov 21, 2011 at 09:18:09PM +0000, Matthew Garrett wrote:
> On Mon, Nov 21, 2011 at 03:14:38PM -0600, Seth Forshee wrote:
> > On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> > > They're intended to be there, in the same way that the keyboard code 
> > > generates unknown scancode errors. We want to know when users have keys 
> > > that don't work.
> > 
> > All right, but what about the unknown WMI events? Those are the bigger
> > problem anyway, as at least one machine (the Latitude XT2) can generate
> > a lot of these messages for unknown reasons. It doesn't appear that
> > anyone has been making use of these messages since we still only support
> > one event. Would you object to demoting that one?
> 
> Those can probably be ignored, but again it would be nice to find out 
> what they actually mean so we know whether we can do anything with them. 
> Any chance of you being able to work with Dell on that?

I'll see what I can do.

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Matthew Garrett]

Great, thanks. If we don't work out what they are then we can certainly 
drop the error - I agree that telling the user about something that just 
confuses us isn't helpful.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Seth Forshee]

On Mon, Nov 21, 2011 at 10:32:09PM +0000, Matthew Garrett wrote:
> > > On Mon, Nov 21, 2011 at 03:14:38PM -0600, Seth Forshee wrote:
> > > > On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> > > > > They're intended to be there, in the same way that the keyboard code
> > > > > generates unknown scancode errors. We want to know when users have keys
> > > > > that don't work.
> > > >
> > > > All right, but what about the unknown WMI events? Those are the bigger
> > > > problem anyway, as at least one machine (the Latitude XT2) can generate
> > > > a lot of these messages for unknown reasons. It doesn't appear that
> > > > anyone has been making use of these messages since we still only support
> > > > one event. Would you object to demoting that one?
> > >
> > > Those can probably be ignored, but again it would be nice to find out
> > > what they actually mean so we know whether we can do anything with them.
> > > Any chance of you being able to work with Dell on that?
> > 
> > I'll see what I can do.
> 
> Great, thanks. If we don't work out what they are then we can certainly 
> drop the error - I agree that telling the user about something that just 
> confuses us isn't helpful.

I've tried a few different avenues trying to get this informatin out of
Dell, but so far I've gotten very little. 0x11 is a status change event
notification, apparently sent when there are changes to things like
battery charge status, ALS level, etc. I haven't received any
information about how to interpret the payload, and what little Dell kit
I have doesn't seem to send this event.

I've received no informtion at all on the 0xf event. The only thing I
really know about is from a user report stating that it's seen on the
XT2 when opening and closing the lid in notebook orientation.

I can take further action if Dell becomes more forthcoming, but I'm not
holding out much hope at this point. So I can send a new patch demoting
only the WMI event message to debug, or I can silence it only for these
specific events. Do you have a preference?

Thanks,
Seth

RE: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Rezwanul_Kabir]

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-
> x86-owner@vger.kernel.org] On Behalf Of Seth Forshee
> Sent: Wednesday, December 07, 2011 10:43 AM
> To: Matthew Garrett
> Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] dell-wmi: Change debug statements from pr_info to
> pr_debug
> 
> On Mon, Nov 21, 2011 at 10:32:09PM +0000, Matthew Garrett wrote:
> > > > On Mon, Nov 21, 2011 at 03:14:38PM -0600, Seth Forshee wrote:
> > > > > On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> > > > > > They're intended to be there, in the same way that the
> > > > > > keyboard code generates unknown scancode errors. We want to
> > > > > > know when users have keys that don't work.
> > > > >
> > > > > All right, but what about the unknown WMI events? Those are the
> > > > > bigger problem anyway, as at least one machine (the Latitude
> > > > > XT2) can generate a lot of these messages for unknown reasons.
> > > > > It doesn't appear that anyone has been making use of these
> > > > > messages since we still only support one event. Would you object to
> demoting that one?
> > > >
> > > > Those can probably be ignored, but again it would be nice to find
> > > > out what they actually mean so we know whether we can do anything
> with them.
> > > > Any chance of you being able to work with Dell on that?
> > >
> > > I'll see what I can do.
> >
> > Great, thanks. If we don't work out what they are then we can
> > certainly drop the error - I agree that telling the user about
> > something that just confuses us isn't helpful.
> 
> I've tried a few different avenues trying to get this informatin out of Dell, but
> so far I've gotten very little. 0x11 is a status change event notification,
> apparently sent when there are changes to things like battery charge status,
> ALS level, etc. I haven't received any information about how to interpret the
> payload, and what little Dell kit I have doesn't seem to send this event.
> 
> I've received no informtion at all on the 0xf event. The only thing I really
> know about is from a user report stating that it's seen on the
> XT2 when opening and closing the lid in notebook orientation.
> 
> I can take further action if Dell becomes more forthcoming, but I'm not
> holding out much hope at this point. So I can send a new patch demoting only
> the WMI event message to debug, or I can silence it only for these specific
> events. Do you have a preference?
> 

My opinion would be to deprioritize the log level for now, if necessary. I am working on gathering more information on unhandled events and will submit a patch in the next few weeks to handle them.

Thanks
  --rez

Rezwanul Kabir
Dell Linux Development
512-725-0766





> Thanks,
> Seth
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86"
> in the body of a message to majordomo@vger.kernel.org More majordomo
> info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dell-wmi: Change debug statements from pr_info to pr_debug

[Seth Forshee]

On Wed, Dec 07, 2011 at 02:22:31PM -0600, Rezwanul_Kabir@Dell.com wrote:
> 
> > -----Original Message-----
> > From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-
> > x86-owner@vger.kernel.org] On Behalf Of Seth Forshee
> > Sent: Wednesday, December 07, 2011 10:43 AM
> > To: Matthew Garrett
> > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] dell-wmi: Change debug statements from pr_info to
> > pr_debug
> > 
> > On Mon, Nov 21, 2011 at 10:32:09PM +0000, Matthew Garrett wrote:
> > > > > On Mon, Nov 21, 2011 at 03:14:38PM -0600, Seth Forshee wrote:
> > > > > > On Mon, Nov 21, 2011 at 08:52:30PM +0000, Matthew Garrett wrote:
> > > > > > > They're intended to be there, in the same way that the
> > > > > > > keyboard code generates unknown scancode errors. We want to
> > > > > > > know when users have keys that don't work.
> > > > > >
> > > > > > All right, but what about the unknown WMI events? Those are the
> > > > > > bigger problem anyway, as at least one machine (the Latitude
> > > > > > XT2) can generate a lot of these messages for unknown reasons.
> > > > > > It doesn't appear that anyone has been making use of these
> > > > > > messages since we still only support one event. Would you object to
> > demoting that one?
> > > > >
> > > > > Those can probably be ignored, but again it would be nice to find
> > > > > out what they actually mean so we know whether we can do anything
> > with them.
> > > > > Any chance of you being able to work with Dell on that?
> > > >
> > > > I'll see what I can do.
> > >
> > > Great, thanks. If we don't work out what they are then we can
> > > certainly drop the error - I agree that telling the user about
> > > something that just confuses us isn't helpful.
> > 
> > I've tried a few different avenues trying to get this informatin out of Dell, but
> > so far I've gotten very little. 0x11 is a status change event notification,
> > apparently sent when there are changes to things like battery charge status,
> > ALS level, etc. I haven't received any information about how to interpret the
> > payload, and what little Dell kit I have doesn't seem to send this event.
> > 
> > I've received no informtion at all on the 0xf event. The only thing I really
> > know about is from a user report stating that it's seen on the
> > XT2 when opening and closing the lid in notebook orientation.
> > 
> > I can take further action if Dell becomes more forthcoming, but I'm not
> > holding out much hope at this point. So I can send a new patch demoting only
> > the WMI event message to debug, or I can silence it only for these specific
> > events. Do you have a preference?
> > 
> 
> My opinion would be to deprioritize the log level for now, if necessary. I am working on gathering more information on unhandled events and will submit a patch in the next few weeks to handle them.

Thanks for the response, Rez. It will be nice to have those events
supported.

I was only advocating dropping the priority of the message because it
looked like nothing would ever be done about them. If you'll be seending
a patch soon-ish then we can just demote it in Ubuntu to stop the bug
reports while we wait for the patch.

Seth