Re: [PATCH v6 4/5] pinctrl: mediatek: Backward compatible to previous Mediatek's bias-pull usage
Hi,
On Thu, Sep 26, 2019 at 10:02 PM Light Hsieh wrote:
>
> Refine mtk_pinconf_set()/mtk_pinconf_get() for backward compatibility to
> previous Mediatek's bias-pull usage.
MediaTek
> In PINCTRL_MTK that use pinctrl-mtk-common.c, bias-pull setting for pins
> with 2 pull resistors can be specified as value for bias-pull-up and
> bias-pull-down. For example:
> bias-pull-up = ;
> bias-pull-up = ;
> bias-pull-up = ;
> bias-pull-up = ;
> bias-pull-down = ;
> bias-pull-down = ;
> bias-pull-down = ;
> bias-pull-down = ;
>
> On the other hand, PINCTRL_MTK_PARIS use customized properties
> "mediatek,pull-up-adv" and "mediatek,pull-down-adv" to specify bias-pull
> setting for pins with 2 pull resistors.
> This introduce in-compatibility in device tree and increatse porting
s/increatse/inscrease/
> effort to Mediatek's customer that had already used PINCTRL_MTK version.
s/Mediatek/MediaTek/
> Besides, if customers are not awared of this change and still write devicetree
s/awared/aware/
> for PINCTRL_MTK version, they may encounter runtime failure with pinctrl and
> spent time to debug.
>
> This patch add backward compatible to previous Mediatek's bias-pull usage
s/add/adds
> so that Mediatek's customer need not use a new devicetree property name.
s/Mediatek/MediaTek/
> The rationale is that: changing driver implemenation had better leave
s/implemenation/implementation/
> interface unchanged.
>
There are many tags missing here too.
> ---
> drivers/pinctrl/mediatek/pinctrl-mt6765.c| 6 +-
> drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 285
> +++
> drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h | 11 +
> drivers/pinctrl/mediatek/pinctrl-paris.c | 49 ++--
> 4 files changed, 327 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6765.c
> b/drivers/pinctrl/mediatek/pinctrl-mt6765.c
> index bada37f..ae85fdc 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt6765.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6765.c
> @@ -1072,10 +1072,8 @@
> .gpio_m = 0,
> .base_names = mt6765_pinctrl_register_base_names,
> .nbase_names = ARRAY_SIZE(mt6765_pinctrl_register_base_names),
> - .bias_disable_set = mtk_pinconf_bias_disable_set,
> - .bias_disable_get = mtk_pinconf_bias_disable_get,
> - .bias_set = mtk_pinconf_bias_set,
> - .bias_get = mtk_pinconf_bias_get,
> + .bias_set_combo = mtk_pinconf_bias_set_combo,
> + .bias_get_combo = mtk_pinconf_bias_get_combo,
> .drive_set = mtk_pinconf_drive_set_direct_val,
> .drive_get = mtk_pinconf_drive_get_direct_val,
> .adv_pull_get = mtk_pinconf_adv_pull_get,
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index acfddf9..6d9972f 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -13,6 +13,8 @@
> #include
> #include
>
> +#include
> +
List the header declarations in alphabetic order.
> #include "mtk-eint.h"
> #include "pinctrl-mtk-common-v2.h"
>
> @@ -206,6 +208,20 @@ int mtk_hw_set_value(struct mtk_pinctrl *hw, const
> struct mtk_pin_desc *desc,
> return 0;
> }
>
> +void mtk_hw_set_value_no_lookup(struct mtk_pinctrl *hw,
> + const struct mtk_pin_desc *desc,
> + int value, struct mtk_pin_field *pf)
> +{
> + if (value < 0 || value > pf->mask)
> + return;
> +
> + if (!pf->next)
> + mtk_rmw(hw, pf->index, pf->offset, pf->mask << pf->bitpos,
> + (value & pf->mask) << pf->bitpos);
> + else
> + mtk_hw_write_cross_field(hw, pf, value);
> +}
> +
> int mtk_hw_get_value(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc,
> int field, int *value)
> {
> @@ -225,6 +241,17 @@ int mtk_hw_get_value(struct mtk_pinctrl *hw, const
> struct mtk_pin_desc *desc,
> return 0;
> }
>
> +void mtk_hw_get_value_no_lookup(struct mtk_pinctrl *hw,
> + const struct mtk_pin_desc *desc,
> + int *value, struct mtk_pin_field *pf)
> +{
> + if (!pf->next)
> + *value = (mtk_r32(hw, pf->index, pf->offset)
> + >> pf->bitpos) & pf->mask;
> + else
> + mtk_hw_read_cross_field(hw, pf, value);
> +}
> +
We are able to improve mtk_hw_[set,get]_value with a cache for the
recently visited pin_desc to speed up the subsequent access to the
same register for all clients, rather than creating a specific one
with no_lookup just for a few clients. Generally, all clients should
be able to just apply the mtk_hw_[set,get]_value to compose what they
actually need.
And the changes related to the improvement on mtk_hw_[set,get]_value
with a cache is needed to put to a separate patch
[PATCH v6 4/5] pinctrl: mediatek: Backward compatible to previous Mediatek's bias-pull usage
Refine mtk_pinconf_set()/mtk_pinconf_get() for backward compatibility to
previous Mediatek's bias-pull usage.
In PINCTRL_MTK that use pinctrl-mtk-common.c, bias-pull setting for pins
with 2 pull resistors can be specified as value for bias-pull-up and
bias-pull-down. For example:
bias-pull-up = ;
bias-pull-up = ;
bias-pull-up = ;
bias-pull-up = ;
bias-pull-down = ;
bias-pull-down = ;
bias-pull-down = ;
bias-pull-down = ;
On the other hand, PINCTRL_MTK_PARIS use customized properties
"mediatek,pull-up-adv" and "mediatek,pull-down-adv" to specify bias-pull
setting for pins with 2 pull resistors.
This introduce in-compatibility in device tree and increatse porting
effort to Mediatek's customer that had already used PINCTRL_MTK version.
Besides, if customers are not awared of this change and still write devicetree
for PINCTRL_MTK version, they may encounter runtime failure with pinctrl and
spent time to debug.
This patch add backward compatible to previous Mediatek's bias-pull usage
so that Mediatek's customer need not use a new devicetree property name.
The rationale is that: changing driver implemenation had better leave
interface unchanged.
---
drivers/pinctrl/mediatek/pinctrl-mt6765.c| 6 +-
drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 285 +++
drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h | 11 +
drivers/pinctrl/mediatek/pinctrl-paris.c | 49 ++--
4 files changed, 327 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6765.c
b/drivers/pinctrl/mediatek/pinctrl-mt6765.c
index bada37f..ae85fdc 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt6765.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt6765.c
@@ -1072,10 +1072,8 @@
.gpio_m = 0,
.base_names = mt6765_pinctrl_register_base_names,
.nbase_names = ARRAY_SIZE(mt6765_pinctrl_register_base_names),
- .bias_disable_set = mtk_pinconf_bias_disable_set,
- .bias_disable_get = mtk_pinconf_bias_disable_get,
- .bias_set = mtk_pinconf_bias_set,
- .bias_get = mtk_pinconf_bias_get,
+ .bias_set_combo = mtk_pinconf_bias_set_combo,
+ .bias_get_combo = mtk_pinconf_bias_get_combo,
.drive_set = mtk_pinconf_drive_set_direct_val,
.drive_get = mtk_pinconf_drive_get_direct_val,
.adv_pull_get = mtk_pinconf_adv_pull_get,
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
index acfddf9..6d9972f 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
@@ -13,6 +13,8 @@
#include
#include
+#include
+
#include "mtk-eint.h"
#include "pinctrl-mtk-common-v2.h"
@@ -206,6 +208,20 @@ int mtk_hw_set_value(struct mtk_pinctrl *hw, const struct
mtk_pin_desc *desc,
return 0;
}
+void mtk_hw_set_value_no_lookup(struct mtk_pinctrl *hw,
+ const struct mtk_pin_desc *desc,
+ int value, struct mtk_pin_field *pf)
+{
+ if (value < 0 || value > pf->mask)
+ return;
+
+ if (!pf->next)
+ mtk_rmw(hw, pf->index, pf->offset, pf->mask << pf->bitpos,
+ (value & pf->mask) << pf->bitpos);
+ else
+ mtk_hw_write_cross_field(hw, pf, value);
+}
+
int mtk_hw_get_value(struct mtk_pinctrl *hw, const struct mtk_pin_desc *desc,
int field, int *value)
{
@@ -225,6 +241,17 @@ int mtk_hw_get_value(struct mtk_pinctrl *hw, const struct
mtk_pin_desc *desc,
return 0;
}
+void mtk_hw_get_value_no_lookup(struct mtk_pinctrl *hw,
+ const struct mtk_pin_desc *desc,
+ int *value, struct mtk_pin_field *pf)
+{
+ if (!pf->next)
+ *value = (mtk_r32(hw, pf->index, pf->offset)
+ >> pf->bitpos) & pf->mask;
+ else
+ mtk_hw_read_cross_field(hw, pf, value);
+}
+
static int mtk_xt_find_eint_num(struct mtk_pinctrl *hw, unsigned long eint_n)
{
const struct mtk_pin_desc *desc;
@@ -517,6 +544,264 @@ int mtk_pinconf_bias_get_rev1(struct mtk_pinctrl *hw,
return 0;
}
+/* Combo for the following pull register type:
+ * 1. PU + PD
+ * 2. PULLSEL + PULLEN
+ * 3. PUPD + R0 + R1
+ */
+int mtk_pinconf_bias_set_pu_pd(struct mtk_pinctrl *hw,
+ const struct mtk_pin_desc *desc,
+ u32 pullup, u32 arg)
+{
+ struct mtk_pin_field pf;
+ int err = -EINVAL;
+ int pu, pd;
+
+ err = mtk_hw_pin_field_lookup(hw, desc, PINCTRL_PIN_REG_PU, );
+ if (err)
+ goto out;
+
+ if (arg == MTK_DISABLE) {
+ pu = 0;
+ pd = 0;
+ } else if ((arg == MTK_ENABLE) && pullup) {
+ pu = 1;
+ pd = 0;
+ } else if ((arg == MTK_ENABLE) && !pullup) {
+ pu = 0;
+
