[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