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: <string> >> 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: <none> >> Definition: The specified pins should be configued as pull down. >> >> -- bias-pull-up: >> - Usage: optional >> - Value type: <u32> (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 >> - <dt-bindings/pinctrl/qcom,pm8xxx-gpio.h>: >> - 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: <empty> > Definition: The specified pins should be configured as pull up. > > - qcom,pull-up-strength: > Usage: optional > Value type: <u32> > Definition: Specifies the strength to use for pull up, if selected. > Valid values are; as defined in > <dt-bindings/pinctrl/qcom,pm8xxx-gpio.h>: > 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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
