| Message ID | 20170615064832.4148-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: > Add touchscreen info for the Point of View mobii wintab p800w tablet. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/platform/x86/silead_dmi.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) ... > + /* Point of View mobii wintab p800w */ > + .driver_data = (void *)&pov_mobii_wintab_p800w_data, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), > + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), > + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), > + /* Above matches are too generic, add bios-date match */ > + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), This is the first time I've seen a BIOS date match used to determine hardware features. DMI matching is a (necessary) hack to begin with (the vendors should be providing this data via ACPI _DSD anyway) but a date match means we would need a kernel patch every time one of these tablets gets a BIOS update... With words like "Aptio CRB" it's clear the vendor isn't doing their job and just using unmodified reference code. The problem with this of course is that the vendor is not providing a way to identify this hardware. Andy, I'd appreciate your thoughts on this... I'm leaning towards not accepting bios date (or indeed, BIOS version) as a way to identify a platform.
Hi, On 15-06-17 18:53, Darren Hart wrote: > On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: >> Add touchscreen info for the Point of View mobii wintab p800w tablet. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/platform/x86/silead_dmi.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) > > ... > >> + /* Point of View mobii wintab p800w */ >> + .driver_data = (void *)&pov_mobii_wintab_p800w_data, >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >> + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), >> + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), >> + /* Above matches are too generic, add bios-date match */ >> + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), > > This is the first time I've seen a BIOS date match used to determine hardware > features. DMI matching is a (necessary) hack to begin with (the vendors should > be providing this data via ACPI _DSD anyway) but a date match means we would > need a kernel patch every time one of these tablets gets a BIOS update... I don't think this specific model is still being produced and I've never seen a vendor issue a BIOS update to one of these cheap tablets. The Asus and Dell, etc. Bay and Cherry Trail devices get BIOS updates (and have proper DMI strings), the cheap OEM models are more a case of ship it and never look back. So for this specific entry I'm not worried about BIOS updates. > With words like "Aptio CRB" it's clear the vendor isn't doing their job and just > using unmodified reference code. The problem with this of course is that the > vendor is not providing a way to identify this hardware. Yes, I'm just as unhappy with this as you are, the alternative is users with such devices not having a working touchscreen. So my vote goes to live with the ugliness (added to the ugliness of needing the table at all), but I understand your reluctance towards this. Regards, Hans
On Thu, Jun 15, 2017 at 7:53 PM, Darren Hart <dvhart@infradead.org> wrote: > On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: >> + /* Point of View mobii wintab p800w */ >> + .driver_data = (void *)&pov_mobii_wintab_p800w_data, >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >> + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), >> + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), >> + /* Above matches are too generic, add bios-date match */ >> + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), > > This is the first time I've seen a BIOS date match used to determine hardware > features. DMI matching is a (necessary) hack to begin with (the vendors should > be providing this data via ACPI _DSD anyway) but a date match means we would > need a kernel patch every time one of these tablets gets a BIOS update... > > With words like "Aptio CRB" it's clear the vendor isn't doing their job and just > using unmodified reference code. The problem with this of course is that the > vendor is not providing a way to identify this hardware. > > Andy, I'd appreciate your thoughts on this... I'm leaning towards not accepting > bios date (or indeed, BIOS version) as a way to identify a platform. The question is what is the anticipated amount of affected devices with BIOS date included and otherwise? If Hans believes that there will be no update for some devices, while there are devices with the same DMI strings, but different date and _fixed_ issue, I think we have no other choice for now. Also can we use some other strings to distinguish group of devices which are affected? In pinctrl-cherryview.c we faced same issue, though for now there all platforms are affected (first approach was to distinguish with BIOS date) and we decide to go with black and white lists (white is empty for now and will be created whenever we will have an update).
Hi, On 16-06-17 14:44, Andy Shevchenko wrote: > On Thu, Jun 15, 2017 at 7:53 PM, Darren Hart <dvhart@infradead.org> wrote: >> On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: > >>> + /* Point of View mobii wintab p800w */ >>> + .driver_data = (void *)&pov_mobii_wintab_p800w_data, >>> + .matches = { >>> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >>> + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), >>> + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), >>> + /* Above matches are too generic, add bios-date match */ >>> + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), >> >> This is the first time I've seen a BIOS date match used to determine hardware >> features. DMI matching is a (necessary) hack to begin with (the vendors should >> be providing this data via ACPI _DSD anyway) but a date match means we would >> need a kernel patch every time one of these tablets gets a BIOS update... >> >> With words like "Aptio CRB" it's clear the vendor isn't doing their job and just >> using unmodified reference code. The problem with this of course is that the >> vendor is not providing a way to identify this hardware. >> >> Andy, I'd appreciate your thoughts on this... I'm leaning towards not accepting >> bios date (or indeed, BIOS version) as a way to identify a platform. > > The question is what is the anticipated amount of affected devices > with BIOS date included and otherwise? I expect there to be collisions (false positive matches) without the BIOS_DATE check, a quick web-search finds other devices with a 3BAIR1013 bios version. Those don't necessarily also use a Silead touchscreen (which is needed for a collision to happen), but given the popularity of Silead touchscreens on cheap devices a collision is not unlikely. With the bios-date check added, I expect this match to be unique, for it to not be unique we would need to be really unlucky. > If Hans believes that there will be no update for some devices, Yeah I'm pretty sure this specific device will not see any BIOS updates ever. > while there are devices with the same DMI strings, but different date and > _fixed_ issue, I think we have no other choice for now. > Also can we use some other strings to distinguish group of devices > which are affected? bios_date: 08/22/2014 bios_vendor: American Megatrends Inc. bios_version: 3BAIR1013 board_asset_tag: To be filled by O.E.M. board_name: Aptio CRB board_serial: T80091A4C11B0848 board_vendor: AMI Corporation board_version: To be filled by O.E.M. chassis_asset_tag: To Be Filled By O.E.M. chassis_serial: To Be Filled By O.E.M. chassis_type: 3 chassis_vendor: To Be Filled By O.E.M. chassis_version: To Be Filled By O.E.M. product_name: To be filled by O.E.M. product_serial: To be filled by O.E.M. product_uuid: 03000200-0400-0500-0006-000700080009 product_version: To be filled by O.E.M. sys_vendor: To be filled by O.E.M. The product-uuid is a known example uuid, so is no good. The board_serial might be useful, but only if it is unique for the model and not per tablet. Unfortunately I only have 1 of these tablets, so I cannot tell. Regards, Hans
On Fri, Jun 16, 2017 at 03:22:45PM +0200, Hans de Goede wrote: > Hi, > > On 16-06-17 14:44, Andy Shevchenko wrote: > > On Thu, Jun 15, 2017 at 7:53 PM, Darren Hart <dvhart@infradead.org> wrote: > > > On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: > > > > > > + /* Point of View mobii wintab p800w */ > > > > + .driver_data = (void *)&pov_mobii_wintab_p800w_data, > > > > + .matches = { > > > > + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), > > > > + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), > > > > + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), > > > > + /* Above matches are too generic, add bios-date match */ > > > > + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), > > > > > > This is the first time I've seen a BIOS date match used to determine hardware > > > features. DMI matching is a (necessary) hack to begin with (the vendors should > > > be providing this data via ACPI _DSD anyway) but a date match means we would > > > need a kernel patch every time one of these tablets gets a BIOS update... > > > > > > With words like "Aptio CRB" it's clear the vendor isn't doing their job and just > > > using unmodified reference code. The problem with this of course is that the > > > vendor is not providing a way to identify this hardware. > > > > > > Andy, I'd appreciate your thoughts on this... I'm leaning towards not accepting > > > bios date (or indeed, BIOS version) as a way to identify a platform. > > > > The question is what is the anticipated amount of affected devices > > with BIOS date included and otherwise? > > I expect there to be collisions (false positive matches) without the > BIOS_DATE check, a quick web-search finds other devices with a > 3BAIR1013 bios version. Those don't necessarily also use a Silead > touchscreen (which is needed for a collision to happen), but given > the popularity of Silead touchscreens on cheap devices a collision > is not unlikely. > > With the bios-date check added, I expect this match to be unique, > for it to not be unique we would need to be really unlucky. > > > If Hans believes that there will be no update for some devices, > > Yeah I'm pretty sure this specific device will not see any > BIOS updates ever. > > > while there are devices with the same DMI strings, but different date and > > _fixed_ issue, I think we have no other choice for now. > > Also can we use some other strings to distinguish group of devices > > which are affected? > > bios_date: 08/22/2014 > bios_vendor: American Megatrends Inc. > bios_version: 3BAIR1013 > board_asset_tag: To be filled by O.E.M. > board_name: Aptio CRB > board_serial: T80091A4C11B0848 > board_vendor: AMI Corporation > board_version: To be filled by O.E.M. > chassis_asset_tag: To Be Filled By O.E.M. > chassis_serial: To Be Filled By O.E.M. > chassis_type: 3 > chassis_vendor: To Be Filled By O.E.M. > chassis_version: To Be Filled By O.E.M. > product_name: To be filled by O.E.M. > product_serial: To be filled by O.E.M. > product_uuid: 03000200-0400-0500-0006-000700080009 > product_version: To be filled by O.E.M. > sys_vendor: To be filled by O.E.M. > > The product-uuid is a known example uuid, so is > no good. The board_serial might be useful, but > only if it is unique for the model and not per > tablet. Unfortunately I only have 1 of these > tablets, so I cannot tell. Do we have any indication that this BIOS Date isn't just the default value provided by AMI? Does it offer any more information than the BIOS Version? I suppose we may be able to do some kind of a partial match on the Board Serial if even that is platform specific (I suspect it is with the T800 at the beginning. The sloppy handling of this firmware really irks me. That's obviously not Hans' fault, so we'll take the patch. If we see a conflict in the future, we'll just have to compare the other DMI strings for a match and see what we can do.... I'm even tempted to insert a printk on this match, dumping the DMI values and requesting the user to copy.paste them into an email to this list.... I think we've already spent too much time on this patch based on this review: https://www.notebookcheck.net/Point-of-View-Mobii-WinTab-800W-Tablet-Review.129561.0.html Nice...
Hi, On 17-06-17 01:38, Darren Hart wrote: > On Fri, Jun 16, 2017 at 03:22:45PM +0200, Hans de Goede wrote: >> Hi, >> >> On 16-06-17 14:44, Andy Shevchenko wrote: >>> On Thu, Jun 15, 2017 at 7:53 PM, Darren Hart <dvhart@infradead.org> wrote: >>>> On Thu, Jun 15, 2017 at 08:48:31AM +0200, Hans de Goede wrote: >>> >>>>> + /* Point of View mobii wintab p800w */ >>>>> + .driver_data = (void *)&pov_mobii_wintab_p800w_data, >>>>> + .matches = { >>>>> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >>>>> + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), >>>>> + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), >>>>> + /* Above matches are too generic, add bios-date match */ >>>>> + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), >>>> >>>> This is the first time I've seen a BIOS date match used to determine hardware >>>> features. DMI matching is a (necessary) hack to begin with (the vendors should >>>> be providing this data via ACPI _DSD anyway) but a date match means we would >>>> need a kernel patch every time one of these tablets gets a BIOS update... >>>> >>>> With words like "Aptio CRB" it's clear the vendor isn't doing their job and just >>>> using unmodified reference code. The problem with this of course is that the >>>> vendor is not providing a way to identify this hardware. >>>> >>>> Andy, I'd appreciate your thoughts on this... I'm leaning towards not accepting >>>> bios date (or indeed, BIOS version) as a way to identify a platform. >>> >>> The question is what is the anticipated amount of affected devices >>> with BIOS date included and otherwise? >> >> I expect there to be collisions (false positive matches) without the >> BIOS_DATE check, a quick web-search finds other devices with a >> 3BAIR1013 bios version. Those don't necessarily also use a Silead >> touchscreen (which is needed for a collision to happen), but given >> the popularity of Silead touchscreens on cheap devices a collision >> is not unlikely. >> >> With the bios-date check added, I expect this match to be unique, >> for it to not be unique we would need to be really unlucky. >> >>> If Hans believes that there will be no update for some devices, >> >> Yeah I'm pretty sure this specific device will not see any >> BIOS updates ever. >> >>> while there are devices with the same DMI strings, but different date and >>> _fixed_ issue, I think we have no other choice for now. >>> Also can we use some other strings to distinguish group of devices >>> which are affected? >> >> bios_date: 08/22/2014 >> bios_vendor: American Megatrends Inc. >> bios_version: 3BAIR1013 >> board_asset_tag: To be filled by O.E.M. >> board_name: Aptio CRB >> board_serial: T80091A4C11B0848 >> board_vendor: AMI Corporation >> board_version: To be filled by O.E.M. >> chassis_asset_tag: To Be Filled By O.E.M. >> chassis_serial: To Be Filled By O.E.M. >> chassis_type: 3 >> chassis_vendor: To Be Filled By O.E.M. >> chassis_version: To Be Filled By O.E.M. >> product_name: To be filled by O.E.M. >> product_serial: To be filled by O.E.M. >> product_uuid: 03000200-0400-0500-0006-000700080009 >> product_version: To be filled by O.E.M. >> sys_vendor: To be filled by O.E.M. >> >> The product-uuid is a known example uuid, so is >> no good. The board_serial might be useful, but >> only if it is unique for the model and not per >> tablet. Unfortunately I only have 1 of these >> tablets, so I cannot tell. > > Do we have any indication that this BIOS Date isn't just the default value > provided by AMI? In my experience with devices with similar generic DMI strings, the BIOS date is different for all of them, I believe the tools use to build the BIOS set this to the actual build date. > Does it offer any more information than the BIOS Version? Yes it does a web search for "3BAIR1013" finds unrelated matches, so that alone is not enough, where as the BIOS date tends to be unique. > I suppose we may be able to do some kind of a partial match on the Board Serial > if even that is platform specific (I suspect it is with the T800 at the > beginning. > > The sloppy handling of this firmware really irks me. That's obviously not Hans' > fault, so we'll take the patch. Thank you. Regards, Hans
diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/silead_dmi.c index db3a877d2160..46c5e1ebfb53 100644 --- a/drivers/platform/x86/silead_dmi.c +++ b/drivers/platform/x86/silead_dmi.c @@ -93,6 +93,20 @@ static const struct silead_ts_dmi_data gp_electronic_t701_data = { .properties = gp_electronic_t701_props, }; +static const struct property_entry pov_mobii_wintab_p800w_props[] = { + PROPERTY_ENTRY_U32("touchscreen-size-x", 1800), + PROPERTY_ENTRY_U32("touchscreen-size-y", 1150), + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), + PROPERTY_ENTRY_STRING("firmware-name", + "gsl3692-pov-mobii-wintab-p800w.fw"), + { } +}; + +static const struct silead_ts_dmi_data pov_mobii_wintab_p800w_data = { + .acpi_name = "MSSL1680:00", + .properties = pov_mobii_wintab_p800w_props, +}; + static const struct dmi_system_id silead_ts_dmi_table[] = { { /* CUBE iwork8 Air */ @@ -139,6 +153,17 @@ static const struct dmi_system_id silead_ts_dmi_table[] = { DMI_MATCH(DMI_BIOS_VERSION, "BYT70A.YNCHENG.WIN.007"), }, }, + { + /* Point of View mobii wintab p800w */ + .driver_data = (void *)&pov_mobii_wintab_p800w_data, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), + DMI_MATCH(DMI_BOARD_NAME, "Aptio CRB"), + DMI_MATCH(DMI_BIOS_VERSION, "3BAIR1013"), + /* Above matches are too generic, add bios-date match */ + DMI_MATCH(DMI_BIOS_DATE, "08/22/2014"), + }, + }, { }, };
Add touchscreen info for the Point of View mobii wintab p800w tablet. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/silead_dmi.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)