All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-4.10] drivers/leds: Add GPIO keys for PCA955x and ast24/2500 gpio
@ 2017-06-30 22:55 Christopher Bostic
  2017-07-03  5:40 ` Joel Stanley
  0 siblings, 1 reply; 3+ messages in thread
From: Christopher Bostic @ 2017-06-30 22:55 UTC (permalink / raw)
  To: joel; +Cc: Christopher Bostic, openbmc

Define gpio keys for power supply presence, ucd alert, and fan presence.

Link pca955x to parent i2c interrupt controller so that new pca955x gpio
keys for fan presence haning off of pca955x can be defined.

Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 77 ++++++++++++++++++++++++
 drivers/leds/leds-pca955x.c                      | 64 ++++++++++++++++++++
 2 files changed, 141 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 31315d0..24ea9944 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -46,6 +46,48 @@
 			gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
 			linux,code = <ASPEED_GPIO(J, 2)>;
 		};
+
+		ps0-presence {
+			label = "ps0-presence";
+			gpios = <&gpio ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>;
+			linux,code = <ASPEED_GPIO(P, 7)>;
+		};
+
+		ps1-presence {
+			label = "ps1-presence";
+			gpios = <&gpio ASPEED_GPIO(N, 0) GPIO_ACTIVE_LOW>;
+			linux,code = <ASPEED_GPIO(N, 0)>;
+		};
+
+		ucd-alert {
+			label = "ucd-alert";
+			gpios = <&gpio ASPEED_GPIO(I, 2) GPIO_ACTIVE_LOW>;
+			linux,code = <ASPEED_GPIO(I, 2)>;
+		};
+
+		fan0-presence {
+			label = "fan0-presence";
+			gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
+			linux,code = <0>;
+		};
+
+		fan1-presence {
+			label = "fan1-presence";
+			gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
+			linux,code = <1>;
+		};
+
+		fan2-presence {
+			label = "fan2-presence";
+			gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
+			linux,code = <2>;
+		};
+
+		fan3-presence {
+			label = "fan3-presence";
+			gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
+			linux,code = <3>;
+		};
 	};
 
 	leds {
@@ -203,6 +245,41 @@
 		compatible = "infineon,dps310";
 		reg = <0x76>;
 	};
+
+	pca0: pca9552@60 {
+		compatible = "nxp,pca9552";
+		reg = <0x60>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		interrupt-parent = <&i2c_ic>;
+		interrupts = <3>;
+
+		gpio-line-names = "FAN0_PRESENSE_N", "FAN1_PRESENSE_N",
+				"FAN2_PRESENSE_N", "FAN3_PRESENSE_N";
+
+		gpio@4 {
+			reg = <4>;
+			type = <PCA955X_TYPE_GPIO>;
+		};
+
+		gpio@5 {
+			reg = <5>;
+			type = <PCA955X_TYPE_GPIO>;
+		};
+
+		gpio@6 {
+			reg = <6>;
+			type = <PCA955X_TYPE_GPIO>;
+		};
+
+		gpio@7 {
+			reg = <7>;
+			type = <PCA955X_TYPE_GPIO>;
+		};
+	};
 };
 
 &i2c4 {
diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 9216742..88990f1 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -52,6 +52,7 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/interrupt.h>
 
 #include <dt-bindings/leds/leds-pca955x.h>
 
@@ -389,6 +390,66 @@ static int pca955x_gpio_direction_output(struct gpio_chip *gc,
 }
 #endif
 
+static void pca955x_irq_mask(struct irq_data *d)
+{
+}
+
+static void pca955x_irq_unmask(struct irq_data *d)
+{
+}
+
+static void pca955x_irq_bus_lock(struct irq_data *d)
+{
+}
+
+static void pca955x_irq_bus_sync_unlock(struct irq_data *d)
+{
+}
+
+static int pca955x_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	return 0;
+}
+
+static struct irq_chip pca955x_irq_chip = {
+	.name			= "pca955x",
+	.irq_mask		= pca955x_irq_mask,
+	.irq_unmask		= pca955x_irq_unmask,
+	.irq_bus_lock		= pca955x_irq_bus_lock,
+	.irq_bus_sync_unlock	= pca955x_irq_bus_sync_unlock,
+	.irq_set_type		= pca955x_irq_set_type,
+};
+
+static int pca955x_irq_setup(struct pca955x *chip, int irq_base)
+{
+	struct i2c_client *client;
+	int ret;
+
+	if (!chip)
+		return 0;
+
+	client = chip->client;
+	if (!client)
+		return 0;
+
+	if (client->irq && irq_base >= 0) {
+		ret = gpiochip_irqchip_add_nested(&chip->gpio,
+						&pca955x_irq_chip,
+						irq_base,
+						handle_simple_irq,
+						IRQ_TYPE_NONE);
+		if (ret) {
+			dev_err(&client->dev,
+				"could not connect irqchip to gpiochip\n");
+			return ret;
+		}
+		gpiochip_set_nested_irqchip(&chip->gpio,
+					&pca955x_irq_chip,
+					client->irq);
+	}
+	return 0;
+}
+
 static int pca955x_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
@@ -543,6 +604,9 @@ static int pca955x_probe(struct i2c_client *client,
 		}
 	}
 #endif
+	err = pca955x_irq_setup(pca955x, 0);
+	if (err)
+		return err;
 
 	return 0;
 }
-- 
1.8.2.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH linux dev-4.10] drivers/leds: Add GPIO keys for PCA955x and ast24/2500 gpio
  2017-06-30 22:55 [PATCH linux dev-4.10] drivers/leds: Add GPIO keys for PCA955x and ast24/2500 gpio Christopher Bostic
@ 2017-07-03  5:40 ` Joel Stanley
  2017-07-05 19:56   ` Christopher Bostic
  0 siblings, 1 reply; 3+ messages in thread
From: Joel Stanley @ 2017-07-03  5:40 UTC (permalink / raw)
  To: Christopher Bostic; +Cc: OpenBMC Maillist

On Sat, Jul 1, 2017 at 8:25 AM, Christopher Bostic
<cbostic@linux.vnet.ibm.com> wrote:
> Define gpio keys for power supply presence, ucd alert, and fan presence.
>
> Link pca955x to parent i2c interrupt controller so that new pca955x gpio
> keys for fan presence haning off of pca955x can be defined.
>
> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 77 ++++++++++++++++++++++++
>  drivers/leds/leds-pca955x.c                      | 64 ++++++++++++++++++++

The change to the source file and the .dts should be in separate patches.

>  2 files changed, 141 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index 31315d0..24ea9944 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -46,6 +46,48 @@
>                         gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
>                         linux,code = <ASPEED_GPIO(J, 2)>;
>                 };
> +
> +               ps0-presence {
> +                       label = "ps0-presence";
> +                       gpios = <&gpio ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>;
> +                       linux,code = <ASPEED_GPIO(P, 7)>;
> +               };
> +
> +               ps1-presence {
> +                       label = "ps1-presence";
> +                       gpios = <&gpio ASPEED_GPIO(N, 0) GPIO_ACTIVE_LOW>;
> +                       linux,code = <ASPEED_GPIO(N, 0)>;
> +               };
> +
> +               ucd-alert {
> +                       label = "ucd-alert";
> +                       gpios = <&gpio ASPEED_GPIO(I, 2) GPIO_ACTIVE_LOW>;
> +                       linux,code = <ASPEED_GPIO(I, 2)>;
> +               };

I don't think this is correct. Isn't the alert line part of the sbmus device?

> +
> +               fan0-presence {
> +                       label = "fan0-presence";
> +                       gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
> +                       linux,code = <0>;
> +               };
> +
> +               fan1-presence {
> +                       label = "fan1-presence";
> +                       gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
> +                       linux,code = <1>;
> +               };
> +
> +               fan2-presence {
> +                       label = "fan2-presence";
> +                       gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
> +                       linux,code = <2>;
> +               };
> +
> +               fan3-presence {
> +                       label = "fan3-presence";
> +                       gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
> +                       linux,code = <3>;
> +               };
>         };
>
>         leds {
> @@ -203,6 +245,41 @@
>                 compatible = "infineon,dps310";
>                 reg = <0x76>;
>         };
> +
> +       pca0: pca9552@60 {
> +               compatible = "nxp,pca9552";
> +               reg = <0x60>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +
> +               interrupt-parent = <&i2c_ic>;
> +               interrupts = <3>;
> +
> +               gpio-line-names = "FAN0_PRESENSE_N", "FAN1_PRESENSE_N",
> +                               "FAN2_PRESENSE_N", "FAN3_PRESENSE_N";
> +
> +               gpio@4 {
> +                       reg = <4>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio@5 {
> +                       reg = <5>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio@6 {
> +                       reg = <6>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +
> +               gpio@7 {
> +                       reg = <7>;
> +                       type = <PCA955X_TYPE_GPIO>;
> +               };
> +       };
>  };
>
>  &i2c4 {
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index 9216742..88990f1 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -52,6 +52,7 @@
>  #include <linux/of.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
> +#include <linux/interrupt.h>
>
>  #include <dt-bindings/leds/leds-pca955x.h>
>
> @@ -389,6 +390,66 @@ static int pca955x_gpio_direction_output(struct gpio_chip *gc,
>  }
>  #endif
>
> +static void pca955x_irq_mask(struct irq_data *d)
> +{
> +}
> +
> +static void pca955x_irq_unmask(struct irq_data *d)
> +{
> +}
> +
> +static void pca955x_irq_bus_lock(struct irq_data *d)
> +{
> +}
> +
> +static void pca955x_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +}

It looks like you sent a half-finished version.

> +
> +static int pca955x_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       return 0;
> +}
> +
> +static struct irq_chip pca955x_irq_chip = {
> +       .name                   = "pca955x",
> +       .irq_mask               = pca955x_irq_mask,
> +       .irq_unmask             = pca955x_irq_unmask,
> +       .irq_bus_lock           = pca955x_irq_bus_lock,
> +       .irq_bus_sync_unlock    = pca955x_irq_bus_sync_unlock,
> +       .irq_set_type           = pca955x_irq_set_type,
> +};
> +
> +static int pca955x_irq_setup(struct pca955x *chip, int irq_base)
> +{
> +       struct i2c_client *client;
> +       int ret;
> +
> +       if (!chip)
> +               return 0;
> +
> +       client = chip->client;
> +       if (!client)
> +               return 0;
> +
> +       if (client->irq && irq_base >= 0) {
> +               ret = gpiochip_irqchip_add_nested(&chip->gpio,
> +                                               &pca955x_irq_chip,
> +                                               irq_base,
> +                                               handle_simple_irq,
> +                                               IRQ_TYPE_NONE);
> +               if (ret) {
> +                       dev_err(&client->dev,
> +                               "could not connect irqchip to gpiochip\n");
> +                       return ret;
> +               }
> +               gpiochip_set_nested_irqchip(&chip->gpio,
> +                                       &pca955x_irq_chip,
> +                                       client->irq);
> +       }
> +       return 0;
> +}
> +
>  static int pca955x_probe(struct i2c_client *client,
>                                         const struct i2c_device_id *id)
>  {
> @@ -543,6 +604,9 @@ static int pca955x_probe(struct i2c_client *client,
>                 }
>         }
>  #endif
> +       err = pca955x_irq_setup(pca955x, 0);
> +       if (err)
> +               return err;
>
>         return 0;
>  }
> --
> 1.8.2.2
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH linux dev-4.10] drivers/leds: Add GPIO keys for PCA955x and ast24/2500 gpio
  2017-07-03  5:40 ` Joel Stanley
