Received: by 10.223.185.116 with SMTP id b49csp2063429wrg; Sun, 4 Mar 2018 17:27:10 -0800 (PST) X-Google-Smtp-Source: AG47ELvJDBAOxCk12ZM4IYyuo4fBMbG8GdkpTr3DeD7MOlUdVTNDLscwGmt+IenWIb6VmH35mILu X-Received: by 2002:a17:902:3a1:: with SMTP id d30-v6mr11296240pld.409.1520213230840; Sun, 04 Mar 2018 17:27:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520213230; cv=none; d=google.com; s=arc-20160816; b=UOuoX1ubMZ6YtdF+y/9aw4rGLy/dtXjWkVd+9XngHBvuYq3SGYq8/iFIeN5gWH5DYw u+tf3izN9CIowPKN5UINWel2mXUu3rreQoUhhh/g0qtjaCXKlK9BlHSL7j/mxUIVz/Nx lVO8L9gS/X3WjHNogCkA+DcOgSef5h4P706nZpwArukbPlnuKTscDBuabUHvSEqTiPYC KnH9nx1I+pizoQLdM0IHV1THxXg2I+vv2JY1xrg0fHW/I2BWtW02DtFmZq+s6/DZ9C1o QarmsBeLherNtodQKxkcWmviFsepfOfE5GDgdIdj8W0jn0ZRxoR/i7XruLVoox3yGrTJ ozvg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:content-transfer-encoding :references:in-reply-to:date:cc:to:from:subject:message-id :arc-authentication-results; bh=3JTm8aZZl4RbkQbic/ZduHhN5B6M/FW/fO+lKu1hyac=; b=IoOTPUoKiSSgtcjkeuVWfW+lcvcGehwNaXr+zUMCdQnKLEBUE+7QcbhntclPq88jJW lx3OY5Va/+hLUG/1qoprFQ6lXDcCqVRss7BTebhqgFvNXM1+AWtxOVUwcXgwynf2WbEF FAUMoWYFM9tSqesUmuzc5l2JBE2cCnfo+jbuPYuo142eWBnJnhC+QBqbvvBfIjyeaV4t j3lqbWh/10Hf1YbIk/nwDT49N7YHPA6uEvcT70LTPUMnVcR65cyNIdE+zu0T1A0OMeVT d33+fadRzL21K4JJeI0vxktqjHgKXvq0gOKOM1lBspcGgqs1YZ0uSj1OrHQVoZUQGATP C0jA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id y7si7605319pgv.128.2018.03.04.17.26.44; Sun, 04 Mar 2018 17:27:10 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932501AbeCEBXn (ORCPT + 99 others); Sun, 4 Mar 2018 20:23:43 -0500 Received: from Mailgw01.mediatek.com ([1.203.163.78]:38483 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S932083AbeCEBXi (ORCPT ); Sun, 4 Mar 2018 20:23:38 -0500 X-UUID: 2221437ab41d426987a9395cd80b8f0d-20180305 Received: from mtkcas35.mediatek.inc [(172.27.4.250)] by mailgw01.mediatek.com (envelope-from ) (mailgw01.mediatek.com ESMTP with TLS) with ESMTP id 597705886; Mon, 05 Mar 2018 09:23:10 +0800 Received: from MTKCAS32.mediatek.inc (172.27.4.184) by MTKMBS31N2.mediatek.inc (172.27.4.87) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Mon, 5 Mar 2018 09:23:09 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS32.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1210.3 via Frontend Transport; Mon, 5 Mar 2018 09:23:08 +0800 Message-ID: <1520212988.23198.22.camel@mhfsdcap03> Subject: Re: [PATCH v2 4/4] pintcrl: support bias-disable of generic and special pins simultaneously From: Zhiyong Tao To: Sean Wang CC: , , , , , , , , , , , , , , , Date: Mon, 5 Mar 2018 09:23:08 +0800 In-Reply-To: <1519890183.8089.114.camel@mtkswgap22> References: <1519634042-12063-1-git-send-email-zhiyong.tao@mediatek.com> <1519634042-12063-5-git-send-email-zhiyong.tao@mediatek.com> <1519803217.8089.31.camel@mtkswgap22> <1519804157.23198.9.camel@mhfsdcap03> <1519876072.23198.17.camel@mhfsdcap03> <1519890183.8089.114.camel@mtkswgap22> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2018-03-01 at 15:43 +0800, Sean Wang wrote: > On Thu, 2018-03-01 at 11:47 +0800, Zhiyong Tao wrote: > > 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 > > > > > --- > > > > > 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. > > why not rewrite it with ==> Hi sean, Thanks for your comments, we will try to do it. If it is ok, we will modify it in v3. > > @@ -301,8 +301,10 @@ 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) { > + unsigned int r1r0 = 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); > + pin, pctl->devdata->port_align, isup, r1r0); > if (!ret) > return 0; > } > > and also I appeared to find another bug in the original code when I had > a close look at the code path, but it's not related to the patch: > > a special pin would still go through the code path to handle > generic pin when the special pin uses an invalid argument. > > It looks like err-code from pctl->devdata->spec_pull_set cannot be > used directly to judge its pin type. It's also possible to use it as an > indication of the real failure during special pin is being set up. > ==>Hi sean, The function seems ok. it is not related to the patch. We will think about your comment. If there is better idea to modify the code flow. We will change it in another patch. 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; > > > > > } > > > > > > > > > > > > > > > > >