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@mediatek.com> wrote:

quoted

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 <redacted>
---
 .../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.

quoted

+                               }
+                       } 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;
+}
+

[...]

quoted

+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.

quoted

+
+       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.

quoted

+ * @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

quoted

+               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

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek