gpio: langwell: ensure alternate function is cleared

PATCH Applied Adrian Hunter 2 participants 7 messages Oct 3, 2011 – Oct 6, 2011 GPIO

lore

Share & tools

lore

Adrian Hunter

author

2011-10-03 11:36 UTC #0 lore raw

PATCH 1/1 gpio: langwell: ensure alternate function is cleared

Alternate function must be zero for the pin to act as a GPIO.

Signed-off-by Adrian Hunter <[email protected]>

Signed-off-by Alan Cox <[email protected]>

---
 drivers/gpio/gpio-langwell.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

drivers/gpio/gpio-langwell.c +27 -0

index d2eb57c..00692e8 100644
@@ -59,6 +59,7 @@ enum GPIO_REG {
 	GRER,		/* rising edge detect */
 	GFER,		/* falling edge detect */
 	GEDR,		/* edge detect result */
+	GAFR,		/* alt function */
 };
 
 struct lnw_gpio {

@@ -81,6 +82,31 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset,
 	return ptr;
 }
 
+static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset,
+				   enum GPIO_REG reg_type)
+{
+	struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip);
+	unsigned nreg = chip->ngpio / 32;
+	u8 reg = offset / 16;
+	void __iomem *ptr;
+
+	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);
+	return ptr;
+}
+
+static int lnw_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	void __iomem *gafr = gpio_reg_2bit(chip, offset, GAFR);
+	u32 value = readl(gafr);
+	int shift = (offset % 16) << 1, af = (value >> shift) & 3;
+
+	if (af) {
+		value &= ~(3 << shift);
+		writel(value, gafr);
+	}
+	return 0;
+}
+
 static int lnw_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	void __iomem *gplr = gpio_reg(chip, offset, GPLR);

@@ -321,6 +347,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
 	lnw->reg_base = base;
 	lnw->irq_base = irq_base;
 	lnw->chip.label = dev_name(&pdev->dev);
+	lnw->chip.request = lnw_gpio_request;
 	lnw->chip.direction_input = lnw_gpio_direction_input;
 	lnw->chip.direction_output = lnw_gpio_direction_output;
 	lnw->chip.get = lnw_gpio_get;

Signature · 1 line

1.7.6

Grant Likely

2011-10-04 20:40 UTC #1 lore raw

Quoted 24 lines · Adrian Hunter

Alternate function must be zero for the pin to act as a GPIO.

Signed-off-by Adrian Hunter <[email protected]>

Signed-off-by Alan Cox <[email protected]>

---
 drivers/gpio/gpio-langwell.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

drivers/gpio/gpio-langwell.c +1 -0

index d2eb57c..00692e8 100644
@@ -59,6 +59,7 @@ enum GPIO_REG {
 	GRER,		/* rising edge detect */
 	GFER,		/* falling edge detect */
 	GEDR,		/* edge detect result */
+	GAFR,		/* alt function */
 };
 
 struct lnw_gpio {

@@ -81,6 +82,31 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset,
 	return ptr;
 }

drivers/gpio/gpio-langwell.c static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, · line 93

Quoted 10 lines

drivers/gpio/gpio-langwell.c +9 -0

 
+static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset,
+				   enum GPIO_REG reg_type)
+{
+	struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip);
+	unsigned nreg = chip->ngpio / 32;
+	u8 reg = offset / 16;
+	void __iomem *ptr;
+
+	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);

This looks wrong. It looks to me like the __iomem annotation should be added to ->reg_base.

Otherwise, the patch looks okay.

Adrian Hunter

author

2011-10-05 07:52 UTC #2 lore raw

Quoted 39 lines · Grant Likely

Quoted 24 lines · Adrian Hunter
Alternate function must be zero for the pin to act as a GPIO.
Signed-off-by Adrian Hunter<[email protected]>
Signed-off-by Alan Cox<[email protected]>
---
  drivers/gpio/gpio-langwell.c |   27 +++++++++++++++++++++++++++
  1 files changed, 27 insertions(+), 0 deletions(-)