@ 2017-07-05 19:56   ` Christopher Bostic
  0 siblings, 0 replies; 3+ messages in thread
From: Christopher Bostic @ 2017-07-05 19:56 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist



On 7/3/17 12:40 AM, Joel Stanley wrote:
> On Sat, Jul 1, 2017 at 8:25 AM, Christopher Bostic
> <cbostic@linux.vnet.ibm.com> wrote:
>> Define gpio keys for power supply presence, ucd alert, and fan presence.
>>
>> Link pca955x to parent i2c interrupt controller so that new pca955x gpio
>> keys for fan presence haning off of pca955x can be defined.
>>
>> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
>> ---
>>   arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 77 ++++++++++++++++++++++++
>>   drivers/leds/leds-pca955x.c                      | 64 ++++++++++++++++++++
> The change to the source file and the .dts should be in separate patches.
Will separate them.
>
>>   2 files changed, 141 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> index 31315d0..24ea9944 100644
>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>> @@ -46,6 +46,48 @@
>>                          gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_LOW>;
>>                          linux,code = <ASPEED_GPIO(J, 2)>;
>>                  };
>> +
>> +               ps0-presence {
>> +                       label = "ps0-presence";
>> +                       gpios = <&gpio ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(P, 7)>;
>> +               };
>> +
>> +               ps1-presence {
>> +                       label = "ps1-presence";
>> +                       gpios = <&gpio ASPEED_GPIO(N, 0) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(N, 0)>;
>> +               };
>> +
>> +               ucd-alert {
>> +                       label = "ucd-alert";
>> +                       gpios = <&gpio ASPEED_GPIO(I, 2) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(I, 2)>;
>> +               };
> I don't think this is correct. Isn't the alert line part of the sbmus device?
According to the Box Elder schematic revision 03232017 page 24 there is 
a line listed as 'BMC_UCD_ALERT_N' Connected to ast2500 GPIO I2 as an 
input.   It appears this net is also connected to the smbus device.   
Maybe I'm referencing the wrong signal?

>> +
>> +               fan0-presence {
>> +                       label = "fan0-presence";
>> +                       gpios = <&pca0 0 GPIO_ACTIVE_LOW>;
>> +                       linux,code = <0>;
>> +               };
>> +
>> +               fan1-presence {
>> +                       label = "fan1-presence";
>> +                       gpios = <&pca0 1 GPIO_ACTIVE_LOW>;
>> +                       linux,code = <1>;
>> +               };
>> +
>> +               fan2-presence {
>> +                       label = "fan2-presence";
>> +                       gpios = <&pca0 2 GPIO_ACTIVE_LOW>;
>> +                       linux,code = <2>;
>> +               };
>> +
>> +               fan3-presence {
>> +                       label = "fan3-presence";
>> +                       gpios = <&pca0 3 GPIO_ACTIVE_LOW>;
>> +                       linux,code = <3>;
>> +               };
>>          };
>>
>>          leds {
>> @@ -203,6 +245,41 @@
>>                  compatible = "infineon,dps310";
>>                  reg = <0x76>;
>>          };
>> +
>> +       pca0: pca9552@60 {
>> +               compatible = "nxp,pca9552";
>> +               reg = <0x60>;
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               gpio-controller;
>> +               #gpio-cells = <2>;
>> +
>> +               interrupt-parent = <&i2c_ic>;
>> +               interrupts = <3>;
>> +
>> +               gpio-line-names = "FAN0_PRESENSE_N", "FAN1_PRESENSE_N",
>> +                               "FAN2_PRESENSE_N", "FAN3_PRESENSE_N";
>> +
>> +               gpio@4 {
>> +                       reg = <4>;
>> +                       type = <PCA955X_TYPE_GPIO>;
>> +               };
>> +
>> +               gpio@5 {
>> +                       reg = <5>;
>> +                       type = <PCA955X_TYPE_GPIO>;
>> +               };
>> +
>> +               gpio@6 {
>> +                       reg = <6>;
>> +                       type = <PCA955X_TYPE_GPIO>;
>> +               };
>> +
>> +               gpio@7 {
>> +                       reg = <7>;
>> +                       type = <PCA955X_TYPE_GPIO>;
>> +               };
>> +       };
>>   };
>>
>>   &i2c4 {
>> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
>> index 9216742..88990f1 100644
>> --- a/drivers/leds/leds-pca955x.c
>> +++ b/drivers/leds/leds-pca955x.c
>> @@ -52,6 +52,7 @@
>>   #include <linux/of.h>
>>   #include <linux/slab.h>
>>   #include <linux/string.h>
>> +#include <linux/interrupt.h>
>>
>>   #include <dt-bindings/leds/leds-pca955x.h>
>>
>> @@ -389,6 +390,66 @@ static int pca955x_gpio_direction_output(struct gpio_chip *gc,
>>   }
>>   #endif
>>
>> +static void pca955x_irq_mask(struct irq_data *d)
>> +{
>> +}
>> +
>> +static void pca955x_irq_unmask(struct irq_data *d)
>> +{
>> +}
>> +
>> +static void pca955x_irq_bus_lock(struct irq_data *d)
>> +{
>> +}
>> +
>> +static void pca955x_irq_bus_sync_unlock(struct irq_data *d)
>> +{
>> +}
> It looks like you sent a half-finished version.
I added the basic capability required to register the GPIO key with 
interrupt support.   The intent was to use the 'generic' interrupt 
support provided by the kernel, handle_simple_irq( ) etc...   These 
functions were left undefined since its not clear what need there will 
be for them.  Maybe the best choice is to not define them in that case?


Thanks,
Chris
>> +
>> +static int pca955x_irq_set_type(struct irq_data *d, unsigned int type)
>> +{
>> +       return 0;
>> +}
>> +
>> +static struct irq_chip pca955x_irq_chip = {
>> +       .name                   = "pca955x",
>> +       .irq_mask               = pca955x_irq_mask,
>> +       .irq_unmask             = pca955x_irq_unmask,
>> +       .irq_bus_lock           = pca955x_irq_bus_lock,
>> +       .irq_bus_sync_unlock    = pca955x_irq_bus_sync_unlock,
>> +       .irq_set_type           = pca955x_irq_set_type,
>> +};
>> +
>> +static int pca955x_irq_setup(struct pca955x *chip, int irq_base)
>> +{
>> +       struct i2c_client *client;
>> +       int ret;
>> +
>> +       if (!chip)
>> +               return 0;
>> +
>> +       client = chip->client;
>> +       if (!client)
>> +               return 0;
>> +
>> +       if (client->irq && irq_base >= 0) {
>> +               ret = gpiochip_irqchip_add_nested(&chip->gpio,
>> +                                               &pca955x_irq_chip,
>> +                                               irq_base,
>> +                                               handle_simple_irq,
>> +                                               IRQ_TYPE_NONE);
>> +               if (ret) {
>> +                       dev_err(&client->dev,
>> +                               "could not connect irqchip to gpiochip\n");
>> +                       return ret;
>> +               }
>> +               gpiochip_set_nested_irqchip(&chip->gpio,
>> +                                       &pca955x_irq_chip,
>> +                                       client->irq);
>> +       }
>> +       return 0;
>> +}
>> +
>>   static int pca955x_probe(struct i2c_client *client,
>>                                          const struct i2c_device_id *id)
>>   {
>> @@ -543,6 +604,9 @@ static int pca955x_probe(struct i2c_client *client,
>>                  }
>>          }
>>   #endif
>> +       err = pca955x_irq_setup(pca955x, 0);
>> +       if (err)
>> +               return err;
>>
>>          return 0;
>>   }
>> --
>> 1.8.2.2
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-07-05 19:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30 22:55 [PATCH linux dev-4.10] drivers/leds: Add GPIO keys for PCA955x and ast24/2500 gpio Christopher Bostic
2017-07-03  5:40 ` Joel Stanley
2017-07-05 19:56   ` Christopher Bostic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.