Re: [RFC PATCH 2/3] pinctrl: mediatek: mt8516/mt8167: Migrate driver to pinctrl-paris platform

[sashiko-bot@kernel.org]

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