Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933986AbaGWXri (ORCPT ); Wed, 23 Jul 2014 19:47:38 -0400 Received: from smtp.codeaurora.org ([198.145.11.231]:38906 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758548AbaGWXrg (ORCPT ); Wed, 23 Jul 2014 19:47:36 -0400 Message-ID: <53D04996.2030902@codeaurora.org> Date: Wed, 23 Jul 2014 16:47:34 -0700 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Bjorn Andersson CC: "Ivan T. Ivanov" , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Linus Walleij , Mark Brown , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-msm@vger.kernel.org" Subject: Re: [PATCH RESEND v2 1/4] pinctrl: Update Qualcomm pm8xxx GPIO parameters definitions References: <1405610748-7583-5-git-send-email-iivanov@mm-sol.com> <1405626085-14069-1-git-send-email-iivanov@mm-sol.com> <20140722214643.GH19700@sonymobile.com> In-Reply-To: <20140722214643.GH19700@sonymobile.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/22/14 14:46, Bjorn Andersson wrote: > For pm8941 the valid power supply values are: > GPIO 1-14 > 0: VPH > 2: SMPS3 > 3: LDO6 > > GPIO 15-18 > 2: SMPS3 > 3: LDO6 > > GPIO 19-36 > 0: VPH > 1: VDD_TORCH > 2: SPMS3 > 3: LDO6 > > MPP 1-8 > 0: VPH > 1: LDO1 > 2: SPMS3 > 3: LDO6 > > For pma8084 the valid power supply values are: > GPIO 1-22 > 0: VPH > 2: SPMS4 > 3: LDO6 > > MPP 1-8 > 0: VPH > 1: LDO1 > 2: SMPS4 > 3: LDO6 > > Please add these constants to the table of valid power-source values and use > something like I did to translate them to register values - it makes the DT > much more readable. The DT could be similarly readable if we had a bunch of #defines for the different VIN settings that resolved to the final register value for that pmic. Something like PM8921_GPIO1_14_VPH, PM8921_GPIO19_36_VPH, etc. There would be a lot of them, but then the driver could be really simple and just jam whatever value is in the DT into the register without having to bounce through a mapping table in software to figure out the register value. If we did this for the functions also then I believe we achieve readability without requiring a bunch of drivers for each and every single pmic? > >> Value type: >> Definition: Specify the alternative function to be configured for the >> - specified pins. Valid values are: >> - "normal", >> - "paired", >> - "func1", >> - "func2", >> - "dtest1", >> - "dtest2", >> - "dtest3", >> - "dtest4" >> + specified pins. Valid values is: "gpio" >> >> - bias-disable: >> Usage: optional >> @@ -99,18 +95,6 @@ to specify in a pin configuration subnode: >> Value type: >> Definition: The specified pins should be configued as pull down. >> >> -- bias-pull-up: >> - Usage: optional >> - Value type: (optional) >> - Definition: The specified pins should be configued as pull up. An >> - optional argument can be used to configure the strength. >> - Valid values are; as defined in >> - : >> - 1: 30uA (PM8XXX_GPIO_PULL_UP_30) >> - 2: 1.5uA (PM8XXX_GPIO_PULL_UP_1P5) >> - 3: 31.5uA (PM8XXX_GPIO_PULL_UP_31P5) >> - 4: 1.5uA + 30uA boost (PM8XXX_GPIO_PULL_UP_1P5_30) >> - > As described above, I belive we should make this: > > - bias-pull-up: > Usage: optional > Value type: > Definition: The specified pins should be configured as pull up. > > - qcom,pull-up-strength: > Usage: optional > Value type: > Definition: Specifies the strength to use for pull up, if selected. > Valid values are; as defined in > : > 1: 30uA (PM8XXX_GPIO_PULL_UP_30) > 2: 1.5uA (PM8XXX_GPIO_PULL_UP_1P5) > 3: 31.5uA (PM8XXX_GPIO_PULL_UP_31P5) > 4: 1.5uA + 30uA boost (PM8XXX_GPIO_PULL_UP_1P5_30) > If this property is ommited 30uA strength will be used if > pull up is selected. Why is 30uA special? Just because most drivers use it? I'd prefer we always be explicit about which pull-up we want so that nothing is left up to the driver implementation. Also according to the hw folks the 1.5uA + 30uA boost has never been used so I say let's drop that feature. If we need it one day we can always add a qcom,pull-up-boost or something (I highly doubt we'll need it). Doing that allows us to specify this in actual SI units. Maybe even allowing us to have a generic pull-up-strength (or bias-pull-up-strength) specified in SI units of mA? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/