Skip to contentSkip to conversation

pinctrl-baytrail: miscellaneous minor fixes

PATCH In review Andy Shevchenko 3 participants 14 messages Jul 10, 2013 – Jul 22, 2013 Pin control

lore
Share & tools
lore
#0 lore raw

cover pinctrl-baytrail: miscellaneous minor fixes

This is a set of small minor fixes against pinctrl-baytrail driver.

Andy Shevchenko (5):
pinctrl-baytrail: fix indentations
pinctrl-baytrail: change lvl to level
pinctrl-baytrail: remove redundant ptr variable
pinctrl-baytrail: introduce to_byt_gpio() macro
pinctrl-baytrail: fix to avoid sparse warnings

 drivers/pinctrl/Kconfig            |  2 +-
 drivers/pinctrl/pinctrl-baytrail.c | 37 +++++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 17 deletions(-)
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/5 pinctrl-baytrail: fix indentations

There are two minor issues with indentation in the code. This patch fixes them. No functional changes.

---
 drivers/pinctrl/Kconfig            | 2 +-
 drivers/pinctrl/pinctrl-baytrail.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
drivers/pinctrl/Kconfig +1 -1
index 5a8ad51..bc830af 100644
@@ -61,7 +61,7 @@ config PINCTRL_AT91
 config PINCTRL_BAYTRAIL
 	bool "Intel Baytrail GPIO pin control"
 	depends on GPIOLIB && ACPI && X86
-        select IRQ_DOMAIN
+	select IRQ_DOMAIN
 	help
 	  driver for memory mapped GPIO functionality on Intel Baytrail
 	  platforms. Supports 3 banks with 102, 28 and 44 gpios.
drivers/pinctrl/pinctrl-baytrail.c +1 -0
index e9d735d..fccf72e 100644
@@ -516,6 +516,7 @@ static int byt_gpio_remove(struct platform_device *pdev)
 {
 	struct byt_gpio *vg = platform_get_drvdata(pdev);
 	int err;
+
 	pm_runtime_disable(&pdev->dev);
 	err = gpiochip_remove(&vg->chip);
 	if (err)
Signature · 1 line
1.8.3.2
#2 lore raw

Re: [PATCH 1/5] pinctrl-baytrail: fix indentations

Quoted 4 lines · Andy Shevchenko

There are two minor issues with indentation in the code. This patch fixes them. No functional changes.

Patch applied.

Yours,
Linus Walleij

#3 lore raw

PATCH 2/5 pinctrl-baytrail: change lvl to level

In reply to Andy Shevchenko

Additionally remove trailing whitespace when print triggering type.

---
 drivers/pinctrl/pinctrl-baytrail.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/pinctrl-baytrail.c +1 -1
index fccf72e..06ffeee 100644
@@ -296,7 +296,7 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 			   conf0 & 0x7,
 			   conf0 & BYT_TRIG_NEG ? "fall " : "",
 			   conf0 & BYT_TRIG_POS ? "rise " : "",
-			   conf0 & BYT_TRIG_LVL ? "lvl " : "");
+			   conf0 & BYT_TRIG_LVL ? "level" : "");
 	}
 	spin_unlock_irqrestore(&vg->lock, flags);
 }
Signature · 1 line
1.8.3.2
#4 lore raw

PATCH 1/1 pinctrl-baytrail: change lvl to level

Additionally remove trailing whitespace when print triggering type.

---
 drivers/pinctrl/pinctrl-baytrail.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/pinctrl/pinctrl-baytrail.c +3 -3
index fccf72e..f3d3053 100644
@@ -294,9 +294,9 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 			   val & BYT_LEVEL ? "hi" : "lo",
 			   vg->range->pins[i], offs,
 			   conf0 & 0x7,
