[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aEw_DcqpCpcsBGd0@smile.fi.intel.com>
Date: Fri, 13 Jun 2025 18:09:01 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Marek Vasut <marek.vasut+bmc150@...lbox.org>,
Hans de Goede <hansg@...nel.org>
Cc: linux-iio@...r.kernel.org,
Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>,
David Lechner <dlechner@...libre.com>,
Jonathan Cameron <jic23@...nel.org>,
Julien Stephan <jstephan@...libre.com>,
Peter Zijlstra <peterz@...radead.org>,
Salvatore Bonaccorso <carnil@...ian.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: accel: bmc150: Do not configure IRQ registers if no
IRQ connected
Strange I don't see Hans in the Cc list, so added.
Thanks for the report and patch, my comments below.
On Fri, Jun 13, 2025 at 02:45:22PM +0200, Marek Vasut wrote:
> The BMC150 on Onemix 2S does not have IRQ line described in ACPI tables,
> which leads to bmc150_accel_core_probe() being called with irq=0, which
> leads to bmc150_accel_interrupts_setup() never being called, which leads
> to struct bmc150_accel_data *data ->interrupts[i].info being left unset
> to NULL. Later, userspace can indirectly trigger bmc150_accel_set_interrupt()
> which depends on struct bmc150_accel_data *data ->interrupts[i].info being
> non-NULL, and which triggers NULL pointer dereference. This is triggered
> e.g. from iio-sensor-proxy.
>
> Fix this by skipping the IRQ register configuration in case there is no
> IRQ connected in hardware, in a manner similar to what the driver did in
> the very first commit which added the driver.
>
> ACPI table dump:
> Device (BMA2)
> {
> Name (_ADR, Zero) // _ADR: Address
> Name (_HID, "BOSC0200") // _HID: Hardware ID
> Name (_CID, "BOSC0200") // _CID: Compatible ID
> Name (_DDN, "Accelerometer") // _DDN: DOS Device Name
> Name (_UID, One) // _UID: Unique ID
> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
> {
> Name (RBUF, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0019, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C0",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
> Return (RBUF) /* \_SB_.PCI0.I2C0.BMA2._CRS.RBUF */
> }
These lines...
> Method (ROTM, 0, NotSerialized)
> {
> Name (SBUF, Package (0x03)
> {
> "0 1 0",
> "1 0 0 ",
> "0 0 1"
> })
> Return (SBUF) /* \_SB_.PCI0.I2C0.BMA2.ROTM.SBUF */
> }
>
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> Return (0x0F)
> }
...are irrelevant.
> }
> "
>
> Splat, collected from debian unstable, probably not very useful:
Oh my gosh, please leave only ~3-5 *important* lines out of this, or move it
completely to the comment block (after '---' cutter line).
This is requirement written in Submitting Patches.
...
As for the solution, are you sure the line is not wired at all?
IIRC Hans had a broken tales where it was simply forgotten, meaning
the Android / Windows driver simply hardcoded needed info.
If it's the case, it should be solved differently around PDx86 special quirk
driver for the cases like this.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists