Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

Andy,

I apologize for the long response, but there's several issues to address 
here.


First, I believe the "bmc150" in the subject line is in some way a 
misnomer.  You'd have to ask Jeremy for more details on what he intended 
it to refer to.  However, I believe the device in question is actually 
the bma250[1], which does not have a magnetometer component.  I'm 
unfortunately away from my notes, but I can check later if you need me 
to verify the exact chip.


Second, we're seeing a difference between what's in the data sheet and 
what's exposed in the wild via ACPI.  I own the laptop that started the 
process of building this patch and I did the original ACPI-tables 
investigation.


The device in question (BOSC0200) appears in the Lenovo Yoga 11e (and 
possibly other laptops - this happens to be the one I own). These 
laptops have a 360-degree hinge between the screen and the keyboard, 
letting them convert into tablets, if the user desires. The 11e 
implements this mode-switching by placing an accelerometer in each of 
the screen and keyboard, then doing math with the resulting vectors to 
figure out the angle between the two.  For whatever reason, Lenovo chose 
to expose these two (physically separate) accelerometers via a single 
ACPI device which presents two i2c devices at sequential addresses.


As part of my original investigation of the Yoga 11e, I wrote a 
proof-of-concept of pulling accelerometer data from the two devices 
exposed under the BOSC0200 ID and using that to calculate the position 
of the screen relative to the keyboard.  So based on my empirical 
experience, I can tell you the BOSC0200 device ID can expose two 
accelerometers at sequential addresses in the wild.


I don't understand why Lenovo has reused the BOSC0200 ACPI device ID for 
a device that is fundamentally different from the base device. The ID 
doesn't belong to them and we're (apparently) now stuck in this 
situation where this ACPI device ID could represent two different device 
layouts.


Finally - Andy, I apologize if I came across as challenging you in my 
initial mail.  I was trying to strike a balance between 
brevity/respecting your time and asking a question.  Evidently I struck 
the wrong balance and should have given you more background on why I was 
doubting what you saw.  This is my fault and you have my sincerest 
apologies for any offense I have caused.


Steve

[1] 
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA250E-DS004-06.pdf



On 01/30/2018 12:38 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 7:25 PM, Steve Presser  wrote:

Andy,

Where did the assertion the second device is a magnetometer come from? Just
the data sheet?

Yep. See chapter 8.2. Isn't enough proof? Or you believe in two
accelerometers with off-by-one conflicting address on a cheap laptop
with left unused two magnetometers on the same time?

And we have a driver for magnetometer separately.

So, it looks like we need to move ACPI ID to a new "kinda I2C mfd" IIO
driver under drivers/iio/imu/bmc150_i2c.c



Steve


On Tue, Jan 30, 2018, 12:05 PM Andy Shevchenko 
wrote:

On Tue, Jan 30, 2018 at 6:40 PM, Andy Shevchenko
 wrote:

On Tue, Jan 30, 2018 at 6:01 PM, Jonathan Cameron
 wrote:

On Mon, 29 Jan 2018 16:07:02 +0200
Andy Shevchenko  wrote:

But that would take much longer.  Feel free to propose it and a
patch
removing the ifdef fun if you like!

Where can I see the patch?

Doh. I clearly forgot to push out.  Should be able to push to
iio.git on kernel.org later.

Thanks, I can see it now.

This patch almost wrong. Not by functionality it brings, but by style.

Oy vey, the second device is *not* accelerometer, it is a magnetometer
[1].

[1]: https://www.mouser.com/ds/2/783/BST-BMC150-DS000-04-786477.pdf


I'll send soon a series of fixes to the driver (compile tested only)
to provide my view on the matters.

P.S. In the future (I have some kind of deja vu I have told this
already to someone), please, Cc one or more of Rafael, Mika and/or me
for ACPI matters.

--
With Best Regards,
Andy Shevchenko








smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser <st...@pressers.name> wrote:

Andy,

