LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/1] Consolidate similar algorithms in gpio-sch
@ 2015-01-21 10:32 Chang Rebecca Swee Fun
  2015-01-21 10:32 ` [PATCH 1/1] gpio: sch: Consolidate similar algorithms Chang Rebecca Swee Fun
  0 siblings, 1 reply; 6+ messages in thread
From: Chang Rebecca Swee Fun @ 2015-01-21 10:32 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Mika Westerberg
  Cc: Chang Rebecca Swee Fun, GPIO Subsystem Mailing List,
	Linux Kernel Mailing List

Hi,

Some background about this patch:
This patch come from the previous threads on enabling Quark X1000 support in
gpio-sch. The patches were found to break the build on devel branch.
Refer to: https://lists.01.org/pipermail/kbuild-all/2015-January/008513.html

One of the patch (mentioned below) was already applied in devel branch.
(Patch 2/2) 9202149 gpio: sch: Add support for Intel Quark X1000 SoC.
Meanwhile the patch that breaks the build was removed from devel branch.

So, in this email, I've attached a patch that contains the algorithms
consolidation together with the fix for build warnings on wrong arguments
passing in sch_gpio_reg_set().

The patch was built with the configurations provided in the mail by
Wu, Fengguang:
https://lists.01.org/pipermail/kbuild-all/2015-January/008511.html

Procedures for build test on devel branch:
$ cp ATT23421.config .config
$ make ARCH=x86_64 oldconfig
$ make ARCH=x86_64
Same procedures applied for ARCH=x86 and no build warnings was found on
gpio-sch.

Driver test procedures on Galileo board using Yocto Project as reference OS:

Check driver availability:
$ ls /sys/bus/platform/drivers/
Result: Driver sch_gpio available

Check device availability:
$ ls /sys/bus/platform/devices/
Result: Device sch_gpio.2398 available

Check the driver binded with the device successfully through sysfs:
$ ls -al /sys/class/gpio/gpiochip0/device
Result: gpiochip0/device/ -> ../../../sch_gpio.2398

Check using sysfs debug:
$ cat /sys/kernel/debug/gpio
GPIOs 0-7, platform/sch_gpio.2398, sch_gpio.2398:
Result: Both driver and device are available

The driver should allow user to export GPIO pin.
$ echo 3 > export
Result: gpio3 create with no error message

The driver should allow user to configure GPIO pin direction and pin value.
Since gpio3 (on sysfs for Galileo) is connected to an LED, we will use this as
our test equipment.
$ echo out > gpio3/direction
$ cat gpio3/direction
out
Result: Pin direction was set without error prompts.

$ echo 1 > gpio3/value
$ cat gpio3/value
1
Result: LED will turn on

$ echo 0 > gpio3/value
$ cat gpio3/value
0
Result: LED will turn off

$ echo in > gpio3/direction
$ cat gpio3/direction
in
Result: Pin direction was set without error prompts.

The driver should allow user to do unexport for unused pin.
$ echo 3 > unexport
Result: gpio3 directory was wiped out without error logs.

Please review the patch and provide feedback if any. Thanks.

Regards,
Rebecca

Chang Rebecca Swee Fun (1):
  gpio: sch: Consolidate similar algorithms

 drivers/gpio/gpio-sch.c | 83 +++++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 54 deletions(-)

-- 
1.9.1


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

* [PATCH 1/1] gpio: sch: Consolidate similar algorithms
  2015-01-21 10:32 [PATCH 0/1] Consolidate similar algorithms in gpio-sch Chang Rebecca Swee Fun
@ 2015-01-21 10:32 ` Chang Rebecca Swee Fun
  2015-01-22  3:08   ` Alexandre Courbot
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chang Rebecca Swee Fun @ 2015-01-21 10:32 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Mika Westerberg
  Cc: Chang Rebecca Swee Fun, GPIO Subsystem Mailing List,
	Linux Kernel Mailing List

Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.

Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
---
 drivers/gpio/gpio-sch.c | 83 +++++++++++++++++--------------------------------
 1 file changed, 29 insertions(+), 54 deletions(-)

diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 0271022..b72906f 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -41,7 +41,7 @@ struct sch_gpio {
 	unsigned short resume_base;
 };
 
-#define to_sch_gpio(c)	container_of(c, struct sch_gpio, chip)
+#define to_sch_gpio(gc)	container_of(gc, struct sch_gpio, chip)
 
 static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
 				unsigned reg)
@@ -63,75 +63,59 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
 	return gpio % 8;
 }
 
-static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
 {
+	struct sch_gpio *sch = to_sch_gpio(gc);
 	unsigned short offset, bit;
-	u8 enable;
-
-	spin_lock(&sch->lock);
+	u8 reg_val;
 
-	offset = sch_gpio_offset(sch, gpio, GEN);
+	offset = sch_gpio_offset(sch, gpio, reg);
 	bit = sch_gpio_bit(sch, gpio);
 
-	enable = inb(sch->iobase + offset);
-	if (!(enable & (1 << bit)))
-		outb(enable | (1 << bit), sch->iobase + offset);
+	reg_val = !!(inb(sch->iobase + offset) & BIT(bit));
 
-	spin_unlock(&sch->lock);
+	return reg_val;
 }
 
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned  gpio_num)
+static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
+			     int val)
 {
 	struct sch_gpio *sch = to_sch_gpio(gc);
-	u8 curr_dirs;
 	unsigned short offset, bit;
+	u8 reg_val;
 
-	spin_lock(&sch->lock);
+	offset = sch_gpio_offset(sch, gpio, reg);
+	bit = sch_gpio_bit(sch, gpio);
 
-	offset = sch_gpio_offset(sch, gpio_num, GIO);
-	bit = sch_gpio_bit(sch, gpio_num);
+	reg_val = inb(sch->iobase + offset);
 
-	curr_dirs = inb(sch->iobase + offset);
+	if (val)
+		outb(reg_val | BIT(bit), sch->iobase + offset);
+	else
+		outb((reg_val & ~BIT(bit)), sch->iobase + offset);
+}
 
-	if (!(curr_dirs & (1 << bit)))
-		outb(curr_dirs | (1 << bit), sch->iobase + offset);
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+{
+	struct sch_gpio *sch = to_sch_gpio(gc);
 
+	spin_lock(&sch->lock);
+	sch_gpio_reg_set(gc, gpio_num, GIO, 1);
 	spin_unlock(&sch->lock);
 	return 0;
 }
 
 static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
 {
-	struct sch_gpio *sch = to_sch_gpio(gc);
-	int res;
-	unsigned short offset, bit;
-
-	offset = sch_gpio_offset(sch, gpio_num, GLV);
-	bit = sch_gpio_bit(sch, gpio_num);
-
-	res = !!(inb(sch->iobase + offset) & (1 << bit));
-
-	return res;
+	return sch_gpio_reg_get(gc, gpio_num, GLV);
 }
 
 static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
 {
 	struct sch_gpio *sch = to_sch_gpio(gc);
-	u8 curr_vals;
-	unsigned short offset, bit;
 
 	spin_lock(&sch->lock);
-
-	offset = sch_gpio_offset(sch, gpio_num, GLV);
-	bit = sch_gpio_bit(sch, gpio_num);
-
-	curr_vals = inb(sch->iobase + offset);
-
-	if (val)
-		outb(curr_vals | (1 << bit), sch->iobase + offset);
-	else
-		outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
-
+	sch_gpio_reg_set(gc, gpio_num, GLV, val);
 	spin_unlock(&sch->lock);
 }
 
@@ -139,18 +123,9 @@ static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
 				  int val)
 {
 	struct sch_gpio *sch = to_sch_gpio(gc);
-	u8 curr_dirs;
-	unsigned short offset, bit;
 
 	spin_lock(&sch->lock);
-
-	offset = sch_gpio_offset(sch, gpio_num, GIO);
-	bit = sch_gpio_bit(sch, gpio_num);
-
-	curr_dirs = inb(sch->iobase + offset);
-	if (curr_dirs & (1 << bit))
-		outb(curr_dirs & ~(1 << bit), sch->iobase + offset);
-
+	sch_gpio_reg_set(gc, gpio_num, GIO, 0);
 	spin_unlock(&sch->lock);
 
 	/*
@@ -209,13 +184,13 @@ static int sch_gpio_probe(struct platform_device *pdev)
 		 * GPIO7 is configured by the CMC as SLPIOVR
 		 * Enable GPIO[9:8] core powered gpios explicitly
 		 */
-		sch_gpio_enable(sch, 8);
-		sch_gpio_enable(sch, 9);
+		sch_gpio_reg_set(&sch->chip, 8, GEN, 1);
+		sch_gpio_reg_set(&sch->chip, 9, GEN, 1);
 		/*
 		 * SUS_GPIO[2:0] enabled by default
 		 * Enable SUS_GPIO3 resume powered gpio explicitly
 		 */
-		sch_gpio_enable(sch, 13);
+		sch_gpio_reg_set(&sch->chip, 13, GEN, 1);
 		break;
 
 	case PCI_DEVICE_ID_INTEL_ITC_LPC:
-- 
1.9.1


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

* Re: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
  2015-01-21 10:32 ` [PATCH 1/1] gpio: sch: Consolidate similar algorithms Chang Rebecca Swee Fun