drivers/gpio/gpio-langwell.c +1 -0
index d2eb57c..00692e8 100644
@@ -59,6 +59,7 @@ enum GPIO_REG {
  	GRER,		/* rising edge detect */
  	GFER,		/* falling edge detect */
  	GEDR,		/* edge detect result */
+	GAFR,		/* alt function */
  };
 
  struct lnw_gpio {
@@ -81,6 +82,31 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset,
  	return ptr;
  }
drivers/gpio/gpio-langwell.c static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset, · line 93
Quoted 10 lines
drivers/gpio/gpio-langwell.c +9 -0
 
+static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset,
+				   enum GPIO_REG reg_type)
+{
+	struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip);
+	unsigned nreg = chip->ngpio / 32;
+	u8 reg = offset / 16;
+	void __iomem *ptr;
+
+	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);
This looks wrong. It looks to me like the __iomem annotation should be added to ->reg_base.

Quoted 2 lines

Otherwise, the patch looks okay.

Deserves a separate patch...

From: Adrian Hunter <[email protected]>
Date: Wed, 5 Oct 2011 10:29:27 +0300
Subject: [PATCH] gpio: langwell: declare reg_base as __iomem

reg_base is __iomem so add that to the declaration and fix up assignment casts and types.

Signed-off-by Adrian Hunter <[email protected]>

---
 drivers/gpio/gpio-langwell.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

drivers/gpio/gpio-langwell.c +6 -6

index 00692e8..b7465e5 100644
@@ -64,7 +64,7 @@ enum GPIO_REG {
 
 struct lnw_gpio {
 	struct gpio_chip		chip;
-	void				*reg_base;
+	void __iomem			*reg_base;
 	spinlock_t			lock;
 	unsigned			irq_base;
 	struct pci_dev			*pdev;

@@ -78,7 +78,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset,
 	u8 reg = offset / 32;
 	void __iomem *ptr;
 
-	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);
+	ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
 	return ptr;
 }

@@ -90,7 +90,7 @@ static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset,
 	u8 reg = offset / 16;
 	void __iomem *ptr;
 
-	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);
+	ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
 	return ptr;
 }

@@ -299,7 +299,7 @@ static const struct dev_pm_ops lnw_gpio_pm_ops = {
 static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
 			const struct pci_device_id *id)
 {
-	void *base;
+	void __iomem *base;
 	int i;
 	resource_size_t start, len;
 	struct lnw_gpio *lnw;

@@ -324,8 +324,8 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
 		dev_err(&pdev->dev, "error mapping bar1\n");
 		goto err3;
 	}
-	irq_base = *(u32 *)base;
-	gpio_base = *((u32 *)base + 1);
+	irq_base = *(__force u32 *)base;
+	gpio_base = *((__force u32 *)base + 1);
 	/* release the IO mapping, since we already get the info from bar1 */
 	iounmap(base);
 	/* get the register base from bar0 */

Signature · 1 line

1.7.6

Grant Likely

2011-10-05 16:59 UTC #3 lore raw

Quoted 6 lines · Adrian Hunter

Quoted 5 lines · Grant Likely
Quoted 2 lines · Adrian Hunter
+ ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg
* 4);

Quoted 7 lines

Quoted 5 lines
This looks wrong. It looks to me like the __iomem annotation should be added to ->reg_base.
Otherwise, the patch looks okay.
Deserves a separate patch...

What order do they get applied in? </grant-being-lazy>

Quoted 73 lines

From: Adrian Hunter <[email protected]>
Date: Wed, 5 Oct 2011 10:29:27 +0300
Subject: [PATCH] gpio: langwell: declare reg_base as __iomem

reg_base is __iomem so add that to the declaration and fix up assignment casts and types.

Signed-off-by Adrian Hunter <[email protected]>