I apologize for the long response, but there's several issues to address
here.

NP, it it a good explanation why. That's what commit message missed apparently.
Probably my fault anyway - I don't recall discussing with Jeremy exactly 
what chip was inside this little Frankenstein.



First, I believe the "bmc150" in the subject line is in some way a misnomer.
You'd have to ask Jeremy for more details on what he intended it to refer
to.  However, I believe the device in question is actually the bma250[1],
which does not have a magnetometer component.  I'm unfortunately away from
my notes, but I can check later if you need me to verify the exact chip.

Please do, I would really be on the safe side here.
Will do.  My digital notes indicate I worked from what was exposed back 
to what chip matched.  If you can give me through Friday evening, I'll 
crack it and do a visual verification.  (Alas, I'm traveling and won't 
be back to it until then).



Second, we're seeing a difference between what's in the data sheet and
what's exposed in the wild via ACPI.  I own the laptop that started the
process of building this patch and I did the original ACPI-tables
investigation.

The device in question (BOSC0200) appears in the Lenovo Yoga 11e (and
possibly other laptops - this happens to be the one I own). These laptops
have a 360-degree hinge between the screen and the keyboard, letting them
convert into tablets, if the user desires. The 11e implements this
mode-switching by placing an accelerometer in each of the screen and
keyboard, then doing math with the resulting vectors to figure out the angle
between the two.

This makes a lot of sense.


  For whatever reason, Lenovo chose to expose these two
(physically separate) accelerometers via a single ACPI device which presents
two i2c devices at sequential addresses.



As part of my original investigation of the Yoga 11e, I wrote a
proof-of-concept of pulling accelerometer data from the two devices exposed
under the BOSC0200 ID and using that to calculate the position of the screen
relative to the keyboard.  So based on my empirical experience, I can tell
you the BOSC0200 device ID can expose two accelerometers at sequential
addresses in the wild.

I don't understand why Lenovo has reused the BOSC0200 ACPI device ID for a
device that is fundamentally different from the base device. The ID doesn't
belong to them and we're (apparently) now stuck in this situation where this
ACPI device ID could represent two different device layouts.

Bad, bad Lenovo. (DMI strings might help here)

What particular DMI strings would be helpful?  All of them?



Finally - Andy, I apologize if I came across as challenging you in my
initial mail.  I was trying to strike a balance between brevity/respecting
your time and asking a question.  Evidently I struck the wrong balance and
should have given you more background on why I was doubting what you saw.
This is my fault and you have my sincerest apologies for any offense I have
caused.

No need, the root cause is lack of description in the commit message.

Nevertheless, the approach chosen I don't like. It looks like an ugly hack.

What we can do here is:
- do not contaminate core part with I2C/SPI/etc
- do not create another driver via board_info, we already in *the same* driver,
so, the better approach here AFAICS is to add DMI quirk into i2c-core-acpi




Steve

[1]
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA250E-DS004-06.pdf





smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

Andy,


The information is in kernel bug 198671.


Steve


On 01/30/2018 04:20 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 10:12 PM, Andy Shevchenko
<andy.shevche...@gmail.com> wrote:

On Tue, Jan 30, 2018 at 9:27 PM, Steven Presser <st...@pressers.name> wrote:

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser <st...@pressers.name>
wrote:

Let's do this way. Create a bug on kernel bugzilla, attach output of

