Re: [PATCH v2 2/2] pinctrl: qcom: Add MSM8952 tlmm pinctrl driver
From: Konrad Dybcio
Date: Wed Sep 02 2026 - 09:18:25 EST
On 20-Aug-26 13:17, Muzaffer Kadir via B4 Relay wrote:
> From: Muzaffer Kadir <muzafferkadir@xxxxxxxxxxxxxx>
>
> MSM8952 has same pin count and similar pinctrl
> functions mostly with msm8917, but some functions use different pins.
>
> Signed-off-by: Muzaffer Kadir <muzafferkadir@xxxxxxxxxxxxxx>
> ---
The following is a review from my AI agent who inspected the
hw specification and prior art/review feedback regarding qcom
pinctrl. I read it and approve of it:
Hi Muzaffer,
The driver currently contains function/group declarations which are not connected to any PINGROUP() entry.
For each item below, add the function to the corresponding GPIO’s PINGROUP() definition:
Function GPIO
━━━━━━━━━━━━━━━━━━━━━━ ━━━━━━
mipi_dsi0 0
────────────────────── ──────
smb_int 1
────────────────────── ──────
qdss_cti_trig_out_b0 2
────────────────────── ──────
rcm_marker2 3
────────────────────── ──────
qdss_cti_trig_out_a1 3
────────────────────── ──────
codec_mad 13
────────────────────── ──────
nfc_disable 16
────────────────────── ──────
nfc_irq 17
────────────────────── ──────
flash_strobe 33
────────────────────── ──────
cam1_standby 35
────────────────────── ──────
cam1_rst 36
────────────────────── ──────
cam2_standby 37
────────────────────── ──────
cam2_rst 38
────────────────────── ──────
webcam_standby 39
────────────────────── ──────
webcam_rst 40
────────────────────── ──────
rcm_marker1 41
────────────────────── ──────
accel_int 42
────────────────────── ──────
alsp_int 43
────────────────────── ──────
mag_int 44
────────────────────── ──────
gyro_int 45
────────────────────── ──────
pressure_int 46
────────────────────── ──────
fp_gpio 47
────────────────────── ──────
fp_int 48
────────────────────── ──────
us_euro 63
────────────────────── ──────
ts_resout 64
────────────────────── ──────
ts_sample 65
────────────────────── ──────
sdcard_det 67
────────────────────── ──────
audio_reset 68
────────────────────── ──────
codec_int1 73
────────────────────── ──────
codec_int2 74
────────────────────── ──────
key_volp 85
────────────────────── ──────
key_snapshot 86
────────────────────── ──────
key_focus 87
────────────────────── ──────
key_home 88
────────────────────── ──────
backlight_en 91
────────────────────── ──────
lcd_en 92
────────────────────── ──────
usbc_irq 93
────────────────────── ──────
uim3_ldo 96
────────────────────── ──────
wsa_irq 97
For example:
> static const char *const mipi_dsi0_groups[] = { "gpio0" };
must have a matching mipi_dsi0 entry in the GPIO0 PINGROUP() definition. The same consistency is required for every item in the table. The gpio function is intentionally excluded because it is inserted
implicitly by the PINGROUP() macro.
There are also functions missing from the GPIO table. Please add these to the corresponding groups. The final column is the function index from the MSM8952 GPIO mapping:
Function GPIO Function index
━━━━━━━━━━━ ━━━━━━ ━━━━━━━━━━━━━━━━
gp_pdm_2a 7 4
─────────── ────── ────────────────
gp_pdm_1b 20 3
─────────── ────── ────────────────
gp_pdm_2b 38 1
─────────── ────── ────────────────
gp_mn 39 2
─────────── ────── ────────────────
gp_pdm_1a 45 1
─────────── ────── ────────────────
gp0_clk 46 1
─────────── ────── ────────────────
gp1_clk 47 2
─────────── ────── ────────────────
gp_pdm_0b 48 2
─────────── ────── ────────────────
gp_pdm_0a 66 3
Finally, please move:
> MODULE_DEVICE_TABLE(of, msm8952_pinctrl_of_match);
immediately after the OF match table, consistent with the other Qualcomm pinctrl drivers.
Best regards,
Konrad