---
drivers/gpio/gpio-langwell.c | 12 ++++++------

 1 files changed, 6 insertions(+), 6 deletions(-)

drivers/gpio/gpio-langwell.c +2 -2

index 00692e8..b7465e5 100644
@@ -64,7 +64,7 @@ enum GPIO_REG {
 
  struct lnw_gpio {
        struct gpio_chip                chip;
-       void                            *reg_base;
+       void __iomem                    *reg_base;
        spinlock_t                      lock;
        unsigned                        irq_base;
        struct pci_dev                  *pdev;

@@ -78,7 +78,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip,
 unsigned offset,
        u8 reg = offset / 32;
        void __iomem *ptr;
 
-       ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg *
 4);
+       ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
        return ptr;

}

drivers/gpio/gpio-langwell.c +1 -1

@@ -90,7 +90,7 @@ static void __iomem *gpio_reg_2bit(struct gpio_chip *chip,
 unsigned offset,
        u8 reg = offset / 16;
        void __iomem *ptr;
 
-       ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg *
 4);
+       ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
        return ptr;

}

drivers/gpio/gpio-langwell.c +3 -3

@@ -299,7 +299,7 @@ static const struct dev_pm_ops lnw_gpio_pm_ops = {
  static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
                        const struct pci_device_id *id)
  {
-       void *base;
+       void __iomem *base;
        int i;
        resource_size_t start, len;
        struct lnw_gpio *lnw;

@@ -324,8 +324,8 @@ static int __devinit lnw_gpio_probe(struct pci_dev
 *pdev,
                dev_err(&pdev->dev, "error mapping bar1\n");
                goto err3;
        }
-       irq_base = *(u32 *)base;
-       gpio_base = *((u32 *)base + 1);
+       irq_base = *(__force u32 *)base;
+       gpio_base = *((__force u32 *)base + 1);
        /* release the IO mapping, since we already get the info from bar1 */
        iounmap(base);

/* get the register base from bar0 */ --
1.7.6

Signature · 2 lines

Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Grant Likely

2011-10-06 10:19 UTC #5 lore raw

Quoted 11 lines · Adrian Hunter

Quoted 10 lines · Grant Likely
Quoted 9 lines · Adrian Hunter
Quoted 8 lines · Grant Likely
Quoted 2 lines · Adrian Hunter
+ ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg
* 4);
This looks wrong. It looks to me like the __iomem annotation should be added to ->reg_base.

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Otherwise, the patch looks okay.
Deserves a separate patch...
What order do they get applied in?</grant-being-lazy>
First one first ;-)

Please repost the 2nd patch properly. The way it was copied in caused whitespace corruption and forces me to manually apply the patch. It adds a lot of extra work if I cannot use "git am" to apply from my inbox.

Thanks,
g.

Quoted 88 lines

i.e.

gpio: langwell: ensure alternate function is cleared
gpio: langwell: declare reg_base as __iomem

Since the absence of __iomem was present before the original patch, there is no advantage to having the second one first, but the original patch would need to be changed as well.

Quoted 77 lines

Quoted 74 lines

From: Adrian Hunter<[email protected]>
Date: Wed, 5 Oct 2011 10:29:27 +0300
Subject: [PATCH] gpio: langwell: declare reg_base as __iomem

reg_base is __iomem so add that to the declaration and fix up assignment casts and types.

Signed-off-by Adrian Hunter<[email protected]>

---
 drivers/gpio/gpio-langwell.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

drivers/gpio/gpio-langwell.c +2 -2

