[PATCH linux dev-4.10 v6 1/2] ARM: dts: aspeed: Witherspoon add gpio-keys-polled for fan presence
Andrew Jeffery
andrew at aj.id.au
Thu Aug 3 14:57:54 AEST 2017
On Wed, 2017-08-02 at 21:27 -0500, 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.
>
> 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.
I'm starting to think it's best to describe the device as a pin-
controller, and "mux" the lines between GPIO and LED. This allows
describing the all the lines as GPIO, or providing a map between the
GPIO and LED line number spaces. This would resolve the "index is
greater than the count" problem that you're running into.
See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/gpio/gpio.txt?h=v4.12#n230
And
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt?h=v4.12#n282
Now I don't know whether there is a bit to describe the line as LED or
GPIO, but even if there's not you can just stub-out the set_mux() and
request_gpio_enable() callbacks. It shouldn't be too much effort. But I
also haven't thought about it in great depth - it's a train-of-thought
suggestion at the moment.
Cheers,
Andrew
>
> Maybe a quick conference call might be useful?
>
> Thanks,
> Chris
>
>
> > Thanks,
> >
> > C.
> >
> >
> > > C.
> > >
> > > > Thanks
> > > > Chris
> > > >
> > > > > > > > > > > > fan0: led at 0 {
> > > > > > label = "fan0";
> > > > > > @@ -302,6 +334,26 @@
> > > > > > reg = <3>;
> > > > > > type = <PCA955X_TYPE_LED>;
> > > > > > };
> > > > > > > > > > > > + fan0_pres_gpio: gpio at 4 {
> > > > > > + label = "fan0_presence";
> > > > > > + reg = <4>;
> > > > > > + type = <PCA955X_TYPE_GPIO>;
> > > > > > + };
> > > > > > > > > > > > + fan1_pres_gpio: gpio at 5 {
> > > > > > + label = "fan1_presence";
> > > > > > + reg = <5>;
> > > > > > + type = <PCA955X_TYPE_GPIO>;
> > > > > > + };
> > > > > > > > > > > > + fan2_pres_gpio: gpio at 6 {
> > > > > > + label = "fan2_presence";
> > > > > > + reg = <6>;
> > > > > > + type = <PCA955X_TYPE_GPIO>;
> > > > > > + };
> > > > > > > > > > > > + fan3_pres_gpio: gpio at 7 {
> > > > > > + label = "fan3_presence";
> > > > > > + reg = <7>;
> > > > > > + type = <PCA955X_TYPE_GPIO>;
> > > > > > + };
> > > > > > > > > > > > front_fault: led at 13 {
> > > > > > label = "front-fault";
> > > > > > default-state = "keep";
> > > > > >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170803/4d08f01a/attachment-0001.sig>
More information about the openbmc
mailing list