From: Pengyu Luo <mitltlatltl@gmail.com> Date: 2026-06-07 16:06:16
Now that the predefined MCLK pinctrl configuration is available in the
sc8280xp SoC DTSI, switch the ThinkPad X13s to use it. Drop the local,
duplicate MCLK pin definition from the camera pinctrl node.
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
.../boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Silent reduction of camera MCLK drive strength from 16 mA to 6 mA.
--
commit 68374459d8e6941a056f52b1b8fa71811fd7137a
Author: Pengyu Luo [off-list ref]
arm64: dts: qcom: sc8280xp-x13s: Use predefined MCLK pinctrl
This commit switches the ThinkPad X13s to use the predefined MCLK pinctrl
configuration from the sc8280xp SoC DTSI. It drops the local MCLK pin
definition from the camera pinctrl node.
[Severity: High]
Does this unintentionally reduce the camera MCLK drive strength?
The commit message notes this is dropping a duplicate MCLK pin definition, but
the generic cam_mclk3_default definition in sc8280xp.dtsi now used by the
camera node defaults to a lower drive strength of 6 mA:
arch/arm64/boot/dts/qcom/sc8280xp.dtsi:
cam_mclk3_default: cam-mclk3-default-state {
pins = "gpio17";
function = "cam_mclk";
drive-strength = <6>;
bias-disable;
};
Reducing the drive capability from 16 mA to 6 mA for a high-frequency master
clock signal routed over long traces might degrade signal integrity.
Could this lead to intermittent or complete camera initialization failures on
this hardware?
sc-rgb-xshut-n-pins {
pins = "gpio15";
function = "gpio";
From: Konrad Dybcio <hidden> Date: 2026-06-09 12:18:29
On 6/7/26 6:04 PM, Pengyu Luo wrote:
quoted hunk
Now that the predefined MCLK pinctrl configuration is available in the
sc8280xp SoC DTSI, switch the ThinkPad X13s to use it. Drop the local,
duplicate MCLK pin definition from the camera pinctrl node.
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
.../boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
Other platforms set this to 2 by default.
What's the value set on Windows when the camera is in use?
It is 6mA.
Let us get ctl_reg first on Windows
lkd> !dd f111000 L8
# f111000 00000284 00000002 000000e2 00000000
# f111010 00000001 00000801 00000000 00000000
ctl_reg => 0x284
in msm_gpio_dbg_show_one()
...
drive = (ctl_reg >> g->drv_bit) & 7; // (0x284 >> 6) & 7 == 2
...
seq_printf(s, " %dmA", msm_regval_to_drive(drive)); // (drive + 1) * 2 == 6;
...
x13s should be the same as gaokun3 in this part.
--
Best wishes,
Pengyu
Other platforms set this to 2 by default.
What's the value set on Windows when the camera is in use?
It is 6mA.
Let us get ctl_reg first on Windows
lkd> !dd f111000 L8
# f111000 00000284 00000002 000000e2 00000000
# f111010 00000001 00000801 00000000 00000000
ctl_reg => 0x284
in msm_gpio_dbg_show_one()
...
drive = (ctl_reg >> g->drv_bit) & 7; // (0x284 >> 6) & 7 == 2
...
seq_printf(s, " %dmA", msm_regval_to_drive(drive)); // (drive + 1) * 2 == 6;
...
x13s should be the same as gaokun3 in this part.
I confirmed as much and I'm willing to believe this is a default for
all 8280 devices
Reviewed-by: Konrad Dybcio <redacted>
for the second patch, please mention in the commit message that the value
will now match windows and please add a fixes tag
Konrad
Other platforms set this to 2 by default.
What's the value set on Windows when the camera is in use?
It is 6mA.
Let us get ctl_reg first on Windows
lkd> !dd f111000 L8
# f111000 00000284 00000002 000000e2 00000000
# f111010 00000001 00000801 00000000 00000000
ctl_reg => 0x284
in msm_gpio_dbg_show_one()
...
drive = (ctl_reg >> g->drv_bit) & 7; // (0x284 >> 6) & 7 == 2
...
seq_printf(s, " %dmA", msm_regval_to_drive(drive)); // (drive + 1) * 2 == 6;
...
x13s should be the same as gaokun3 in this part.
I confirmed as much and I'm willing to believe this is a default for
all 8280 devices
Reviewed-by: Konrad Dybcio <redacted>
for the second patch, please mention in the commit message that the value
will now match windows and please add a fixes tag
I believe the second change cannot be tagged as Fixes in sense that it
strictly depends on a not going to be backported non-fix commit, and thus
backporting of just 2/2 change as is will break the matter. Reordering of
the commits placing the fix commit as the first one should be fine though.
--
Best wishes,
Vladimir
Other platforms set this to 2 by default.
What's the value set on Windows when the camera is in use?
It is 6mA.
Let us get ctl_reg first on Windows
lkd> !dd f111000 L8
# f111000 00000284 00000002 000000e2 00000000
# f111010 00000001 00000801 00000000 00000000
ctl_reg => 0x284
in msm_gpio_dbg_show_one()
...
drive = (ctl_reg >> g->drv_bit) & 7; // (0x284 >> 6) & 7 == 2
...
seq_printf(s, " %dmA", msm_regval_to_drive(drive)); // (drive + 1) * 2 == 6;
...
x13s should be the same as gaokun3 in this part.
I confirmed as much and I'm willing to believe this is a default for
all 8280 devices
Reviewed-by: Konrad Dybcio <redacted>
for the second patch, please mention in the commit message that the value
will now match windows and please add a fixes tag
I believe the second change cannot be tagged as Fixes in sense that it
strictly depends on a not going to be backported non-fix commit, and thus
backporting of just 2/2 change as is will break the matter. Reordering of
the commits placing the fix commit as the first one should be fine though.
The Fixes tag makes the patch eligible for backporting through AUTOSEL
but is itself not the same as "please backport"
Konrad
Other platforms set this to 2 by default.
What's the value set on Windows when the camera is in use?
It is 6mA.
Let us get ctl_reg first on Windows
lkd> !dd f111000 L8
# f111000 00000284 00000002 000000e2 00000000
# f111010 00000001 00000801 00000000 00000000
ctl_reg => 0x284
in msm_gpio_dbg_show_one()
...
drive = (ctl_reg >> g->drv_bit) & 7; // (0x284 >> 6) & 7 == 2
...
seq_printf(s, " %dmA", msm_regval_to_drive(drive)); // (drive + 1) * 2 == 6;
...
x13s should be the same as gaokun3 in this part.
I confirmed as much and I'm willing to believe this is a default for
all 8280 devices
Reviewed-by: Konrad Dybcio <redacted>
for the second patch, please mention in the commit message that the value
will now match windows and please add a fixes tag
I believe the second change cannot be tagged as Fixes in sense that it
strictly depends on a not going to be backported non-fix commit, and thus
backporting of just 2/2 change as is will break the matter. Reordering of
the commits placing the fix commit as the first one should be fine though.
The Fixes tag makes the patch eligible for backporting through AUTOSEL
but is itself not the same as "please backport"
That's correct, and due Documentation/process/stable-kernel-rules.rst it
would make sense to add Cc: [off-list ref] to the next
version of the change to help stable tree maintainers, since it is known
in advance that the unmodified and Fixes tagged 2/2 change shall not be
considered as a candidate change to the stable tree. Or is it excessive?
IMHO here it might be better to properly arrange the changes and backport
the fix.
--
Best wishes,
Vladimir