2023-02-09 07:45:27

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v4 1/2] pinctrl: qcom: Add support for i2c specific pull feature

Add support for the new i2c_pull property introduced for SM8550 setting
a I2C specific pull mode on I2C able pins. Add the bit to the SM8550
specific driver while at it.

Co-developed-by: Neil Armstrong <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---

The v3 of this specific patch is here:
https://lore.kernel.org/all/[email protected]/

Changes since v3:
* changed the condition in msm_config_group_set to "arg == MSM_I2C_STRONG_PULL_UP"
as Bjorn suggested

Changes since v2:
* This time, this patch is sent separate w.r.t. SM8550 pinctrl driver
* The qcom,i2c-pull is dropped, bias-pull-up with value is used instead
* Default value for i2c pull up is 2.2kOhms and since SM8550 is the
first one to use it, we hard code it for now
* changed the authorship as the implementation looks entirely different now

drivers/pinctrl/qcom/pinctrl-msm.c | 7 +++++++
drivers/pinctrl/qcom/pinctrl-msm.h | 1 +
drivers/pinctrl/qcom/pinctrl-sm8550.c | 1 +
3 files changed, 9 insertions(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 5142c363480a..a69f93e74435 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -310,6 +310,8 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
case PIN_CONFIG_BIAS_PULL_UP:
*bit = g->pull_bit;
*mask = 3;
+ if (g->i2c_pull_bit)
+ *mask |= BIT(g->i2c_pull_bit) >> *bit;
break;
case PIN_CONFIG_DRIVE_OPEN_DRAIN:
*bit = g->od_bit;
@@ -336,6 +338,7 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
#define MSM_KEEPER 2
#define MSM_PULL_UP_NO_KEEPER 2
#define MSM_PULL_UP 3
+#define MSM_I2C_STRONG_PULL_UP 2200

static unsigned msm_regval_to_drive(u32 val)
{
@@ -387,6 +390,8 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
case PIN_CONFIG_BIAS_PULL_UP:
if (pctrl->soc->pull_no_keeper)
arg = arg == MSM_PULL_UP_NO_KEEPER;
+ else if (arg & BIT(g->i2c_pull_bit))
+ arg = MSM_I2C_STRONG_PULL_UP;
else
arg = arg == MSM_PULL_UP;
if (!arg)
@@ -467,6 +472,8 @@ static int msm_config_group_set(struct pinctrl_dev *pctldev,
case PIN_CONFIG_BIAS_PULL_UP:
if (pctrl->soc->pull_no_keeper)
arg = MSM_PULL_UP_NO_KEEPER;
+ else if (g->i2c_pull_bit && arg == MSM_I2C_STRONG_PULL_UP)
+ arg = BIT(g->i2c_pull_bit) | MSM_PULL_UP;
else
arg = MSM_PULL_UP;
break;
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h
index 05a1209bf9ae..985eceda2517 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.h
+++ b/drivers/pinctrl/qcom/pinctrl-msm.h
@@ -80,6 +80,7 @@ struct msm_pingroup {

unsigned pull_bit:5;
unsigned drv_bit:5;
+ unsigned i2c_pull_bit:5;

unsigned od_bit:5;
unsigned egpio_enable:5;
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8550.c b/drivers/pinctrl/qcom/pinctrl-sm8550.c
index 0b7db7d4054a..c9d038098f2c 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm8550.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm8550.c
@@ -47,6 +47,7 @@
.mux_bit = 2, \
.pull_bit = 0, \
.drv_bit = 6, \
+ .i2c_pull_bit = 13, \
.egpio_enable = 12, \
.egpio_present = 11, \
.oe_bit = 9, \
--
2.34.1



2023-02-09 07:45:31

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v4 2/2] arm64: dts: qcom: sm8550: Add bias pull up value to tlmm i2c data clk states

The default bias pull up value for the tlmm i2c data clk states is
2.2kOhms. Add this value to make sure the driver factors in the i2c pull
up bit when writing the config register.

Signed-off-by: Abel Vesa <[email protected]>
---

The v3 of this specific patch is here:
https://lore.kernel.org/all/[email protected]/

Changes since v3:
* none

arch/arm64/boot/dts/qcom/sm8550.dtsi | 30 ++++++++++++++--------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 1dea055a6815..6e60afc748cf 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2695,7 +2695,7 @@ qup_i2c0_data_clk: qup-i2c0-data-clk-state {
pins = "gpio28", "gpio29";
function = "qup1_se0";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c1_data_clk: qup-i2c1-data-clk-state {
@@ -2703,7 +2703,7 @@ qup_i2c1_data_clk: qup-i2c1-data-clk-state {
pins = "gpio32", "gpio33";
function = "qup1_se1";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c2_data_clk: qup-i2c2-data-clk-state {
@@ -2711,7 +2711,7 @@ qup_i2c2_data_clk: qup-i2c2-data-clk-state {
pins = "gpio36", "gpio37";
function = "qup1_se2";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c3_data_clk: qup-i2c3-data-clk-state {
@@ -2719,7 +2719,7 @@ qup_i2c3_data_clk: qup-i2c3-data-clk-state {
pins = "gpio40", "gpio41";
function = "qup1_se3";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c4_data_clk: qup-i2c4-data-clk-state {
@@ -2727,7 +2727,7 @@ qup_i2c4_data_clk: qup-i2c4-data-clk-state {
pins = "gpio44", "gpio45";
function = "qup1_se4";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c5_data_clk: qup-i2c5-data-clk-state {
@@ -2735,7 +2735,7 @@ qup_i2c5_data_clk: qup-i2c5-data-clk-state {
pins = "gpio52", "gpio53";
function = "qup1_se5";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c6_data_clk: qup-i2c6-data-clk-state {
@@ -2743,7 +2743,7 @@ qup_i2c6_data_clk: qup-i2c6-data-clk-state {
pins = "gpio48", "gpio49";
function = "qup1_se6";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c8_data_clk: qup-i2c8-data-clk-state {
@@ -2751,14 +2751,14 @@ scl-pins {
pins = "gpio57";
function = "qup2_se0_l1_mira";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

sda-pins {
pins = "gpio56";
function = "qup2_se0_l0_mira";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};
};

@@ -2767,7 +2767,7 @@ qup_i2c9_data_clk: qup-i2c9-data-clk-state {
pins = "gpio60", "gpio61";
function = "qup2_se1";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c10_data_clk: qup-i2c10-data-clk-state {
@@ -2775,7 +2775,7 @@ qup_i2c10_data_clk: qup-i2c10-data-clk-state {
pins = "gpio64", "gpio65";
function = "qup2_se2";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c11_data_clk: qup-i2c11-data-clk-state {
@@ -2783,7 +2783,7 @@ qup_i2c11_data_clk: qup-i2c11-data-clk-state {
pins = "gpio68", "gpio69";
function = "qup2_se3";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c12_data_clk: qup-i2c12-data-clk-state {
@@ -2791,7 +2791,7 @@ qup_i2c12_data_clk: qup-i2c12-data-clk-state {
pins = "gpio2", "gpio3";
function = "qup2_se4";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c13_data_clk: qup-i2c13-data-clk-state {
@@ -2799,7 +2799,7 @@ qup_i2c13_data_clk: qup-i2c13-data-clk-state {
pins = "gpio80", "gpio81";
function = "qup2_se5";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_i2c15_data_clk: qup-i2c15-data-clk-state {
@@ -2807,7 +2807,7 @@ qup_i2c15_data_clk: qup-i2c15-data-clk-state {
pins = "gpio72", "gpio106";
function = "qup2_se7";
drive-strength = <2>;
- bias-pull-up;
+ bias-pull-up = <2200>;
};

qup_spi0_cs: qup-spi0-cs-state {
--
2.34.1


2023-02-10 04:02:11

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] pinctrl: qcom: Add support for i2c specific pull feature

On Thu, Feb 09, 2023 at 09:45:09AM +0200, Abel Vesa wrote:
> Add support for the new i2c_pull property introduced for SM8550 setting
> a I2C specific pull mode on I2C able pins. Add the bit to the SM8550
> specific driver while at it.
>
> Co-developed-by: Neil Armstrong <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> Signed-off-by: Abel Vesa <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

> ---
>
> The v3 of this specific patch is here:
> https://lore.kernel.org/all/[email protected]/
>
> Changes since v3:
> * changed the condition in msm_config_group_set to "arg == MSM_I2C_STRONG_PULL_UP"
> as Bjorn suggested
>
> Changes since v2:
> * This time, this patch is sent separate w.r.t. SM8550 pinctrl driver
> * The qcom,i2c-pull is dropped, bias-pull-up with value is used instead
> * Default value for i2c pull up is 2.2kOhms and since SM8550 is the
> first one to use it, we hard code it for now
> * changed the authorship as the implementation looks entirely different now
>
> drivers/pinctrl/qcom/pinctrl-msm.c | 7 +++++++
> drivers/pinctrl/qcom/pinctrl-msm.h | 1 +
> drivers/pinctrl/qcom/pinctrl-sm8550.c | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 5142c363480a..a69f93e74435 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -310,6 +310,8 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
> case PIN_CONFIG_BIAS_PULL_UP:
> *bit = g->pull_bit;
> *mask = 3;
> + if (g->i2c_pull_bit)
> + *mask |= BIT(g->i2c_pull_bit) >> *bit;
> break;
> case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> *bit = g->od_bit;
> @@ -336,6 +338,7 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
> #define MSM_KEEPER 2
> #define MSM_PULL_UP_NO_KEEPER 2
> #define MSM_PULL_UP 3
> +#define MSM_I2C_STRONG_PULL_UP 2200
>
> static unsigned msm_regval_to_drive(u32 val)
> {
> @@ -387,6 +390,8 @@ static int msm_config_group_get(struct pinctrl_dev *pctldev,
> case PIN_CONFIG_BIAS_PULL_UP:
> if (pctrl->soc->pull_no_keeper)
> arg = arg == MSM_PULL_UP_NO_KEEPER;
> + else if (arg & BIT(g->i2c_pull_bit))
> + arg = MSM_I2C_STRONG_PULL_UP;
> else
> arg = arg == MSM_PULL_UP;
> if (!arg)
> @@ -467,6 +472,8 @@ static int msm_config_group_set(struct pinctrl_dev *pctldev,
> case PIN_CONFIG_BIAS_PULL_UP:
> if (pctrl->soc->pull_no_keeper)
> arg = MSM_PULL_UP_NO_KEEPER;
> + else if (g->i2c_pull_bit && arg == MSM_I2C_STRONG_PULL_UP)
> + arg = BIT(g->i2c_pull_bit) | MSM_PULL_UP;
> else
> arg = MSM_PULL_UP;
> break;
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h
> index 05a1209bf9ae..985eceda2517 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.h
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.h
> @@ -80,6 +80,7 @@ struct msm_pingroup {
>
> unsigned pull_bit:5;
> unsigned drv_bit:5;
> + unsigned i2c_pull_bit:5;
>
> unsigned od_bit:5;
> unsigned egpio_enable:5;
> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8550.c b/drivers/pinctrl/qcom/pinctrl-sm8550.c
> index 0b7db7d4054a..c9d038098f2c 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sm8550.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sm8550.c
> @@ -47,6 +47,7 @@
> .mux_bit = 2, \
> .pull_bit = 0, \
> .drv_bit = 6, \
> + .i2c_pull_bit = 13, \
> .egpio_enable = 12, \
> .egpio_present = 11, \
> .oe_bit = 9, \
> --
> 2.34.1
>

2023-02-10 22:48:28

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] pinctrl: qcom: Add support for i2c specific pull feature

On Thu, Feb 9, 2023 at 8:45 AM Abel Vesa <[email protected]> wrote:

> Add support for the new i2c_pull property introduced for SM8550 setting
> a I2C specific pull mode on I2C able pins. Add the bit to the SM8550
> specific driver while at it.
>
> Co-developed-by: Neil Armstrong <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> Signed-off-by: Abel Vesa <[email protected]>

Applied to the pin control tree for v6.3, I expect that Bjorn will apply
patch 2/2 to the qcom tree!

Yours,
Linus Walleij

2023-03-07 04:17:16

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH v4 1/2] pinctrl: qcom: Add support for i2c specific pull feature

On Thu, 9 Feb 2023 09:45:09 +0200, Abel Vesa wrote:
> Add support for the new i2c_pull property introduced for SM8550 setting
> a I2C specific pull mode on I2C able pins. Add the bit to the SM8550
> specific driver while at it.
>
>

Applied, thanks!

[2/2] arm64: dts: qcom: sm8550: Add bias pull up value to tlmm i2c data clk states
commit: 4059297ed0a5adf8e5fd0bd734d702a24202c02e

Best regards,
--
Bjorn Andersson <[email protected]>