-			   conf0 & BYT_TRIG_NEG ? "fall " : "",
-			   conf0 & BYT_TRIG_POS ? "rise " : "",
-			   conf0 & BYT_TRIG_LVL ? "lvl " : "");
+			   conf0 & BYT_TRIG_NEG ? " fall" : "",
+			   conf0 & BYT_TRIG_POS ? " rise" : "",
+			   conf0 & BYT_TRIG_LVL ? " level" : "");
 	}
 	spin_unlock_irqrestore(&vg->lock, flags);
 }
Signature · 1 line
1.8.3.2
#6 lore raw

PATCH 3/5 pinctrl-baytrail: remove redundant ptr variable

In reply to Andy Shevchenko

There is no need to have an additional variable in byt_gpio_reg().

---
 drivers/pinctrl/pinctrl-baytrail.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pinctrl/pinctrl-baytrail.c +1 -3
index 06ffeee..14a143c 100644
@@ -135,15 +135,13 @@ static void __iomem *byt_gpio_reg(struct gpio_chip *chip, unsigned offset,
 {
 	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
 	u32 reg_offset;
-	void __iomem *ptr;
 
 	if (reg == BYT_INT_STAT_REG)
 		reg_offset = (offset / 32) * 4;
 	else
 		reg_offset = vg->range->pins[offset] * 16;
 
-	ptr = (void __iomem *) (vg->reg_base + reg_offset + reg);
-	return ptr;
+	return vg->reg_base + reg_offset + reg;
 }
 
 static int byt_gpio_request(struct gpio_chip *chip, unsigned offset)
Signature · 1 line
1.8.3.2
#7 lore raw

Re: [PATCH 3/5] pinctrl-baytrail: remove redundant ptr variable

Quoted 3 lines · Andy Shevchenko

There is no need to have an additional variable in byt_gpio_reg().

Patch applied.

Yours,
Linus Walleij

#8 lore raw

PATCH 4/5 pinctrl-baytrail: introduce to_byt_gpio() macro

In reply to Andy Shevchenko

The introduced macro helps to convert struct gpio_chip to struct byt_gpio.

---
 drivers/pinctrl/pinctrl-baytrail.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
drivers/pinctrl/pinctrl-baytrail.c +10 -8
index 14a143c..14e1e22 100644
@@ -130,10 +130,12 @@ struct byt_gpio {
 	struct pinctrl_gpio_range	*range;
 };
 
+#define to_byt_gpio(c)	container_of(c, struct byt_gpio, chip)
+
 static void __iomem *byt_gpio_reg(struct gpio_chip *chip, unsigned offset,
 				 int reg)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	u32 reg_offset;
 
 	if (reg == BYT_INT_STAT_REG)
@@ -146,7 +148,7 @@ static void __iomem *byt_gpio_reg(struct gpio_chip *chip, unsigned offset,
 
 static int byt_gpio_request(struct gpio_chip *chip, unsigned offset)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 
 	pm_runtime_get(&vg->pdev->dev);
 
@@ -155,7 +157,7 @@ static int byt_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 static void byt_gpio_free(struct gpio_chip *chip, unsigned offset)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	void __iomem *reg = byt_gpio_reg(&vg->chip, offset, BYT_CONF0_REG);
 	u32 value;
 
@@ -216,7 +218,7 @@ static int byt_gpio_get(struct gpio_chip *chip, unsigned offset)
 
 static void byt_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_VAL_REG);
 	unsigned long flags;
 	u32 old_val;
@@ -235,7 +237,7 @@ static void byt_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 
 static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_VAL_REG);
 	unsigned long flags;
 	u32 value;
@@ -254,7 +256,7 @@ static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 static int byt_gpio_direction_output(struct gpio_chip *chip,
 				     unsigned gpio, int value)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	void __iomem *reg = byt_gpio_reg(chip, gpio, BYT_VAL_REG);
 	unsigned long flags;
 	u32 reg_val;
@@ -272,7 +274,7 @@ static int byt_gpio_direction_output(struct gpio_chip *chip,
 
 static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	int i;
 	unsigned long flags;
 	u32 conf0, val, offs;
@@ -301,7 +303,7 @@ static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 
 static int byt_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
-	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	struct byt_gpio *vg = to_byt_gpio(chip);
 	return irq_create_mapping(vg->domain, offset);
 }
 
