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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53D04996.2030902@codeaurora.org>
Date:	Wed, 23 Jul 2014 16:47:34 -0700
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Bjorn Andersson <bjorn.andersson@...ymobile.com>
CC:	"Ivan T. Ivanov" <iivanov@...sol.com>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Mark Brown <broonie@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH RESEND v2 1/4] pinctrl: Update Qualcomm pm8xxx GPIO parameters
 definitions

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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/