On Wed, 2018-02-28 at 15:49 +0800, Zhiyong Tao wrote: > On Wed, 2018-02-28 at 15:33 +0800, Sean Wang wrote: > > On Mon, 2018-02-26 at 16:34 +0800, Zhiyong Tao wrote: > > > For generic pins, parameter "arg" is 0 or 1. > > > For special pins, bias-disable is set by R0R1, > > > so we need transmited "00" to set bias-disable > > > When we set "bias-disable" as high-z property, > > > the parameter should be "MTK_PUPD_SET_R1R0_00". > > > > > > Signed-off-by: Zhiyong Tao <zhiyong.tao@xxxxxxxxxxxx> > > > --- > > > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 13 +++++++++++-- > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > > index 3cf384f..e88ba04 100644 > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > > > @@ -301,8 +301,17 @@ static int mtk_pconf_set_pull_select(struct mtk_pinctrl *pctl, > > > * resistor bit, so we need this special handle. > > > */ > > > if (pctl->devdata->spec_pull_set) { > > > - ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, pin), > > > - pin, pctl->devdata->port_align, isup, arg); > > > + if (enable) { > > > + ret = pctl->devdata->spec_pull_set( > > > + mtk_get_regmap(pctl, pin), pin, > > > + pctl->devdata->port_align, isup, > > > + arg); > > > + } else { > > > + ret = pctl->devdata->spec_pull_set( > > > + mtk_get_regmap(pctl, pin), pin, > > > + pctl->devdata->port_align, isup, > > > + MTK_PUPD_SET_R1R0_00); > > > + } > > > > > > it looks like you can use as following snippet with reusing original > > logic and less effort on maintaining these common code (?) > ==> Thanks for your suggestion very much. It seems better. > I will change it in v3. ==> Hi sean, we try it in our site. we can't change like this. Because we will check "arg" after setting "arg = MTK_PUPD_SET_R1R0_00;". following snippet which check "arg" value is o or 1. If it not, which will print error. so we can't change like this. /* For generic pull config, default arg value should be 0 or 1. */ if (arg != 0 && arg != 1) { dev_err(pctl->dev, "invalid pull-up argument %d on pin %d .\n", arg, pin); return -EINVAL; } Thanks. > > > > > > if (pctl->devdata->spec_pull_set) { > > + if (!enable) > > + arg = MTK_PUPD_SET_R1R0_00; > > > > ret = pctl->devdata->spec_pull_set(mtk_get_regmap(pctl, pin), > > pin, pctl->devdata->port_align, isup, arg); > > > > > > Also, it's better to add more comments to such kind of special path for > > allowing future SoCs to follow and extend more easily. > > ==> Thanks for your suggestion very much. I will add more comments here. > > > > > if (!ret) > > > return 0; > > > } > > > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html