diff mbox

pinctrl: amd: Add support for additional GPIO

Message ID MWHPR12MB1357A3E7488C15C8E6E6BE45A0A20@MWHPR12MB1357.namprd12.prod.outlook.com
State New
Headers show

Commit Message

Shah, Nehal-bakulchandra Nov. 4, 2016, 3:46 a.m. UTC 
This patch provides support for additional GPIO devices and also provides IRQ sharing for AMD's GPIO devices and set
IRQCHIP_SKIP_SET_WAKE flag.

Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@amd.com>
---
 drivers/pinctrl/pinctrl-amd.c | 28 ++++++++++++++++++++--------
 drivers/pinctrl/pinctrl-amd.h |  7 ++++---
 2 files changed, 24 insertions(+), 11 deletions(-)

Comments

Linus Walleij Nov. 8, 2016, 9:41 a.m. UTC | #1 
On Fri, Nov 4, 2016 at 4:46 AM, Shah, Nehal-bakulchandra
<Nehal-bakulchandra.Shah@amd.com> wrote:

> This patch provides support for additional GPIO devices and also provides IRQ sharing for AMD's GPIO devices and set
> IRQCHIP_SKIP_SET_WAKE flag.
>
> Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@amd.com>

Patch applied.

Every time there is a patch to the AMD driver it comes from a new
person inside AMD, how do you actually manage this? It is better
for building trust in the long run if a person steps up as maintainer
and take a bit of long term responsibility.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Nov. 8, 2016, 1:13 p.m. UTC | #2 
On Fri, Nov 4, 2016 at 4:46 AM, Shah, Nehal-bakulchandra
<Nehal-bakulchandra.Shah@amd.com> wrote:

> This patch provides support for additional GPIO devices and also provides IRQ sharing for AMD's GPIO devices and set
> IRQCHIP_SKIP_SET_WAKE flag.
>
> Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@amd.com>

So the build servers are protesting against this patch because:

> +       ret = devm_request_irq(&pdev->dev, irq_base, amd_gpio_irq_handler,
> +                              IRQF_SHARED, dev_name(&pdev->dev), gpio_dev);

This is assigning amd_gpio_irq_handler() as IRQ handler, but that
one returns void and a proper irq handler needs to return
irqreturn_t.

Please look at this, I have backed out the patch.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index aea310a..e3189ef4 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -186,7 +186,7 @@  static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 	char *output_value;
 	char *output_enable;
 
