* [PATCH v4 01/10] Input: cap11xx - clean up duplicate log and add probe error logs
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Duplicated device detection log exists at line 537 and line 542,
which brings redundant kernel print messages. Drop one redundant
log entry to clean up dmesg output.
Meanwhile add missing error logs when I2C communication fails
during driver probe(), helping debug.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 2447c1ae2166..485d8ba97723 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -512,7 +512,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read product ID\n");
if (val != cap->product_id) {
dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
@@ -522,7 +522,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_MANUFACTURER_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read manufacturer ID\n");
if (val != CAP11XX_MANUFACTURER_ID) {
dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
@@ -531,11 +531,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
}
error = regmap_read(priv->regmap, CAP11XX_REG_REVISION, &rev);
- if (error < 0)
- return error;
-
- dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
- id->name, rev);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to read revision\n");
priv->model = cap;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 07/10] Input: cap11xx - refactor code for better CAP1114 support.
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Extend cap11xx_hw_model structure to support CAP1114 with
different register offsets and hardware characteristics:
- led_output_control_reg_base: different address on CAP1114
- sensor_input_reg_base: different address on CAP1114
- num_sensor_thresholds: separate value from num_channels for CAP1114
- has_repeat_en: repeat enable support, disabled by default on CAP1114
Include linux/bits.h, update the register operations related to LEDs.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 73 +++++++++++++++++++++++---------
1 file changed, 53 insertions(+), 20 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 1db4a9090705..0f19ee036e78 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
* (c) 2014 Daniel Mack <linux@zonque.org>
*/
+#include <linux/bits.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -36,7 +37,6 @@
#define CAP11XX_REG_LED_DUTY_CYCLE_4 0x93
#define CAP11XX_REG_LED_DUTY_MAX_MASK (0xf0)
-#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT (4)
#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
#define CAP11XX_REG_PRODUCT_ID 0xfd
@@ -77,10 +77,14 @@ struct cap11xx_priv {
struct cap11xx_hw_model {
u8 product_id;
+ u8 led_output_control_reg_base;
+ u8 sensor_input_reg_base;
unsigned int num_channels;
unsigned int num_leds;
+ unsigned int num_sensor_thresholds;
bool has_gain;
bool has_irq_config;
+ bool has_repeat_en;
bool has_sensitivity_control;
bool has_signal_guard;
};
@@ -211,8 +215,8 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
}
if (!of_property_read_u32_array(node, "microchip,input-threshold",
- priv->thresholds, priv->model->num_channels)) {
- for (i = 0; i < priv->model->num_channels; i++) {
+ priv->thresholds, priv->model->num_sensor_thresholds)) {
+ for (i = 0; i < priv->model->num_sensor_thresholds; i++) {
if (priv->thresholds[i] > 127) {
dev_err(dev, "Invalid input-threshold value %u\n",
priv->thresholds[i]);
@@ -286,10 +290,12 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
of_property_read_u32_array(node, "linux,keycodes",
priv->keycodes, priv->model->num_channels);
- /* Disable autorepeat. The Linux input system has its own handling. */
- error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
- if (error)
- return error;
+ if (priv->model->has_repeat_en) {
+ /* Disable autorepeat. The Linux input system has its own handling. */
+ error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+ if (error)
+ return error;
+ }
return 0;
}
@@ -308,7 +314,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
if (ret < 0)
goto out;
- ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
+ ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base, &status);
if (ret < 0)
goto out;
@@ -362,7 +368,7 @@ static int cap11xx_led_set(struct led_classdev *cdev,
* 0 (OFF) and 1 (ON).
*/
return regmap_update_bits(priv->regmap,
- CAP11XX_REG_LED_OUTPUT_CONTROL,
+ priv->model->led_output_control_reg_base,
BIT(led->reg),
value ? BIT(led->reg) : 0);
}
@@ -374,6 +380,7 @@ static int cap11xx_init_leds(struct device *dev,
struct cap11xx_led *led;
int cnt = of_get_child_count(node);
int error;
+ u32 duty_val;
if (!num_leds || !cnt)
return 0;
@@ -387,15 +394,18 @@ static int cap11xx_init_leds(struct device *dev,
priv->leds = led;
+ /* Set all LEDs to off */
error = regmap_update_bits(priv->regmap,
- CAP11XX_REG_LED_OUTPUT_CONTROL, 0xff, 0);
+ priv->model->led_output_control_reg_base,
+ GENMASK(min(num_leds, 8) - 1, 0), 0);
if (error)
return error;
+ duty_val = FIELD_PREP(CAP11XX_REG_LED_DUTY_MAX_MASK,
+ CAP11XX_REG_LED_DUTY_MAX_VALUE);
+
error = regmap_update_bits(priv->regmap, CAP11XX_REG_LED_DUTY_CYCLE_4,
- CAP11XX_REG_LED_DUTY_MAX_MASK,
- CAP11XX_REG_LED_DUTY_MAX_VALUE <<
- CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);
+ CAP11XX_REG_LED_DUTY_MAX_MASK, duty_val);
if (error)
return error;
@@ -561,41 +571,64 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
}
static const struct cap11xx_hw_model cap1106_model = {
- .product_id = 0x55, .num_channels = 6, .num_leds = 0,
+ .product_id = 0x55,
+ .num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1126_model = {
- .product_id = 0x53, .num_channels = 6, .num_leds = 2,
+ .product_id = 0x53,
+ .num_channels = 6, .num_leds = 2, .num_sensor_thresholds = 6,
+ .led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1188_model = {
- .product_id = 0x50, .num_channels = 8, .num_leds = 8,
+ .product_id = 0x50,
+ .num_channels = 8, .num_leds = 8, .num_sensor_thresholds = 8,
+ .led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
.has_irq_config = true,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1203_model = {
- .product_id = 0x6d, .num_channels = 3, .num_leds = 0,
+ .product_id = 0x6d,
+ .num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1206_model = {
- .product_id = 0x67, .num_channels = 6, .num_leds = 0,
+ .product_id = 0x67,
+ .num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+ .has_repeat_en = true,
};
static const struct cap11xx_hw_model cap1293_model = {
- .product_id = 0x6f, .num_channels = 3, .num_leds = 0,
+ .product_id = 0x6f,
+ .num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
+ .has_repeat_en = true,
.has_sensitivity_control = true,
.has_signal_guard = true,
};
static const struct cap11xx_hw_model cap1298_model = {
- .product_id = 0x71, .num_channels = 8, .num_leds = 0,
+ .product_id = 0x71,
+ .num_channels = 8, .num_leds = 0, .num_sensor_thresholds = 8,
+ .sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
.has_gain = true,
+ .has_repeat_en = true,
.has_sensitivity_control = true,
.has_signal_guard = true,
};
--
2.54.0
^ permalink raw reply related
* [PATCH v4 06/10] Input: cap11xx - add reset gpio support
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Some CAP11xx devices (CAP1126/CAP1188) have a dedicated RESET pin.
Add hardware reset operation to improve device reliability and
ensure proper initialization on probe.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index fae26f035186..1db4a9090705 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
* (c) 2014 Daniel Mack <linux@zonque.org>
*/
+#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/interrupt.h>
@@ -44,6 +45,9 @@
#define CAP11XX_MANUFACTURER_ID 0x5d
+#define CAP11XX_T_RST_FILT_MIN_US 10000
+#define CAP11XX_T_RST_ON_MIN_MS 400
+
#ifdef CONFIG_LEDS_CLASS
struct cap11xx_led {
struct cap11xx_priv *priv;
@@ -56,6 +60,7 @@ struct cap11xx_priv {
struct regmap *regmap;
struct device *dev;
struct input_dev *idev;
+ struct gpio_desc *reset_gpio;
const struct cap11xx_hw_model *model;
struct cap11xx_led *leds;
@@ -459,6 +464,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
+ priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
+ "Failed to get 'reset' GPIO\n");
+
+ if (priv->reset_gpio) {
+ usleep_range(CAP11XX_T_RST_FILT_MIN_US, CAP11XX_T_RST_FILT_MIN_US * 2);
+ gpiod_set_value_cansleep(priv->reset_gpio, 0);
+ msleep(CAP11XX_T_RST_ON_MIN_MS);
+ }
+
error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
if (error)
return dev_err_probe(dev, error, "Failed to read product ID\n");
--
2.54.0
^ permalink raw reply related
* [PATCH v4 05/10] dt-bindings: input: microchip,cap11xx: Add reset-gpios property
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Add support for the optional reset-gpios property to describe
the active-high reset pin for CAP1126/CAP1188 devices.
Driving the GPIO high asserts reset and deep sleep, while driving
it low releases reset for normal operation.
Restrict this property to be available only on CAP1126 and CAP1188
chips, as other CAP11xx variants do not have a hardware reset pin.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 798035e942af..b97e5b2735f1 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -49,6 +49,13 @@ properties:
device's ALERT#/CM_IRQ# pin is connected to.
The device only has one interrupt source.
+ reset-gpios:
+ description: |
+ GPIO connected to the active-high RESET pin of the chip;
+ driving it high asserts reset and deep sleep, while driving
+ it low releases reset for normal operation.
+ maxItems: 1
+
autorepeat:
description: |
Enables the Linux input system's autorepeat feature on the input device.
@@ -157,6 +164,20 @@ patternProperties:
allOf:
- $ref: input.yaml
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1106
+ - microchip,cap1203
+ - microchip,cap1206
+ - microchip,cap1293
+ - microchip,cap1298
+ then:
+ properties:
+ reset-gpios: false
+
- if:
properties:
compatible:
@@ -207,6 +228,8 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
+
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -228,6 +251,8 @@ examples:
<109>, /* KEY_PAGEDOWN */
<104>; /* KEY_PAGEUP */
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+
#address-cells = <1>;
#size-cells = <0>;
--
2.54.0
^ permalink raw reply related
* [PATCH v4 04/10] dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg constraints
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Apply per-chip LED channel limits:
- CAP1126: max 2 channels (0-1)
- CAP1188: max 8 channels (0-7)
- CAP1106, CAP12xx: no LED support
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index eabf06a1163e..798035e942af 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -171,6 +171,19 @@ allOf:
patternProperties:
"^led@": false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1126
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 1
+
- if:
properties:
compatible:
--
2.54.0
^ permalink raw reply related
* [PATCH v4 03/10] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
- Add datasheet links for all supported CAP11xx variants.
- Update LED node regex and replace enum constraints with minimum/maximum
for LED reg ranges in preparation for CAP1114 support.
CAP1114 has 11 LED channels. minimum/maximum constraints are easier to
maintain than long enum lists when expanding channel count later.
Drop unnecessary led unit-address pattern.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 7ade03f1b32b..eabf06a1163e 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -10,6 +10,15 @@ description: |
The Microchip CAP1xxx Family of RightTouchTM multiple-channel capacitive
touch controllers and LED drivers. The device communication via I2C only.
+ For more product information please see the links below:
+ CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+ CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
+ CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
+ CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
+ CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.pdf
+ CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.pdf
+ CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.pdf
+
maintainers:
- Rob Herring <robh@kernel.org>
@@ -131,7 +140,9 @@ patternProperties:
properties:
reg:
- enum: [0, 1, 2, 3, 4, 5, 6, 7]
+ description: LED channel number
+ minimum: 0
+ maximum: 7
label: true
@@ -158,7 +169,7 @@ allOf:
- microchip,cap1298
then:
patternProperties:
- "^led@[0-7]$": false
+ "^led@": false
- if:
properties:
--
2.54.0
^ permalink raw reply related
* [PATCH v4 02/10] Input: cap11xx - remove unused register macros
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
Remove unused register address macros and unused definitions in
the cap11xx_reg_defaults array and cap11xx_volatile_reg.
This cleanup reduces code clutter and makes the driver easier to
maintain without affecting functionality.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 51 --------------------------------
1 file changed, 51 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 485d8ba97723..fae26f035186 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -20,53 +20,24 @@
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
#define CAP11XX_REG_MAIN_CONTROL_DLSEEP BIT(4)
-#define CAP11XX_REG_GENERAL_STATUS 0x02
#define CAP11XX_REG_SENSOR_INPUT 0x03
-#define CAP11XX_REG_NOISE_FLAG_STATUS 0x0a
#define CAP11XX_REG_SENOR_DELTA(X) (0x10 + (X))
#define CAP11XX_REG_SENSITIVITY_CONTROL 0x1f
#define CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK 0x70
-#define CAP11XX_REG_CONFIG 0x20
-#define CAP11XX_REG_SENSOR_ENABLE 0x21
-#define CAP11XX_REG_SENSOR_CONFIG 0x22
-#define CAP11XX_REG_SENSOR_CONFIG2 0x23
-#define CAP11XX_REG_SAMPLING_CONFIG 0x24
-#define CAP11XX_REG_CALIBRATION 0x26
-#define CAP11XX_REG_INT_ENABLE 0x27
#define CAP11XX_REG_REPEAT_RATE 0x28
#define CAP11XX_REG_SIGNAL_GUARD_ENABLE 0x29
-#define CAP11XX_REG_MT_CONFIG 0x2a
-#define CAP11XX_REG_MT_PATTERN_CONFIG 0x2b
-#define CAP11XX_REG_MT_PATTERN 0x2d
-#define CAP11XX_REG_RECALIB_CONFIG 0x2f
#define CAP11XX_REG_SENSOR_THRESH(X) (0x30 + (X))
-#define CAP11XX_REG_SENSOR_NOISE_THRESH 0x38
-#define CAP11XX_REG_STANDBY_CHANNEL 0x40
-#define CAP11XX_REG_STANDBY_CONFIG 0x41
-#define CAP11XX_REG_STANDBY_SENSITIVITY 0x42
-#define CAP11XX_REG_STANDBY_THRESH 0x43
#define CAP11XX_REG_CONFIG2 0x44
#define CAP11XX_REG_CONFIG2_ALT_POL BIT(6)
-#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
-#define CAP11XX_REG_LED_POLARITY 0x73
#define CAP11XX_REG_LED_OUTPUT_CONTROL 0x74
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG 0x80
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2 0x81
-
-#define CAP11XX_REG_LED_DUTY_CYCLE_1 0x90
-#define CAP11XX_REG_LED_DUTY_CYCLE_2 0x91
-#define CAP11XX_REG_LED_DUTY_CYCLE_3 0x92
#define CAP11XX_REG_LED_DUTY_CYCLE_4 0x93
-#define CAP11XX_REG_LED_DUTY_MIN_MASK (0x0f)
-#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT (0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK (0xf0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT (4)
#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
-#define CAP11XX_REG_SENSOR_CALIB (0xb1 + (X))
-#define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
-#define CAP11XX_REG_SENSOR_CALIB_LSB2 0xba
#define CAP11XX_REG_PRODUCT_ID 0xfd
#define CAP11XX_REG_MANUFACTURER_ID 0xfe
#define CAP11XX_REG_REVISION 0xff
@@ -111,37 +82,15 @@ struct cap11xx_hw_model {
static const struct reg_default cap11xx_reg_defaults[] = {
{ CAP11XX_REG_MAIN_CONTROL, 0x00 },
- { CAP11XX_REG_GENERAL_STATUS, 0x00 },
- { CAP11XX_REG_SENSOR_INPUT, 0x00 },
- { CAP11XX_REG_NOISE_FLAG_STATUS, 0x00 },
{ CAP11XX_REG_SENSITIVITY_CONTROL, 0x2f },
- { CAP11XX_REG_CONFIG, 0x20 },
- { CAP11XX_REG_SENSOR_ENABLE, 0x3f },
- { CAP11XX_REG_SENSOR_CONFIG, 0xa4 },
- { CAP11XX_REG_SENSOR_CONFIG2, 0x07 },
- { CAP11XX_REG_SAMPLING_CONFIG, 0x39 },
- { CAP11XX_REG_CALIBRATION, 0x00 },
- { CAP11XX_REG_INT_ENABLE, 0x3f },
{ CAP11XX_REG_REPEAT_RATE, 0x3f },
- { CAP11XX_REG_MT_CONFIG, 0x80 },
- { CAP11XX_REG_MT_PATTERN_CONFIG, 0x00 },
- { CAP11XX_REG_MT_PATTERN, 0x3f },
- { CAP11XX_REG_RECALIB_CONFIG, 0x8a },
{ CAP11XX_REG_SENSOR_THRESH(0), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(1), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(2), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
- { CAP11XX_REG_SENSOR_NOISE_THRESH, 0x01 },
- { CAP11XX_REG_STANDBY_CHANNEL, 0x00 },
- { CAP11XX_REG_STANDBY_CONFIG, 0x39 },
- { CAP11XX_REG_STANDBY_SENSITIVITY, 0x02 },
- { CAP11XX_REG_STANDBY_THRESH, 0x40 },
{ CAP11XX_REG_CONFIG2, 0x40 },
- { CAP11XX_REG_LED_POLARITY, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB1, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB2, 0x00 },
};
static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
--
2.54.0
^ permalink raw reply related
* [PATCH v4 00/10] Input: cap11xx - Add support for CAP1114
From: Jun Yan @ 2026-06-17 15:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
Patches 1-4 perform driver cleanup and DT binding tweaks.
Patches 5-6 add reset-gpios support for CAP11xx.
Patches 7-10 add support for CAP1114.
Changes in v4:
- Revert cleanup operation for CAP11XX_REG_SENSOR_DELTA.
- Move the LED unit-address update operation to the CAP1114 support patch.
- Limit the CAP1114 LED unit-address range from 0x0 to 0xa.
- Link to v3:
https://lore.kernel.org/all/20260615142103.352163-1-jerrysteve1101@gmail.com/
Changes in v3:
- Simplified the logic of the reset pin operation.
- Adjust linux,keycodes configuration for CAP11xx.
- Drop unnecessary CAP11XX_REG_SENSOR_THRESH(8).
- Checks for the presence of microchip,calib-sensitivity and
microchip,signal-guard properties before processing them.
- Link to v2:
https://lore.kernel.org/all/20260612072237.1177304-1-jerrysteve1101@gmail.com/
Changes in v2:
- Drop LED property tweaks, keep only reg changes and node regex
update in DT bindings.
- Split microchip,cap1126 LED reg constraints into a separate patch.
- Replace usleep_range() with msleep() for 500 ms delay during
reset pin handling.
- Add missing <linux/delay.h> for usleep_range() and msleep().
- Add CAP1114 to unsupported enum for microchip,signal-guard and
microchip,calib-sensitivity
- Add constraint for linux,keycodes to support CAP1114.
- When reading CAP1114 button status, mask STATUS1 to bits 0-5
and OR with STATUS2.
- Adjust code style.
- Link to v1:
https://lore.kernel.org/all/20260606150458.250606-1-jerrysteve1101@gmail.com
Jun Yan (10):
Input: cap11xx - clean up duplicate log and add probe error logs
Input: cap11xx - remove unused register macros
dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED
reg range
dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg
constraints
dt-bindings: input: microchip,cap11xx: Add reset-gpios property
Input: cap11xx - add reset gpio support
Input: cap11xx - refactor code for better CAP1114 support.
Input: cap11xx - guard unsupported DT properties before parsing
dt-bindings: input: microchip,cap11xx: Add CAP1114 support
Input: cap11xx - add support for CAP1114
.../bindings/input/microchip,cap11xx.yaml | 90 +++++-
drivers/input/keyboard/cap11xx.c | 274 +++++++++++-------
2 files changed, 254 insertions(+), 110 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH] dt-bindings: sound: add toshiba,apb-dummy-codec binding
From: Pablo D. Bergamasco @ 2026-06-17 14:55 UTC (permalink / raw)
To: krzk
Cc: broonie, conor+dt, danpablo, devicetree, krzk+dt, lgirdwood,
linux-kernel, linux-sound, mgreer, robh, vaibhav.sr
In-Reply-To: <20260617-marvellous-cunning-curassow-0113ab@quoll>
On Wed, Jun 17, 2026, Krzysztof Kozlowski wrote:
> Nope. We don't take bindings for staging. Isn't this documented in
> staging docs already?
I checked drivers/staging/greybus/Documentation/ and found only
firmware and sysfs documentation. There is no existing DT binding
documentation for the "toshiba,apb-dummy-codec" compatible string.
Should I add the binding documentation within the staging directory
itself, or is there a preferred approach for staging drivers?
Best regards,
Pablo D. Bergamasco
^ permalink raw reply
* Re: [PATCH v8 3/5] iio: adc: versal-sysmon: add I2C driver
From: Andy Shevchenko @ 2026-06-17 14:53 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-4-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:57PM +0100, Salih Erim wrote:
> Add an I2C transport driver for the Versal SysMon block. The SysMon
> provides an I2C slave interface that allows an external master to
> read voltage and temperature measurements through the same register
> map used by the MMIO path.
>
> The I2C command frame is an 8-byte structure containing a 4-byte data
> payload, a 2-byte register offset, and a 1-byte instruction field.
> Read operations send the frame with a read instruction, then receive
> a 4-byte response containing the register value.
>
> Events are not supported on the I2C path because there is no
> interrupt line and the I2C regmap backend cannot be called from
> atomic context.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
From: Frank Li @ 2026-06-17 14:52 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, robh, conor+dt, devicetree, linux-i3c, Frank.Li
In-Reply-To: <20260617111331.7AB671F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:13:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The binding documents GPIO capability for target ports but omits the standard GPIO provider properties, violating subsystem guidelines.
> - [Medium] The binding fails to include compatible strings for the 4-port variants and unconditionally allows up to 8 ports.
> - [Low] The unit address in the I3C example does not match its `reg` property.
> --
>
> commit af3b77595fece3e15864b37b9b683be835a0e1eb
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
>
> This commit adds device tree bindings for the NXP P3H2x4x multi-port I3C hub
> family. It supports connecting a host via I3C/I2C/SMBus to multiple downstream
> peripherals across up to 8 ports.
>
> > diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> > new file mode 100644
> > index 0000000000000..c080eeb0eeaa8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> > @@ -0,0 +1,291 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2025 NXP
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i3c/nxp,p3h2840.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP P3H2X4X I3C HUB
> > +
> > +maintainers:
> > + - Aman Kumar Pandey <aman.kumarpandey@nxp.com>
> > + - Vikash Bansal <vikash.bansal@nxp.com>
> > + - Lakshay Piplani <lakshay.piplani@nxp.com>
> > +
> > +description: |
> > + P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
> > + hub devices that connect to:-
> > + 1. A host CPU via I3C/I2C/SMBus bus on upstream side and connect to multiple
> > + peripheral devices on the downstream side.
> > + 2. Have two Controller Ports which can support either
> > + I2C/SMBus or I3C buses and connect to a CPU, BMC or SOC.
> > + 3. P3H2840/ P3H2841 are 8 port I3C hub with eight I3C/I2C Target Port.
> > + 4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
> > + Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
> > + peripherals.
>
> [Severity: Medium]
> Since the description advertises that target ports can be configured as GPIO,
> should this binding include the standard GPIO provider properties like
> gpio-controller and #gpio-cells?
>
> Because unevaluatedProperties is false and there are no child node patterns
> defined for GPIO ports in patternProperties, won't adding a GPIO child node
> fail schema validation?
>
> > +
> > +properties:
> > + compatible:
> > + const: nxp,p3h2840
>
> [Severity: Medium]
> The commit message and description state the binding supports the P3H2440
> and P3H2441 4-port hubs alongside the 8-port P3H2840/P3H2841.
>
> Should compatible strings be included for the 4-port variants here as well?
>
> [ ... ]
>
> > +patternProperties:
> > + "^i3c@[0-7]$":
> > + type: object
> > + $ref: /schemas/i3c/i3c.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + reg:
> > + description:
> > + The I3C HUB Target Port number.
> > + maximum: 7
>
> [Severity: Medium]
> Does unconditionally allowing a maximum port index of 7 cause issues for the
> 4-port variants mentioned in the description?
>
> It seems this allows invalid configurations that define non-existent ports
> (4-7) for the 4-port hardware.
>
> [ ... ]
>
> > +examples:
> > + - |
> > + i3c {
> > + #address-cells = <3>;
> > + #size-cells = <0>;
> > +
> > + hub@70,236153000c2 {
> > + reg = <0x70 0x236 0x3000c2>;
>
> [Severity: Low]
> Does the unit address in this example match its reg property?
>
> The reg property combines to a provisioned ID of 0x236003000c2, which appears
> to differ from 236153000c2 in the unit address. Could this mismatch trigger a
> simple_bus_reg warning during make dt_binding_check?
I remember I3C mask 0x15, it'd better kept node name consisent with reg
Frank
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v8 2/5] iio: adc: add Versal SysMon driver
From: Andy Shevchenko @ 2026-06-17 14:52 UTC (permalink / raw)
To: Salih Erim
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260616131559.3029543-3-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:56PM +0100, Salih Erim wrote:
> Add the core driver and MMIO platform driver for the AMD/Xilinx Versal
> System Monitor (SysMon) block.
>
> The SysMon block resides in the platform management controller (PMC) and
> provides on-chip voltage and temperature monitoring through a 10-bit,
> 200 kSPS ADC. It can monitor up to 160 voltage channels and 64
> temperature satellites distributed across the SoC, with a consistent
> sample rate of 8 kSPS per channel regardless of how many channels are
> enabled.
>
> The hardware also provides four aggregate temperature registers that
> are always present regardless of the device tree configuration: the
> current max and min across all active satellites, and the peak and
> trough values recorded since the last hardware reset.
>
> The driver is split into two compilation units:
> - versal-sysmon-core: Channel parsing, IIO registration, read_raw
> - versal-sysmon: MMIO platform driver with custom regmap accessors
>
> Voltage results are stored in a 19-bit modified floating-point format
> and converted to millivolts. Temperature results are stored in Q8.7
> signed fixed-point Celsius format and converted to millicelsius.
>
> The MMIO regmap backend uses a custom reg_write accessor that
> automatically unlocks the NPI (NoC programming interface) lock
> register before each write, as required by the hardware. The regmap
> is configured with fast_io since the underlying MMIO accessors are
> safe to call from atomic context.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
with a caveat of using namespaced exports (see below).
...
> +EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
Please, also use symbol namespace.
...
> +static int sysmon_mmio_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
Make it a single line. It will be more readable.
static int sysmon_mmio_reg_read(void *context, unsigned int reg, unsigned int *val)
...
> +static int sysmon_mmio_reg_write(void *context, unsigned int reg,
> + unsigned int val)
In the similar way.
static int sysmon_mmio_reg_write(void *context, unsigned int reg, unsigned int val)
In both cases it's only 83 characters.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
From: Frank Li @ 2026-06-17 14:50 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, linux-i3c, robh, conor+dt, Frank.Li, devicetree
In-Reply-To: <20260617111744.1A0971F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:17:43AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Concurrent regulator operations race on the shared device-level protection register, leading to silent hardware configuration failures.
> - [Medium] The driver module will not auto-load because it lacks a MODULE_ALIAS or platform_device_id table.
> - [Medium] If regmap_read or the unlock sequence fails in the guard constructor, the driver proceeds with the regulator operation on locked registers.
> - [Low] Typo in regulator config assignment assigns dev->of_node to itself instead of rcfg.of_node.
> --
>
> commit e68081b58005a2422d441a5611dac29f8cffe2f4
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub
>
> The NXP P3H2x4x family integrates on-die regulators alongside I3C hub
> functionality. This driver registers the regulators using the MFD
> framework and exposes them via the regulator subsystem.
>
> > --- /dev/null
> > +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
>
> [ ... ]
>
> > +static void p3h2x4x_reg_guard_enter(struct regulator_dev *rdev,
> > + struct p3h2x4x_reg_state *state)
> > +{
> > + state->restore = false;
> > +
> > + if (regmap_read(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + &state->orig))
> > + return;
>
> [Severity: Medium]
> If regmap_read() fails here, the function returns early and swallows the
> error since the guard constructor returns void. Will this cause the caller
> to proceed with regulator operations on locked hardware, potentially
> resulting in silent hardware configuration failures?
>
> If the device ACKs the transaction despite ignoring the write, wouldn't
> the driver incorrectly report success to the regulator core?
>
> > +
> > + if (state->orig != P3H2X4X_REGISTERS_UNLOCK_CODE) {
> > + regmap_write(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + P3H2X4X_REGISTERS_UNLOCK_CODE);
> > + state->restore = true;
> > + }
> > +}
> > +
> > +static void p3h2x4x_reg_guard_exit(struct regulator_dev *rdev,
> > + struct p3h2x4x_reg_state *state)
> > +{
> > + if (state->restore)
> > + regmap_write(rdev->regmap,
> > + P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + state->orig);
> > +}
> > +
> > +DEFINE_LOCK_GUARD_1(p3h2x4x_reg, struct regulator_dev,
> > + p3h2x4x_reg_guard_enter(_T->lock, &_T->state),
> > + p3h2x4x_reg_guard_exit(_T->lock, &_T->state),
> > + struct p3h2x4x_reg_state state);
>
> [Severity: High]
> Does this guard provide mutual exclusion across the four different regulators
> on the same device?
>
> The regmap lock protects individual I/O operations, but since there is no
> device-level lock held across the unlock/modify/lock sequence, can concurrent
> operations on different regulators interleave?
Look like make sense, need use mutex to protect this sequency.
>
> For example, could one thread re-lock the device in p3h2x4x_reg_guard_exit()
> while another thread is just about to write its configuration, causing the
> hardware write to be silently ignored? Is a shared device-level mutex needed
> to serialize these sequences?
>
> [ ... ]
>
> > +static int p3h2x4x_regulator_probe(struct platform_device *pdev)
> > +{
> > + struct p3h2x4x_dev *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> > + struct p3h2x4x_regulator_dev *p3h2x4x_regulator;
> > + struct regulator_config rcfg = { };
> > + struct device *dev = &pdev->dev;
> > + struct regulator_dev *rdev;
> > + int i;
> > +
> > + p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
> > + if (!p3h2x4x_regulator)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, p3h2x4x_regulator);
> > +
> > + p3h2x4x_regulator->regmap = p3h2x4x->regmap;
> > + device_set_of_node_from_dev(dev, dev->parent);
> > +
> > + rcfg.dev = dev;
> > + rcfg.dev->of_node = dev->of_node;
>
> [Severity: Low]
> Should this be rcfg.of_node = dev->of_node? Since rcfg.dev is assigned to dev
> on the previous line, this just assigns dev->of_node to itself.
Although it is small problem, look likes not necesary to set of_node here.
It'd better to fix it at next version.
Frank
>
> > + rcfg.regmap = p3h2x4x_regulator->regmap;
> > + rcfg.driver_data = p3h2x4x_regulator;
>
> [ ... ]
>
> > +static struct platform_driver p3h2x4x_regulator_driver = {
> > + .driver = {
> > + .name = "p3h2x4x-regulator",
> > + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> > + },
> > + .probe = p3h2x4x_regulator_probe,
> > +};
> > +module_platform_driver(p3h2x4x_regulator_driver);
> > +
> > +MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@nxp.com>");
> > +MODULE_AUTHOR("Vikash Bansal <vikash.bansal@nxp.com>");
> > +MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
> > +MODULE_DESCRIPTION("NXP P3H2X4X I3C HUB Regulator driver");
> > +MODULE_LICENSE("GPL");
>
> [Severity: Medium]
> Since the driver is registered as a platform driver with the name
> "p3h2x4x-regulator" but the module is compiled as
> "p3h2840_i3c_hub_regulator.ko", will this module fail to auto-load?
>
> Without a MODULE_ALIAS("platform:p3h2x4x-regulator") or an exported
> platform_device_id table, udev may not be able to load the module when the
> MFD core creates the child platform device.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply
* Re: (subset) [PATCH v2 2/2] leds: pwm-multicolor: Introduce default-intensity property
From: Lee Jones @ 2026-06-17 14:47 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jacek Anaszewski, Sven Schwermer, Jonas Rebmann
Cc: Pavel Machek, linux-leds, devicetree, linux-kernel, kernel
In-Reply-To: <20260605-multicolor-default-v2-2-ed07271df6b0@pengutronix.de>
On Fri, 05 Jun 2026 09:27:48 +0200, Jonas Rebmann wrote:
> Like all LED drivers, pwm-multicolor supports turning on an LED on boot
> by setting linux,default-trigger, e.g. to "default-on". pwm-multicolor
> however scales the brightness of the color-component sub-LEDs with their
> individual intensity value. Since these intensities are
> zero-initialized, on boot a trigger is invisible until colors are set
> from userspace.
>
> [...]
Applied, thanks!
[2/2] leds: pwm-multicolor: Introduce default-intensity property
commit: 69dd3b9d8681085eceeb35459a12d67b848372d4
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v6 2/2] drm/panel: Add Ilitek ILI9488 controller driver
From: sashiko-bot @ 2026-06-17 14:44 UTC (permalink / raw)
To: Igor Reznichenko; +Cc: dri-devel, devicetree, robh, conor+dt
In-Reply-To: <20260617143245.372182-3-igor@reznichenko.net>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for the result of device_get_match_data() leads to a kernel oops.
- [High] Undefined function call and missing DRM panel unregistration leading to Use-After-Free.
--
commit c22f05cd630ecc7dace66102729e2ea4cdfd93fc
Author: Igor Reznichenko <igor@reznichenko.net>
drm/panel: Add Ilitek ILI9488 controller driver
This commit adds support for the Ilitek ILI9488 DSI controller.
It introduces power sequencing and DSI initialization tailored
for the FocusLCDs E35GH-I-MW800-CB panel.
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9488.c b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
> new file mode 100644
> index 0000000000000..7302766034fc9
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
[ ... ]
> +static int ili9488_dsi_probe(struct mipi_dsi_device *dsi)
> +{
> + struct device *dev = &dsi->dev;
> + struct ili9488 *ili;
> + int i, ret;
> +
> + ili = devm_drm_panel_alloc(dev, struct ili9488, panel, &ili9488_funcs,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(ili))
> + return PTR_ERR(ili);
> +
> + ili->desc = device_get_match_data(dev);
> + ili->dsi = dsi;
> +
> + dsi->mode_flags = ili->desc->mode_flags;
[Severity: High]
Can this dereference a NULL pointer if the device is instantiated without
matching firmware data?
The function device_get_match_data() can return NULL, for example via sysfs
or string-based matching. Verifying the return value before accessing
ili->desc->mode_flags would prevent a potential NULL pointer dereference.
[ ... ]
> + ret = drm_panel_of_backlight(&ili->panel);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get backlight\n");
> +
> + ili->panel.prepare_prev_first = true;
> +
> + ret = devm_drm_panel_add(dev, &ili->panel);
[Severity: High]
Does the devm_drm_panel_add() function exist in the kernel, or will this
cause a build failure?
Additionally, if this call is replaced with drm_panel_add(), the driver
lacks a corresponding .remove() callback or a devres action to unregister
the panel with drm_panel_remove(). Since the panel structure is allocated
with devm_drm_panel_alloc() and freed on driver unbind, failing to unregister
the panel will leave it on the DRM core's global panel list, leading to a
Use-After-Free when the list is later accessed.
> + if (ret)
> + return ret;
> +
> + ret = devm_mipi_dsi_attach(dev, dsi);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to attach to DSI host\n");
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617143245.372182-1-igor@reznichenko.net?part=2
^ permalink raw reply
* [PATCH v6 2/2] drm/panel: Add Ilitek ILI9488 controller driver
From: Igor Reznichenko @ 2026-06-17 14:32 UTC (permalink / raw)
To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Lad Prabhakar,
Manivannan Sadhasivam, Kael D'Alcamo, Kever Yang
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260617143245.372182-1-igor@reznichenko.net>
Add support for Ilitek ILI9488 DSI controller which is used in
FocusLCDs E35GH-I-MW800-CB 320x480 MIPI DSI panel. The mode timing
was adjusted after STM32MP157 testing. The previous 14.256 MHz mode
worked on AM62P, but was awkward for STM32 DSI/LTDC clock synthesis.
Signed-off-by: Igor Reznichenko <igor@reznichenko.net>
---
MAINTAINERS | 6 +
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9488.c | 289 +++++++++++++++++++
4 files changed, 305 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9488.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ac87c217ab1f..987635948cde 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8040,6 +8040,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
F: Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
F: drivers/gpu/drm/tiny/ili9486.c
+DRM DRIVER FOR ILITEK ILI9488 PANELS
+M: Igor Reznichenko <igor@reznichenko.net>
+S: Maintained
+F: Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
+F: drivers/gpu/drm/panel/panel-ilitek-ili9488.c
+
DRM DRIVER FOR ILITEK ILI9805 PANELS
M: Michael Trimarchi <michael@amarulasolutions.com>
S: Maintained
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 7450b27622a2..37987e47f3f7 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -284,6 +284,15 @@ config DRM_PANEL_ILITEK_ILI9341
QVGA (240x320) RGB panels. support serial & parallel rgb
interface.
+config DRM_PANEL_ILITEK_ILI9488
+ tristate "Ilitek ILI9488-based panels"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ help
+ Say Y if you want to enable support for panels based on the
+ Ilitek ILI9488 controller.
+
config DRM_PANEL_ILITEK_ILI9805
tristate "Ilitek ILI9805-based panels"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index c2c5cf817116..a1344a0a3fd0 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
obj-$(CONFIG_DRM_PANEL_HYDIS_HV101HD1) += panel-hydis-hv101hd1.o
obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9488) += panel-ilitek-ili9488.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9805) += panel-ilitek-ili9805.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_CORE) += panel-ilitek-ili9806e-core.o
obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9806E_DSI) += panel-ilitek-ili9806e-dsi.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9488.c b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
new file mode 100644
index 000000000000..7302766034fc
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9488.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+#include <video/mipi_display.h>
+
+struct ili9488_desc {
+ const struct drm_display_mode *display_mode;
+ unsigned long mode_flags;
+ enum mipi_dsi_pixel_format format;
+ unsigned int lanes;
+ unsigned int bpc;
+ void (*init_sequence)(struct mipi_dsi_multi_context *ctx);
+};
+
+struct ili9488 {
+ struct drm_panel panel;
+ struct mipi_dsi_device *dsi;
+ struct gpio_desc *reset;
+ struct regulator_bulk_data supplies[2];
+ const struct ili9488_desc *desc;
+ enum drm_panel_orientation orientation;
+};
+
+static const char * const regulator_names[] = {
+ "vci",
+ "iovcc",
+};
+
+static void e35gh_i_mw800cb_init(struct mipi_dsi_multi_context *ctx)
+{
+ /* Gamma control 1,2 */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE0, 0x00, 0x10, 0x14, 0x01, 0x0E, 0x04, 0x33,
+ 0x56, 0x48, 0x03, 0x0C, 0x0B, 0x2B, 0x34, 0x0F);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE1, 0x00, 0x12, 0x18, 0x05, 0x12, 0x06, 0x40,
+ 0x34, 0x57, 0x06, 0x10, 0x0C, 0x3B, 0x3F, 0x0F);
+ /* Power control 1,2 */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC0, 0x0F, 0x0C);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC1, 0x41);
+ /* VCOM Control */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xC5, 0x00, 0x25, 0x80);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x36, 0x48);
+ /* Interface pixel format 18bpp */
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x3A, 0x66);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB0, 0x00);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB1, 0xA0);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB4, 0x02);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xB6, 0x02, 0x02, 0x3B);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xE9, 0x00);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0xF7, 0xA9, 0x51, 0x2C, 0x82);
+ mipi_dsi_dcs_write_seq_multi(ctx, 0x21);
+}
+
+static const struct drm_display_mode e35gh_i_mw800cb_display_mode = {
+ .clock = 14400,
+
+ .hdisplay = 320,
+ .hsync_start = 320 + 60,
+ .hsync_end = 320 + 60 + 20,
+ .htotal = 320 + 60 + 20 + 42,
+
+ .vdisplay = 480,
+ .vsync_start = 480 + 20,
+ .vsync_end = 480 + 20 + 10,
+ .vtotal = 480 + 20 + 10 + 33,
+
+ .width_mm = 48,
+ .height_mm = 73,
+
+ .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+ .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+};
+
+static inline struct ili9488 *panel_to_ili9488(struct drm_panel *panel)
+{
+ return container_of(panel, struct ili9488, panel);
+}
+
+static int ili9488_power_on(struct ili9488 *ili)
+{
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(ili->supplies), ili->supplies);
+ if (ret < 0) {
+ dev_err(&dsi->dev, "regulator bulk enable failed: %d\n", ret);
+ return ret;
+ }
+
+ gpiod_set_value_cansleep(ili->reset, 0);
+ usleep_range(1000, 5000);
+ gpiod_set_value_cansleep(ili->reset, 1);
+ usleep_range(1000, 5000);
+ gpiod_set_value_cansleep(ili->reset, 0);
+ usleep_range(5000, 10000);
+
+ return 0;
+}
+
+static int ili9488_power_off(struct ili9488 *ili)
+{
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ gpiod_set_value_cansleep(ili->reset, 1);
+
+ ret = regulator_bulk_disable(ARRAY_SIZE(ili->supplies), ili->supplies);
+ if (ret)
+ dev_err(&dsi->dev, "regulator bulk disable failed: %d\n", ret);
+
+ return ret;
+}
+
+static int ili9488_activate(struct ili9488 *ili)
+{
+ struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
+
+ if (ili->desc->init_sequence)
+ ili->desc->init_sequence(&ctx);
+
+ mipi_dsi_dcs_exit_sleep_mode_multi(&ctx);
+ mipi_dsi_msleep(&ctx, 120);
+ mipi_dsi_dcs_set_display_on_multi(&ctx);
+
+ return ctx.accum_err;
+}
+
+static int ili9488_prepare(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ int ret;
+
+ ret = ili9488_power_on(ili);
+ if (ret)
+ return ret;
+
+ ret = ili9488_activate(ili);
+ if (ret) {
+ ili9488_power_off(ili);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ili9488_deactivate(struct ili9488 *ili)
+{
+ struct mipi_dsi_multi_context ctx = { .dsi = ili->dsi };
+
+ mipi_dsi_dcs_set_display_off_multi(&ctx);
+ mipi_dsi_dcs_enter_sleep_mode_multi(&ctx);
+ mipi_dsi_msleep(&ctx, 120);
+
+ return ctx.accum_err;
+}
+
+static int ili9488_unprepare(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ struct mipi_dsi_device *dsi = ili->dsi;
+ int ret;
+
+ ili9488_deactivate(ili);
+ ret = ili9488_power_off(ili);
+ if (ret < 0)
+ dev_err(&dsi->dev, "power off failed: %d\n", ret);
+
+ return ret;
+}
+
+static int ili9488_get_modes(struct drm_panel *panel, struct drm_connector *connector)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+ const struct drm_display_mode *mode = ili->desc->display_mode;
+
+ connector->display_info.bpc = ili->desc->bpc;
+
+ return drm_connector_helper_get_modes_fixed(connector, mode);
+}
+
+static enum drm_panel_orientation ili9488_get_orientation(struct drm_panel *panel)
+{
+ struct ili9488 *ili = panel_to_ili9488(panel);
+
+ return ili->orientation;
+}
+
+static const struct drm_panel_funcs ili9488_funcs = {
+ .prepare = ili9488_prepare,
+ .unprepare = ili9488_unprepare,
+ .get_modes = ili9488_get_modes,
+ .get_orientation = ili9488_get_orientation,
+};
+
+static int ili9488_dsi_probe(struct mipi_dsi_device *dsi)
+{
+ struct device *dev = &dsi->dev;
+ struct ili9488 *ili;
+ int i, ret;
+
+ ili = devm_drm_panel_alloc(dev, struct ili9488, panel, &ili9488_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ if (IS_ERR(ili))
+ return PTR_ERR(ili);
+
+ ili->desc = device_get_match_data(dev);
+ ili->dsi = dsi;
+
+ dsi->mode_flags = ili->desc->mode_flags;
+ dsi->format = ili->desc->format;
+ dsi->lanes = ili->desc->lanes;
+
+ ili->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(ili->reset))
+ return dev_err_probe(dev, PTR_ERR(ili->reset),
+ "failed to get reset-gpios\n");
+
+ for (i = 0; i < ARRAY_SIZE(ili->supplies); i++)
+ ili->supplies[i].supply = regulator_names[i];
+
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ili->supplies),
+ ili->supplies);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to get regulators\n");
+
+ ret = of_drm_get_panel_orientation(dev->of_node, &ili->orientation);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get orientation\n");
+
+ ret = drm_panel_of_backlight(&ili->panel);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get backlight\n");
+
+ ili->panel.prepare_prev_first = true;
+
+ ret = devm_drm_panel_add(dev, &ili->panel);
+ if (ret)
+ return ret;
+
+ ret = devm_mipi_dsi_attach(dev, dsi);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to attach to DSI host\n");
+
+ return 0;
+}
+
+static const struct ili9488_desc e35gh_i_mw800cb_desc = {
+ .init_sequence = e35gh_i_mw800cb_init,
+ .display_mode = &e35gh_i_mw800cb_display_mode,
+ .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS,
+ .format = MIPI_DSI_FMT_RGB666_PACKED,
+ .lanes = 1,
+ .bpc = 6,
+};
+
+static const struct of_device_id ili9488_of_match[] = {
+ { .compatible = "focuslcds,e35gh-i-mw800cb", .data = &e35gh_i_mw800cb_desc },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, ili9488_of_match);
+
+static struct mipi_dsi_driver ili9488_dsi_driver = {
+ .probe = ili9488_dsi_probe,
+ .driver = {
+ .name = "ili9488-dsi",
+ .of_match_table = ili9488_of_match,
+ },
+};
+module_mipi_dsi_driver(ili9488_dsi_driver);
+
+MODULE_AUTHOR("Igor Reznichenko <igor@reznichenko.net>");
+MODULE_DESCRIPTION("Ilitek ILI9488 Controller Driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v6 1/2] dt-bindings: display: panel: Add Ilitek ILI9488 panel controller
From: Igor Reznichenko @ 2026-06-17 14:32 UTC (permalink / raw)
To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Lad Prabhakar,
Manivannan Sadhasivam, Kael D'Alcamo, Kever Yang
Cc: dri-devel, devicetree, linux-kernel, Conor Dooley
In-Reply-To: <20260617143245.372182-1-igor@reznichenko.net>
Add binding for the Ilitek ILI9488 panel controller which is found on
the FocusLCDs E35GH-I-MW800-CB MIPI DSI panel. Add "focuslcds" to
vendor-prefixes.yaml as it's a brandname and a website
(https://focuslcds.com/) for Focus Display Solutions, Inc.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Igor Reznichenko <igor@reznichenko.net>
---
.../display/panel/ilitek,ili9488.yaml | 63 +++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
2 files changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
new file mode 100644
index 000000000000..ea7449273022
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/ilitek,ili9488.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ILI9488 based MIPI-DSI panels
+
+maintainers:
+ - Igor Reznichenko <igor@reznichenko.net>
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - focuslcds,e35gh-i-mw800cb
+ - const: ilitek,ili9488
+
+ reg:
+ maxItems: 1
+
+ vci-supply: true
+ iovcc-supply: true
+
+required:
+ - compatible
+ - reg
+ - vci-supply
+ - iovcc-supply
+ - reset-gpios
+ - backlight
+ - port
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ panel@0 {
+ compatible = "focuslcds,e35gh-i-mw800cb", "ilitek,ili9488";
+ reg = <0>;
+ vci-supply = <®_vci_panel>;
+ iovcc-supply = <®_iovcc_panel>;
+ reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+ backlight = <&pwm_bl>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&dsi_out>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6d742a317642..db0dc4b44ed5 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -602,6 +602,8 @@ patternProperties:
description: Flipkart Inc.
"^focaltech,.*":
description: FocalTech Systems Co.,Ltd
+ "^focuslcds,.*":
+ description: Focus Display Solutions, Inc.
"^forlinx,.*":
description: Baoding Forlinx Embedded Technology Co., Ltd.
"^foursemi,.*":
--
2.43.0
^ permalink raw reply related
* [PATCH v6 0/2] drm/panel: Add support for the FocusLCDs E35GH-I-MW800CB
From: Igor Reznichenko @ 2026-06-17 14:32 UTC (permalink / raw)
To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner, Lad Prabhakar,
Manivannan Sadhasivam, Kael D'Alcamo, Kever Yang
Cc: dri-devel, devicetree, linux-kernel
Add support for E35GH-I-MW800CB 320x480 MIPI DSI panel by FocusLCDs.
The panel uses Ilitek ILI9488 driver IC in DSI mode specifically.
ILI9488 also appears in DBI/SPI panels, but those require different bus drivers.
This panel driver has been tested on STM32MP157D-DK1 in addition to TI AM62P EVK.
The panel works in video mode on both platforms. Additional panels using
ILI9488 DSI can be added later with their own timings.
v6: Rebased
No functional changes
v5: Rebased
Switched to devm_drm_panel_add() and devm_mipi_dsi_attach()
Dropped not required anymore .remove callback
Added display_info.bpc=6 initialization
Changed 0x21 DCS command to send 0 arguments as per spec
https://lore.kernel.org/all/20260529062836.203990-1-igor@reznichenko.net/
v4: Rebased
Tested 7.1-rc5 on STM32MP157D-DK1
Adjusted E35GH-I-MW800CB mode timing to a 14.4 MHz pixel clock with
slightly increased blanking to make it more synthesizable across DSI hosts
https://lore.kernel.org/all/20260527062300.88928-1-igor@reznichenko.net/
v3: Fixed missing Reviewed-by tag
https://lore.kernel.org/all/20260204060114.345219-1-igor@reznichenko.net/
v2: Added comment explaining the "focuslcds" vendor prefix
https://lore.kernel.org/all/20260203054121.335441-1-igor@reznichenko.net/
v1:
https://lore.kernel.org/all/20260131034101.307486-1-igor@reznichenko.net/
Igor Reznichenko (2):
dt-bindings: display: panel: Add Ilitek ILI9488 panel controller
drm/panel: Add Ilitek ILI9488 controller driver
.../display/panel/ilitek,ili9488.yaml | 63 ++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 6 +
drivers/gpu/drm/panel/Kconfig | 9 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9488.c | 289 ++++++++++++++++++
6 files changed, 370 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9488.yaml
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9488.c
base-commit: 4af021a977735420e8fe18d64bef18ad0608d981
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2 2/6] iommu/arm-smmu: Add interconnect bandwidth voting support
From: Bibek Kumar Patro @ 2026-06-17 14:26 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <sdm7sqiokmsgczeat2mxch42ois5rwabav6c5fm7abct2xoepf@j3kraqrjvpoc>
On 6/16/2026 5:51 AM, Dmitry Baryshkov wrote:
> On Mon, Jun 15, 2026 at 06:36:51PM +0530, Bibek Kumar Patro wrote:
>>
>>
>> On 6/8/2026 7:25 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 26, 2026 at 08:12:03PM +0530, Bibek Kumar Patro wrote:
>>>> On some SoCs the SMMU registers require an active interconnect
>>>> bandwidth vote to be accessible. While other clients typically
>>>> satisfy this requirement implicitly, certain corner cases (e.g.
>>>> during sleep/wakeup transitions) can leave the SMMU without a
>>>> vote, causing intermittent register access failures.
>>>>
>>>> Add support for an optional interconnect path to the arm-smmu
>>>> driver and vote for bandwidth while the SMMU is active. The path
>>>> is acquired from DT if present and ignored otherwise.
>>>>
>>>> The bandwidth vote is enabled before accessing SMMU registers
>>>> during probe and runtime resume, and released during runtime
>>>> suspend and on error paths.
>>>>
>>>> Generally, from an architectural perspective, GEM_NOC and DDR are
>>>> expected to have an active vote whenever the adreno_smmu block is
>>>> powered on. In most common use cases, this requirement is implicitly
>>>> satisfied because other GPU-related clients (for example, the GMU
>>>> device) already hold a GEM_NOC vote when adreno_smmu is enabled.
>>>>
>>>> However, there are certain corner cases, such as during sleep/wakeup
>>>> transitions, where the GEM_NOC vote can be removed before adreno_smmu
>>>> is powered down. If adreno_smmu is then accessed while the interconnect
>>>> vote is missing, it can lead to the observed failures. Because of the
>>>> precise ordering involved, this scenario is difficult to reproduce
>>>> consistently.
>>>> (also GDSC is involved in adreno usecases can have an independent vote)
>>>>
>>>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>>> ---
>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 57 +++++++++++++++++++++++++++++++++--
>>>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 ++
>>>> 2 files changed, 57 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>> index 0bd21d206eb3e75c3b9fb1364cdc92e82c5aa499..07c7e44ec6a5bd1488f00f87d859a20495e46601 100644
>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>>> @@ -53,6 +53,11 @@
>>>> #define MSI_IOVA_BASE 0x8000000
>>>> #define MSI_IOVA_LENGTH 0x100000
>>>> +/* Interconnect bandwidth vote values for the SMMU register access path */
>>>> +#define ARM_SMMU_ICC_AVG_BW 0
>>>> +#define ARM_SMMU_ICC_PEAK_BW_HIGH 1000
>>>
>>> totally random numbers, which might be different for non-Qualcomm platform.
>>>
>>
>> Ideally, any non-zero value would be enough to keep the path active.
>
> This is true for Qualcomm devices. However, you are adding this to a
> generic code.
>
>> Here 1 Would be enough to keep the path active, but might be too small to
>> reliably keep the bus active.
>> Other is UINT_MAX, which will reliably keep the bus active but might cause a
>> power penalty.
>>
>> #define ARM_SMMU_ICC_PEAK_BW_HIGH UINT_MAX
>>
>> seems to be suitable here to reliably keep the bus active by BCM
>> for both Qualcomm and non-Qualcomm platforms (with some power penalty).
>>
>> LMK, if you feel otherwise.
>
> Shift it to the qcom instance or provide platform-specific values? (My
> preference would be towards the first solution).
>
To support platform-specific values, we may need to introduce a
LUT-based approach in the driver. (Bandwidth voting values cannot be
placed in device-tree property IIRC ?)
Currently, all Qualcomm platforms use 0x1000 for SMMU ICC voting. I
can evaluate if this could be moved to a Qualcomm-specific
implementation.
To clarify, this applies only to the bandwidth values.
Since the ICC path itself can remain part of struct arm_smmu_device,
similar to clocks and IRQs, as it represents common infrastructure
required for the SMMU device.
Thanks & regards,
Bibek
>>
>>
>>>> +#define ARM_SMMU_ICC_PEAK_BW_LOW 0
>>>> +
>>>> static int force_stage;
>>>> module_param(force_stage, int, S_IRUGO);
>>>> MODULE_PARM_DESC(force_stage,
>
^ permalink raw reply
* Re: [PATCH 1/4] arm64: dts: qcom: pm4125: Add VADC and temp alarm nodes
From: Rakesh Kota @ 2026-06-17 14:17 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jishnu Prakash, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel, Kamal Wadhwa
In-Reply-To: <y3b3i4p6nr3tj5ds3a5zispbrxt7pzucoswh5grtthmj2m5en2@ucniw6w45572>
On Mon, Jun 15, 2026 at 12:41:53AM +0300, Dmitry Baryshkov wrote:
> On Sat, Jun 13, 2026 at 01:39:21PM +0530, Jishnu Prakash wrote:
> > Add VADC node with some channels under it, for voltage and
> > temperature readings. Add temperature alarm node, used for
> > PMIC thermal mitigation.
> >
> > Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/pm4125.dtsi | 77 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 77 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > index 542e8fe030da..7113504d5941 100644
> > --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > @@ -65,6 +65,83 @@ pm4125_typec: typec@1500 {
> > status = "disabled";
> > };
> >
> > + pm4125_tz: temp-alarm@2400 {
> > + compatible = "qcom,spmi-temp-alarm";
> > + reg = <0x2400>;
> > + interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
> > + io-channels = <&pm4125_adc ADC5_DIE_TEMP>;
> > + io-channel-names = "thermal";
> > + #thermal-sensor-cells = <0>;
> > + status = "disabled";
>
> Why would it be disabled by default?
>
Ok, we will update in upcoming patch.
> > + };
> > +
> > + pm4125_adc: adc@3100 {
> > + compatible = "qcom,spmi-adc5";
> > + reg = <0x3100>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #io-channel-cells = <1>;
> > + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> > + status = "disabled";
>
> Again, why?
>
Ok, we will update in upcoming patch.
regards
Rakesh kota
> > +
> > + /* Channel nodes */
> > + channel@0 {
> > + reg = <ADC5_REF_GND>;
> > + label = "ref_gnd";
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@1 {
> > + reg = <ADC5_1P25VREF>;
> > + label = "vref_1p25";
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@6 {
> > + reg = <ADC5_DIE_TEMP>;
> > + label = "die_temp";
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@7 {
> > + reg = <ADC5_USB_IN_I>;
> > + label = "usb_in_i_uv";
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@8 {
> > + reg = <ADC5_USB_IN_V_16>;
> > + label = "usb_in_v_div_16";
> > + qcom,pre-scaling = <1 16>;
> > + };
> > +
> > + channel@9 {
> > + reg = <ADC5_CHG_TEMP>;
> > + label = "chg_temp";
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@4b {
> > + reg = <ADC5_BAT_ID_100K_PU>;
> > + label = "bat_id";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@83 {
> > + reg = <ADC5_VPH_PWR>;
> > + label = "vph_pwr";
> > + qcom,pre-scaling = <1 3>;
> > + };
> > +
> > + channel@84 {
> > + reg = <ADC5_VBAT_SNS>;
> > + label = "vbat_sns";
> > + qcom,pre-scaling = <1 3>;
> > + };
> > + };
> > +
> > rtc@6000 {
> > compatible = "qcom,pm8941-rtc";
> > reg = <0x6000>, <0x6100>;
> >
> > --
> > 2.43.0
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* Re: [PATCH 2/4] arm64: dts: qcom: pm8005: Add temp alarm node
From: Rakesh Kota @ 2026-06-17 14:16 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jishnu Prakash, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel, Kamal Wadhwa
In-Reply-To: <633bh5nvxafrwnga5mdvvfdkazfgmrkt2jgn36dh7u5dlgjkgs@vewjc6tkgytq>
On Mon, Jun 15, 2026 at 12:42:38AM +0300, Dmitry Baryshkov wrote:
> On Sat, Jun 13, 2026 at 01:39:22PM +0530, Jishnu Prakash wrote:
> > Add temperature alarm node, used for PMIC thermal mitigation.
> >
> > Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/pm8005.dtsi | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pm8005.dtsi b/arch/arm64/boot/dts/qcom/pm8005.dtsi
> > index 0f0ab2da8305..180809a0ee73 100644
> > --- a/arch/arm64/boot/dts/qcom/pm8005.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm8005.dtsi
> > @@ -11,6 +11,14 @@ pm8005_lsid0: pmic@4 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > + pm8005_tz: temp-alarm@2400 {
> > + compatible = "qcom,spmi-temp-alarm";
> > + reg = <0x2400>;
> > + interrupts = <0x4 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
> > + #thermal-sensor-cells = <0>;
> > + status = "disabled";
>
> Missing thermal zone, also enable it by default.
>
we will update in upcoming patch.
regard
Rakesh Kota
> > + };
> > +
> > pm8005_gpios: gpio@c000 {
> > compatible = "qcom,pm8005-gpio", "qcom,spmi-gpio";
> > reg = <0xc000>;
> >
> > --
> > 2.43.0
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* Re: [PATCH v6 02/10] ACPI: APEI: GHES: move CPER read helpers
From: sashiko-bot @ 2026-06-17 14:15 UTC (permalink / raw)
To: Ahmed Tiba; +Cc: conor+dt, devicetree, linux-cxl, robh
In-Reply-To: <20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-2-91f725174aa0@arm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] A double fetch (TOCTOU) vulnerability when reading the CPER error status block from firmware memory can lead to out-of-bounds reads.
- [High] ghes_ack_error() incorrectly shifts read_ack_preserve and read_ack_write by the register bit_offset, unconditionally clearing bits below bit_offset and potentially clobbering other hardware state in the register.
--
commit 6a836df1ccfeca08e11c00049d33e954958fa36f
Author: Ahmed Tiba <ahmed.tiba@arm.com>
ACPI: APEI: GHES: move CPER read helpers
This commit relocates the CPER buffer mapping, peek, and clear helpers
from ghes.c into ghes_cper.c so they can be shared with other
firmware-first providers. It only shuffles code without changing
behavior.
Reviewed-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Ahmed Tiba <ahmed.tiba@arm.com>
> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
> new file mode 100644
> index 0000000000000..b365c42efce49
> --- /dev/null
> +++ b/drivers/acpi/apei/ghes_cper.c
> @@ -0,0 +1,196 @@
[ ... ]
> +static void ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
> +{
> + int rc;
> + u64 val = 0;
> +
> + rc = apei_read(&val, &gv2->read_ack_register);
> + if (rc)
> + return;
> +
> + val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
> + val |= gv2->read_ack_write << gv2->read_ack_register.bit_offset;
[Severity: High]
This is a pre-existing issue, but does this code unintentionally clobber the
lower bits of the hardware register?
When ghes_ack_error() masks val using:
val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
Any bits in val below bit_offset are unconditionally cleared to zero.
If there is hardware state in those lower bits, wouldn't writing back the
modified val via apei_write() overwrite and destroy it?
> +
> + apei_write(val, &gv2->read_ack_register);
> +}
[ ... ]
> +int ghes_read_estatus(struct ghes *ghes,
> + struct acpi_hest_generic_status *estatus,
> + u64 *buf_paddr, enum fixed_addresses fixmap_idx)
> +{
> + int rc;
> +
> + rc = __ghes_peek_estatus(ghes, estatus, buf_paddr, fixmap_idx);
> + if (rc)
> + return rc;
> +
> + rc = __ghes_check_estatus(ghes, estatus);
> + if (rc)
> + return rc;
> +
> + return __ghes_read_estatus(estatus, *buf_paddr, fixmap_idx,
> + cper_estatus_len(estatus));
[Severity: High]
This isn't a bug introduced by this patch, but is there a potential double
fetch issue in the error status read path?
Looking at the sequence in ghes_read_estatus(), we first fetch and validate the
header:
__ghes_peek_estatus() copies the header from physical memory into estatus.
__ghes_check_estatus() validates estatus->data_length.
Then we call __ghes_read_estatus() which copies from the same physical memory
again, overwriting the previously validated estatus header:
__ghes_read_estatus(...) {
ghes_copy_tofrom_phys(estatus, buf_paddr, buf_len, 1, fixmap_idx);
if (cper_estatus_check(estatus)) {
...
}
If the underlying firmware memory is modified between the peek and the read,
the local estatus will get a new, unvalidated data_length.
Since cper_estatus_check() relies on data_length to iterate over the sections,
could this new unvalidated length cause an out-of-bounds read?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=2
^ permalink raw reply
* Re: [PATCH v6 06/10] ACPI: APEI: GHES: move CXL CPER helpers
From: sashiko-bot @ 2026-06-17 14:14 UTC (permalink / raw)
To: Ahmed Tiba; +Cc: robh, devicetree, linux-cxl, conor+dt
In-Reply-To: <20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-6-91f725174aa0@arm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Inconsistent spinlock usage between process context and interrupt context for `cxl_cper_work_lock` and `cxl_cper_prot_err_work_lock` causes a potential deadlock.
- [High] Unconditional fixed-size copy leads to out-of-bounds read if the ACPI payload is short.
- [High] Lack of bounds checking for DVSEC length leads to out-of-bounds read in CXL protocol error handling.
--
commit bb34e6b6044e94825caae9bb7ad029942aef5c28
Author: Ahmed Tiba <ahmed.tiba@arm.com>
ACPI: APEI: GHES: move CXL CPER helpers
Move the CXL CPER handling paths out of ghes.c and into ghes_cper.c so the
helpers can be reused. The code is moved as-is, with the public
prototypes updated so GHES keeps calling into the new translation unit.
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
[ ... ]
> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
> index b26943eafd792..66bf1af4db00b 100644
> --- a/drivers/acpi/apei/ghes_cper.c
> +++ b/drivers/acpi/apei/ghes_cper.c
[ ... ]
> @@ -336,6 +339,138 @@ void ghes_defer_non_standard_event(struct acpi_hest_generic_data *gdata,
[ ... ]
> +void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> + int severity)
> +{
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
> + struct cxl_cper_prot_err_work_data wd;
> +
> + if (cxl_cper_sec_prot_err_valid(prot_err))
> + return;
> +
> + guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> +
> + if (!cxl_cper_prot_err_work)
> + return;
> +
> + if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
[Severity: High]
This is a pre-existing issue, but could this setup step read out of bounds?
The cxl_cper_setup_prot_err_work_data() function uses the firmware-provided
dvsec_len to compute a pointer for copying the RAS capability registers:
ghes_helpers.c:cxl_cper_setup_prot_err_work_data() {
...
cap_start = dvsec_start + prot_err->dvsec_len;
memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));
Since cxl_cper_post_prot_err() and its callers do not validate that
dvsec_len falls within the maximum bounds of the ACPI payload
(gdata->error_data_length), could a malformed firmware record cause cap_start
to point beyond the valid buffer?
> + return;
> +
> + if (!kfifo_put(&cxl_cper_prot_err_fifo, wd)) {
> + pr_err_ratelimited("CXL CPER kfifo overflow\n");
> + return;
> + }
> +
> + schedule_work(cxl_cper_prot_err_work);
> +#endif
> +}
> +
> +int cxl_cper_register_prot_err_work(struct work_struct *work)
> +{
> + if (cxl_cper_prot_err_work)
> + return -EINVAL;
> +
> + guard(spinlock)(&cxl_cper_prot_err_work_lock);
[Severity: High]
This isn't a bug introduced by this patch, but is there a potential deadlock
between process context and interrupt context here?
The registration function acquires the lock with guard(spinlock), leaving
local interrupts enabled.
If a hardware interrupt occurs while this lock is held, and the interrupt
handler calls cxl_cper_post_prot_err() which attempts to acquire the same
lock using guard(spinlock_irqsave):
cxl_cper_post_prot_err() {
...
guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
Could this cause the interrupt handler to spin forever on the local CPU?
> + cxl_cper_prot_err_work = work;
> + return 0;
> +}
[ ... ]
> +void cxl_cper_post_event(enum cxl_event_type event_type,
> + struct cxl_cper_event_rec *rec)
> +{
> + struct cxl_cper_work_data wd;
> +
> + if (rec->hdr.length <= sizeof(rec->hdr) ||
> + rec->hdr.length > sizeof(*rec)) {
> + pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
> + rec->hdr.length);
> + return;
> + }
> +
> + if (!(rec->hdr.validation_bits & CPER_CXL_COMP_EVENT_LOG_VALID)) {
> + pr_err(FW_WARN "CXL CPER invalid event\n");
> + return;
> + }
> +
> + guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +
> + if (!cxl_cper_work)
> + return;
> +
> + wd.event_type = event_type;
> + memcpy(&wd.rec, rec, sizeof(wd.rec));
[Severity: High]
This is a pre-existing issue, but can this copy read out of bounds if the
ACPI payload is unexpectedly short?
The memcpy unconditionally copies sizeof(wd.rec) bytes from the
firmware-provided payload.
Since gdata->error_data_length is never verified to be at least the size
of the record before being passed to this function by
ghes_cper_handle_status(), would a malformed payload smaller than the
struct cause an uncontrolled read of kernel memory?
> +
> + if (!kfifo_put(&cxl_cper_fifo, wd)) {
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add PMIC thermal support for Shikra CQ2390M SoM platform
From: Rakesh Kota @ 2026-06-17 14:14 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jishnu Prakash, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel, Kamal Wadhwa, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <ry5y452gxtnjdzux32saufyvolrkvudlaldmwxdvkcqgrkmaxn@53ikkvwsazkw>
On Mon, Jun 15, 2026 at 12:45:03AM +0300, Dmitry Baryshkov wrote:
> On Sat, Jun 13, 2026 at 01:39:23PM +0530, Jishnu Prakash wrote:
> > Add ADC channels for system thermistors, used for thermal mitigation.
> > Add ADC thermal bridge nodes for pa/quiet/msm thermistors. Enable temperature
> > alarm nodes for PM4125 and PM8005. Add thermal zones for temp-alarm devices
> > and system thermistors exposed as thermal bridge nodes.
> >
> > Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi | 184 +++++++++++++++++++++++++++
> > 1 file changed, 184 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > index dc3861489f64..c6c09d773abe 100644
> > --- a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> > #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> >
> > #include "shikra.dtsi"
> > @@ -27,9 +28,184 @@ key-volume-up {
> > linux,can-disable;
> > };
> > };
> > +
> > + pm4125_msm_therm_bridge: pm4125-msm-therm-bridge {
>
> Generic node names, please. See, how other platforms name them.
we will update in upcoming patch.
>
> > + compatible = "generic-adc-thermal";
> > + io-channels = <&pm4125_adc ADC5_AMUX_THM3_100K_PU>;
> > + io-channel-names = "sensor-channel";
> > + #thermal-sensor-cells = <0>;
> > + };
> > +
> > + pm4125_pa_therm_bridge: pm4125-pa-therm-bridge {
> > + compatible = "generic-adc-thermal";
> > + io-channels = <&pm4125_adc ADC5_AMUX_THM1_100K_PU>;
> > + io-channel-names = "sensor-channel";
> > + #thermal-sensor-cells = <0>;
> > + };
> > +
> > + pm4125_quiet_therm_bridge: pm4125-quiet-therm-bridge {
> > + compatible = "generic-adc-thermal";
> > + io-channels = <&pm4125_adc ADC5_AMUX_THM2_100K_PU>;
> > + io-channel-names = "sensor-channel";
> > + #thermal-sensor-cells = <0>;
> > + };
> > +
> > + thermal-zones {
> > + pm4125-thermal {
> > + polling-delay-passive = <100>;
> > + polling-delay = <0>;
> > + thermal-sensors = <&pm4125_tz>;
>
> This should be a part of the pm4125.dtsi
>
ok, we will move it to pm4125.dtsi.
> > +
> > + trips {
> > + pm4125_trip0: trip0 {
> > + temperature = <105000>;
> > + hysteresis = <0>;
> > + type = "passive";
> > + };
> > +
> > + pm4125_trip1: trip1 {
> > + temperature = <125000>;
> > + hysteresis = <0>;
> > + type = "hot";
> > + };
> > +
> > + pm4125_trip2: trip2 {
> > + temperature = <155000>;
> > + hysteresis = <0>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + pm8005-thermal {
> > + polling-delay-passive = <0>;
> > + polling-delay = <0>;
> > + thermal-sensors = <&pm8005_tz>;
>
> pm8005.dtsi
>
ok, we will move it to pm8005.dtsi.
> > +
> > + trips {
> > + pm8005_trip0: trip0 {
> > + temperature = <105000>;
> > + hysteresis = <0>;
> > + type = "passive";
> > + };
> > +
> > + pm8005_trip1: trip1 {
> > + temperature = <125000>;
> > + hysteresis = <0>;
> > + type = "passive";
> > + };
> > +
> > + pm8005_trip2: trip2 {
> > + temperature = <145000>;
> > + hysteresis = <0>;
> > + type = "passive";
> > + };
> > + };
> > + };
> > +
> > + sys-1-thermal {
> > + polling-delay-passive = <2000>;
> > + polling-delay = <0>;
> > + thermal-sensors = <&pm4125_pa_therm_bridge>;
> > +
> > + trips {
> > + active-config0 {
> > + temperature = <125000>;
> > + hysteresis = <1000>;
> > + type = "passive";
>
> Passive cooling at 125°C sounds very strange. Especially without any
> cooling device attached.
>
Yes, that's correct. Since no cooling device is present, I'll update
active-config0 to a 'hot' trip type at 80°C with 2°C hysteresis for
thermal monitoring. This will be included in the upcoming patch.
active-config0 {
temperature = <80000>;
hysteresis = <2000>;
type = "hot";
};
> > + };
> > + };
> > + };
> > +
> > + sys-2-thermal {
> > + polling-delay-passive = <2000>;
> > + polling-delay = <0>;
> > + thermal-sensors = <&pm4125_quiet_therm_bridge>;
> > +
> > + trips {
> > + active-config0 {
> > + temperature = <125000>;
> > + hysteresis = <1000>;
> > + type = "passive";
> > + };
> > + };
> > + };
> > +
> > + sys-3-thermal {
> > + polling-delay-passive = <2000>;
> > + polling-delay = <0>;
> > + thermal-sensors = <&pm4125_msm_therm_bridge>;
> > +
> > + trips {
> > + active-config0 {
> > + temperature = <125000>;
> > + hysteresis = <1000>;
> > + type = "passive";
> > + };
> > + };
> > + };
> > + };
> > +};
> > +
> > +&pm4125_adc {
> > + pinctrl-0 = <&pm4125_adc_gpio5_default>, <&pm4125_adc_gpio6_default>;
> > + pinctrl-names = "default";
> > + status = "okay";
>
> Empty line before status.
>
We will update the upcoming patch.
regards
Rakesh Kota
> > +
> > + channel@4d {
> > + reg = <ADC5_AMUX_THM1_100K_PU>;
> > + label = "pa_therm";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@4e {
> > + reg = <ADC5_AMUX_THM2_100K_PU>;
> > + label = "quiet_therm";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@4f {
> > + reg = <ADC5_AMUX_THM3_100K_PU>;
> > + label = "msm_therm";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@54 {
> > + reg = <ADC5_GPIO3_100K_PU>;
> > + label = "chgr_skin";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > +
> > + channel@55 {
> > + reg = <ADC5_GPIO4_100K_PU>;
> > + label = "gnss_therm";
> > + qcom,ratiometric;
> > + qcom,hw-settle-time = <200>;
> > + qcom,pre-scaling = <1 1>;
> > + };
> > };
> >
> > &pm4125_gpios {
> > + pm4125_adc_gpio5_default: pm4125-adc-gpio5-state {
> > + pins = "gpio5";
> > + function = PMIC_GPIO_FUNC_NORMAL;
> > + bias-high-impedance;
> > + };
> > +
> > + pm4125_adc_gpio6_default: pm4125-adc-gpio6-state {
> > + pins = "gpio6";
> > + function = PMIC_GPIO_FUNC_NORMAL;
> > + bias-high-impedance;
> > + };
> > +
> > vol_up_n: vol-up-n-state {
> > pins = "gpio9";
> > function = PMIC_GPIO_FUNC_NORMAL;
> > @@ -45,10 +221,18 @@ &pm4125_resin {
> > status = "okay";
> > };
> >
> > +&pm4125_tz {
> > + status = "okay";
> > +};
> > +
> > &pm8005_regulators {
> > status = "disabled";
> > };
> >
> > +&pm8005_tz {
> > + status = "okay";
> > +};
> > +
> > &rpm_requests {
> > regulators {
> > compatible = "qcom,rpm-pm2250-regulators";
> >
> > --
> > 2.43.0
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply
* Re: [PATCH v6 08/10] ACPI: APEI: share GHES CPER helpers
From: sashiko-bot @ 2026-06-17 14:11 UTC (permalink / raw)
To: Ahmed Tiba; +Cc: linux-cxl, devicetree, robh, conor+dt
In-Reply-To: <20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-8-91f725174aa0@arm.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Incorrect size subtraction in `ghes_handle_arm_hw_error()` inflates the remaining payload length, leading to out-of-bounds reads.
- [High] Memory allocated for aer_info is permanently leaked if the aer_recover_ring kfifo overflows.
- [High] The `queued` boolean flag is overwritten on each loop iteration, which can unnecessarily force a task to be killed with SIGBUS.
- [High] Memory allocated for PCI AER recovery is leaked if the associated PCI device is not found.
- [High] Potential deadlock due to missing IRQ disable in CXL work registration functions.
--
commit 552b224fbe0e1a9a6cbfb11eef23d5902aa40cf8
Author: Ahmed Tiba <ahmed.tiba@arm.com>
ACPI: APEI: share GHES CPER helpers
This patch moves the GHES CPER helper routines from ghes.c to ghes_cper.c to
allow them to be shared with other firmware-first providers. It also
serializes the ghes_estatus_pool initialization to prevent race conditions
and pool reuse issues.
> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
> index 66bf1af4db00b..460fe12b6513c 100644
> --- a/drivers/acpi/apei/ghes_cper.c
> +++ b/drivers/acpi/apei/ghes_cper.c
> @@ -36,9 +47,449 @@
[ ... ]
> +bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
> + int sev, bool sync)
> +{
> + struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
> + int flags = sync ? MF_ACTION_REQUIRED : 0;
> + int length = gdata->error_data_length;
> + char error_type[120];
> + bool queued = false;
> + int sec_sev, i;
> + char *p;
> +
> + sec_sev = ghes_severity(gdata->error_severity);
> + if (length >= sizeof(*err)) {
> + log_arm_hw_error(err, sec_sev);
> + } else {
> + pr_warn(FW_BUG "arm error length: %d\n", length);
> + pr_warn(FW_BUG "length is too small\n");
> + pr_warn(FW_BUG "firmware-generated error record is incorrect\n");
> + return false;
> + }
> +
> + if (sev != GHES_SEV_RECOVERABLE || sec_sev != GHES_SEV_RECOVERABLE)
> + return false;
> +
> + p = (char *)(err + 1);
> + length -= sizeof(err);
[Severity: Critical]
This is a pre-existing issue, but does this subtract the size of the pointer
instead of the struct?
In drivers/acpi/apei/ghes_cper.c:ghes_handle_arm_hw_error(), err is a
pointer to struct cper_sec_proc_arm. Subtracting sizeof(err) subtracts
8 bytes instead of the actual structure size (40 bytes).
This artificially inflates the remaining length by 32 bytes. During the
subsequent loop, the boundary check if (length < sizeof(*err_info)) can
evaluate to true even when p has advanced past the end of the gdata
buffer, potentially causing out-of-bounds reads.
Should this use sizeof(*err) instead?
> +
> + for (i = 0; i < err->err_info_num; i++) {
> + struct cper_arm_err_info *err_info;
> + bool is_cache, has_pa;
> +
> + /* Ensure we have enough data for the error info header */
> + if (length < sizeof(*err_info))
> + break;
> +
> + err_info = (struct cper_arm_err_info *)p;
> +
> + /* Validate the claimed length before using it */
> + length -= err_info->length;
> + if (length < 0)
> + break;
> +
> + is_cache = err_info->type & CPER_ARM_CACHE_ERROR;
> + has_pa = (err_info->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR);
> +
> + /*
> + * The field (err_info->error_info & BIT(26)) is fixed to set to
> + * 1 in some old firmware of HiSilicon Kunpeng920. We assume that
> + * firmware won't mix corrected errors in an uncorrected section,
> + * and don't filter out 'corrected' error here.
> + */
> + if (is_cache && has_pa) {
> + queued = ghes_do_memory_failure(err_info->physical_fault_addr, flags);
[Severity: High]
This isn't a regression introduced by this patch, but is it possible for
queued to be overwritten if there are multiple error infos?
In drivers/acpi/apei/ghes_cper.c:ghes_handle_arm_hw_error(), if an early error
info successfully queues memory failure work, but a subsequent one does not,
queued is overwritten to false.
Later in the caller, this might cause if (sync && !queued) to evaluate to true,
force-killing the task with SIGBUS even though the memory error was
actually queued for recovery.
Should this use queued |= ghes_do_memory_failure(...) to accumulate the
results?
> + p += err_info->length;
> + continue;
> + }
[ ... ]
> +void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
> +{
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
> + struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
> +
> + if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> + pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
> + unsigned int devfn;
> + int aer_severity;
> + u8 *aer_info;
> +
> + devfn = PCI_DEVFN(pcie_err->device_id.device,
> + pcie_err->device_id.function);
> + aer_severity = cper_severity_to_aer(gdata->error_severity);
> +
> + /*
> + * If firmware reset the component to contain
> + * the error, we must reinitialize it before
> + * use, so treat it as a fatal AER error.
> + */
> + if (gdata->flags & CPER_SEC_RESET)
> + aer_severity = AER_FATAL;
> +
> + aer_info = (void *)gen_pool_alloc(ghes_estatus_pool,
> + sizeof(struct aer_capability_regs));
> + if (!aer_info)
> + return;
> + memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));
> +
> + aer_recover_queue(pcie_err->device_id.segment,
> + pcie_err->device_id.bus,
> + devfn, aer_severity,
> + (struct aer_capability_regs *)
> + aer_info);
[Severity: High]
This is a pre-existing issue, but does aer_info leak if aer_recover_queue()
fails to add the entry to the kfifo or if the device isn't found?
In drivers/pci/pcie/aer.c:aer_recover_queue():
if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
&aer_recover_ring_lock))
schedule_work(&aer_recover_work);
else
pr_err(...);
If the kfifo overflows, it drops the entry and returns without freeing the
buffer allocated from the fixed-size ghes_estatus_pool.
Additionally, in drivers/pci/pcie/aer.c:aer_recover_work_func(), if
pci_get_domain_bus_and_slot() returns NULL (e.g. if the device fell off the
bus), the code executes a continue statement which skips the
ghes_estatus_pool_region_free() call, leaking the memory.
Since ghes_estatus_pool is a fixed-size pool, repeated leaks could exhaust it
and prevent future RAS errors from being processed. Could these error paths
free the memory?
> + }
> +#endif
> +}
[ ... ]
> +void ghes_cper_handle_status(struct device *dev,
> + const struct acpi_hest_generic *generic,
> + const struct acpi_hest_generic_status *estatus,
> + bool sync)
> +{
> + int sev, sec_sev;
> + struct acpi_hest_generic_data *gdata;
> + guid_t *sec_type;
> + const guid_t *fru_id = &guid_null;
> + char *fru_text = "";
> + bool queued = false;
> +
> + sev = ghes_severity(estatus->error_severity);
> + apei_estatus_for_each_section(estatus, gdata) {
> + sec_type = (guid_t *)gdata->section_type;
> + sec_sev = ghes_severity(gdata->error_severity);
> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
> + fru_id = (guid_t *)gdata->fru_id;
> +
> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
> + fru_text = gdata->fru_text;
> +
> + ghes_log_hwerr(sev, sec_type);
> + if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) {
> + struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata);
> +
> + atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
> +
> + arch_apei_report_mem_error(sev, mem_err);
> + queued = ghes_handle_memory_failure(gdata, sev, sync);
[Severity: High]
This isn't a regression introduced by this patch, but does queued get
overwritten here as well?
Just like in ghes_handle_arm_hw_error() above, if an early section in
drivers/acpi/apei/ghes_cper.c:ghes_cper_handle_status() succeeds but a
later section does not, queued is overwritten to false.
This can result in the loop completing with queued == false despite a previous
successful queueing, which will then incorrectly trigger the sync && !queued
force kill logic below.
> + } else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
> + ghes_handle_aer(gdata);
> + } else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
> + queued = ghes_handle_arm_hw_error(gdata, sev, sync);
[Severity: High]
This is a pre-existing issue, but this direct assignment of queued has the
same overwriting problem across multiple sections as noted above.
> + } else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
[ ... ]
> diff --git a/include/acpi/ghes_cper.h b/include/acpi/ghes_cper.h
> index a853a5996cdfa..e09a33d343d79 100644
> --- a/include/acpi/ghes_cper.h
> +++ b/include/acpi/ghes_cper.h
[ ... ]
> @@ -107,6 +110,23 @@ void ghes_estatus_cache_add(struct acpi_hest_generic *generic,
> struct acpi_hest_generic_status *estatus);
> void ghes_defer_non_standard_event(struct acpi_hest_generic_data *gdata,
> int sev);
[ ... ]
> void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> int severity);
> int cxl_cper_register_prot_err_work(struct work_struct *work);
[Severity: High]
This is a pre-existing issue, but could the CXL work registration functions
deadlock with the GHES interrupt handler?
In drivers/acpi/apei/ghes_cper.c, the spinlocks cxl_cper_work_lock and
cxl_cper_prot_err_work_lock are acquired in hardirq/NMI-deferred context by
cxl_cper_post_event() and cxl_cper_post_prot_err() using guard(spinlock_irqsave).
However, the registration functions cxl_cper_register_work() and
cxl_cper_register_prot_err_work() acquire these locks using guard(spinlock),
leaving interrupts enabled.
If a GHES hardware error interrupt fires on the same CPU while the lock is held
by a registration function, the interrupt handler will attempt to acquire the
same lock, resulting in a deadlock. Could these registration functions use
guard(spinlock_irqsave)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=8
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox