[PATCH v13 4/5] pinctrl: mediatek: support rsel feature
zhiyong.tao
zhiyong.tao at mediatek.com
Thu Sep 23 01:38:40 PDT 2021
On Thu, 2021-09-23 at 15:35 +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Wed, Sep 22, 2021 at 10:59 AM Zhiyong Tao <
> zhiyong.tao at mediatek.com> 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 <zhiyong.tao at mediatek.com>
> > ---
> > .../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.
We use "found" variable to identify whether user set the right si unit
value or not .
>
> 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?
It couldn't be reused. Because in the api, if user use rsel si unit, it
can return si unit value, if user use rsel define, it can return rsel
define value.
>
> > +
> > + 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.
we will fix it in the next version.
>
> > + * @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.
>
we will add it in the next version.
>
> 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 at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
More information about the linux-arm-kernel
mailing list