Bug 214453

Summary: skl_int3472_unregister_clock: kernel NULL pointer dereference (HP Elite x2 1013 G3)
Product: Drivers Reporter: NotTheEvilOne (kernel-NTEO)
Component: Platform_x86Assignee: platform_x86_64 (platform_x86_64)
Status: RESOLVED CODE_FIX    
Severity: low CC: djrscally
Priority: P1    
Hardware: Intel   
OS: Linux   
Kernel Version: 5.14.5 Subsystem:
Regression: No Bisected commit-id:
Attachments: dmesg oops log
proposed patch
DSDT for HP Elite x2 1013 G3/8414, BIOS Q87 Ver. 01.17.00 08/04/2021
Camera data dump for HP Elite x2 1013 G3 01.17.00

Description NotTheEvilOne 2021-09-17 12:18:27 UTC
Created attachment 298863 [details]
dmesg oops log

On a HP Elite x2 1013 G3 a kernel oops is caused by int3472:

[    8.820565] int3472-discrete INT3472:02: No sensor module config
[    8.820569] int3472-discrete INT3472:02: error -EINVAL: Failed to map regulator to sensor
[    8.820574] BUG: kernel NULL pointer dereference, address: 0000000000000000
[...]
[    8.820654] Call Trace:
[    8.820657]  clkdev_drop+0x1d/0x60
[    8.820665]  skl_int3472_unregister_clock+0x15/0x30 [intel_skl_int3472 a61329898df9463f8661b1eee1ed1e20b1626f62]
Comment 1 Dan Scally 2021-10-08 22:01:31 UTC
Created attachment 299141 [details]
proposed patch
Comment 2 Dan Scally 2021-10-08 22:04:06 UTC
Thanks for the bug report, and sorry for the delayed response. The attached patch should resolve this, but a better approach might be to make clkdev_drop() null aware. I'll raise that on the list, but if you can confirm this works that would be great.

As an aside, if you can share the DSDT tables for this device that would be great - I'm assuming the cameras don't work on linux currently, we might be able to add support for it.
Comment 3 NotTheEvilOne 2021-10-10 09:25:06 UTC
Hi Dan,

Thanks for your reply. I'm sorry but it will take some time before I can report back on the custom compiled kernel.

Please find attached the binary DSDT file.
Comment 4 NotTheEvilOne 2021-10-10 09:26:46 UTC
Created attachment 299157 [details]
DSDT for HP Elite x2 1013 G3/8414, BIOS Q87 Ver. 01.17.00 08/04/2021
Comment 5 Dan Scally 2021-10-11 22:46:23 UTC
Don't worry about it - I'm confident that it'll be the right fix, so I posted the patch and the platform driver maintainer has picked it up.

Thanks for the DSDT. Unfortunately, some DSDT tables, including this one, are done in an annoying way wherein lots of possible configuration for a system is included and the correct one determined by reading values from OpRegions at runtime. There's no way to deduce the right one from the DSDT tables alone, we have to read them at runtime. If you're interested, there's a module you can build and load here: https://github.com/kitakar5525/surface-ipu3-cameras/tree/master/misc/dump_intel_ipu_data

Or alternatively, if it's the same as a HP Elite x2 1012 which we already have results for, you could just run ls /sys/bus/i2c/devices and verify there's an i2c-INT33BE:00 entry there. If there is we could probably get the camera working with fairly minimal effort, as we already have a driver for that sensor, so we just need to do some minor config.
Comment 6 NotTheEvilOne 2021-10-12 05:56:32 UTC
As you expected the check added fixes the intermediate issue. We are now back at the (I think) pre 5.14 state:

[...]
[   10.009143] int3472-discrete INT3472:02: No sensor module config
[   10.011990] int3472-discrete INT3472:02: error -EINVAL: Failed to map regulator to sensor
[   10.012414] int3472-discrete: probe of INT3472:02 failed with error -22
[   10.024850] mc: Linux media interface: v0.10
[   10.034182] input: Intel HID events as /devices/platform/INT33D5:00/input/input17
[   10.034860] videodev: Linux video capture interface: v2.00
[   10.083481] input: Intel Virtual Buttons as /devices/pci0000:00/0000:00:1f.0/PNP0C09:00/INT33D6:00/input/input18
[   10.084194] input: Intel Virtual Switches as /devices/pci0000:00/0000:00:1f.0/PNP0C09:00/INT33D6:00/input/input19
[   10.197840] intel_ish_ipc 0000:00:13.0: enabling device (0000 -> 0002)
[   10.199868] mousedev: PS/2 mouse device common for all mice
[   10.209294] ipu3_imgu: module is from the staging directory, the quality is unknown, you have been warned.
[   10.209647] proc_thermal 0000:00:04.0: enabling device (0000 -> 0002)
[   10.210655] ipu3-imgu 0000:00:05.0: enabling device (0000 -> 0002)
[   10.210808] ipu3-imgu 0000:00:05.0: device 0x1919 (rev: 0x1)
[   10.210823] ipu3-imgu 0000:00:05.0: physical base address 0x0000001ffb000000, 4194304 bytes
[   10.211187] intel_rapl_common: Found RAPL domain package
[   10.212067] ipu3-cio2 0000:00:14.3: enabling device (0000 -> 0002)
[   10.212262] ipu3-cio2 0000:00:14.3: device 0x9d32 (rev: 0x1)
[   10.259315] ipu3-imgu 0000:00:05.0: loaded firmware version irci_irci_ecr-master_20161208_0213_20170112_1500, 17 binaries, 1212984 bytes
[   10.308711] Bluetooth: Core ver 2.22
[   10.308726] NET: Registered PF_BLUETOOTH protocol family
[   10.308727] Bluetooth: HCI device and connection manager initialized
[   10.308730] Bluetooth: HCI socket layer initialized
[   10.308732] Bluetooth: L2CAP socket layer initialized
[   10.308734] Bluetooth: SCO socket layer initialized
[   10.310406] intel-lpss 0000:00:15.0: enabling device (0000 -> 0002)
[   10.310749] idma64 idma64.0: Found Intel integrated DMA 64-bit
[   10.314182] mei_me 0000:00:16.0: enabling device (0004 -> 0006)
[   10.318186] usbcore: registered new interface driver btusb
[   10.319563] Bluetooth: hci0: Bootloader revision 0.0 build 26 week 38 2015
[   10.320574] Bluetooth: hci0: Device revision is 16
[   10.320577] Bluetooth: hci0: Secure boot is enabled
[   10.320578] Bluetooth: hci0: OTP lock is enabled
[   10.320579] Bluetooth: hci0: API lock is enabled
[   10.320580] Bluetooth: hci0: Debug lock is disabled
[   10.320581] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
[   10.321974] Bluetooth: hci0: Found device firmware: intel/ibt-12-16.sfi
[   10.323279] intel-lpss 0000:00:15.2: enabling device (0000 -> 0002)
[   10.323524] idma64 idma64.1: Found Intel integrated DMA 64-bit
[   10.332988] int3472-tps68470 i2c-INT3472:05: TPS68470 REVID: 0x21
[   10.338551] hi556 i2c-INT3537:00: can't get clock frequency
[   10.339167] ov8856 i2c-OVTI8856:00: failed to get HW configuration: -22
[   10.343680] ov8856: probe of i2c-OVTI8856:00 failed with error -22
[   10.343681] hi556 i2c-INT3537:00: failed to check HW configuration: -22
[   10.347730] hi556: probe of i2c-INT3537:00 failed with error -22
[   10.348566] intel-lpss 0000:00:15.3: enabling device (0000 -> 0002)
[...]

I've attached the dump output of the kernel module. The i2c entry in question does not exist.
Comment 7 NotTheEvilOne 2021-10-12 05:57:58 UTC
Created attachment 299187 [details]
Camera data dump for HP Elite x2 1013 G3 01.17.00
Comment 8 Dan Scally 2021-10-12 07:43:09 UTC
Great; thanks very much. The bad news is that it's not the same as the 1012, so we haven't done any work on these sensors yet. The driver that crashed for you originally is designed to handle the device that provides power to the camera sensors. It comes in a couple of different varieties, and this system has one of each. The INT3537 sensor (which is a HI556) uses the easier variety. I can probably make that camera work without too much effort - there's a driver already in the kernel, it'll just need a couple of minor tweaks (it expects ACPI to handle power at the moment) plus some config added to the INT3472 driver.

The INT347E is the IR camera, we're working on that one already but the IPU3 driver doesn't handle images in that format yet, so it's a lot tougher.

The OVTI8856 uses the harder variety of power management IC - we can make that work too but we need to create a board file defining which lines from the INT3472 connect to which lines on the camera sensor itself. That has to be absolutely right, or we risk some magic smoke. Getting the information is a case of either finding a schematic, or else running Windows on the device and using some code to read the registers that are set in the PMIC and Sensor chip to see what configuration is running in that OS, and then mirror it.

I'll put together a wiki page on running the Windows code to get the right config for the OVTI8856 - no pressure if you're not interested in doing it of course, but I need to write the page anyway so may as well be now.

For the INT3537 sensor, I'll try and put together a patch series over the next couple of days. If you found time to try it, that would be great.