Skip to contentSkip to conversation

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
#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.

---
 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
#1 lore raw
Quoted 24 lines · Adrian Hunter

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

---
 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.

#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.

---
  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.

---
 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
#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.

---
 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.
#4 lore raw
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 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 ;-)

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.

---
  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

#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.

---
 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

Applied
#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.

Applied, thanks.

g.

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