[PATCH linux dev-4.10 v6 1/2] ARM: dts: aspeed: Witherspoon add gpio-keys-polled for fan presence

Cédric Le Goater clg at kaod.org
Thu Aug 3 19:06:06 AEST 2017


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.

> 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