Signature · 1 line
1.8.3.2
#9 lore raw

Re: [PATCH 4/5] pinctrl-baytrail: introduce to_byt_gpio() macro

Quoted 3 lines · Andy Shevchenko

The introduced macro helps to convert struct gpio_chip to struct byt_gpio.

Patch applied.

Yours,
Linus Walleij

#10 lore raw

PATCH 5/5 pinctrl-baytrail: fix to avoid sparse warnings

In reply to Andy Shevchenko

There are couple of sparse warnings we could avoid if we use a bit verbose version of the code in byt_gpio_direction_output().

drivers/pinctrl/pinctrl-baytrail.c:266:45: warning: dubious: x | !y drivers/pinctrl/pinctrl-baytrail.c:267:36: warning: dubious: x | !y

Additionally simplify a bit the code in byt_gpio_direction_input().

---
 drivers/pinctrl/pinctrl-baytrail.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/pinctrl/pinctrl-baytrail.c +8 -4
index 14e1e22..f808b4b 100644
@@ -245,7 +245,7 @@ static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 	spin_lock_irqsave(&vg->lock, flags);
 
 	value = readl(reg) | BYT_DIR_MASK;
-	value = value & (~BYT_INPUT_EN); /* active low */
+	value &= ~BYT_INPUT_EN;		/* active low */
 	writel(value, reg);
 
 	spin_unlock_irqrestore(&vg->lock, flags);
@@ -263,9 +263,13 @@ static int byt_gpio_direction_output(struct gpio_chip *chip,
 
 	spin_lock_irqsave(&vg->lock, flags);
 
-	reg_val = readl(reg) | (BYT_DIR_MASK | !!value);
-	reg_val &= ~(BYT_OUTPUT_EN | !value);
-	writel(reg_val, reg);
+	reg_val = readl(reg) | BYT_DIR_MASK;
+	reg_val &= ~BYT_OUTPUT_EN;
+
+	if (value)
+		writel(reg_val | BYT_LEVEL, reg);
+	else
+		writel(reg_val & ~BYT_LEVEL, reg);
 
 	spin_unlock_irqrestore(&vg->lock, flags);
 
Signature · 1 line
1.8.3.2
#11 lore raw

Re: [PATCH 5/5] pinctrl-baytrail: fix to avoid sparse warnings

Quoted 9 lines · Andy Shevchenko

There are couple of sparse warnings we could avoid if we use a bit verbose version of the code in byt_gpio_direction_output().

drivers/pinctrl/pinctrl-baytrail.c:266:45: warning: dubious: x | !y drivers/pinctrl/pinctrl-baytrail.c:267:36: warning: dubious: x | !y

Additionally simplify a bit the code in byt_gpio_direction_input().

Patch applied.

Yours,
Linus Walleij

#12 lore raw
In reply to Andy Shevchenko
Quoted 1 line · Andy Shevchenko

This is a set of small minor fixes against pinctrl-baytrail driver.

Linus, Mika, anyone do you have any comment on the series?

Quoted 12 lines

Andy Shevchenko (5):
pinctrl-baytrail: fix indentations
pinctrl-baytrail: change lvl to level
pinctrl-baytrail: remove redundant ptr variable
pinctrl-baytrail: introduce to_byt_gpio() macro
pinctrl-baytrail: fix to avoid sparse warnings

 drivers/pinctrl/Kconfig            |  2 +-
 drivers/pinctrl/pinctrl-baytrail.c | 37 +++++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 17 deletions(-)
Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
Reviewed-by
#13 lore raw
Quoted 4 lines · Andy Shevchenko
Quoted 1 line · Andy Shevchenko

This is a set of small minor fixes against pinctrl-baytrail driver.

Linus, Mika, anyone do you have any comment on the series?

Well, looks good to me but I don't know the HW details. Anyway, I reviewed the code so: