BMA150 - Githubissues

PabloPL commented 5 years ago

Patches created by @xc-racer99 (spitted one of patches into two separate, first making changes in driver and second one adding documentation).

PabloPL commented 5 years ago

@xc-racer99 How did You tested it? Using evtest i'm always getting following values

Input driver version is 1.0.1
Input device ID: bus 0x18 vendor 0x0 product 0x0 version 0x0
Input device name: "bma150"
Supported events:
  Event type 0 (EV_SYN)
  Event type 3 (EV_ABS)
    Event code 0 (ABS_X)
      Value      0
      Min     -511
      Max      511
    Event code 1 (ABS_Y)
      Value      0
      Min     -511
      Max      511
    Event code 2 (ABS_Z)
      Value      0
      Min     -511
      Max      511
Properties:
Testing ... (interrupt to exit)

xc-racer99 commented 5 years ago

Sorry, I don't really remember. I might have added some debugging printks or else I was just using cat on the input device. I didn't use evtest.

PabloPL commented 5 years ago

Ok, will try to look at this later (also tried just cat on device and it didn't reported any other values). I need to go over all branches, rebase them aganist master, build, test, sign patches, split (there are few of them), checkpatch and submit upstream.

xc-racer99 commented 5 years ago

Can confirm that the patches aren't working (probe but no input). I may not have tested them to begin with.

xc-racer99 commented 5 years ago

Turns out the IRQ isn't connected on aries; so use the polled method. Branch updated.

PabloPL commented 5 years ago

Tested - now it's working. I've rebased this branch against for-upstream/aries-patches-v2. Now will prepare and submit to mainline (node and defconfig changes will need to be send in separate PR).

Edit 1: Patches send to mainline

xc-racer99 commented 5 years ago

Patches received feedback at mainline; changes made include:

Correcting devm input unregistering
Added patch removing pdata
DT properties for bma150_cfg added and documented
Documentation IRQ type corrected

Can confirm that even after changing IRQ type that there is never an input received so we definitely do need the polled version.

xc-racer99 commented 5 years ago

Still TODO: Figure out why unloading/reloading the module causes a crash.

PabloPL commented 5 years ago

Stacktrace?

xc-racer99 commented 5 years ago

[  211.228704] input: bma150 as /devices/platform/i2c-gpio-2/i2c-5/5-0038/input/input3
[  211.243869] Unable to handle kernel NULL pointer dereference at virtual address 00000018
[  211.270328] pgd = (ptrval)
[  211.270428] [00000018] *pgd=55dac831, *pte=00000000, *ppte=00000000
[  211.275558] Internal error: Oops: 17 [#1] PREEMPT ARM
[  211.279039] Modules linked in: bma150 brcmfmac hci_uart sha256_generic btbcm cfg80211 s5p_jpeg v4l2_mem2mem bluetooth v4l2_common videobuf2_dma_contig brcmutil videobuf2_memops videobuf2_v4l2 videodev videobuf2_common phy_exynos_usb2 ecdh_generic atmel_mxt_ts input_polldev rtc_max8998 [last unloaded: bma150]
[  211.305623] CPU: 0 PID: 2870 Comm: accelerometer Not tainted 5.0.0-rc3-14476-g8364143a233e-dirty #46
[  211.312869] Hardware name: Samsung S5PC110/S5PV210-based board
[  211.317287] PC is at input_event+0x8/0x60
[  211.319918] LR is at bma150_report_xyz+0x9c/0xe0 [bma150]
[  211.324019] pc : [<80450f70>]    lr : [<7f0a614c>]    psr: 800d0013
[  211.328978] sp : a4c1fd78  ip : 00000081  fp : 00020000
[  211.332847] r10: 00000000  r9 : a5e2944c  r8 : a7455000
[  211.336748] r7 : 00000016  r6 : 00000101  r5 : a7617940  r4 : 80909048
[  211.341985] r3 : fffffff2  r2 : 00000000  r1 : 00000003  r0 : 00000000
[  211.347182] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  211.353010] Control: 10c5387d  Table: 54e34019  DAC: 00000051
[  211.357406] Process accelerometer (pid: 2870, stack limit = 0x(ptrval))
[  211.362702] Stack: (0xa4c1fd78 to 0xa4c20000)
[  211.365671] fd60:                                                       fffffff3 fc813f6c
[  211.372627] fd80: 40410581 d7530ce3 a5e2817c a7617f00 a5e29404 a5e2817c 00000000 7f008324
[  211.379482] fda0: a5e28000 8044f59c a5fdd9d0 a5e2945c a46a4a00 a5e29668 a7455000 80454f10
[  211.386320] fdc0: 80909048 a5e29668 a5fdd9d0 a46a4a00 806316d0 00000000 a46a4a00 801df5f0
[  211.393168] fde0: 00000000 d7530ce3 a4c1fec0 a46a4a00 00000000 a5fdd9d0 a46a4a08 801df53c
[  211.400009] fe00: 00000000 801d74bc a4c1fec0 00000000 a4c1ff70 00000000 a7038da8 00000000
[  211.406857] fe20: a46a4a00 801e91fc a411bbe0 801f2e88 00000004 00000000 80909048 00000041
[  211.413703] fe40: 00000000 00020000 00000000 dead4ead a6a88da0 00000000 ffffe000 806fcae8
[  211.420572] fe60: a4c1fec8 00000000 80909048 00000002 a5fdd9d0 a7660110 a411bab0 00000001
[  211.427393] fe80: dead4ead ffffffff ffffffff a4c1fe8c a4c1fe8c d7530ce3 20000013 80909048
[  211.440245] fea0: 80909048 a4c1ff70 00000001 fffff000 a4c1e000 00000005 00026038 801eabd8
[  211.457939] fec0: a7660110 a411bab0 b9394901 00000006 a696201b 76fb3000 00000000 a7039720
[  211.475658] fee0: a5fdd9d0 00000101 00000002 00000096 00000000 00000000 00000000 a4c1ff00
[  211.493380] ff00: a6b310f4 805cb174 a6b310f4 00000010 00000fe0 00000010 a4c1e000 d7530ce3
[  211.511262] ff20: 00000003 a5f41400 a5f41424 00000000 a6962000 00000000 00000003 00000002
[  211.529162] ff40: ffffff9c 000a0000 80909048 d7530ce3 a6962000 00000003 80909048 ffffff9c
[  211.546993] ff60: a6962000 801d890c 00000000 00000000 00020000 a7590000 00000004 00000100
[  211.564801] ff80: 00000001 d7530ce3 000288b8 00026320 000288b8 00000005 80101204 a4c1e000
[  211.582578] ffa0: 00000005 80101000 000288b8 00026320 000288b8 000a0000 00000000 00000000
[  211.600388] ffc0: 000288b8 00026320 000288b8 00000005 7eef3bac 000264e8 00028ad8 00026038
[  211.618268] ffe0: 00000005 7eef3300 76f76e91 76f78546 800d0030 000288b8 00000000 00000000
[  211.636235] [<80450f70>] (input_event) from [<a5e2817c>] (0xa5e2817c)
[  211.646309] Code: e1a08148 eaffffa8 e351001f 812fff1e (e590c018) 
[  211.677794] ---[ end trace 1c691ee85f2ff243 ]---
[  211.722930] random: crng init done
[  211.729964] random: 7 urandom warning(s) missed due to ratelimiting

xc-racer99 commented 5 years ago

Issue solved with https://github.com/PabloPL/linux/commit/7677ba0522fb0ab55f4d1ad159cbaaeaaf7bb62f which means you can add -Fix race condition in registering of input device To what has changed.

Ready to be resubmitted to mainline either by you or me.

PabloPL commented 5 years ago

@xc-racer99 I can send v2 to mainline, but need one thing. You described what has changed since v2, but could You split this information by patch? It looks like changelog should be added in two places:

in cover letter for whole patchset (if cover letter is present). It should contain information about changes in all patches in patchset.
in separate patches (information what changed since prev version).LIke it was done for example https://patchwork.kernel.org/patch/10492281/ . If nothing changed in this patch, there won't be such information there.

So either, if You could post this information here and i'll add (amend) this information to patches or do it by yourself. Thanks!

xc-racer99 commented 5 years ago

Alright, done and force-pushed.

For the cover letter changes from v1:

Correct devm input unregistering
Correct IRQ type in DT documentation
Add DT properties for bma150_cfg and document
Add patch removing pdata
Fix race condition in input device registering

Thanks!

PabloPL commented 5 years ago

@xc-racer99 Thanks, i'll add my Signed-off-by to all patches (since i'm "touching" those patches - sending is also "touching" like someone wrote in review of one of my old patches) and submit to upstream.

Edit 1: Made fetch from upstream (merged to master) and also rebased aries-patches-v2 (and also this branch).

Edit 2: Need first to fix one issue. In dt-bindings-input-Add-binding-for-bma150-sensor.patch

DT binding docs and includes should be a separate patch.

I think it doesn't make any sense to drop that stacktrace from last patch which fixes race condition).

xc-racer99 commented 5 years ago

Edit 2: Need first to fix one issue. In dt-bindings-input-Add-binding-for-bma150-sensor.patch

DT binding docs and includes should be a separate patch.

Yes, I know, but the only change to the includes was to move from the main include area to the dt-bindings include so we don't have two headers with the same info. Should it instead be broken up into two commits, the second merely changing include/linux/bma150.h to include the dt-bindings file?

I think it doesn't make any sense to drop that stacktrace from last patch which fixes race condition).

I agree, I've noticed several other patches that include stacktraces that go beyond the 75char commit limit.

PabloPL commented 5 years ago

I'm not sure if it makes sense to split them...(and didn't had similar situation yet). Will send all patches in current state and we'll see after review.

xc-racer99 commented 5 years ago

Interesting to note that according to https://media.digikey.com/pdf/Data%20Sheets/Bosch/BMA150.pdf there's a integrated temperature sensor as well. Curious if this works on our device or not...

May end up adding support for BMA150 to the IIO BMA180 driver.

xc-racer99 commented 5 years ago

Alright, looks like we don't have the BMA150, but rather the BMA020 or BMA023 (this means no temperature sensor).

I have added support for BMA150 and BMA023 to the BMA180 IIO driver and uploaded the changes to the branch at https://github.com/PabloPL/linux/commits/for-upstream/bma150-iio

xc-racer99 commented 3 years ago

BMA150 IIO driver is merged upstream as DTS changes.

PabloPL / linux

BMA150 #20