diff mbox

Remove ambiguous logging for "Unsupported brightness interface"

Message ID 1453937211.10099.31.camel@perches.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Joe Perches Jan. 27, 2016, 11:26 p.m. UTC
On Wed, 2016-01-27 at 22:14 +0000, Eric Curtin wrote:
> Message gets logged on machines that are well supported.
> 
> Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index a268a7a..4eb41aa 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6661,7 +6661,6 @@ static void __init
> tpacpi_detect_brightness_capabilities(void)
>                 pr_info("detected a 8-level brightness capable
> ThinkPad\n");
>                 break;
>         default:
> -               pr_info("Unsupported brightness interface\n");
>                 tp_features.bright_unkfw = 1;
>                 bright_maxlvl = b - 1;
>         }

Perhaps this should be something like this instead:
---
 drivers/platform/x86/thinkpad_acpi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--
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

Comments

Eric Curtin Jan. 28, 2016, 12:36 a.m. UTC | #1
On 27 January 2016 at 23:26, Joe Perches <joe@perches.com> wrote:
> On Wed, 2016-01-27 at 22:14 +0000, Eric Curtin wrote:
>> Message gets logged on machines that are well supported.
>>
>> Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c
>> b/drivers/platform/x86/thinkpad_acpi.c
>> index a268a7a..4eb41aa 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -6661,7 +6661,6 @@ static void __init
>> tpacpi_detect_brightness_capabilities(void)
>>                 pr_info("detected a 8-level brightness capable
>> ThinkPad\n");
>>                 break;
>>         default:
>> -               pr_info("Unsupported brightness interface\n");
>>                 tp_features.bright_unkfw = 1;
>>                 bright_maxlvl = b - 1;
>>         }
>
> Perhaps this should be something like this instead:
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index a268a7a..bd12c71 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6653,18 +6653,16 @@ static void __init tpacpi_detect_brightness_capabilities(void)
>         switch (b) {
>         case 16:
>                 bright_maxlvl = 15;
> -               pr_info("detected a 16-level brightness capable ThinkPad\n");
>                 break;
>         case 8:
>         case 0:
>                 bright_maxlvl = 7;
> -               pr_info("detected a 8-level brightness capable ThinkPad\n");
>                 break;
>         default:
> -               pr_info("Unsupported brightness interface\n");
>                 tp_features.bright_unkfw = 1;
>                 bright_maxlvl = b - 1;
>         }
> +       pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
>  }
>
>  static int __init brightness_init(struct ibm_init_struct *iibm)

Maybe, but the other logging issues kinda mean something. There are
many, many reports of people thinking their brightness interface is
broken because of this logging message, when in reality the i915 driver
supports their devices just fine as previously stated.
--
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
Joe Perches Jan. 28, 2016, 12:43 a.m. UTC | #2
On Thu, 2016-01-28 at 00:36 +0000, Eric Curtin wrote:
> On 27 January 2016 at 23:26, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2016-01-27 at 22:14 +0000, Eric Curtin wrote:
> > > Message gets logged on machines that are well supported.
[]
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
[]
> > +       pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
> >  }
> > 
> >  static int __init brightness_init(struct ibm_init_struct *iibm)
> 
> Maybe, but the other logging issues kinda mean something. There are
> many, many reports of people thinking their brightness interface is
> broken because of this logging message, when in reality the i915 driver
> supports their devices just fine as previously stated.

That's why I suggest changing it to show the number
of brightness levels detected on any device.

--
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
Eric Curtin Jan. 28, 2016, 12:52 a.m. UTC | #3
On 28 January 2016 at 00:43, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-01-28 at 00:36 +0000, Eric Curtin wrote:
>> On 27 January 2016 at 23:26, Joe Perches <joe@perches.com> wrote:
>> > On Wed, 2016-01-27 at 22:14 +0000, Eric Curtin wrote:
>> > > Message gets logged on machines that are well supported.
> []
>> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> []
>> > +       pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
>> >  }
>> >
>> >  static int __init brightness_init(struct ibm_init_struct *iibm)
>>
>> Maybe, but the other logging issues kinda mean something. There are
>> many, many reports of people thinking their brightness interface is
>> broken because of this logging message, when in reality the i915 driver
>> supports their devices just fine as previously stated.
>
> That's why I suggest changing it to show the number
> of brightness levels detected on any device.
>

Sorry, I missed that line. Looks like a better fix! +1
--
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
Henrique de Moraes Holschuh Jan. 30, 2016, 12:20 p.m. UTC | #4
On Wed, 27 Jan 2016, Joe Perches wrote:
> On Wed, 2016-01-27 at 22:14 +0000, Eric Curtin wrote:
> > Message gets logged on machines that are well supported.
> > 
> > Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
> > ---
> >  drivers/platform/x86/thinkpad_acpi.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c
> > b/drivers/platform/x86/thinkpad_acpi.c
> > index a268a7a..4eb41aa 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -6661,7 +6661,6 @@ static void __init
> > tpacpi_detect_brightness_capabilities(void)
> >                 pr_info("detected a 8-level brightness capable
> > ThinkPad\n");
> >                 break;
> >         default:
> > -               pr_info("Unsupported brightness interface\n");
> >                 tp_features.bright_unkfw = 1;
> >                 bright_maxlvl = b - 1;
> >         }
> 
> Perhaps this should be something like this instead:
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index a268a7a..bd12c71 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6653,18 +6653,16 @@ static void __init tpacpi_detect_brightness_capabilities(void)
>  	switch (b) {
>  	case 16:
>  		bright_maxlvl = 15;
> -		pr_info("detected a 16-level brightness capable ThinkPad\n");
>  		break;
>  	case 8:
>  	case 0:
>  		bright_maxlvl = 7;
> -		pr_info("detected a 8-level brightness capable ThinkPad\n");
>  		break;
>  	default:
> -		pr_info("Unsupported brightness interface\n");
>  		tp_features.bright_unkfw = 1;
>  		bright_maxlvl = b - 1;
>  	}
> +	pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
>  }

This can be made pr_debug, since we're touching it...
diff mbox

Patch

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index a268a7a..bd12c71 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6653,18 +6653,16 @@  static void __init tpacpi_detect_brightness_capabilities(void)
 	switch (b) {
 	case 16:
 		bright_maxlvl = 15;
-		pr_info("detected a 16-level brightness capable ThinkPad\n");
 		break;
 	case 8:
 	case 0:
 		bright_maxlvl = 7;
-		pr_info("detected a 8-level brightness capable ThinkPad\n");
 		break;
 	default:
-		pr_info("Unsupported brightness interface\n");
 		tp_features.bright_unkfw = 1;
 		bright_maxlvl = b - 1;
 	}
+	pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
 }
 
 static int __init brightness_init(struct ibm_init_struct *iibm)