Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 16 Mar 2019 15:54:25 +0530
From:   Himanshu Jha <himanshujha199640@gmail.com>
To:     Mike Looijmans <mike.looijmans@topic.nl>
Cc:     linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
        jic23@kernel.org, knaack.h@gmx.de, lars@metafoo.de,
        pmeerw@pmeerw.net, dpfrey@gmail.com, colin.king@canonical.com
Subject: Re: [PATCH v3 2/2] iio/chemical/bme680: Fix SPI read interface
Message-ID: <20190316102425.GA26481@himanshu-Vostro-3559>
References: <1550238475-25698-1-git-send-email-mike.looijmans@topic.nl>
 <1551857508-4254-2-git-send-email-mike.looijmans@topic.nl>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1551857508-4254-2-git-send-email-mike.looijmans@topic.nl>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@topic.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

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.

Then I build and booted the kernel on BeagleBone Black Wireless with
DT matching this time:

debian@beaglebone:~$ uname -a
Linux beaglebone 4.19.5-ti-r5 #1xross SMP PREEMPT Sat Mar 16 12:11:50 IST 2019 armv7l GNU/Linux
debian@beaglebone:~$ 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
debian@beaglebone:~$ lsmod | grep 'bme'
bme680_spi             16384  0
bme680_core            20480  1 bme680_spi
debian@beaglebone:~$ cat /sys/bus/spi/devices/spi0.0/
modalias    of_node/    power/      statistics/ subsystem/  uevent
debian@beaglebone:~$ cat /sys/bus/spi/devices/spi0.0/modalias
spi:bme680
debian@beaglebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/
compatible         name               reg                spi-max-frequency
debian@beaglebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/compatible
bme680
debian@beaglebone:~$ cat /sys/bus/spi/devices/spi0.0/of_node/name
bme680
debian@beaglebone:~$ 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:

root@beaglebone:/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!


-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology