* [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 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 08/10] Input: cap11xx - guard unsupported DT properties before parsing
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>
Check of_property_present() before parsing microchip,calib-sensitivity
and microchip,signal-guard, so that models which do not support these
properties (e.g. CAP1114) skip the parsing entirely.
This prevents a potential buffer overflow in calib_sensitivities[8] and
signal_guard_inputs_mask when a model with more than 8 channels
(CAP1114 has 14) would otherwise call of_property_read_u32_array()
with num_channels as the element count.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 52 +++++++++++++++++---------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 0f19ee036e78..275eb79a7193 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -231,10 +231,13 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
}
}
- if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
- priv->calib_sensitivities,
- priv->model->num_channels)) {
- if (priv->model->has_sensitivity_control) {
+ if (of_property_present(node, "microchip,calib-sensitivity")) {
+ if (!priv->model->has_sensitivity_control) {
+ dev_warn(dev,
+ "This model doesn't support 'calib-sensitivity'\n");
+ } else if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
+ priv->calib_sensitivities,
+ priv->model->num_channels)) {
for (i = 0; i < priv->model->num_channels; i++) {
if (!is_power_of_2(priv->calib_sensitivities[i]) ||
priv->calib_sensitivities[i] > 4) {
@@ -254,32 +257,31 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
if (error)
return error;
}
- } else {
- dev_warn(dev,
- "This model doesn't support 'calib-sensitivity'\n");
}
}
- for (i = 0; i < priv->model->num_channels; i++) {
- if (!of_property_read_u32_index(node, "microchip,signal-guard",
- i, &u32_val)) {
- if (u32_val > 1)
- return -EINVAL;
- if (u32_val)
- priv->signal_guard_inputs_mask |= 0x01 << i;
- }
- }
-
- if (priv->signal_guard_inputs_mask) {
- if (priv->model->has_signal_guard) {
- error = regmap_write(priv->regmap,
- CAP11XX_REG_SIGNAL_GUARD_ENABLE,
- priv->signal_guard_inputs_mask);
- if (error)
- return error;
- } else {
+ if (of_property_present(node, "microchip,signal-guard")) {
+ if (!priv->model->has_signal_guard) {
dev_warn(dev,
"This model doesn't support 'signal-guard'\n");
+ } else {
+ for (i = 0; i < priv->model->num_channels; i++) {
+ if (!of_property_read_u32_index(node, "microchip,signal-guard",
+ i, &u32_val)) {
+ if (u32_val > 1)
+ return -EINVAL;
+ if (u32_val)
+ priv->signal_guard_inputs_mask |= 0x01 << i;
+ }
+ }
+
+ if (priv->signal_guard_inputs_mask) {
+ error = regmap_write(priv->regmap,
+ CAP11XX_REG_SIGNAL_GUARD_ENABLE,
+ priv->signal_guard_inputs_mask);
+ if (error)
+ return error;
+ }
}
}
--
2.54.0
^ permalink raw reply related
* [PATCH v4 09/10] dt-bindings: input: microchip,cap11xx: Add 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, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
Add the compatible string for CAP1114, add its datasheet URL,
update the maximum of LED channel reg, and add constraint for
linux,keycodes.
Previously, the LED reg property had a default maximum of 7 for CAP1188.
With the addition of CAP1114, the default maximum is now 11.
An if-then constraint is added to limit the LED count for CAP1188.
Update description for microchip,input-threshold: CAP1114 only provides
eight threshold entries, which does not match its total channel count.
CAP1114 does not support microchip,signal-guard and
microchip,calib-sensitivity.
Add CAP1114 to the unsupported enum list.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 39 +++++++++++++++++--
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index b97e5b2735f1..2a37ac252c37 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -12,6 +12,7 @@ description: |
For more product information please see the links below:
CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+ CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.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
@@ -26,6 +27,7 @@ properties:
compatible:
enum:
- microchip,cap1106
+ - microchip,cap1114
- microchip,cap1126
- microchip,cap1188
- microchip,cap1203
@@ -62,7 +64,7 @@ properties:
linux,keycodes:
minItems: 3
- maxItems: 8
+ maxItems: 14
description: |
Specifies an array of numeric keycode values to
be used for the channels. If this property is
@@ -122,6 +124,8 @@ properties:
is required for a touch to be registered, making the touch sensor less
sensitive.
The number of entries must correspond to the number of channels.
+ CAP1114 is an exception where channels 8~14 reuse the eighth entry's
+ threshold, so counts differ.
microchip,calib-sensitivity:
$ref: /schemas/types.yaml#/definitions/uint32-array
@@ -140,7 +144,7 @@ properties:
The number of entries must correspond to the number of channels.
patternProperties:
- "^led@[0-7]$":
+ "^led@[0-9a]$":
type: object
description: CAP11xx LEDs
$ref: /schemas/leds/common.yaml#
@@ -149,7 +153,7 @@ patternProperties:
reg:
description: LED channel number
minimum: 0
- maximum: 7
+ maximum: 10
label: true
@@ -178,6 +182,21 @@ allOf:
properties:
reset-gpios: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1114
+ then:
+ properties:
+ linux,keycodes:
+ minItems: 14
+ else:
+ properties:
+ linux,keycodes:
+ maxItems: 8
+
- if:
properties:
compatible:
@@ -205,12 +224,26 @@ allOf:
reg:
maximum: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1188
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 7
+
- if:
properties:
compatible:
contains:
enum:
- microchip,cap1106
+ - microchip,cap1114
- microchip,cap1126
- microchip,cap1188
- microchip,cap1203
--
2.54.0
^ permalink raw reply related
* [PATCH v4 10/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
In-Reply-To: <20260617150318.753148-1-jerrysteve1101@gmail.com>
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
The CAP1114 uses two control registers for LED output management and
requires two button status registers for touch input state reporting.
By default, channels CS8~CS14 operate as a single grouped block.
Set the corresponding register enable bit to enable these channels as
independent touch inputs. Note these channels share the input threshold
of the eighth entry, causing num_sensor_thresholds to differ from
num_channels.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 73 ++++++++++++++++++++++++++++++--
1 file changed, 69 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 275eb79a7193..865c58533eb9 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -18,6 +18,12 @@
#include <linux/gpio/consumer.h>
#include <linux/bitfield.h>
+#define CAP1114_REG_BUTTON_STATUS1 0x03
+#define CAP1114_REG_BUTTON_STATUS2 0x04
+#define CAP1114_REG_CONFIG2 0x40
+#define CAP1114_REG_CONFIG2_VOL_UP_DOWN BIT(1)
+#define CAP1114_REG_LED_OUTPUT_CONTROL1 0x73
+
#define CAP11XX_REG_MAIN_CONTROL 0x00
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
@@ -83,6 +89,7 @@ struct cap11xx_hw_model {
unsigned int num_leds;
unsigned int num_sensor_thresholds;
bool has_gain;
+ bool has_grouped_sensors;
bool has_irq_config;
bool has_repeat_en;
bool has_sensitivity_control;
@@ -99,6 +106,8 @@ static const struct reg_default cap11xx_reg_defaults[] = {
{ CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(6), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(7), 0x40 },
{ CAP11XX_REG_CONFIG2, 0x40 },
};
@@ -107,6 +116,12 @@ static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
switch (reg) {
case CAP11XX_REG_MAIN_CONTROL:
case CAP11XX_REG_SENSOR_INPUT:
+ /*
+ * CAP1114_REG_BUTTON_STATUS1 (CAP11XX_REG_SENSOR_INPUT) and
+ * CAP1114_REG_BUTTON_STATUS2 is volatile for the CAP1114,
+ * which supports more than 8 touch channels.
+ */
+ case CAP1114_REG_BUTTON_STATUS2:
case CAP11XX_REG_SENOR_DELTA(0):
case CAP11XX_REG_SENOR_DELTA(1):
case CAP11XX_REG_SENOR_DELTA(2):
@@ -292,6 +307,17 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
of_property_read_u32_array(node, "linux,keycodes",
priv->keycodes, priv->model->num_channels);
+ /*
+ * CAP1114 needs dedicated configuration to split
+ * grouped sensors into independent inputs.
+ */
+ if (priv->model->has_grouped_sensors) {
+ error = regmap_set_bits(priv->regmap, CAP1114_REG_CONFIG2,
+ CAP1114_REG_CONFIG2_VOL_UP_DOWN);
+ 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);
@@ -320,6 +346,21 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
if (ret < 0)
goto out;
+ if (priv->model->num_channels > 8) {
+ unsigned int status2;
+
+ ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base + 1, &status2);
+ if (ret < 0)
+ goto out;
+
+ /*
+ * CAP1114 STATUS1 register only contains data for the first 6 channels.
+ * the remaining channels is stored in STATUS2.
+ */
+ status &= GENMASK(5, 0);
+ status |= FIELD_PREP(GENMASK(13, 6), status2);
+ }
+
for (i = 0; i < priv->idev->keycodemax; i++)
input_report_key(priv->idev, priv->keycodes[i],
status & (1 << i));
@@ -369,10 +410,16 @@ static int cap11xx_led_set(struct led_classdev *cdev,
* limitation. Brightness levels per LED are either
* 0 (OFF) and 1 (ON).
*/
- return regmap_update_bits(priv->regmap,
- priv->model->led_output_control_reg_base,
- BIT(led->reg),
- value ? BIT(led->reg) : 0);
+ if (led->reg >= 8)
+ return regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base + 1,
+ BIT(led->reg - 8),
+ value ? BIT(led->reg - 8) : 0);
+ else
+ return regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base,
+ BIT(led->reg),
+ value ? BIT(led->reg) : 0);
}
static int cap11xx_init_leds(struct device *dev,
@@ -403,6 +450,14 @@ static int cap11xx_init_leds(struct device *dev,
if (error)
return error;
+ if (num_leds > 8) {
+ error = regmap_update_bits(priv->regmap,
+ priv->model->led_output_control_reg_base + 1,
+ GENMASK(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);
@@ -581,6 +636,14 @@ static const struct cap11xx_hw_model cap1106_model = {
.has_repeat_en = true,
};
+static const struct cap11xx_hw_model cap1114_model = {
+ .product_id = 0x3a,
+ .num_channels = 14, .num_leds = 11, .num_sensor_thresholds = 8,
+ .led_output_control_reg_base = CAP1114_REG_LED_OUTPUT_CONTROL1,
+ .sensor_input_reg_base = CAP1114_REG_BUTTON_STATUS1,
+ .has_grouped_sensors = true,
+};
+
static const struct cap11xx_hw_model cap1126_model = {
.product_id = 0x53,
.num_channels = 6, .num_leds = 2, .num_sensor_thresholds = 6,
@@ -637,6 +700,7 @@ static const struct cap11xx_hw_model cap1298_model = {
static const struct of_device_id cap11xx_dt_ids[] = {
{ .compatible = "microchip,cap1106", .data = &cap1106_model },
+ { .compatible = "microchip,cap1114", .data = &cap1114_model },
{ .compatible = "microchip,cap1126", .data = &cap1126_model },
{ .compatible = "microchip,cap1188", .data = &cap1188_model },
{ .compatible = "microchip,cap1203", .data = &cap1203_model },
@@ -649,6 +713,7 @@ MODULE_DEVICE_TABLE(of, cap11xx_dt_ids);
static const struct i2c_device_id cap11xx_i2c_ids[] = {
{ .name = "cap1106", .driver_data = (kernel_ulong_t)&cap1106_model },
+ { .name = "cap1114", .driver_data = (kernel_ulong_t)&cap1114_model },
{ .name = "cap1126", .driver_data = (kernel_ulong_t)&cap1126_model },
{ .name = "cap1188", .driver_data = (kernel_ulong_t)&cap1188_model },
{ .name = "cap1203", .driver_data = (kernel_ulong_t)&cap1203_model },
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v5 10/16] media: iris: add minimal GET_PROPERTY implementation
From: Vishnu Reddy @ 2026-06-17 15:04 UTC (permalink / raw)
To: Dmitry Baryshkov, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-media, linux-arm-msm, linux-kernel, devicetree,
Dikshita Agarwal
In-Reply-To: <20260616-iris-ar50lt-v5-10-583b42770b6a@oss.qualcomm.com>
On 6/16/2026 5:34 AM, Dmitry Baryshkov wrote:
> AR50Lt with the Gen1 firmware requires host to read
> HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS property, otherwise it doesn't
> update internal data and fails the HFI_CMD_SESSION_LOAD_RESOURCES
> command. Implement minimal support for querying the properties from the
> firmware.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 +
> .../platform/qcom/iris/iris_hfi_gen1_command.c | 21 +++++++++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_defines.h | 15 +++++++++++++++
> .../platform/qcom/iris/iris_hfi_gen1_response.c | 6 ++++++
> 4 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> index a27447eb2519..16099f9a25b6 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> @@ -121,6 +121,7 @@ struct iris_hfi_session_ops {
> int (*session_set_property)(struct iris_inst *inst,
> u32 packet_type, u32 flag, u32 plane, u32 payload_type,
> void *payload, u32 payload_size);
> + int (*session_get_property)(struct iris_inst *inst, u32 packet_type);
> int (*session_open)(struct iris_inst *inst);
> int (*session_start)(struct iris_inst *inst, u32 plane);
> int (*session_queue_buf)(struct iris_inst *inst, struct iris_buffer *buffer);
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 7674b47ad6c4..99e82e5510ab 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -1117,10 +1117,31 @@ static int iris_hfi_gen1_session_set_config_params(struct iris_inst *inst, u32 p
> return 0;
> }
>
> +static int iris_hfi_gen1_session_get_property(struct iris_inst *inst, u32 packet_type)
> +{
> + struct hfi_session_get_property_pkt pkt;
> + int ret;
> +
> + pkt.shdr.hdr.size = sizeof(pkt);
> + pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_GET_PROPERTY;
> + pkt.shdr.session_id = inst->session_id;
> + pkt.num_properties = 1;
> + pkt.data = packet_type;
> +
> + reinit_completion(&inst->completion);
> +
> + ret = iris_hfi_queue_cmd_write(inst->core, &pkt, pkt.shdr.hdr.size);
> + if (ret)
> + return ret;
> +
> + return iris_wait_for_session_response(inst, false);
> +}
> +
> static const struct iris_hfi_session_ops iris_hfi_gen1_session_ops = {
> .session_open = iris_hfi_gen1_session_open,
> .session_set_config_params = iris_hfi_gen1_session_set_config_params,
> .session_set_property = iris_hfi_gen1_session_set_property,
> + .session_get_property = iris_hfi_gen1_session_get_property,
> .session_start = iris_hfi_gen1_session_start,
> .session_queue_buf = iris_hfi_gen1_session_queue_buffer,
> .session_release_buf = iris_hfi_gen1_session_unset_buffers,
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> index 0e4dee192384..bb495a1d2623 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
> @@ -35,6 +35,7 @@
> #define HFI_CMD_SESSION_EMPTY_BUFFER 0x211004
> #define HFI_CMD_SESSION_FILL_BUFFER 0x211005
> #define HFI_CMD_SESSION_FLUSH 0x211008
> +#define HFI_CMD_SESSION_GET_PROPERTY 0x211009
> #define HFI_CMD_SESSION_RELEASE_BUFFERS 0x21100b
> #define HFI_CMD_SESSION_RELEASE_RESOURCES 0x21100c
> #define HFI_CMD_SESSION_CONTINUE 0x21100d
> @@ -113,6 +114,7 @@
> #define HFI_MSG_SESSION_FLUSH 0x221006
> #define HFI_MSG_SESSION_EMPTY_BUFFER 0x221007
> #define HFI_MSG_SESSION_FILL_BUFFER 0x221008
> +#define HFI_MSG_SESSION_PROPERTY_INFO 0x221009
> #define HFI_MSG_SESSION_RELEASE_RESOURCES 0x22100a
> #define HFI_MSG_SESSION_RELEASE_BUFFERS 0x22100c
>
> @@ -205,6 +207,12 @@ struct hfi_session_set_property_pkt {
> u32 data[];
> };
>
> +struct hfi_session_get_property_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 data;
> +};
> +
> struct hfi_sys_pc_prep_pkt {
> struct hfi_pkt_hdr hdr;
> };
> @@ -574,6 +582,13 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt {
> u32 data[];
> };
>
> +struct hfi_msg_session_property_info_pkt {
> + struct hfi_session_hdr_pkt shdr;
> + u32 num_properties;
> + u32 property;
> + u8 data[];
> +};
> +
> struct hfi_msg_session_release_buffers_done_pkt {
> struct hfi_msg_session_hdr_pkt shdr;
> u32 num_buffers;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> index bfd7495bf44f..23fc7194b1e3 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c
> @@ -591,6 +591,10 @@ static const struct iris_hfi_gen1_response_pkt_info pkt_infos[] = {
> .pkt = HFI_MSG_SESSION_RELEASE_BUFFERS,
> .pkt_sz = sizeof(struct hfi_msg_session_release_buffers_done_pkt),
> },
> + {
> + .pkt = HFI_MSG_SESSION_PROPERTY_INFO,
> + .pkt_sz = sizeof(struct hfi_msg_session_property_info_pkt),
> + },
> };
>
> static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response)
> @@ -652,6 +656,8 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response
> iris_hfi_gen1_session_etb_done(inst, hdr);
> } else if (hdr->pkt_type == HFI_MSG_SESSION_FILL_BUFFER) {
> iris_hfi_gen1_session_ftb_done(inst, hdr);
> + } else if (hdr->pkt_type == HFI_MSG_SESSION_PROPERTY_INFO) {
> + complete(&inst->completion);
> } else {
> struct hfi_msg_session_hdr_pkt *shdr;
>
Reviewed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v8 4/6] leds: is31fl32xx: Add powerdown pin for hardware shutdown mode
From: Lee Jones @ 2026-06-17 15:09 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-5-jerrysteve1101@gmail.com>
/* Sashiko Automation: Issues Found (2 Findings) */
Please consider these.
On Sun, 07 Jun 2026, Jun Yan wrote:
> The IS31FL32XX series features an SDB shutdown pin. Driving it low
> (active low) places the chip into hardware shutdown mode for power
> saving, while all register contents are preserved and registers are
> not reset.
>
> Deassert/assert the pin accordingly during probe() and remove() to
> manage hardware shutdown mode.
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 6c8d6b833260..b34fcef2c173 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -10,6 +10,7 @@
> */
>
> #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/kernel.h>
> #include <linux/leds.h>
> @@ -60,6 +61,7 @@ struct is31fl32xx_led_data {
> struct is31fl32xx_priv {
> const struct is31fl32xx_chipdef *cdef;
> struct i2c_client *client;
> + struct gpio_desc *powerdown_gpio;
> unsigned int num_leds;
> struct is31fl32xx_led_data leds[];
> };
> @@ -421,6 +423,15 @@ static int is31fl32xx_parse_dt(struct device *dev,
> const struct is31fl32xx_chipdef *cdef = priv->cdef;
> int ret = 0;
>
> + /*
> + * Driving this GPIO line low takes the chip out of shutdown,
> + * as it is flagged as GPIO_ACTIVE_LOW in provider (such as the device tree).
> + */
> + priv->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->powerdown_gpio))
> + return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
> + "Failed to get 'powerdown' GPIO\n");
> +
[Severity: Medium]
Does this leave the chip actively powered on if probe fails later?
The powerdown gpio is acquired and driven logically low with GPIOD_OUT_LOW,
which wakes the chip from hardware shutdown.
If a subsequent initialization step in probe() fails (for example, if
is31fl32xx_init_regs() returns an error), probe() will return an error
without explicitly reverting the GPIO to its shutdown state.
The devres framework automatically frees the GPIO descriptor, but it does
not revert its physical output level. Will this leave the device powered
on indefinitely instead of returning it to its low-power standby state?
> if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
> of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
>
> @@ -609,6 +620,9 @@ static void is31fl32xx_remove(struct i2c_client *client)
> if (ret)
> dev_err(&client->dev, "Failed to reset registers on removal (%pe)\n",
> ERR_PTR(ret));
> +
> + if (priv->powerdown_gpio)
> + gpiod_set_value_cansleep(priv->powerdown_gpio, 1);
[Severity: Medium]
Could this cause an I2C error cascade during driver unbind due to the teardown
order?
The driver registers LEDs using devm_led_classdev_register_ext(), which
means the LED class devices are unregistered by devres after the driver's
remove() callback completes.
When this asserts the powerdown_gpio, it places the chip into hardware
shutdown mode. Subsequently, devres invokes led_classdev_unregister() which
attempts to gracefully turn off each LED:
drivers/leds/led-class.c:led_classdev_unregister() {
...
if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
led_set_brightness(led_cdev, LED_OFF);
...
}
This triggers an I2C write to the chip via is31fl32xx_brightness_set().
Because the hardware is already in hardware shutdown mode, will these
I2C writes fail and result in errors?
> }
>
> /*
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/6] leds: is31fl32xx: Fix missing brightness_steps for is31fl3236
From: Lee Jones @ 2026-06-17 15:10 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-6-jerrysteve1101@gmail.com>
/* Sashiko Automation: Issues Found (5 Findings) */
And these.
On Sun, 07 Jun 2026, Jun Yan wrote:
> Add missing brightness_steps for is31fl3236 to fix brightness control.
>
> Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index b34fcef2c173..cbda7edbfb48 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -492,6 +492,7 @@ static const struct is31fl32xx_chipdef is31fl3236_cdef = {
> .pwm_register_base = 0x01,
> .led_control_register_base = 0x26,
> .enable_bits_per_led_control_register = 1,
> + .brightness_steps = 256,
> };
>
> static const struct is31fl32xx_chipdef is31fl3236a_cdef = {
While reviewing this file, I also noticed a few other regressions in the same
driver:
drivers/leds/leds-is31fl32xx.c:is31fl3293_reset() {
...
for (i = 0; i < priv->num_leds; i++) {
struct is31fl32xx_led_data *led_data = &priv->leds[i];
int current_level_reg = IS31FL3293_CL_REG + led_data->channel - 1;
int microamp = max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP);
int current_level = (microamp * 0xff) / IS31FL3293_MAX_MICROAMP;
ret = is31fl32xx_write(priv, current_level_reg, current_level);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but does this calculation handle the
current limit bounds correctly?
It appears that max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP) forces
microamp to be at least 20,000 (the value of IS31FL3293_MAX_MICROAMP). If a
device tree requires a lower safety limit (e.g., 5000), it seems it would be
ignored.
Conversely, if the device tree specifies a value larger than 20,000 (e.g.,
30,000), the calculated current_level evaluates to 382. When passed to
is31fl32xx_write(), which takes a u8 parameter, won't this overflow and
truncate to 126, silently programming a drastically lower current limit?
drivers/leds/leds-is31fl32xx.c:is31fl32xx_probe() {
...
ret = is31fl32xx_parse_dt(dev, priv);
if (ret)
return ret;
ret = is31fl32xx_init_regs(priv);
...
}
[Severity: High]
This is a pre-existing issue, but could this ordering lead to a race condition
with userspace?
is31fl32xx_parse_dt() calls devm_led_classdev_register_ext(), which
immediately publishes the LED devices. The LED subsystem might apply
default-state triggers, or userspace might set the brightness, which sends
PWM writes to the device over I2C.
However, the hardware registers are only initialized and reset in
is31fl32xx_init_regs() afterwards. Won't this reset wipe out any brightness
state configured during or immediately after LED registration? Should the
hardware be fully initialized before device registration?
drivers/leds/leds-is31fl32xx.c:
...
static const struct is31fl32xx_chipdef is31fl3293_cdef = {
.channels = 3,
.shutdown_reg = IS31FL32XX_REG_NONE,
.pwm_update_reg = 0x28,
.pwm_update_value = 0xc5,
.global_control_reg = IS31FL32XX_REG_NONE,
.reset_reg = IS31FL32XX_REG_NONE,
.pwm_register_base = 0x19,
.led_control_register_base = IS31FL32XX_REG_NONE,
.brightness_steps = 4096,
.reset_func = is31fl3293_reset,
.sw_shutdown_func = is31fl3293_software_shutdown,
};
...
[Severity: High]
Is is31fl3293_cdef missing the initialization for output_frequency_setting_reg?
Since this field isn't set to IS31FL32XX_REG_NONE, it is implicitly initialized
to 0x00. During probe, is31fl32xx_init_regs() checks if this field is
!= IS31FL32XX_REG_NONE (which is 0xFF). Since 0x00 != 0xFF, if the DT
contains the issi,22khz-pwm property, the driver will erroneously write 0x01
to register 0x00.
For the IS31FL3293, register 0x00 is undefined. Can this silently corrupt the
chip state?
drivers/leds/leds-is31fl32xx.c:is31fl32xx_remove() {
...
static void is31fl32xx_remove(struct i2c_client *client)
{
struct is31fl32xx_priv *priv = i2c_get_clientdata(client);
int ret;
ret = is31fl32xx_reset_regs(priv);
...
if (priv->powerdown_gpio)
gpiod_set_value_cansleep(priv->powerdown_gpio, 1);
}
...
[Severity: High]
Will the device acknowledge I2C writes after being powered down during removal?
In is31fl32xx_remove(), the driver asserts powerdown_gpio to place the chip
into a hardware shutdown mode. After remove() completes, devres automatically
unregisters the LED class devices that were allocated in is31fl32xx_parse_dt().
During unregistration, the LED subsystem turns off the LEDs by calling
is31fl32xx_brightness_set(), which attempts to write to the chip over I2C.
Because the chip is already physically powered down, won't this result in I2C
timeouts or EREMOTEIO errors on every driver removal?
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v4 2/2] hw_random: timeriomem-rng: add configurable read width and data mask
From: Thomas Gleixner @ 2026-06-17 15:11 UTC (permalink / raw)
To: Jad Keskes, Krzysztof Kozlowski
Cc: Olivia Mackall, Herbert Xu, Rob Herring, Conor Dooley,
Alexander Clouter, linux-crypto, devicetree, linux-kernel,
Jad Keskes
In-Reply-To: <20260617114642.1911191-1-inasj268@gmail.com>
Jad!
On Wed, Jun 17 2026 at 12:46, Jad Keskes wrote:
> The TODO for supporting read sizes other than 32 bits and masking has
> been sitting in this driver since 2009. Implement it.
>
> Add reg-io-width (1, 2, or 4 bytes) and mask support. The read loop
> dispatches on width using readb/readw/readl so a configured 1-byte
> access doesn't trigger a bus error on hardware that rejects 32-bit
> reads to that address. The mask is ANDed with the value before storing.
>
> These are platform properties, not runtime policy -- width depends on
> SoC integration, mask reflects which output bits carry entropy.
>
> The alignment check in probe is updated to verify the resource is
> aligned to the configured width instead of hardcoding 4-byte alignment.
So this is the 4th version of the same thing within 24 hours and without
any explanation what the difference between v1/2/3/4 is.
Please stop this frenzy and send out new versions only if there is a
good and documented reason. Otherwise give people the time to review
your patch. All of this is documented in Documentation/process.
^ permalink raw reply
* Re: [PATCH v4 3/5] arm64: dts: qcom: sm8550-hdk: add SPMI ADC channels and thermal nodes
From: sashiko-bot @ 2026-06-17 15:13 UTC (permalink / raw)
To: Neil Armstrong; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260617-topic-sm8x50-adc5-gen3-v4-3-4af9251731f1@linaro.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Device tree compilation fails because the patch uses parameterized macros (e.g., `ADC5_GEN3_DIE_TEMP(1)` and `ADC5_GEN3_AMUX1_THM_100K_PU(1)`) that are undefined. The required local header `qcom-adc5-gen3.h` (included via `#include "qcom-adc5-gen3.h"` in `pmk8550.dtsi` in patch 1 of the series) is completely missing from the tree and was not added by any patch in this series. Furthermore, the similarly named kernel-internal header `include/linux/iio/adc/qcom-adc5-gen3-common.h` defines these as flat integers (e.g., `#define ADC5_GEN3_DIE_TEMP 0x03`), not parameterized macros, so even if included it would cause DTC syntax errors (e.g., `0x03(1)`).
--
commit ab5e68d3e2e92bec8b24c3b38a79b453baff7e21
Author: Neil Armstrong <neil.armstrong@linaro.org>
arm64: dts: qcom: sm8550-hdk: add SPMI ADC channels and thermal nodes
Add the SPMI ADC channels on the PMK8550 SPMI5 ADC3 for the other PMICS
on the system. The thermal nodes are sorted by the sensor channel to be
coherent with the system thermal nodes ordering.
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> index ee13e6136a825..f19419f63e36b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> @@ -280,6 +280,92 @@ platform {
> };
> };
>
> + thermal-zones {
> + skin-thermal {
> + thermal-sensors = <&pmk8550_vadc ADC5_GEN3_AMUX1_THM_100K_PU(1)>;
[Severity: Low]
Does this compile? It appears the parameterized macro
ADC5_GEN3_AMUX1_THM_100K_PU(1) is undefined because the required header
is missing from the tree.
[ ... ]
> @@ -1102,27 +1188,218 @@ led@3 {
> };
> };
>
> +&pm8550_temp_alarm {
> + io-channels = <&pmk8550_vadc ADC5_GEN3_DIE_TEMP(1)>;
[Severity: Low]
Similarly here, ADC5_GEN3_DIE_TEMP(1) seems to be missing its definition.
Even if include/linux/iio/adc/qcom-adc5-gen3-common.h were included, it
defines ADC5_GEN3_DIE_TEMP as a flat integer (0x03) rather than a macro
accepting parameters, which would cause a DTC syntax error.
> + io-channel-names = "thermal";
> +};
[ ... ]
> +&pmk8550_vadc {
> + /* PM8550 Channel nodes */
> + channel@100 {
> + reg = <ADC5_GEN3_REF_GND(1)>;
[Severity: Low]
Are these macros defined anywhere in the tree? The ADC channel definitions
throughout this file use these parameterized macros, which appear to rely
on a missing header.
> + label = "pm8550_offset_ref";
> + qcom,pre-scaling = <1 1>;
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-topic-sm8x50-adc5-gen3-v4-0-4af9251731f1@linaro.org?part=3
^ permalink raw reply
* [PATCH v2] dt-bindings: arm: qcom: Document Hawi SoC and its reference boards
From: Mukesh Ojha @ 2026-06-17 15:16 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Mukesh Ojha
Add Hawi SoC and its reference boards to the Qualcomm binding.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
Changes in v2: https://lore.kernel.org/lkml/20260617080147.1657632-1-mukesh.ojha@oss.qualcomm.com/
- Fixed the position of the Documentation.
- Corrected the commit text.
Documentation/devicetree/bindings/arm/qcom.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index 50cc18a6ec5e..e229c0097e6f 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -303,6 +303,11 @@ properties:
- xiaomi,sagit
- const: qcom,msm8998
+ - items:
+ - enum:
+ - qcom,hawi-mtp
+ - const: qcom,hawi
+
- items:
- enum:
- 8dev,jalapeno
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v8 6/6] leds: is31fl32xx: Move pwm frequency setting to init_regs()
From: Lee Jones @ 2026-06-17 15:17 UTC (permalink / raw)
To: Jun Yan
Cc: linux-kernel, devicetree, linux-leds, robh, krzk+dt, conor+dt,
luccafachinetti, pzalewski, daniel
In-Reply-To: <20260607152002.446617-7-jerrysteve1101@gmail.com>
/* Sashiko Automation: Reviewed (2 Findings) */
And this one please.
On Sun, 07 Jun 2026, Jun Yan wrote:
> commit a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> swapped the order of is31fl32xx_parse_dt() and is31fl32xx_init_regs().
[Severity: High]
This isn't a bug introduced by this patch, but registering LED class devices
before hardware initialization completes exposes the LEDs prematurely.
In is31fl32xx_probe(), is31fl32xx_parse_dt() is called, which internally
executes devm_led_classdev_register_ext() for each LED:
drivers/leds/leds-is31fl32xx.c:is31fl32xx_probe() {
...
ret = is31fl32xx_parse_dt(dev, priv);
if (ret)
return ret;
ret = is31fl32xx_init_regs(priv);
...
}
This registers the LEDs to the subsystem, potentially immediately triggering
led_set_brightness_nopm() via default state settings (e.g., "default-on").
This schedules an asynchronous work item to configure the LED over I2C.
However, is31fl32xx_probe() then immediately calls is31fl32xx_init_regs(),
which performs a full hardware reset (is31fl32xx_reset_regs()).
If the hardware reset races with or occurs after the default state triggers,
will the initial LED states be wiped out? Must the hardware be fully
initialized and reset before registering the LED devices?
>
> This causes the PWM frequency configuration programmed in
> is31fl32xx_parse_dt() to be overwritten by the register reset
> operation performed in is31fl32xx_init_regs().
>
> Move the PWM frequency setting logic from is31fl32xx_parse_dt() to
> is31fl32xx_init_regs() and separates device tree parsing from hardware
> initialization.
>
> Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
> drivers/leds/leds-is31fl32xx.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index cbda7edbfb48..b43879bffb06 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
[Severity: High]
This is a pre-existing issue, but there is an incorrect use of max()
instead of min() in is31fl3293_reset():
drivers/leds/leds-is31fl32xx.c:is31fl3293_reset() {
...
for (i = 0; i < priv->num_leds; i++) {
struct is31fl32xx_led_data *led_data = &priv->leds[i];
int current_level_reg = IS31FL3293_CL_REG + led_data->channel - 1;
int microamp = max(led_data->max_microamp, IS31FL3293_MAX_MICROAMP);
int current_level = (microamp * 0xff) / IS31FL3293_MAX_MICROAMP;
ret = is31fl32xx_write(priv, current_level_reg, current_level);
...
}
This bounds the user-specified current limit against the hardware maximum of
20,000 µA (IS31FL3293_MAX_MICROAMP) using max(), forcing the current to
always be at least the max chip rating.
If an LED is only rated for 5mA, does this force it to 20mA, which could
physically damage it?
Furthermore, if a user requests a value larger than 20mA (e.g., 30,000 µA),
max() allows it. The subsequent calculation exceeds 255. Because
is31fl32xx_write() takes a u8 argument, this value is truncated, resulting in
unexpectedly lower current.
Should the code use min() to properly enforce the hardware's maximum rating?
> @@ -63,6 +63,7 @@ struct is31fl32xx_priv {
> struct i2c_client *client;
> struct gpio_desc *powerdown_gpio;
> unsigned int num_leds;
> + bool pwm_22khz;
> struct is31fl32xx_led_data leds[];
> };
>
> @@ -346,6 +347,13 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
> if (ret)
> return ret;
>
> + if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) && priv->pwm_22khz) {
> + ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
> + IS31FL32XX_PWM_FREQUENCY_22KHZ);
> + if (ret)
> + return ret;
> + }
> +
> /*
> * Set enable bit for all channels.
> * We will control state with PWM registers alone.
> @@ -420,7 +428,6 @@ static struct is31fl32xx_led_data *is31fl32xx_find_led_data(
> static int is31fl32xx_parse_dt(struct device *dev,
> struct is31fl32xx_priv *priv)
> {
> - const struct is31fl32xx_chipdef *cdef = priv->cdef;
> int ret = 0;
>
> /*
> @@ -432,17 +439,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
> return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
> "Failed to get 'powerdown' GPIO\n");
>
> - if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
> - of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
> -
> - ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
> - IS31FL32XX_PWM_FREQUENCY_22KHZ);
> -
> - if (ret) {
> - dev_err(dev, "Failed to write output PWM frequency register\n");
> - return ret;
> - }
> - }
> + priv->pwm_22khz = of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm");
>
> for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
> struct led_init_data init_data = {};
> --
> 2.54.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub
From: Frank Li @ 2026-06-17 15:19 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, robh, conor+dt, Frank.Li, devicetree, linux-i3c
In-Reply-To: <20260617111804.ABFF71F00A3D@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:18:04AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
> - [Critical] Deterministic NULL pointer dereference in `i3c_hub_free_ibi()` due to recursive callback chain.
> - [Critical] Memory leak and double-free memory corruption in `i3c_hub_request_ibi()` error paths.
> - [Critical] Critical race condition and parent bus state corruption in `i3c_hub_master_priv_xfers()` due to unlocked address mutation.
> - [Critical] Unimplemented `i3c_hub_recycle_ibi_slot()` permanently leaks hardware IBI slots.
> - [Critical] Architectural bypass in `i3c_hub_do_daa()` attaches dynamically discovered devices to the parent bus instead of the virtual hub bus.
> - [High] Calling `i3c_hub_enable_port()` outside of any locks allows concurrent operations to physically enable multiple downstream ports simultaneously, breaking electrical isolation.
> - [High] Silently dropping broadcast RSTDAA commands prevents resetting of downstream devices.
> - [Medium] Accepting I2C devices while unconditionally rejecting all I2C transfers violates the API contract.
> --
>
> commit 2a201a9e0e9a55b31467d874340085240b34afc3
> Author: Lakshay Piplani <lakshay.piplani@nxp.com>
>
> i3c: hub: Add support for the I3C interface in the I3C hub
>
> This patch adds virtual I3C bus support for the hub and provides an
> interface to enable or disable downstream ports.
>
> > diff --git a/drivers/i3c/hub.c b/drivers/i3c/hub.c
> > new file mode 100644
> > index 0000000000000..e8744b5172340
> > --- /dev/null
> > +++ b/drivers/i3c/hub.c
> > @@ -0,0 +1,483 @@
>
> [ ... ]
>
> > +static int i3c_hub_do_daa(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_do_daa(parent);
>
> [Severity: Critical]
> Since `i3c_master_do_daa()` executes the DAA sequence natively on the parent
> controller, will the newly discovered devices be allocated and attached
> directly to the parent bus instead of this virtual hub's bus?
>
> If they are attached to the parent bus, won't subsequent transfers completely
> bypass `i3c_hub_master_priv_xfers()`, causing communication with these
> dynamically enumerated devices to fail?
I forget some background, does all devices under hub port shared the same
I3C address space? that's means dynmatic address is difference for all
difference ports?
I2C switcher can support the same address at difference port. look like
diffenent port devices should attach to hub's bus. Only temparily attach
to parent during transfer?
Frank
>
> [Severity: High]
> Is there a risk of breaking electrical isolation here? `i3c_hub_enable_port()`
> is called before acquiring any parent bus locks.
>
> If multiple threads operate on different virtual controllers concurrently,
> could they physically enable multiple downstream ports at the same time
> before the bus lock is acquired?
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_send_ccc_cmd(struct i3c_hub *hub,
> > + struct i3c_master_controller *controller,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + struct i3c_master_controller *parent;
> > + int ret;
> > +
> > + if (!hub || !hub->hub_dev)
> > + return -ENODEV;
> > +
> > + parent = i3c_dev_get_master(hub->hub_dev->desc);
> > + if (!parent)
> > + return -ENODEV;
> > +
> > + i3c_hub_enable_port(controller);
> > + ret = i3c_master_send_ccc_cmd(parent, cmd);
>
> [Severity: High]
> Similar to `i3c_hub_do_daa()`, does calling `i3c_hub_enable_port()` without a
> lock risk enabling multiple ports concurrently if multiple CCC commands are
> issued on different ports?
>
> > + i3c_hub_disable_port(controller);
> > +
> > + return ret;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_master_priv_xfers(struct i3c_dev_desc *dev,
> > + struct i3c_xfer *xfers,
> > + int nxfers,
> > + enum i3c_xfer_mode mode)
> > +{
>
> [ ... ]
>
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = target_addr;
> > +
> > + ret = i3c_master_reattach_i3c_dev_locked(hub_dev, hub_addr);
> > + up_write(&parent->bus.lock);
> > + if (ret) {
> > + hub_dev->info.dyn_addr = hub_addr;
> > + goto disable;
> > + }
> > + }
> > +
> > + ret = i3c_device_do_xfers(hub->hub_dev, xfers, nxfers, mode);
>
> [Severity: Critical]
> Does dropping `parent->bus.lock` before calling `i3c_device_do_xfers()` expose
> a race condition?
>
> If multiple downstream ports initiate transfers concurrently, could they
> overwrite the shared `hub_dev->info.dyn_addr` before the transfer completes,
> causing packets to be routed to the wrong physical device?
>
> > +
> > + if (hub_addr != target_addr) {
> > + down_write(&parent->bus.lock);
> > + hub_dev->info.dyn_addr = hub_addr;
> > +
> > + xfer_ret = i3c_master_reattach_i3c_dev_locked(hub_dev,
> > + target_addr);
>
> [Severity: Critical]
> Could passing `target_addr` as the old address corrupt the parent bus state?
>
> `i3c_master_reattach_i3c_dev_locked()` frees the old address slot in the bus
> slot map. Will this erroneously mark `target_addr` as `I3C_ADDR_SLOT_FREE`
> on the parent bus?
>
> > +
> > + up_write(&parent->bus.lock);
> > + if (!ret)
> > + ret = xfer_ret;
> > + }
> > +disable:
> > + i3c_hub_disable_port(controller);
> > + return ret;
> > +}
> > +
> > +static int i3c_hub_attach_i2c_dev(struct i2c_dev_desc *dev)
> > +{
> > + return 0;
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_i2c_xfers(struct i2c_dev_desc *dev,
> > + struct i2c_msg *xfers, int nxfers)
> > +{
> > + return -EOPNOTSUPP;
> > +}
>
> [Severity: Medium]
> Is it intentional to return `0` from `i3c_hub_attach_i2c_dev()` while rejecting
> all I2C transfers with `-EOPNOTSUPP`?
>
> Would it be safer to return an error from the attach callback to cleanly
> prevent I2C driver binding if the hub does not support I2C forwarding?
>
> [ ... ]
>
> > +static int i3c_hub_master_send_ccc_cmd(struct i3c_master_controller *controller,
> > + struct i3c_ccc_cmd *cmd)
> > +{
> > + struct i3c_hub_controller *hub_controller;
> > + struct i3c_hub *hub;
> > +
> > + hub_controller = dev_get_drvdata(&controller->dev);
> > + if (!hub_controller || !hub_controller->hub)
> > + return -ENODEV;
> > +
> > + hub = hub_controller->hub;
> > +
> > + if (!hub->hub_dev)
> > + return -ENODEV;
> > +
> > + if (cmd->id == I3C_CCC_RSTDAA(true))
> > + return 0;
>
> [Severity: High]
> Does silently dropping the broadcast RSTDAA command prevent downstream devices
> from being correctly reset by the hub?
>
> If the command is dropped instead of being forwarded to the downstream ports,
> will devices retain their old dynamic addresses and fail to re-initialize
> during hot-plug recovery?
>
> > +
> > + return i3c_hub_send_ccc_cmd(hub, controller, cmd);
> > +}
>
> [ ... ]
>
> > +static int i3c_hub_request_ibi(struct i3c_dev_desc *desc,
> > + const struct i3c_ibi_setup *req)
> > +{
>
> [ ... ]
>
> > + ret = i3c_master_direct_attach_i3c_dev_locked(parent, desc);
> > + if (ret) {
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > + return ret;
> > + }
> > +
> > + kfree(desc->ibi);
>
> [Severity: Critical]
> Does freeing `desc->ibi` here leak `desc->ibi->wq`?
>
> When the core function `i3c_dev_request_ibi_locked()` allocates `ibi`, it also
> allocates `ibi->wq`.
>
> Also, if the inner call to `i3c_dev_request_ibi_locked()` fails, this function
> returns an error to the original core caller. Will the outer caller's error
> path execute `kfree(ibi)` on its local variable, resulting in a double-free
> since it was already freed here?
>
> > + desc->ibi = NULL;
> > + ret = i3c_dev_request_ibi_locked(desc, req);
> > + if (ret)
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
> > +
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > +
> > + up_write(&parent->bus.lock);
> > +
> > + return ret;
> > +}
> > +
> > +static void i3c_hub_free_ibi(struct i3c_dev_desc *desc)
> > +{
>
> [ ... ]
>
> > + i3c_hub_enable_port(controller);
> > +
> > + down_write(&parent->bus.lock);
> > + orig_parent = i3c_hub_update_desc_parent(&desc->common, parent);
> > + i3c_master_direct_detach_i3c_dev_locked(desc);
> > + i3c_dev_free_ibi_locked(desc);
>
> [Severity: Critical]
> Can this recursive call to `i3c_dev_free_ibi_locked(desc)` cause a null
> pointer dereference?
>
> When the core initially calls `i3c_dev_free_ibi_locked(desc)`, it invokes
> this hub callback. The inner call here successfully frees `desc->ibi` and sets
> it to null.
>
> When control returns to the outer core function, it unconditionally evaluates:
>
> if (dev->ibi->wq) {
>
> Will this immediately dereference the null pointer?
>
> > + i3c_hub_update_desc_parent(&desc->common, orig_parent);
> > + up_write(&parent->bus.lock);
> > +
> > + i3c_hub_disable_port(controller);
> > +}
>
> [ ... ]
>
> > +static void i3c_hub_recycle_ibi_slot(struct i3c_dev_desc *desc,
> > + struct i3c_ibi_slot *slot)
> > +{
> > +}
>
> [Severity: Critical]
> Does leaving this function empty permanently leak hardware IBI slots?
>
> When the core finishes processing an IBI, it returns the slot to the virtual
> controller. Because this doesn't forward the recycle request to the parent
> hardware controller, will the parent controller eventually exhaust its hardware
> slots and permanently stop processing IBIs?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply
* Re: [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
From: Frank Li @ 2026-06-17 15:28 UTC (permalink / raw)
To: sashiko-reviews
Cc: Lakshay Piplani, devicetree, robh, linux-i3c, Frank.Li, conor+dt
In-Reply-To: <20260617111846.EAB851F000E9@smtp.kernel.org>
On Wed, Jun 17, 2026 at 11:18:46AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] The SMBus transaction wait logic uses a fixed sleep instead of polling the hardware completion flag, leading to premature reads of incomplete transactions and data corruption.
> - [High] Overwriting the parent device's `of_node` pointer iteratively causes a memory leak of the original parent node, a double-free of the target port nodes, and state corruption.
> - [Medium] Potential `of_node` reference leak in `p3h2x4x_get_target_port_dt_conf()` if multiple child nodes specify the same target port index.
> - [Medium] The hub's protected registers remain permanently unlocked if the hardware configuration or bus registration fails during probe.
> - [Low] Comment describes a read followed by a write, but the code implements a write followed by a read.
> --
>
> commit 1c982ecf71c6fb3b765caf255191a821a5e37cb5
> Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>
>
> i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
>
> Add I3C hub functionality for the NXP P3H2x4x family of multiport hubs.
> These devices support downstream target ports that can be configured
> as I3C, I2C, or SMBus.
>
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> > new file mode 100644
> > index 0000000000000..05e19bff20d5c
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> [ ... ]
> > +static void p3h2x4x_get_target_port_dt_conf(struct device *dev,
> > + const struct device_node *node)
> > +{
> > + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
> > + u64 tp_port;
> > +
> > + for_each_available_child_of_node_scoped(node, dev_node) {
> > + if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
> > + continue;
> > +
> > + if (tp_port < P3H2X4X_TP_MAX_COUNT) {
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].of_node = of_node_get(dev_node);
>
> [Severity: Medium]
> Could this leak the of_node reference if a malformed device tree specifies
> multiple child nodes with the same target port index?
>
> If multiple child nodes specify the same reg value, the pointer is
> overwritten with a new of_node_get() result, which abandons the previous
> reference.
>
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_mask = P3H2X4X_SET_BIT(tp_port);
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].p3h2x4x_i3c_hub = p3h2x4x_i3c_hub;
> > + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_port = tp_port;
> > + }
> > + }
> > +}
> [ ... ]
> > +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> > +{
> [ ... ]
> > + /* Unlock access to protected registers */
> > + ret = regmap_write(p3h2x4x_i3c_hub->regmap, P3H2X4X_DEV_REG_PROTECTION_CODE,
> > + P3H2X4X_REGISTERS_UNLOCK_CODE);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to unlock HUB's protected registers\n");
> > +
> > + ret = p3h2x4x_configure_hw(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to configure the HUB\n");
>
> [Severity: Medium]
> Are the hub's protected registers left permanently unlocked if probe fails
> during p3h2x4x_configure_hw() or bus registration?
>
> The driver unlocks the hardware registers at the start of probe and locks
> them at the very end. However, if any initialization step fails and returns
> an error, the function exits immediately without locking the registers again.
>
> Could this be mitigated by using a devm_add_action_or_reset() callback or
> explicit error handling labels to ensure the registers are safely relocked?
It is not big problem if device can support P3H2X4X_REGISTERS_UNLOCK_CODE
twice without LOCK. The driver should recover from failure state.
>
> > +
> > + /* Register virtual I3C master controllers for I3C target ports */
> > + if (p3h2x4x->i3cdev) {
> [ ... ]
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> > new file mode 100644
> > index 0000000000000..8adb235b992c0
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> [ ... ]
> > +int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
> > +{
> [ ... ]
> > + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
> > + if (!p3h2x4x_hub->tp_bus[tp].of_node ||
> > + p3h2x4x_hub->hub_config.tp_config[tp].mode != P3H2X4X_TP_MODE_I3C)
> > + continue;
> > +
> > + /* Assign DT node for this TP */
> > + p3h2x4x_hub->dev->of_node = p3h2x4x_hub->tp_bus[tp].of_node;
>
> [Severity: High]
> Does overwriting the parent device's of_node pointer iteratively cause a
> memory leak of the original parent node and a double-free of the target port
> nodes?
Look like AI is correct.
>
> The loop over target ports directly mutates the shared pdev->dev.of_node
> pointer which was populated in probe using device_set_of_node_from_dev().
> This leaks the reference to the original parent node, and leaves
> dev->of_node pointing to the last target port node.
>
> When the driver is unbound, the driver core will drop a reference on
> dev->of_node, but p3h2x4x_put_target_port_of_nodes() also drops a reference
> on the same node, which can cause a use-after-free or double-free refcount
> underflow. Mutating dev->of_node also creates a race with sysfs/uevent.
>
> Should the driver assign the node to controller->dev.of_node directly
> instead of mutating the parent's pointer?
>
> > +
> > + struct i3c_hub_controller *hub_controller =
> > + &p3h2x4x_hub->tp_bus[tp].hub_controller;
> [ ... ]
> > diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> > new file mode 100644
> > index 0000000000000..cf664425af071
> > --- /dev/null
> > +++ b/drivers/i3c/hub/p3h2840_i3c_hub_smbus.c
> [ ... ]
> > +static int p3h2x4x_read_smbus_transaction_status(struct p3h2x4x_i3c_hub_dev *hub,
> > + u8 target_port_status,
> > + u8 data_length)
> > +{
> > + u32 status_read;
> > + u8 status;
> > + int ret;
> > +
> > + fsleep(P3H2X4X_SMBUS_400kHz_TRANSFER_TIMEOUT(data_length));
> > +
> > + ret = regmap_read(hub->regmap, target_port_status, &status_read);
>
> [Severity: High]
> Does using a hardcoded sleep based on bytes transferred instead of polling a
> hardware completion status bit lead to premature reads of incomplete
> transactions?
>
> If the downstream device performs clock stretching or is slow to respond,
> the transfer will take longer than the hardcoded timeout. The driver then
> reads target_port_status and shifts to check the status bits.
>
> Since an ongoing transaction leaves the status bits at 0
> (P3H2X4X_SMBUS_CNTRL_STATUS_TXN_OK), the driver will incorrectly assume the
> transaction has finished successfully. It will then proceed to read stale or
> uninitialized memory buffers and return garbage data.
>
> Could this be updated to use a polling loop checking the finish flag?
Yes, it should use read_poll_timeout here.
>
> > + if (ret)
> > + return ret;
> > +
> > + status = (u8)status_read;
> [ ... ]
> > +static s32 p3h2x4x_tp_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > +{
> [ ... ]
> > + rw = (msgs[msg_count].flags & I2C_M_RD) ? 1 : 0;
> > + if (!rw) {
> > + /* If a read message is immediately followed by a write message to
> > + * the same address, consider combining them into a single transaction.
> > + */
>
> [Severity: Low]
> This isn't a bug, but the comment describes a read followed by a write while
> the code implements a write followed by a read.
>
> The actual code block checks !rw for the first message and flags & I2C_M_RD
> for the second message, which accurately handles a write message immediately
> followed by a read message.
small issue, easy to fix it.
Frank
>
> > + if (msg_count + 1 < num &&
> > + msgs[msg_count].addr == msgs[msg_count + 1].addr &&
> > + (msgs[msg_count + 1].flags & I2C_M_RD)) {
> > + if (msgs[msg_count].len + msgs[msg_count + 1].len >
> > + P3H2X4X_SMBUS_PAYLOAD_SIZE)
> > + return -EINVAL;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: gpu: img,powervr-rogue: Document GE8300 GPU in Renesas R-Car D3
From: Conor Dooley @ 2026-06-17 15:28 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Marek Vasut, Geert Uytterhoeven, Conor Dooley, David Airlie,
Frank Binns, Krzysztof Kozlowski, Maarten Lankhorst, Magnus Damm,
Matt Coster, Maxime Ripard, Rob Herring, Simona Vetter,
Thomas Zimmermann, devicetree, dri-devel, linux-renesas-soc
In-Reply-To: <20260616182802.GB1662668@fsdn.se>
[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]
On Tue, Jun 16, 2026 at 08:28:02PM +0200, Niklas Söderlund wrote:
> On 2026-06-16 19:58:34 +0200, Niklas Söderlund wrote:
> > Document Imagination Technologies PowerVR Rogue GE8300 BNVC 22.67.54.30
> > present in Renesas R-Car R8A77995 D3 SoCs.
> >
> > Compared to other R-Car Gen3 SoCs the D3 only have one power domain and
> > it is always on. Extend the list of special cases for this to also cover
> > R8A77995 and update the description of it.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > * Changes since v1
> > - Sort img,img-ge8300 after img,img-ge7800.
> > - Fold special case for power domain into an existing one and update the
> > description.
> > ---
> > .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > index a1f54dbae3f3..b93f49f1fa0a 100644
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -25,6 +25,11 @@ properties:
> > - renesas,r8a779a0-gpu
> > - const: img,img-ge7800
> > - const: img,img-rogue
> > + - items:
> > + - enum:
> > + - renesas,r8a77995-gpu
> > + - const: img,img-ge8300
> > + - const: img,img-rogue
> > - items:
> > - enum:
> > - ti,am62-gpu
> > @@ -114,6 +119,7 @@ allOf:
> > contains:
> > enum:
> > - img,img-ge7800
> > + - img,img-ge8300
> > - img,img-gx6250
> > - thead,th1520-gpu
> > then:
> > @@ -159,14 +165,14 @@ allOf:
> > - if:
> > properties:
> > compatible:
> > - contains:
>
> The 'contains' node should have been kept, my bad. I wonder why 'make
> dt_binding_check' or `make dtbs_check' did not catch it. Sorry for the
> noise.
Because it's valid syntax, the contains syntax means that it'll match
against things that use the listed compatibles as fallbacks. What you
have done works for exact matches only.
I think recently Rob said the contains syntax is preferred, with some
rationale that escapes me.
>
> > - const: thead,th1520-gpu
> > + enum:
> > + - renesas,r8a77995-gpu
> > + - thead,th1520-gpu
> > then:
> > properties:
> > power-domains:
> > items:
> > - - description: The single, unified power domain for the GPU on the
> > - TH1520 SoC, integrating all internal IP power domains.
> > + - description: The single, unified power domain for the GPU.
> > power-domain-names: false
> > required:
> > - power-domains
> > --
> > 2.54.0
> >
>
> --
> Kind Regards,
> Niklas Söderlund
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v3] dt-bindings: arm: xen: Convert to DT schema
From: Tejas Mutalikdesai @ 2026-06-17 15:28 UTC (permalink / raw)
To: devicetree; +Cc: robh, krzk+dt, conor+dt, sstabellini, Tejas
From: Tejas <tejasmutalikdesai@gmail.com>
Convert the Xen ARM device tree binding documentation from the legacy
plain-text format (Documentation/devicetree/bindings/arm/xen.txt) to
the DT schema format, as required by the modern DT binding process.
Signed-off-by: Tejas Mutalikdesai <tejasmutalikdesai@gmail.com>
---
Changes since v2:
- Replace 'YAML schema' with 'DT schema' in the commit description
- Drop unnecessary '|' block scalars from description fields that do
not need to preserve literal formatting
- Fix unit_address_vs_reg warning: replace $nodename const with a
pattern requiring a unit address; update example to hypervisor@b0000000
and drop the root node, GIC, and interrupt-parent
- Update Documentation reference in arch/arm/xen/enlighten.c from
xen.txt to xen.yaml
Documentation/devicetree/bindings/arm/xen.txt | 62 ----------
.../devicetree/bindings/arm/xen.yaml | 109 ++++++++++++++++++
arch/arm/xen/enlighten.c | 2 +-
3 files changed, 110 insertions(+), 63 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/xen.txt
create mode 100644 Documentation/devicetree/bindings/arm/xen.yaml
diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
deleted file mode 100644
index f925290d4641..000000000000
--- a/Documentation/devicetree/bindings/arm/xen.txt
+++ /dev/null
@@ -1,62 +0,0 @@
-* Xen hypervisor device tree bindings
-
-Xen ARM virtual platforms shall have a top-level "hypervisor" node with
-the following properties:
-
-- compatible:
- compatible = "xen,xen-<version>", "xen,xen";
- where <version> is the version of the Xen ABI of the platform.
-
-- reg: specifies the base physical address and size of the regions in memory
- where the special resources should be mapped to, using an HYPERVISOR_memory_op
- hypercall.
- Region 0 is reserved for mapping grant table, it must be always present.
- The memory region is large enough to map the whole grant table (it is larger
- or equal to gnttab_max_grant_frames()).
- Regions 1...N are extended regions (unused address space) for mapping foreign
- GFNs and grants, they might be absent if there is nothing to expose.
-
-- interrupts: the interrupt used by Xen to inject event notifications.
- A GIC node is also required.
-
-To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
-under /hypervisor with following parameters:
-
-________________________________________________________________________________
-Name | Size | Description
-================================================================================
-xen,uefi-system-table | 64-bit | Guest physical address of the UEFI System
- | | Table.
---------------------------------------------------------------------------------
-xen,uefi-mmap-start | 64-bit | Guest physical address of the UEFI memory
- | | map.
---------------------------------------------------------------------------------
-xen,uefi-mmap-size | 32-bit | Size in bytes of the UEFI memory map
- | | pointed to in previous entry.
---------------------------------------------------------------------------------
-xen,uefi-mmap-desc-size | 32-bit | Size in bytes of each entry in the UEFI
- | | memory map.
---------------------------------------------------------------------------------
-xen,uefi-mmap-desc-ver | 32-bit | Version of the mmap descriptor format.
---------------------------------------------------------------------------------
-
-Example (assuming #address-cells = <2> and #size-cells = <2>):
-
-hypervisor {
- compatible = "xen,xen-4.3", "xen,xen";
- reg = <0 0xb0000000 0 0x20000>;
- interrupts = <1 15 0xf08>;
- uefi {
- xen,uefi-system-table = <0xXXXXXXXX>;
- xen,uefi-mmap-start = <0xXXXXXXXX>;
- xen,uefi-mmap-size = <0xXXXXXXXX>;
- xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
- xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
- };
-};
-
-The format and meaning of the "xen,uefi-*" parameters are similar to those in
-Documentation/arch/arm/uefi.rst, which are provided by the regular UEFI stub. However
-they differ because they are provided by the Xen hypervisor, together with a set
-of UEFI runtime services implemented via hypercalls, see
-http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html.
diff --git a/Documentation/devicetree/bindings/arm/xen.yaml b/Documentation/devicetree/bindings/arm/xen.yaml
new file mode 100644
index 000000000000..a22e950566c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/xen.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/xen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xen hypervisor
+
+maintainers:
+ - Stefano Stabellini <sstabellini@kernel.org>
+
+description:
+ Xen ARM virtual platforms shall have a top-level "hypervisor" node with
+ the properties defined below.
+
+properties:
+ $nodename:
+ pattern: "^hypervisor@[0-9a-f]+$"
+
+ compatible:
+ description:
+ Specifies the Xen hypervisor. The version of the Xen ABI is encoded
+ in the first item as "xen,xen-<version>", followed by the generic
+ "xen,xen" string.
+ items:
+ - pattern: "^xen,xen-[0-9]+\\.[0-9]+$"
+ - const: xen,xen
+
+ reg:
+ description: |
+ Base physical address and size of the regions in memory where special
+ resources should be mapped to, using a HYPERVISOR_memory_op hypercall.
+
+ Region 0 is reserved for mapping the grant table and must always be
+ present. The memory region must be large enough to map the whole grant
+ table (it is larger or equal to gnttab_max_grant_frames()).
+
+ Regions 1...N are extended regions (unused address space) for mapping
+ foreign GFNs and grants. They might be absent if there is nothing to
+ expose.
+ minItems: 1
+
+ interrupts:
+ description:
+ The interrupt used by Xen to inject event notifications.
+ A GIC node is also required.
+ maxItems: 1
+
+ uefi:
+ type: object
+ description:
+ Node populated by Xen to support UEFI on Xen ARM virtual platforms.
+ The format and meaning of the "xen,uefi-*" parameters are similar to
+ those in Documentation/arch/arm/uefi.rst, but are provided by the Xen
+ hypervisor together with a set of UEFI runtime services implemented via
+ hypercalls.
+ properties:
+ xen,uefi-system-table:
+ description: Guest physical address of the UEFI System Table.
+ $ref: /schemas/types.yaml#/definitions/uint64
+
+ xen,uefi-mmap-start:
+ description: Guest physical address of the UEFI memory map.
+ $ref: /schemas/types.yaml#/definitions/uint64
+
+ xen,uefi-mmap-size:
+ description: Size in bytes of the UEFI memory map pointed to by xen,uefi-mmap-start.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xen,uefi-mmap-desc-size:
+ description: Size in bytes of each entry in the UEFI memory map.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ xen,uefi-mmap-desc-ver:
+ description: Version of the mmap descriptor format.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ required:
+ - xen,uefi-system-table
+ - xen,uefi-mmap-start
+ - xen,uefi-mmap-size
+ - xen,uefi-mmap-desc-size
+ - xen,uefi-mmap-desc-ver
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ hypervisor@b0000000 {
+ compatible = "xen,xen-4.3", "xen,xen";
+ reg = <0xb0000000 0x20000>;
+ interrupts = <1 15 0xf08>;
+
+ uefi {
+ xen,uefi-system-table = /bits/ 64 <0x1301415>;
+ xen,uefi-mmap-start = /bits/ 64 <0x7591400>;
+ xen,uefi-mmap-size = <0x1800>;
+ xen,uefi-mmap-desc-size = <0x30>;
+ xen,uefi-mmap-desc-ver = <1>;
+ };
+ };
+...
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 25a0ce3b4584..0b7b7e3417e3 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -251,7 +251,7 @@ static int __init fdt_find_hyper_node(unsigned long node, const char *uname,
}
/*
- * see Documentation/devicetree/bindings/arm/xen.txt for the
+ * see Documentation/devicetree/bindings/arm/xen.yaml for the
* documentation of the Xen Device Tree format.
*/
void __init xen_early_init(void)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] drm/tiny: add support for PIXPAPER 4.26 monochrome e-ink panel
From: Thomas Zimmermann @ 2026-06-17 15:31 UTC (permalink / raw)
To: LiangCheng Wang, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Wig Cheng
Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20260529-bar-v3-2-5c2ac1c751ee@gmail.com>
Hi,
apologies for being late with the review.
Am 29.05.26 um 12:31 schrieb LiangCheng Wang:
> Introduce a DRM driver for the Mayqueen Pixpaper 4.26
> monochrome e-ink display panel, which is controlled via SPI.
> The driver supports an 800x480 display with XRGB8888
> framebuffer input.
>
> Also, add Kconfig and Makefile entries for the driver and
> update MAINTAINERS for the Pixpaper DRM drivers and binding.
>
> Signed-off-by: LiangCheng Wang <zaq14760@gmail.com>
> ---
> MAINTAINERS | 3 +-
> drivers/gpu/drm/tiny/Kconfig | 16 +
> drivers/gpu/drm/tiny/Makefile | 1 +
> drivers/gpu/drm/tiny/pixpaper-426m.c | 817 +++++++++++++++++++++++++++++++++++
> 4 files changed, 836 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 882214b0e7db53bb8cc8e75b5d2269ee0591ea20..eebd73ee1f531d3785ec963da03fbab265c2d188 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8234,11 +8234,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
> F: Documentation/devicetree/bindings/display/repaper.txt
> F: drivers/gpu/drm/tiny/repaper.c
>
> -DRM DRIVER FOR PIXPAPER E-INK PANEL
> +DRM DRIVER FOR PIXPAPER E-INK PANELS
> M: LiangCheng Wang <zaq14760@gmail.com>
> L: dri-devel@lists.freedesktop.org
> S: Maintained
> F: Documentation/devicetree/bindings/display/mayqueen,pixpaper.yaml
> +F: drivers/gpu/drm/tiny/pixpaper-426m.c
> F: drivers/gpu/drm/tiny/pixpaper.c
>
> DRM DRIVER FOR QEMU'S CIRRUS DEVICE
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f0e72d4b6a4709564e63c758e857bdb4a320dbe7..028c4314106ac31dfa717f6433c28e58b34c21e8 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -98,6 +98,22 @@ config DRM_PIXPAPER
>
> If M is selected, the module will be built as pixpaper.ko.
>
> +config DRM_PIXPAPER_426M
> + tristate "DRM support for PIXPAPER 4.26 monochrome display panel"
> + depends on DRM && SPI
> + depends on MMU
> + select DRM_CLIENT_SELECTION
> + select DRM_GEM_SHMEM_HELPER
> + select DRM_KMS_HELPER
> + help
> + DRM driver for the Mayqueen Pixpaper 4.26 monochrome e-ink
> + display panel.
> +
> + This driver supports SPI-connected 800x480 monochrome panels
> + with an XRGB8888 framebuffer input format.
> +
> + If M is selected, the module will be built as pixpaper-426m.ko.
> +
> config TINYDRM_HX8357D
> tristate "DRM support for HX8357D display panels"
> depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 48d30bf6152f979404ac1004174587823a30109e..037b751a1a851cc2f86f701ff71008bcb9c59f29 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus-qemu.o
> obj-$(CONFIG_DRM_GM12U320) += gm12u320.o
> obj-$(CONFIG_DRM_PANEL_MIPI_DBI) += panel-mipi-dbi.o
> obj-$(CONFIG_DRM_PIXPAPER) += pixpaper.o
> +obj-$(CONFIG_DRM_PIXPAPER_426M) += pixpaper-426m.o
> obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
> obj-$(CONFIG_TINYDRM_ILI9163) += ili9163.o
> obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
> diff --git a/drivers/gpu/drm/tiny/pixpaper-426m.c b/drivers/gpu/drm/tiny/pixpaper-426m.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..99464d564f315543037a3621ff85f98f1bd8f34c
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/pixpaper-426m.c
> @@ -0,0 +1,817 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DRM driver for PIXPAPER 4.26 monochrome e-ink panel
> + *
> + * Author: LiangCheng Wang <zaq14760@gmail.com>,
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/string.h>
> +
> +#include <drm/clients/drm_client_setup.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_shmem.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> +
> +MODULE_IMPORT_NS("DMA_BUF");
> +
> +/* Panel visible resolution */
> +#define PIXPAPER_WIDTH 800
> +#define PIXPAPER_HEIGHT 480
We already use the prefixes PIXPAPER_, and pixpaper, and variants
thereof in the current pixpaper driver. You have to choose a different
prefix. If I may suggest one, pix426m_ would be fine.
> +
> +/*
> + * The panel datasheet specifies an active area of 92.8 mm x 55.68 mm.
> + * Round to whole millimeters for drm_display_info.
> + */
> +#define PIXPAPER_WIDTH_MM 93
> +#define PIXPAPER_HEIGHT_MM 56
> +
> +/*
> + * According to the panel datasheet, no RGB-style timing parameters
> + * (porches, sync widths, or a dot clock) are provided. Define a minimal
> + * fixed mode only to satisfy the DRM mode API for this SPI-driven
> + * e-paper panel.
> + */
> +#define PIXPAPER_HSYNC_LEN 1
> +#define PIXPAPER_HFRONT_PORCH 1
> +#define PIXPAPER_HBACK_PORCH 1
> +#define PIXPAPER_VSYNC_LEN 1
> +#define PIXPAPER_VFRONT_PORCH 1
> +#define PIXPAPER_VBACK_PORCH 1
> +#define PIXPAPER_MODE_REFRESH_HZ 1
> +#define PIXPAPER_MODE_CLOCK_KHZ \
> + (((PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN + \
> + PIXPAPER_HBACK_PORCH) * \
> + (PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN + \
> + PIXPAPER_VBACK_PORCH) * \
> + PIXPAPER_MODE_REFRESH_HZ) / 1000)
You can, I think, drop all these constants. Further below where you
define a display mode, just use DRM_SIMPLE_MODE() with the pixel
resolution and physical dimensions. This is how other drivers make up
display modes.
> +
> +#define PIXPAPER_SPI_BITS_PER_WORD 8
> +#define PIXPAPER_SPI_SPEED_DEFAULT 1000000
> +
> +#define PIXPAPER_TX_BUF_SIZE 8
> +
> +#define PIXPAPER_PIXEL_THRESHOLD 128
> +
> +#define PIXPAPER_BUSY_TIMEOUT_MS 10000
> +#define PIXPAPER_BUSY_POLL_INITIAL_US_MIN 1000
> +#define PIXPAPER_BUSY_POLL_INITIAL_US_MAX 1500
> +#define PIXPAPER_BUSY_POLL_US_MIN 100
> +#define PIXPAPER_BUSY_POLL_US_MAX 200
> +
> +#define PIXPAPER_RAM_START_ADDR 0x00
> +
> +#define PIXPAPER_LUMA_R_WEIGHT 299
> +#define PIXPAPER_LUMA_G_WEIGHT 587
> +#define PIXPAPER_LUMA_B_WEIGHT 114
> +#define PIXPAPER_LUMA_DIVISOR 1000
> +#define PIXPAPER_LUMA_ROUNDING_BIAS 500
> +
> +#define PIXPAPER_CMD_DRIVER_OUTPUT_CTRL 0x01
> +#define PIXPAPER_CMD_BOOSTER_SOFT_START_CTRL 0x0C
> +#define PIXPAPER_CMD_TEMP_SENSOR_CONTROL 0x18
> +#define PIXPAPER_CMD_MASTER_ACTIVATION 0x20
> +#define PIXPAPER_CMD_DISPLAY_UPDATE_CTRL2 0x22
> +#define PIXPAPER_CMD_WRITE_RAM_BW 0x24
> +#define PIXPAPER_CMD_BORDER_WAVEFORM_CONTROL 0x3C
> +#define PIXPAPER_CMD_SET_RAM_X_START_END 0x44
> +#define PIXPAPER_CMD_SET_RAM_Y_START_END 0x45
> +#define PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER 0x4E
> +#define PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER 0x4F
> +
> +#define PIXPAPER_DRIVER_OUTPUT_SM BIT(1)
> +
> +#define PIXPAPER_BORDER_WAVEFORM_GS_TRANSITION (0x0 << 6)
> +#define PIXPAPER_BORDER_WAVEFORM_LUT1_SEL 0x1
> +
> +#define PIXPAPER_UPDATE_CTRL2_ENABLE_CLK BIT(7)
> +#define PIXPAPER_UPDATE_CTRL2_ENABLE_ANALOG BIT(6)
> +#define PIXPAPER_UPDATE_CTRL2_LOAD_TEMP BIT(5)
> +#define PIXPAPER_UPDATE_CTRL2_LOAD_LUT BIT(4)
> +#define PIXPAPER_UPDATE_CTRL2_PATTERN_DISPLAY BIT(2)
> +
> +#define PIXPAPER_TEMP_SENSOR_INTERNAL 0x80
> +#define PIXPAPER_SOFTSTART_A 0xAE
> +#define PIXPAPER_SOFTSTART_B 0xC7
> +#define PIXPAPER_SOFTSTART_C 0xC3
> +#define PIXPAPER_SOFTSTART_D 0xC0
> +#define PIXPAPER_SOFTSTART_E 0x80
> +#define PIXPAPER_DRIVER_OUTPUT_GD_SM_TB PIXPAPER_DRIVER_OUTPUT_SM
> +#define PIXPAPER_BORDER_LUT1 \
> + (PIXPAPER_BORDER_WAVEFORM_GS_TRANSITION | \
> + PIXPAPER_BORDER_WAVEFORM_LUT1_SEL)
> +#define PIXPAPER_UPDATE_INITIAL \
> + (PIXPAPER_UPDATE_CTRL2_ENABLE_CLK | \
> + PIXPAPER_UPDATE_CTRL2_ENABLE_ANALOG | \
> + PIXPAPER_UPDATE_CTRL2_LOAD_TEMP | \
> + PIXPAPER_UPDATE_CTRL2_LOAD_LUT | \
> + PIXPAPER_UPDATE_CTRL2_PATTERN_DISPLAY)
Empty line here?
> +struct pixpaper_error_ctx {
> + int errno_code;
> +};
> +
> +struct pixpaper_init_seq {
> + u8 cmd;
> + const u8 *data;
> + u8 len;
> +};
> +
> +struct pixpaper_panel {
> + struct drm_device drm;
> + struct drm_plane plane;
> + struct drm_crtc crtc;
> + struct drm_encoder encoder;
> + struct drm_connector connector;
> +
> + struct spi_device *spi;
> + struct gpio_desc *reset;
> + struct gpio_desc *busy;
> + struct gpio_desc *dc;
> +
> + u8 *tx_buf;
> +};
> +
> +static const uint32_t pixpaper_formats[] = {
> + DRM_FORMAT_XRGB8888,
> +};
> +
> +static const u8 pixpaper_init_temp_sensor[] = {
> + PIXPAPER_TEMP_SENSOR_INTERNAL,
> +};
> +
> +static const u8 pixpaper_init_softstart[] = {
> + PIXPAPER_SOFTSTART_A,
> + PIXPAPER_SOFTSTART_B,
> + PIXPAPER_SOFTSTART_C,
> + PIXPAPER_SOFTSTART_D,
> + PIXPAPER_SOFTSTART_E,
> +};
> +
> +static const u8 pixpaper_init_driver_output[] = {
> + (PIXPAPER_HEIGHT - 1) & 0xff,
> + (PIXPAPER_HEIGHT - 1) >> 8,
> + PIXPAPER_DRIVER_OUTPUT_GD_SM_TB,
> +};
> +
> +static const u8 pixpaper_init_border[] = {
> + PIXPAPER_BORDER_LUT1,
> +};
> +
> +static const u8 pixpaper_init_ram_x_window[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> + (PIXPAPER_WIDTH - 1) & 0xff,
> + (PIXPAPER_WIDTH - 1) >> 8,
> +};
> +
> +static const u8 pixpaper_init_ram_y_window[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> + (PIXPAPER_HEIGHT - 1) & 0xff,
> + (PIXPAPER_HEIGHT - 1) >> 8,
> +};
> +
> +static const u8 pixpaper_init_ram_x_counter[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> +};
> +
> +static const u8 pixpaper_init_ram_y_counter[] = {
> + PIXPAPER_RAM_START_ADDR,
> + PIXPAPER_RAM_START_ADDR,
> +};
> +
> +static const struct pixpaper_init_seq pixpaper_init_seqs[] = {
> + {
> + .cmd = PIXPAPER_CMD_TEMP_SENSOR_CONTROL,
> + .data = pixpaper_init_temp_sensor,
> + .len = ARRAY_SIZE(pixpaper_init_temp_sensor),
> + },
> + {
> + .cmd = PIXPAPER_CMD_BOOSTER_SOFT_START_CTRL,
> + .data = pixpaper_init_softstart,
> + .len = ARRAY_SIZE(pixpaper_init_softstart),
> + },
> + {
> + .cmd = PIXPAPER_CMD_DRIVER_OUTPUT_CTRL,
> + .data = pixpaper_init_driver_output,
> + .len = ARRAY_SIZE(pixpaper_init_driver_output),
> + },
> + {
> + .cmd = PIXPAPER_CMD_BORDER_WAVEFORM_CONTROL,
> + .data = pixpaper_init_border,
> + .len = ARRAY_SIZE(pixpaper_init_border),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_X_START_END,
> + .data = pixpaper_init_ram_x_window,
> + .len = ARRAY_SIZE(pixpaper_init_ram_x_window),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_Y_START_END,
> + .data = pixpaper_init_ram_y_window,
> + .len = ARRAY_SIZE(pixpaper_init_ram_y_window),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER,
> + .data = pixpaper_init_ram_x_counter,
> + .len = ARRAY_SIZE(pixpaper_init_ram_x_counter),
> + },
> + {
> + .cmd = PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER,
> + .data = pixpaper_init_ram_y_counter,
> + .len = ARRAY_SIZE(pixpaper_init_ram_y_counter),
> + },
> +};
> +
> +static inline struct pixpaper_panel *to_pixpaper_panel(struct drm_device *drm)
> +{
> + return container_of(drm, struct pixpaper_panel, drm);
> +}
> +
> +static void pixpaper_wait_for_panel(struct pixpaper_panel *panel)
> +{
> + unsigned int timeout_ms = PIXPAPER_BUSY_TIMEOUT_MS;
> + unsigned long timeout_jiffies = jiffies + msecs_to_jiffies(timeout_ms);
> +
> + usleep_range(PIXPAPER_BUSY_POLL_INITIAL_US_MIN,
> + PIXPAPER_BUSY_POLL_INITIAL_US_MAX);
> + while (gpiod_get_value_cansleep(panel->busy) != 0) {
> + if (time_after(jiffies, timeout_jiffies)) {
> + /*
> + * Treat a busy timeout as warning-only. Some panels may
> + * keep BUSY asserted longer than expected during
> + * initialization or refresh.
> + */
> + drm_warn(&panel->drm, "Busy wait timed out\n");
> + return;
> + }
> + usleep_range(PIXPAPER_BUSY_POLL_US_MIN,
> + PIXPAPER_BUSY_POLL_US_MAX);
> + }
> +}
> +
> +static void pixpaper_spi_write(struct pixpaper_panel *panel, int dc,
> + const void *buf, size_t len,
> + struct pixpaper_error_ctx *err)
> +{
> + int ret;
> +
> + if (err->errno_code || !len)
> + return;
> +
> + gpiod_set_value_cansleep(panel->dc, dc);
> + usleep_range(1, 5);
> +
> + ret = spi_write(panel->spi, buf, len);
> + if (ret < 0)
> + err->errno_code = ret;
> +}
> +
> +static void pixpaper_send_cmd(struct pixpaper_panel *panel, u8 cmd,
> + struct pixpaper_error_ctx *err)
> +{
> + panel->tx_buf[0] = cmd;
> + pixpaper_spi_write(panel, 0, panel->tx_buf, sizeof(cmd), err);
> +}
> +
> +static void pixpaper_send_data(struct pixpaper_panel *panel, u8 data,
> + struct pixpaper_error_ctx *err)
> +{
> + panel->tx_buf[0] = data;
> + pixpaper_spi_write(panel, 1, panel->tx_buf, sizeof(data), err);
> +}
> +
> +static void pixpaper_reset_ram_counters(struct pixpaper_panel *panel,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_SET_RAM_X_ADDR_COUNTER, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_SET_RAM_Y_ADDR_COUNTER, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> + pixpaper_send_data(panel, PIXPAPER_RAM_START_ADDR, err);
> +}
> +
> +static void pixpaper_write_ram(struct pixpaper_panel *panel, u8 cmd,
> + const u8 *buf, u32 len,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code || !buf || !len)
> + return;
> +
> + pixpaper_reset_ram_counters(panel, err);
> +
> + pixpaper_send_cmd(panel, cmd, err);
> + pixpaper_spi_write(panel, 1, buf, len, err);
> +}
> +
> +static void pixpaper_send_init_seq(struct pixpaper_panel *panel,
> + const struct pixpaper_init_seq *seq,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code || !seq->data || !seq->len)
> + return;
> +
> + if (seq->len > PIXPAPER_TX_BUF_SIZE) {
> + err->errno_code = -EINVAL;
> + return;
> + }
> +
> + pixpaper_send_cmd(panel, seq->cmd, err);
> + memcpy(panel->tx_buf, seq->data, seq->len);
> + pixpaper_spi_write(panel, 1, panel->tx_buf, seq->len, err);
> +}
> +
> +static void pixpaper_trigger_update(struct pixpaper_panel *panel,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_DISPLAY_UPDATE_CTRL2, err);
> + pixpaper_send_data(panel, PIXPAPER_UPDATE_INITIAL, err);
> + pixpaper_send_cmd(panel, PIXPAPER_CMD_MASTER_ACTIVATION, err);
> + pixpaper_wait_for_panel(panel);
> +}
> +
> +static void pixpaper_xrgb8888_to_bw(const void *src, void *dst, u32 height,
> + u32 width, u32 src_pitch, u32 dst_pitch)
The code in this function is very similar to drm_fb_xrgb8888_to_mono();
just reversed. Unfortunately, there's no simple way of reusing anything.
https://elixir.bootlin.com/linux/v7.1/source/drivers/gpu/drm/drm_format_helper.c#L1204
> +{
> + const uint8_t *src_base = src;
> + uint8_t *dst_pixels = dst;
> +
> + if (dst == NULL || src == NULL)
> + return;
> +
> + for (u32 y = 0; y < height; y++) {
> + uint8_t *dst_row = dst_pixels + y * dst_pitch;
> + const __le32 *src_pixels =
> + (const __le32 *)(src_base + y * src_pitch);
> +
> + for (u32 x = 0; x < width; x++) {
> + /*
> + * The panel RAM X direction is reversed relative to DRM
> + * coordinates. Read pixels from right to left so the
> + * displayed image matches the expected orientation on
> + * the panel.
> + */
> + u32 src_x = width - 1 - x;
> + uint8_t r, g, b;
> + u8 bit;
> + u32 bit_pos = x % 8;
> + u32 byte_pos = x / 8;
> + uint32_t gray_val;
> + uint32_t pixel;
> +
> + pixel = le32_to_cpu(src_pixels[src_x]);
> + r = (pixel >> 16) & 0xFF;
> + g = (pixel >> 8) & 0xFF;
> + b = pixel & 0xFF;
> +
> + gray_val = (r * PIXPAPER_LUMA_R_WEIGHT +
> + g * PIXPAPER_LUMA_G_WEIGHT +
> + b * PIXPAPER_LUMA_B_WEIGHT +
> + PIXPAPER_LUMA_ROUNDING_BIAS) /
> + PIXPAPER_LUMA_DIVISOR;
> + bit = gray_val >= PIXPAPER_PIXEL_THRESHOLD;
> +
> + if (bit)
> + dst_row[byte_pos] |= BIT(7 - bit_pos);
> + else
> + dst_row[byte_pos] &= ~BIT(7 - bit_pos);
> + }
> + }
> +}
> +
> +static void *pixpaper_prepare_buffer(const void *vaddr,
> + const struct drm_framebuffer *fb,
> + u32 *dst_pitch,
> + struct pixpaper_error_ctx *err)
> +{
> + void *dst;
> +
> + if (err->errno_code)
> + return NULL;
> +
> + *dst_pitch = DIV_ROUND_UP(fb->width, 8);
> + dst = kzalloc(*dst_pitch * fb->height, GFP_KERNEL);
> + if (!dst) {
> + err->errno_code = -ENOMEM;
> + return NULL;
> + }
> +
> + pixpaper_xrgb8888_to_bw(vaddr, dst, fb->height, fb->width,
> + fb->pitches[0], *dst_pitch);
> +
> + return dst;
> +}
> +
> +static void pixpaper_write_image(struct pixpaper_panel *panel,
> + const u8 *buf, u32 len,
> + struct pixpaper_error_ctx *err)
> +{
> + if (err->errno_code)
> + return;
> +
> + pixpaper_write_ram(panel, PIXPAPER_CMD_WRITE_RAM_BW, buf, len, err);
> +}
> +
> +static int pixpaper_panel_hw_init(struct pixpaper_panel *panel)
> +{
> + struct pixpaper_error_ctx err = { .errno_code = 0 };
> + u8 i;
> +
> + gpiod_set_value_cansleep(panel->reset, 0);
> + msleep(50);
> + gpiod_set_value_cansleep(panel->reset, 1);
> + msleep(50);
> +
> + pixpaper_wait_for_panel(panel);
> +
> + for (i = 0; i < ARRAY_SIZE(pixpaper_init_seqs); i++) {
> + pixpaper_send_init_seq(panel, &pixpaper_init_seqs[i], &err);
> + if (err.errno_code)
> + goto init_fail;
> + }
> +
> + return 0;
> +
> +init_fail:
> + drm_err(&panel->drm, "Hardware initialization failed (err=%d)\n",
> + err.errno_code);
> + return err.errno_code;
> +}
> +
> +static int pixpaper_plane_helper_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *new_plane_state =
> + drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc *new_crtc = new_plane_state->crtc;
> + struct drm_crtc_state *new_crtc_state = NULL;
> + int ret;
> +
> + if (new_crtc)
> + new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> + new_crtc_state, DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING, false, false);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int pixpaper_crtc_helper_atomic_check(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state =
> + drm_atomic_get_new_crtc_state(state, crtc);
> +
> + if (!crtc_state->enable)
> + return 0;
> +
> + return drm_atomic_helper_check_crtc_primary_plane(crtc_state);
> +}
> +
> +static void pixpaper_crtc_atomic_enable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> + struct drm_device *drm = &panel->drm;
> + int idx;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + drm_dev_exit(idx);
Simply keep this helper empty.
> +}
> +
> +static void pixpaper_crtc_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> + struct drm_device *drm = &panel->drm;
> + int idx;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + drm_dev_exit(idx);
Simply keep this helper empty.
> +}
> +
> +static void pixpaper_plane_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *plane_state =
> + drm_atomic_get_new_plane_state(state, plane);
> + struct drm_shadow_plane_state *shadow_plane_state =
> + to_drm_shadow_plane_state(plane_state);
> + struct pixpaper_panel *panel = to_pixpaper_panel(plane->dev);
> +
> + if (!plane_state->crtc || !plane_state->fb || !plane_state->visible)
> + return;
> +
> + {
No such blocks, please.
> + struct drm_device *drm = &panel->drm;
> + struct drm_framebuffer *fb = plane_state->fb;
> + struct iosys_map map = shadow_plane_state->data[0];
> + const void *vaddr = map.vaddr;
> + int idx;
> + struct pixpaper_error_ctx err = { .errno_code = 0 };
> + uint32_t dst_pitch;
> + void *dst = NULL;
> + u32 dst_len;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + if (fb->format->format != DRM_FORMAT_XRGB8888) {
No need to do a format check. The DRM framework checks this for you.
> + err.errno_code = -EINVAL;
> + drm_err_once(drm, "Unsupported framebuffer format: 0x%08x\n",
> + fb->format->format);
> + goto update_cleanup;
> + }
> +
> + dst = pixpaper_prepare_buffer(vaddr, fb, &dst_pitch, &err);
> + if (err.errno_code) {
> + drm_err_once(drm, "Failed to allocate temporary buffer\n");
> + goto update_cleanup;
> + }
This call allocates memory. atomic_update is not a good place to do that.
While you operate on the framebuffer memory, you need to use
drm_gem_fb_begin_cpu_access() and drm_gem_fb_end_cpu_access().
Otherwise, another DMA and other drivers can mess around with your
buffer content. See the other drivers for examples.
> +
> + dst_len = dst_pitch * fb->height;
> + pixpaper_write_image(panel, dst, dst_len, &err);
> + if (err.errno_code)
> + goto update_cleanup;
And this call always writes out the full image, I think. Is it possible
to use damage clipping like the other drivers do?
> +
> + pixpaper_trigger_update(panel, &err);
> + if (err.errno_code)
> + goto update_cleanup;
> +update_cleanup:
> + if (err.errno_code && err.errno_code != -ETIMEDOUT)
> + drm_err_once(drm, "Frame update failed: %d\n",
> + err.errno_code);
> +
> + kfree(dst);
And here you free the allocated memory. There should never be a reason
to allocate more than 800x400 bits. I think you can pre-allocate the
buffer in the main device structure and re-use it on each transfer.
Best regards
Thomas
> + drm_dev_exit(idx);
> + }
> +}
> +
> +static const struct drm_display_mode pixpaper_mode = {
> + .clock = PIXPAPER_MODE_CLOCK_KHZ,
> + .hdisplay = PIXPAPER_WIDTH,
> + .hsync_start = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH,
> + .hsync_end = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN,
> + .htotal = PIXPAPER_WIDTH + PIXPAPER_HFRONT_PORCH + PIXPAPER_HSYNC_LEN +
> + PIXPAPER_HBACK_PORCH,
> + .vdisplay = PIXPAPER_HEIGHT,
> + .vsync_start = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH,
> + .vsync_end = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN,
> + .vtotal = PIXPAPER_HEIGHT + PIXPAPER_VFRONT_PORCH + PIXPAPER_VSYNC_LEN +
> + PIXPAPER_VBACK_PORCH,
> + .width_mm = PIXPAPER_WIDTH_MM,
> + .height_mm = PIXPAPER_HEIGHT_MM,
> + .type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static int pixpaper_connector_get_modes(struct drm_connector *connector)
> +{
> + return drm_connector_helper_get_modes_fixed(connector, &pixpaper_mode);
> +}
> +
> +static enum drm_mode_status
> +pixpaper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode)
> +{
> + if (mode->hdisplay == PIXPAPER_WIDTH &&
> + mode->vdisplay == PIXPAPER_HEIGHT)
> + return MODE_OK;
> +
> + return MODE_BAD;
> +}
> +
> +static const struct drm_plane_funcs pixpaper_plane_funcs = {
> + .update_plane = drm_atomic_helper_update_plane,
> + .disable_plane = drm_atomic_helper_disable_plane,
> + .destroy = drm_plane_cleanup,
> + DRM_GEM_SHADOW_PLANE_FUNCS,
> +};
> +
> +static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = {
> + DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> + .atomic_check = pixpaper_plane_helper_atomic_check,
> + .atomic_update = pixpaper_plane_atomic_update,
> +};
> +
> +static const struct drm_crtc_funcs pixpaper_crtc_funcs = {
> + .set_config = drm_atomic_helper_set_config,
> + .page_flip = drm_atomic_helper_page_flip,
> + .reset = drm_atomic_helper_crtc_reset,
> + .destroy = drm_crtc_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static const struct drm_crtc_helper_funcs pixpaper_crtc_helper_funcs = {
> + .mode_valid = pixpaper_mode_valid,
> + .atomic_check = pixpaper_crtc_helper_atomic_check,
> + .atomic_enable = pixpaper_crtc_atomic_enable,
> + .atomic_disable = pixpaper_crtc_atomic_disable,
> +};
> +
> +static const struct drm_encoder_funcs pixpaper_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +static const struct drm_connector_funcs pixpaper_connector_funcs = {
> + .reset = drm_atomic_helper_connector_reset,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_connector_helper_funcs pixpaper_connector_helper_funcs = {
> + .get_modes = pixpaper_connector_get_modes,
> +};
> +
> +DEFINE_DRM_GEM_FOPS(pixpaper_fops);
> +
> +static struct drm_driver pixpaper_drm_driver = {
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> + .fops = &pixpaper_fops,
> + .name = "pixpaper-426m",
> + .desc = "DRM driver for PIXPAPER 4.26 monochrome e-ink panel",
> + .major = 1,
> + .minor = 0,
> + DRM_GEM_SHMEM_DRIVER_OPS,
> + DRM_FBDEV_SHMEM_DRIVER_OPS,
> +};
> +
> +static const struct drm_mode_config_funcs pixpaper_mode_config_funcs = {
> + .fb_create = drm_gem_fb_create_with_dirty,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int pixpaper_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct pixpaper_panel *panel;
> + struct drm_device *drm;
> + int ret;
> +
> + panel = devm_drm_dev_alloc(dev, &pixpaper_drm_driver,
> + struct pixpaper_panel, drm);
> + if (IS_ERR(panel))
> + return PTR_ERR(panel);
> +
> + drm = &panel->drm;
> + panel->spi = spi;
> + spi_set_drvdata(spi, panel);
> +
> + panel->tx_buf = devm_kzalloc(dev, PIXPAPER_TX_BUF_SIZE, GFP_KERNEL);
> + if (!panel->tx_buf)
> + return -ENOMEM;
> +
> + ret = drmm_mode_config_init(drm);
> + if (ret)
> + return ret;
> +
> + spi->mode = SPI_MODE_0;
> + spi->bits_per_word = PIXPAPER_SPI_BITS_PER_WORD;
> +
> + if (!spi->max_speed_hz) {
> + drm_warn(drm,
> + "spi-max-frequency not specified in DT, using default %u Hz\n",
> + PIXPAPER_SPI_SPEED_DEFAULT);
> + spi->max_speed_hz = PIXPAPER_SPI_SPEED_DEFAULT;
> + }
> +
> + ret = spi_setup(spi);
> + if (ret < 0) {
> + drm_err(drm, "SPI setup failed: %d\n", ret);
> + return ret;
> + }
> +
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret) {
> + drm_err(drm, "Failed to set DMA mask: %d\n", ret);
> + return ret;
> + }
> +
> + panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(panel->reset))
> + return PTR_ERR(panel->reset);
> +
> + panel->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
> + if (IS_ERR(panel->busy))
> + return PTR_ERR(panel->busy);
> +
> + panel->dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_HIGH);
> + if (IS_ERR(panel->dc))
> + return PTR_ERR(panel->dc);
> +
> + ret = pixpaper_panel_hw_init(panel);
> + if (ret) {
> + drm_err(drm, "Panel hardware initialization failed: %d\n", ret);
> + return ret;
> + }
> +
> + drm->mode_config.funcs = &pixpaper_mode_config_funcs;
> + drm->mode_config.min_width = PIXPAPER_WIDTH;
> + drm->mode_config.max_width = PIXPAPER_WIDTH;
> + drm->mode_config.min_height = PIXPAPER_HEIGHT;
> + drm->mode_config.max_height = PIXPAPER_HEIGHT;
> +
> + ret = drm_universal_plane_init(drm, &panel->plane, 1, &pixpaper_plane_funcs,
> + pixpaper_formats, ARRAY_SIZE(pixpaper_formats), NULL,
> + DRM_PLANE_TYPE_PRIMARY, NULL);
> + if (ret)
> + return ret;
> + drm_plane_helper_add(&panel->plane, &pixpaper_plane_helper_funcs);
> +
> + ret = drm_crtc_init_with_planes(drm, &panel->crtc, &panel->plane, NULL,
> + &pixpaper_crtc_funcs, NULL);
> + if (ret)
> + return ret;
> + drm_crtc_helper_add(&panel->crtc, &pixpaper_crtc_helper_funcs);
> +
> + ret = drm_encoder_init(drm, &panel->encoder, &pixpaper_encoder_funcs,
> + DRM_MODE_ENCODER_NONE, NULL);
> + if (ret)
> + return ret;
> +
> + ret = drm_connector_init(drm, &panel->connector,
> + &pixpaper_connector_funcs,
> + DRM_MODE_CONNECTOR_SPI);
> + if (ret)
> + return ret;
> +
> + drm_connector_helper_add(&panel->connector,
> + &pixpaper_connector_helper_funcs);
> + drm_connector_attach_encoder(&panel->connector, &panel->encoder);
> + panel->encoder.possible_crtcs = drm_crtc_mask(&panel->crtc);
> +
> + drm_mode_config_reset(drm);
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret)
> + return ret;
> +
> + drm_client_setup(drm, NULL);
> +
> + return 0;
> +}
> +
> +static void pixpaper_remove(struct spi_device *spi)
> +{
> + struct pixpaper_panel *panel = spi_get_drvdata(spi);
> +
> + if (!panel)
> + return;
> +
> + drm_dev_unplug(&panel->drm);
> + drm_atomic_helper_shutdown(&panel->drm);
> +}
> +
> +static const struct spi_device_id pixpaper_ids[] = { { "pixpaper-426m", 0 }, {} };
> +MODULE_DEVICE_TABLE(spi, pixpaper_ids);
> +
> +static const struct of_device_id pixpaper_dt_ids[] = {
> + { .compatible = "mayqueen,pixpaper-426m" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, pixpaper_dt_ids);
> +
> +static struct spi_driver pixpaper_spi_driver = {
> + .driver = {
> + .name = "pixpaper-426m",
> + .of_match_table = pixpaper_dt_ids,
> + },
> + .id_table = pixpaper_ids,
> + .probe = pixpaper_probe,
> + .remove = pixpaper_remove,
> +};
> +
> +module_spi_driver(pixpaper_spi_driver);
> +
> +MODULE_AUTHOR("LiangCheng Wang");
> +MODULE_DESCRIPTION("DRM SPI driver for PIXPAPER 4.26 monochrome e-ink panel");
> +MODULE_LICENSE("GPL");
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
From: Conor Dooley @ 2026-06-17 15:34 UTC (permalink / raw)
To: David Lechner
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
In-Reply-To: <c24982d7-40d6-4fd6-a083-90a8d3ce7f63@baylibre.com>
[-- Attachment #1: Type: text/plain, Size: 3818 bytes --]
On Tue, Jun 16, 2026 at 04:04:43PM -0500, David Lechner wrote:
> On 6/16/26 3:50 PM, Conor Dooley wrote:
> > On Tue, Jun 16, 2026 at 02:54:55PM -0500, David Lechner wrote:
> >> On 6/16/26 11:07 AM, Conor Dooley wrote:
> >>> On Mon, Jun 15, 2026 at 04:59:59PM -0500, David Lechner (TI) wrote:
> >>>> Add new bindings for ti,ads122c14 and similar devices.
> >>>>
> >>>> This is an ADC that is primarily intended for use with temperature
> >>>> sensors. There are a few unusual properties because of this. In
> >>>> particular, the reference voltage source and current output requirements
> >>>> can be different for each measurement, so these are included in the
> >>>> channel bindings.
> >>>>
> >>>> The REFP/REFN reference voltage is usually just connected to a resistor
> >>>> that is being driven by the ADC's current outputs, so there is special
> >>>> property for this case rather than requiring a regulator to be defined
> >>>> to represent that.
> >>>>
> >>>> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> >>>> have preferred an enum of strings).
> >>>>
> >>>> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> >>>> ---
> >>>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> >>>> MAINTAINERS | 7 +
> >>>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> >>>> 3 files changed, 242 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..dc7f37cad772
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> >>>> @@ -0,0 +1,224 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: Texas Instruments' ADS112C14 and similar ADC chips
> >>>> +
> >>>> +description: |
> >>>> + Supports the following Texas Instruments' ADC chips:
> >>>> + - ADS112C14 (16-bit)
> >>>> + - ADS122C14 (24-bit)
> >>>> +
> >>>> + https://www.ti.com/lit/ds/symlink/ads122c14.pdf
> >>>> +
> >>>> + These chips are primarily designed for use with temperature sensors such as
> >>>> + RTDs and thermocouples. The channel bindings reflect this in that each channel
> >>>> + represents the conditions required to make a measurement rather than strictly
> >>>> + just the physical input channels.
> >>>> +
> >>>> +maintainers:
> >>>> + - David Lechner <dlechner@baylibre.com>
> >>>> +
> >>>> +unevaluatedProperties: false
> >>>
> >>> Weird positioning of this.
> >>
> >> IIRC, Rob asked that I do it in this order on another binding a while
> >> ago (the reasoning being that it was too far away from properties:
> >> otherwise), so I've done it like this on a few bindings now. It doesn't
> >> make much difference to me though.
> >
> > Too far away because it refers to properties in the "main" node, but
> > appears conventionally after a rake of properties belonging to the
> > children?
> >
> I found the original request:
>
> https://lore.kernel.org/all/20241022204312.GA1524310-robh@kernel.org/
>
> "Easier to read the indented cases that way."
>
> Reading it again, it sounds like the request was just for the indented
> additionalProperties to be moved.
I think so, yeah. It's definitely common to see
patternProperties:
"^dma-channel@[0-9a-f]+$":
type: object
unevaluatedProperties: false
description:
DMA channel properties based on HDL compile-time configuration.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Conor Dooley @ 2026-06-17 15:36 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Amit Barzilai, robh, krzk+dt, conor+dt, devicetree, dri-devel,
linux-kernel, airlied, maarten.lankhorst, mripard, simona,
tzimmermann
In-Reply-To: <87a4suzc0c.fsf@ocarina.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]
On Tue, Jun 16, 2026 at 06:27:31PM +0200, Javier Martinez Canillas wrote:
> Conor Dooley <conor@kernel.org> writes:
>
> Hello Conor,
>
> > On Mon, Jun 15, 2026 at 08:56:20PM +0300, Amit Barzilai wrote:
> >> Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
> >> RGB OLED display controller driven over a 4-wire SPI bus. The binding
> >> builds on the shared solomon,ssd-common.yaml properties already used by
> >> the other Solomon display controllers.
> >>
> >> Assisted-by: Claude:claude-opus-4-8
> >> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
> >> ---
> >> Changes since v1:
> >> - Drop solomon,width / solomon,height: both are deducible from the
> >> compatible and are already declared (as optional) by the referenced
> >> solomon,ssd-common.yaml, so a local override is unnecessary.
> >> - Drop the rotation property: it has no consumer (rotation is being removed from the driver).
> >> - Use dt-bindings/gpio/gpio.h flag defines in the example
> >> (reset-gpios active-low, dc-gpios active-high).
> >
> > The user for this appears to be in staging. As far as I understand, the
> > policy is that we only add bindings for staging things when they move
> > out of staging.
> > Sure, this is straightforward but why should an exception be made here?
> > Are you working on moving this out of staging?
> >
>
>
> This DT binding was part of a series to add support for "solomon,ssd1351"
> to drivers/gpu/drm/solomon/ DRM driver. Amit only sent a v2 of the binding
> schema because he had some questions about the driver:
>
> https://lore.kernel.org/dri-devel/87cxxqzwxn.fsf@ocarina.mail-host-address-is-not-set/
>
> But yes, I agree that it would had been better for him to post this as a
> part of v2 (and I still expect him to do it), otherwise it is confusing.
>
> Specially since as you pointed out, there is an existing fbdev driver for
> the same device in staging.
Right, I'll expect this to reappear in a larger patchset then that
deals with the fbdev driver and adds the drm driver.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v8 4/5] iio: adc: versal-sysmon: add threshold event support
From: Andy Shevchenko @ 2026-06-17 15:38 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-5-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:58PM +0100, Salih Erim wrote:
> Add threshold event support for temperature and supply voltage
> channels.
>
> Temperature events:
> - Rising threshold with configurable value on the device
> temperature channel (current max across all satellites)
> - Per-channel hysteresis as a millicelsius value
> - Event direction is IIO_EV_DIR_RISING (hysteresis mode)
>
> Supply voltage events:
> - Rising/falling threshold per supply channel
> - Per-channel alarm enable via alarm configuration registers
>
> The hardware supports both window and hysteresis alarm modes for
> temperature. This driver uses hysteresis mode, where the upper
> threshold triggers the alarm and the lower threshold clears it
> (re-arm point). The hardware has a single ISR bit per temperature
> channel with no indication of which threshold was crossed, so
> hysteresis mode is the natural fit. The lower threshold register
> is computed internally as (upper - hysteresis).
>
> Hysteresis is stored in the driver as a millicelsius value,
> initialized from the hardware registers at probe. Writing the
> rising threshold or hysteresis recomputes the lower register.
> ALARM_CONFIG is hard-coded to hysteresis mode during init.
>
> The hardware also provides a separate over-temperature (OT)
> threshold, but it is not exposed through IIO as it serves as a
> hardware safety mechanism for platform shutdown. OT will be
> exposed through the thermal framework in a follow-up series.
>
> The interrupt handler masks active threshold interrupts (which are
> level-sensitive) and schedules a delayed worker to poll for condition
> clear before unmasking. When no hardware IRQ is available, event
> specs are not attached and interrupt init is skipped, since the
> I2C regmap backend cannot be called from atomic context.
>
> When disabling a supply channel alarm, the group interrupt remains
> active if any other channel in the same alarm group still has an
> alarm enabled.
>
> A devm cleanup action masks all interrupts on driver unbind to
> prevent unhandled interrupt storms after the IRQ handler is freed.
...
> +static void sysmon_supply_processedtoraw(int val, u32 reg_val, u32 *raw_data)
> +{
> + int exponent = FIELD_GET(SYSMON_MODE_MASK, reg_val);
> + int format = FIELD_GET(SYSMON_FMT_MASK, reg_val);
> + int scale, tmp;
> +
> + scale = BIT(SYSMON_SUPPLY_MANTISSA_BITS - exponent);
> + tmp = (val * scale) / (int)MILLI;
> +
> + if (format)
> + tmp = clamp(tmp, S16_MIN, S16_MAX);
> + else
> + tmp = clamp(tmp, 0, U16_MAX);
Double check that minmax.h is included.
> + *raw_data = (u16)tmp;
> +}
...
> +static int sysmon_supply_thresh_offset(int address,
> + enum iio_event_direction dir)
Make it a single line. OTOH why is 'address' signed? Perhaps u32?
Or for some reason unsigned long as per _alarm_config()?
> +{
> + if (dir == IIO_EV_DIR_RISING)
> + return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_UP;
> + if (dir == IIO_EV_DIR_FALLING)
> + return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_LOW;
> +
> + return -EINVAL;
> +}
...
> +static int sysmon_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + unsigned int imr;
> + int config_value;
> + u32 mask;
> + int ret;
> +
> + mask = sysmon_get_event_mask(chan);
Just make it together, as we don't validate the value of 'mask'.
struct sysmon *sysmon = iio_priv(indio_dev);
u32 mask = sysmon_get_event_mask(chan);
...
int ret;
> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
> + if (ret)
> + return ret;
> +
> + /* IMR bits are 1=masked, invert to get 1=enabled */
> + imr = ~imr;
> +
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + config_value = sysmon_read_alarm_config(sysmon, chan->address);
> + if (config_value < 0)
> + return config_value;
> + return config_value && (imr & mask);
> +
> + case IIO_TEMP:
> + /*
> + * Return the administrative state, not the hardware IMR.
> + * The IRQ handler temporarily masks the interrupt during
> + * the polling window; reading IMR would show it as disabled.
> + * temp_mask bit is set when administratively disabled.
> + */
> + return !(sysmon->temp_mask & mask);
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static int sysmon_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> +{
> + u32 offset = SYSMON_ALARM_OFFSET(chan->address);
> + struct sysmon *sysmon = iio_priv(indio_dev);
> + u32 ier = sysmon_get_event_mask(chan);
Here you call the variable 'ier'. Please, make this consistent in the related
APIs (see above).
> + unsigned int alarm_config;
> + int ret;
> +
> + guard(mutex)(&sysmon->lock);
> +
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + ret = sysmon_write_alarm_config(sysmon, chan->address, state);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(sysmon->regmap, offset, &alarm_config);
> + if (ret)
> + return ret;
> +
> + if (alarm_config)
> + return regmap_write(sysmon->regmap, SYSMON_IER, ier);
> +
> + return regmap_write(sysmon->regmap, SYSMON_IDR, ier);
> +
> + case IIO_TEMP:
> + if (state) {
> + ret = regmap_write(sysmon->regmap, SYSMON_IER, ier);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask &= ~ier;
> + } else {
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, ier);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask |= ier;
> + }
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static int sysmon_update_temp_lower(struct sysmon *sysmon)
> +{
> + unsigned int upper_reg;
> + int upper_mc, lower_mc;
> + u32 raw_val;
> + int ret;
> +
> + ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, &upper_reg);
> + if (ret)
> + return ret;
> +
> + sysmon_q8p7_to_millicelsius(upper_reg, &upper_mc);
> +
^^^
> + lower_mc = upper_mc - sysmon->temp_hysteresis;
Either add a blank line here, or remove the one above as these three is kinda
semantically coupled.
> + sysmon_millicelsius_to_q8p7(&raw_val, lower_mc);
> +
> + return regmap_write(sysmon->regmap, SYSMON_TEMP_TH_LOW, raw_val);
> +}
...
> +static void sysmon_unmask_temp(struct sysmon *sysmon, unsigned int isr)
> +{
> + unsigned int unmask, status;
As per above perhaps name 'unmask' as 'u32 ier'? Or did I miss the use case?
> + status = isr & SYSMON_TEMP_INTR_MASK;
> +
> + unmask = ~status & sysmon->masked_temp;
> + sysmon->masked_temp &= status;
> +
> + /* Only unmask if not administratively disabled by userspace */
> + unmask &= ~sysmon->temp_mask;
> +
> + regmap_write(sysmon->regmap, SYSMON_IER, unmask);
> +}
Also looking at all this, please double check variable names in all functions
and make types and names consistent across the whole driver code.
> + }
...
> - num_chan = size_add(num_temp, size_add(ARRAY_SIZE(temp_channels), num_supply));
> + num_static = ARRAY_SIZE(temp_channels);
> + num_chan = size_add(num_temp, size_add(num_static, num_supply));
At glance I don't see any additional arguments, can we introduce num_static in
the previous patch to reduce a churn here?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: (subset) [PATCH v3 0/3] dt-bindings: mfd: syscon: Tighten checks
From: Lee Jones @ 2026-06-17 15:41 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Jacky Huang,
Shan-Chun Hung, Geert Uytterhoeven, Magnus Damm, Heiko Stuebner,
Aaro Koskinen, Andreas Kemnade, Kevin Hilman, Roger Quadros,
Tony Lindgren, Krzysztof Kozlowski
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
linux-renesas-soc, linux-rockchip, linux-omap
In-Reply-To: <20260608-n-dt-bindings-simple-bus-syscon-v3-0-4eba9ec1212a@oss.qualcomm.com>
On Mon, 08 Jun 2026 22:44:23 +0200, Krzysztof Kozlowski wrote:
> Changes in v3:
> - Drop patch #2:
> dt-bindings: mfd: syscon: Drop unneeded case for syscon + simple-mfd
> - Bump dtschema requirement
> - Link to v2: https://patch.msgid.link/20260608-n-dt-bindings-simple-bus-syscon-v2-0-0203e6c249dc@oss.qualcomm.com
>
> Changes in v2:
> 1. New patches #2 and #3
> 1. Add missing part of patch #1, thus not adding Rob's Ack.
> https://lore.kernel.org/all/20260531110404.12768-3-krzysztof.kozlowski@oss.qualcomm.com/
>
> [...]
Applied, thanks!
[1/3] dt-bindings: mfd: syscon: Disallow simple-bus with syscon
commit: c11c918b40295dcb0ad2460d9534454072386f4c
[2/3] dt-bindings: mfd: syscon: Drop custom select for older dtschema
commit: f78049ca80ba2e68f7f46870b0d68eb54a6ce378
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v8 5/5] iio: adc: versal-sysmon: add oversampling support
From: Andy Shevchenko @ 2026-06-17 15:42 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-6-salih.erim@amd.com>
On Tue, Jun 16, 2026 at 02:15:59PM +0100, Salih Erim wrote:
> Add support for reading and writing the oversampling ratio through
> the IIO oversampling_ratio attribute. The hardware supports averaging
> 2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
>
> Temperature and supply channels share oversampling configuration at
> the type level (all temperature channels share one ratio, all supply
> channels share another), exposed through info_mask_shared_by_type.
>
> The hardware encoding uses sample_count / 2 in a 4-bit field within
> the CONFIG register. Per-channel averaging enable registers must also
> be updated to activate or deactivate averaging.
This one LGTM now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: (subset) [PATCH v2 3/4] mfd: mt6397-core: add mt6323 EFUSE support
From: Lee Jones @ 2026-06-17 15:46 UTC (permalink / raw)
To: Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Srinivas Kandagatla, Roman Vivchar
Cc: Andy Shevchenko, linux-pm, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, Ben Grisdale
In-Reply-To: <20260617-mt6323-nvmem-v2-3-4f30e36aa0f4@protonmail.com>
On Wed, 17 Jun 2026 12:48:46 +0300, Roman Vivchar wrote:
> The mt6323 PMIC includes an EFUSE. Register the EFUSE in the mt6323
> devices array to allow the corresponding driver to probe using compatible
> string.
Applied, thanks!
[3/4] mfd: mt6397-core: add mt6323 EFUSE support
commit: f5ce60535d04ca21799d558cfa13ba91bbe714b5
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Conor Dooley @ 2026-06-17 15:51 UTC (permalink / raw)
To: Gerald Loacker
Cc: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-phy, linux-arm-kernel,
linux-rockchip, linux-kernel, devicetree
In-Reply-To: <20260617-feature-mipi-csi-dphy-4k60-v1-2-4611ff00b0ff@wolfvision.net>
[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]
On Wed, Jun 17, 2026 at 02:23:14PM +0200, Gerald Loacker wrote:
> Add support for the optional rockchip,clk-lane-phase device tree property
> to allow board-specific tuning of the clock lane sampling phase for
> improved signal integrity across supported data rates.
>
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
> Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 03950b3cad08c..0d824d1511bc0 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -56,6 +56,13 @@ properties:
> description:
> Some additional phy settings are access through GRF regs.
>
> + rockchip,clk-lane-phase:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + description:
> + Clock lane sampling phase in 40 ps steps. The hardware default is 3.
Can this instead become rockchip,clk-lane-phase-ps and be listed in the
actual unit?
With the -ps suffix, you can then drop the $ref.
The default should be listed as "default: 3" (or default: 120)
pw-bot: changes-requested
> +
> required:
> - compatible
> - reg
>
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/5] dt-bindings: soc: cix,sky1-system-control: add audss system control
From: Conor Dooley @ 2026-06-17 15:54 UTC (permalink / raw)
To: joakim.zhang
Cc: mturquette, sboyd, bmasney, robh, krzk+dt, conor+dt, p.zabel,
gary.yang, cix-kernel-upstream, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <20260617060437.1474816-2-joakim.zhang@cixtech.com>
[-- Attachment #1: Type: text/plain, Size: 4379 bytes --]
On Wed, Jun 17, 2026 at 02:04:33PM +0800, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
>
> The Cix Sky1 Audio Subsystem (AUDSS) groups audio-related clock, reset
> and control registers in a dedicated CRU block. Software reset lines are
> exposed on the syscon parent via #reset-cells, following the same model
> as the existing Sky1 FCH and S5 system control bindings.
>
> A clock-controller child node is required under the audss syscon. It has
> no reg property of its own and accesses the parent register block for mux,
> divider and gate fields.
>
> The AUDSS is also controlled by one power domain and reset part.
>
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---
> .../soc/cix/cix,sky1-system-control.yaml | 48 +++++++++++++++++++
> .../reset/cix,sky1-audss-system-control.h | 25 ++++++++++
> 2 files changed, 73 insertions(+)
> create mode 100644 include/dt-bindings/reset/cix,sky1-audss-system-control.h
>
> diff --git a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> index a01a515222c6..5a1cd5c24ade 100644
> --- a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-control.yaml
> @@ -19,6 +19,7 @@ properties:
> - enum:
> - cix,sky1-system-control
> - cix,sky1-s5-system-control
> + - cix,sky1-audss-system-control
> - const: syscon
If the only thing these share are being a reset controller and having a
syscon fallback, I think it should be in a different file.
pw-bot: changes-requested
Cheers,
Conor.
>
> reg:
> @@ -27,6 +28,38 @@ properties:
> '#reset-cells':
> const: 1
>
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + clock-controller:
> + type: object
> + properties:
> + compatible:
> + const: cix,sky1-audss-clock
> + required:
> + - compatible
> + additionalProperties: true
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: cix,sky1-audss-system-control
> + then:
> + required:
> + - clock-controller
> + - power-domains
> + - resets
> + else:
> + properties:
> + clock-controller: false
> + power-domains: false
> + resets: false
> +
> required:
> - compatible
> - reg
> @@ -40,3 +73,18 @@ examples:
> reg = <0x4160000 0x100>;
> #reset-cells = <1>;
> };
> + - |
> + audss_syscon: system-controller@7110000 {
> + compatible = "cix,sky1-audss-system-control", "syscon";
> + reg = <0x7110000 0x10000>;
> + power-domains = <&smc_devpd 0>;
> + resets = <&s5_syscon 31>;
> + #reset-cells = <1>;
> +
> + clock-controller {
> + compatible = "cix,sky1-audss-clock";
> + #clock-cells = <1>;
> + clocks = <&scmi_clk 0>, <&scmi_clk 2>, <&scmi_clk 4>, <&scmi_clk 5>;
> + clock-names = "x8k", "x11k", "sys", "48m";
> + };
> + };
> diff --git a/include/dt-bindings/reset/cix,sky1-audss-system-control.h b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
> new file mode 100644
> index 000000000000..aabdce60b094
> --- /dev/null
> +++ b/include/dt-bindings/reset/cix,sky1-audss-system-control.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright 2026 Cix Technology Group Co., Ltd.
> + */
> +#ifndef DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
> +#define DT_BINDING_RESET_CIX_SKY1_AUDSS_SYSTEM_CONTROL_H
> +
> +#define AUDSS_I2S0_SW_RST 0
> +#define AUDSS_I2S1_SW_RST 1
> +#define AUDSS_I2S2_SW_RST 2
> +#define AUDSS_I2S3_SW_RST 3
> +#define AUDSS_I2S4_SW_RST 4
> +#define AUDSS_I2S5_SW_RST 5
> +#define AUDSS_I2S6_SW_RST 6
> +#define AUDSS_I2S7_SW_RST 7
> +#define AUDSS_I2S8_SW_RST 8
> +#define AUDSS_I2S9_SW_RST 9
> +#define AUDSS_WDT_SW_RST 10
> +#define AUDSS_TIMER_SW_RST 11
> +#define AUDSS_MB0_SW_RST 12
> +#define AUDSS_MB1_SW_RST 13
> +#define AUDSS_HDA_SW_RST 14
> +#define AUDSS_DMAC_SW_RST 15
> +
> +#endif
> --
> 2.50.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ 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