| 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: <1501578845.32089.18.camel@mtksdaap41>
Date: Tue, 1 Aug 2017 17:14:05 +0800
From: Yingjoe Chen <yingjoe.chen@...iatek.com>
To: Zhiyong Tao <zhiyong.tao@...iatek.com>
CC: <robh+dt@...nel.org>, <linus.walleij@...aro.org>,
<mark.rutland@....com>, <matthias.bgg@...il.com>,
<srv_heupstream@...iatek.com>, <liguo.zhang@...iatek.com>,
<hongkun.cao@...iatek.com>, <biao.huang@...iatek.com>,
<yt.shen@...iatek.com>, <hongzhou.yang@...iatek.com>,
<erin.lo@...iatek.com>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>, <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH 3/3] pinctrl: add mt2712 pinctrl driver
Hi Zhiyong,
On Mon, 2017-07-31 at 16:22 +0800, Zhiyong Tao wrote:
<...>
> 3)Add "spec_dir_set" and "spec_dir_get" in "mtk_pinctrl_devdata".
> 4)Change "spec_dir_set" and add "spec_dir_get" in "pinctrl-mt2701.c"
> and "pinctrl-mtk-common.c".
I think these deserve another patch.
Please also explain why we need this.
> 5)Change "port_mask" from "7" to "6" for EINT.
I'm assuming this is a bug fix for mt2701?
If yes, this should be a separate patch.
> 6)Remove generic pull config condition in "mtk_pconf_set_pull_select".
> 7)Change "arg" to "MTK_PUPD_SET_R1R0_00" of "mtk_pconf_set_pull_select".
Why we need to change arg?
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@...iatek.com>
> ---
> drivers/pinctrl/mediatek/Kconfig | 8 +
> drivers/pinctrl/mediatek/Makefile | 1 +
> drivers/pinctrl/mediatek/pinctrl-mt2701.c | 21 +-
> drivers/pinctrl/mediatek/pinctrl-mt2712.c | 670 +++++++++
> drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 16 +-
> drivers/pinctrl/mediatek/pinctrl-mtk-common.h | 44 +-
> drivers/pinctrl/mediatek/pinctrl-mtk-mt2712.h | 1858 +++++++++++++++++++++++++
> 7 files changed, 2586 insertions(+), 32 deletions(-)
> create mode 100644 drivers/pinctrl/mediatek/pinctrl-mt2712.c
> create mode 100644 drivers/pinctrl/mediatek/pinctrl-mtk-mt2712.h
>
<...>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt2701.c b/drivers/pinctrl/mediatek/pinctrl-mt2701.c
> index f86f3b3..4a43f5c 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt2701.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt2701.c
> @@ -503,10 +503,26 @@ static void mt2701_spec_pinmux_set(struct regmap *reg, unsigned int pin,
> regmap_update_bits(reg, mt2701_spec_pinmux[i].offset, mask, value);
> }
>
> -static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
> +static int mt2701_spec_dir_set(struct mtk_pinctrl *pctl,
> + unsigned int *reg_addr,
> + unsigned int pin,
> + bool input)
> {
> if (pin > 175)
> *reg_addr += 0x10;
> +
> + return 0;
> +}
> +
> +static int mt2701_spec_dir_get(struct mtk_pinctrl *pctl,
> + unsigned int *reg_addr,
> + unsigned int pin,
> + bool input)
incorrect prototype?
> +{
> + if (pin > 175)
> + *reg_addr += 0x10;
> +
> + return 0;
> }
>
> static const struct mtk_pinctrl_devdata mt2701_pinctrl_data = {
> @@ -520,6 +536,7 @@ static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
> .spec_ies_smt_set = mt2701_ies_smt_set,
> .spec_pinmux_set = mt2701_spec_pinmux_set,
> .spec_dir_set = mt2701_spec_dir_set,
> + .spec_dir_get = mt2701_spec_dir_get,
> .dir_offset = 0x0000,
> .pullen_offset = 0x0150,
> .pullsel_offset = 0x0280,
> @@ -551,7 +568,7 @@ static void mt2701_spec_dir_set(unsigned int *reg_addr, unsigned int pin)
> .dbnc_ctrl = 0x500,
> .dbnc_set = 0x600,
> .dbnc_clr = 0x700,
> - .port_mask = 6,
> + .port_mask = 7,
> .ports = 6,
> },
> .ap_num = 169,
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt2712.c b/drivers/pinctrl/mediatek/pinctrl-mt2712.c
> new file mode 100644
> index 0000000..c933b75
> --- /dev/null
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt2712.c
<...>
> +
> +static int mt2712_spec_dir_set(struct mtk_pinctrl *pctl,
> + unsigned int *reg_addr,
> + unsigned int pin,
> + bool input)
> +{
> + u32 reg_val;
> +
> + if (pin == 16) {
> + regmap_read(pctl->regmap2, 0x940, ®_val);
> + reg_val |= BIT(15);
> + if (input == true)
> + reg_val &= ~BIT(14);
> + else
> + reg_val |= BIT(14);
> + regmap_write(pctl->regmap2, 0x940, reg_val);
> + }
> +
> + if (pin == 17) {
> + regmap_read(pctl->regmap2, 0x940, ®_val);
> + reg_val |= BIT(7);
> + if (input == true)
> + reg_val &= ~BIT(6);
> + else
> + reg_val |= BIT(6);
> + regmap_write(pctl->regmap2, 0x940, reg_val);
> + }
> +
> + return 0;
> +}
Does this means pin 16, 17 is in different/special reg/bit location?
I didn't see spec_dir_get in your patch, does this means they are in
standard location or you just forgot it?
The original idea of spec_dir_set is to get the register offset for the
pin, so both set_direction and get_direction are using the same
extension function. Instead of adding a new spec_dir_get, can we just
extend the function to also include bit location?
<...>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 3cf384f..aeec87e 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> @@ -84,7 +84,7 @@ static int mtk_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> bit = BIT(offset & 0xf);
>
> if (pctl->devdata->spec_dir_set)
> - pctl->devdata->spec_dir_set(®_addr, offset);
> + pctl->devdata->spec_dir_set(pctl, ®_addr, offset, input);
>
> if (input)
> /* Different SoC has different alignment offset. */
> @@ -307,13 +307,6 @@ static int mtk_pconf_set_pull_select(struct mtk_pinctrl *pctl,
> return 0;
> }
>
> - /* For generic pull config, default arg value should be 0 or 1. */
> - if (arg != 0 && arg != 1) {
> - dev_err(pctl->dev, "invalid pull-up argument %d on pin %d .\n",
> - arg, pin);
> - return -EINVAL;
> - }
> -
Why we need to remove this?
> bit = BIT(pin & 0xf);
> if (enable)
> reg_pullen = SET_ADDR(mtk_get_port(pctl, pin) +
> @@ -343,7 +336,8 @@ static int mtk_pconf_parse_conf(struct pinctrl_dev *pctldev,
>
> switch (param) {
> case PIN_CONFIG_BIAS_DISABLE:
> - ret = mtk_pconf_set_pull_select(pctl, pin, false, false, arg);
> + ret = mtk_pconf_set_pull_select(pctl, pin, false, false,
> + MTK_PUPD_SET_R1R0_00);
Why we need to change this?
Joe.C
Powered by blists - more mailing lists