% acpidump -o tables.dat # tables.dat file
% grep -H 15 /sys/bus/acpi/devices/*/status
% dmidecode

and share the number here. I will take it.

Here [1] is the branch with another approach. Can you test it and tell
if it fixes the issue?

[1]: https://bitbucket.org/andy-shev/linux/branch/topic/iio-acpi
It's based on linux-next + compiletest branch of IIO subsystem.






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

All,

I had a chance to sit back down with the machine.  I didn't take it all 
the way apart - there are pieces that I'm afraid of breaking without 
directions on how to properly disassemble them.


However, I did recover an exact chip ID - the chips in use are BMA255s 
[1].  Rather than take the machine apart (and because the chips are 
2mmx2mm), I queried the chip over SMBus.  On page 50 of the below 
document, you can see that register 0x00 is a read-only chip ID.  This 
chipID is unique per Bosch product.  So, using SMBus, I asked the chip 
for it's chip ID (0xFA, in this case) and then searched likely products 
until I found the matching chipID.


Does this suffice to settle which chips are in use?  If not, I can 
finish taking the machine apart, I'd just prefer to avoid the risk of 
breaking something.


As soon as I finish screwing everything back together, I'll grab the 
other software IDs asked for and build the branch referenced elsewhere.


Steve


[1] 
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA255-DS004-05_published.pdf



On 01/30/2018 03:12 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 9:27 PM, Steven Presser <st...@pressers.name> wrote:

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser <st...@pressers.name>
wrote:

First, I believe the "bmc150" in the subject line is in some way a
misnomer.
You'd have to ask Jeremy for more details on what he intended it to refer
to.  However, I believe the device in question is actually the bma250[1],
which does not have a magnetometer component.  I'm unfortunately away
from
my notes, but I can check later if you need me to verify the exact chip.

Please do, I would really be on the safe side here.

Will do.  My digital notes indicate I worked from what was exposed back to
what chip matched.  If you can give me through Friday evening, I'll crack it
and do a visual verification.  (Alas, I'm traveling and won't be back to it
until then).

We are in the merge window anyway, so, no hurry.

I'm looking right now in the clean solution. Looks promising.


Bad, bad Lenovo. (DMI strings might help here)

What particular DMI strings would be helpful?  All of them?

Let's do this way. Create a bug on kernel bugzilla, attach output of

% acpidump -o tables.dat # tables.dat file
% grep -H 15 /sys/bus/acpi/devices/*/status
% dmidecode

and share the number here. I will take it.






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

All,

I had a chance to sit back down with the machine.  I didn't take it all 
the way apart - there are pieces that I'm afraid of breaking without 
directions on how to properly disassemble them.


However, I did recover an exact chip ID - the chips in use are BMA255s 
[1].  Rather than take the machine apart (and because the chips are 
2mmx2mm), I queried the chip over SMBus.  On page 50 of the below 
document, you can see that register 0x00 is a read-only chip ID.  This 
chipID is unique per Bosch product.  So, using SMBus, I asked the chip 
for it's chip ID (0xFA, in this case) and then searched likely products 
until I found the matching chipID.


Does this suffice to settle which chips are in use?  If not, I can 
finish taking the machine apart, I'd just prefer to avoid the risk of 
breaking something.


As soon as I finish screwing everything back together, I'll grab the 
other software IDs asked for and build the branch referenced elsewhere.


Steve


[1] 
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA255-DS004-05_published.pdf



On 01/30/2018 03:12 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 9:27 PM, Steven Presser  wrote:

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser 
wrote:

First, I believe the "bmc150" in the subject line is in some way a
misnomer.
You'd have to ask Jeremy for more details on what he intended it to refer
to.  However, I believe the device in question is actually the bma250[1],
which does not have a magnetometer component.  I'm unfortunately away
from
my notes, but I can check later if you need me to verify the exact chip.

Please do, I would really be on the safe side here.

Will do.  My digital notes indicate I worked from what was exposed back to
what chip matched.  If you can give me through Friday evening, I'll crack it
and do a visual verification.  (Alas, I'm traveling and won't be back to it
until then).

We are in the merge window anyway, so, no hurry.

I'm looking right now in the clean solution. Looks promising.


Bad, bad Lenovo. (DMI strings might help here)

What particular DMI strings would be helpful?  All of them?

Let's do this way. Create a bug on kernel bugzilla, attach output of

% acpidump -o tables.dat # tables.dat file
% grep -H 15 /sys/bus/acpi/devices/*/status
% dmidecode

and share the number here. I will take it.






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

Andy,


The information is in kernel bug 198671.


Steve


On 01/30/2018 04:20 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 10:12 PM, Andy Shevchenko
 wrote:

On Tue, Jan 30, 2018 at 9:27 PM, Steven Presser  wrote:

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser 
wrote:

Let's do this way. Create a bug on kernel bugzilla, attach output of

% acpidump -o tables.dat # tables.dat file
% grep -H 15 /sys/bus/acpi/devices/*/status
% dmidecode

and share the number here. I will take it.

Here [1] is the branch with another approach. Can you test it and tell
if it fixes the issue?

[1]: https://bitbucket.org/andy-shev/linux/branch/topic/iio-acpi
It's based on linux-next + compiletest branch of IIO subsystem.






smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

Andy,

I apologize for the long response, but there's several issues to address 
here.


First, I believe the "bmc150" in the subject line is in some way a 
misnomer.  You'd have to ask Jeremy for more details on what he intended 
it to refer to.  However, I believe the device in question is actually 
the bma250[1], which does not have a magnetometer component.  I'm 
unfortunately away from my notes, but I can check later if you need me 
to verify the exact chip.


Second, we're seeing a difference between what's in the data sheet and 
what's exposed in the wild via ACPI.  I own the laptop that started the 
process of building this patch and I did the original ACPI-tables 
investigation.


The device in question (BOSC0200) appears in the Lenovo Yoga 11e (and 
possibly other laptops - this happens to be the one I own). These 
laptops have a 360-degree hinge between the screen and the keyboard, 
letting them convert into tablets, if the user desires. The 11e 
implements this mode-switching by placing an accelerometer in each of 
the screen and keyboard, then doing math with the resulting vectors to 
figure out the angle between the two.  For whatever reason, Lenovo chose 
to expose these two (physically separate) accelerometers via a single 
ACPI device which presents two i2c devices at sequential addresses.


As part of my original investigation of the Yoga 11e, I wrote a 
proof-of-concept of pulling accelerometer data from the two devices 
exposed under the BOSC0200 ID and using that to calculate the position 
of the screen relative to the keyboard.  So based on my empirical 
experience, I can tell you the BOSC0200 device ID can expose two 
accelerometers at sequential addresses in the wild.


I don't understand why Lenovo has reused the BOSC0200 ACPI device ID for 
a device that is fundamentally different from the base device. The ID 
doesn't belong to them and we're (apparently) now stuck in this 
situation where this ACPI device ID could represent two different device 
layouts.


Finally - Andy, I apologize if I came across as challenging you in my 
initial mail.  I was trying to strike a balance between 
brevity/respecting your time and asking a question.  Evidently I struck 
the wrong balance and should have given you more background on why I was 
doubting what you saw.  This is my fault and you have my sincerest 
apologies for any offense I have caused.


Steve

[1] 
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA250E-DS004-06.pdf



On 01/30/2018 12:38 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 7:25 PM, Steve Presser  wrote:

Andy,

Where did the assertion the second device is a magnetometer come from? Just
the data sheet?

Yep. See chapter 8.2. Isn't enough proof? Or you believe in two
accelerometers with off-by-one conflicting address on a cheap laptop
with left unused two magnetometers on the same time?

And we have a driver for magnetometer separately.

So, it looks like we need to move ACPI ID to a new "kinda I2C mfd" IIO
driver under drivers/iio/imu/bmc150_i2c.c



Steve


On Tue, Jan 30, 2018, 12:05 PM Andy Shevchenko 
wrote:

On Tue, Jan 30, 2018 at 6:40 PM, Andy Shevchenko
 wrote:

On Tue, Jan 30, 2018 at 6:01 PM, Jonathan Cameron
 wrote:

On Mon, 29 Jan 2018 16:07:02 +0200
Andy Shevchenko  wrote:

But that would take much longer.  Feel free to propose it and a
patch
removing the ifdef fun if you like!

Where can I see the patch?

