| Message ID | 20171114161414.14297-1-bberg@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com> wrote: > On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT > mode is reported. Decode it correctly rather than warning about an > unexpected multi mode status to be reported. If we can't check on all available hardware, better to avoid "doing for all" fixes. I suppose DMI match can help here. It would be nice to hear from Henrique and others as well. > case 4: > - valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > - TP_ACPI_MULTI_MODE_TABLET | > - TP_ACPI_MULTI_MODE_STAND | > - TP_ACPI_MULTI_MODE_TENT; > - break; The common practice is to put /* fallthrough */ instead. > case 5: > + /* In mode 4, FLAT is not specified as a valid mode. However, > + * it can be seen at least on the X1 Yoga 2nd Generation. > + */ We don't use network subsystem style of comments. > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > TP_ACPI_MULTI_MODE_FLAT | > TP_ACPI_MULTI_MODE_TABLET | > TP_ACPI_MULTI_MODE_STAND | > TP_ACPI_MULTI_MODE_TENT; > break;
lgtm. Reviewed-by: Lyude Paul <lyude@redhat.com> On Tue, 2017-11-14 at 17:14 +0100, Benjamin Berg wrote: > On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT > mode is reported. Decode it correctly rather than warning about an > unexpected multi mode status to be reported. > > Signed-off-by: Benjamin Berg <bberg@redhat.com> > Cc: Peter FP1 Zhang <zhangfp1@lenovo.com > Cc: Lyude <lyude@redhat.com> > --- > drivers/platform/x86/thinkpad_acpi.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c > b/drivers/platform/x86/thinkpad_acpi.c > index 91fab1a13a6d..d23c84222e9f 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int > *has_tablet_mode) > TP_ACPI_MULTI_MODE_FLAT | > TP_ACPI_MULTI_MODE_TABLET | > TP_ACPI_MULTI_MODE_STAND | > TP_ACPI_MULTI_MODE_TENT; > break; > case 3: > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > TP_ACPI_MULTI_MODE_FLAT; > break; > case 4: > - valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > - TP_ACPI_MULTI_MODE_TABLET | > - TP_ACPI_MULTI_MODE_STAND | > - TP_ACPI_MULTI_MODE_TENT; > - break; > case 5: > + /* In mode 4, FLAT is not specified as a valid mode. > However, > + * it can be seen at least on the X1 Yoga 2nd Generation. > + */ > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > TP_ACPI_MULTI_MODE_FLAT | > TP_ACPI_MULTI_MODE_TABLET | > TP_ACPI_MULTI_MODE_STAND | > TP_ACPI_MULTI_MODE_TENT; > break; > default: > pr_err("Unknown multi mode status type %d with value > 0x%04X, please report this to %s\n", > type, value, TPACPI_MAIL); > return 0;
On Tue, 2017-11-14 at 18:58 +0200, Andy Shevchenko wrote: > On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com> > wrote: > > On the X1 Yoga 2nd Generation and most likely other notebooks the > > FLAT > > mode is reported. Decode it correctly rather than warning about an > > unexpected multi mode status to be reported. > > If we can't check on all available hardware, better to avoid "doing > for all" fixes. > I suppose DMI match can help here. The documentation I saw suggests that the flat mode cannot be detected by machines reporting a multi mode status of type 4. This is why the original patch excluded the FLAT mode in the list of valid modes on those devices. I think it is safe to simply assume that all laptops reporting type 4 can actually detect the flat state. Doing so will not affect actual state reporting on machines that may not be able to detect it. And I don't think that the information whether the flat state can be detected or not is interesting enough to bother with DMI matches. Benjamin > It would be nice to hear from Henrique and others as well. > > > > case 4: > > - valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > > - TP_ACPI_MULTI_MODE_TABLET | > > - TP_ACPI_MULTI_MODE_STAND | > > - TP_ACPI_MULTI_MODE_TENT; > > - break; > > The common practice is to put > /* fallthrough */ > instead. > > > case 5: > > + /* In mode 4, FLAT is not specified as a valid > > mode. However, > > + * it can be seen at least on the X1 Yoga 2nd > > Generation. > > + */ > > We don't use network subsystem style of comments. > > > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > > TP_ACPI_MULTI_MODE_FLAT | > > TP_ACPI_MULTI_MODE_TABLET | > > TP_ACPI_MULTI_MODE_STAND | > > TP_ACPI_MULTI_MODE_TENT; > > break; > >
On Tue, Nov 14, 2017 at 6:14 PM, Benjamin Berg <bberg@redhat.com> wrote: > On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT > mode is reported. Decode it correctly rather than warning about an > unexpected multi mode status to be reported. > > Signed-off-by: Benjamin Berg <bberg@redhat.com> > Cc: Peter FP1 Zhang <zhangfp1@lenovo.com > Cc: Lyude <lyude@redhat.com> Applied to my review and testing queue, thanks! > --- > drivers/platform/x86/thinkpad_acpi.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 91fab1a13a6d..d23c84222e9f 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode) > TP_ACPI_MULTI_MODE_FLAT | > TP_ACPI_MULTI_MODE_TABLET | > TP_ACPI_MULTI_MODE_STAND | > TP_ACPI_MULTI_MODE_TENT; > break; > case 3: > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > TP_ACPI_MULTI_MODE_FLAT; > break; > case 4: > - valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > - TP_ACPI_MULTI_MODE_TABLET | > - TP_ACPI_MULTI_MODE_STAND | > - TP_ACPI_MULTI_MODE_TENT; > - break; > case 5: > + /* In mode 4, FLAT is not specified as a valid mode. However, > + * it can be seen at least on the X1 Yoga 2nd Generation. > + */ > valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | > TP_ACPI_MULTI_MODE_FLAT | > TP_ACPI_MULTI_MODE_TABLET | > TP_ACPI_MULTI_MODE_STAND | > TP_ACPI_MULTI_MODE_TENT; > break; > default: > pr_err("Unknown multi mode status type %d with value 0x%04X, please report this to %s\n", > type, value, TPACPI_MAIL); > return 0; > -- > 2.13.6 >
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 91fab1a13a6d..d23c84222e9f 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -2106,26 +2106,24 @@ static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode) TP_ACPI_MULTI_MODE_FLAT | TP_ACPI_MULTI_MODE_TABLET | TP_ACPI_MULTI_MODE_STAND | TP_ACPI_MULTI_MODE_TENT; break; case 3: valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | TP_ACPI_MULTI_MODE_FLAT; break; case 4: - valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | - TP_ACPI_MULTI_MODE_TABLET | - TP_ACPI_MULTI_MODE_STAND | - TP_ACPI_MULTI_MODE_TENT; - break; case 5: + /* In mode 4, FLAT is not specified as a valid mode. However, + * it can be seen at least on the X1 Yoga 2nd Generation. + */ valid_modes = TP_ACPI_MULTI_MODE_LAPTOP | TP_ACPI_MULTI_MODE_FLAT | TP_ACPI_MULTI_MODE_TABLET | TP_ACPI_MULTI_MODE_STAND | TP_ACPI_MULTI_MODE_TENT; break; default: pr_err("Unknown multi mode status type %d with value 0x%04X, please report this to %s\n", type, value, TPACPI_MAIL); return 0;
On the X1 Yoga 2nd Generation and most likely other notebooks the FLAT mode is reported. Decode it correctly rather than warning about an unexpected multi mode status to be reported. Signed-off-by: Benjamin Berg <bberg@redhat.com> Cc: Peter FP1 Zhang <zhangfp1@lenovo.com Cc: Lyude <lyude@redhat.com> --- drivers/platform/x86/thinkpad_acpi.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)