| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <00529b74cd1a98984a793fbc37fb6e9ac984afcb.camel@mediatek.com>
Date: Mon, 13 Jan 2025 09:32:45 +0000
From: Cathy Xu (许华婷) <ot_cathy.xu@...iatek.com>
To: "matthias.bgg@...il.com" <matthias.bgg@...il.com>, "sean.wang@...nel.org"
<sean.wang@...nel.org>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, "linus.walleij@...aro.org"
<linus.walleij@...aro.org>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"guodong.liu@...iatek.corp-partner.google.com"
<guodong.liu@...iatek.corp-partner.google.com>, "linux-gpio@...r.kernel.org"
<linux-gpio@...r.kernel.org>, Guodong Liu (刘国栋)
<Guodong.Liu@...iatek.com>
Subject: Re: [PATCH] pinctrl: mediatek: Add pinctrl driver
On Mon, 2024-11-11 at 14:39 +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 11/11/24 08:40, ot907280 ha scritto:
> > From: Guodong Liu <guodong.liu@...iatek.corp-partner.google.com>
> >
> > Add pinctrl driver for mt8196
> >
>
> Please fix the commit title, add a meaningful description ... and
> also please fix
> your name, as your email is sent by "ot907280" and not from "Cathy
> Xu".
Thank you for your review.It will be fixed in next version.
>
> > Signed-off-by: Guodong Liu <guodong.liu@...iatek.com>
> > Cathy Xu <ot_cathy.xu@...iatek.com>
>
> You're missing the "Signed-off-by: " part before your name.
It will be fixed in next version.
>
> > ---
> > drivers/pinctrl/mediatek/Kconfig | 12 +
> > drivers/pinctrl/mediatek/Makefile | 1 +
> > drivers/pinctrl/mediatek/pinctrl-mt8196.c | 1757 +++++++++++
> > drivers/pinctrl/mediatek/pinctrl-mtk-mt8196.h | 2791
> > +++++++++++++++++
> > 4 files changed, 4561 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 a417a031659c..149a78e4216e 100644
> > --- a/drivers/pinctrl/mediatek/Kconfig
> > +++ b/drivers/pinctrl/mediatek/Kconfig
> > @@ -256,6 +256,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 1405d434218e..b4a39c1bafb7 100644
> > --- a/drivers/pinctrl/mediatek/Makefile
> > +++ b/drivers/pinctrl/mediatek/Makefile
> > @@ -35,6 +35,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..6d2bee706718
> > --- /dev/null
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8196.c
> > @@ -0,0 +1,1757 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2024 Mediatek Inc.
> > + * Author: Guodong Liu <Guodong.Liu@...iatek.com>
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include "pinctrl-mtk-mt8196.h"
> > +#include "pinctrl-paris.h"
> > +
> > +#define PIN_FIELD_BASE(s_pin, e_pin, i_base, s_addr, x_addrs,
> > s_bit, x_bits) \
>
> It doesn't look like there's any s_pin with a different number from
> e_pin - unless
> I am misreading something, you can change `s_pin` to be named
> `se_pin`, so that we
> stop declaring the number twice; makes it a little more readable.
The definitions of the macros 'PIN_FIELD_BASE' and 'PIN_FIELD' both
come from 'PIN_FIELD_CALC', where the s_pin and e_pin in 'PIN_FIELD'
are different. This way, the code only needs one line to represent
the addresses of all pins.
PIN_FIELD define:
#define PIN_FIELD(_s_pin, _e_pin, _s_addr, _x_addrs, _s_bit,
_x_bits) \
PIN_FIELD_CALC(_s_pin, _e_pin, 0, _s_addr, _x_addrs,
_s_bit, _x_bits, 32, 0)
PIN_FIELD use:
static const struct mtk_pin_field_calc mt8196_pin_mode_range[] = {
PIN_FIELD(0, 270, 0x0300, 0x10, 0, 4),
};
>
> > + PIN_FIELD_CALC(s_pin, e_pin, i_base, s_addr, x_addrs, s_bit,
> > x_bits, \
> > + 32, 0)
> > +
> > +#define PINS_FIELD_BASE(s_pin, e_pin, i_base, s_addr, x_addrs,
> > s_bit, x_bits) \
>
> Same here.
Same as above.
>
> > + PIN_FIELD_CALC(s_pin, e_pin, i_base, s_addr, x_addrs, s_bit,
> > x_bits, \
> > + 32, 1)
> > +
> > +static const struct mtk_pin_field_calc mt8196_pin_mode_range[] = {
> > + PIN_FIELD(0, 270, 0x0300, 0x10, 0, 4),
> > +};
> > +
> > +static const struct mtk_pin_field_calc mt8196_pin_dir_range[] = {
> > + PIN_FIELD(0, 270, 0x0000, 0x10, 0, 1),
> > +};
> > +
> > +static const struct mtk_pin_field_calc mt8196_pin_di_range[] = {
> > + PIN_FIELD(0, 270, 0x0200, 0x10, 0, 1),
> > +};
> > +
> > +static const struct mtk_pin_field_calc mt8196_pin_do_range[] = {
> > + PIN_FIELD(0, 270, 0x0100, 0x10, 0, 1),
> > +};
> > +
>
> ..snip..
>
> > +static const unsigned int mt8196_pull_type[] = {
> > + MTK_PULL_PU_PD_TYPE,/*0*/ MTK_PULL_PU_PD_TYPE,/
> > *1*/
> > + MTK_PULL_PU_PD_TYPE,/*2*/ MTK_PULL_PU_PD_TYPE,/
> > *3*/
> > + MTK_PULL_PU_PD_TYPE,/*4*/ MTK_PULL_PU_PD_TYPE,/
> > *5*/
> > + MTK_PULL_PU_PD_TYPE,/*6*/ MTK_PULL_PU_PD_TYPE,/
> > *7*/
> > + MTK_PULL_PU_PD_TYPE,/*8*/ MTK_PULL_PU_PD_TYPE,/
> > *9*/
> > + MTK_PULL_PU_PD_TYPE,/*10*/ MTK_PULL_PU_PD_TYPE,/
> > *11*/
> > + MTK_PULL_PU_PD_TYPE,/*12*/ MTK_PULL_PU_PD_TYPE,/
> > *13*/
> > + MTK_PULL_PU_PD_TYPE,/*14*/ MTK_PULL_PU_PD_TYPE,/
> > *15*/
> > + MTK_PULL_PU_PD_TYPE,/*16*/ MTK_PULL_PU_PD_TYPE,/
> > *17*/
> > + MTK_PULL_PU_PD_TYPE,/*18*/ MTK_PULL_PU_PD_TYPE,/
> > *19*/
> > + MTK_PULL_PU_PD_TYPE,/*20*/ MTK_PULL_PU_PD_TYPE,/
> > *21*/
> > + MTK_PULL_PU_PD_TYPE,/*22*/ MTK_PULL_PU_PD_TYPE,/
> > *23*/
> > + MTK_PULL_PU_PD_TYPE,/*24*/ MTK_PULL_PU_PD_TYPE,/
> > *25*/
> > + MTK_PULL_PU_PD_TYPE,/*26*/ MTK_PULL_PU_PD_TYPE,/
> > *27*/
> > + MTK_PULL_PU_PD_TYPE,/*28*/ MTK_PULL_PU_PD_TYPE,/
> > *29*/
> > + MTK_PULL_PU_PD_TYPE,/*30*/ MTK_PULL_PU_PD_TYPE,/
> > *31*/
> > + MTK_PULL_PU_PD_TYPE,/*32*/ MTK_PULL_PU_PD_TYPE,/
> > *33*/
> > + MTK_PULL_PU_PD_TYPE,/*34*/ MTK_PULL_PU_PD_TYPE,/
> > *35*/
> > + MTK_PULL_PU_PD_TYPE,/*36*/ MTK_PULL_PU_PD_TYPE,/
> > *37*/
> > + MTK_PULL_PU_PD_TYPE,/*38*/ MTK_PULL_PU_PD_TYPE,/
> > *39*/
> > + MTK_PULL_PU_PD_TYPE,/*40*/ MTK_PULL_PU_PD_TYPE,/
> > *41*/
> > + MTK_PULL_PU_PD_TYPE,/*42*/ MTK_PULL_PU_PD_TYPE,/
> > *43*/
> > + MTK_PULL_PU_PD_TYPE,/*44*/ MTK_PULL_PU_PD_TYPE,/
> > *45*/
> > + MTK_PULL_PU_PD_RSEL_TYPE,/*46*/
> > MTK_PULL_PU_PD_RSEL_TYPE,/*47*/
> > + MTK_PULL_PU_PD_RSEL_TYPE,/*48*/
> > MTK_PULL_PU_PD_RSEL_TYPE,/*49*/
> > + MTK_PULL_PU_PD_RSEL_TYPE,/*50*/
> > MTK_PULL_PU_PD_RSEL_TYPE,/*51*/
>
> Please fix the indentation to be consistent.
It will be fixed in next version.
>
> > + MTK_PULL_PU_PD_RSEL_TYPE,/*52*/
> > MTK_PULL_PU_PD_RSEL_TYPE,/*53*/
>
> ..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),
>
> Where is eint?!
It will be add in next version.
>
> > + .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,
> > + .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,
> > +};
> > +
> > +static const struct of_device_id mt8196_pinctrl_of_match[] = {
> > + { .compatible = "mediatek,mt8196-pinctrl", .data =
> > &mt8196_data },
> > + { }
>
> { /* sentinel */ }
>
> > +};
>
> Regards,
> Angelo
>
Powered by blists - more mailing lists