Doh. I clearly forgot to push out.  Should be able to push to
iio.git on kernel.org later.

Thanks, I can see it now.

This patch almost wrong. Not by functionality it brings, but by style.

Oy vey, the second device is *not* accelerometer, it is a magnetometer
[1].

[1]: https://www.mouser.com/ds/2/783/BST-BMC150-DS000-04-786477.pdf


I'll send soon a series of fixes to the driver (compile tested only)
to provide my view on the matters.

P.S. In the future (I have some kind of deja vu I have told this
already to someone), please, Cc one or more of Rafael, Mika and/or me
for ACPI matters.

--
With Best Regards,
Andy Shevchenko








smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v2] iio: accel: bmc150: Check for a second ACPI device for BOSC0200

[Steven Presser]

On 01/30/2018 02:05 PM, Andy Shevchenko wrote:

On Tue, Jan 30, 2018 at 8:34 PM, Steven Presser  wrote:

Andy,

I apologize for the long response, but there's several issues to address
here.

NP, it it a good explanation why. That's what commit message missed apparently.
Probably my fault anyway - I don't recall discussing with Jeremy exactly 
what chip was inside this little Frankenstein.



First, I believe the "bmc150" in the subject line is in some way a misnomer.
You'd have to ask Jeremy for more details on what he intended it to refer
to.  However, I believe the device in question is actually the bma250[1],
which does not have a magnetometer component.  I'm unfortunately away from
my notes, but I can check later if you need me to verify the exact chip.

Please do, I would really be on the safe side here.
Will do.  My digital notes indicate I worked from what was exposed back 
to what chip matched.  If you can give me through Friday evening, I'll 
crack it and do a visual verification.  (Alas, I'm traveling and won't 
be back to it until then).



Second, we're seeing a difference between what's in the data sheet and
what's exposed in the wild via ACPI.  I own the laptop that started the
process of building this patch and I did the original ACPI-tables
investigation.

The device in question (BOSC0200) appears in the Lenovo Yoga 11e (and
possibly other laptops - this happens to be the one I own). These laptops
have a 360-degree hinge between the screen and the keyboard, letting them
convert into tablets, if the user desires. The 11e implements this
mode-switching by placing an accelerometer in each of the screen and
keyboard, then doing math with the resulting vectors to figure out the angle
between the two.

This makes a lot of sense.


  For whatever reason, Lenovo chose to expose these two
(physically separate) accelerometers via a single ACPI device which presents
two i2c devices at sequential addresses.



As part of my original investigation of the Yoga 11e, I wrote a
proof-of-concept of pulling accelerometer data from the two devices exposed
under the BOSC0200 ID and using that to calculate the position of the screen
relative to the keyboard.  So based on my empirical experience, I can tell
you the BOSC0200 device ID can expose two accelerometers at sequential
addresses in the wild.

I don't understand why Lenovo has reused the BOSC0200 ACPI device ID for a
device that is fundamentally different from the base device. The ID doesn't
belong to them and we're (apparently) now stuck in this situation where this
ACPI device ID could represent two different device layouts.

Bad, bad Lenovo. (DMI strings might help here)

What particular DMI strings would be helpful?  All of them?



Finally - Andy, I apologize if I came across as challenging you in my
initial mail.  I was trying to strike a balance between brevity/respecting
your time and asking a question.  Evidently I struck the wrong balance and
should have given you more background on why I was doubting what you saw.
This is my fault and you have my sincerest apologies for any offense I have
caused.

No need, the root cause is lack of description in the commit message.

Nevertheless, the approach chosen I don't like. It looks like an ugly hack.

What we can do here is:
- do not contaminate core part with I2C/SPI/etc
- do not create another driver via board_info, we already in *the same* driver,
so, the better approach here AFAICS is to add DMI quirk into i2c-core-acpi




Steve

[1]
https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMA250E-DS004-06.pdf





smime.p7s
Description: S/MIME Cryptographic Signature