.../bindings/pinctrl/qcom,nord-tlmm.yaml | 5 +-- drivers/pinctrl/qcom/pinctrl-nord.c | 34 +++++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-)
QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two
pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART
TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130
respectively.
Both mux values were named "qup1_se2" (respectively "qup1_se3"), so the
two distinct lane pairs became indistinguishable. msm_pinmux_set_mux()
stops at the first entry matching the requested function, which means
mux value 1 was always selected and the I2C lanes could never be muxed
out. In practice i2c9 and i2c10 got the UART lanes and did not work,
while uart9 and uart10 happened to be muxed correctly.
Give each lane pair its own function, following the _01/_23 naming
already used for the same hardware arrangement by the shikra, eliza,
hawi and maili TLMM drivers. Both functions still cover the full pin
pair, so a single pinctrl state per protocol remains sufficient.
Drop gpio129/gpio130 from the SE2 group list, since those pins
belong to SE3 and were never reachable through the SE2 function.
Also rename QUP1 SE2/SE3 functions in the binding doc accordingly.
While at it, add missing QUP3 SE0 functions qup3_se0_mira and
qup3_se0_mirb to the binding doc.
Fixes: c24dd0826f06 ("pinctrl: qcom: add the TLMM driver for the Nord platforms")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
.../bindings/pinctrl/qcom,nord-tlmm.yaml | 5 +--
drivers/pinctrl/qcom/pinctrl-nord.c | 34 +++++++++++++------
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,nord-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,nord-tlmm.yaml
index 4bb511719f31..6b5ec9f49971 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,nord-tlmm.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,nord-tlmm.yaml
@@ -98,9 +98,10 @@ $defs:
pcie3_clk_req_n, phase_flag, pll_bist_sync, pll_clk_aux,
prng_rosc0, prng_rosc1, pwrbrk_i_n, qdss, qdss_cti, qspi,
qup0_se0, qup0_se1, qup0_se2, qup0_se3, qup0_se4, qup0_se5,
- qup1_se0, qup1_se1, qup1_se3, qup1_se2, qup1_se4, qup1_se5,
+ qup1_se0, qup1_se1, qup1_se2_01, qup1_se2_23, qup1_se3_01,
+ qup1_se3_23, qup1_se4, qup1_se5,
qup1_se6, qup2_se0, qup2_se1, qup2_se2, qup2_se3, qup2_se4,
- qup2_se5, qup2_se6,
+ qup2_se5, qup2_se6, qup3_se0_mira, qup3_se0_mirb,
sailss_ospi, sdc4_clk, sdc4_cmd, sdc4_data, smb_alert,
smb_alert_n, smb_clk, smb_dat, tb_trig_sdc4, tmess_prng0,
tmess_prng1, tsc_timer, tsense_pwm, usb0_hs,
diff --git a/drivers/pinctrl/qcom/pinctrl-nord.c b/drivers/pinctrl/qcom/pinctrl-nord.c
index 7c21306e77ff..7f37f8e819ba 100644
--- a/drivers/pinctrl/qcom/pinctrl-nord.c
+++ b/drivers/pinctrl/qcom/pinctrl-nord.c
@@ -570,8 +570,10 @@ enum nord_functions {
msm_mux_qup0_se5,
msm_mux_qup1_se0,
msm_mux_qup1_se1,
- msm_mux_qup1_se2,
- msm_mux_qup1_se3,
+ msm_mux_qup1_se2_01,
+ msm_mux_qup1_se2_23,
+ msm_mux_qup1_se3_01,
+ msm_mux_qup1_se3_23,
msm_mux_qup1_se4,
msm_mux_qup1_se5,
msm_mux_qup1_se6,
@@ -1152,11 +1154,19 @@ static const char *const qup1_se1_groups[] = {
"gpio123", "gpio124", "gpio125", "gpio126",
};
-static const char *const qup1_se2_groups[] = {
- "gpio127", "gpio128", "gpio129", "gpio130",
+static const char *const qup1_se2_01_groups[] = {
+ "gpio127", "gpio128",
};
-static const char *const qup1_se3_groups[] = {
+static const char *const qup1_se2_23_groups[] = {
+ "gpio127", "gpio128",
+};
+
+static const char *const qup1_se3_01_groups[] = {
+ "gpio129", "gpio130",
+};
+
+static const char *const qup1_se3_23_groups[] = {
"gpio129", "gpio130",
};
@@ -1428,8 +1438,10 @@ static const struct pinfunction nord_functions[] = {
MSM_PIN_FUNCTION(qup0_se5),
MSM_PIN_FUNCTION(qup1_se0),
MSM_PIN_FUNCTION(qup1_se1),
- MSM_PIN_FUNCTION(qup1_se2),
- MSM_PIN_FUNCTION(qup1_se3),
+ MSM_PIN_FUNCTION(qup1_se2_01),
+ MSM_PIN_FUNCTION(qup1_se2_23),
+ MSM_PIN_FUNCTION(qup1_se3_01),
+ MSM_PIN_FUNCTION(qup1_se3_23),
MSM_PIN_FUNCTION(qup1_se4),
MSM_PIN_FUNCTION(qup1_se5),
MSM_PIN_FUNCTION(qup1_se6),
@@ -1633,13 +1645,13 @@ static const struct msm_pingroup nord_groups[] = {
_, _, _, _, _, _, _),
[126] = PINGROUP(126, qup1_se1, qup1_se0, ccu_i2c_scl, mdp1_vsync_out,
_, atest_usb20, ddr_pxi, _, _, _, _),
- [127] = PINGROUP(127, qup1_se2, qup1_se2, _, atest_usb21, ddr_pxi,
+ [127] = PINGROUP(127, qup1_se2_23, qup1_se2_01, _, atest_usb21, ddr_pxi,
_, _, _, _, _, _),
- [128] = PINGROUP(128, qup1_se2, qup1_se2, _, atest_usb20, ddr_pxi,
+ [128] = PINGROUP(128, qup1_se2_23, qup1_se2_01, _, atest_usb20, ddr_pxi,
_, _, _, _, _, _),
- [129] = PINGROUP(129, qup1_se3, qup1_se3, ccu_i2c_sda, mdp1_vsync_out,
+ [129] = PINGROUP(129, qup1_se3_23, qup1_se3_01, ccu_i2c_sda, mdp1_vsync_out,
_, atest_usb21, ddr_pxi, _, _, _, _),
- [130] = PINGROUP(130, qup1_se3, qup1_se3, ccu_i2c_scl, mdp1_vsync_out,
+ [130] = PINGROUP(130, qup1_se3_23, qup1_se3_01, ccu_i2c_scl, mdp1_vsync_out,
_, atest_usb20, ddr_pxi, _, _, _, _),
[131] = PINGROUP(131, qup1_se4, qup1_se6, ccu_i2c_sda, mdp1_vsync_out,
_, atest_usb21, ddr_pxi, _, _, _, _),
--
2.43.0On Wed, Aug 19, 2026 at 11:23:56PM +0800, Shawn Guo wrote:
> QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two
> pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART
> TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130
> respectively.
>
> Both mux values were named "qup1_se2" (respectively "qup1_se3"), so the
> two distinct lane pairs became indistinguishable. msm_pinmux_set_mux()
> stops at the first entry matching the requested function, which means
> mux value 1 was always selected and the I2C lanes could never be muxed
> out. In practice i2c9 and i2c10 got the UART lanes and did not work,
> while uart9 and uart10 happened to be muxed correctly.
>
> Give each lane pair its own function, following the _01/_23 naming
> already used for the same hardware arrangement by the shikra, eliza,
> hawi and maili TLMM drivers. Both functions still cover the full pin
> pair, so a single pinctrl state per protocol remains sufficient.
>
> Drop gpio129/gpio130 from the SE2 group list, since those pins
> belong to SE3 and were never reachable through the SE2 function.
>
> Also rename QUP1 SE2/SE3 functions in the binding doc accordingly.
>
> While at it, add missing QUP3 SE0 functions qup3_se0_mira and
> qup3_se0_mirb to the binding doc.
Just spot that "gpio" is also missing from function enum list in
the binding. Will send v2.
Shawn
>
> Fixes: c24dd0826f06 ("pinctrl: qcom: add the TLMM driver for the Nord platforms")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
On 8/19/26 5:23 PM, Shawn Guo wrote: > QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two > pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART > TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130 > respectively. [...] > + msm_mux_qup1_se2_01, other drivers where this is necessary stick an 'l' before the lane indices, e.g. se2_l2, let's keep this The bindings change must be separate Konrad
On Wed, Aug 19, 2026 at 05:27:34PM +0200, Konrad Dybcio wrote: > On 8/19/26 5:23 PM, Shawn Guo wrote: > > QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two > > pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART > > TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130 > > respectively. > > [...] > > > + msm_mux_qup1_se2_01, > > other drivers where this is necessary stick an 'l' before the > lane indices, e.g. se2_l2, let's keep this I can see that for single lane function. But for lane-pair function, it's not a new thing with nord, and the naming convention seems to be without 'l', based on what I see from shikra, eliza, hawi and maili drivers. > > The bindings change must be separate I understand that's mandatory for new feature changes, but for such a bug fix where both binding and driver changes are closely coupled, it seems more reasonable to not split. Shawn
On 8/20/26 3:15 AM, Shawn Guo wrote: > On Wed, Aug 19, 2026 at 05:27:34PM +0200, Konrad Dybcio wrote: >> On 8/19/26 5:23 PM, Shawn Guo wrote: >>> QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two >>> pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART >>> TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130 >>> respectively. >> >> [...] >> >>> + msm_mux_qup1_se2_01, >> >> other drivers where this is necessary stick an 'l' before the >> lane indices, e.g. se2_l2, let's keep this > > I can see that for single lane function. But for lane-pair function, > it's not a new thing with nord, and the naming convention seems to be > without 'l', based on what I see from shikra, eliza, hawi and maili > drivers. Hm, seems like that happened by oversight, but yeah let's stick to it now. >> The bindings change must be separate > > I understand that's mandatory for new feature changes, but for such > a bug fix where both binding and driver changes are closely coupled, > it seems more reasonable to not split. Up to brgl Konrad
On Mon, Aug 24, 2026 at 10:44 AM Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote: > > On 8/20/26 3:15 AM, Shawn Guo wrote: > > On Wed, Aug 19, 2026 at 05:27:34PM +0200, Konrad Dybcio wrote: > >> On 8/19/26 5:23 PM, Shawn Guo wrote: > >>> QUP1 SE2 and SE3 pack all four of their lanes pair-wise onto only two > >>> pins each: lanes 0/1 (I2C SDA/SCL) at mux value 2 and lanes 2/3 (UART > >>> TX/RX) at mux value 1, on gpio127/gpio128 and gpio129/gpio130 > >>> respectively. > >> > >> [...] > >> > >>> + msm_mux_qup1_se2_01, > >> > >> other drivers where this is necessary stick an 'l' before the > >> lane indices, e.g. se2_l2, let's keep this > > > > I can see that for single lane function. But for lane-pair function, > > it's not a new thing with nord, and the naming convention seems to be > > without 'l', based on what I see from shikra, eliza, hawi and maili > > drivers. > > Hm, seems like that happened by oversight, but yeah let's stick to it > now. > > >> The bindings change must be separate > > > > I understand that's mandatory for new feature changes, but for such > > a bug fix where both binding and driver changes are closely coupled, > > it seems more reasonable to not split. > > Up to brgl > I'm fine with this. Krzyszof, Rob, Conor: any objections? Thanks, Bartosz
© 2016 - 2026 Red Hat, Inc.