-	for (bank = 0; bank < AMD_GPIO_TOTAL_BANKS; bank++) {
+	for (bank = 0; bank < gpio_dev->hwbank_num; bank++) {
 		seq_printf(s, "GPIO bank%d\t", bank);
 
 		switch (bank) {
@@ -244,17 +244,17 @@  static void amd_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 				interrupt_mask =
 					"interrupt is masked|";
 
-			if (pin_reg & BIT(WAKE_CNTRL_OFF))
+			if (pin_reg & BIT(WAKE_CNTRL_OFF_S0I3))
 				wake_cntrl0 = "enable wakeup in S0i3 state|";
 			else
 				wake_cntrl0 = "disable wakeup in S0i3 state|";
 
-			if (pin_reg & BIT(WAKE_CNTRL_OFF))
+			if (pin_reg & BIT(WAKE_CNTRL_OFF_S3))
 				wake_cntrl1 = "enable wakeup in S3 state|";
 			else
 				wake_cntrl1 = "disable wakeup in S3 state|";
 
-			if (pin_reg & BIT(WAKE_CNTRL_OFF))
+			if (pin_reg & BIT(WAKE_CNTRL_OFF_S4))
 				wake_cntrl2 = "enable wakeup in S4/S5 state|";
 			else
 				wake_cntrl2 = "disable wakeup in S4/S5 state|";
@@ -479,6 +479,7 @@  static void amd_irq_ack(struct irq_data *d)
 	.irq_unmask   = amd_gpio_irq_unmask,
 	.irq_eoi      = amd_gpio_irq_eoi,
 	.irq_set_type = amd_gpio_irq_set_type,
+	.flags        = IRQCHIP_SKIP_SET_WAKE,
 };
 
 static void amd_gpio_irq_handler(struct irq_desc *desc)
@@ -727,10 +728,14 @@  static int amd_pinconf_group_set(struct pinctrl_dev *pctldev,
 
 static int amd_gpio_probe(struct platform_device *pdev)
 {
+	int hwnum;
 	int ret = 0;
 	int irq_base;
 	struct resource *res;
 	struct amd_gpio *gpio_dev;
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+
+	ret = kstrtoint(acpi_device_uid(adev), 2, &hwnum);
 
 	gpio_dev = devm_kzalloc(&pdev->dev,
 				sizeof(struct amd_gpio), GFP_KERNEL);
@@ -763,16 +768,17 @@  static int amd_gpio_probe(struct platform_device *pdev)
 	gpio_dev->gc.set			= amd_gpio_set_value;
 	gpio_dev->gc.set_debounce	= amd_gpio_set_debounce;
 	gpio_dev->gc.dbg_show		= amd_gpio_dbg_show;
+	gpio_dev->gc.base               = gpio_dev->gc.ngpio * hwnum;
 
-	gpio_dev->gc.base			= 0;
 	gpio_dev->gc.label			= pdev->name;
 	gpio_dev->gc.owner			= THIS_MODULE;
 	gpio_dev->gc.parent			= &pdev->dev;
-	gpio_dev->gc.ngpio			= TOTAL_NUMBER_OF_PINS;
+	gpio_dev->gc.ngpio			= resource_size(res) / 4;
 #if defined(CONFIG_OF_GPIO)
 	gpio_dev->gc.of_node			= pdev->dev.of_node;
 #endif
 
+	gpio_dev->hwbank_num = gpio_dev->gc.ngpio / 64;
 	gpio_dev->groups = kerncz_groups;
 	gpio_dev->ngroups = ARRAY_SIZE(kerncz_groups);
 
@@ -789,7 +795,7 @@  static int amd_gpio_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = gpiochip_add_pin_range(&gpio_dev->gc, dev_name(&pdev->dev),
-				0, 0, TOTAL_NUMBER_OF_PINS);
+				0, 0, gpio_dev->gc.ngpio);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add pin range\n");
 		goto out2;
@@ -809,7 +815,12 @@  static int amd_gpio_probe(struct platform_device *pdev)
 	gpiochip_set_chained_irqchip(&gpio_dev->gc,
 				 &amd_gpio_irqchip,
 				 irq_base,
-				 amd_gpio_irq_handler);
+				 NULL);
+
+	ret = devm_request_irq(&pdev->dev, irq_base, amd_gpio_irq_handler,
+			       IRQF_SHARED, dev_name(&pdev->dev), gpio_dev);
+	if (ret)
+		goto out2;
 
 	platform_set_drvdata(pdev, gpio_dev);
 
@@ -829,6 +840,7 @@  static int amd_gpio_remove(struct platform_device *pdev)
 	gpio_dev = platform_get_drvdata(pdev);
 
 	gpiochip_remove(&gpio_dev->gc);
+	pinctrl_unregister(gpio_dev->pctrl);
 
 	return 0;
 }
diff --git a/drivers/pinctrl/pinctrl-amd.h b/drivers/pinctrl/pinctrl-amd.h
index 7bfea47..0ed47c6 100644
--- a/drivers/pinctrl/pinctrl-amd.h
+++ b/drivers/pinctrl/pinctrl-amd.h
@@ -13,9 +13,7 @@ 
 #ifndef _PINCTRL_AMD_H
 #define _PINCTRL_AMD_H
 
-#define TOTAL_NUMBER_OF_PINS	192
 #define AMD_GPIO_PINS_PER_BANK  64
-#define AMD_GPIO_TOTAL_BANKS    3
 
 #define AMD_GPIO_PINS_BANK0     63
 #define AMD_GPIO_PINS_BANK1     64
@@ -35,7 +33,9 @@ 
 #define ACTIVE_LEVEL_OFF		9
 #define INTERRUPT_ENABLE_OFF		11
 #define INTERRUPT_MASK_OFF		12
-#define WAKE_CNTRL_OFF			13
+#define WAKE_CNTRL_OFF_S0I3             13
+#define WAKE_CNTRL_OFF_S3               14
+#define WAKE_CNTRL_OFF_S4               15
 #define PIN_STS_OFF			16
 #define DRV_STRENGTH_SEL_OFF		17
 #define PULL_UP_SEL_OFF			19
@@ -93,6 +93,7 @@  struct amd_gpio {
 	u32 ngroups;
 	struct pinctrl_dev *pctrl;
 	struct gpio_chip        gc;
+	unsigned int            hwbank_num;
 	struct resource         *res;
 	struct platform_device  *pdev;
 };