| 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: Sun, 17 Mar 2019 15:18:06 +0530
From: Himanshu Jha <himanshujha199640@...il.com>
To: Mike Looijmans <mike.looijmans@...ic.nl>
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 Sat, Mar 16, 2019 at 01:00:39PM +0000, Mike Looijmans wrote:
> 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.
OK. Will give it a try at university lab.
> > 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.
Agreed! I was just trying to triage the issue.
> > 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?
No. I doesn't appear now.
I also build a more latest kernel but same error:
[ 519.741364] bme680_spi spi0.0: Wrong chip ID, got ff expected 61
> > 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.
Yes!
> > 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.
I know about resistance measurement failing intially "sometimes"
and it depends on environment factors as well. If there is sufficient
heat, then less chances of failure on initial reading.
> 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.
I use watch command instead.
> It's a big improvement over the previous version where the SPI interface
> wouldn't work at all.
Agreed, Sir _/\_
> --
> Mike Looijmans
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
Powered by blists - more mailing lists