Re: [PATCH RESEND v2 1/4] pinctrl: Update Qualcomm pm8xxx GPIO parameters definitions

[Stephen Boyd <sboyd@codeaurora.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: <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