index 00692e8..b7465e5 100644
@@ -64,7 +64,7 @@ enum GPIO_REG {
 
 struct lnw_gpio {
       struct gpio_chip                chip;
-       void                            *reg_base;
+       void __iomem                    *reg_base;
       spinlock_t                      lock;
       unsigned                        irq_base;
       struct pci_dev                  *pdev;

@@ -78,7 +78,7 @@ static void __iomem *gpio_reg(struct gpio_chip *chip,
 unsigned offset,
       u8 reg = offset / 32;
       void __iomem *ptr;
 
-       ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg *
 4);
+       ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
       return ptr;

}

drivers/gpio/gpio-langwell.c +1 -1

@@ -90,7 +90,7 @@ static void __iomem *gpio_reg_2bit(struct gpio_chip *chip,
 unsigned offset,
       u8 reg = offset / 16;
       void __iomem *ptr;
 
-       ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg *
 4);
+       ptr = lnw->reg_base + reg_type * nreg * 4 + reg * 4;
       return ptr;

}

drivers/gpio/gpio-langwell.c +3 -3

@@ -299,7 +299,7 @@ static const struct dev_pm_ops lnw_gpio_pm_ops = {
 static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
                       const struct pci_device_id *id)
 {
-       void *base;
+       void __iomem *base;
       int i;
       resource_size_t start, len;
       struct lnw_gpio *lnw;

@@ -324,8 +324,8 @@ static int __devinit lnw_gpio_probe(struct pci_dev
 *pdev,
               dev_err(&pdev->dev, "error mapping bar1\n");
               goto err3;
       }
-       irq_base = *(u32 *)base;
-       gpio_base = *((u32 *)base + 1);
+       irq_base = *(__force u32 *)base;
+       gpio_base = *((__force u32 *)base + 1);
       /* release the IO mapping, since we already get the info from bar1 */
       iounmap(base);

       /* get the register base from bar0 */

--
1.7.6

Grant Likely

Applied

2011-10-06 10:15 UTC #6 lore raw

In reply to Adrian Hunter

Quoted 5 lines · Adrian Hunter

Alternate function must be zero for the pin to act as a GPIO.
Signed-off-by Adrian Hunter <[email protected]>
Signed-off-by Alan Cox <[email protected]>

Applied, thanks.

Quoted 59 lines

---
 drivers/gpio/gpio-langwell.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

drivers/gpio/gpio-langwell.c +27 -0

index d2eb57c..00692e8 100644
@@ -59,6 +59,7 @@ enum GPIO_REG {
 	GRER,		/* rising edge detect */
 	GFER,		/* falling edge detect */
 	GEDR,		/* edge detect result */
+	GAFR,		/* alt function */
 };
 
 struct lnw_gpio {

@@ -81,6 +82,31 @@ static void __iomem *gpio_reg(struct gpio_chip *chip, unsigned offset,
 	return ptr;
 }
 
+static void __iomem *gpio_reg_2bit(struct gpio_chip *chip, unsigned offset,
+				   enum GPIO_REG reg_type)
+{
+	struct lnw_gpio *lnw = container_of(chip, struct lnw_gpio, chip);
+	unsigned nreg = chip->ngpio / 32;
+	u8 reg = offset / 16;
+	void __iomem *ptr;
+
+	ptr = (void __iomem *)(lnw->reg_base + reg_type * nreg * 4 + reg * 4);
+	return ptr;
+}
+
+static int lnw_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	void __iomem *gafr = gpio_reg_2bit(chip, offset, GAFR);
+	u32 value = readl(gafr);
+	int shift = (offset % 16) << 1, af = (value >> shift) & 3;
+
+	if (af) {
+		value &= ~(3 << shift);
+		writel(value, gafr);
+	}
+	return 0;
+}
+
 static int lnw_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	void __iomem *gplr = gpio_reg(chip, offset, GPLR);

@@ -321,6 +347,7 @@ static int __devinit lnw_gpio_probe(struct pci_dev *pdev,
 	lnw->reg_base = base;
 	lnw->irq_base = irq_base;
 	lnw->chip.label = dev_name(&pdev->dev);
+	lnw->chip.request = lnw_gpio_request;
 	lnw->chip.direction_input = lnw_gpio_direction_input;
 	lnw->chip.direction_output = lnw_gpio_direction_output;
 	lnw->chip.get = lnw_gpio_get;

Signature · 1 line

1.7.6