Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753463AbdFSCH7 (ORCPT ); Sun, 18 Jun 2017 22:07:59 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:60644 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753241AbdFSCH4 (ORCPT ); Sun, 18 Jun 2017 22:07:56 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 4CFD06084E Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=fenglinw@codeaurora.org Subject: Re: [PATCH V1 1/3] pinctrl: qcom: spmi-gpio: Add support for GPIO LV/MV subtype From: Fenglin Wu To: Rob Herring Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, Linus Walleij , Mark Rutland , Andy Gross , David Brown , Srinivas Kandagatla , linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-soc@vger.kernel.org, collinsd@codeaurora.org, aghayal@codeaurora.org, wruan@codeaurora.org, kgunda@codeaurora.org References: <20170613061707.13892-1-fenglinw@codeaurora.org> <20170613061707.13892-2-fenglinw@codeaurora.org> <20170618140456.orzknmxg4fpce4tk@rob-hp-laptop> <11ad6f89-6805-a09f-eee4-21279b096e8b@codeaurora.org> Message-ID: <1748fcd1-b133-c675-0e14-b0e1227fa5f8@codeaurora.org> Date: Mon, 19 Jun 2017 10:07:49 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <11ad6f89-6805-a09f-eee4-21279b096e8b@codeaurora.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3834 Lines: 93 On 6/19/2017 9:00 AM, Fenglin Wu wrote: > On 6/18/2017 10:04 PM, Rob Herring wrote: >> On Tue, Jun 13, 2017 at 02:16:03PM +0800, fenglinw@codeaurora.org wrote: >>> From: Fenglin Wu >>> >>> GPIO LV (low voltage)/MV (medium voltage) subtypes have different >>> features and register mappings than 4CH/8CH subtypes. Add support >>> for LV and MV subtypes. >>> >>> Signed-off-by: Fenglin Wu >>> --- >>> .../devicetree/bindings/pinctrl/qcom,pmic-gpio.txt | 28 ++- >>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 269 >>> ++++++++++++++++++--- >>> include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 9 + >>> 3 files changed, 264 insertions(+), 42 deletions(-) >> >> [...] >> >>> diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h >>> b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h >>> index d33f17c..85d8809 100644 >>> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h >>> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h >>> @@ -93,15 +93,24 @@ >>> #define PM8994_GPIO_S4 2 >>> #define PM8994_GPIO_L12 3 >>> +/* ATEST MUX selection for analog-pass-through mode */ >>> +#define PMIC_GPIO_AOUT_ATEST1 0 >>> +#define PMIC_GPIO_AOUT_ATEST2 1 >>> +#define PMIC_GPIO_AOUT_ATEST3 2 >>> +#define PMIC_GPIO_AOUT_ATEST4 3 >>> + >>> /* To be used with "function" */ >>> #define PMIC_GPIO_FUNC_NORMAL "normal" >>> #define PMIC_GPIO_FUNC_PAIRED "paired" >>> #define PMIC_GPIO_FUNC_FUNC1 "func1" >>> #define PMIC_GPIO_FUNC_FUNC2 "func2" >>> +#define PMIC_GPIO_FUNC_FUNC3 "func3" >>> +#define PMIC_GPIO_FUNC_FUNC4 "func4" >>> #define PMIC_GPIO_FUNC_DTEST1 "dtest1" >>> #define PMIC_GPIO_FUNC_DTEST2 "dtest2" >>> #define PMIC_GPIO_FUNC_DTEST3 "dtest3" >>> #define PMIC_GPIO_FUNC_DTEST4 "dtest4" >>> +#define PMIC_GPIO_FUNC_ANALOG "analog" >> >> defines for strings? That's really pointless. I'd prefer you drop using >> them than add more. >> > Thanks for the suggestion, I will remove these string definitions in > next patch. > Does other part look good? I would post a new patch if no other comments. > Sorry, I hadn't noticed there are so many definitions depend on them. I take my word back and I think it deserves more discussion. The function names "func1/func2/func3/func4" defined for GPIO hardware module are very generic. Each GPIO located in different PMICs would have its special function according to different hardware design, such as: batt_alarm, keypad_drv, div_clk, etc. The dt-binding header file defines the PMIC GPIOs' special functions which depending on these string definitions (samples list below). This gives a good understanding to end user if they requires to set the GPIO special function but not having a hardware specification to explain the details. If we remove these string definitions, we would have another place to explain these mapping of "funcx" to any GPIOs' special functions in each PMICs, would dt-binding document be a good place to have them? #define PM8038_GPIO1_2_LPG_DRV PMIC_GPIO_FUNC_FUNC1 #define PM8038_GPIO3_5V_BOOST_EN PMIC_GPIO_FUNC_FUNC1 #define PM8038_GPIO4_SSBI_ALT_CLK PMIC_GPIO_FUNC_FUNC1 #define PM8038_GPIO5_6_EXT_REG_EN PMIC_GPIO_FUNC_FUNC1 #define PM8038_GPIO10_11_EXT_REG_EN PMIC_GPIO_FUNC_FUNC1 #define PM8038_GPIO6_7_CLK PMIC_GPIO_FUNC_FUNC1 ... >> Rob >> -- >> To unsubscribe from this list: send the line "unsubscribe >> linux-arm-msm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.