Linux IIO development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.17-6.6] iio: adc: imx93_adc: load calibrated values even calibration failed
       [not found] <20251025160905.3857885-1-sashal@kernel.org>
@ 2025-10-25 15:55 ` Sasha Levin
  2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] iio: imu: bmi270: Match PNP ID found on newer GPD firmware Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-10-25 15:55 UTC (permalink / raw)
  To: patches, stable
  Cc: Haibo Chen, Frank Li, Primoz Fiser, Jonathan Cameron, Sasha Levin,
	shawnguo, linux-iio, imx, linux-arm-kernel

From: Haibo Chen <haibo.chen@nxp.com>

[ Upstream commit 12c9b09e981ab14ebec8e4eefa946cbd26dd306b ]

ADC calibration might fail because of the noise on reference voltage.
To avoid calibration fail, need to meet the following requirement:
    ADC reference voltage Noise < 1.8V * 1/2^ENOB

For the case which the ADC reference voltage on board do not meet
the requirement, still load the calibrated values, so ADC can also
work but maybe not that accurate.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Primoz Fiser <primoz.fiser@norik.com>
Link: https://patch.msgid.link/20250812-adc-v2-2-0260833f13b8@nxp.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

Explanation
- Problem addressed: On some boards the i.MX93 ADC calibration fails due
  to reference voltage noise (commit message). In the current code, a
  calibration failure aborts driver probe, leaving the ADC unusable for
  users.
- Current failure path: `imx93_adc_calibration()` returns `-EAGAIN` on
  calibration failure, explicitly powering down the ADC and aborting
  probe:
  - Failure check/return: drivers/iio/adc/imx93_adc.c:181 (checks
    `IMX93_ADC_MSR_CALFAIL_MASK`) and drivers/iio/adc/imx93_adc.c:185
    (returns `-EAGAIN`).
  - Probe abort on error: drivers/iio/adc/imx93_adc.c:367 (calibration
    call) and drivers/iio/adc/imx93_adc.c:368–396 (error unwinding).
- What the patch changes:
  - Adds `IMX93_ADC_CALCFG0` (0x3A0) and `IMX93_ADC_CALCFG0_LDFAIL_MASK`
    (BIT(4)) so the driver can instruct hardware to load calibrated
    values even if calibration “fails”.
  - In `imx93_adc_calibration()` (drivers/iio/adc/imx93_adc.c:146),
    before starting calibration, it writes
    `IMX93_ADC_CALCFG0_LDFAIL_MASK` to `IMX93_ADC_CALCFG0` to enable
    “load-on-fail”.
  - It changes the failure handling on `CALFAIL`: instead of returning
    `-EAGAIN`, it logs a warning and continues, allowing the driver to
    register and the ADC to function, albeit with potentially reduced
    accuracy.
  - The timeout path remains unchanged and still returns an error if
    calibration never completes (drivers/iio/adc/imx93_adc.c:171–178),
    preserving safety for a hard failure.
- User impact: This is a practical fix for real boards where Vref noise
  is above the stated threshold; without it, the ADC never comes up.
  With it, the ADC works (possibly with lower accuracy), which is
  typically preferable to complete unavailability.
- Scope and risk:
  - Small, contained change in a single driver
    (`drivers/iio/adc/imx93_adc.c`) with no ABI or framework changes.
  - No architectural refactoring; only adds a register define and a
    single bit write plus relaxed error handling.
  - Timeout/hard-error behavior is unchanged; only soft failure
    (CALFAIL) behavior is relaxed.
  - The driver matches only `nxp,imx93-adc`
    (drivers/iio/adc/imx93_adc.c:465–469), so the change is isolated to
    this hardware.
- Stable criteria:
  - Fixes a user-visible bug (driver failing to probe on noisy Vref
    boards).
  - Minimal and low risk; confined to probe/calibration logic.
  - No new features; behavior change is a robustness fix with guarded
    warning.
  - No broader side effects beyond this ADC device.

Given these points, this is a solid candidate for backporting to any
stable trees that contain the i.MX93 ADC driver and its current fail-
hard calibration path.

 drivers/iio/adc/imx93_adc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
index 7feaafd2316f2..9f1546c3d39d5 100644
--- a/drivers/iio/adc/imx93_adc.c
+++ b/drivers/iio/adc/imx93_adc.c
@@ -38,6 +38,7 @@
 #define IMX93_ADC_PCDR6		0x118
 #define IMX93_ADC_PCDR7		0x11c
 #define IMX93_ADC_CALSTAT	0x39C
+#define IMX93_ADC_CALCFG0	0x3A0
 
 /* ADC bit shift */
 #define IMX93_ADC_MCR_MODE_MASK			BIT(29)
@@ -58,6 +59,8 @@
 #define IMX93_ADC_IMR_ECH_MASK			BIT(0)
 #define IMX93_ADC_PCDR_CDATA_MASK		GENMASK(11, 0)
 
+#define IMX93_ADC_CALCFG0_LDFAIL_MASK		BIT(4)
+
 /* ADC status */
 #define IMX93_ADC_MSR_ADCSTATUS_IDLE			0
 #define IMX93_ADC_MSR_ADCSTATUS_POWER_DOWN		1
@@ -145,7 +148,7 @@ static void imx93_adc_config_ad_clk(struct imx93_adc *adc)
 
 static int imx93_adc_calibration(struct imx93_adc *adc)
 {
-	u32 mcr, msr;
+	u32 mcr, msr, calcfg;
 	int ret;
 
 	/* make sure ADC in power down mode */
@@ -158,6 +161,11 @@ static int imx93_adc_calibration(struct imx93_adc *adc)
 
 	imx93_adc_power_up(adc);
 
+	/* Enable loading of calibrated values even in fail condition */
+	calcfg = readl(adc->regs + IMX93_ADC_CALCFG0);
+	calcfg |= IMX93_ADC_CALCFG0_LDFAIL_MASK;
+	writel(calcfg, adc->regs + IMX93_ADC_CALCFG0);
+
 	/*
 	 * TODO: we use the default TSAMP/NRSMPL/AVGEN in MCR,
 	 * can add the setting of these bit if need in future.
@@ -180,9 +188,13 @@ static int imx93_adc_calibration(struct imx93_adc *adc)
 	/* check whether calbration is success or not */
 	msr = readl(adc->regs + IMX93_ADC_MSR);
 	if (msr & IMX93_ADC_MSR_CALFAIL_MASK) {
+		/*
+		 * Only give warning here, this means the noise of the
+		 * reference voltage do not meet the requirement:
+		 *     ADC reference voltage Noise < 1.8V * 1/2^ENOB
+		 * And the resault of ADC is not that accurate.
+		 */
 		dev_warn(adc->dev, "ADC calibration failed!\n");
-		imx93_adc_power_down(adc);
-		return -EAGAIN;
 	}
 
 	return 0;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.17] iio: imu: bmi270: Match PNP ID found on newer GPD firmware
       [not found] <20251025160905.3857885-1-sashal@kernel.org>
  2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.6] iio: adc: imx93_adc: load calibrated values even calibration failed Sasha Levin
@ 2025-10-25 15:59 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-10-25 15:59 UTC (permalink / raw)
  To: patches, stable
  Cc: Cryolitia PukNgae, Andy Shevchenko, Alex Lanzano,
	Jonathan Cameron, Sasha Levin, linux-iio

From: Cryolitia PukNgae <cryolitia@uniontech.com>

[ Upstream commit dc757dc1572d579c2634c05d0a03c5676227c571 ]

GPD devices originally used BMI160 sensors with the "BMI0160" PNP ID.
When they switched to BMI260 sensors in newer hardware, they reused
the existing Windows driver which accepts both "BMI0160" and "BMI0260"
IDs. Consequently, they kept "BMI0160" in DSDT tables for new BMI260
devices, causing driver mismatches in Linux.

1. GPD updated BIOS v0.40+[1] for newer devices to report "BMI0260" for
BMI260 sensors to avoid loading the bmi160 driver on Linux. While this
isn't Bosch's VID;
2. Bosch's official Windows driver uses "BMI0260" as a compatible ID
3. We're seeing real devices shipping with "BMI0260" in DSDT

The DSDT excerpt of GPD G1619-04 with BIOS v0.40:

Scope (_SB.I2CC)
{
    Device (BMA2)
    {
        Name (_ADR, Zero)  // _ADR: Address
        Name (_HID, "BMI0260")  // _HID: Hardware ID
        Name (_CID, "BMI0260")  // _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 (0x0069, ControllerInitiated, 0x00061A80,
                    AddressingMode7Bit, "\\_SB.I2CC",
                    0x00, ResourceConsumer, , Exclusive,
                    )
            })
            Return (RBUF) /* \_SB_.I2CC.BMA2._CRS.RBUF */
        }
        # omit some noise
    }
}

Link: http://download.softwincn.com/WIN%20Max%202024/Max2-7840-BIOS-V0.41.zip #1
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Acked-by: Alex Lanzano <lanzano.alex@gmail.com>
Link: https://patch.msgid.link/20250821-bmi270-gpd-acpi-v4-1-5279b471d749@uniontech.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## BACKPORT RECOMMENDATION: **YES**

### Executive Summary

This commit adds ACPI device ID "BMI0260" to the bmi270 IIO driver to
support BMI260 IMU sensors on newer GPD devices (BIOS v0.40+). The
change is minimal (2 lines), safe, and addresses a real hardware
compatibility issue affecting users in the field.

---

### Comprehensive Analysis

#### 1. **Problem Being Fixed**

**User-Visible Bug**: GPD gaming handheld devices with updated BIOS
firmware (v0.40+) declare "BMI0260" as the ACPI hardware ID for their
BMI260 IMU sensors. Without this patch, these sensors are completely
non-functional because no Linux driver claims the "BMI0260" ACPI ID.

**Background Context** (from commit f35f3c832eb58):
- GPD originally used BMI160 sensors with "BMI0160" ACPI ID
- They switched to BMI260 hardware but initially kept "BMI0160" in
  firmware tables (causing driver mismatches)
- GPD released BIOS v0.40+ that properly reports "BMI0260" for BMI260
  sensors
- Linux needs to be updated to recognize this new ACPI ID

**Affected Hardware**: GPD Win Max 2 2023, GPD G1619-04, and potentially
other GPD gaming handhelds with BIOS v0.40+

#### 2. **Code Changes Analysis**

The commit modifies `drivers/iio/imu/bmi270/bmi270_i2c.c`:

```c
static const struct acpi_device_id bmi270_acpi_match[] = {
    /* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
    { "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
+   /* GPD Win Max 2 2023(sincice BIOS v0.40), etc. */
+   { "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
    { }
};
```

**Change Characteristics**:
- **Size**: 2 lines added (1 comment, 1 ACPI ID entry)
- **Scope**: Single driver file, single match table
- **Type**: Device ID addition (no logic changes)
- **Complexity**: Trivial

**Minor Issue Noted**: Typo in comment: "sincice" should be "since"
- **Impact**: None - cosmetic only, doesn't affect functionality
- **Note**: In Linux kernel stable backports, typos in comments are
  acceptable if the code is correct

#### 3. **Safety & Risk Assessment**

**Regression Risk: ESSENTIALLY ZERO**

Evidence supporting zero regression risk:

1. **No Existing Functionality Modified**: The change only adds a new
   ACPI ID that currently matches NO driver
2. **Isolated Impact**: Only affects devices that declare "BMI0260" in
   their ACPI tables
3. **Hardware Validation Present**: The driver
   (bmi270_core.c:bmi270_validate_chip_id) reads the actual chip ID from
   hardware registers and validates it:
  ```c
  if (chip_id == BMI160_CHIP_ID_VAL)
  return -ENODEV;  // Reject BMI160 chips

  if (chip_id == bmi260_chip_info.chip_id)
  data->chip_info = &bmi260_chip_info;
  else if (chip_id == bmi270_chip_info.chip_id)
  data->chip_info = &bmi270_chip_info;
  ```
4. **Defense in Depth**: Even if ACPI tables are wrong, the driver
   detects mismatches via chip ID
5. **No Driver Conflicts**: The bmi160 driver does NOT claim "BMI0260"
   (I verified via grep - no files in bmi160 directory contain
   "BMI0260")

**Security Assessment**: LOW risk (from security audit agent)
- Chip ID validation prevents device confusion attacks
- No new attack surfaces introduced
- Firmware loading security is a pre-existing concern, not introduced by
  this change

#### 4. **Dependencies & Backport Target**

**Required Dependency**: BMI260 hardware support
- Commit: f35f3c832eb58 "iio: imu: bmi270: Add support for BMI260"
- Merged in: v6.13-rc1 (verified via `git describe --contains`)

**Backport Target**: Stable kernels v6.13 and later

**Reason**: Earlier kernels don't have BMI260 support, so adding the
ACPI ID would be meaningless

#### 5. **Stable Tree Criteria Evaluation**

| Criterion | Met? | Evidence |
|-----------|------|----------|
| Fixes important bug | ✅ YES | Hardware completely non-functional
without fix |
| Small and contained | ✅ YES | 2 lines, single file, single driver |
| Minimal regression risk | ✅ YES | Zero risk - only affects new device
IDs |
| Clear side effects? | ✅ NO | No side effects beyond fixing the issue |
| Architectural changes? | ✅ NO | None whatsoever |
| Critical subsystem? | ✅ NO | IIO driver for specific sensor |
| Well-reviewed | ✅ YES | Reviewed by Andy Shevchenko, Acked by Alex
Lanzano |
| Explicit stable tag | ⚠️ NO | Not present, but often added during
backport process |

**Score**: 7/8 criteria clearly met, 1 not required

#### 6. **Historical Context & Precedent**

**Similar Backported Commits**:
- commit ca2f16c315683: "Add 10EC5280 to bmi160_i2c ACPI IDs" - Similar
  ACPI ID addition for BMI160
- Many other IIO driver ACPI ID additions routinely get backported

**Established Pattern**: The kernel community regularly backports simple
device ID additions like this, as they:
- Enable hardware support for real devices
- Have zero regression risk
- Are trivial changes
- Don't modify existing functionality

#### 7. **ACPI Matching Mechanism Research**

From my kernel code researcher agent's findings:

**Key Insights**:
1. Multiple drivers CAN safely declare the same ACPI ID
2. The bmi160 and bmi270 drivers already share "BMI0160" successfully
3. When multiple drivers match, kernel tries each in order
4. Returning -ENODEV from probe allows fallback to next driver
5. The bmi270 driver explicitly checks chip ID and returns -ENODEV for
   BMI160 chips

**Safety Mechanism**: This design allows hardware-based driver selection
rather than relying solely on ACPI tables, which manufacturers sometimes
get wrong.

#### 8. **Real-World Impact**

**Without this patch**:
- Users with GPD devices on BIOS v0.40+ have non-functional IMU sensors
- Accelerometer and gyroscope features don't work
- Gaming handhelds rely on these sensors for screen rotation and game
  controls

**With this patch**:
- IMU sensors work correctly
- All sensor functionality enabled (accelerometer, gyroscope,
  temperature)
- Users can use screen rotation and sensor-based controls

**User Base**: Gaming handheld enthusiasts who keep their firmware
updated

#### 9. **Quality Indicators**

**Review Process**:
- Reviewed-by: Andy Shevchenko <andy@kernel.org> (experienced kernel
  developer)
- Acked-by: Alex Lanzano <lanzano.alex@gmail.com> (BMI270 driver
  maintainer)
- Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> (IIO
  subsystem maintainer)

**Testing**: Presumably tested on actual GPD hardware with BIOS v0.40+

**Code Style**: Follows existing patterns in the driver exactly

#### 10. **Comparison to Other Backport Candidates**

This commit is more straightforward than many other backport candidates
because:
- It's purely additive (no modifications to existing code)
- The affected hardware is completely non-functional without it
- There's zero chance of breaking existing functionality
- It's been reviewed by multiple experienced maintainers

---

### Final Recommendation: **YES - SHOULD BE BACKPORTED**

**Backport to**: Stable kernels v6.13+

**Justification**:
1. ✅ **Fixes Real User Problem**: Hardware compatibility issue affecting
   real devices
2. ✅ **Zero Regression Risk**: Only adds new device ID, no existing
   functionality changed
3. ✅ **Trivial Change**: 2 lines, well-reviewed, follows established
   patterns
4. ✅ **Safety Mechanisms**: Driver has chip ID validation preventing
   mismatches
5. ✅ **Meets Stable Criteria**: Small, contained, important bugfix,
   minimal risk
6. ✅ **Precedent**: Similar changes routinely backported

**Note on Typo**: The "sincice" → "since" typo in the comment is
cosmetic and doesn't affect functionality. In Linux kernel stable
backports, such typos are acceptable when the code itself is correct.

**Dependencies**: Requires BMI260 support (commit f35f3c832eb58), which
is present in v6.13+

 drivers/iio/imu/bmi270/bmi270_i2c.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/imu/bmi270/bmi270_i2c.c b/drivers/iio/imu/bmi270/bmi270_i2c.c
index c77839b03a969..b909a421ad017 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -41,6 +41,8 @@ static const struct i2c_device_id bmi270_i2c_id[] = {
 static const struct acpi_device_id bmi270_acpi_match[] = {
 	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
 	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
+	/* GPD Win Max 2 2023(sincice BIOS v0.40), etc. */
+	{ "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
 	{ }
 };
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-10-25 16:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.6] iio: adc: imx93_adc: load calibrated values even calibration failed Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] iio: imu: bmi270: Match PNP ID found on newer GPD firmware Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox