linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: BMI160 accelerometer on AyaNeo tablet
       [not found] <CACAwPwb7edLzX-KO1XVNWuQ3w=U0BfA=_kwiGCjZOpKfZpc2pw@mail.gmail.com>
@ 2021-10-16 16:27 ` Maxim Levitsky
  2021-10-17 10:58   ` Jonathan Cameron
  2021-10-18 15:30   ` Andy Shevchenko
  0 siblings, 2 replies; 15+ messages in thread
From: Maxim Levitsky @ 2021-10-16 16:27 UTC (permalink / raw)
  To: linux-iio; +Cc: Lars-Peter Clausen, Andy Shevchenko, LKML

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

On Sat, Oct 16, 2021 at 7:19 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>
> I recently bought this device, and it has this accelerometer/gyroscope.
>
> Unfortunately, the device is advertised in ACPI as 10EC5280, instead of BMI0160
>
> I attached a patch that does add this 10EC5280 to the list of ACPI ids of this driver, and the device seems to work fine, showing both acceleration and angular velocity in /sys IIO attributes with reasonable values.
>

( resend using plain text - reminds me to never use Gmail's web
interface, even on weekends .)

> Best regards,
>    Maxim Levitsky

[-- Attachment #2: accel_fix.patch --]
[-- Type: text/x-patch, Size: 771 bytes --]

commit 880bbf2e5ee2fa0e99798482664997a7db225f56
Author: Maxim Levitsky <mlevitsk@redhat.com>
Date:   Sat Oct 2 20:54:00 2021 +0300

    BMI160: AYA NEA accelometer ID
    
    On AYA NEO, the accelerometer is BMI160 but it is exposed
    via ACPI as 10EC5280
    
    Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
index 26398614eddfa..2b3e3e15e2e04 100644
--- a/drivers/iio/imu/bmi160/bmi160_i2c.c
+++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
@@ -43,6 +43,7 @@ MODULE_DEVICE_TABLE(i2c, bmi160_i2c_id);
 
 static const struct acpi_device_id bmi160_acpi_match[] = {
 	{"BMI0160", 0},
+	{"10EC5280", 0}, /* AYA NEO tablet */
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-16 16:27 ` BMI160 accelerometer on AyaNeo tablet Maxim Levitsky
@ 2021-10-17 10:58   ` Jonathan Cameron
  2021-10-18  7:40     ` Andy Shevchenko
  2021-10-18 15:30   ` Andy Shevchenko
  1 sibling, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-17 10:58 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: linux-iio, Lars-Peter Clausen, Andy Shevchenko, LKML

On Sat, 16 Oct 2021 19:27:50 +0300
Maxim Levitsky <maximlevitsky@gmail.com> wrote:

>  BMI160: AYA NEA accelometer ID
>     
>     On AYA NEO, the accelerometer is BMI160 but it is exposed
>     via ACPI as 10EC5280
>     
>     Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

I guess it is hopelessly optimistic to hope that we could let someone
at the supplier know that's a totally invalid ACPI id and that they
should clean up their act.

Curiously it looks like a valid PCI ID pair though for a realtek device.

Ah well.  Applied to the iio-togreg branch of iio.git and pushed out
as testing to see if 0-day can find any issues with it.

Thanks,

Jonathan
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-17 10:58   ` Jonathan Cameron
@ 2021-10-18  7:40     ` Andy Shevchenko
  2021-10-18 15:17       ` Jonathan Cameron
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-18  7:40 UTC (permalink / raw)
  To: Jonathan Cameron, Hans de Goede
  Cc: Maxim Levitsky, linux-iio, Lars-Peter Clausen, Andy Shevchenko, LKML

+Cc: Hans

On Mon, Oct 18, 2021 at 6:41 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sat, 16 Oct 2021 19:27:50 +0300
> Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>
> >  BMI160: AYA NEA accelometer ID

accelerometer

> >     On AYA NEO, the accelerometer is BMI160 but it is exposed
> >     via ACPI as 10EC5280
> >
> >     Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>
> I guess it is hopelessly optimistic to hope that we could let someone
> at the supplier know that's a totally invalid ACPI id and that they
> should clean up their act.
>
> Curiously it looks like a valid PCI ID pair though for a realtek device.
>
> Ah well.  Applied to the iio-togreg branch of iio.git and pushed out
> as testing to see if 0-day can find any issues with it.

NAK. And I explain below why and how to make progress with it.

The commit message should contain at least the link to the DSDT and
official technical description of the platform. Besides that, it
should have a corresponding comment near to the ID in the code.

