Re: [PATCH v4 3/3] pinctrl: mediatek: Add pinctrl driver on mt8196

["Cathy Xu (许华婷)" <ot_cathy.xu@mediatek.com>]

On Fri, 2025-02-28 at 09:49 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 28/02/25 02:16, Cathy Xu ha scritto:
> > From: Guodong Liu <guodong.liu@mediatek.com>
> > 
> > Add pinctrl driver support for MediaTek Soc mt8196.
> > 
> > Signed-off-by: Guodong Liu <guodong.liu@mediatek.com>
> > Signed-off-by: Cathy Xu <ot_cathy.xu@mediatek.com>
> > ---
> >   drivers/pinctrl/mediatek/Kconfig              |   12 +
> >   drivers/pinctrl/mediatek/Makefile             |    1 +
> >   drivers/pinctrl/mediatek/pinctrl-mt8196.c     | 1857 +++++++++++
> >   drivers/pinctrl/mediatek/pinctrl-mtk-mt8196.h | 2789
> > +++++++++++++++++
> >   4 files changed, 4659 insertions(+)
> >   create mode 100644 drivers/pinctrl/mediatek/pinctrl-mt8196.c
> >   create mode 100644 drivers/pinctrl/mediatek/pinctrl-mtk-mt8196.h
> > 
> > diff --git a/drivers/pinctrl/mediatek/Kconfig
> > b/drivers/pinctrl/mediatek/Kconfig
> > index 58f32818a0e6..b59d94a0fe3b 100644
> > --- a/drivers/pinctrl/mediatek/Kconfig
> > +++ b/drivers/pinctrl/mediatek/Kconfig
> > @@ -263,6 +263,18 @@ config PINCTRL_MT8195
> >       default ARM64 && ARCH_MEDIATEK
> >       select PINCTRL_MTK_PARIS
> > 
> > +config PINCTRL_MT8196
> > +     bool "MediaTek MT8196 pin control"
> > +     depends on OF
> > +     depends on ARM64 || COMPILE_TEST
> > +     default ARM64 && ARCH_MEDIATEK
> > +     select PINCTRL_MTK_PARIS
> > +     help
> > +       Say yes here to support pin controller and gpio driver
> > +       on MediaTek MT8196 SoC.
> > +       In MTK platform, we support virtual gpio and use it to
> > +       map specific eint which doesn't have real gpio pin.
> > +
> >   config PINCTRL_MT8365
> >       bool "MediaTek MT8365 pin control"
> >       depends on OF
> > diff --git a/drivers/pinctrl/mediatek/Makefile
> > b/drivers/pinctrl/mediatek/Makefile
> > index 721ae83476d0..08e7239fcd8b 100644
> > --- a/drivers/pinctrl/mediatek/Makefile
> > +++ b/drivers/pinctrl/mediatek/Makefile
> > @@ -36,6 +36,7 @@ obj-$(CONFIG_PINCTRL_MT8186)                +=
> > pinctrl-mt8186.o
> >   obj-$(CONFIG_PINCTRL_MT8188)                += pinctrl-mt8188.o
> >   obj-$(CONFIG_PINCTRL_MT8192)                += pinctrl-mt8192.o
> >   obj-$(CONFIG_PINCTRL_MT8195)                += pinctrl-mt8195.o
> > +obj-$(CONFIG_PINCTRL_MT8196)         += pinctrl-mt8196.o
> >   obj-$(CONFIG_PINCTRL_MT8365)                += pinctrl-mt8365.o
> >   obj-$(CONFIG_PINCTRL_MT8516)                += pinctrl-mt8516.o
> >   obj-$(CONFIG_PINCTRL_MT6397)                += pinctrl-mt6397.o
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8196.c
> > b/drivers/pinctrl/mediatek/pinctrl-mt8196.c
> > new file mode 100644
> > index 000000000000..1baefdc7e36d
> > --- /dev/null
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8196.c
> > @@ -0,0 +1,1857 @@
> 
> ..snip..
> 
> > +
> > +static const struct mtk_pin_soc mt8196_data = {
> > +     .reg_cal = mt8196_reg_cals,
> > +     .pins = mtk_pins_mt8196,
> > +     .npins = ARRAY_SIZE(mtk_pins_mt8196),
> > +     .ngrps = ARRAY_SIZE(mtk_pins_mt8196),
> > +     .eint_hw = &mt8196_eint_hw,
> > +     .nfuncs = 8,
> > +     .gpio_m = 0,
> > +     .base_names = mt8196_pinctrl_register_base_names,
> > +     .nbase_names =
> > ARRAY_SIZE(mt8196_pinctrl_register_base_names),
> > +     .pull_type = mt8196_pull_type,
> > +     .pin_rsel = mt8196_pin_rsel_val_range,
> > +     .npin_rsel = ARRAY_SIZE(mt8196_pin_rsel_val_range),
> > +     .bias_set_combo = mtk_pinconf_bias_set_combo,
> > +     .bias_get_combo = mtk_pinconf_bias_get_combo,
> > +     .drive_set = mtk_pinconf_drive_set_rev1,
> > +     .drive_get = mtk_pinconf_drive_get_rev1,
> > +     .adv_drive_get = mtk_pinconf_adv_drive_get_raw,
> > +     .adv_drive_set = mtk_pinconf_adv_drive_set_raw,
> 
> In the cover letter, you said:
> 
> "Add values in SI units option to |struct mtk_pin_soc| in pinctrl-
> mt8196.c"
> 
> ....I don't see that here though?

  Thank you for your review.
  I add |.pin_rsel = mt8196_pin_rsel_val_range| to support SI units, 
you can see |mt8196_pin_rsel_val_range| array.

> 
> > +};
> > +
> > +static const struct of_device_id mt8196_pinctrl_of_match[] = {
> > +     { .compatible = "mediatek,mt8196-pinctrl", .data =
> > &mt8196_data },
> > +     { }
> 
> { /* sentinel */ }
> 
> > +};
> > +
> > +static struct platform_driver mt8196_pinctrl_driver = {
> > +     .driver = {
> > +             .name = "mt8196-pinctrl",
> > +             .of_match_table = mt8196_pinctrl_of_match,
> > +             .pm = pm_sleep_ptr(&mtk_paris_pinctrl_pm_ops),
> > +     },
> > +     .probe = mtk_paris_pinctrl_probe,
> > +};
> > +
> > +static int __init mt8196_pinctrl_init(void)
> > +{
> > +     return platform_driver_register(&mt8196_pinctrl_driver);
> > +}
> > +arch_initcall(mt8196_pinctrl_init);
> > +
> > +MODULE_DESCRIPTION("MediaTek MT8196 Pinctrl Driver");
> 
> MODULE_LICENSE is missing!
> 
> Cheers,
> Angelo
> 
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-mt8196.h
> > b/drivers/pinctrl/mediatek/pinctrl-mtk-mt8196.h
> > new file mode 100644
> > index 000000000000..d342d091672c