@ 2015-01-22  3:08   ` Alexandre Courbot
  2015-01-22  9:46   ` Mika Westerberg
  2015-01-29 12:48   ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-01-22  3:08 UTC (permalink / raw)
  To: Chang Rebecca Swee Fun
  Cc: Linus Walleij, Mika Westerberg, GPIO Subsystem Mailing List,
	Linux Kernel Mailing List

On Wed, Jan 21, 2015 at 7:32 PM, Chang Rebecca Swee Fun
<rebecca.swee.fun.chang@intel.com> wrote:
> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>

Looks like there has been some confusion due to the adding/removal of
patches on Linus' branch.

I (as well as Mika IIRC) gave my Reviewed-by on an earlier version of
this patch. Has it changed that much that it needs to be reviewed from
start again? If you just fixed it to compile on Linus' latest devel
branch, you can (and should) carry tags over.

Anyway:

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

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

* Re: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
  2015-01-21 10:32 ` [PATCH 1/1] gpio: sch: Consolidate similar algorithms Chang Rebecca Swee Fun
  2015-01-22  3:08   ` Alexandre Courbot
@ 2015-01-22  9:46   ` Mika Westerberg
  2015-01-22 10:01     ` Chang, Rebecca Swee Fun
  2015-01-29 12:48   ` Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2015-01-22  9:46 UTC (permalink / raw)
  To: Chang Rebecca Swee Fun
  Cc: Linus Walleij, Alexandre Courbot, GPIO Subsystem Mailing List,
	Linux Kernel Mailing List

On Wed, Jan 21, 2015 at 06:32:21PM +0800, Chang Rebecca Swee Fun wrote:
> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
> 
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>

Like Alexandre said, you can carry the Reviewed-by: tags with the patch
if you only fixed build warning.

In future please make sure that patches compile without warnings and
that they have proper testing done beforehand. Thanks.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

* RE: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
  2015-01-22  9:46   ` Mika Westerberg
@ 2015-01-22 10:01     ` Chang, Rebecca Swee Fun
  0 siblings, 0 replies; 6+ messages in thread
From: Chang, Rebecca Swee Fun @ 2015-01-22 10:01 UTC (permalink / raw)
  To: Westerberg, Mika, Alexandre Courbot
  Cc: Linus Walleij, GPIO Subsystem Mailing List, Linux Kernel Mailing List

Hi both,

Noted with thanks!

Rebecca

> -----Original Message-----
> From: Westerberg, Mika
> Sent: 22 January, 2015 5:46 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; Alexandre Courbot; GPIO Subsystem Mailing List; Linux Kernel
> Mailing List
> Subject: Re: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
> 
> On Wed, Jan 21, 2015 at 06:32:21PM +0800, Chang Rebecca Swee Fun wrote:
> > Consolidating similar algorithms into common functions to make GPIO
> > SCH simpler and manageable.
> >
> > Signed-off-by: Chang Rebecca Swee Fun
> > <rebecca.swee.fun.chang@intel.com>
> 
> Like Alexandre said, you can carry the Reviewed-by: tags with the patch if you
> only fixed build warning.
> 
> In future please make sure that patches compile without warnings and that
> they have proper testing done beforehand. Thanks.
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

* Re: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
  2015-01-21 10:32 ` [PATCH 1/1] gpio: sch: Consolidate similar algorithms Chang Rebecca Swee Fun
  2015-01-22  3:08   ` Alexandre Courbot
  2015-01-22  9:46   ` Mika Westerberg
@ 2015-01-29 12:48   ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2015-01-29 12:48 UTC (permalink / raw)
  To: Chang Rebecca Swee Fun
  Cc: Alexandre Courbot, Mika Westerberg, GPIO Subsystem Mailing List,
	Linux Kernel Mailing List

On Wed, Jan 21, 2015 at 11:32 AM, Chang Rebecca Swee Fun
<rebecca.swee.fun.chang@intel.com> wrote:

> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>

Applied again, with the review-tags.

Yours,
Linus Walleij

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

end of thread, other threads:[~2015-01-29 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 10:32 [PATCH 0/1] Consolidate similar algorithms in gpio-sch Chang Rebecca Swee Fun
2015-01-21 10:32 ` [PATCH 1/1] gpio: sch: Consolidate similar algorithms Chang Rebecca Swee Fun
2015-01-22  3:08   ` Alexandre Courbot
2015-01-22  9:46   ` Mika Westerberg
2015-01-22 10:01     ` Chang, Rebecca Swee Fun
2015-01-29 12:48   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).