2023-02-08 08:20:00

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v3 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 v2 of this specific patch is here:
https://lore.kernel.org/all/[email protected]/

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..510c964dd0f5 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 > 1)
+ 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-08 08:20:03

by Abel Vesa

[permalink] [raw]
Subject: [PATCH v3 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]>
---

This patch does not have a v2 (or a v1). This is an entirely new patch and it is
part of this patchset to provide full context.

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-09 03:45:58

by Bjorn Andersson

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

On Wed, Feb 08, 2023 at 10:18:35AM +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]>
> ---
>
> The v2 of this specific patch is here:
> https://lore.kernel.org/all/[email protected]/
>
> 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..510c964dd0f5 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 > 1)

I would prefer that we do arg == MSM_I2C_STRONG_PULL_UP, primarily to
improve the symmetry with the getter, but also just to ensure that
someone accidentally writing bias-pull-up = <1>; gets less of a
surprise.

Looks good otherwise.

Thanks,
Bjorn

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