From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 60490C433EF for ; Thu, 9 Sep 2021 07:46:29 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 181CC610C8 for ; Thu, 9 Sep 2021 07:46:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 181CC610C8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=EeDedU9omXkewtukM0ZatnN9NRCobsMbo6o4mmn/kjc=; b=bOBjkVoz4ny16+ QSe7YrdztlqU8HaySgFlwDukR0HmPpdXAD0/t8zs3B2Bo1DjgPkA8YiNKveFHjtXce4WsKhSeIbde 4fWbHqZWWn7TuSu+9mfsbtv/Rdecee2hU694gbjPstCw9jUvp9tjtSAD9ymvAlATNRcto/klBqWKS x3rEcd1MutnY5yrJVNirUkZAWp5MOrsY7mQnlCpWKu3LpDYVkZ3of3h4LaIjcJ9YVckF24kvMEmGw 1lt7h6uOmOvgpryXrfF/qUMxpHq0zoiRFVdbrQvJyjG7LYRb3McGOu2D/+k2tAN6hpr5ZRSfuvBlJ PhxUNZZ1SC4dvT/FB0FQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOElF-008TSn-8D; Thu, 09 Sep 2021 07:46:17 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOElA-008TSJ-4Z; Thu, 09 Sep 2021 07:46:16 +0000 X-UUID: 3ca83b0a26154ab7a4045d3f8a6d5913-20210909 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=6OExwzly8ojabM5m+fp8KUyz7yZ3K6TerpWbxhH2yE8=; b=cQo5vKc8ORrKtxh/QsZh0tnlIOlgpfGsaoaP/+IPFuaR9bh2sO7XXPrauKwrS4h31/OO2u6wCdTiXKbHx+DGK5AS+4gMH/VnzHhrvgJalmhu7UlejMtCbPfBB25hBzQxmjNXV4/nxXQ25U48aVOQWsbYmnw8Oexa662m9pqVGj0=; X-UUID: 3ca83b0a26154ab7a4045d3f8a6d5913-20210909 Received: from mtkcas66.mediatek.inc [(172.29.193.44)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 36408918; Thu, 09 Sep 2021 00:46:08 -0700 Received: from MTKMBS07N2.mediatek.inc (172.21.101.141) by MTKMBS62N1.mediatek.inc (172.29.193.41) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 9 Sep 2021 00:46:03 -0700 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs07n2.mediatek.inc (172.21.101.141) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 9 Sep 2021 15:46:00 +0800 Received: from mhfsdcap04 (10.17.3.154) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Thu, 9 Sep 2021 15:45:59 +0800 Message-ID: <60c01a7fb76aea17211b204b228eacf928739689.camel@mediatek.com> Subject: Re: [PATCH v11 4/4] pinctrl: mediatek: add rsel setting on MT8195 From: zhiyong.tao To: Chen-Yu Tsai , Linus Walleij CC: Rob Herring , Mark Rutland , Matthias Brugger , Sean Wang , srv_heupstream , , "Eddie Huang" , Light Hsieh , Biao Huang , Hongzhou Yang , Sean Wang , Seiya Wang , Devicetree List , LKML , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , "moderated list:ARM/Mediatek SoC support" , "open list:GPIO SUBSYSTEM" Date: Thu, 9 Sep 2021 15:46:01 +0800 In-Reply-To: References: <20210830003603.31864-1-zhiyong.tao@mediatek.com> <20210830003603.31864-5-zhiyong.tao@mediatek.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210909_004612_271056_73188EB8 X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Mon, 2021-09-06 at 18:09 +0800, Chen-Yu Tsai wrote: > On Mon, Sep 6, 2021 at 11:17 AM zhiyong.tao > wrote: > > > > On Wed, 2021-09-01 at 18:10 +0800, Chen-Yu Tsai wrote: > > > Hi, > > > > > > On Mon, Aug 30, 2021 at 8:37 AM Zhiyong Tao < > > > zhiyong.tao@mediatek.com > > > > wrote: > > > > > > > > This patch provides rsel setting on MT8195 > > > > > > A bit more context, like what is rsel, would be nice. > > > > > > > we will add more context it in the next version. > > > > > > > > Signed-off-by: Zhiyong Tao > > > > --- > > > > drivers/pinctrl/mediatek/pinctrl-mt8195.c | 133 > > > > +++++++++++++ > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 181 > > > > ++++++++++++++++-- > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 35 +++- > > > > drivers/pinctrl/mediatek/pinctrl-paris.c | 24 ++- > > > > drivers/pinctrl/mediatek/pinctrl-paris.h | 2 +- > > > > 5 files changed, 348 insertions(+), 27 deletions(-) > > > > > > Could you split this patch into two, so the common parts are in > > > the > > > first > > > patch, and the mt8195 specific changes are in the second? > > > > > > > ok, we will split in the next version. > > > > > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > b/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > index 892e79703f98..275b7ba4386d 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > @@ -779,6 +779,135 @@ static const struct mtk_pin_field_calc > > > > mt8195_pin_drv_adv_range[] = { > > > > PIN_FIELD_BASE(45, 45, 1, 0x040, 0x10, 9, 3), > > > > }; > > > > > > > > +static const struct mtk_pin_field_calc mt8195_pin_rsel_range[] > > > > = { > > > > + PIN_FIELD_BASE(8, 8, 4, 0x0c0, 0x10, 15, 3), > > > > + PIN_FIELD_BASE(9, 9, 4, 0x0c0, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(10, 10, 4, 0x0c0, 0x10, 18, 3), > > > > + PIN_FIELD_BASE(11, 11, 4, 0x0c0, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(12, 12, 4, 0x0c0, 0x10, 21, 3), > > > > + PIN_FIELD_BASE(13, 13, 4, 0x0c0, 0x10, 6, 3), > > > > + PIN_FIELD_BASE(14, 14, 4, 0x0c0, 0x10, 24, 3), > > > > + PIN_FIELD_BASE(15, 15, 4, 0x0c0, 0x10, 9, 3), > > > > + PIN_FIELD_BASE(16, 16, 4, 0x0c0, 0x10, 27, 3), > > > > + PIN_FIELD_BASE(17, 17, 4, 0x0c0, 0x10, 12, 3), > > > > + PIN_FIELD_BASE(29, 29, 2, 0x080, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(30, 30, 2, 0x080, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(34, 34, 1, 0x0e0, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(35, 35, 1, 0x0e0, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(44, 44, 1, 0x0e0, 0x10, 6, 3), > > > > + PIN_FIELD_BASE(45, 45, 1, 0x0e0, 0x10, 9, 3), > > > > +}; > > > > + > > > > +static const struct mtk_pin_rsel mt8195_pin_rsel_val_range[] = > > > > { > > > > + PIN_RSEL(8, 17, 0x0, 75000, 75000), > > > > + PIN_RSEL(8, 17, 0x1, 10000, 5000), > > > > + PIN_RSEL(8, 17, 0x2, 5000, 75000), > > > > + PIN_RSEL(8, 17, 0x3, 4000, 5000), > > > > + PIN_RSEL(8, 17, 0x4, 3000, 75000), > > > > + PIN_RSEL(8, 17, 0x5, 2000, 5000), > > > > + PIN_RSEL(8, 17, 0x6, 1500, 75000), > > > > + PIN_RSEL(8, 17, 0x7, 1000, 5000), > > > > + PIN_RSEL(29, 30, 0x0, 75000, 75000), > > > > + PIN_RSEL(29, 30, 0x1, 10000, 5000), > > > > + PIN_RSEL(29, 30, 0x2, 5000, 75000), > > > > + PIN_RSEL(29, 30, 0x3, 4000, 5000), > > > > + PIN_RSEL(29, 30, 0x4, 3000, 75000), > > > > + PIN_RSEL(29, 30, 0x5, 2000, 5000), > > > > + PIN_RSEL(29, 30, 0x6, 1500, 75000), > > > > + PIN_RSEL(29, 30, 0x7, 1000, 5000), > > > > + PIN_RSEL(34, 35, 0x0, 75000, 75000), > > > > + PIN_RSEL(34, 35, 0x1, 10000, 5000), > > > > + PIN_RSEL(34, 35, 0x2, 5000, 75000), > > > > + PIN_RSEL(34, 35, 0x3, 4000, 5000), > > > > + PIN_RSEL(34, 35, 0x4, 3000, 75000), > > > > + PIN_RSEL(34, 35, 0x5, 2000, 5000), > > > > + PIN_RSEL(34, 35, 0x6, 1500, 75000), > > > > + PIN_RSEL(34, 35, 0x7, 1000, 5000), > > > > + PIN_RSEL(44, 45, 0x0, 75000, 75000), > > > > + PIN_RSEL(44, 45, 0x1, 10000, 5000), > > > > + PIN_RSEL(44, 45, 0x2, 5000, 75000), > > > > + PIN_RSEL(44, 45, 0x3, 4000, 5000), > > > > + PIN_RSEL(44, 45, 0x4, 3000, 75000), > > > > + PIN_RSEL(44, 45, 0x5, 2000, 5000), > > > > + PIN_RSEL(44, 45, 0x6, 1500, 75000), > > > > + PIN_RSEL(44, 45, 0x7, 1000, 5000), > > > > +}; > > > > + > > > > +static const unsigned int mt8195_pull_type[] = { > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 0 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 1 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 2 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 3 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 4 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 5 */, > > > > + MTK_PULL_PU_PD_TYPE /* 6 */, MTK_PULL_PU_PD_TYPE /* 7 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 8 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 9 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 10 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 11 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 12 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 13 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 14 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 15 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 16 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 17 */, > > > > + MTK_PULL_PU_PD_TYPE /* 18 */, MTK_PULL_PU_PD_TYPE /* 19 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 20 */, MTK_PULL_PU_PD_TYPE /* 21 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 22 */, MTK_PULL_PU_PD_TYPE /* 23 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 24 */, MTK_PULL_PU_PD_TYPE /* 25 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 26 */, MTK_PULL_PU_PD_TYPE /* 27 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 28 */, MTK_PULL_PU_PD_RSEL_TYPE > > > > /* > > > > 29 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 30 */, MTK_PULL_PU_PD_TYPE > > > > /* > > > > 31 */, > > > > + MTK_PULL_PU_PD_TYPE /* 32 */, MTK_PULL_PU_PD_TYPE /* 33 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 34 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 35 */, > > > > + MTK_PULL_PU_PD_TYPE /* 36 */, MTK_PULL_PU_PD_TYPE /* 37 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 38 */, MTK_PULL_PU_PD_TYPE /* 39 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 40 */, MTK_PULL_PU_PD_TYPE /* 41 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 42 */, MTK_PULL_PU_PD_TYPE /* 43 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 44 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 45 */, > > > > + MTK_PULL_PU_PD_TYPE /* 46 */, MTK_PULL_PU_PD_TYPE /* 47 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 48 */, MTK_PULL_PU_PD_TYPE /* 49 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 50 */, MTK_PULL_PU_PD_TYPE /* 51 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 52 */, MTK_PULL_PU_PD_TYPE /* 53 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 54 */, MTK_PULL_PU_PD_TYPE /* 55 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 56 */, MTK_PULL_PU_PD_TYPE /* 57 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 58 */, MTK_PULL_PU_PD_TYPE /* 59 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 60 */, MTK_PULL_PU_PD_TYPE /* 61 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 62 */, MTK_PULL_PU_PD_TYPE /* 63 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 64 */, MTK_PULL_PU_PD_TYPE /* 65 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 66 */, MTK_PULL_PU_PD_TYPE /* 67 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 68 */, MTK_PULL_PU_PD_TYPE /* 69 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 70 */, MTK_PULL_PU_PD_TYPE /* 71 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 72 */, MTK_PULL_PU_PD_TYPE /* 73 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 74 */, MTK_PULL_PU_PD_TYPE /* 75 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 76 */, MTK_PULL_PUPD_R1R0_TYPE > > > > /* 77 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 78 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 79 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 80 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 81 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 82 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 83 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 84 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 85 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 86 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 87 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 88 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 89 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 90 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 91 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 92 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 93 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 94 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 95 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 96 */, MTK_PULL_PU_PD_TYPE > > > > /* 97 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 98 */, MTK_PULL_PU_PD_TYPE /* 99 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 100 */, MTK_PULL_PU_PD_TYPE /* > > > > 101 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 102 */, MTK_PULL_PU_PD_TYPE /* > > > > 103 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 104 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 105 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 106 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 107 */, > > > > + MTK_PULL_PU_PD_TYPE /* 108 */, MTK_PULL_PU_PD_TYPE /* > > > > 109 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 110 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 111 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 112 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 113 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 114 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 115 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 116 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 117 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 118 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 119 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 120 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 121 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 122 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 123 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 124 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 125 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 126 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 127 */, > > > > + MTK_PULL_PU_PD_TYPE /* 128 */, MTK_PULL_PU_PD_TYPE /* > > > > 129 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 130 */, MTK_PULL_PU_PD_TYPE /* > > > > 131 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 132 */, MTK_PULL_PU_PD_TYPE /* > > > > 133 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 134 */, MTK_PULL_PU_PD_TYPE /* > > > > 135 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 136 */, MTK_PULL_PU_PD_TYPE /* > > > > 137 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 138 */, MTK_PULL_PU_PD_TYPE /* > > > > 139 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 140 */, MTK_PULL_PU_PD_TYPE /* > > > > 141 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 142 */, MTK_PULL_PU_PD_TYPE /* > > > > 143 > > > > */, > > > > +}; > > > > + > > > > static const struct mtk_pin_reg_calc > > > > mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { > > > > [PINCTRL_PIN_REG_MODE] = > > > > MTK_RANGE(mt8195_pin_mode_range), > > > > [PINCTRL_PIN_REG_DIR] = > > > > MTK_RANGE(mt8195_pin_dir_range), > > > > @@ -793,6 +922,7 @@ static const struct mtk_pin_reg_calc > > > > mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { > > > > [PINCTRL_PIN_REG_R0] = MTK_RANGE(mt8195_pin_r0_range), > > > > [PINCTRL_PIN_REG_R1] = MTK_RANGE(mt8195_pin_r1_range), > > > > [PINCTRL_PIN_REG_DRV_ADV] = > > > > MTK_RANGE(mt8195_pin_drv_adv_range), > > > > + [PINCTRL_PIN_REG_RSEL] = > > > > MTK_RANGE(mt8195_pin_rsel_range), > > > > }; > > > > > > > > static const char * const mt8195_pinctrl_register_base_names[] > > > > = { > > > > @@ -817,6 +947,9 @@ static const struct mtk_pin_soc mt8195_data > > > > = { > > > > .gpio_m = 0, > > > > .base_names = mt8195_pinctrl_register_base_names, > > > > .nbase_names = > > > > ARRAY_SIZE(mt8195_pinctrl_register_base_names), > > > > + .pull_type = mt8195_pull_type, > > > > + .pin_rsel = mt8195_pin_rsel_val_range, > > > > + .npin_rsel = ARRAY_SIZE(mt8195_pin_rsel_val_range), > > > > .bias_set_combo = mtk_pinconf_bias_set_combo, > > > > .bias_get_combo = mtk_pinconf_bias_get_combo, > > > > .drive_set = mtk_pinconf_drive_set_rev1, > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > index 5b3b048725cc..b6a89d41f040 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > @@ -641,6 +641,9 @@ static int > > > > mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > } else if (arg == MTK_PUPD_SET_R1R0_11) { > > > > r0 = 1; > > > > r1 = 1; > > > > + } else if (arg == MTK_ENABLE) { > > > > + r0 = 1; > > > > + r1 = 0; > > > > > > This change does not seem related. This should be in a separate > > > patch > > > and why and what this change is should be described in the commit > > > log. > > > I assume this is to support "bias-pull-up" or "bias-pull-down" > > > without > > > arguments on pins using R1R0? > > > > we will separate it in next version. > > some users want only pull up/down, they don't care about the > > resistance > > value. So we support the case which only use "bias-pull-up" or > > "bias- > > pull-down" without arguments on pins using R1R0. > > I see. This should be added as a separate patch before the RSEL stuff > then. That way it would also be easier to backport in case it is > needed. > ok, we will split in the next version. > > > > > > > } else { > > > > err = -EINVAL; > > > > goto out; > > > > @@ -661,6 +664,75 @@ static int > > > > mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > return err; > > > > } > > > > > > > > +static int mtk_hw_pin_rsel_lookup(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 pullup, u32 arg, u32 > > > > *rsel_val) > > > > +{ > > > > + const struct mtk_pin_rsel *rsel; > > > > + int check; > > > > + bool found = false; > > > > + > > > > + rsel = hw->soc->pin_rsel; > > > > + > > > > + for (check = 0; check <= hw->soc->npin_rsel - 1; > > > > check++) { > > > > + if (desc->number >= rsel[check].s_pin && > > > > + desc->number <= rsel[check].e_pin) { > > > > + if (pullup) { > > > > + if (rsel[check].up_rsel == arg) > > > > { > > > > + found = true; > > > > + *rsel_val = > > > > rsel[check].rsel_index; > > > > + break; > > > > + } > > > > + } else { > > > > + if (rsel[check].down_rsel == > > > > arg) { > > > > + found = true; > > > > + *rsel_val = > > > > rsel[check].rsel_index; > > > > + break; > > > > + } > > > > + } > > > > + } > > > > + } > > > > + > > > > + if (!found) { > > > > + dev_err(hw->dev, "Not support rsel value %d Ohm > > > > for > > > > pin = %d (%s)\n", > > > > + arg, desc->number, desc->name); > > > > + return -EOPNOTSUPP; > > > > > > I believe ENOTSUPP is the correct one. EOPNOTSUPP seems to be > > > network > > > related. > > > > > > > if we change it as "ENOTSUPP", it will report warning"ENOTSUPP is > > not > > SUSV4 error code, prefer EOPNOTSUP" when checking patch. > > The context surrounding this warning seems to be that ENOTSUPP is > hard > for userspace to understand. AFAIK the return code here does not get > passed to userspace? And the pinctrl core does check for EINVAL or > ENOTSUPP, so I think this is a valid use case. > > Linus? > > > > I also think it would make sense to differentiate between the pin > > > not > > > supporting RSEL (return -ENOTSUPP) vs supporting RSEL but a > > > resistor > > > value not in the table was requested (return -EINVAL). This might > > > be > > > easier to do with the two-table design I mentioned. > > > > It only support resistor value on the patch on mt8195. we think we > > need > > only maintain one-table design. > > OK. I think there's a possibility that some corner case might happen. > I guess we'll deal with them if they occur. > > > > > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int mtk_pinconf_bias_set_rsel(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 pullup, u32 arg) > > > > +{ > > > > + int err, rsel_val; > > > > + > > > > + if (hw->soc->pin_rsel) { > > > > + /* find pin rsel_index from pin_rsel array*/ > > > > + err = mtk_hw_pin_rsel_lookup(hw, desc, pullup, > > > > arg, > > > > &rsel_val); > > > > + if (err) > > > > + goto out; > > > > + } else { > > > > + if (arg < MTK_PULL_SET_RSEL_000 || > > > > + arg > MTK_PULL_SET_RSEL_111) { > > > > + err = -EINVAL; > > > > + goto out; > > > > + } > > > > + > > > > + rsel_val -= MTK_PULL_SET_RSEL_000; > > > > + } > > > > > > This is not structured correctly. Think about this: on a chip, > > > say > > > MT8195, > > > which has hw->soc->pin_rsel defined, but the device tree writer > > > chose > > > to use the macros instead of SI unit values. This is exactly the > > > scenario > > > you discussed privately with me. > > > > > > This also fails the case where bias-pull-{up,down} is specified > > > without > > > an argument, in which case arg == 1. This does seem to be covered > > > by > > > falling back to just PU/PD, but it's not exactly obvious, > > > especially > > > since we are skipping mtk_pinconf_bias_set_pu_pd() just below. > > > > > > So you actually need to check for the magic values first, and if > > > arg > > > doesn't match any of them, assume an SI unit argument and try to > > > do > > > a lookup. > > > > > > > one very important point of upstream is keeping backward > > compatibility, > > since almost all customers of other MTK platforms does not care > > about > > resistor values so we leave user to choose MTK_PULL_SET_RSEL_XXX > > without necessity to know resistor value. > > I'm not sure about the connection to backward compatibility here, > given > that you are just introducing support for RSEL. > > > Our policy is making most customers/users convenient. > > Encourage of using SI unit is not in concern. > > > > For mt8195, we choose "hw->soc->pin_rsel", so it can support SI > > unit. > > Regarding my concern, there are a lot of moving pieces here, so let's > see > how the next revision turns out and if this is still an issue. > > > > > + > > > > + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_RSEL, > > > > rsel_val); > > > > + if (err) > > > > + goto out; > > > > + > > > > + err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, > > > > MTK_ENABLE); > > > > + > > > > +out: > > > > + return err; > > > > +} > > > > + > > > > static int mtk_pinconf_bias_get_pu_pd(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc > > > > *desc, > > > > u32 *pullup, u32 *enable) > > > > @@ -742,44 +814,117 @@ static int > > > > mtk_pinconf_bias_get_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > > > This hunk is somewhat whacky. I assume it's because you also > > > fixed up > > > the indentation for mtk_pinconf_bias_set_combo(). While nice to > > > do, > > > in this case it actually works against having an easy to read > > > patch. > > > > > > > yes, it's because to fix up > > the indentation for mtk_pinconf_bias_set_combo(). > > I suggest moving that to a separate patch. While whitespace changes > are > sometimes frowned upon, I think this is OK given that it would > improve > readability of the patches following it. You should mention that in > the > commit message to justify the change. > ok, we will split in the next version. > > > > return err; > > > > } > > > > > > > > -int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, > > > > - const struct mtk_pin_desc > > > > *desc, > > > > - u32 pullup, u32 arg) > > > > +static int mtk_pinconf_bias_get_rsel(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 *pullup, u32 *enable) > > > > { > > > > - int err; > > > > + int pu, pd, rsel, err; > > > > > > > > - err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, > > > > arg); > > > > - if (!err) > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_RSEL, > > > > &rsel); > > > > + if (err) > > > > goto out; > > > > > > > > - err = mtk_pinconf_bias_set_pullsel_pullen(hw, desc, > > > > pullup, > > > > arg); > > > > - if (!err) > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PU, > > > > &pu); > > > > + if (err) > > > > goto out; > > > > > > > > - err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, pullup, > > > > arg); > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PD, > > > > &pd); > > > > > > Maybe this could reuse mtk_pinconf_bias_get_pu_pd(), much like on > > > the > > > `set` > > > side? > > > > > > > we will try to reuse mtk_pinconf_bias_get_pu_pd(). > > > > > > + > > > > + if (pu == 0 && pd == 0) { > > > > + *pullup = 0; > > > > + *enable = MTK_DISABLE; > > > > + } else if (pu == 1 && pd == 0) { > > > > + *pullup = 1; > > > > + *enable = rsel + MTK_PULL_SET_RSEL_000; > > > > + } else if (pu == 0 && pd == 1) { > > > > + *pullup = 0; > > > > + *enable = rsel + MTK_PULL_SET_RSEL_000; > > > > + } else { > > > > + err = -EINVAL; > > > > + goto out; > > > > + } > > > > > > > > out: > > > > return err; > > > > } > > > > + > > > > +int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc *desc, > > > > + u32 pullup, u32 arg) > > > > +{ > > > > + int err = -EOPNOTSUPP; > > > > + bool try_all_type; > > > > + > > > > + try_all_type = hw->soc->pull_type ? false : true; > > > > > > Using ternary ops is discouraged. Maybe this could be structured > > > like: > > > > > > u32 pull_type; > > > > > > if (hw->soc->pull_type) > > > pull_type = hw->soc->pull_type[desc->number]; > > > else > > > pull_type = MTK_PULL_TYPE_MASK; > > > > > > That way you don't need to test `try_all_type` every time. > > > > we will try to change it in next version. > > > > > > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_RSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_set_rsel(hw, desc, > > > > pullup, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PU_PD_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pu_pd(hw, desc, > > > > pullup, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PULLSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pullsel_pullen(hw, > > > > desc, > > > > + pullu > > > > p, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PUPD_R1R0_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, > > > > pullup, arg); > > > > + if (err) > > > > + dev_err(hw->dev, "Invalid pull > > > > argument\n"); > > > > > > The "if (err)" could be moved outside the upper "if" block. That > > > way > > > the code flow looks more consistent, plus we get an error message > > > regardless of the pull type supported. > > > > we will move "if (err)" outside the upper "if" block in the next > > version. > > > > > > > + } > > > > + > > > > + return err; > > > > +} > > > > EXPORT_SYMBOL_GPL(mtk_pinconf_bias_set_combo); > > > > > > > > int mtk_pinconf_bias_get_combo(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc *desc, > > > > u32 *pullup, u32 *enable) > > > > { > > > > - int err; > > > > + int err = -EOPNOTSUPP; > > > > + bool try_all_type; > > > > > > > > - err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, > > > > enable); > > > > - if (!err) > > > > - goto out; > > > > + try_all_type = hw->soc->pull_type ? false : true; > > > > > > Same here. > > > > > > > we will try to change it in next version. > > > > > > > > > > - err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, > > > > pullup, > > > > enable); > > > > - if (!err) > > > > - goto out; > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_RSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_get_rsel(hw, desc, > > > > pullup, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PU_PD_TYPE)) { > > > > + err = mtk_pinconf_bias_get_pu_pd(hw, desc, > > > > pullup, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > > > > > - err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, > > > > enable); > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PULLSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_get_pullsel_pullen(hw, > > > > desc, > > > > + pullu > > > > p, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PUPD_R1R0_TYPE)) > > > > + err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, > > > > pullup, enable); > > > > > > > > -out: > > > > return err; > > > > } > > > > EXPORT_SYMBOL_GPL(mtk_pinconf_bias_get_combo); > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > index a6f1bdb2083b..a3d73f153efe 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > @@ -17,6 +17,13 @@ > > > > #define MTK_ENABLE 1 > > > > #define MTK_PULLDOWN 0 > > > > #define MTK_PULLUP 1 > > > > +#define MTK_PULL_PU_PD_TYPE BIT(0) > > > > +#define MTK_PULL_PULLSEL_TYPE BIT(1) > > > > +#define MTK_PULL_PUPD_R1R0_TYPE BIT(2) > > > > +#define MTK_PULL_RSEL_TYPE BIT(3) > > > > +#define MTK_PULL_PU_PD_RSEL_TYPE (MTK_PULL_PU_PD_TYPE \ > > > > + | MTK_PULL_RSEL_TYPE) > > > > > > MTK_PULL_RSEL_TYPE deserves a comment saying why it should not be > > > used > > > on its own. > > > > > > But since mtk_pinconf_bias_set_rsel() is already calling > > > mtk_pinconf_bias_set_pu_pd(), if the former function is reworked, > > > then > > > the two types don't need to be combined for the fallback > > > behavior. > > > > > > > > > MTK_PULL_PU_PD_RSEL_TYPE is type wihch is can be control by > > MTK_PULL_PU_PD_TYPE or MTK_PULL_RSEL_TYPE. so we combibe them > > together. > > But can RSEL be used without PU/PD? The definitions read like > "PU_PD_RSEL" > means either RSEL or PU/PD can be used, but if I understand the > hardware > correctly, RSEL by itself only selects the resistor value, but > doesn't > turn on/off the bias nor select pull up or down. So RSEL is an > extension > of PU/PD, and by itself won't have any real effect. > yes, it can select the resister value and can be turn on/off itself. But it can't select pull up/down. > Since this could be interpreted either way, a comment explaining > things > should be added to document the author's intentions. > We will add a comment explaining it here. > > > > #define EINT_NA U16_MAX > > > > #define NO_EINT_SUPPORT EINT_NA > > > > @@ -42,6 +49,14 @@ > > > > PIN_FIELD_CALC(_s_pin, _e_pin, 0, _s_addr, _x_addrs, > > > > _s_bit, \ > > > > _x_bits, 32, 1) > > > > > > > > +#define PIN_RSEL(_s_pin, _e_pin, _rsel_index, _up_resl, > > > > _down_rsel) { \ > > > > > > ^ rsel? > > > > > > > + .s_pin = > > > > _s_pin, \ > > > > + .e_pin = > > > > _e_pin, \ > > > > + .rsel_index = > > > > _rsel_index, \ > > > > + .up_rsel = > > > > _up_resl, \ > > > > + .down_rsel = > > > > _down_rsel, \ > > > > + } > > > > + > > > > /* List these attributes which could be modified for the pin > > > > */ > > > > enum { > > > > PINCTRL_PIN_REG_MODE, > > > > @@ -67,6 +82,7 @@ enum { > > > > PINCTRL_PIN_REG_DRV_E0, > > > > PINCTRL_PIN_REG_DRV_E1, > > > > PINCTRL_PIN_REG_DRV_ADV, > > > > + PINCTRL_PIN_REG_RSEL, > > > > PINCTRL_PIN_REG_MAX, > > > > }; > > > > > > > > @@ -129,6 +145,21 @@ struct mtk_pin_field_calc { > > > > u8 fixed; > > > > }; > > > > > > > > +/* struct mtk_pin_rsel - the structure that providing bias > > > > resistance selection. > > > > > > ^ provides > > > > we will change it in next version. > > > > > > > > > > > + * @s_pin: the start pin within the rsel range > > > > + * @e_pin: the end pin within the rsel range > > > > + * @rsel_index: the rsel bias resistance index > > > > + * @up_rsel: the pullup rsel bias resistance value > > > > + * @down_rsel: the pulldown rsel bias resistance value > > > > + */ > > > > +struct mtk_pin_rsel { > > > > + u16 s_pin; > > > > + u16 e_pin; > > > > + u16 rsel_index; > > > > + u32 up_rsel; > > > > + u32 down_rsel; > > > > +}; > > > > + > > > > > > If it were up to me, I would split this into two layers, one for > > > the > > > register value <-> resistor value mapping, lets call it "rsel > > > table", > > > and the other for pin <-> "rsel table" mapping. > > > > > > I assume most if not all pins that support rsel would have the > > > same > > > set > > > of resistor values, so that would trade a level of indirection > > > for > > > better > > > usage of space. > > > > > > > /* struct mtk_pin_reg_calc - the structure that holds all > > > > ranges > > > > used to > > > > * determine which register the pin > > > > would > > > > make use of > > > > * for certain pin attribute. > > > > @@ -206,6 +237,9 @@ struct mtk_pin_soc { > > > > bool ies_present; > > > > const char * const *base_names; > > > > unsigned int nbase_names; > > > > + const unsigned int *pull_type; > > > > + const struct mtk_pin_rsel *pin_rsel; > > > > + unsigned int npin_rsel; > > > > > > > > /* Specific pinconfig operations */ > > > > int (*bias_disable_set)(struct mtk_pinctrl *hw, > > > > @@ -237,7 +271,6 @@ struct mtk_pin_soc { > > > > const struct mtk_pin_desc *desc, > > > > u32 > > > > arg); > > > > int (*adv_drive_get)(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc *desc, > > > > u32 > > > > *val); > > > > - > > > > > > Unrelated whitespace change. Please remove it. > > > > we will remove it in next version. > > > > > > > > > /* Specific driver data */ > > > > void *driver_data; > > > > }; > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > b/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > index 85db2e4377f0..8990cfe47d72 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > @@ -577,9 +577,9 @@ static int mtk_hw_get_value_wrap(struct > > > > mtk_pinctrl *hw, unsigned int gpio, int > > > > mtk_hw_get_value_wrap(hw, gpio, PINCTRL_PIN_REG_DRV) > > > > > > > > ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > > > > - unsigned int gpio, char *buf, unsigned int bufLen) > > > > + unsigned int gpio, char *buf, unsigned int buf_len) > > > > > > Unrelated change. > > > > we will sparate it in next version. > > > > > > > > > { > > > > - int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1; > > > > + int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1, > > > > rsel > > > > = -1; > > > > const struct mtk_pin_desc *desc; > > > > > > > > if (gpio >= hw->soc->npins) > > > > @@ -591,6 +591,8 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pinmux -= hw->soc->nfuncs; > > > > > > > > mtk_pinconf_bias_get_combo(hw, desc, &pullup, &pullen); > > > > + > > > > + /* Case for: R1R0 */ > > > > if (pullen == MTK_PUPD_SET_R1R0_00) { > > > > pullen = 0; > > > > r1 = 0; > > > > @@ -607,10 +609,16 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pullen = 1; > > > > r1 = 1; > > > > r0 = 1; > > > > - } else if (pullen != MTK_DISABLE && pullen != > > > > MTK_ENABLE) { > > > > - pullen = 0; > > > > } > > > > - len += scnprintf(buf + len, bufLen - len, > > > > + > > > > + /* Case for: RSEL */ > > > > + if (pullen >= MTK_PULL_SET_RSEL_000 && > > > > + pullen <= MTK_PULL_SET_RSEL_111) { > > > > + rsel = pullen - MTK_PULL_SET_RSEL_000; > > > > + pullen = 1; > > > > + } > > > > + > > > > + len += scnprintf(buf + len, buf_len - len, > > > > > > Unrelated change. > > > > it is used to get rsel debug changes > > > > > > > "%03d: %1d%1d%1d%1d%02d%1d%1d%1d%1d", > > > > gpio, > > > > pinmux, > > > > @@ -624,10 +632,12 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pullup); > > > > > > > > if (r1 != -1) { > > > > - len += scnprintf(buf + len, bufLen - len, " > > > > (%1d > > > > %1d)\n", > > > > + len += scnprintf(buf + len, buf_len - len, " > > > > (%1d > > > > %1d)\n", > > > > r1, r0); > > > > + } else if (rsel != -1) { > > > > + len += scnprintf(buf + len, buf_len - len, " > > > > (%1d)\n", rsel); > > > > } else { > > > > - len += scnprintf(buf + len, bufLen - len, > > > > "\n"); > > > > + len += scnprintf(buf + len, buf_len - len, > > > > "\n"); > > > > > > Unrelated changes. > > > > it is used to get rsel debug changes > > Sorry about the confusion. The unrelated change I meant to point out > is actually just the "bufLen" to "buf_len" change. > > > Regards > ChenYu Ok, I understand, we will separate it. > > > > > > > > > > > } > > > > > > > > return len; > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > b/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > index afb7650fd25b..681267c0e1a4 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > @@ -61,7 +61,7 @@ int mtk_paris_pinctrl_probe(struct > > > > platform_device *pdev, > > > > const struct mtk_pin_soc *soc); > > > > > > > > ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > > > > - unsigned int gpio, char *buf, unsigned int bufLen); > > > > + unsigned int gpio, char *buf, unsigned int buf_len); > > > > > > Unrelated change. > > > > > > > sparate it in next version. > > > > > > > > Regards > > > ChenYu > > > > > > > > > > > extern const struct dev_pm_ops mtk_paris_pinctrl_pm_ops; > > > > > > > > -- > > > > 2.18.0 > > > > _______________________________________________ > > > > Linux-mediatek mailing list > > > > Linux-mediatek@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatekwe > > > > will > > > > > > _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7CB37C433F5 for ; Thu, 9 Sep 2021 07:48:46 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 354C761132 for ; Thu, 9 Sep 2021 07:48:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 354C761132 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ebWPGteE+138y+rQ2X+8nAbHEgXsPlKo5+c/ZRMGqWM=; b=pBeQT7codOFCAH tUyfIgo7f6Arei68V0gQQWqoewV5eqsr6RnU8YDjM+jorgttHyRT0Ej+rvcBqeokR7Uh6pdpjNzks 4UmKH4lVMxxtuBHLgNwiK08y3YnaDlxlXV47MNnUNRL8lwrlOFI7nC4tUzUxG6Qw3MB/OiMUYeRr7 CCI5awTRI56z8YyPKyJP9DuR966UsH6Runw+J0uQiCmLDemqhOSxbohKrl54/EYqLwqgHYOlA2Eh6 dwkOd8fMacsoQAr7j0D0+K2IcpzSY/BJKst2kZsudG9x973/f3BSfqvC1uQpx6wk8HI75hOK/J5oE Ok7Y/506EX/7NMqf1PQw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOElH-008TSw-7h; Thu, 09 Sep 2021 07:46:19 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOElA-008TSJ-4Z; Thu, 09 Sep 2021 07:46:16 +0000 X-UUID: 3ca83b0a26154ab7a4045d3f8a6d5913-20210909 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=6OExwzly8ojabM5m+fp8KUyz7yZ3K6TerpWbxhH2yE8=; b=cQo5vKc8ORrKtxh/QsZh0tnlIOlgpfGsaoaP/+IPFuaR9bh2sO7XXPrauKwrS4h31/OO2u6wCdTiXKbHx+DGK5AS+4gMH/VnzHhrvgJalmhu7UlejMtCbPfBB25hBzQxmjNXV4/nxXQ25U48aVOQWsbYmnw8Oexa662m9pqVGj0=; X-UUID: 3ca83b0a26154ab7a4045d3f8a6d5913-20210909 Received: from mtkcas66.mediatek.inc [(172.29.193.44)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 36408918; Thu, 09 Sep 2021 00:46:08 -0700 Received: from MTKMBS07N2.mediatek.inc (172.21.101.141) by MTKMBS62N1.mediatek.inc (172.29.193.41) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 9 Sep 2021 00:46:03 -0700 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs07n2.mediatek.inc (172.21.101.141) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 9 Sep 2021 15:46:00 +0800 Received: from mhfsdcap04 (10.17.3.154) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Thu, 9 Sep 2021 15:45:59 +0800 Message-ID: <60c01a7fb76aea17211b204b228eacf928739689.camel@mediatek.com> Subject: Re: [PATCH v11 4/4] pinctrl: mediatek: add rsel setting on MT8195 From: zhiyong.tao To: Chen-Yu Tsai , Linus Walleij CC: Rob Herring , Mark Rutland , Matthias Brugger , Sean Wang , srv_heupstream , , "Eddie Huang" , Light Hsieh , Biao Huang , Hongzhou Yang , Sean Wang , Seiya Wang , Devicetree List , LKML , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , "moderated list:ARM/Mediatek SoC support" , "open list:GPIO SUBSYSTEM" Date: Thu, 9 Sep 2021 15:46:01 +0800 In-Reply-To: References: <20210830003603.31864-1-zhiyong.tao@mediatek.com> <20210830003603.31864-5-zhiyong.tao@mediatek.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210909_004612_271056_73188EB8 X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 2021-09-06 at 18:09 +0800, Chen-Yu Tsai wrote: > On Mon, Sep 6, 2021 at 11:17 AM zhiyong.tao > wrote: > > > > On Wed, 2021-09-01 at 18:10 +0800, Chen-Yu Tsai wrote: > > > Hi, > > > > > > On Mon, Aug 30, 2021 at 8:37 AM Zhiyong Tao < > > > zhiyong.tao@mediatek.com > > > > wrote: > > > > > > > > This patch provides rsel setting on MT8195 > > > > > > A bit more context, like what is rsel, would be nice. > > > > > > > we will add more context it in the next version. > > > > > > > > Signed-off-by: Zhiyong Tao > > > > --- > > > > drivers/pinctrl/mediatek/pinctrl-mt8195.c | 133 > > > > +++++++++++++ > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.c | 181 > > > > ++++++++++++++++-- > > > > .../pinctrl/mediatek/pinctrl-mtk-common-v2.h | 35 +++- > > > > drivers/pinctrl/mediatek/pinctrl-paris.c | 24 ++- > > > > drivers/pinctrl/mediatek/pinctrl-paris.h | 2 +- > > > > 5 files changed, 348 insertions(+), 27 deletions(-) > > > > > > Could you split this patch into two, so the common parts are in > > > the > > > first > > > patch, and the mt8195 specific changes are in the second? > > > > > > > ok, we will split in the next version. > > > > > > > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > b/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > index 892e79703f98..275b7ba4386d 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8195.c > > > > @@ -779,6 +779,135 @@ static const struct mtk_pin_field_calc > > > > mt8195_pin_drv_adv_range[] = { > > > > PIN_FIELD_BASE(45, 45, 1, 0x040, 0x10, 9, 3), > > > > }; > > > > > > > > +static const struct mtk_pin_field_calc mt8195_pin_rsel_range[] > > > > = { > > > > + PIN_FIELD_BASE(8, 8, 4, 0x0c0, 0x10, 15, 3), > > > > + PIN_FIELD_BASE(9, 9, 4, 0x0c0, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(10, 10, 4, 0x0c0, 0x10, 18, 3), > > > > + PIN_FIELD_BASE(11, 11, 4, 0x0c0, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(12, 12, 4, 0x0c0, 0x10, 21, 3), > > > > + PIN_FIELD_BASE(13, 13, 4, 0x0c0, 0x10, 6, 3), > > > > + PIN_FIELD_BASE(14, 14, 4, 0x0c0, 0x10, 24, 3), > > > > + PIN_FIELD_BASE(15, 15, 4, 0x0c0, 0x10, 9, 3), > > > > + PIN_FIELD_BASE(16, 16, 4, 0x0c0, 0x10, 27, 3), > > > > + PIN_FIELD_BASE(17, 17, 4, 0x0c0, 0x10, 12, 3), > > > > + PIN_FIELD_BASE(29, 29, 2, 0x080, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(30, 30, 2, 0x080, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(34, 34, 1, 0x0e0, 0x10, 0, 3), > > > > + PIN_FIELD_BASE(35, 35, 1, 0x0e0, 0x10, 3, 3), > > > > + PIN_FIELD_BASE(44, 44, 1, 0x0e0, 0x10, 6, 3), > > > > + PIN_FIELD_BASE(45, 45, 1, 0x0e0, 0x10, 9, 3), > > > > +}; > > > > + > > > > +static const struct mtk_pin_rsel mt8195_pin_rsel_val_range[] = > > > > { > > > > + PIN_RSEL(8, 17, 0x0, 75000, 75000), > > > > + PIN_RSEL(8, 17, 0x1, 10000, 5000), > > > > + PIN_RSEL(8, 17, 0x2, 5000, 75000), > > > > + PIN_RSEL(8, 17, 0x3, 4000, 5000), > > > > + PIN_RSEL(8, 17, 0x4, 3000, 75000), > > > > + PIN_RSEL(8, 17, 0x5, 2000, 5000), > > > > + PIN_RSEL(8, 17, 0x6, 1500, 75000), > > > > + PIN_RSEL(8, 17, 0x7, 1000, 5000), > > > > + PIN_RSEL(29, 30, 0x0, 75000, 75000), > > > > + PIN_RSEL(29, 30, 0x1, 10000, 5000), > > > > + PIN_RSEL(29, 30, 0x2, 5000, 75000), > > > > + PIN_RSEL(29, 30, 0x3, 4000, 5000), > > > > + PIN_RSEL(29, 30, 0x4, 3000, 75000), > > > > + PIN_RSEL(29, 30, 0x5, 2000, 5000), > > > > + PIN_RSEL(29, 30, 0x6, 1500, 75000), > > > > + PIN_RSEL(29, 30, 0x7, 1000, 5000), > > > > + PIN_RSEL(34, 35, 0x0, 75000, 75000), > > > > + PIN_RSEL(34, 35, 0x1, 10000, 5000), > > > > + PIN_RSEL(34, 35, 0x2, 5000, 75000), > > > > + PIN_RSEL(34, 35, 0x3, 4000, 5000), > > > > + PIN_RSEL(34, 35, 0x4, 3000, 75000), > > > > + PIN_RSEL(34, 35, 0x5, 2000, 5000), > > > > + PIN_RSEL(34, 35, 0x6, 1500, 75000), > > > > + PIN_RSEL(34, 35, 0x7, 1000, 5000), > > > > + PIN_RSEL(44, 45, 0x0, 75000, 75000), > > > > + PIN_RSEL(44, 45, 0x1, 10000, 5000), > > > > + PIN_RSEL(44, 45, 0x2, 5000, 75000), > > > > + PIN_RSEL(44, 45, 0x3, 4000, 5000), > > > > + PIN_RSEL(44, 45, 0x4, 3000, 75000), > > > > + PIN_RSEL(44, 45, 0x5, 2000, 5000), > > > > + PIN_RSEL(44, 45, 0x6, 1500, 75000), > > > > + PIN_RSEL(44, 45, 0x7, 1000, 5000), > > > > +}; > > > > + > > > > +static const unsigned int mt8195_pull_type[] = { > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 0 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 1 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 2 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 3 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 4 */, > > > > MTK_PULL_PUPD_R1R0_TYPE /* > > > > 5 */, > > > > + MTK_PULL_PU_PD_TYPE /* 6 */, MTK_PULL_PU_PD_TYPE /* 7 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 8 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 9 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 10 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 11 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 12 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 13 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 14 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 15 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 16 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 17 */, > > > > + MTK_PULL_PU_PD_TYPE /* 18 */, MTK_PULL_PU_PD_TYPE /* 19 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 20 */, MTK_PULL_PU_PD_TYPE /* 21 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 22 */, MTK_PULL_PU_PD_TYPE /* 23 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 24 */, MTK_PULL_PU_PD_TYPE /* 25 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 26 */, MTK_PULL_PU_PD_TYPE /* 27 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 28 */, MTK_PULL_PU_PD_RSEL_TYPE > > > > /* > > > > 29 */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 30 */, MTK_PULL_PU_PD_TYPE > > > > /* > > > > 31 */, > > > > + MTK_PULL_PU_PD_TYPE /* 32 */, MTK_PULL_PU_PD_TYPE /* 33 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 34 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 35 */, > > > > + MTK_PULL_PU_PD_TYPE /* 36 */, MTK_PULL_PU_PD_TYPE /* 37 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 38 */, MTK_PULL_PU_PD_TYPE /* 39 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 40 */, MTK_PULL_PU_PD_TYPE /* 41 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 42 */, MTK_PULL_PU_PD_TYPE /* 43 > > > > */, > > > > + MTK_PULL_PU_PD_RSEL_TYPE /* 44 */, > > > > MTK_PULL_PU_PD_RSEL_TYPE > > > > /* 45 */, > > > > + MTK_PULL_PU_PD_TYPE /* 46 */, MTK_PULL_PU_PD_TYPE /* 47 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 48 */, MTK_PULL_PU_PD_TYPE /* 49 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 50 */, MTK_PULL_PU_PD_TYPE /* 51 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 52 */, MTK_PULL_PU_PD_TYPE /* 53 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 54 */, MTK_PULL_PU_PD_TYPE /* 55 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 56 */, MTK_PULL_PU_PD_TYPE /* 57 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 58 */, MTK_PULL_PU_PD_TYPE /* 59 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 60 */, MTK_PULL_PU_PD_TYPE /* 61 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 62 */, MTK_PULL_PU_PD_TYPE /* 63 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 64 */, MTK_PULL_PU_PD_TYPE /* 65 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 66 */, MTK_PULL_PU_PD_TYPE /* 67 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 68 */, MTK_PULL_PU_PD_TYPE /* 69 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 70 */, MTK_PULL_PU_PD_TYPE /* 71 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 72 */, MTK_PULL_PU_PD_TYPE /* 73 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 74 */, MTK_PULL_PU_PD_TYPE /* 75 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 76 */, MTK_PULL_PUPD_R1R0_TYPE > > > > /* 77 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 78 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 79 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 80 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 81 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 82 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 83 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 84 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 85 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 86 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 87 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 88 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 89 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 90 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 91 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 92 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 93 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 94 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 95 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 96 */, MTK_PULL_PU_PD_TYPE > > > > /* 97 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 98 */, MTK_PULL_PU_PD_TYPE /* 99 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 100 */, MTK_PULL_PU_PD_TYPE /* > > > > 101 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 102 */, MTK_PULL_PU_PD_TYPE /* > > > > 103 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 104 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 105 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 106 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 107 */, > > > > + MTK_PULL_PU_PD_TYPE /* 108 */, MTK_PULL_PU_PD_TYPE /* > > > > 109 > > > > */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 110 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 111 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 112 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 113 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 114 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 115 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 116 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 117 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 118 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 119 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 120 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 121 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 122 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 123 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 124 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 125 */, > > > > + MTK_PULL_PUPD_R1R0_TYPE /* 126 */, > > > > MTK_PULL_PUPD_R1R0_TYPE > > > > /* 127 */, > > > > + MTK_PULL_PU_PD_TYPE /* 128 */, MTK_PULL_PU_PD_TYPE /* > > > > 129 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 130 */, MTK_PULL_PU_PD_TYPE /* > > > > 131 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 132 */, MTK_PULL_PU_PD_TYPE /* > > > > 133 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 134 */, MTK_PULL_PU_PD_TYPE /* > > > > 135 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 136 */, MTK_PULL_PU_PD_TYPE /* > > > > 137 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 138 */, MTK_PULL_PU_PD_TYPE /* > > > > 139 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 140 */, MTK_PULL_PU_PD_TYPE /* > > > > 141 > > > > */, > > > > + MTK_PULL_PU_PD_TYPE /* 142 */, MTK_PULL_PU_PD_TYPE /* > > > > 143 > > > > */, > > > > +}; > > > > + > > > > static const struct mtk_pin_reg_calc > > > > mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { > > > > [PINCTRL_PIN_REG_MODE] = > > > > MTK_RANGE(mt8195_pin_mode_range), > > > > [PINCTRL_PIN_REG_DIR] = > > > > MTK_RANGE(mt8195_pin_dir_range), > > > > @@ -793,6 +922,7 @@ static const struct mtk_pin_reg_calc > > > > mt8195_reg_cals[PINCTRL_PIN_REG_MAX] = { > > > > [PINCTRL_PIN_REG_R0] = MTK_RANGE(mt8195_pin_r0_range), > > > > [PINCTRL_PIN_REG_R1] = MTK_RANGE(mt8195_pin_r1_range), > > > > [PINCTRL_PIN_REG_DRV_ADV] = > > > > MTK_RANGE(mt8195_pin_drv_adv_range), > > > > + [PINCTRL_PIN_REG_RSEL] = > > > > MTK_RANGE(mt8195_pin_rsel_range), > > > > }; > > > > > > > > static const char * const mt8195_pinctrl_register_base_names[] > > > > = { > > > > @@ -817,6 +947,9 @@ static const struct mtk_pin_soc mt8195_data > > > > = { > > > > .gpio_m = 0, > > > > .base_names = mt8195_pinctrl_register_base_names, > > > > .nbase_names = > > > > ARRAY_SIZE(mt8195_pinctrl_register_base_names), > > > > + .pull_type = mt8195_pull_type, > > > > + .pin_rsel = mt8195_pin_rsel_val_range, > > > > + .npin_rsel = ARRAY_SIZE(mt8195_pin_rsel_val_range), > > > > .bias_set_combo = mtk_pinconf_bias_set_combo, > > > > .bias_get_combo = mtk_pinconf_bias_get_combo, > > > > .drive_set = mtk_pinconf_drive_set_rev1, > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > index 5b3b048725cc..b6a89d41f040 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c > > > > @@ -641,6 +641,9 @@ static int > > > > mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > } else if (arg == MTK_PUPD_SET_R1R0_11) { > > > > r0 = 1; > > > > r1 = 1; > > > > + } else if (arg == MTK_ENABLE) { > > > > + r0 = 1; > > > > + r1 = 0; > > > > > > This change does not seem related. This should be in a separate > > > patch > > > and why and what this change is should be described in the commit > > > log. > > > I assume this is to support "bias-pull-up" or "bias-pull-down" > > > without > > > arguments on pins using R1R0? > > > > we will separate it in next version. > > some users want only pull up/down, they don't care about the > > resistance > > value. So we support the case which only use "bias-pull-up" or > > "bias- > > pull-down" without arguments on pins using R1R0. > > I see. This should be added as a separate patch before the RSEL stuff > then. That way it would also be easier to backport in case it is > needed. > ok, we will split in the next version. > > > > > > > } else { > > > > err = -EINVAL; > > > > goto out; > > > > @@ -661,6 +664,75 @@ static int > > > > mtk_pinconf_bias_set_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > return err; > > > > } > > > > > > > > +static int mtk_hw_pin_rsel_lookup(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 pullup, u32 arg, u32 > > > > *rsel_val) > > > > +{ > > > > + const struct mtk_pin_rsel *rsel; > > > > + int check; > > > > + bool found = false; > > > > + > > > > + rsel = hw->soc->pin_rsel; > > > > + > > > > + for (check = 0; check <= hw->soc->npin_rsel - 1; > > > > check++) { > > > > + if (desc->number >= rsel[check].s_pin && > > > > + desc->number <= rsel[check].e_pin) { > > > > + if (pullup) { > > > > + if (rsel[check].up_rsel == arg) > > > > { > > > > + found = true; > > > > + *rsel_val = > > > > rsel[check].rsel_index; > > > > + break; > > > > + } > > > > + } else { > > > > + if (rsel[check].down_rsel == > > > > arg) { > > > > + found = true; > > > > + *rsel_val = > > > > rsel[check].rsel_index; > > > > + break; > > > > + } > > > > + } > > > > + } > > > > + } > > > > + > > > > + if (!found) { > > > > + dev_err(hw->dev, "Not support rsel value %d Ohm > > > > for > > > > pin = %d (%s)\n", > > > > + arg, desc->number, desc->name); > > > > + return -EOPNOTSUPP; > > > > > > I believe ENOTSUPP is the correct one. EOPNOTSUPP seems to be > > > network > > > related. > > > > > > > if we change it as "ENOTSUPP", it will report warning"ENOTSUPP is > > not > > SUSV4 error code, prefer EOPNOTSUP" when checking patch. > > The context surrounding this warning seems to be that ENOTSUPP is > hard > for userspace to understand. AFAIK the return code here does not get > passed to userspace? And the pinctrl core does check for EINVAL or > ENOTSUPP, so I think this is a valid use case. > > Linus? > > > > I also think it would make sense to differentiate between the pin > > > not > > > supporting RSEL (return -ENOTSUPP) vs supporting RSEL but a > > > resistor > > > value not in the table was requested (return -EINVAL). This might > > > be > > > easier to do with the two-table design I mentioned. > > > > It only support resistor value on the patch on mt8195. we think we > > need > > only maintain one-table design. > > OK. I think there's a possibility that some corner case might happen. > I guess we'll deal with them if they occur. > > > > > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int mtk_pinconf_bias_set_rsel(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 pullup, u32 arg) > > > > +{ > > > > + int err, rsel_val; > > > > + > > > > + if (hw->soc->pin_rsel) { > > > > + /* find pin rsel_index from pin_rsel array*/ > > > > + err = mtk_hw_pin_rsel_lookup(hw, desc, pullup, > > > > arg, > > > > &rsel_val); > > > > + if (err) > > > > + goto out; > > > > + } else { > > > > + if (arg < MTK_PULL_SET_RSEL_000 || > > > > + arg > MTK_PULL_SET_RSEL_111) { > > > > + err = -EINVAL; > > > > + goto out; > > > > + } > > > > + > > > > + rsel_val -= MTK_PULL_SET_RSEL_000; > > > > + } > > > > > > This is not structured correctly. Think about this: on a chip, > > > say > > > MT8195, > > > which has hw->soc->pin_rsel defined, but the device tree writer > > > chose > > > to use the macros instead of SI unit values. This is exactly the > > > scenario > > > you discussed privately with me. > > > > > > This also fails the case where bias-pull-{up,down} is specified > > > without > > > an argument, in which case arg == 1. This does seem to be covered > > > by > > > falling back to just PU/PD, but it's not exactly obvious, > > > especially > > > since we are skipping mtk_pinconf_bias_set_pu_pd() just below. > > > > > > So you actually need to check for the magic values first, and if > > > arg > > > doesn't match any of them, assume an SI unit argument and try to > > > do > > > a lookup. > > > > > > > one very important point of upstream is keeping backward > > compatibility, > > since almost all customers of other MTK platforms does not care > > about > > resistor values so we leave user to choose MTK_PULL_SET_RSEL_XXX > > without necessity to know resistor value. > > I'm not sure about the connection to backward compatibility here, > given > that you are just introducing support for RSEL. > > > Our policy is making most customers/users convenient. > > Encourage of using SI unit is not in concern. > > > > For mt8195, we choose "hw->soc->pin_rsel", so it can support SI > > unit. > > Regarding my concern, there are a lot of moving pieces here, so let's > see > how the next revision turns out and if this is still an issue. > > > > > + > > > > + err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_RSEL, > > > > rsel_val); > > > > + if (err) > > > > + goto out; > > > > + > > > > + err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, > > > > MTK_ENABLE); > > > > + > > > > +out: > > > > + return err; > > > > +} > > > > + > > > > static int mtk_pinconf_bias_get_pu_pd(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc > > > > *desc, > > > > u32 *pullup, u32 *enable) > > > > @@ -742,44 +814,117 @@ static int > > > > mtk_pinconf_bias_get_pupd_r1_r0(struct mtk_pinctrl *hw, > > > > > > This hunk is somewhat whacky. I assume it's because you also > > > fixed up > > > the indentation for mtk_pinconf_bias_set_combo(). While nice to > > > do, > > > in this case it actually works against having an easy to read > > > patch. > > > > > > > yes, it's because to fix up > > the indentation for mtk_pinconf_bias_set_combo(). > > I suggest moving that to a separate patch. While whitespace changes > are > sometimes frowned upon, I think this is OK given that it would > improve > readability of the patches following it. You should mention that in > the > commit message to justify the change. > ok, we will split in the next version. > > > > return err; > > > > } > > > > > > > > -int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, > > > > - const struct mtk_pin_desc > > > > *desc, > > > > - u32 pullup, u32 arg) > > > > +static int mtk_pinconf_bias_get_rsel(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc > > > > *desc, > > > > + u32 *pullup, u32 *enable) > > > > { > > > > - int err; > > > > + int pu, pd, rsel, err; > > > > > > > > - err = mtk_pinconf_bias_set_pu_pd(hw, desc, pullup, > > > > arg); > > > > - if (!err) > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_RSEL, > > > > &rsel); > > > > + if (err) > > > > goto out; > > > > > > > > - err = mtk_pinconf_bias_set_pullsel_pullen(hw, desc, > > > > pullup, > > > > arg); > > > > - if (!err) > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PU, > > > > &pu); > > > > + if (err) > > > > goto out; > > > > > > > > - err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, pullup, > > > > arg); > > > > + err = mtk_hw_get_value(hw, desc, PINCTRL_PIN_REG_PD, > > > > &pd); > > > > > > Maybe this could reuse mtk_pinconf_bias_get_pu_pd(), much like on > > > the > > > `set` > > > side? > > > > > > > we will try to reuse mtk_pinconf_bias_get_pu_pd(). > > > > > > + > > > > + if (pu == 0 && pd == 0) { > > > > + *pullup = 0; > > > > + *enable = MTK_DISABLE; > > > > + } else if (pu == 1 && pd == 0) { > > > > + *pullup = 1; > > > > + *enable = rsel + MTK_PULL_SET_RSEL_000; > > > > + } else if (pu == 0 && pd == 1) { > > > > + *pullup = 0; > > > > + *enable = rsel + MTK_PULL_SET_RSEL_000; > > > > + } else { > > > > + err = -EINVAL; > > > > + goto out; > > > > + } > > > > > > > > out: > > > > return err; > > > > } > > > > + > > > > +int mtk_pinconf_bias_set_combo(struct mtk_pinctrl *hw, > > > > + const struct mtk_pin_desc *desc, > > > > + u32 pullup, u32 arg) > > > > +{ > > > > + int err = -EOPNOTSUPP; > > > > + bool try_all_type; > > > > + > > > > + try_all_type = hw->soc->pull_type ? false : true; > > > > > > Using ternary ops is discouraged. Maybe this could be structured > > > like: > > > > > > u32 pull_type; > > > > > > if (hw->soc->pull_type) > > > pull_type = hw->soc->pull_type[desc->number]; > > > else > > > pull_type = MTK_PULL_TYPE_MASK; > > > > > > That way you don't need to test `try_all_type` every time. > > > > we will try to change it in next version. > > > > > > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_RSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_set_rsel(hw, desc, > > > > pullup, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PU_PD_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pu_pd(hw, desc, > > > > pullup, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PULLSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pullsel_pullen(hw, > > > > desc, > > > > + pullu > > > > p, > > > > arg); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PUPD_R1R0_TYPE)) { > > > > + err = mtk_pinconf_bias_set_pupd_r1_r0(hw, desc, > > > > pullup, arg); > > > > + if (err) > > > > + dev_err(hw->dev, "Invalid pull > > > > argument\n"); > > > > > > The "if (err)" could be moved outside the upper "if" block. That > > > way > > > the code flow looks more consistent, plus we get an error message > > > regardless of the pull type supported. > > > > we will move "if (err)" outside the upper "if" block in the next > > version. > > > > > > > + } > > > > + > > > > + return err; > > > > +} > > > > EXPORT_SYMBOL_GPL(mtk_pinconf_bias_set_combo); > > > > > > > > int mtk_pinconf_bias_get_combo(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc *desc, > > > > u32 *pullup, u32 *enable) > > > > { > > > > - int err; > > > > + int err = -EOPNOTSUPP; > > > > + bool try_all_type; > > > > > > > > - err = mtk_pinconf_bias_get_pu_pd(hw, desc, pullup, > > > > enable); > > > > - if (!err) > > > > - goto out; > > > > + try_all_type = hw->soc->pull_type ? false : true; > > > > > > Same here. > > > > > > > we will try to change it in next version. > > > > > > > > > > - err = mtk_pinconf_bias_get_pullsel_pullen(hw, desc, > > > > pullup, > > > > enable); > > > > - if (!err) > > > > - goto out; > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_RSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_get_rsel(hw, desc, > > > > pullup, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PU_PD_TYPE)) { > > > > + err = mtk_pinconf_bias_get_pu_pd(hw, desc, > > > > pullup, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > > > > > - err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, pullup, > > > > enable); > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PULLSEL_TYPE)) { > > > > + err = mtk_pinconf_bias_get_pullsel_pullen(hw, > > > > desc, > > > > + pullu > > > > p, > > > > enable); > > > > + if (!err) > > > > + return err; > > > > + } > > > > + > > > > + if (try_all_type || > > > > + (hw->soc->pull_type[desc->number] & > > > > MTK_PULL_PUPD_R1R0_TYPE)) > > > > + err = mtk_pinconf_bias_get_pupd_r1_r0(hw, desc, > > > > pullup, enable); > > > > > > > > -out: > > > > return err; > > > > } > > > > EXPORT_SYMBOL_GPL(mtk_pinconf_bias_get_combo); > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > index a6f1bdb2083b..a3d73f153efe 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h > > > > @@ -17,6 +17,13 @@ > > > > #define MTK_ENABLE 1 > > > > #define MTK_PULLDOWN 0 > > > > #define MTK_PULLUP 1 > > > > +#define MTK_PULL_PU_PD_TYPE BIT(0) > > > > +#define MTK_PULL_PULLSEL_TYPE BIT(1) > > > > +#define MTK_PULL_PUPD_R1R0_TYPE BIT(2) > > > > +#define MTK_PULL_RSEL_TYPE BIT(3) > > > > +#define MTK_PULL_PU_PD_RSEL_TYPE (MTK_PULL_PU_PD_TYPE \ > > > > + | MTK_PULL_RSEL_TYPE) > > > > > > MTK_PULL_RSEL_TYPE deserves a comment saying why it should not be > > > used > > > on its own. > > > > > > But since mtk_pinconf_bias_set_rsel() is already calling > > > mtk_pinconf_bias_set_pu_pd(), if the former function is reworked, > > > then > > > the two types don't need to be combined for the fallback > > > behavior. > > > > > > > > > MTK_PULL_PU_PD_RSEL_TYPE is type wihch is can be control by > > MTK_PULL_PU_PD_TYPE or MTK_PULL_RSEL_TYPE. so we combibe them > > together. > > But can RSEL be used without PU/PD? The definitions read like > "PU_PD_RSEL" > means either RSEL or PU/PD can be used, but if I understand the > hardware > correctly, RSEL by itself only selects the resistor value, but > doesn't > turn on/off the bias nor select pull up or down. So RSEL is an > extension > of PU/PD, and by itself won't have any real effect. > yes, it can select the resister value and can be turn on/off itself. But it can't select pull up/down. > Since this could be interpreted either way, a comment explaining > things > should be added to document the author's intentions. > We will add a comment explaining it here. > > > > #define EINT_NA U16_MAX > > > > #define NO_EINT_SUPPORT EINT_NA > > > > @@ -42,6 +49,14 @@ > > > > PIN_FIELD_CALC(_s_pin, _e_pin, 0, _s_addr, _x_addrs, > > > > _s_bit, \ > > > > _x_bits, 32, 1) > > > > > > > > +#define PIN_RSEL(_s_pin, _e_pin, _rsel_index, _up_resl, > > > > _down_rsel) { \ > > > > > > ^ rsel? > > > > > > > + .s_pin = > > > > _s_pin, \ > > > > + .e_pin = > > > > _e_pin, \ > > > > + .rsel_index = > > > > _rsel_index, \ > > > > + .up_rsel = > > > > _up_resl, \ > > > > + .down_rsel = > > > > _down_rsel, \ > > > > + } > > > > + > > > > /* List these attributes which could be modified for the pin > > > > */ > > > > enum { > > > > PINCTRL_PIN_REG_MODE, > > > > @@ -67,6 +82,7 @@ enum { > > > > PINCTRL_PIN_REG_DRV_E0, > > > > PINCTRL_PIN_REG_DRV_E1, > > > > PINCTRL_PIN_REG_DRV_ADV, > > > > + PINCTRL_PIN_REG_RSEL, > > > > PINCTRL_PIN_REG_MAX, > > > > }; > > > > > > > > @@ -129,6 +145,21 @@ struct mtk_pin_field_calc { > > > > u8 fixed; > > > > }; > > > > > > > > +/* struct mtk_pin_rsel - the structure that providing bias > > > > resistance selection. > > > > > > ^ provides > > > > we will change it in next version. > > > > > > > > > > > + * @s_pin: the start pin within the rsel range > > > > + * @e_pin: the end pin within the rsel range > > > > + * @rsel_index: the rsel bias resistance index > > > > + * @up_rsel: the pullup rsel bias resistance value > > > > + * @down_rsel: the pulldown rsel bias resistance value > > > > + */ > > > > +struct mtk_pin_rsel { > > > > + u16 s_pin; > > > > + u16 e_pin; > > > > + u16 rsel_index; > > > > + u32 up_rsel; > > > > + u32 down_rsel; > > > > +}; > > > > + > > > > > > If it were up to me, I would split this into two layers, one for > > > the > > > register value <-> resistor value mapping, lets call it "rsel > > > table", > > > and the other for pin <-> "rsel table" mapping. > > > > > > I assume most if not all pins that support rsel would have the > > > same > > > set > > > of resistor values, so that would trade a level of indirection > > > for > > > better > > > usage of space. > > > > > > > /* struct mtk_pin_reg_calc - the structure that holds all > > > > ranges > > > > used to > > > > * determine which register the pin > > > > would > > > > make use of > > > > * for certain pin attribute. > > > > @@ -206,6 +237,9 @@ struct mtk_pin_soc { > > > > bool ies_present; > > > > const char * const *base_names; > > > > unsigned int nbase_names; > > > > + const unsigned int *pull_type; > > > > + const struct mtk_pin_rsel *pin_rsel; > > > > + unsigned int npin_rsel; > > > > > > > > /* Specific pinconfig operations */ > > > > int (*bias_disable_set)(struct mtk_pinctrl *hw, > > > > @@ -237,7 +271,6 @@ struct mtk_pin_soc { > > > > const struct mtk_pin_desc *desc, > > > > u32 > > > > arg); > > > > int (*adv_drive_get)(struct mtk_pinctrl *hw, > > > > const struct mtk_pin_desc *desc, > > > > u32 > > > > *val); > > > > - > > > > > > Unrelated whitespace change. Please remove it. > > > > we will remove it in next version. > > > > > > > > > /* Specific driver data */ > > > > void *driver_data; > > > > }; > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > b/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > index 85db2e4377f0..8990cfe47d72 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c > > > > @@ -577,9 +577,9 @@ static int mtk_hw_get_value_wrap(struct > > > > mtk_pinctrl *hw, unsigned int gpio, int > > > > mtk_hw_get_value_wrap(hw, gpio, PINCTRL_PIN_REG_DRV) > > > > > > > > ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > > > > - unsigned int gpio, char *buf, unsigned int bufLen) > > > > + unsigned int gpio, char *buf, unsigned int buf_len) > > > > > > Unrelated change. > > > > we will sparate it in next version. > > > > > > > > > { > > > > - int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1; > > > > + int pinmux, pullup, pullen, len = 0, r1 = -1, r0 = -1, > > > > rsel > > > > = -1; > > > > const struct mtk_pin_desc *desc; > > > > > > > > if (gpio >= hw->soc->npins) > > > > @@ -591,6 +591,8 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pinmux -= hw->soc->nfuncs; > > > > > > > > mtk_pinconf_bias_get_combo(hw, desc, &pullup, &pullen); > > > > + > > > > + /* Case for: R1R0 */ > > > > if (pullen == MTK_PUPD_SET_R1R0_00) { > > > > pullen = 0; > > > > r1 = 0; > > > > @@ -607,10 +609,16 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pullen = 1; > > > > r1 = 1; > > > > r0 = 1; > > > > - } else if (pullen != MTK_DISABLE && pullen != > > > > MTK_ENABLE) { > > > > - pullen = 0; > > > > } > > > > - len += scnprintf(buf + len, bufLen - len, > > > > + > > > > + /* Case for: RSEL */ > > > > + if (pullen >= MTK_PULL_SET_RSEL_000 && > > > > + pullen <= MTK_PULL_SET_RSEL_111) { > > > > + rsel = pullen - MTK_PULL_SET_RSEL_000; > > > > + pullen = 1; > > > > + } > > > > + > > > > + len += scnprintf(buf + len, buf_len - len, > > > > > > Unrelated change. > > > > it is used to get rsel debug changes > > > > > > > "%03d: %1d%1d%1d%1d%02d%1d%1d%1d%1d", > > > > gpio, > > > > pinmux, > > > > @@ -624,10 +632,12 @@ ssize_t mtk_pctrl_show_one_pin(struct > > > > mtk_pinctrl *hw, > > > > pullup); > > > > > > > > if (r1 != -1) { > > > > - len += scnprintf(buf + len, bufLen - len, " > > > > (%1d > > > > %1d)\n", > > > > + len += scnprintf(buf + len, buf_len - len, " > > > > (%1d > > > > %1d)\n", > > > > r1, r0); > > > > + } else if (rsel != -1) { > > > > + len += scnprintf(buf + len, buf_len - len, " > > > > (%1d)\n", rsel); > > > > } else { > > > > - len += scnprintf(buf + len, bufLen - len, > > > > "\n"); > > > > + len += scnprintf(buf + len, buf_len - len, > > > > "\n"); > > > > > > Unrelated changes. > > > > it is used to get rsel debug changes > > Sorry about the confusion. The unrelated change I meant to point out > is actually just the "bufLen" to "buf_len" change. > > > Regards > ChenYu Ok, I understand, we will separate it. > > > > > > > > > > > } > > > > > > > > return len; > > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > b/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > index afb7650fd25b..681267c0e1a4 100644 > > > > --- a/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.h > > > > @@ -61,7 +61,7 @@ int mtk_paris_pinctrl_probe(struct > > > > platform_device *pdev, > > > > const struct mtk_pin_soc *soc); > > > > > > > > ssize_t mtk_pctrl_show_one_pin(struct mtk_pinctrl *hw, > > > > - unsigned int gpio, char *buf, unsigned int bufLen); > > > > + unsigned int gpio, char *buf, unsigned int buf_len); > > > > > > Unrelated change. > > > > > > > sparate it in next version. > > > > > > > > Regards > > > ChenYu > > > > > > > > > > > extern const struct dev_pm_ops mtk_paris_pinctrl_pm_ops; > > > > > > > > -- > > > > 2.18.0 > > > > _______________________________________________ > > > > Linux-mediatek mailing list > > > > Linux-mediatek@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-mediatekwe > > > > will > > > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel