[PATCH linux dev-4.10 v6 1/2] ARM: dts: aspeed: Witherspoon add gpio-keys-polled for fan presence
Christopher Bostic
cbostic at linux.vnet.ibm.com
Fri Aug 4 04:31:17 AEST 2017
On 8/3/17 4:06 AM, Cédric Le Goater wrote:
> On 08/03/2017 04:27 AM, Christopher Bostic wrote:
>>
>> On 8/2/17 3:20 AM, Cédric Le Goater wrote:
>>> On 08/02/2017 09:10 AM, Cédric Le Goater wrote:
>>>> On 08/02/2017 04:33 AM, Christopher Bostic wrote:
>>>>> On 7/31/17 2:48 AM, Cédric Le Goater wrote:
>>>>>> On 07/29/2017 08:36 PM, Christopher Bostic wrote:
>>>>>>> Add gpio-keys-polled node for pca955x fan presence lines.
>>>>>>> Add GPIO details to pca955x for the presence lines as well.
>>>>>>>
>>>>>>> Polling period of 1 second determined to be the max acceptable
>>>>>>> value based on discussions with Brad Bishop and Matt Spinler.
>>>>>>> Tested with gpiomon program - verified.
>>>>>>>
>>>>>>> Signed-off-by: Christopher Bostic <cbostic at linux.vnet.ibm.com>
>>>>>>> ---
>>>>>>> v6 - Add gpio pin details for fan presence to pca955x node
>>>>>>> v5 - Fix gpio-keys-polled driver probe errors. Missing pca955x
>>>>>>> gpio cells property and wrong format for poll-interval.
>>>>>>> v4 - Add details regarding polling period to commit message.
>>>>>>> v3 - Remove 'autorepeat'
>>>>>>> - Increase polling period to 1 second.
>>>>>>> - Remove GPIO specifiers from pca955x in dev tree
>>>>>>> v2 - Change 'linux,code' from a unique global value to a local one
>>>>>>> for all the gpio-keys-polled devices.
>>>>>>> ---
>>>>>>> arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 52 ++++++++++++++++++++++++
>>>>>>> 1 file changed, 52 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>>>>>> index 5afc03a..70328668 100644
>>>>>>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>>>>>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>>>>>>> @@ -56,6 +56,37 @@
>>>>>>> };
>>>>>>> };
>>>>>>> + gpio-keys-polled {
>>>>>>> + compatible = "gpio-keys-polled";
>>>>>>> + #address-cells = <1>;
>>>>>>> + #size-cells = <0>;
>>>>>>> + poll-interval = <1000>;
>>>>>>> +
>>>>>>> + fan0-presence {
>>>>>>> + label = "fan0-presence";
>>>>>>> + gpios = <&pca0 4 GPIO_ACTIVE_LOW>;
>>>>>>> + linux,code = <4>;
>>>>>>> + };
>>>>>>> +
>>>>>>> + fan1-presence {
>>>>>>> + label = "fan1-presence";
>>>>>>> + gpios = <&pca0 5 GPIO_ACTIVE_LOW>;
>>>>>>> + linux,code = <5>;
>>>>>>> + };
>>>>>>> +
>>>>>>> + fan2-presence {
>>>>>>> + label = "fan2-presence";
>>>>>>> + gpios = <&pca0 6 GPIO_ACTIVE_LOW>;
>>>>>>> + linux,code = <6>;
>>>>>>> + };
>>>>>>> +
>>>>>>> + fan3-presence {
>>>>>>> + label = "fan3-presence";
>>>>>>> + gpios = <&pca0 7 GPIO_ACTIVE_LOW>;
>>>>>>> + linux,code = <7>;
>>>>>>> + };
>>>>>>> + };
>>>>>>> +
>>>>>>> leds {
>>>>>>> compatible = "gpio-leds";
>>>>>>> @@ -277,6 +308,7 @@
>>>>>>> reg = <0x60>;
>>>>>>> #address-cells = <1>;
>>>>>>> #size-cells = <0>;
>>>>>>> + #gpio-cells = <2>;
>>>>>> you should also add a :
>>>>>>
>>>>>> gpio-controller;
>>>>>>
>>>>>> if you want to use the lines as GPIOs
>>>>>>
>>>>>> C.
>>>>> Hi Cedric,
>>>>>
>>>>> If I add 'gpio-controller' here are you indicating that would not
>>>>> require any modifications I made to the leds-pca955x.c file?
>>>> yes.
>>>>
>>>>> I've tried that and found I still need that leds-pca955x.c change
>>>>> even if I add 'gpio-controller' attribute to get gpio-keys-polled
>>>>> device driver to properly probe the pca955x lines.
>>>> OK. I will give it a try on a QEMU. Maybe there are some incompatibilities
>>>> with the gpio-keys driver bindings
>>> It fails indeed, but the fix you are proposing does not look right
>>> either.
>>>
>>> Maybe we should fully define the pca9552 as a gpio controller and
>>> then define a gpio-leds and a gpio-keys-polled on top of it ?
>> Hi Cedric,
>>
>> What would be entailed in fully defining the pca9552 as a gpio controller?
>> Any other driver examples would help if you know of any.
> See below for an example. Quickly tested on QEMU.
Hi Cedric
Thanks for the example. Will use this approach.
-Chris
>> Currently I see that it has a struct gpio_chip embedded in its 955x chip
>> structure and that it defines its various properties, .direction_input,
>> .direction_output, etc... The struct gpio_chip ngpio field is set to a
>> subset of pins that are not labeled as LEDS. Hence my added change to count
>> LEDS towards total ngpio's. Given that the pins I'm interested in are 4-7
>> this falls outside the bounds of what the gpio chip considers total gpio
>> count. As a result gpio-keys-polled driver probe fails.
>>
>> Maybe a quick conference call might be useful?
> Sure if needed.
>
> Thanks,
>
> C.
>
>
> From: Cédric Le Goater <clg at kaod.org>
> Subject: [PATCH] ARM: dts: aspeed: witherspoon: add a gpio-keys-polled binding
> Date: Fri, 05 May 2017 09:30:25 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Change the current binding of the pca9552 device to be a GPIO expander
> and add on top of it a 'gpio-leds' binding to redefine the led pins of
> the pca9552. Finally add a 'gpio-keys-polled' binding.
>
> Signed-off-by: Cédric Le Goater <clg at kaod.org>
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 151 ++++++++++++++++++-----
> 1 file changed, 123 insertions(+), 28 deletions(-)
>
> Index: linux-openbmc-4.10.git/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> ===================================================================
> --- linux-openbmc-4.10.git.orig/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ linux-openbmc-4.10.git/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -93,6 +93,77 @@
> compatible = "iio-hwmon";
> io-channels = <&bmp 1>;
> };
> +
> + gpio-keys-polled {
> + compatible = "gpio-keys-polled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + poll-interval = <1000>;
> +
> + fan0-presence {
> + label = "fan0-presence";
> + gpios = <&pca0 4 GPIO_ACTIVE_LOW>;
> + linux,code = <4>;
> + };
> +
> + fan1-presence {
> + label = "fan1-presence";
> + gpios = <&pca0 5 GPIO_ACTIVE_LOW>;
> + linux,code = <5>;
> + };
> +
> + fan2-presence {
> + label = "fan2-presence";
> + gpios = <&pca0 6 GPIO_ACTIVE_LOW>;
> + linux,code = <6>;
> + };
> +
> + fan3-presence {
> + label = "fan3-presence";
> + gpios = <&pca0 7 GPIO_ACTIVE_LOW>;
> + linux,code = <7>;
> + };
> + };
> +
> + pca_leds {
> + compatible = "gpio-leds";
> +
> + fan0 {
> + label = "fan0";
> + default-state = "keep";
> + gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
> + };
> + fan1 {
> + label = "fan1";
> + default-state = "keep";
> + gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
> + };
> + fan2 {
> + label = "fan2";
> + default-state = "keep";
> + gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
> + };
> + fan3 {
> + label = "fan3";
> + default-state = "keep";
> + gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
> + };
> + front_fault {
> + label = "front-fault";
> + default-state = "keep";
> + gpios = <&pca0 13 GPIO_ACTIVE_LOW>;
> + };
> + front_power {
> + label = "front-power";
> + default-state = "keep";
> + gpios = <&pca0 14 GPIO_ACTIVE_LOW>;
> + };
> + front_id {
> + label = "front-id";
> + default-state = "keep";
> + gpios = <&pca0 15 GPIO_ACTIVE_LOW>;
> + };
> + };
> };
>
> #include "ibm-power9-cfam.dtsi"
> @@ -280,48 +351,72 @@
> reg = <0x60>;
> #address-cells = <1>;
> #size-cells = <0>;
> + gpio-controller;
> + #gpio-cells = <2>;
>
> - fan0: led at 0 {
> - label = "fan0";
> - default-state = "keep";
> + gpio at 0 {
> reg = <0>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - fan1: led at 1 {
> - label = "fan1";
> - default-state = "keep";
> + gpio at 1 {
> reg = <1>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - fan2: led at 2 {
> - label = "fan2";
> - default-state = "keep";
> + gpio at 2 {
> reg = <2>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - fan3: led at 3 {
> - label = "fan3";
> - default-state = "keep";
> + gpio at 3 {
> reg = <3>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - front_fault: led at 13 {
> - label = "front-fault";
> - default-state = "keep";
> + gpio at 4 {
> + reg = <4>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 5 {
> + reg = <5>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 6 {
> + reg = <6>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 7 {
> + reg = <7>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 8 {
> + reg = <8>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 9 {
> + reg = <9>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 10 {
> + reg = <10>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 11 {
> + reg = <11>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 12 {
> + reg = <12>;
> + type = <PCA955X_TYPE_GPIO>;
> + };
> + gpio at 13 {
> reg = <13>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - front_power: led at 14 {
> - label = "front-power";
> - default-state = "keep";
> + gpio at 14 {
> reg = <14>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> - front_id: led at 15 {
> - label = "front-id";
> - default-state = "keep";
> + gpio at 15 {
> reg = <15>;
> - type = <PCA955X_TYPE_LED>;
> + type = <PCA955X_TYPE_GPIO>;
> };
> };
> };
>
>
>
More information about the openbmc
mailing list