On top of that, in particular to this case, the ID is very valid from
the ACPI specification point of view, but in this case it's a
representation of the PCI ID 10ec:5280 which is Realtek owned. So, we
need to hear (okay in reasonable time) from Realtek (I believe they
are active in the Linux kernel) and that OEM.

I hardly believe that Realtek has issued a special ID from the range
where mostly PCIe ports or so are allocated, although it's possible.
We need proof.

What I believe is the case here is that OEMs are just quite diletants
in ACPI and firmware and they messed up with BIOS somehow that it
issued the ID for the device.
There are also two other possibilities: OEM stole the ID (deliberately
or accidentally), or the device is not just gyro, but something which
contains gyro.

As to the last paragraph, see above, we must see DSDT. Without it I
have a strong NAK.

P.S. Jonathan, please do not be so fast next time with ACPI IDs.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18  7:40     ` Andy Shevchenko
@ 2021-10-18 15:17       ` Jonathan Cameron
  2021-10-18 15:22         ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-18 15:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Hans de Goede, Maxim Levitsky, linux-iio,
	Lars-Peter Clausen, Andy Shevchenko, LKML

On Mon, 18 Oct 2021 10:40:33 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> +Cc: Hans
> 
> On Mon, Oct 18, 2021 at 6:41 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sat, 16 Oct 2021 19:27:50 +0300
> > Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> >  
> > >  BMI160: AYA NEA accelometer ID  
> 
> accelerometer
> 
> > >     On AYA NEO, the accelerometer is BMI160 but it is exposed
> > >     via ACPI as 10EC5280
> > >
> > >     Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>  
> >
> > I guess it is hopelessly optimistic to hope that we could let someone
> > at the supplier know that's a totally invalid ACPI id and that they
> > should clean up their act.
> >
> > Curiously it looks like a valid PCI ID pair though for a realtek device.
> >
> > Ah well.  Applied to the iio-togreg branch of iio.git and pushed out
> > as testing to see if 0-day can find any issues with it.  
> 
> NAK. And I explain below why and how to make progress with it.
> 
> The commit message should contain at least the link to the DSDT and
> official technical description of the platform. Besides that, it
> should have a corresponding comment near to the ID in the code.
> 
> On top of that, in particular to this case, the ID is very valid from
> the ACPI specification point of view, but in this case it's a
> representation of the PCI ID 10ec:5280 which is Realtek owned. So, we
> need to hear (okay in reasonable time) from Realtek (I believe they
> are active in the Linux kernel) and that OEM.
> 
> I hardly believe that Realtek has issued a special ID from the range
> where mostly PCIe ports or so are allocated, although it's possible.
> We need proof.
> 
> What I believe is the case here is that OEMs are just quite diletants
> in ACPI and firmware and they messed up with BIOS somehow that it
> issued the ID for the device.
> There are also two other possibilities: OEM stole the ID (deliberately
> or accidentally), or the device is not just gyro, but something which
> contains gyro.
> 
> As to the last paragraph, see above, we must see DSDT. Without it I
> have a strong NAK.
> 
> P.S. Jonathan, please do not be so fast next time with ACPI IDs.

No problem.  Will pull this one once I'm back on correct PC.

Jonathan

> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 15:17       ` Jonathan Cameron
@ 2021-10-18 15:22         ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-18 15:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jonathan Cameron, Hans de Goede, Maxim Levitsky, linux-iio,
	Lars-Peter Clausen, Andy Shevchenko, LKML

On Mon, Oct 18, 2021 at 6:17 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Mon, 18 Oct 2021 10:40:33 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Mon, Oct 18, 2021 at 6:41 AM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Sat, 16 Oct 2021 19:27:50 +0300
> > > Maxim Levitsky <maximlevitsky@gmail.com> wrote:

...

> > I hardly believe that Realtek has issued a special ID from the range

Slight correction: ID --> PCI ID

> > where mostly PCIe ports or so are allocated, although it's possible.
> > We need proof.
> >
> > What I believe is the case here is that OEMs are just quite diletants
> > in ACPI and firmware and they messed up with BIOS somehow that it
> > issued the ID for the device.
> > There are also two other possibilities: OEM stole the ID (deliberately
> > or accidentally), or the device is not just gyro, but something which
> > contains gyro.
> >
> > As to the last paragraph, see above, we must see DSDT. Without it I
> > have a strong NAK.
> >
> > P.S. Jonathan, please do not be so fast next time with ACPI IDs.
>
> No problem.  Will pull this one once I'm back on correct PC.

Thank you!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-16 16:27 ` BMI160 accelerometer on AyaNeo tablet Maxim Levitsky
  2021-10-17 10:58   ` Jonathan Cameron
@ 2021-10-18 15:30   ` Andy Shevchenko
  2021-10-18 18:02     ` Maxim Levitsky
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-18 15:30 UTC (permalink / raw)
  To: Maxim Levitsky, Hans de Goede, linux-realtek-soc, Oder Chiou,
	Ping-Ke Shih, nic_swsd, Derek Fang, Hayes Wang, Kailang Yang
  Cc: linux-iio, Lars-Peter Clausen, Andy Shevchenko, LKML

+Cc: Realtek people whom I found in MAINTAINERS or so. Please
waterfall to the people inside Realtek who can answer the question.
(Note, you may access this discussion in full via:
https://lore.kernel.org/linux-iio/CACAwPwYQHRcrabw9=0tvenPzAcwwW1pTaR6a+AEWBF9Hqf_wXQ@mail.gmail.com/T/#u)

The problem here is to have an official confirmation of what 10ec:5280
ID is from Realtek's point of view. Context: the current discussion
and a patch state that it's related to gyro sensor. Is it so?

On Mon, Oct 18, 2021 at 6:36 AM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>
> On Sat, Oct 16, 2021 at 7:19 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> >
> > I recently bought this device, and it has this accelerometer/gyroscope.
> >
> > Unfortunately, the device is advertised in ACPI as 10EC5280, instead of BMI0160
> >
> > I attached a patch that does add this 10EC5280 to the list of ACPI ids of this driver, and the device seems to work fine, showing both acceleration and angular velocity in /sys IIO attributes with reasonable values.
> >
>
> ( resend using plain text - reminds me to never use Gmail's web
> interface, even on weekends .)


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 15:30   ` Andy Shevchenko
@ 2021-10-18 18:02     ` Maxim Levitsky
  2021-10-18 19:02       ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Maxim Levitsky @ 2021-10-18 18:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Hans de Goede, linux-realtek-soc, Oder Chiou, Ping-Ke Shih,
	nic_swsd, Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, Andy Shevchenko, LKML, info

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

I also suspect a mistake from the hardware vendors.

I attached all DSDT decompiled, which shows that they indeed use that
ID, and I also attached the windows driver .INF which was published on
their website  with the driver (https://www.ayaneo.com/downloads)


They are a small startup so they might have used the realtek ID by mistake.
I added them to the CC.

BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
BMI160 driver doesn't recognize it.

Best regards,
    Maxim Levitsky


On Mon, Oct 18, 2021 at 6:31 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> +Cc: Realtek people whom I found in MAINTAINERS or so. Please
> waterfall to the people inside Realtek who can answer the question.
> (Note, you may access this discussion in full via:
> https://lore.kernel.org/linux-iio/CACAwPwYQHRcrabw9=0tvenPzAcwwW1pTaR6a+AEWBF9Hqf_wXQ@mail.gmail.com/T/#u)
>
> The problem here is to have an official confirmation of what 10ec:5280
> ID is from Realtek's point of view. Context: the current discussion
> and a patch state that it's related to gyro sensor. Is it so?
>
> On Mon, Oct 18, 2021 at 6:36 AM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> >
> > On Sat, Oct 16, 2021 at 7:19 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > >
> > > I recently bought this device, and it has this accelerometer/gyroscope.
> > >
> > > Unfortunately, the device is advertised in ACPI as 10EC5280, instead of BMI0160
> > >
> > > I attached a patch that does add this 10EC5280 to the list of ACPI ids of this driver, and the device seems to work fine, showing both acceleration and angular velocity in /sys IIO attributes with reasonable values.
> > >
> >
> > ( resend using plain text - reminds me to never use Gmail's web
> > interface, even on weekends .)
>
>
> --
> With Best Regards,
> Andy Shevchenko

[-- Attachment #2: BMI160.inf --]
[-- Type: application/octet-stream, Size: 2351 bytes --]

/*++
;
;Copyright (c) Microsoft Corporation.  All rights reserved.
;
;Module Name:
;    BMI160.INF
;
;Abstract:
;    INF file for installing the Sensors 2.0 BMI160 sample driver
;
;Installation Notes:
;    Using Devcon: Type "devcon install BMI160.inf umdf2\BMI160" to install
;
;--*/

[Version]
Signature   = "$WINDOWS NT$"
Class       = Sensor
ClassGuid   = {5175D334-C371-4806-B3BA-71FD53C9258D}
Provider    = %BOSCH%
CatalogFile = BMI160.cat
DriverVer = 07/06/2020,1.0.0.17

[SourceDisksNames]
1 = %MediaDescription%,,,""

[SourceDisksFiles]
BMI160.dll   = 1,,

[Manufacturer]
%ManufacturerName% = BMI160_Device, NTamd64

;*******************************
; BMI160 Install Section
;*******************************

[BMI160_Device.NTamd64]
; DisplayName       Section          DeviceId
; -----------       -------          --------
%BMI160_DevDesc% = BMI160_Inst, ACPI\10EC5280

[BMI160_Inst.NT]
CopyFiles = BMI160DriverCopy

[BMI160DriverCopy]
BMI160.dll

[DestinationDirs]
BMI160DriverCopy = 12,UMDF

;-------------- Service installation

[BMI160_Inst.NT.Services]
AddService = WUDFRd,0x000001fa,WUDFRD_ServiceInstall

[BMI160_Inst.NT.CoInstallers]
AddReg = CoInstallers_AddReg

[WUDFRD_ServiceInstall]
DisplayName   = %WudfRdDisplayName%
ServiceType   = %SERVICE_KERNEL_DRIVER%
StartType     = %SERVICE_DEMAND_START%
ErrorControl  = %SERVICE_ERROR_NORMAL%
ServiceBinary = %12%\WUDFRd.sys

;-------------- WDF specific section

[BMI160_Inst.NT.Wdf]
UmdfService              = BMI160, BMI160_Install
UmdfServiceOrder         = BMI160
UmdfDirectHardwareAccess = AllowDirectHardwareAccess
UmdfFileObjectPolicy     = AllowNullAndUnknownFileObjects
UmdfFsContextUsePolicy   = CannotUseFsContexts

[BMI160_Install]
UmdfLibraryVersion       = 2.15.0
ServiceBinary            = %12%\UMDF\BMI160.dll
UmdfExtensions           = SensorsCx0102

[CoInstallers_AddReg]
HKR,,CoInstallers32,0x00010000,"WudfCoinstaller.dll"

[Strings]
MediaDescription         = "Windows BMI160 Driver"
BOSCH                    = "Bosch"
ManufacturerName         = "Bosch"
BMI160_DevDesc        = "BMI160"
WudfRdDisplayName        = "Windows Driver Foundation - User-mode Driver Framework Reflector"

SERVICE_KERNEL_DRIVER    = 1
SERVICE_DEMAND_START     = 3
SERVICE_ERROR_NORMAL     = 1

[-- Attachment #3: DSDT.zip --]
[-- Type: application/zip, Size: 26703 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 18:02     ` Maxim Levitsky
@ 2021-10-18 19:02       ` Andy Shevchenko
  2021-10-18 20:42         ` Maxim Levitsky
  2021-10-19  8:10         ` Hans de Goede
  0 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-18 19:02 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Hans de Goede, linux-realtek-soc, Oder Chiou, Ping-Ke Shih,
	nic_swsd, Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:
> I also suspect a mistake from the hardware vendors.
> 
> I attached all DSDT decompiled, which shows that they indeed use that
> ID, and I also attached the windows driver .INF which was published on
> their website  with the driver (https://www.ayaneo.com/downloads)
> 
> They are a small startup so they might have used the realtek ID by mistake.
> I added them to the CC.

Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
ID. So there are questions I have:
- Is the firmware available in the wild?
- Do they plan to update firmware to fix this?
- Can we make sure that guys got their mistake and will be more careful
  in the future?

Realtek probably should make this ID marked somehow broken and not use
in their products in case the answer to the first of the above question
is "yes". (Of course in case the ID will be used for solely PCI enumerated
product there will be no conflict, I just propose to be on the safest side,
but remark should be made somewhere).

> BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
> BMI160 driver doesn't recognize it.

This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
mount-matrix from ACPI") which needs to be amended to take care about
more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
Hans, what do you think?

P.S. As I said, the commit message and the code (in the comments) should
be very well elaborated and only accepted in case the firmware is already
in the wild on the market.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 19:02       ` Andy Shevchenko
@ 2021-10-18 20:42         ` Maxim Levitsky
  2021-10-19  9:58           ` Andy Shevchenko
  2021-10-19  8:10         ` Hans de Goede
  1 sibling, 1 reply; 15+ messages in thread
From: Maxim Levitsky @ 2021-10-18 20:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Hans de Goede, linux-realtek-soc, Oder Chiou, Ping-Ke Shih,
	nic_swsd, Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

On Mon, Oct 18, 2021 at 10:02 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:
> > I also suspect a mistake from the hardware vendors.
> >
> > I attached all DSDT decompiled, which shows that they indeed use that
> > ID, and I also attached the windows driver .INF which was published on
> > their website  with the driver (https://www.ayaneo.com/downloads)
> >
> > They are a small startup so they might have used the realtek ID by mistake.
> > I added them to the CC.
>
> Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
> ID. So there are questions I have:
> - Is the firmware available in the wild?

Likely so. It looks Aya team only released a single windows driver which
works on all revisions of their device including the Founder Edition,
which was released more that a year ago.

It is likely that all 3 revisions that they sold carry this ACPI ID.
(The founder edition, first batch of IndieGoGo orders which had a
redesigned shell,
and 2nd batch (which I have) which has a new wifi card, a bit better
controller,
among other changes).


> - Do they plan to update firmware to fix this?
> - Can we make sure that guys got their mistake and will be more careful
>   in the future?

I CCed them, hoping that they would hear us. I can also raise this on their
discord when I find time to look there.

>
> Realtek probably should make this ID marked somehow broken and not use
> in their products in case the answer to the first of the above question
> is "yes". (Of course in case the ID will be used for solely PCI enumerated
> product there will be no conflict, I just propose to be on the safest side,
> but remark should be made somewhere).

>
> > BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
> > BMI160 driver doesn't recognize it.
>
> This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> mount-matrix from ACPI") which needs to be amended to take care about
> more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> Hans, what do you think?

If you like to, I can probably volunteer to prepare a patch for this myself next
weekend, using this pointer as a reference.

>
> P.S. As I said, the commit message and the code (in the comments) should
> be very well elaborated and only accepted in case the firmware is already
> in the wild on the market.

I will prepare a patch with a better commit description this weekend.


Thank you very much,
Best regards,
      Maxim Levitsky


>
> --
> With Best Regards,
> Andy Shevchenko
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 19:02       ` Andy Shevchenko
  2021-10-18 20:42         ` Maxim Levitsky
@ 2021-10-19  8:10         ` Hans de Goede
  1 sibling, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2021-10-19  8:10 UTC (permalink / raw)
  To: Andy Shevchenko, Maxim Levitsky
  Cc: linux-realtek-soc, Oder Chiou, Ping-Ke Shih, nic_swsd,
	Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

