* [PATCH v3 2/3] arm64: dts: qcom: sm8250: Add cam_mclk3_default pinctrl
From: Xin Xu @ 2026-06-25 12:05 UTC (permalink / raw)
To: konrad.dybcio, andersson, konradybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Xin Xu
In-Reply-To: <tencent_D4AB219A942591742C3A6DA4DFB0EBA49505@qq.com>
Add a default pin configuration for CAM_MCLK3 (GPIO97), which is used
as a master clock for camera sensors on SM8250 boards.
Signed-off-by: Xin Xu <xxsemail@qq.com>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 7076720413ab..05e765616112 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -5165,6 +5165,13 @@ mclk-pins {
};
};
+ cam_mclk3_default: cam-mclk3-default-state {
+ pins = "gpio97";
+ function = "cam_mclk";
+ drive-strength = <4>;
+ bias-disable;
+ };
+
cci0_default: cci0-default-state {
cci0_i2c0_default: cci0-i2c0-default-pins {
/* SDA, SCL */
--
2.53.0
^ permalink raw reply related
* [PATCH v3 1/3] arm64: dts: qcom: sm8250-xiaomi-elish: Add pm8008 PMIC
From: Xin Xu @ 2026-06-25 11:50 UTC (permalink / raw)
To: konrad.dybcio, andersson, konradybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Xin Xu
Add the pm8008 PMIC node on i2c15 with seven LDOs,
using GPIO84 as interrupt and GPIO76 as reset.
Signed-off-by: Xin Xu <xxsemail@qq.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Changes in v3:
- Split pinctrl into two separate states for interrupt and reset, drop
input-enable.
- Reorder bias properties after drive-strength.
Changes in v2:
- Fix coding style (blank line, interrupts-extended, property order,
drop output-high)
- Correct voltage constraints for ldo1 and ldo2
.../dts/qcom/sm8250-xiaomi-elish-common.dtsi | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
index 51b57c697a75..05c366eaeff1 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
@@ -599,6 +599,82 @@ fuel-gauge@55 {
};
};
+&i2c15 {
+ clock-frequency = <400000>;
+
+ status = "okay";
+
+ pm8008: pmic@8 {
+ compatible = "qcom,pm8008";
+ reg = <0x8>;
+
+ interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_RISING>;
+ reset-gpios = <&tlmm 76 GPIO_ACTIVE_LOW>;
+
+ vdd-l1-l2-supply = <&vreg_s8c_1p35>;
+ vdd-l3-l4-supply = <&vreg_bob>;
+ vdd-l5-supply = <&vreg_bob>;
+ vdd-l6-supply = <&vreg_bob>;
+ vdd-l7-supply = <&vreg_bob>;
+
+ pinctrl-0 = <&pm8008_int_default>, <&pm8008_reset_default>;
+ pinctrl-names = "default";
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pm8008 0 0 2>;
+
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ #thermal-sensor-cells = <0>;
+
+ regulators {
+ vreg_l1p: ldo1 {
+ regulator-name = "vreg_l1p";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ vreg_l2p: ldo2 {
+ regulator-name = "vreg_l2p";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ vreg_l3p: ldo3 {
+ regulator-name = "vreg_l3p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l4p: ldo4 {
+ regulator-name = "vreg_l4p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l5p: ldo5 {
+ regulator-name = "vreg_l5p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l6p: ldo6 {
+ regulator-name = "vreg_l6p";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ vreg_l7p: ldo7 {
+ regulator-name = "vreg_l7p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2900000>;
+ };
+ };
+ };
+};
+
&mdss {
status = "okay";
};
@@ -801,6 +877,20 @@ bt_en_state: bt-default-state {
bias-pull-up;
};
+ pm8008_int_default: pm8008-int-default-state {
+ pins = "gpio84";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ pm8008_reset_default: pm8008-reset-default-state {
+ pins = "gpio76";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
wlan_en_state: wlan-default-state {
pins = "gpio20";
function = "gpio";
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v4 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection
From: Konrad Dybcio @ 2026-06-25 11:51 UTC (permalink / raw)
To: Elson Serrao, Bjorn Andersson, Dmitry Baryshkov,
Greg Kroah-Hartman, Konrad Dybcio, Krzysztof Kozlowski,
Rob Herring, Conor Dooley, Souradeep Chowdhury
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
In-Reply-To: <20260501170635.2641748-3-elson.serrao@oss.qualcomm.com>
On 5/1/26 7:06 PM, Elson Serrao wrote:
> EUD can be mapped to either the primary USB port or the secondary USB port
> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
> attribute to allow userspace to select which port EUD should operate on
> and update the ABI documentation. This is needed for systems with dual
> USB ports where EUD needs to be accessible on either port depending on the
> system configuration and use case.
>
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v1 4/8] arm64: dts: qcom: shikra: Add soundwire and macro nodes
From: Konrad Dybcio @ 2026-06-25 11:45 UTC (permalink / raw)
To: Mohammad Rafi Shaik, Srinivas Kandagatla, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-sound, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260616201315.2565115-5-mohammad.rafi.shaik@oss.qualcomm.com>
On 6/16/26 10:13 PM, Mohammad Rafi Shaik wrote:
> Add SoC-level SoundWire masters and LPASS RX/VA macro nodes, along with
> DMIC and SWR pinctrl states required by the audio data path.
>
> Keep these nodes disabled in shikra.dtsi so board dts files can selectively
> enable and configure them.
>
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
Just a couple style nits:
[...]
> + rxmacro: codec@a040000 {
> + compatible = "qcom,shikra-lpass-rx-macro";
> + reg = <0x0 0x0a040000 0x0 0x1000>;
> +
> + pinctrl-0 = <&rx_swr_active>;
> + pinctrl-names = "default";
Let's move this down.. maybe above all the #cells
> +
> + clocks = <&audiocorecc AUDIO_CORE_CC_RX_MCLK_CLK>,
> + <&audiocorecc AUDIO_CORE_CC_RX_MCLK_2X_CLK>,
Please align all <s
> + <&vamacro>;
> + clock-names = "mclk",
> + "npl",
> + "fsgen";
> +
> + #clock-cells = <0>;
> + clock-output-names = "mclk";
> + #sound-dai-cells = <1>;
> + status = "disabled";
Let's keep a \n before 'status', everywhere
Konrad
^ permalink raw reply
* Re: [PATCH 4/5] media: i2c: vd55g1: Add support for vd55g4
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajkKkSeDNoijIsub@zed>
Hi Jacopo,
Le 22/06/2026 à 12:16, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:58AM +0200, Benjamin Mugnier wrote:
>> vd55g4 is the same device as vd65g4 but outputs in monochrome instead of
>> RGB. Adapt the driver structure according to this new variant, and add
>> its support.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>> drivers/media/i2c/vd55g1.c | 110 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 74 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 2c962fcb41d2..9f62fc0428a1 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -29,11 +29,7 @@
>>
>> /* Register Map */
>> #define VD55G1_REG_MODEL_ID CCI_REG32_LE(0x0000)
>> -#define VD55G1_MODEL_ID_VD55G1 0x53354731 /* Mono */
>> -#define VD55G1_MODEL_ID_VD65G4 0x53354733 /* RGB */
>> -#define VD55G1_REG_REVISION CCI_REG16_LE(0x0004)
>> -#define VD55G1_REVISION_CCB 0x2020
>> -#define VD55G1_REVISION_BAYER 0x3030
>> +#define VD55G1_REG_COLOR_VERSION CCI_REG32_LE(0x0670)
>> #define VD55G1_REG_FWPATCH_REVISION CCI_REG16_LE(0x0012)
>> #define VD55G1_REG_FWPATCH_START_ADDR CCI_REG8(0x2000)
>> #define VD55G1_REG_SYSTEM_FSM CCI_REG8(0x001c)
>> @@ -138,8 +134,39 @@
>> #define VD55G1_MIPI_RATE_MIN (250 * MEGA)
>> #define VD55G1_MIPI_RATE_MAX (1200 * MEGA)
>>
>> -#define VD55G1_MODEL_ID_NAME(id) \
>> - ((id) == VD55G1_MODEL_ID_VD55G1 ? "vd55g1" : "vd65g4")
>> +enum vd55g1_model_id {
>> + VD55G1_MODEL_ID_2 = 0x53354731,
>> + VD55G1_MODEL_ID_3 = 0x53354733,
>> +};
>> +
>> +enum vd55g1_color_version {
>> + VD55G1_COLOR_VERSION_MONO = 0x0,
>> + VD55G1_COLOR_VERSION_BAYER = 0x1,
>
> nit: you don't need to initialize the enum members here
Thank you, will do.
>
>> +};
>> +
>> +struct vd55g1_version {
>> + char *name;
>> + enum vd55g1_model_id id;
>> + enum vd55g1_color_version color;
>> +};
>> +
>> +static const struct vd55g1_version vd55g1_versions[] = {
>> + {
>> + .name = "vd55g1",
>> + .id = VD55G1_MODEL_ID_2,
>> + .color = VD55G1_COLOR_VERSION_MONO,
>> + },
>> + {
>> + .name = "vd55g4",
>> + .id = VD55G1_MODEL_ID_3,
>> + .color = VD55G1_COLOR_VERSION_MONO,
>> + },
>> + {
>> + .name = "vd65g4",
>> + .id = VD55G1_MODEL_ID_3,
>> + .color = VD55G1_COLOR_VERSION_BAYER,
>> + },
>> +};
>>
>> static const u8 vd55g1_patch_array[] = {
>> 0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,
>> @@ -535,7 +562,7 @@ struct vd55g1_vblank_limits {
>>
>> struct vd55g1 {
>> struct device *dev;
>> - unsigned int id;
>> + const struct vd55g1_version *version;
>> struct v4l2_subdev sd;
>> struct media_pad pad;
>> struct regulator_bulk_data supplies[ARRAY_SIZE(vd55g1_supply_name)];
>> @@ -628,7 +655,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
>> {
>> unsigned int i, j;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
>> return code;
>
> As pointed out in the previous patch, you seem to have 2 mono formats.
> Is this still ok ?
>
>>
>> for (i = 0; i < ARRAY_SIZE(vd55g1_mbus_formats_bayer); i++) {
>> @@ -1183,8 +1210,8 @@ static int vd55g1_patch(struct vd55g1 *sensor)
>> u64 patch;
>> int ret = 0;
>>
>> - /* vd55g1 needs a patch while vd65g4 does not */
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
>> + /* Version 2 needs a patch while version 3 does not */
>> + if (sensor->version->id == VD55G1_MODEL_ID_2) {
>> vd55g1_write_array(sensor, VD55G1_REG_FWPATCH_START_ADDR,
>> sizeof(vd55g1_patch_array),
>> vd55g1_patch_array, &ret);
>
> You might want to consider renaming vd55g1_patch_array ?
It doesn't really patch an array, it writes an array of values into
continuous i2c registers. Just like before converting to cci_write() I
used to have a vd55g1_write() function to write a register, I derived a
vd55g1_write_array() function. Here it happens to be a firmware patch,
but it could be anything you want ;)
>
>> @@ -1256,7 +1283,7 @@ static int vd55g1_enum_mbus_code(struct v4l2_subdev *sd,
>> struct vd55g1 *sensor = to_vd55g1(sd);
>> u32 base_code;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER) {
>> if (code->index >= ARRAY_SIZE(vd55g1_mbus_formats_mono))
>> return -EINVAL;
>> base_code = vd55g1_mbus_formats_mono[code->index];
>> @@ -1372,7 +1399,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> if (ret)
>> return ret;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
>> code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> else
>> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>> @@ -1659,38 +1686,48 @@ static int vd55g1_init_ctrls(struct vd55g1 *sensor)
>> return ret;
>> }
>>
>> +static const struct vd55g1_version *
>> + vd55g1_get_version(enum vd55g1_model_id id,
>> + enum vd55g1_color_version color)
>
> Should you indent one tab left ?
>
checkpatch.pl is fine with both. I can indent left, it looks cleaner.
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vd55g1_versions); i++) {
>
> You can declare i inside the for loop
>
Yes, thank you.
>> + if (vd55g1_versions[i].id == id &&
>> + vd55g1_versions[i].color == color)
>> + return &vd55g1_versions[i];
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static int vd55g1_detect(struct vd55g1 *sensor)
>> {
>> - unsigned int dt_id = (uintptr_t)device_get_match_data(sensor->dev);
>> - u64 rev, id;
>> - int ret;
>> + const struct vd55g1_version *dt_version =
>> + device_get_match_data(sensor->dev);
>> + const struct vd55g1_version *version;
>> + u64 color, id;
>> + int ret = 0;
>>
>> - ret = vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, NULL);
>> + vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, &ret);
>> + vd55g1_read(sensor, VD55G1_REG_COLOR_VERSION, &color, &ret);
>> if (ret)
>> return ret;
>>
>> - if (id != VD55G1_MODEL_ID_VD55G1 && id != VD55G1_MODEL_ID_VD65G4) {
>> - dev_warn(sensor->dev, "Unsupported sensor id 0x%x\n",
>> - (u32)id);
>> + version = vd55g1_get_version(id, color);
>> + if (!version) {
>> + dev_warn(sensor->dev, "Unsupported sensor version, expected %s\n",
>> + dt_version->name);
>> return -ENODEV;
>> }
>> - if (id != dt_id) {
>> - dev_err(sensor->dev, "Probed sensor %s and device tree definition (%s) mismatch",
>> - VD55G1_MODEL_ID_NAME(id), VD55G1_MODEL_ID_NAME(dt_id));
>> + if (version->id != dt_version->id ||
>> + version->color != dt_version->color) {
>> + dev_err(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
>> + version->name, dt_version->name);
>> return -ENODEV;
>> }
>> - sensor->id = id;
>>
>> - ret = vd55g1_read(sensor, VD55G1_REG_REVISION, &rev, NULL);
>> - if (ret)
>> - return ret;
>> -
>> - if ((id == VD55G1_MODEL_ID_VD55G1 && rev != VD55G1_REVISION_CCB) &&
>> - (id == VD55G1_MODEL_ID_VD65G4 && rev != VD55G1_REVISION_BAYER)) {
>> - dev_err(sensor->dev, "Unsupported sensor revision 0x%x for sensor %s\n",
>> - (u16)rev, VD55G1_MODEL_ID_NAME(id));
>> - return -ENODEV;
>> - }
>> + sensor->version = version;
>>
>> return 0;
>> }
>> @@ -2048,8 +2085,9 @@ static void vd55g1_remove(struct i2c_client *client)
>> }
>>
>> static const struct of_device_id vd55g1_dt_ids[] = {
>> - { .compatible = "st,vd55g1", .data = (void *)VD55G1_MODEL_ID_VD55G1 },
>> - { .compatible = "st,vd65g4", .data = (void *)VD55G1_MODEL_ID_VD65G4 },
>> + { .compatible = "st,vd55g1", .data = (void *)&vd55g1_versions[0] },
>> + { .compatible = "st,vd55g4", .data = (void *)&vd55g1_versions[1] },
>> + { .compatible = "st,vd65g4", .data = (void *)&vd55g1_versions[2] },
>> { /* sentinel */ }
>> };
>
> All minors
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>
>> MODULE_DEVICE_TABLE(of, vd55g1_dt_ids);
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/5] media: i2c: vd55g1: Fix media bus code initialization
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajj90hhNwx7bLkOZ@zed>
Hi Jacopo,
Thank you for your review.
Le 22/06/2026 à 11:28, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:55AM +0200, Benjamin Mugnier wrote:
>> In the driver initialization, the index of the default media bus code
>> from the supported media bus code array is passed directly to the
>> vd55g1_get_fmt_code() function instead of the proper media bus code.
>>
>> This works correctly as a proper media bus code is set after
>> initialization but could not have been the case. This also resulted in
>> mutliple "Unsupported mbus format" error messages.
>>
>> Retrieve the media bus code from the media bus code array, and pass this
>> media bus code to vd55g1_get_fmt_code() instead of the code index.
>>
>> Rename VD55G1_MBUS_CODE_DEF to VD55G1_MBUS_CODE_IDX_DEF and
>> VD55G1_MODE_DEF to VD55G1_MODE_IDX_DEF while at it to avoid future
>> confusions. Display the guilty error code in warning message.
>>
>> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
>>
> You should cc stable for fixes
>
> Cc: stable@vger.kernel.org
>
We talked about this very recently and somehow I still forgot.
>
> The CI should have flagged that, but for some reason it didn't run
> properly on your series
> https://gitlab.freedesktop.org/linux-media/users/patchwork/-/pipelines/1655147
>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>> drivers/media/i2c/vd55g1.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 78d18c028154..1e9db21322e3 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -114,9 +114,9 @@
>>
>> #define VD55G1_WIDTH 804
>> #define VD55G1_HEIGHT 704
>> -#define VD55G1_MODE_DEF 0
>> +#define VD55G1_MODE_IDX_DEF 0
>> #define VD55G1_NB_GPIOS 4
>> -#define VD55G1_MBUS_CODE_DEF 0
>> +#define VD55G1_MBUS_CODE_IDX_DEF 0
>> #define VD55G1_DGAIN_DEF 256
>> #define VD55G1_AGAIN_DEF 19
>> #define VD55G1_EXPO_MAX_TERM 64
>> @@ -634,7 +634,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
>
> Unrelated, but it seems you now have 2 codes for MONO. Does
>
> if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> return code;
>
> need an update ?>
Not in this patch because it does not add the new MONO sensor, but in
4/5 I separated the model ID from the color code. Example for the vd55g4 :
.name = "vd55g4",
.id = VD55G1_MODEL_ID_3,
.color = VD55G1_COLOR_VERSION_MONO,
So the patch 4/5 updates the previous 'if' you mentioned to check the
color member instead of the model :
if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
Which IMO is a good way to handle this problematic. Tell me if you're
thinking about something else.
>> goto adapt_bayer_pattern;
>> }
>> }
>> - dev_warn(sensor->dev, "Unsupported mbus format\n");
>> + dev_warn(sensor->dev, "Unsupported mbus format: 0x%x\n", code);
>>
>> return code;
>>
>> @@ -1347,6 +1347,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> {
>> struct vd55g1 *sensor = to_vd55g1(sd);
>> struct v4l2_subdev_format fmt = { 0 };
>> + int code;
>> struct v4l2_subdev_route routes[] = {
>> { .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE }
>> };
>> @@ -1361,9 +1362,13 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> if (ret)
>> return ret;
>>
>> - vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
>> - vd55g1_get_fmt_code(sensor, VD55G1_MBUS_CODE_DEF),
>> - &fmt.format);
>> + if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> + else
>> + code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>
> Being this a multi-dimensional array, I don't seem much value in
> defining VD55G1_MBUS_CODE_IDX_DEF if this is the only place where it
> is used. What's the meaning of VD55G1_MBUS_CODE_IDX_DEF for
> vd55g1_mbus_formats_bayer ? Does it represent the bitwidth or does it
> represent the bayer pattern ?
For vd55g1_mbus_formats_bayer, the first dimension of the array is the
bitwidth, and the second one is the bayer pattern.
>
> I would rather define a
> VD55G1_DEF_MBUS_CODE_MONO MEDIA_BUS_FMT_Y8_1X8
> VD55G1_DEF_MBUS_CODE_BAYER MEDIA_BUS_FMT_SRGGB8_1X8
>
> Or maybe do
>
> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF]
> [VD55G1_MBUS_CODE_IDX_DEF];
>
> if easier.
>
> I understand it's a minor, so up to you.
As you mentioned it's only used here. I won't mind removing
VD55G1_MBUS_CODE_IDX_DEF entirely and do :
code = vd55g1_mbus_formats_bayer[0][0];
Does that sound okay ?
>
>
>
>> + vd55g1_update_pad_fmt(sensor,
>> + &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
>> + vd55g1_get_fmt_code(sensor, code), &fmt.format);
>>
>> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
>> }
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH 2/5] media: i2c: vd55g1: Remove spurious pad format update on init_state()
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajkAdol1szOK_XPB@zed>
Hi Jacopo,
Le 22/06/2026 à 11:30, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:56AM +0200, Benjamin Mugnier wrote:
>> vd55g1_update_pad_fmt() is called in vd55g1_init_state(). But
>> vd55g1_set_pad_fmt(), called at the end of vd55g1_init_state(), also
>> calls vd55g1_update_pad_fmt() itself.
>>
>> Enhance readability and clear confusion by only preparing the format in
>> vd55g1_init_state() and let vd55g1_set_pad_fmt() update it instead,
>> effectively calling it only 1 time instead of 2.
>>
>> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
>
> Does this qualify as a fix ?
That's a good question, indeed this patch does not correct any 'bug' per
se. It's more of a code flow error so I think you're correct. I'm fine
with removing the 'Fixes:' tag.
>
> I think you could maybe squash it with the previous one if you want
> also this change to be backported as part of a larger fix
>
I don't know, this patch solves another issue than the previous patch
does. I'd like to keep it this in the git history if you don't mind.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>
>> ---
>> drivers/media/i2c/vd55g1.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 1e9db21322e3..e44174056ace 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -1366,9 +1366,9 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> else
>> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>> - vd55g1_update_pad_fmt(sensor,
>> - &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
>> - vd55g1_get_fmt_code(sensor, code), &fmt.format);
>> + fmt.format.code = vd55g1_get_fmt_code(sensor, code);
>> + fmt.format.width = vd55g1_supported_modes[VD55G1_MODE_IDX_DEF].width;
>> + fmt.format.height = vd55g1_supported_modes[VD55G1_MODE_IDX_DEF].height;
>>
>> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
>> }
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: Konrad Dybcio @ 2026-06-25 11:36 UTC (permalink / raw)
To: Daniel Lezcano, sre, hansg, ilpo.jarvinen, linux, andersson,
konradybcio, robh, krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-4-daniel.lezcano@oss.qualcomm.com>
On 6/24/26 11:08 PM, Daniel Lezcano wrote:
> Add thermal zone descriptions for the keyboard skin temperature and
> battery charging circuitry sensors exposed by the embedded controller.
>
> Expose the EC as a thermal sensor provider and define two thermal zones
> using the temperatures reported by the EC:
>
> - a keyboard skin temperature zone with passive and hot trip points,
> - a charging circuitry temperature zone with multiple passive trip
> points and CPU frequency mitigation levels.
>
> The charging thermal zone progressively throttles the different CPU
> clusters as the charging circuitry temperature rises and triggers a hot
> condition at the highest trip point.
>
> This provides thermal framework integration for the EC temperature
> sensors and enables platform thermal management through standard thermal
> zone definitions.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
[...]
> + ec-charging-thermal {
> + polling-delay = <5000>;
> + polling-delay-passive = <2000>;
> +
> + thermal-sensors = <&ec 3>;
> +
> + trips {
> + psv0: trip-point0 {
"ec_charging_tripN", please - these labels are file-wide, so it's
better if they're not overly short
> + temperature = <55000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + alert0: trip-point1 {
> + temperature = <63000>;
> + hysteresis = <0>;
> + type = "hot";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&psv0>;
> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +
> + };
Stray \n above, might also possibly want to throttle the GPU.
I don't know.
Konrad
^ permalink raw reply
* [PATCH] arm64: dts: ti: k3-am62a7-sk: Add bootph-all property in cpsw_mac_syscon node
From: Chintan Vankar @ 2026-06-25 11:32 UTC (permalink / raw)
To: Conor Dooley, Krzysztof Kozlowski, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon
Cc: c-vankar, linux-kernel, devicetree, linux-arm-kernel
Ethernet boot requires CPSW node to be present starting from R5 SPL stage.
Add "bootph-all" property in CPSW MAC's eFuse node "cpsw_mac_syscon" to
enable this node during SPL stage along with later boot stage so that CPSW
port will get static MAC address.
Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---
Hello All,
This patch is based on linux-next tagged next-20260623.
arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
index 821a9705bb7d..d3b3675e7a8f 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
@@ -230,6 +230,10 @@ AM62AX_MCU_IOPAD(0x0030, PIN_OUTPUT, 0) /* (C8) WKUP_UART0_RTSn */
};
};
+&cpsw_mac_syscon {
+ bootph-all;
+};
+
/* WKUP UART0 is used for DM firmware logs */
&wkup_uart0 {
pinctrl-names = "default";
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: sm8250-xiaomi-elish: add ov8856 front camera
From: Xin Xu @ 2026-06-25 11:30 UTC (permalink / raw)
To: konrad.dybcio
Cc: andersson, devicetree, konradybcio, linux-arm-msm, linux-kernel,
xxsemail
In-Reply-To: <896f3704-e7bb-48da-a501-0c9b0228573d@oss.qualcomm.com>
On Wed, 2026-06-24 at 13:56 +0200, Konrad Dybcio wrote:
> On 6/22/26 8:52 PM, Xin Xu wrote:
> > Add the ov8856 front camera, connected on CCI1 to CSIPHY4 and
> > powered by pm8008 LDOs and other supplies.
> >
> > Signed-off-by: Xin Xu <xxsemail@qq.com>
> > ---
>
> [...]
>
> > &cdsp {
> > firmware-name = "qcom/sm8250/xiaomi/elish/cdsp.mbn";
> > status = "okay";
> > @@ -877,6 +933,20 @@ bt_en_state: bt-default-state {
> > bias-pull-up;
> > };
> >
> > + camera_front_active: camera-front-active-state {
> > + pins = "gpio109";
> > + function = "gpio";
> > + bias-disable;
> > + drive-strength = <2>;
> > + };
> > +
> > + mclk3_active: mclk3-active-state {
> > + pins = "gpio97";
> > + function = "cam_mclk";
> > + bias-disable;
> > + drive-strength = <4>;
> > + };
>
> nit: other nodes have drive-strength before bias-disable
>
> Also let's move the mclk pin state definition to sm8250, it's not
> specific to this board
>
> Konrad
I will fix the coding style issues in the next patch.
Also, in the next version of the series, the mclk pin state
will be applied to sm8250.dtsi in a separate patch.
Best regards,
Xin Xu
^ permalink raw reply
* Re: [PATCH v4 04/19] firmware: qcom: scm: use dev_err_probe() for dload address failure
From: Mukesh Ojha @ 2026-06-25 11:29 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260624192213.C82691F000E9@smtp.kernel.org>
On Wed, Jun 24, 2026 at 07:22:13PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The global `qcom_tzmem_dev` pointer is set during `qcom_tzmem_enable()` but is never cleared on probe failure, causing subsequent probe retries to permanently fail with `-EBUSY`.
> - [High] The probe function calls `of_reserved_mem_device_init()` but does not call `of_reserved_mem_device_release()` on subsequent error paths, leading to permanent probe failure or resource leaks upon probe retry.
> --
>
> commit 6dc15526252b78a9c217f06ed35ed4966a794450
> Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>
> firmware: qcom: scm: use dev_err_probe() for dload address failure
>
> Replace the bare return ret after qcom_scm_find_dload_address() with
> dev_err_probe() to produce a consistent, deferred-probe-aware error
> message when the download-mode address cannot be resolved.
>
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index c8a84836887f0..6cd4d9468456f 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -2750,7 +2750,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
> > scm->dev = &pdev->dev;
> > ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
> > if (ret < 0)
> > - return ret;
> > + return dev_err_probe(&pdev->dev, ret,
> > + "Failed to get download mode address\n");
> >
> > mutex_init(&scm->scm_bw_lock);
>
> [Severity: High]
> This is a pre-existing issue, but does qcom_scm_probe() properly release
> resources on later probe failures?
>
> Later in qcom_scm_probe(), of_reserved_mem_device_init() is called to setup
> reserved memory:
>
> drivers/firmware/qcom/qcom_scm.c:qcom_scm_probe() {
> ...
> ret = of_reserved_mem_device_init(scm->dev);
> if (ret && ret != -ENODEV)
> return dev_err_probe(scm->dev, ret,
> "Failed to setup the reserved memory region for TZ mem\n");
> ...
> }
>
> However, subsequent error paths, such as platform_get_irq_optional() returning
> -EPROBE_DEFER, exit directly without calling of_reserved_mem_device_release().
>
> If the probe is deferred, will this cause the next probe attempt to fail with
> -EBUSY, or leak memory by duplicating the rmem_assigned_device list entry on
> every retry?
>
> [Severity: High]
> This is a pre-existing issue, but does qcom_tzmem_enable() cause a permanent
> probe failure if qcom_scm_probe() defers later in the function?
>
> qcom_tzmem_enable() sets a static qcom_tzmem_dev pointer in qcom_tzmem.c:
>
> drivers/firmware/qcom/qcom_tzmem.c:qcom_tzmem_enable() {
> if (qcom_tzmem_dev)
> return -EBUSY;
>
> qcom_tzmem_dev = dev;
>
> return qcom_tzmem_init();
> }
>
> Since qcom_scm_probe() does not clear this pointer if it defers after this
> call, will subsequent probe retries permanently fail with -EBUSY when
> qcom_tzmem_enable() checks the still-set pointer?
>
Since they are existing issues, sent patches for them in the list
separately.
--
> Sashiko AI review · https://sashiko.dev/#/patchset/20260624190830.3131112-1-mukesh.ojha@oss.qualcomm.com?part=4
--
-Mukesh Ojha
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: power: supply: Add TI BQ25630 charger
From: Waqar Hameed @ 2026-06-25 11:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
kernel, linux-pm, devicetree, linux-kernel
In-Reply-To: <20260622-whimsical-inescapable-pronghorn-5f94bb@quoll>
On Mon, Jun 22, 2026 at 13:06 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Thu, Jun 18, 2026 at 03:37:59PM +0200, Waqar Hameed wrote:
>> +allOf:
>> + - $ref: power-supply.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: ti,bq25630
>> +
>> + reg:
>> + const: 0x6b
>> + description:
>> + Device I2C address.
>
> Drop description, obvious.
I see that several others actually don't have a description for this
property. I'll drop it!
>
>> +
>> + interrupts:
>> + maxItems: 1
>> + description: |
>
> Do not need '|' unless you need to preserve formatting.
Right, let's remove it.
>
>> + Device sends active low 256 µs pulse. Type should therefore be
>> + IRQ_TYPE_EDGE_FALLING.
>> +
>> + monitored-battery: true
>
> Drop this one
I was actually unsure about this one, but several other bindings had
this one. We can remove it.
>
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - monitored-battery
>> +
>> +additionalProperties: false
>
> And here use 'unevaluatedProperties: false' instead.
Hm, reading the documentation [1], `unevaluatedProperties` should be
used, because of the "include" of `power-supply.yaml`. I'll change to
that.
However, there are tons of other places where `additionalProperties` is
used instead _with_ a reference to `power-supply.yaml` (which is what I
followed). Are all of them actually "wrong" then?
>
> With these fixed:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Thank you for your review Krzysztof!
[1] `Documentation/devicetree/bindings/writing-schema.rst`
^ permalink raw reply
* [PATCH v4] dt-bindings: pwm: st,sti-pwm: convert to DT schema
From: Charan Pedumuru @ 2026-06-25 11:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones
Cc: linux-pwm, devicetree, linux-kernel, Charan Pedumuru
Convert STMicroelectronics STiH41x PWM/Capture controller binding
to DT schema.
Changes during conversion:
- Fix compatible string from "st,pwm" to "st,sti-pwm" to match the
actual hardware variant naming convention used across STi bindings.
- Drop pinctrl-names from the required list as pinctrl properties are
inherited and validated by the pinctrl schema.
Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
---
Changes in v4:
- Fix compatible to "st,sti-pwm" in examples.
- Modify subject line to point to the correct YAML file.
- Link to v3: https://patch.msgid.link/20260625-st-pwm-v3-1-ace9bff88529@gmail.com
Changes in v3:
- Fix compatible to "st,sti-pwm".
- Drop pinctrl-names from required.
- Modify the commit message to explain the changes made.
- Change maintainer to "Lee Jones <lee.jones@linaro.org>".
- Link to v2: https://patch.msgid.link/20260618-st-pwm-v2-1-c792d5795ce2@gmail.com
Changes in v2:
- Add the missing interrupts to the required following the old binding.
- Modify the commit message to explain the changes made.
- Link to v1: https://patch.msgid.link/20260613-st-pwm-v1-1-458c2c89709a@gmail.com
---
Documentation/devicetree/bindings/pwm/pwm-st.txt | 43 ----------
.../devicetree/bindings/pwm/st,sti-pwm.yaml | 92 ++++++++++++++++++++++
2 files changed, 92 insertions(+), 43 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-st.txt b/Documentation/devicetree/bindings/pwm/pwm-st.txt
deleted file mode 100644
index 19fce774cafa..000000000000
--- a/Documentation/devicetree/bindings/pwm/pwm-st.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-STMicroelectronics PWM driver bindings
---------------------------------------
-
-Required parameters:
-- compatible : "st,pwm"
-- #pwm-cells : Number of cells used to specify a PWM. First cell
- specifies the per-chip index of the PWM to use and the
- second cell is the period in nanoseconds - fixed to 2
- for STiH41x.
-- reg : Physical base address and length of the controller's
- registers.
-- pinctrl-names: Set to "default".
-- pinctrl-0: List of phandles pointing to pin configuration nodes
- for PWM module.
- For Pinctrl properties, please refer to [1].
-- clock-names: Valid entries are "pwm" and/or "capture".
-- clocks: phandle of the clock used by the PWM module.
- For Clk properties, please refer to [2].
-- interrupts: IRQ for the Capture device
-
-Optional properties:
-- st,pwm-num-chan: Number of available PWM channels. Default is 0.
-- st,capture-num-chan: Number of available Capture channels. Default is 0.
-
-[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Example:
-
-pwm1: pwm@fe510000 {
- compatible = "st,pwm";
- reg = <0xfe510000 0x68>;
- #pwm-cells = <2>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_pwm1_chan0_default
- &pinctrl_pwm1_chan1_default
- &pinctrl_pwm1_chan2_default
- &pinctrl_pwm1_chan3_default>;
- clocks = <&clk_sysin>;
- clock-names = "pwm";
- st,pwm-num-chan = <4>;
- st,capture-num-chan = <2>;
-};
diff --git a/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
new file mode 100644
index 000000000000..e980c76b54e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/st,sti-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STiH41x PWM/Capture controller
+
+maintainers:
+ - Lee Jones <lee.jones@linaro.org>
+
+description:
+ The STiH41x PWM controller supports both PWM output and input capture
+ functionality. It provides multiple PWM output channels for generating
+ variable duty-cycle waveforms, and multiple input capture channels for
+ measuring external signal periods and pulse widths. PWM output channels
+ and input capture channels are configured independently via
+ st,pwm-num-chan and st,capture-num-chan respectively.
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ const: st,sti-pwm
+
+ reg:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 2
+
+ pinctrl-names:
+ const: default
+
+ clock-names:
+ items:
+ enum: [pwm, capture]
+ minItems: 1
+ maxItems: 2
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ interrupts:
+ description: IRQ line for the capture device.
+ maxItems: 1
+
+ st,pwm-num-chan:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of available PWM output channels.
+ minimum: 0
+ maximum: 4
+ default: 0
+
+ st,capture-num-chan:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of available input capture channels.
+ minimum: 0
+ maximum: 4
+ default: 0
+
+required:
+ - reg
+ - compatible
+ - "#pwm-cells"
+ - clock-names
+ - clocks
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ pwm@fe510000 {
+ compatible = "st,sti-pwm";
+ reg = <0xfe510000 0x68>;
+ interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
+ #pwm-cells = <2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm1_chan0_default
+ &pinctrl_pwm1_chan1_default
+ &pinctrl_pwm1_chan2_default
+ &pinctrl_pwm1_chan3_default>;
+ clocks = <&clk_sysin>;
+ clock-names = "pwm";
+ st,pwm-num-chan = <4>;
+ st,capture-num-chan = <2>;
+ };
+...
---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260613-st-pwm-1feade6cfea3
Best regards,
--
Charan Pedumuru <charan.pedumuru@gmail.com>
^ permalink raw reply related
* Re: [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver
From: sashiko-bot @ 2026-06-25 11:20 UTC (permalink / raw)
To: Chi-Wen Weng; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260625110638.38438-3-cwweng.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Multi-channel buffered scans read stale or actively converting data for subsequent channels due to premature interrupt mapping.
- [High] Unpaced software re-triggering loop in the buffer interrupt handler causes an IRQ storm and livelock.
- [High] Missing sign-extension and incorrect scan type for negative differential readings.
- [High] Device suspend does not synchronize with ongoing direct conversions, leading to potential hardware aborts.
- [Medium] Differential negative channel configuration from device tree is accepted but completely ignored by hardware configuration.
--
commit 71778e9c3ef41a6d7da62ab5f861733a638e3f61
Author: Chi-Wen Weng <cwweng@nuvoton.com>
iio: adc: Add Nuvoton MA35D1 EADC driver
This patch adds an IIO driver for the Nuvoton MA35D1 Enhanced ADC
controller, supporting both direct reads and triggered buffered capture.
> diff --git a/drivers/iio/adc/ma35d1_eadc.c b/drivers/iio/adc/ma35d1_eadc.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/ma35d1_eadc.c
[ ... ]
> +static void ma35d1_adc_config_sample(struct ma35d1_adc *adc,
> + unsigned int sample, unsigned int channel)
> +{
> + u32 reg = MA35D1_EADC_SCTL(sample);
> +
> + ma35d1_adc_rmw(adc, reg,
> + MA35D1_EADC_SCTL_CHSEL_MASK |
> + MA35D1_EADC_SCTL_TRGSEL_MASK,
> + FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK, channel) |
> + MA35D1_EADC_SCTL_TRGSEL_ADINT0);
> +}
[Severity: Medium]
Will the hardware properly sample differential pairs if only the positive
channel is configured here?
The driver parses differential configurations using diff-channels and stores
the negative channel in chan->channel2, but only the positive channel is ever
written to the hardware CHSEL mask here.
If the hardware relies on arbitrary negative channel configurations rather
than fixed hardware pairings, the negative pin won't be sampled correctly.
Additionally, ma35d1_adc_parse_channels() doesn't seem to validate against
any fixed hardware pair requirements.
[ ... ]
> +static irqreturn_t ma35d1_adc_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ma35d1_adc *adc = iio_priv(indio_dev);
> + int i;
> +
> + for (i = 0; i < adc->scan_chancnt; i++)
> + adc->scan.channels[i] =
> + ma35d1_adc_read(adc, MA35D1_EADC_DAT(i)) &
> + MA35D1_EADC_DAT_MASK;
[Severity: High]
Will this loop read stale or actively converting data for channels 1 through N
during multi-channel scans?
In ma35d1_adc_buffer_postenable(), the interrupt source is hardcoded to sample
module 0:
ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
MA35D1_EADC_INTSRC0_ADINT0,
MA35D1_EADC_INTSRC0_ADINT0);
This means the interrupt fires as soon as the first channel finishes
converting. When this threaded handler wakes up and immediately reads all
DAT(i) registers, the subsequent sample modules may not have completed their
conversions yet.
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, pf->timestamp);
> + iio_trigger_notify_done(adc->trig);
> +
> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
> + MA35D1_EADC_CTL_ADCIEN0);
> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
[Severity: High]
Does unconditionally triggering the next conversion here cause an interrupt
storm?
Since the driver starts the conversion in postenable by writing SWTRG = 1,
and then unconditionally restarts it immediately at the end of the handler,
this creates a 0-delay free-running loop without any hardware timer or
pacing mechanism. This can livelock the system by flooding the CPU with
interrupts as fast as the ADC converts.
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ma35d1_adc_read_conversion(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val)
> +{
[ ... ]
> + timeout = wait_for_completion_interruptible_timeout(&adc->completion,
> + MA35D1_EADC_TIMEOUT);
> + ma35d1_adc_disable_irq(adc);
> +
> + if (timeout < 0)
> + return timeout;
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + *val = ma35d1_adc_read(adc, MA35D1_EADC_DAT(0)) & MA35D1_EADC_DAT_MASK;
[Severity: High]
Will this return massive positive values for negative differential readings?
Differential channels return two's complement values. Without using
sign_extend32() before assigning to the 32-bit *val, negative voltage
differences will be incorrectly returned to userspace as large positive
integers instead of signed values.
[ ... ]
> +static void ma35d1_adc_init_channel(struct ma35d1_adc *adc,
> + struct iio_chan_spec *chan, u32 vinp,
> + u32 vinn, int scan_index, bool differential)
> +{
[ ... ]
> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + chan->scan_type.sign = 'u';
[Severity: High]
Should differential channels be marked as signed instead of unsigned?
When differential is true, this forces the scan type sign to 'u', which
incorrectly informs the IIO subsystem that the differential data is unsigned
instead of two's complement.
[ ... ]
> +static int ma35d1_adc_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ma35d1_adc *adc = iio_priv(indio_dev);
> +
> + if (iio_buffer_enabled(indio_dev))
> + return -EBUSY;
> +
> + ma35d1_adc_hw_disable(adc);
> + clk_disable_unprepare(adc->clk);
[Severity: High]
Can this race with active direct conversions and cause a bus fault?
ma35d1_adc_suspend() disables the peripheral clock without acquiring
adc->lock or checking iio_device_claim_direct_mode().
If a concurrent direct read is waiting in ma35d1_adc_read_conversion():
timeout = wait_for_completion_interruptible_timeout(&adc->completion,
MA35D1_EADC_TIMEOUT);
ma35d1_adc_disable_irq(adc);
and suspend disables the clock, the wait may time out or be interrupted.
Accessing the ADC registers immediately afterward in ma35d1_adc_disable_irq()
with the clock disabled could result in a synchronous external abort.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625110638.38438-1-cwweng.linux@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: sashiko-bot @ 2026-06-25 11:18 UTC (permalink / raw)
To: Chi-Wen Weng; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260625110638.38438-2-cwweng.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `resets` property for a hardware block that has a dedicated reset line.
--
commit 1f48c0ec2f3eb7b2358c1e0e2db87a26859d5fc5
Author: Chi-Wen Weng <cwweng@nuvoton.com>
dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
This commit adds the devicetree binding for the Enhanced ADC controller
found on Nuvoton MA35D1 SoCs. It describes the register region, interrupt,
functional clock, and standard channel child nodes.
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
> new file mode 100644
> index 0000000000000..ae7ad0f7689ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
[ ... ]
> +properties:
> + compatible:
> + const: nuvoton,ma35d1-eadc
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
[Severity: Low]
Is the resets property missing from this binding? The Nuvoton MA35D1 SoC
explicitly provides a reset line for the Enhanced ADC block (defined as
MA35D1_RESET_EADC0 in include/dt-bindings/reset/nuvoton,ma35d1-reset.h).
Should the resets property be declared here and added to the required list
to accurately reflect the hardware dependencies and avoid dt-schema
validation errors if a board uses the reset line?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625110638.38438-1-cwweng.linux@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v8 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver
From: Julian Braha @ 2026-06-25 11:16 UTC (permalink / raw)
To: hehuan1, linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon,
devicetree, linux-kernel
Cc: ningyu, linmin, pinkesh.vaghela, luyulin, dongxuyang
In-Reply-To: <20260625061214.1648-1-hehuan1@eswincomputing.com>
Hi Huan,
On 6/25/26 07:12, hehuan1@eswincomputing.com wrote:
> +config SENSORS_EIC7700_PVT
> + tristate "Eswin EIC7700 Voltage, Temperature sensor driver"
> + depends on ARCH_ESWIN || COMPILE_TEST
> + depends on HWMON
You already put SENSORS_EIC7700_PVT inside the 'if HWMON .. endif', so
you don't need the 'depends on HWMON'. It creates a duplicate
dependency.
- Julian Braha
^ permalink raw reply
* Re: [RFC PATCH 2/3] pinctrl: mediatek: mt8516/mt8167: Migrate driver to pinctrl-paris platform
From: sashiko-bot @ 2026-06-25 11:16 UTC (permalink / raw)
To: Luca Leonardo Scorcia; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260625104742.113803-3-l.scorcia@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Pinmux mode field spacing is incorrectly changed from 4 bits to 3 bits per pin, corrupting all pinmux configurations.
- [High] Incorrect register bit offsets and widths for Drive Strength (DRV) and Slew Rate (SR) corrupt hardware configuration.
- [High] SMT register bit mappings are swapped between pins 106 and 109.
- [High] SMT bit mappings for MT8516 pins 70-73 (MSDC2 DAT) are incorrectly assigned the MT8167 layout, breaking MT8516 SD/eMMC.
- [High] Driver drops support for `mediatek,pctl-regmap` without providing a fallback, breaking ABI backward compatibility with existing device trees.
--
commit b99ee859c088809165f9a4794ca1471b372efd64
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
pinctrl: mediatek: mt8516/mt8167: Migrate driver to pinctrl-paris platform
Migrate the mt8516/mt8167 pinctrl driver to the paris platform driver.
This change lets us correctly describe the two base addresses (GPIO/EINT)
used by the SoCs in their device tree. It also adds support for driving
strength capability and R1R0 pullup-pulldown on pins.
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8516.c b/drivers/pinctrl/mediatek/pinctrl-mt8516.c
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8516.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8516.c
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_mode_range[] = {
> + PIN_FIELD15(0, 124, 0x300, 0x10, 0, 3),
> +};
[Severity: Critical]
Does this configuration cause a regression in pin multiplexing across the SoC?
The legacy driver used a 4-bit shift per pin to pack 5 pins per 32-bit
register. By passing 3 as the width here, the framework calculates mode
offsets using 3 bits per pin. This causes pinmux writes to align with
the wrong bits and can overwrite the configurations of adjacent pins.
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_smt_range[] = {
[ ... ]
> + PIN_FIELD16(70, 70, 0xa30, 0x10, 3, 1),
> + PIN_FIELD16(71, 71, 0xa30, 0x10, 4, 1),
> + PIN_FIELD16(72, 72, 0xa30, 0x10, 5, 1),
> + PIN_FIELD16(73, 73, 0xa30, 0x10, 6, 1),
[Severity: High]
Does this break the SMT configuration for the MT8516 MSDC2 interface?
The unified array assigns the MT8167 layout to pins 70-73 (bits 3, 4, 5, 6).
The MT8516 requires bits 6, 5, 4, 3, as defined in its IES layout and the
legacy driver.
[ ... ]
> + PIN_FIELD16(106, 106, 0xa20, 0x10, 13, 1),
> + PIN_FIELD16(107, 107, 0xa20, 0x10, 14, 1),
> + PIN_FIELD16(108, 108, 0xa20, 0x10, 15, 1),
> + PIN_FIELD16(109, 109, 0xa30, 0x10, 0, 1),
[Severity: High]
Are the SMT bits for pins 106 and 109 swapped?
In the IES range, pin 106 maps to 0x930 bit 0 and pin 109 maps to 0x920
bit 13. SMT layouts typically mirror IES layouts offset by 0x100.
The mappings here write to the wrong hardware bits for these pins.
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_drv_range[] = {
> + PINS_FIELD16(0, 4, 0xd00, 0x10, 0, 2),
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_sr_range[] = {
> + PINS_FIELD16(0, 4, 0xd00, 0x10, 3, 1),
[Severity: High]
Can this corrupt the Slew Rate hardware configuration?
Setting the Slew Rate offset to 3, and using a 2-bit width starting at bit 0
for the Drive Strength of these pins, differs from the hardware requirements.
Additionally, a 2-bit mask restricts DRV_GRP4 pins (such as MSDC) from
supporting drive strengths greater than 8mA, causing the framework to reject
higher values.
[ ... ]
> +static struct platform_driver mt8516_pinctrl_driver = {
> .driver = {
> .name = "mediatek-mt8516-pinctrl",
> - .of_match_table = mt8516_pctrl_match,
> - .pm = pm_sleep_ptr(&mtk_eint_pm_ops),
> + .of_match_table = mt8516_pinctrl_of_match,
> + .pm = pm_sleep_ptr(&mtk_paris_pinctrl_pm_ops),
> },
> + .probe = mtk_paris_pinctrl_probe,
> };
[Severity: High]
Will this migration cause probing to fail on older device trees?
The paris framework probe relies on devm_platform_ioremap_resource_byname
which strictly requires a reg-names property. The legacy driver supported a
mediatek,pctl-regmap fallback. Dropping this fallback might introduce a
regression with existing device trees lacking the reg-names property.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625104742.113803-1-l.scorcia@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices
From: sashiko-bot @ 2026-06-25 11:09 UTC (permalink / raw)
To: Jie Gan; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260625-enable-byte-cntr-for-ctcu-v19-6-8fbbf22e8381@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [Critical] Global function pointer accessed locklessly leads to Use-After-Free on driver unload
- [Critical] Global sysfs ops pointer triggers NULL pointer dereference on ETR without CTCU
- [High] Driver remove/unbind blocks indefinitely waiting for userspace file closure
- [High] ctcu_byte_cntr_stop() skips setting enable=false for SYSFS mode, causing infinite read hangs
- [High] TOCTOU race re-enables ETR hardware after framework disables it
- [High] Missing wake_up() in error path deadlocks driver remove()
- [High] Unsynchronized checks of irq_enabled lead to resource leaks and driver unbind hangs
- [High] Data race on concurrent read() calls causes NULL pointer dereference
- [Medium] Spurious Interrupt Handling due to missing hardware status check in shared IRQ
- [Medium] Error codes (-ERESTARTSYS) from byte_cntr_ops are swallowed and convert to EOF
- [Medium] Masked error code breaks sysfs reading fallback on memory failure
--
commit 128ffef0065b7b8a8c73e5e3f9b0ce8bebea0593
Author: Jie Gan <jie.gan@oss.qualcomm.com>
coresight: ctcu: enable byte-cntr for TMC ETR devices
This patch introduces the byte-cntr function provided by the CTCU device to
transfer data from the ETR buffer to userspace. It sets up an interrupt
handler to track data size thresholds and switches buffers when full or on
timeout. It also integrates with the TMC ETR framework via sysfs ops.
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
> new file mode 100644
> index 0000000000000..5ab97a71f02f6
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
[ ... ]
> +static irqreturn_t byte_cntr_handler(int irq, void *data)
> +{
> + struct ctcu_byte_cntr *byte_cntr_data = data;
> +
> + atomic_inc(&byte_cntr_data->irq_cnt);
> + wake_up(&byte_cntr_data->wq);
> +
> + return IRQ_HANDLED;
> +}
[Severity: Medium]
Is this handler missing a hardware status check?
Since the IRQ is registered with IRQF_SHARED, another device sharing the line
could fire an interrupt. By unconditionally returning IRQ_HANDLED without
verifying the interrupt source, the kernel's spurious interrupt detection
might be suppressed, and the CTCU buffer switch logic could be triggered
prematurely.
[ ... ]
> +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu,
> + struct coresight_device *etr)
> +{
> + struct ctcu_drvdata *drvdata = dev_get_drvdata(ctcu->dev.parent);
[Severity: Critical]
Could this dereference a NULL pointer if an ETR without a connected CTCU
is used?
If the system has multiple ETRs and one lacks a CTCU, the globally registered
byte_cntr_sysfs_ops might pass a NULL ctcu pointer here. This would cause a
panic when ctcu->dev.parent is accessed.
[ ... ]
> +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *etr_drvdata,
> + struct ctcu_byte_cntr *byte_cntr_data)
> +{
[ ... ]
> + raw_spin_unlock_irqrestore(&etr_drvdata->spinlock, flags);
> +
> + /* Restart the ETR once a free buffer is available */
> + if (found_free_buf &&
> + coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED)
> + tmc_etr_enable_disable_hw(etr_drvdata, true);
> +
> + return found_free_buf;
> +}
[Severity: High]
Does this lockless check introduce a race condition?
If a concurrent operation disables the trace session and drops the lock before
setting the mode to CS_MODE_DISABLED, this code might incorrectly re-enable
the hardware after the framework has explicitly disabled it.
[ ... ]
> +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos,
> + size_t len, char **bufpp)
> +{
[ ... ]
> + pos = byte_cntr_data->buf_node->pos;
> + sysfs_buf = byte_cntr_data->buf_node->sysfs_buf;
> + actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp);
> + if (actual <= 0) {
> + /* Reset buf_node upon reading is finished or failed */
> + byte_cntr_data->buf_node->reading = false;
> + byte_cntr_data->buf_node = NULL;
[Severity: High]
Can concurrent read operations cause a NULL pointer dereference here?
If two threads read from the same ETR file descriptor, they could concurrently
access and modify byte_cntr_data->buf_node without serialization, causing one
thread to clear the pointer while the other is still using it.
[ ... ]
> +static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata)
> +{
> + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
> + struct ctcu_byte_cntr *byte_cntr_data;
> + unsigned long flags;
> + int ret = 0;
> +
> + /* byte-cntr is operating with SYSFS mode being enabled only */
> + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS)
> + return -EINVAL;
> +
> + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
> + if (!byte_cntr_data || !byte_cntr_data->irq_enabled)
> + return -EINVAL;
> +
> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
[Severity: High]
Does checking irq_enabled outside the spinlock create a race?
A concurrent sysfs write could set irq_enabled to 0 immediately before the
lock is acquired. If reading is then set to true, read_unprepare() will later
abort early upon seeing irq_enabled is 0. This would leak the buffer list and
leave reading permanently true, causing the driver unbind to hang forever.
[ ... ]
> + byte_cntr_data->reading = true;
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + /* Setup an available etr_buf_list for byte-cntr */
> + ret = tmc_create_etr_buf_list(etr_drvdata, 2);
> + if (ret) {
> + byte_cntr_data->reading = false;
> + return ret;
> + }
[Severity: High]
Is this error path missing a wake_up() call?
If this allocation fails, the reading flag is cleared, but concurrent readers
waiting in the driver's remove function won't be notified, potentially
deadlocking the unbind process.
[ ... ]
> +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path)
> +{
> + struct coresight_device *sink = coresight_get_sink(path);
> + struct ctcu_byte_cntr *byte_cntr_data;
> +
> + if (coresight_get_mode(sink) == CS_MODE_SYSFS)
> + return;
[Severity: High]
Should the enable flag be cleared for SYSFS mode here?
By returning early, enable remains true. Readers waiting in
tmc_byte_cntr_get_data() might never receive the EOF condition when the
trace is stopped, causing them to loop indefinitely.
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index e8720026c9e3f..2da1a6f3d29f4 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> +static void ctcu_platform_remove(struct platform_device *pdev)
> +{
> + struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev);
> + struct ctcu_byte_cntr *byte_cntr_data;
> + unsigned long flags;
> + int i;
> +
> + if (WARN_ON(!drvdata))
> + return;
> +
> + /*
> + * Signal all active byte-cntr readers to exit, then wait for them to
> + * finish before resetting the ops pointer and freeing driver data.
> + * Without this, a reader blocked in wait_event_interruptible_timeout()
> + * would access the freed ctcu_drvdata wait-queue head (use-after-free).
> + */
> + for (i = 0; i < ETR_MAX_NUM; i++) {
> + byte_cntr_data = &drvdata->byte_cntr_data[i];
> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
> + /* Set enable=false for all ports to signal teardown to racing readers */
> + byte_cntr_data->enable = false;
> + if (!byte_cntr_data->reading) {
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + continue;
> + }
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + wake_up_all(&byte_cntr_data->wq);
> + wait_event(byte_cntr_data->wq, !byte_cntr_data->reading);
> + }
[Severity: High]
Could this wait indefinitely during driver remove?
The reading flag is only cleared when the userspace file is closed. A
userspace process leaving the file descriptor open could permanently block
module unload or device unbind.
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 2b26ce6455a7b..e78f8891f11e0 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
> + loff_t pos, size_t len, char **bufpp)
> +{
> + ssize_t ret;
> + const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> +
> + if (byte_cntr_ops) {
> + ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp);
> + /* Return the filled buffer */
> + if (ret > 0 || ret == -ENOMEM)
> + return ret;
> + }
> +
> + return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp);
> +}
[Severity: Medium]
Does this silently swallow -ERESTARTSYS?
If a signal interrupts the wait inside tmc_byte_cntr_get_data(), it returns
-ERESTARTSYS, which falls through here to a normal sysfs buffer read. This
might convert the signal into an EOF or return unsynced data instead of
letting userspace handle the signal cleanly.
[ ... ]
> +int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> +{
> + int ret = 0;
> + unsigned long flags;
> + const struct tmc_sysfs_ops *byte_cntr_ops;
>
> /* config types are set a boot time and never change */
> if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
> return -EINVAL;
>
> + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> + if (byte_cntr_ops) {
> + ret = byte_cntr_ops->read_prepare(drvdata);
[Severity: Critical]
Is it safe to access this global function pointer locklessly?
If the trace file is opened and the CTCU module is concurrently unloaded, the
memory backing byte_cntr_ops could be freed before read_prepare() executes,
leading to a use-after-free regression.
> + if (!ret || ret == -EBUSY)
> + return ret;
> +
> + ret = 0;
> + }
[Severity: Medium]
Why are errors like -ENOMEM from read_prepare() masked here?
If the memory allocation fails inside tmc_read_prepare_byte_cntr(), hiding the
error breaks the fallback sysfs reading path, leading to delays and hanging
operations instead of propagating the failure properly.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=6
^ permalink raw reply
* Re: [RFC PATCH 3/3] arm64: dts: mt8516/mt8167: Update pinctrl nodes for the new paris driver
From: sashiko-bot @ 2026-06-25 11:07 UTC (permalink / raw)
To: Luca Leonardo Scorcia; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260625104742.113803-4-l.scorcia@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `gpio-ranges` property incorrectly specifies a count of 124, which excludes the final pin (GPIO 124) from being mapped to the GPIO subsystem, as the drivers define exactly 125 pins.
--
commit 36a8d0bd5d2cccdc70b499936029deb0499393de
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
arm64: dts: mt8516/mt8167: Update pinctrl nodes for the new paris driver
This patch updates the MediaTek mt8516 and mt8167 SoC descriptions to
respect the constraints of the Paris pinctrl driver. It sets separate base
and eint registers and removes the deprecated syscfg_pctl syscon node.
> diff --git a/arch/arm64/boot/dts/mediatek/mt8516.dtsi b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> index b5e753759465d..63f36df4d1b48 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8516.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8516.dtsi
[ ... ]
> @@ -231,17 +231,13 @@ keypad: keypad@10002000 {
> status = "disabled";
> };
>
> - syscfg_pctl: syscfg-pctl@10005000 {
> - compatible = "syscon";
> - reg = <0 0x10005000 0 0x1000>;
> - };
> -
> - pio: pinctrl@1000b000 {
> + pio: pinctrl@10005000 {
> compatible = "mediatek,mt8516-pinctrl";
> - reg = <0 0x1000b000 0 0x1000>;
> - mediatek,pctl-regmap = <&syscfg_pctl>;
> + reg = <0 0x10005000 0 0x1000>, <0 0x1000b000 0 0x1000>;
> + reg-names = "base", "eint";
> gpio-controller;
> #gpio-cells = <2>;
> + gpio-ranges = <&pio 0 0 124>;
[Severity: Medium]
Does this count parameter accidentally exclude the final pin (GPIO 124) from
being mapped to the GPIO subsystem?
Looking at the driver definitions for the MT8516 and MT8167 platforms, there
are exactly 125 pins defined. A count of 124 will prevent the mapping of the
final pin, causing any driver or application attempting to request GPIO 124 to
fail. Since mt8167.dtsi includes mt8516.dtsi, this affects both SoCs.
Should this count parameter be 125 instead?
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625104742.113803-1-l.scorcia@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic
From: sashiko-bot @ 2026-06-25 11:07 UTC (permalink / raw)
To: Jie Gan; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260625-enable-byte-cntr-for-ctcu-v19-1-8fbbf22e8381@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Lockless iteration over `pdata->in_conns` array introduces a TOCTOU race condition and potential Use-After-Free.
Pre-existing issues:
- [High] Missing bounds check for Device Tree-provided `port_num` leads to out-of-bounds array access in CTCU trace ID configuration.
- [High] Unsafe direct dereference of sparse array elements in `tpda_get_element_size`.
--
commit f655e6df0885045f179728cb52f455717556ac67
Author: Jie Gan <jie.gan@oss.qualcomm.com>
coresight: core: refactor ctcu_get_active_port and make it generic
This patch removes ctcu_get_active_port from the CTCU module and adds it to
the core framework as coresight_get_in_port. It allows retrieving the active
input port number generically for a given device.
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..7a7a85acdca0d 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -773,6 +773,33 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
[ ... ]
> +int coresight_get_in_port(struct coresight_device *csdev,
> + struct coresight_device *remote)
> +{
> + struct coresight_platform_data *pdata = remote->pdata;
> + int i;
> +
> + for (i = 0; i < pdata->nr_inconns; ++i) {
> + if (!pdata->in_conns[i])
> + continue;
> +
> + if (pdata->in_conns[i]->src_dev == csdev)
[Severity: High]
Can this sequence cause a race condition leading to a NULL pointer
dereference?
The array element is read twice. First, to check if it's NULL, and then
again to access src_dev. If a concurrent device unbind operation like
coresight_remove_conns() sets the element to NULL between these reads, the
second access might trigger a panic.
Should we use READ_ONCE() to load the pointer into a local variable first?
> + return pdata->in_conns[i]->dest_port;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_get_in_port);
> +
> u32 coresight_get_sink_id(struct coresight_device *csdev)
> {
> if (!csdev->ea)
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index 9043cad42f01e..e8720026c9e3f 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> @@ -145,7 +128,7 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight
> return -EINVAL;
> }
>
> - port_num = ctcu_get_active_port(sink, csdev);
> + port_num = coresight_get_in_port(sink, csdev);
> if (port_num < 0)
> return -EINVAL;
[Severity: High]
This isn't a bug introduced by this patch, but does the returned
port_num need to be bounds-checked before being used?
The port_num originates from device tree properties, and if it exceeds
ETR_MAX_NUM (2), it leads to out-of-bounds array accesses in
__ctcu_set_etr_traceid() when indexing into drvdata->atid_offset and
drvdata->traceid_refcnt:
__ctcu_set_etr_traceid()
...
atid_offset = drvdata->atid_offset[port_num];
...
refcnt = drvdata->traceid_refcnt[port_num][traceid];
Would it be safer to validate port_num against the array bounds after
retrieving it?
>
[ ... ]
[Severity: High]
This is a pre-existing issue not introduced by this patch, but while
looking at the sparse in_conns array usage, I noticed a similar issue in
tpda_get_element_size().
Does this code dereference the sparse array element before checking if it
is valid?
drivers/hwtracing/coresight/coresight-tpda.c:tpda_get_element_size() {
...
for (i = 0; i < csdev->pdata->nr_inconns; i++) {
in = csdev->pdata->in_conns[i]->src_dev;
if (!in)
continue;
...
}
If a component is unbound and the connection is removed by
coresight_remove_conns(), csdev->pdata->in_conns[i] could be NULL,
causing a panic before the check is reached.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=1
^ permalink raw reply
* [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver
From: Chi-Wen Weng @ 2026-06-25 11:06 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-arm-kernel, linux-iio, devicetree,
linux-kernel, cwweng, cwweng.linux
In-Reply-To: <20260625110638.38438-1-cwweng.linux@gmail.com>
From: Chi-Wen Weng <cwweng@nuvoton.com>
Add an IIO driver for the Nuvoton MA35D1 Enhanced ADC controller.
The driver supports direct raw reads and triggered buffered capture. The
controller end-of-conversion interrupt is exposed as the device trigger
and is used to push samples into the IIO buffer.
Channels are described by firmware child nodes and can be configured as
single-ended or differential inputs. Since the differential enable bit is
global, mixed single-ended and differential buffered scans are rejected.
DMA support is intentionally not included in this initial upstream driver;
conversions are handled through the interrupt-driven path.
Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
---
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ma35d1_eadc.c | 636 ++++++++++++++++++++++++++++++++++
3 files changed, 647 insertions(+)
create mode 100644 drivers/iio/adc/ma35d1_eadc.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 1c663c98c6c9..43409999a94b 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -981,6 +981,16 @@ config LTC2497
To compile this driver as a module, choose M here: the module will be
called ltc2497.
+config MA35D1_EADC
+ tristate "MA35D1 EADC driver"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say yes here to build support for MA35D1 EADC.
+
+ To compile this driver as a module, choose M here: the module will be
+ called ma35d1.
+
config MAX1027
tristate "Maxim max1027 ADC driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 707dd708912f..7b9b38688223 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_LTC2471) += ltc2471.o
obj-$(CONFIG_LTC2485) += ltc2485.o
obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
+obj-$(CONFIG_MA35D1_EADC) += ma35d1_eadc.o
obj-$(CONFIG_MAX1027) += max1027.o
obj-$(CONFIG_MAX11100) += max11100.o
obj-$(CONFIG_MAX1118) += max1118.o
diff --git a/drivers/iio/adc/ma35d1_eadc.c b/drivers/iio/adc/ma35d1_eadc.c
new file mode 100644
index 000000000000..0c075126e139
--- /dev/null
+++ b/drivers/iio/adc/ma35d1_eadc.c
@@ -0,0 +1,636 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton MA35D1 EADC driver
+ *
+ * Copyright (c) 2026 Nuvoton Technology Corp.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/property.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define MA35D1_EADC_DAT(n) (0x00 + (n) * 0x04)
+#define MA35D1_EADC_CTL 0x50
+#define MA35D1_EADC_SWTRG 0x54
+#define MA35D1_EADC_SCTL(n) (0x80 + (n) * 0x04)
+#define MA35D1_EADC_INTSRC0 0xd0
+#define MA35D1_EADC_STATUS2 0xf8
+#define MA35D1_EADC_SELSMP0 0x140
+#define MA35D1_EADC_REFADJCTL 0x150
+
+#define MA35D1_EADC_CTL_ADCEN BIT(0)
+#define MA35D1_EADC_CTL_ADCIEN0 BIT(2)
+#define MA35D1_EADC_CTL_DIFFEN BIT(8)
+
+#define MA35D1_EADC_SCTL_CHSEL_MASK GENMASK(3, 0)
+#define MA35D1_EADC_SCTL_TRGDLY_MASK GENMASK(15, 8)
+#define MA35D1_EADC_SCTL_TRGSEL_MASK GENMASK(21, 16)
+#define MA35D1_EADC_SCTL_TRGSEL_ADINT0 \
+ FIELD_PREP(MA35D1_EADC_SCTL_TRGSEL_MASK, 2)
+
+#define MA35D1_EADC_DAT_MASK GENMASK(11, 0)
+#define MA35D1_EADC_STATUS2_ADIF0 BIT(0)
+#define MA35D1_EADC_INTSRC0_ADINT0 BIT(0)
+#define MA35D1_EADC_REFADJCTL_EXT_VREF BIT(0)
+
+#define MA35D1_EADC_MAX_CHANNELS 9
+#define MA35D1_EADC_MAX_SAMPLE_MODULES 16
+#define MA35D1_EADC_CHAN_NAME_LEN 16
+#define MA35D1_EADC_TIMEOUT msecs_to_jiffies(1000)
+
+struct ma35d1_adc {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *clk;
+ struct completion completion;
+ /* Protects direct conversions against concurrent register access. */
+ struct mutex lock;
+ struct iio_trigger *trig;
+ unsigned int scan_chancnt;
+ bool scan_differential;
+ char chan_name[MA35D1_EADC_MAX_CHANNELS][MA35D1_EADC_CHAN_NAME_LEN];
+ struct {
+ u16 channels[MA35D1_EADC_MAX_SAMPLE_MODULES];
+ aligned_s64 timestamp;
+ } scan;
+};
+
+static inline u32 ma35d1_adc_read(struct ma35d1_adc *adc, u32 reg)
+{
+ return readl(adc->regs + reg);
+}
+
+static inline void ma35d1_adc_write(struct ma35d1_adc *adc, u32 reg, u32 val)
+{
+ writel(val, adc->regs + reg);
+}
+
+static void ma35d1_adc_rmw(struct ma35d1_adc *adc, u32 reg, u32 mask, u32 val)
+{
+ u32 tmp;
+
+ tmp = ma35d1_adc_read(adc, reg);
+ tmp &= ~mask;
+ tmp |= val;
+ ma35d1_adc_write(adc, reg, tmp);
+}
+
+static void ma35d1_adc_set_diff(struct ma35d1_adc *adc, bool differential)
+{
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_DIFFEN,
+ differential ? MA35D1_EADC_CTL_DIFFEN : 0);
+}
+
+static void ma35d1_adc_config_sample(struct ma35d1_adc *adc,
+ unsigned int sample, unsigned int channel)
+{
+ u32 reg = MA35D1_EADC_SCTL(sample);
+
+ ma35d1_adc_rmw(adc, reg,
+ MA35D1_EADC_SCTL_CHSEL_MASK |
+ MA35D1_EADC_SCTL_TRGSEL_MASK,
+ FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK, channel) |
+ MA35D1_EADC_SCTL_TRGSEL_ADINT0);
+}
+
+static void ma35d1_adc_disable_irq(struct ma35d1_adc *adc)
+{
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0, 0);
+}
+
+static void ma35d1_adc_hw_init(struct ma35d1_adc *adc)
+{
+ ma35d1_adc_disable_irq(adc);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL,
+ MA35D1_EADC_CTL_ADCEN, MA35D1_EADC_CTL_ADCEN);
+ ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
+ MA35D1_EADC_INTSRC0_ADINT0,
+ MA35D1_EADC_INTSRC0_ADINT0);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_REFADJCTL,
+ MA35D1_EADC_REFADJCTL_EXT_VREF,
+ MA35D1_EADC_REFADJCTL_EXT_VREF);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_SELSMP0, GENMASK(1, 0), 3);
+}
+
+static void ma35d1_adc_hw_disable(void *data)
+{
+ struct ma35d1_adc *adc = data;
+
+ ma35d1_adc_disable_irq(adc);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCEN, 0);
+}
+
+static irqreturn_t ma35d1_adc_isr(int irq, void *data)
+{
+ struct iio_dev *indio_dev = data;
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ u32 status;
+
+ status = ma35d1_adc_read(adc, MA35D1_EADC_STATUS2);
+ if (!(status & MA35D1_EADC_STATUS2_ADIF0))
+ return IRQ_NONE;
+
+ ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
+
+ if (iio_buffer_enabled(indio_dev)) {
+ ma35d1_adc_disable_irq(adc);
+ iio_trigger_poll(adc->trig);
+ } else {
+ complete(&adc->completion);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t ma35d1_adc_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int i;
+
+ for (i = 0; i < adc->scan_chancnt; i++)
+ adc->scan.channels[i] =
+ ma35d1_adc_read(adc, MA35D1_EADC_DAT(i)) &
+ MA35D1_EADC_DAT_MASK;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, pf->timestamp);
+ iio_trigger_notify_done(adc->trig);
+
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
+ MA35D1_EADC_CTL_ADCIEN0);
+ ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
+
+ return IRQ_HANDLED;
+}
+
+static int ma35d1_adc_read_conversion(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ long timeout;
+
+ reinit_completion(&adc->completion);
+
+ ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(0),
+ MA35D1_EADC_SCTL_CHSEL_MASK |
+ MA35D1_EADC_SCTL_TRGSEL_MASK,
+ FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK,
+ chan->channel));
+ ma35d1_adc_set_diff(adc, chan->differential);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
+ MA35D1_EADC_CTL_ADCIEN0);
+ ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
+
+ timeout = wait_for_completion_interruptible_timeout(&adc->completion,
+ MA35D1_EADC_TIMEOUT);
+ ma35d1_adc_disable_irq(adc);
+
+ if (timeout < 0)
+ return timeout;
+ if (!timeout)
+ return -ETIMEDOUT;
+
+ *val = ma35d1_adc_read(adc, MA35D1_EADC_DAT(0)) & MA35D1_EADC_DAT_MASK;
+
+ return 0;
+}
+
+static int ma35d1_adc_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+
+ mutex_lock(&adc->lock);
+ ret = ma35d1_adc_read_conversion(indio_dev, chan, val);
+ mutex_unlock(&adc->lock);
+
+ iio_device_release_direct(indio_dev);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ma35d1_adc_validate_scan(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ const struct iio_chan_spec *chan;
+ bool have_single = false;
+ bool have_diff = false;
+ unsigned int count = 0;
+ unsigned long bit;
+
+ for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
+ chan = &indio_dev->channels[bit];
+
+ if (chan->type == IIO_TIMESTAMP)
+ continue;
+ count++;
+ if (chan->differential)
+ have_diff = true;
+ else
+ have_single = true;
+ }
+
+ if (!count || count > MA35D1_EADC_MAX_SAMPLE_MODULES)
+ return -EINVAL;
+
+ if (have_single && have_diff)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int ma35d1_adc_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ const struct iio_chan_spec *chan;
+ unsigned int sample = 0;
+ unsigned long bit;
+ bool differential = false;
+ int ret;
+
+ ret = ma35d1_adc_validate_scan(indio_dev, scan_mask);
+ if (ret)
+ return ret;
+
+ for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
+ chan = &indio_dev->channels[bit];
+ if (chan->type == IIO_TIMESTAMP)
+ continue;
+
+ if (!sample)
+ differential = chan->differential;
+
+ ma35d1_adc_config_sample(adc, sample, chan->channel);
+ sample++;
+ }
+
+ adc->scan_chancnt = sample;
+ adc->scan_differential = differential;
+
+ return 0;
+}
+
+static int ma35d1_adc_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int i;
+
+ if (!adc->scan_chancnt)
+ return -EINVAL;
+
+ ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
+ MA35D1_EADC_INTSRC0_ADINT0,
+ MA35D1_EADC_INTSRC0_ADINT0);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_REFADJCTL,
+ MA35D1_EADC_REFADJCTL_EXT_VREF,
+ MA35D1_EADC_REFADJCTL_EXT_VREF);
+ ma35d1_adc_rmw(adc, MA35D1_EADC_SELSMP0, GENMASK(1, 0), 3);
+ ma35d1_adc_set_diff(adc, adc->scan_differential);
+
+ for (i = 0; i < adc->scan_chancnt; i++)
+ ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
+ MA35D1_EADC_SCTL_TRGDLY_MASK,
+ MA35D1_EADC_SCTL_TRGDLY_MASK);
+
+ ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
+ MA35D1_EADC_CTL_ADCIEN0);
+ ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
+
+ return 0;
+}
+
+static int ma35d1_adc_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int i;
+
+ ma35d1_adc_disable_irq(adc);
+ for (i = 0; i < adc->scan_chancnt; i++)
+ ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
+ MA35D1_EADC_SCTL_TRGSEL_MASK, 0);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops ma35d1_adc_buffer_ops = {
+ .postenable = ma35d1_adc_buffer_postenable,
+ .predisable = ma35d1_adc_buffer_predisable,
+};
+
+static const struct iio_info ma35d1_adc_info = {
+ .read_raw = ma35d1_adc_read_raw,
+ .update_scan_mode = ma35d1_adc_update_scan_mode,
+};
+
+static const struct iio_trigger_ops ma35d1_adc_trigger_ops = {
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static void ma35d1_adc_init_channel(struct ma35d1_adc *adc,
+ struct iio_chan_spec *chan, u32 vinp,
+ u32 vinn, int scan_index, bool differential)
+{
+ char *name = adc->chan_name[vinp];
+
+ chan->type = IIO_VOLTAGE;
+ chan->indexed = 1;
+ chan->channel = vinp;
+ chan->address = vinp;
+ chan->scan_index = scan_index;
+ chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+ chan->scan_type.sign = 'u';
+ chan->scan_type.realbits = 12;
+ chan->scan_type.storagebits = 16;
+ chan->scan_type.endianness = IIO_CPU;
+
+ if (differential) {
+ chan->differential = 1;
+ chan->channel2 = vinn;
+ snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d-in%d", vinp,
+ vinn);
+ } else {
+ snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d", vinp);
+ }
+
+ chan->datasheet_name = name;
+}
+
+static int ma35d1_adc_parse_channels(struct iio_dev *indio_dev,
+ struct device *dev)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ DECLARE_BITMAP(used_channels, MA35D1_EADC_MAX_CHANNELS);
+ struct fwnode_handle *child;
+ struct iio_chan_spec *channels;
+ int num_channels;
+ int scan_index = 0;
+ int ret;
+
+ bitmap_zero(used_channels, MA35D1_EADC_MAX_CHANNELS);
+
+ num_channels = device_get_child_node_count(dev);
+ if (!num_channels)
+ return dev_err_probe(dev, -ENODATA,
+ "no ADC channels configured\n");
+
+ if (num_channels > MA35D1_EADC_MAX_CHANNELS)
+ return dev_err_probe(dev, -EINVAL, "too many ADC channels\n");
+
+ channels = devm_kcalloc(dev, num_channels + 1, sizeof(*channels),
+ GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ device_for_each_child_node(dev, child) {
+ u32 diff[2];
+ u32 reg;
+ bool differential = false;
+
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, ret,
+ "missing channel reg property\n");
+ }
+
+ if (reg >= MA35D1_EADC_MAX_CHANNELS) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, -EINVAL,
+ "invalid ADC channel %u\n", reg);
+ }
+
+ if (test_and_set_bit(reg, used_channels)) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, -EINVAL,
+ "duplicate ADC channel %u\n", reg);
+ }
+
+ if (fwnode_property_present(child, "diff-channels")) {
+ ret = fwnode_property_read_u32_array(child,
+ "diff-channels",
+ diff,
+ ARRAY_SIZE(diff));
+ if (ret) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, ret,
+ "invalid diff-channels for channel %u\n",
+ reg);
+ }
+
+ if (diff[0] != reg ||
+ diff[1] >= MA35D1_EADC_MAX_CHANNELS ||
+ diff[0] == diff[1]) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, -EINVAL,
+ "invalid differential ADC channel %u-%u\n",
+ diff[0], diff[1]);
+ }
+
+ if (test_and_set_bit(diff[1], used_channels)) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, -EINVAL,
+ "ADC channel %u already used\n",
+ diff[1]);
+ }
+
+ differential = true;
+ }
+
+ ma35d1_adc_init_channel(adc, &channels[scan_index], reg,
+ differential ? diff[1] : 0,
+ scan_index, differential);
+ scan_index++;
+ }
+
+ channels[scan_index] = (struct iio_chan_spec)
+ IIO_CHAN_SOFT_TIMESTAMP(scan_index);
+
+ indio_dev->channels = channels;
+ indio_dev->num_channels = scan_index + 1;
+ indio_dev->masklength = indio_dev->num_channels;
+
+ return 0;
+}
+
+static int ma35d1_adc_setup_trigger(struct iio_dev *indio_dev,
+ struct device *dev)
+{
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int ret;
+
+ adc->trig = devm_iio_trigger_alloc(dev, "%s-trigger", dev_name(dev));
+ if (!adc->trig)
+ return -ENOMEM;
+
+ adc->trig->ops = &ma35d1_adc_trigger_ops;
+ iio_trigger_set_drvdata(adc->trig, indio_dev);
+
+ ret = devm_iio_trigger_register(dev, adc->trig);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register trigger\n");
+
+ ret = iio_trigger_set_immutable(indio_dev, adc->trig);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to set trigger\n");
+
+ return 0;
+}
+
+static int ma35d1_adc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iio_dev *indio_dev;
+ struct ma35d1_adc *adc;
+ int irq;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
+ if (!indio_dev)
+ return -ENOMEM;
+ adc = iio_priv(indio_dev);
+ adc->dev = dev;
+ mutex_init(&adc->lock);
+ init_completion(&adc->completion);
+
+ adc->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(adc->regs))
+ return dev_err_probe(dev, PTR_ERR(adc->regs),
+ "failed to map registers\n");
+
+ adc->clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(adc->clk))
+ return dev_err_probe(dev, PTR_ERR(adc->clk),
+ "failed to get and enable ADC clock\n");
+
+ indio_dev->name = "ma35d1-eadc";
+ indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
+ indio_dev->info = &ma35d1_adc_info;
+
+ ret = ma35d1_adc_parse_channels(indio_dev, dev);
+ if (ret)
+ return ret;
+
+ ma35d1_adc_hw_init(adc);
+
+ ret = devm_add_action_or_reset(dev, ma35d1_adc_hw_disable, adc);
+ if (ret)
+ return ret;
+
+ ret = ma35d1_adc_setup_trigger(indio_dev, dev);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_irq(dev, irq, ma35d1_adc_isr, 0, dev_name(dev),
+ indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to request IRQ %d\n", irq);
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+ iio_pollfunc_store_time,
+ ma35d1_adc_trigger_handler,
+ &ma35d1_adc_buffer_ops);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to setup triggered buffer\n");
+
+ platform_set_drvdata(pdev, indio_dev);
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register IIO device\n");
+
+ return 0;
+}
+
+static int ma35d1_adc_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+
+ if (iio_buffer_enabled(indio_dev))
+ return -EBUSY;
+
+ ma35d1_adc_hw_disable(adc);
+ clk_disable_unprepare(adc->clk);
+
+ return 0;
+}
+
+static int ma35d1_adc_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct ma35d1_adc *adc = iio_priv(indio_dev);
+ int ret;
+
+ ret = clk_prepare_enable(adc->clk);
+ if (ret)
+ return ret;
+
+ ma35d1_adc_hw_init(adc);
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ma35d1_adc_pm_ops,
+ ma35d1_adc_suspend, ma35d1_adc_resume);
+
+static const struct of_device_id ma35d1_adc_of_match[] = {
+ { .compatible = "nuvoton,ma35d1-eadc" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ma35d1_adc_of_match);
+
+static struct platform_driver ma35d1_adc_driver = {
+ .probe = ma35d1_adc_probe,
+ .driver = {
+ .name = "ma35d1-eadc",
+ .of_match_table = ma35d1_adc_of_match,
+ .pm = pm_sleep_ptr(&ma35d1_adc_pm_ops),
+ },
+};
+module_platform_driver(ma35d1_adc_driver);
+
+MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 EADC driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: Chi-Wen Weng @ 2026-06-25 11:06 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-arm-kernel, linux-iio, devicetree,
linux-kernel, cwweng, cwweng.linux
In-Reply-To: <20260625110638.38438-1-cwweng.linux@gmail.com>
From: Chi-Wen Weng <cwweng@nuvoton.com>
Add devicetree binding for the Enhanced ADC controller found on
Nuvoton MA35D1 SoCs.
The controller has one register region, one interrupt and one functional
clock. ADC inputs are described using standard channel child nodes,
including optional differential channel pairs.
Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
---
.../bindings/iio/adc/nuvoton,ma35d1-eadc.yaml | 100 ++++++++++++++++++
1 file changed, 100 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
new file mode 100644
index 000000000000..ae7ad0f7689a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nuvoton,ma35d1-eadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 Enhanced Analog to Digital Converter
+
+maintainers:
+ - Chi-Wen Weng <cwweng@nuvoton.com>
+
+description: |
+ The Nuvoton MA35D1 Enhanced Analog to Digital Converter (EADC) is a
+ 12-bit ADC controller integrated in the MA35D1 SoC. Each enabled ADC
+ input is described by a child channel node.
+
+properties:
+ compatible:
+ const: nuvoton,ma35d1-eadc
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ '^channel@[0-8]$':
+ type: object
+ $ref: adc.yaml
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 8
+
+ diff-channels:
+ minItems: 2
+ maxItems: 2
+ items:
+ minimum: 0
+ maximum: 8
+
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - '#address-cells'
+ - '#size-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ adc@40430000 {
+ compatible = "nuvoton,ma35d1-eadc";
+ reg = <0x0 0x40430000 0x0 0x10000>;
+ interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk EADC_GATE>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ };
+
+ channel@2 {
+ reg = <2>;
+ diff-channels = <2 3>;
+ };
+ };
+ };
+...
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] iio: adc: Add Nuvoton MA35D1 EADC support
From: Chi-Wen Weng @ 2026-06-25 11:06 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-arm-kernel, linux-iio, devicetree,
linux-kernel, cwweng, cwweng.linux
From: Chi-Wen Weng <cwweng@nuvoton.com>
This series adds devicetree binding and IIO driver support for the
Nuvoton MA35D1 Enhanced ADC controller.
The MA35D1 EADC controller supports multiple ADC input channels. This
initial upstream driver supports direct raw reads and triggered buffered
capture using the controller end-of-conversion interrupt as the IIO
device trigger.
ADC channels are described using standard firmware child nodes. Both
single-ended and differential channels are supported. Since the
differential enable bit is global in the controller, mixed single-ended
and differential buffered scans are rejected.
DMA support is intentionally not included in this initial version. The
driver uses the interrupt-driven conversion path to keep the first
upstream submission small and easier to review.
Patch 1 adds the devicetree binding.
Patch 2 adds the MA35D1 EADC IIO driver.
Chi-Wen Weng (2):
dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
iio: adc: Add Nuvoton MA35D1 EADC driver
.../bindings/iio/adc/nuvoton,ma35d1-eadc.yaml | 100 +++
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ma35d1_eadc.c | 636 ++++++++++++++++++
4 files changed, 747 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
create mode 100644 drivers/iio/adc/ma35d1_eadc.c
--
2.25.1
^ permalink raw reply
* Re: [PATCH v19 2/7] coresight: tmc: add create/clean functions for etr_buf_list
From: sashiko-bot @ 2026-06-25 11:06 UTC (permalink / raw)
To: Jie Gan; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260625-enable-byte-cntr-for-ctcu-v19-2-8fbbf22e8381@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Lockless read of `drvdata->sysfs_buf` in `tmc_create_etr_buf_list` leads to a Time-of-Check to Time-of-Use race condition, resulting in a Double-Free or Use-After-Free.
- [Medium] The error path in `tmc_create_etr_buf_list` destructively frees the pre-existing `drvdata->sysfs_buf` if the device becomes inactive, leading to the irreversible loss of previously captured trace data.
- [Medium] `tmc_clean_etr_buf_list` leaks `drvdata->sysfs_buf` if called when the device is active in a non-SYSFS mode (e.g., `CS_MODE_PERF`).
--
commit efe04b42620438e6e268cf24ebf0538327ca8629
Author: Jie Gan <jie.gan@oss.qualcomm.com>
coresight: tmc: add create/clean functions for etr_buf_list
This commit introduces functions for creating and cleaning a list of
etr_buf_node elements to manage etr_buf entries. It is intended to support
byte-counter functionality which requires multiple trace data buffers.
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 361a433e6f0c5..9b3ef73e9cf27 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> @@ -1918,6 +1918,132 @@ const struct coresight_ops tmc_etr_cs_ops = {
> .panic_ops = &tmc_etr_sync_ops,
> };
>
> +/**
> + * tmc_clean_etr_buf_list - clean the etr_buf_list.
> + * @drvdata: driver data of the TMC device.
> + *
> + * Remove all nodes from @drvdata->etr_buf_list and free their buffers.
> + * If a node holds the live sysfs_buf and the device is active, the node is
> + * removed but the buffer is not freed; ownership stays with drvdata->sysfs_buf.
> + *
> + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list
> + * and must not hold @drvdata->spinlock. The spinlock is taken internally only
> + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink
> + * enable/disable paths. Must be called from process context: buffers are freed
> + * with the lock released.
> + */
> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
> +{
> + struct etr_buf_node *nd, *next;
> + unsigned long flags;
> +
> + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, link) {
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> + if (nd->sysfs_buf == drvdata->sysfs_buf) {
> + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DISABLED)
> + /*
> + * The device is still active. Keep the live
> + * buffer owned by drvdata->sysfs_buf and only
> + * drop the list's reference to it.
> + */
> + nd->sysfs_buf = NULL;
[Severity: Medium]
Will this leak drvdata->sysfs_buf if the device mode is active in a non-SYSFS
mode (such as CS_MODE_PERF)?
If the mode changes to CS_MODE_PERF concurrently during the
tmc_create_etr_buf_list() error path, this condition evaluates to true and
abandons the pointer by setting nd->sysfs_buf to NULL.
Since PERF mode tracing exclusively manages drvdata->perf_buf, does its
teardown path ever free drvdata->sysfs_buf?
> + else
> + /* Free the buffer below through nd->sysfs_buf */
> + drvdata->sysfs_buf = NULL;
[Severity: Medium]
If tmc_create_etr_buf_list() encounters an allocation failure and calls
tmc_clean_etr_buf_list() to roll back, and the device happens to be inactive,
will this unconditionally clear and free the pre-existing drvdata->sysfs_buf?
Should an initialization error path only clean up the resources it allocated,
rather than destroying a preserved external resource and losing previously
captured trace data?
> + }
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> + /* Free the buffer (NULL is ignored) and the node out of the lock */
> + tmc_etr_free_sysfs_buf(nd->sysfs_buf);
> + list_del(&nd->link);
> + kfree(nd);
> + }
> +}
> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
> +
> +/**
> + * tmc_create_etr_buf_list - create a list to manage the etr_buf_node.
> + * @drvdata: driver data of the TMC device.
> + * @num_nodes: number of nodes want to create with the list.
> + *
> + * Locking: callers must guarantee exclusive access to @drvdata->etr_buf_list
> + * and must not hold @drvdata->spinlock. The spinlock is taken internally only
> + * to serialise the @drvdata->sysfs_buf accesses against the ETR sink
> + * enable/disable paths. Must be called from process context: buffers and nodes
> + * are allocated with the lock released.
> + *
> + * Return 0 upon success and return the error number if fail.
> + */
> +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes)
> +{
> + struct etr_buf_node *new_node;
> + struct etr_buf *sysfs_buf;
> + unsigned long flags;
> + int i = 0, ret = 0;
> +
> + /* We don't need a list if there is only one node */
> + if (num_nodes < 2)
> + return -EINVAL;
> +
> + /*
> + * We expect that sysfs_buf in drvdata has already been allocated.
> + * Wrap the live sysfs_buf into the first node so the captured trace
> + * data is preserved. The list is owned by the caller, so no lock is
> + * needed to read sysfs_buf or to add the node here.
> + */
> + if (drvdata->sysfs_buf) {
> + new_node = kzalloc_obj(*new_node, GFP_KERNEL);
> + if (!new_node)
> + return -ENOMEM;
> +
> + new_node->sysfs_buf = drvdata->sysfs_buf;
[Severity: High]
Does this lockless read of drvdata->sysfs_buf introduce a race condition?
If a concurrent hardware disable/enable path (like tmc_etr_get_sysfs_buffer())
clears and frees drvdata->sysfs_buf immediately after this lockless check,
wouldn't new_node->sysfs_buf be left as a dangling pointer, leading to a
use-after-free or double free?
> + new_node->is_free = false;
> + list_add(&new_node->link, &drvdata->etr_buf_list);
> + i++;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [RFC PATCH 1/3] dt-bindings: pinctrl: mt8516/mt8167: Move compatibles from mt66xx to mt6795
From: sashiko-bot @ 2026-06-25 11:04 UTC (permalink / raw)
To: Luca Leonardo Scorcia; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260625104742.113803-2-l.scorcia@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch series permanently breaks Device Tree backward compatibility (ABI breakage). By moving the mt8167 and mt8516 SoCs to the mt6795 bindings and pinctrl-paris.c driver, the schema now unconditionally requires `reg-names` and `gpio-ranges` and forbids the legacy `mediatek,pctl-regmap` layout. Furthermore, the updated driver calls `devm_platform_ioremap_resource_byname(pdev, "base")` and fails with `-EINVAL` if `reg-names` is absent. Booting a new kernel with an older, previously valid device tree will fail to probe the pinctrl driver, breaking all GPIO-dependent devices.
--
commit ae4fde26bd3f120a43beccf98009171f05144eee
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
dt-bindings: pinctrl: mt8516/mt8167: Move compatibles from mt66xx to mt6795
Pinctrl settings for MediaTek mt8516-mt8167 SoCs use two reg base
addresses, one for GPIO and the other for EINT, as it is common in the
"Paris" pinctrl platform. This commit moves the mediatek,mt8516-pinctrl and
mediatek,mt8167-pinctrl compatible strings to the mt6795-pinctrl binding.
[ ... ]
> diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> index 9a937f414cc98..c703de72e1d52 100644
> --- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt6795-pinctrl.yaml
> @@ -15,7 +15,10 @@ description:
>
> properties:
> compatible:
> - const: mediatek,mt6795-pinctrl
> + enum:
> + - mediatek,mt6795-pinctrl
> + - mediatek,mt8167-pinctrl
> + - mediatek,mt8516-pinctrl
[Severity: Critical]
Does moving these compatibles to this binding break Device Tree backward
compatibility?
The schema now unconditionally requires properties like reg-names and
gpio-ranges, and forbids the legacy mediatek,pctl-regmap layout without an
allOf: if-then conditional to accommodate older hardware descriptions.
Furthermore, the updated driver unconditionally requires the reg-names
property:
drivers/pinctrl/mediatek/pinctrl-paris.c:mtk_paris_pinctrl_probe() {
...
hw->base[i] = devm_platform_ioremap_resource_byname(pdev,
hw->soc->base_names[i]);
...
}
Because older device trees lack the reg-names property, this call returns
-EINVAL and fails the probe.
Will booting a new kernel with an older device tree fail to probe the
pinctrl driver, breaking all GPIO-dependent peripherals?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625104742.113803-1-l.scorcia@gmail.com?part=1
^ 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