| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 16 Mar 2019 13:00:39 +0000
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Himanshu Jha <himanshujha199640@...il.com>
CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jic23@...nel.org" <jic23@...nel.org>,
"knaack.h@....de" <knaack.h@....de>,
"lars@...afoo.de" <lars@...afoo.de>,
"pmeerw@...erw.net" <pmeerw@...erw.net>,
"dpfrey@...il.com" <dpfrey@...il.com>,
"colin.king@...onical.com" <colin.king@...onical.com>
Subject: Re: [PATCH v3 2/2] iio/chemical/bme680: Fix SPI read interface
On 16-03-19 11:24, Himanshu Jha wrote:
> On Wed, Mar 06, 2019 at 08:31:48AM +0100, Mike Looijmans wrote:
>> The SPI interface implementation was completely broken.
>>
>> When using the SPI interface, there are only 7 address bits, the upper bit
>> is controlled by a page select register. The core needs access to both
>> ranges, so implement register read/write for both regions. The regmap
>> paging functionality didn't agree with a register that needs to be read
>> and modified, so I implemented a custom paging algorithm.
>>
>> This fixes that the device wouldn't even probe in SPI mode.
>>
>> The SPI interface then isn't different from I2C, merged them into the core,
>> and the I2C/SPI named registers are no longer needed.
>>
>> Implemented register value caching for the registers to reduce the I2C/SPI
>> data transfers considerably.
>>
>> The calibration set reads as all zeroes until some undefined point in time,
>> and I couldn't determine what makes it valid. The datasheet mentions these
>> registers but does not provide any hints on when they become valid, and they
>> aren't even enumerated in the memory map. So check the calibration and
>> retry reading it from the device after each measurement until it provides
>> something valid.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>> ---
>
> I have been trying to test this patch in the past week and still it
> failed everytime.
>
> First I used ACPI to enumerate the device in QEMU setup:
>
> Added some printks for debugging:
>
> [ 14.510198] bme680_spi spi-BME0680:00: Jumping to core driver now ...
> [ 14.544528] bme680_spi spi-BME0680:00: Page setting done, on Page :0 now
> [ 14.554363] bme680_spi spi-BME0680:00: bme680_regmap_spi_write: on Page :0 now
> [ 14.556151] bme680_spi spi-BME0680:00: bme680_regmap_spi_read: on Page :0 now
> [ 14.567815] bme680_spi spi-BME0680:00: Wrong chip ID, got ff expected 61
>
Looks like the SPI communication isn't working. At this point I'd check
the wires using an osciloscope or analyzer or something.
> I also tried bypassing this by removing the following snippet and force
> registration to see what happens next:
>
> > + ret = regmap_write(regmap, BME680_REG_SOFT_RESET,
> > + BME680_CMD_SOFTRESET);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to reset chip\n");
> > + return ret;
> > + }
> > +
> > + ret = regmap_read(regmap, BME680_REG_CHIP_ID, &val);
> > + if (ret < 0) {
> > + dev_err(dev, "Error reading chip ID\n");
> > + return ret;
> > + }
> > +
> > + if (val != BME680_CHIP_ID_VAL) {
> > + dev_err(dev, "Wrong chip ID, got %x expected %x\n",
> > + val, BME680_CHIP_ID_VAL);
> > + return -ENODEV;
> > + }
> > +
>
> And it registered successfully, but all the bme680 attributes were
> giving wrong values like temp was constant to 0.0000007, resistance
> was resource busy due to insuffient target temperature error.
> Pretty eveything was messed up at this stage.
Makes perfect sense if it's unable to read the registers.
If you cannot read the chip ID, nothing will work, no point skipping that.
> Then I build and booted the kernel on BeagleBone Black Wireless with
> DT matching this time:
>
> debian@...glebone:~$ uname -a
> Linux beaglebone 4.19.5-ti-r5 #1xross SMP PREEMPT Sat Mar 16 12:11:50 IST 2019 armv7l GNU/Linux
> debian@...glebone:~$ dmesg | grep 'bme680'
> [ 30.269207] bme680_spi spi0.0: Wrong chip ID, got ff expected 61
> [ 361.867410] bme680_core: disagrees about version of symbol module_layout
Looks like a compilation problem with your kernel module?
> debian@...glebone:~$ lsmod | grep 'bme'
> bme680_spi 16384 0
> bme680_core 20480 1 bme680_spi
> debian@...glebone:~$ cat /sys/bus/spi/devices/spi0.0/
> modalias of_node/ power/ statistics/ subsystem/ uevent
> debian@...glebone:~$ cat /sys/bus/spi/devices/spi0.0/modalias
> spi:bme680
> debian@...glebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/
> compatible name reg spi-max-frequency
> debian@...glebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/compatible
> bme680
> debian@...glebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/name
> bme680
> debian@...glebone:~$ dtc -f -I fs /proc/device-tree | grep -A3 'bme680'
> bme680@0 {
> compatible = "bme680";
> reg = <0x0>;
> spi-max-frequency = <0x989680>;
> };
>
> Same error again!
>
> I really don't know where the problem is and how to rectify ?
>
> OTOH, I2C works like a charm as it used to work before:
That's reassuring, good to know i didn't kill that part.
>
> root@...glebone:/sys/bus/iio/devices/iio:device1# cat name in_temp_input \
> in_pressure_input in_humidityrelative_input in_resistance_input
>
> bme680
> 26860 --> w/o your patch it used to be 26.86000 degC
> 990.870000000
> 55.265000000
> 10091
>
>
> I'm still assuming that there is some problem on my side, as it works
> flawless for you. But it is really difficult for me to figure out
> exactly where the problem could be!
I would not go as far as calling it "flawless". The "resistance"
measurement usually fails a few times, and the calibration values aren't
available at the first read apparently. After reading the values
multiple times (hint: "grep . *" is really nice for showing file
contents in a sysfs directory) the chip appears to function okay. That's
what my last commit paragraph was explaining.
It's a big improvement over the previous version where the SPI interface
wouldn't work at all.
--
Mike Looijmans
Powered by blists - more mailing lists