Hi,

On 10/18/21 21:02, Andy Shevchenko wrote:
> On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:
>> I also suspect a mistake from the hardware vendors.
>>
>> I attached all DSDT decompiled, which shows that they indeed use that
>> ID, and I also attached the windows driver .INF which was published on
>> their website  with the driver (https://www.ayaneo.com/downloads)
>>
>> They are a small startup so they might have used the realtek ID by mistake.
>> I added them to the CC.
> 
> Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
> ID. So there are questions I have:
> - Is the firmware available in the wild?
> - Do they plan to update firmware to fix this?
> - Can we make sure that guys got their mistake and will be more careful
>   in the future?
> 
> Realtek probably should make this ID marked somehow broken and not use
> in their products in case the answer to the first of the above question
> is "yes". (Of course in case the ID will be used for solely PCI enumerated
> product there will be no conflict, I just propose to be on the safest side,
> but remark should be made somewhere).
> 
>> BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
>> BMI160 driver doesn't recognize it.
> 
> This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> mount-matrix from ACPI") which needs to be amended to take care about
> more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> Hans, what do you think?

First of all the vendor needs to be asked to fix their DSDT to just
use BOSC0200 as HID. That will fix both the driver not binding as well
as it will make the bmc150_apply_acpi_orientation() just work.

If we are going to add the funky ACPI HID to the driver, then this
HID check in bmc150_apply_acpi_orientation():

	if (!adev || !acpi_dev_hid_uid_match(adev, "BOSC0200", NULL))
		return false;

Should probable just be dropped changing the check to just:

	if (!adev)
		return false;

We already check for the method name later, so the HID check is not
really necessary.

This dropping of the HID check should probably be done in a separate
commit, with its own explanation of why this is ok.

Regards,

Hans


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-18 20:42         ` Maxim Levitsky
@ 2021-10-19  9:58           ` Andy Shevchenko
  2021-10-19 16:29             ` Maxim Levitsky
                               ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-19  9:58 UTC (permalink / raw)
  To: Maxim Levitsky, Andy Shevchenko
  Cc: Hans de Goede, linux-realtek-soc, Oder Chiou, Ping-Ke Shih,
	nic_swsd, Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

On Mon, Oct 18, 2021 at 11:42 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> On Mon, Oct 18, 2021 at 10:02 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:

...

> > Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
> > ID. So there are questions I have:
> > - Is the firmware available in the wild?
>
> Likely so. It looks Aya team only released a single windows driver which
> works on all revisions of their device including the Founder Edition,
> which was released more that a year ago.
>
> It is likely that all 3 revisions that they sold carry this ACPI ID.
> (The founder edition, first batch of IndieGoGo orders which had a
> redesigned shell,
> and 2nd batch (which I have) which has a new wifi card, a bit better
> controller,
> among other changes).
>
>
> > - Do they plan to update firmware to fix this?
> > - Can we make sure that guys got their mistake and will be more careful
> >   in the future?
>
> I CCed them, hoping that they would hear us. I can also raise this on their
> discord when I find time to look there.

I expect to have confirmation from them that they have got it and
promise to fix the firmware (ACPI tables) for supported and future
products.Can it be achieved? (Note, Hans already told what the HID
should be used there)

> > Realtek probably should make this ID marked somehow broken and not use
> > in their products in case the answer to the first of the above question
> > is "yes". (Of course in case the ID will be used for solely PCI enumerated
> > product there will be no conflict, I just propose to be on the safest side,
> > but remark should be made somewhere).

Any comments from Realtek, please?

> > > BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
> > > BMI160 driver doesn't recognize it.
> >
> > This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> > mount-matrix from ACPI") which needs to be amended to take care about
> > more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> > Hans, what do you think?
>
> If you like to, I can probably volunteer to prepare a patch for this myself next
> weekend, using this pointer as a reference.

The best is to cooperate with Hans as he is much more involved in the
topic of how these sensors are programmed and used in the Linux
kernel. My job here is to fix ACPI HID and
make every stakeholder be aware now and in the future.

...

> I will prepare a patch with a better commit description this weekend.

Thanks, but let's not be in such a hurry, I really want to hear from
both vendors. I guess a couple of weeks would be a reasonable time to
settle this down.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-19  9:58           ` Andy Shevchenko
@ 2021-10-19 16:29             ` Maxim Levitsky
  2021-10-20  2:31             ` Hayes Wang
  2021-10-20 17:37             ` Jonathan Cameron
  2 siblings, 0 replies; 15+ messages in thread
From: Maxim Levitsky @ 2021-10-19 16:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Hans de Goede, linux-realtek-soc, Oder Chiou,
	Ping-Ke Shih, nic_swsd, Derek Fang, Hayes Wang, Kailang Yang,
	linux-iio, Lars-Peter Clausen, LKML, info

On Tue, Oct 19, 2021 at 12:59 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 11:42 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Mon, Oct 18, 2021 at 10:02 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:
>
> ...
>
> > > Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
> > > ID. So there are questions I have:
> > > - Is the firmware available in the wild?
> >
> > Likely so. It looks Aya team only released a single windows driver which
> > works on all revisions of their device including the Founder Edition,
> > which was released more that a year ago.
> >
> > It is likely that all 3 revisions that they sold carry this ACPI ID.
> > (The founder edition, first batch of IndieGoGo orders which had a
> > redesigned shell,
> > and 2nd batch (which I have) which has a new wifi card, a bit better
> > controller,
> > among other changes).
> >
> >
> > > - Do they plan to update firmware to fix this?
> > > - Can we make sure that guys got their mistake and will be more careful
> > >   in the future?
> >
> > I CCed them, hoping that they would hear us. I can also raise this on their
> > discord when I find time to look there.
>
> I expect to have confirmation from them that they have got it and
> promise to fix the firmware (ACPI tables) for supported and future
> products.Can it be achieved? (Note, Hans already told what the HID
> should be used there)

Small Note: we are talking about BMI160 and not BMC150 and its ACPI HID is
I think is BMI0160. This doesn't change much, other that maybe a bit
more code to be added to read the rotation matrix.

>
> > > Realtek probably should make this ID marked somehow broken and not use
> > > in their products in case the answer to the first of the above question
> > > is "yes". (Of course in case the ID will be used for solely PCI enumerated
> > > product there will be no conflict, I just propose to be on the safest side,
> > > but remark should be made somewhere).
>
> Any comments from Realtek, please?
>
> > > > BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
> > > > BMI160 driver doesn't recognize it.
> > >
> > > This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> > > mount-matrix from ACPI") which needs to be amended to take care about
> > > more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> > > Hans, what do you think?
> >
> > If you like to, I can probably volunteer to prepare a patch for this myself next
> > weekend, using this pointer as a reference.
>
> The best is to cooperate with Hans as he is much more involved in the
> topic of how these sensors are programmed and used in the Linux
> kernel. My job here is to fix ACPI HID and
> make every stakeholder be aware now and in the future.

Yep, not a problem at all, I am open to test any patch to fix these issues,
as well as try to write my own.


>
> ...
>
> > I will prepare a patch with a better commit description this weekend.
>
> Thanks, but let's not be in such a hurry, I really want to hear from
> both vendors. I guess a couple of weeks would be a reasonable time to
> settle this down.

I guess you are right. Waiting a few weeks seems very reasonable.

Thanks,
Best regards,
        Maxim Levitsky


>
> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: BMI160 accelerometer on AyaNeo tablet
  2021-10-19  9:58           ` Andy Shevchenko
  2021-10-19 16:29             ` Maxim Levitsky
@ 2021-10-20  2:31             ` Hayes Wang
  2021-10-20 17:37             ` Jonathan Cameron
  2 siblings, 0 replies; 15+ messages in thread
From: Hayes Wang @ 2021-10-20  2:31 UTC (permalink / raw)
  To: Andy Shevchenko, Maxim Levitsky, Andy Shevchenko
  Cc: Hans de Goede, linux-realtek-soc, Oder Chiou, Pkshih, nic_swsd,
	Derek [方德義],
	Kailang, linux-iio, Lars-Peter Clausen, LKML, info

Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Tuesday, October 19, 2021 5:59 PM
[...]
> > > Realtek probably should make this ID marked somehow broken and not use
> > > in their products in case the answer to the first of the above question
> > > is "yes". (Of course in case the ID will be used for solely PCI enumerated
> > > product there will be no conflict, I just propose to be on the safest side,
> > > but remark should be made somewhere).
> 
> Any comments from Realtek, please?

Excuse me. I don't know this device, so I don't know who I could forward.
Maybe you could try our contract window from our web site.
https://www.realtek.com/en/cu-1-en/cu-1-taiwan-en

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-19  9:58           ` Andy Shevchenko
  2021-10-19 16:29             ` Maxim Levitsky
  2021-10-20  2:31             ` Hayes Wang
@ 2021-10-20 17:37             ` Jonathan Cameron
  2021-10-20 20:03               ` Andy Shevchenko
  2 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2021-10-20 17:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Maxim Levitsky, Andy Shevchenko, Hans de Goede,
	linux-realtek-soc, Oder Chiou, Ping-Ke Shih, nic_swsd,
	Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

On Tue, 19 Oct 2021 12:58:53 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Oct 18, 2021 at 11:42 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Mon, Oct 18, 2021 at 10:02 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:  
> > > On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:  
> 
> ...
> 
> > > Thank you for sharing. Seems they indeed used (deliberately or not) the wrong
> > > ID. So there are questions I have:
> > > - Is the firmware available in the wild?  
> >
> > Likely so. It looks Aya team only released a single windows driver which
> > works on all revisions of their device including the Founder Edition,
> > which was released more that a year ago.
> >
> > It is likely that all 3 revisions that they sold carry this ACPI ID.
> > (The founder edition, first batch of IndieGoGo orders which had a
> > redesigned shell,
> > and 2nd batch (which I have) which has a new wifi card, a bit better
> > controller,
> > among other changes).
> >
> >  
> > > - Do they plan to update firmware to fix this?
> > > - Can we make sure that guys got their mistake and will be more careful
> > >   in the future?  
> >
> > I CCed them, hoping that they would hear us. I can also raise this on their
> > discord when I find time to look there.  
> 
> I expect to have confirmation from them that they have got it and
> promise to fix the firmware (ACPI tables) for supported and future
> products.Can it be achieved? (Note, Hans already told what the HID
> should be used there)
> 
> > > Realtek probably should make this ID marked somehow broken and not use
> > > in their products in case the answer to the first of the above question
> > > is "yes". (Of course in case the ID will be used for solely PCI enumerated
> > > product there will be no conflict, I just propose to be on the safest side,
> > > but remark should be made somewhere).  
> 
> Any comments from Realtek, please?
> 
> > > > BTW, I also notice a rotation matrix embedded in DSTD, but the linux's
> > > > BMI160 driver doesn't recognize it.  
> > >
> > > This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> > > mount-matrix from ACPI") which needs to be amended to take care about
> > > more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> > > Hans, what do you think?  
> >
> > If you like to, I can probably volunteer to prepare a patch for this myself next
> > weekend, using this pointer as a reference.  

Hmm. This isn't part of the ACPI spec but seems to be a microsoft addition.
The webpage google feeds me says this is windows 10 mobile specific...
https://docs.microsoft.com/en-us/windows-hardware/drivers/sensors/sensors-acpi-entries
	
Whilst I haven't been paying particularly close attention, I haven't noticed
any attempt to add this to a future ACPI spec. If anyone happens to have
convenient Microsoft contacts to verify the status of this method that would
be a good step before we in any way imply it is standard.  Until then my inclination
is to keep this in the few drivers in which we know it is useful.

Jonathan

> 
> The best is to cooperate with Hans as he is much more involved in the
> topic of how these sensors are programmed and used in the Linux
> kernel. My job here is to fix ACPI HID and
> make every stakeholder be aware now and in the future.
> 
> ...
> 
> > I will prepare a patch with a better commit description this weekend.  
> 
> Thanks, but let's not be in such a hurry, I really want to hear from
> both vendors. I guess a couple of weeks would be a reasonable time to
> settle this down.
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: BMI160 accelerometer on AyaNeo tablet
  2021-10-20 17:37             ` Jonathan Cameron
@ 2021-10-20 20:03               ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2021-10-20 20:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Maxim Levitsky, Andy Shevchenko, Hans de Goede,
	linux-realtek-soc, Oder Chiou, Ping-Ke Shih, nic_swsd,
	Derek Fang, Hayes Wang, Kailang Yang, linux-iio,
	Lars-Peter Clausen, LKML, info

On Wed, Oct 20, 2021 at 8:33 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Tue, 19 Oct 2021 12:58:53 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Mon, Oct 18, 2021 at 11:42 PM Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > > On Mon, Oct 18, 2021 at 10:02 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Oct 18, 2021 at 09:02:40PM +0300, Maxim Levitsky wrote:

...

> > > > This is done by the commit 8a0672003421 ("iio: accel: bmc150: Get
> > > > mount-matrix from ACPI") which needs to be amended to take care about
> > > > more devices, somewhere in drivers/iio/industialio-acpi.c ? Jonathan,
> > > > Hans, what do you think?
> > >
> > > If you like to, I can probably volunteer to prepare a patch for this myself next
> > > weekend, using this pointer as a reference.
>
> Hmm. This isn't part of the ACPI spec but seems to be a microsoft addition.
> The webpage google feeds me says this is windows 10 mobile specific...
> https://docs.microsoft.com/en-us/windows-hardware/drivers/sensors/sensors-acpi-entries

So, it means it is at least a standard de facto on Windows machines.
We have to support it whether we want it or not. Same happened with
USB (wired) devices and many other things (SPCR, DBG2, CSRT, ...).

> Whilst I haven't been paying particularly close attention, I haven't noticed
> any attempt to add this to a future ACPI spec. If anyone happens to have
> convenient Microsoft contacts to verify the status of this method that would
> be a good step before we in any way imply it is standard.  Until then my inclination
> is to keep this in the few drivers in which we know it is useful.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-10-20 20:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CACAwPwb7edLzX-KO1XVNWuQ3w=U0BfA=_kwiGCjZOpKfZpc2pw@mail.gmail.com>
2021-10-16 16:27 ` BMI160 accelerometer on AyaNeo tablet Maxim Levitsky
2021-10-17 10:58   ` Jonathan Cameron
2021-10-18  7:40     ` Andy Shevchenko
2021-10-18 15:17       ` Jonathan Cameron
2021-10-18 15:22         ` Andy Shevchenko
2021-10-18 15:30   ` Andy Shevchenko
2021-10-18 18:02     ` Maxim Levitsky
2021-10-18 19:02       ` Andy Shevchenko
2021-10-18 20:42         ` Maxim Levitsky
2021-10-19  9:58           ` Andy Shevchenko
2021-10-19 16:29             ` Maxim Levitsky
2021-10-20  2:31             ` Hayes Wang
2021-10-20 17:37             ` Jonathan Cameron
2021-10-20 20:03               ` Andy Shevchenko
2021-10-19  8:10         ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).