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 <neil.armstrong@xxxxxxxxxx> > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx> > --- > > The v2 of this specific patch is here: > https://lore.kernel.org/all/20221123152001.694546-4-abel.vesa@xxxxxxxxxx/ > > 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 >