Received: by 2002:a05:6a11:4021:0:0:0:0 with SMTP id ky33csp333734pxb; Thu, 23 Sep 2021 00:38:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw90RHXmuez6NHXKEcbbCMR3yzbYxx/tlPBBCrGRO7ca+oroouDMJnp3R5sj7Se15JZ2s82 X-Received: by 2002:a92:d40d:: with SMTP id q13mr2576919ilm.161.1632382704718; Thu, 23 Sep 2021 00:38:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1632382704; cv=none; d=google.com; s=arc-20160816; b=aBqYS93MpzFzrBhI9xgPC1uKKrxpKqdYogzpCCfOLex1ZjaEr3Os6HGjYfc+Zh38G8 ZPXdsKwMRcpKGX6UW7t7JtaLkrEqq4MSq3WqUQxEHStRzuhURwcNckYMak2gmT0hpJsU frFq+wzZovZNd+AFI4Oz1hyA061k0xIP0lCW3pHIKzI8Bo+u5++PcV285KjxbUMk5U08 tMH255NZxrviPC/yk1xe0f8df3OPXlBXqHCEGKpBgUuLUI5zN9mFsPaHjeGtAiZ7qMWT QwsjG6Vj5hyGPnjwAaThXCqDcCQjoNYOdQZfXnCpF1cPiQM96ZiQJFkaYcCK570Zopco rpWQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=+cC8xLRJRWGZD+7uLcE09Frc3fOq9euGoUEJen8E9co=; b=uUX0FlEAACtWwQSweczpQ/vZAZy+csMf3WfS/neEt6emsD0l9fucj75NKJatatMEm/ 1xydKhKhxckIRpgU3NU0V+rNDBl4KYGL8xaRjo09/FOo/Ukx5JksFkWgHMS+RwZBj8Mo PPlB4Zl91d8koYKKxgeCV6Yzo/biafdDPg/3g+Rnl0UlN0S5REwrE+VOiSYhzntMYhRI h1V1sF3fGAnmoaq1oow6N7+aZt4fqPqo7I8tMptfzPiyi36GLmtIMCAFNm9ucTwAFY1/ FkMdYkvedpr74C8Rq9w61hZTzFv42XXTWA6wE9Oq4BpCKJGnVZaaNSkTuGnLROQEy3it xUkQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b="j0nw7L4/"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id m10si6396205ilu.53.2021.09.23.00.38.12; Thu, 23 Sep 2021 00:38:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b="j0nw7L4/"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231672AbhIWHhe (ORCPT + 99 others); Thu, 23 Sep 2021 03:37:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36314 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234261AbhIWHhd (ORCPT ); Thu, 23 Sep 2021 03:37:33 -0400 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 414AAC061574 for ; Thu, 23 Sep 2021 00:36:02 -0700 (PDT) Received: by mail-lf1-x12c.google.com with SMTP id i4so22918220lfv.4 for ; Thu, 23 Sep 2021 00:36:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+cC8xLRJRWGZD+7uLcE09Frc3fOq9euGoUEJen8E9co=; b=j0nw7L4/BMsPxeqXknHfUZiRTM18E3iaQ7Zd/S1kr8QhKn2zhsDclGDJr6Xp1nTleh qIeM0YzJ3rR0P9utml9LCoEfM1kmaY9Dy2MUjob0KrF5+4izT203pvcC8JeL1trcdUzn zfHYlJCx1t1Y+w4LxFBuuASLkF05zz2uqaxCM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+cC8xLRJRWGZD+7uLcE09Frc3fOq9euGoUEJen8E9co=; b=XjDM1QIO8QtlKE+Uj+KQOiICA1D5M7dUFQ1E2IW0i8DS8mPPBB3F+CQctyqKXKFsLO yQdkb6Pm2WqpXjSz5bTPw6udHCZWuHsESIi3fKIY/qwwF6/ITHU9ofKo7PjQD1HDagwc 51PY1YUhNNnlBq1q06ctcLpvsOlg/shTO9TMG8qfTx7VnczH+FS4LgktlouDCHPxE6fZ CdPFgwlIGeZJpRNKRXTx7ysmVLDkrfJDUEPcOBgx/70N2glXUNsie7ZuPL6rVZ/mKT4X cG9zQn034jpwdoiojk6QuqDAHtHwVTWAgJ8usUKF0KYVqnk3h4OsqRHGHCb5+SC9Pcyl h71w== X-Gm-Message-State: AOAM530l/pugxMLpFB4iB1mASsKeX+glZPHho8rgYzmfy17SvFvK+t/3 DCFdjSj3nkkH5iUD2paGaIb3XFt9jvjuoAzRJImKRJGq0Y1j4A== X-Received: by 2002:ac2:4e0f:: with SMTP id e15mr2807538lfr.308.1632382560340; Thu, 23 Sep 2021 00:36:00 -0700 (PDT) MIME-Version: 1.0 References: <20210922025640.11600-1-zhiyong.tao@mediatek.com> <20210922025640.11600-5-zhiyong.tao@mediatek.com> In-Reply-To: <20210922025640.11600-5-zhiyong.tao@mediatek.com> From: Chen-Yu Tsai Date: Thu, 23 Sep 2021 15:35:49 +0800 Message-ID: Subject: Re: [PATCH v13 4/5] pinctrl: mediatek: support rsel feature To: Zhiyong Tao Cc: Rob Herring , Linus Walleij , Mark Rutland , Matthias Brugger , Sean Wang , srv_heupstream , hui.liu@mediatek.com, Light Hsieh , Biao Huang , Hongzhou Yang , Sean Wang , Seiya Wang , Devicetree List , LKML , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , "moderated list:ARM/Mediatek SoC support" , "open list:GPIO SUBSYSTEM" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wed, Sep 22, 2021 at 10:59 AM Zhiyong Tao wrote: > > This patch supports rsel(resistance selection) feature for I2C pins. > It provides more resistance selection solution in different ICs. > It provides rsel define and si unit solution by identifying > "mediatek,rsel_resistance_in_si_unit" property in pio dtsi node. > > Signed-off-by: Zhiyong Tao > --- > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 231 +++++++++++++++--- > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 45 ++++ > drivers/pinctrl/mediatek/pinctrl-paris.c | 60 +++-- > 3 files changed, 288 insertions(+), 48 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > index 5b3b048725cc..e84001923aaf 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > @@ -661,6 +661,181 @@ static int mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, > return err; > } > > +static int mtk_hw_pin_rsel_lookup(struct mtk_pinctrl *hw, > + const struct mtk_pin_desc *desc, > + u32 pullup, u32 arg, u32 *rsel_val) > +{ > + const struct mtk_pin_rsel *rsel; > + int check; > + bool found = false; > + > + rsel = hw->soc->pin_rsel; > + > + for (check = 0; check <= hw->soc->npin_rsel - 1; check++) { > + if (desc->number >= rsel[check].s_pin && > + desc->number <= rsel[check].e_pin) { > + if (pullup) { > + if (rsel[check].up_rsel == arg) { > + found = true; > + *rsel_val = rsel[check].rsel_index; > + break; The code could simply `return 0` after setting *rsel_val, then we don't have to have the `found` variable. Unless your goal is to use the last matching value in the case of duplicates, instead of the first. If that is the case you should add a comment stating so along with the reason, And the structure could be written as if (pin not in range) continue; ... check value ... which would decrease the nesting level. Mostly stylistic though. > + } > + } else { > + if (rsel[check].down_rsel == arg) { > + found = true; > + *rsel_val = rsel[check].rsel_index; > + break; > + } > + } > + } > + } > + > + if (!found) { > + dev_err(hw->dev, "Not support rsel value %d Ohm for pin = %d (%s)\n", > + arg, desc->number, desc->name); > + return -ENOTSUPP; > + } > + > + return 0; > +} > + [...] > +static int mtk_pinconf_bias_get_rsel(struct mtk_pinctrl *hw, > + const struct mtk_pin_desc *desc, > + u32 *pullup, u32 *enable) > +{ > + int pu, pd, rsel, err; > + > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_RSEL, &rsel); > + if (err) > + goto out; > + > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PU, &pu); > + if (err) > + goto out; > + > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PD, &pd); mtk_pinconf_bias_get_pu_pd() couldn't be reused? > + > + if (pu == 0 && pd == 0) { > + *pullup = 0; > + *enable = MTK_DISABLE; > + } else if (pu == 1 && pd == 0) { > + *pullup = 1; > + if (hw->rsel_si_unit) > + mtk_rsel_get_si_unit(hw, desc, *pullup, rsel, enable); > + else > + *enable = rsel + MTK_PULL_SET_RSEL_000; > + } else if (pu == 0 && pd == 1) { > + *pullup = 0; > + if (hw->rsel_si_unit) > + mtk_rsel_get_si_unit(hw, desc, *pullup, rsel, enable); > + else > + *enable = rsel + MTK_PULL_SET_RSEL_000; > + } else { > + err = -EINVAL; > + goto out; > + } > + > +out: > + return err; > +} > + > static int mtk_pinconf_bias_get_pu_pd(struct mtk_pinctrl *hw, > const struct mtk_pin_desc *desc, > u32 *pullup, u32 *enable) > @@ -742,44 +917,40 @@ static int mtk_pinconf_bias_get_pupd_r1_r0(struct mtk_pinctrl *hw, > return err; > } > > -int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, > - const struct mtk_pin_desc *desc, > - u32 pullup, u32 arg) > -{ > - int err; > - > - err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, arg); > - if (!err) > - goto out; > - > - err = mtk_pinconf_bias_set_pullsel_pullen(hw, desc, pullup, arg); > - if (!err) > - goto out; > - > - err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, pullup, arg); > - > -out: > - return err; > -} > -EXPORT_SYMBOL_GPL(mtk_pinconf_bias_set_combo); > - > int mtk_pinconf_bias_get_combo(struct mtk_pinctrl *hw, > const struct mtk_pin_desc *desc, > u32 *pullup, u32 *enable) > { > - int err; > + int err = -ENOTSUPP; > + u32 try_all_type; > > - err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, enable); > - if (!err) > - goto out; > + if (hw->soc->pull_type) > + try_all_type = hw->soc->pull_type[desc->number]; > + else > + try_all_type = MTK_PULL_TYPE_MASK; > > - err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, pullup, enable); > - if (!err) > - goto out; > + if (try_all_type & MTK_PULL_RSEL_TYPE) { > + err = mtk_pinconf_bias_get_rsel(hw, desc, pullup, enable); > + if (!err) > + return err; > + } > > - err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, enable); > + if (try_all_type & MTK_PULL_PU_PD_TYPE) { > + err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, enable); > + if (!err) > + return err; > + } > + > + if (try_all_type & MTK_PULL_PULLSEL_TYPE) { > + err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, > + pullup, enable); > + if (!err) > + return err; > + } > + > + if (try_all_type & MTK_PULL_PUPD_R1R0_TYPE) > + err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, enable); > > -out: > return err; > } > EXPORT_SYMBOL_GPL(mtk_pinconf_bias_get_combo); > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > index a6f1bdb2083b..4908c7aedbe0 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > @@ -17,6 +17,22 @@ > #define MTK_ENABLE 1 > #define MTK_PULLDOWN 0 > #define MTK_PULLUP 1 > +#define MTK_PULL_PU_PD_TYPE BIT(0) > +#define MTK_PULL_PULLSEL_TYPE BIT(1) > +#define MTK_PULL_PUPD_R1R0_TYPE BIT(2) > +/* MTK_PULL_RSEL_TYPE can select resistance and can be > + * turned on/off itself. But it can't be selected pull up/down > + */ > +#define MTK_PULL_RSEL_TYPE BIT(3) > +/* MTK_PULL_PU_PD_RSEL_TYPE is a type which is controlled by > + * MTK_PULL_PU_PD_TYPE and MTK_PULL_RSEL_TYPE. > + */ > +#define MTK_PULL_PU_PD_RSEL_TYPE (MTK_PULL_PU_PD_TYPE \ > + | MTK_PULL_RSEL_TYPE) > +#define MTK_PULL_TYPE_MASK (MTK_PULL_PU_PD_TYPE |\ > + MTK_PULL_PULLSEL_TYPE |\ > + MTK_PULL_PUPD_R1R0_TYPE |\ > + MTK_PULL_RSEL_TYPE) > > #define EINT_NA U16_MAX > #define NO_EINT_SUPPORT EINT_NA > @@ -42,6 +58,14 @@ > PIN_FIELD_CALC(_s_pin, _e_pin, 0, _s_addr, _x_addrs, _s_bit, \ > _x_bits, 32, 1) > > +#define PIN_RSEL(_s_pin, _e_pin, _rsel_index, _up_resl, _down_rsel) { \ > + .s_pin = _s_pin, \ > + .e_pin = _e_pin, \ > + .rsel_index = _rsel_index, \ > + .up_rsel = _up_resl, \ > + .down_rsel = _down_rsel, \ > + } > + > /* List these attributes which could be modified for the pin */ > enum { > PINCTRL_PIN_REG_MODE, > @@ -67,6 +91,7 @@ enum { > PINCTRL_PIN_REG_DRV_E0, > PINCTRL_PIN_REG_DRV_E1, > PINCTRL_PIN_REG_DRV_ADV, > + PINCTRL_PIN_REG_RSEL, > PINCTRL_PIN_REG_MAX, > }; > > @@ -129,6 +154,21 @@ struct mtk_pin_field_calc { > u8 fixed; > }; > > +/* struct mtk_pin_rsel - the structure that provides bias resistance selection. Since you went through the trouble of documenting all the fields, would you consider changing this to a kernel-doc style comment? It is similar to Java-doc, and would be like: /** * struct mtk_pin_rsel ...... * @s_pin: .... * ... */ Only the start of the comment block needs to be changed. See Documentation/doc-guide/kernel-doc.rst if you are unsure. > + * @s_pin: the start pin within the rsel range > + * @e_pin: the end pin within the rsel range > + * @rsel_index: the rsel bias resistance index > + * @up_rsel: the pullup rsel bias resistance value > + * @down_rsel: the pulldown rsel bias resistance value > + */ > +struct mtk_pin_rsel { > + u16 s_pin; > + u16 e_pin; > + u16 rsel_index; > + u32 up_rsel; > + u32 down_rsel; > +}; > + > /* struct mtk_pin_reg_calc - the structure that holds all ranges used to > * determine which register the pin would make use of > * for certain pin attribute. > @@ -206,6 +246,9 @@ struct mtk_pin_soc { > bool ies_present; > const char * const *base_names; > unsigned int nbase_names; > + const unsigned int *pull_type; > + const struct mtk_pin_rsel *pin_rsel; > + unsigned int npin_rsel; > > /* Specific pinconfig operations */ > int (*bias_disable_set)(struct mtk_pinctrl *hw, > @@ -254,6 +297,8 @@ struct mtk_pinctrl { > const char **grp_names; > /* lock pin's register resource to avoid multiple threads issue*/ > spinlock_t lock; > + /* identify rsel setting by si unit or rsel define in dts node */ > + bool rsel_si_unit; > }; > > void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set); > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c > index 38aec0177d15..d4e02c5d74a8 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > @@ -579,8 +579,9 @@ static int mtk_hw_get_value_wrap(struct mtk_pinctrl *hw, unsigned int gpio, int > ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > unsigned int gpio, char *buf, unsigned int buf_len) > { > - int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1; > + int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1, rsel = -1; > const struct mtk_pin_desc *desc; > + u32 try_all_type; > > if (gpio >= hw->soc->npins) > return -EINVAL; > @@ -591,24 +592,39 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > pinmux -= hw->soc->nfuncs; > > mtk_pinconf_bias_get_combo(hw, desc, &pullup, &pullen); > - if (pullen == MTK_PUPD_SET_R1R0_00) { > - pullen = 0; > - r1 = 0; > - r0 = 0; > - } else if (pullen == MTK_PUPD_SET_R1R0_01) { > - pullen = 1; > - r1 = 0; > - r0 = 1; > - } else if (pullen == MTK_PUPD_SET_R1R0_10) { > - pullen = 1; > - r1 = 1; > - r0 = 0; > - } else if (pullen == MTK_PUPD_SET_R1R0_11) { > + > + if (hw->soc->pull_type) > + try_all_type = hw->soc->pull_type[desc->number]; > + > + if (hw->rsel_si_unit && (try_all_type & MTK_PULL_RSEL_TYPE)) { > + rsel = pullen; > pullen = 1; > - r1 = 1; > - r0 = 1; > - } else if (pullen != MTK_DISABLE && pullen != MTK_ENABLE) { > - pullen = 0; > + } else { > + /* Case for: R1R0 */ > + if (pullen == MTK_PUPD_SET_R1R0_00) { > + pullen = 0; > + r1 = 0; > + r0 = 0; > + } else if (pullen == MTK_PUPD_SET_R1R0_01) { > + pullen = 1; > + r1 = 0; > + r0 = 1; > + } else if (pullen == MTK_PUPD_SET_R1R0_10) { > + pullen = 1; > + r1 = 1; > + r0 = 0; > + } else if (pullen == MTK_PUPD_SET_R1R0_11) { > + pullen = 1; > + r1 = 1; > + r0 = 1; > + } > + > + /* Case for: RSEL */ > + if (pullen >= MTK_PULL_SET_RSEL_000 && > + pullen <= MTK_PULL_SET_RSEL_111) { > + rsel = pullen - MTK_PULL_SET_RSEL_000; > + pullen = 1; > + } > } > len += scnprintf(buf + len, buf_len - len, > "%03d: %1d%1d%1d%1d%02d%1d%1d%1d%1d", > @@ -626,6 +642,8 @@ ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > if (r1 != -1) { > len += scnprintf(buf + len, buf_len - len, " (%1d %1d)\n", > r1, r0); > + } else if (rsel != -1) { > + len += scnprintf(buf + len, buf_len - len, " (%1d)\n", rsel); > } else { > len += scnprintf(buf + len, buf_len - len, "\n"); > } > @@ -970,6 +988,12 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev, > > hw->nbase = hw->soc->nbase_names; > > + if (of_find_property(hw->dev->of_node, > + "mediatek,rsel_resistance_in_si_unit", NULL)) This new property should be documented in the bindings. Regards ChenYu > + hw->rsel_si_unit = true; > + else > + hw->rsel_si_unit = false; > + > spin_lock_init(&hw->lock); > > err = mtk_pctrl_build_state(pdev); > -- > 2.25.1 > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek