RE: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
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 > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/1] gpio: sch: Consolidate similar algorithms
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.ch...@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.westerb...@linux.intel.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
---
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(>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(>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(>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(>lock);
+ sch_gpio_reg_set(gc, gpio_num, GIO, 1);
spin_unlock(>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(>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(>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(>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(>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(>chip, 8, GEN, 1);
+ sch_gpio_reg_set(>chip, 9, GEN, 1);
/*
[PATCH 0/1] Consolidate similar algorithms in gpio-sch
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@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
[PATCH 0/1] Consolidate similar algorithms in gpio-sch
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv5 2/2] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun Reviewed-by: Mika Westerberg Reviewed-by: Alexandre Courbot --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c | 6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 414d055..24c4f83 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -394,25 +394,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate "Intel SCH/TunnelCreek/Centerton GPIO" + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" depends on PCI && X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate "Intel ICH GPIO" depends on PCI && X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 054a8ea..1495105 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -205,6 +205,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch->chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch->core_base = 0; + sch->resume_base = 2; + sch->chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv5 0/2] Enable Quark X1000 support in gpio-sch
Hi all, This is a revised version for enabling Quark X1000 support in gpio-sch. This version of patch series had changed according to the feedback provided by Alexandre and Linus. Change log for V5: Patch 1: - Change variable curr_dirs to reg_val in order to make driver code easier to understand. Patch 3: - Dropped patch 3 for now. We need to re-design the driver's IRQ implementation. The patches need to be patched on top of Mika Westerberg's commit at: gpio: sch: Consolidate core and resume banks http://marc.info/?l=linux-kernel=141392647225885=2 The patches has been verifed and tested working on Galileo Board. GPIO sysfs was able to export gpio pins and changing pin direction. GPIO values were able to controlled. One of the GPIO pins which is connected to on-board LED was used to test GPIO functionality. We are able to turn the LED on/off by changing the pin direction and pin value. Please help to review the patches once again and thanks for all the review comments. Your comments are valuable to me and help me to gain more in this Open Source community as I'm quite a newbie in this area. Thanks for your patience. Regards, Rebecca Change log for V4: Patch 1: - Removed redundant/duplicated functions of sch_gpio_register_set() and sch_gpio_register_clear(). The function call had been replaced by sch_gpio_reg_set(gc, gpio, reg, 1) in place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for sch_gpio_register_clear(). - Resolved double spinlock issue caught by Alexandre. Patch 3: - Dropped the usage of "if" block that checking irq_data struct - Restructured the irq detect by using platform_get_irq(pdev, 0) instead of platform_get_resource(pdev, IORESOURCE_IRQ, 0) to get IRQ resources from MFD LPS-SCH. Change log for V3: Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (2): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c | 87 +++-- 2 files changed, 43 insertions(+), 55 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
Reviewed-by: Mika Westerberg
Reviewed-by: Alexandre Courbot
---
drivers/gpio/gpio-sch.c | 81 +
1 file changed, 28 insertions(+), 53 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..054a8ea 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -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(>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(>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(>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(>lock);
+ sch_gpio_reg_set(sch, gpio_num, GIO, 1);
spin_unlock(>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(>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(>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(>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(sch, gpio_num, GIO, 0);
spin_unlock(>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, 8, GEN, 1);
+ sch_gpio_reg_set(sch, 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, 13, GEN, 1);
break;
case PCI_DEVICE_ID_INTEL_ITC_LPC:
[PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com
Reviewed-by: Alexandre Courbot acour...@nvidia.com
---
drivers/gpio/gpio-sch.c | 81 +
1 file changed, 28 insertions(+), 53 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..054a8ea 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -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(sch, 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(sch, 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, 8, GEN, 1);
+ sch_gpio_reg_set(sch, 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, 13, GEN, 1);
break;
case PCI_DEVICE_ID_INTEL_ITC_LPC:
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
[PATCHv5 0/2] Enable Quark X1000 support in gpio-sch
Hi all, This is a revised version for enabling Quark X1000 support in gpio-sch. This version of patch series had changed according to the feedback provided by Alexandre and Linus. Change log for V5: Patch 1: - Change variable curr_dirs to reg_val in order to make driver code easier to understand. Patch 3: - Dropped patch 3 for now. We need to re-design the driver's IRQ implementation. The patches need to be patched on top of Mika Westerberg's commit at: gpio: sch: Consolidate core and resume banks http://marc.info/?l=linux-kernelm=141392647225885w=2 The patches has been verifed and tested working on Galileo Board. GPIO sysfs was able to export gpio pins and changing pin direction. GPIO values were able to controlled. One of the GPIO pins which is connected to on-board LED was used to test GPIO functionality. We are able to turn the LED on/off by changing the pin direction and pin value. Please help to review the patches once again and thanks for all the review comments. Your comments are valuable to me and help me to gain more in this Open Source community as I'm quite a newbie in this area. Thanks for your patience. Regards, Rebecca Change log for V4: Patch 1: - Removed redundant/duplicated functions of sch_gpio_register_set() and sch_gpio_register_clear(). The function call had been replaced by sch_gpio_reg_set(gc, gpio, reg, 1) in place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for sch_gpio_register_clear(). - Resolved double spinlock issue caught by Alexandre. Patch 3: - Dropped the usage of if block that checking irq_data struct - Restructured the irq detect by using platform_get_irq(pdev, 0) instead of platform_get_resource(pdev, IORESOURCE_IRQ, 0) to get IRQ resources from MFD LPS-SCH. Change log for V3: Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (2): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c | 87 +++-- 2 files changed, 43 insertions(+), 55 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv5 2/2] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com Reviewed-by: Alexandre Courbot acour...@nvidia.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c | 6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 414d055..24c4f83 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -394,25 +394,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 054a8ea..1495105 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -205,6 +205,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch-core_base = 0; + sch-resume_base = 2; + sch-chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
Hi Linus,
Noted, while I continue to work on this IRQ feature, can I resend patch 1 and
patch 2 (rebase to devel branch) in v5 thread so that you can pull them in
first?
Regards,
Rebecca
> -Original Message-
> From: Linus Walleij [mailto:linus.wall...@linaro.org]
> Sent: 28 November, 2014 10:03 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linux Kernel Mailing List; GPIO Subsystem Mailing List; Westerberg, Mika;
> Denis Turischev; Alexandre Courbot
> Subject: Re: [PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
>
> On Wed, Nov 26, 2014 at 5:48 AM, Chang Rebecca Swee Fun
> wrote:
>
> > ntel Quark X1000 GPIO controller supports interrupt handling for both
> > core power well and resume power well. This patch is to enable the IRQ
> > support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
> > driver.
> >
> > This piece of work is derived from Dan O'Donovan's initial work for
> > Quark X1000 enabling.
> >
> > Signed-off-by: Chang Rebecca Swee Fun
> >
>
> This is just adding handling of cascading interrupts from a GPIO chip as far
> as I
> can tell. We don't do that with local per-driver hacks anymore if there is no
> special reason, we have helpers on gpiolib to handle this.
>
> Make your Kconfig select GPIOLIB_IRQCHIP and take it from there, look at how
> other drivers using GPIOLIB_IRQCHIP are done. Read the documentation for
> chained irqchips in Documentation/gpio/driver.txt
>
> Yours,
> Linus Walleij
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
Hi Linus,
Noted, while I continue to work on this IRQ feature, can I resend patch 1 and
patch 2 (rebase to devel branch) in v5 thread so that you can pull them in
first?
Regards,
Rebecca
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: 28 November, 2014 10:03 PM
To: Chang, Rebecca Swee Fun
Cc: Linux Kernel Mailing List; GPIO Subsystem Mailing List; Westerberg, Mika;
Denis Turischev; Alexandre Courbot
Subject: Re: [PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
On Wed, Nov 26, 2014 at 5:48 AM, Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com wrote:
ntel Quark X1000 GPIO controller supports interrupt handling for both
core power well and resume power well. This patch is to enable the IRQ
support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com
This is just adding handling of cascading interrupts from a GPIO chip as far
as I
can tell. We don't do that with local per-driver hacks anymore if there is no
special reason, we have helpers on gpiolib to handle this.
Make your Kconfig select GPIOLIB_IRQCHIP and take it from there, look at how
other drivers using GPIOLIB_IRQCHIP are done. Read the documentation for
chained irqchips in Documentation/gpio/driver.txt
Yours,
Linus Walleij
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
[PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
ntel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 231 +--
1 file changed, 224 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 1a465bb..498746c 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include
#include
+#include
+#include
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq;
+ int irq_base;
+ bool irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -98,10 +111,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(sch, gpio_num, GIO, 1);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
return 0;
}
@@ -113,20 +127,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(sch, gpio_num, GIO, 0);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -141,6 +157,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch->irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = "sch_gpio",
.owner = THIS_MODULE,
@@ -148,12 +171,155 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d->irq - sch->irq_base;
+ sch_gpio_reg_set(sch, gpio_num, GGPE, 1);
+}
+
+static void sch_gpio_irq_disable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d->irq - sch->irq_base;
+ sch_gpio_reg_set(sch, gpio_num, GGPE, 0);
+}
+
+static void sch_gpio_irq_ack(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d->irq - sch->irq_base;
+ sch_gpio_reg_set(&(sch->chip), gpio_num, GTS, 1);
+}
+
+static int sch_gpio_irq_type(struct irq_data *d, unsigned type)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ unsigned long flags;
+ u32 gpio_num;
+
+ gpio_num = d->irq - sch->irq_base;
+
+ spin_lock_irqsave(>lock, flags);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 1);
+ sch_gpio_reg_set(sch, gpio_num, GTNE, 0);
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ sch_gpio_reg_set(sch, gpio_num, GTNE, 1);
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 0);
+ break;
+
+ case IRQ_TYPE_EDGE_BOTH:
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 1);
+ sch_gpio_r
[PATCHv4 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun Reviewed-by: Mika Westerberg --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 414d055..24c4f83 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -394,25 +394,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate "Intel SCH/TunnelCreek/Centerton GPIO" + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" depends on PCI && X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate "Intel ICH GPIO" depends on PCI && X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 7967f14..1a465bb 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -205,6 +205,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch->chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch->core_base = 0; + sch->resume_base = 2; + sch->chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv4 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 81 ---
1 file changed, 28 insertions(+), 53 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..7967f14 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -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(>lock);
+ u8 curr_dirs;
- 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);
+ curr_dirs = !!(inb(sch->iobase + offset) & BIT(bit));
- spin_unlock(>lock);
+ return curr_dirs;
}
-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 curr_dirs;
- spin_lock(>lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
curr_dirs = inb(sch->iobase + offset);
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), sch->iobase + offset);
+ if (val)
+ outb(curr_dirs | BIT(bit), sch->iobase + offset);
+ else
+ outb((curr_dirs & ~BIT(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(>lock);
+ sch_gpio_reg_set(sch, gpio_num, GIO, 1);
spin_unlock(>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(>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(>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(>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(sch, gpio_num, GIO, 0);
spin_unlock(>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, 8, GEN, 1);
+ sch_gpio_reg_set(sch, 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, 13, GEN, 1);
break;
case PCI_DEVICE_ID_INTEL_ITC_LPC:
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-
[PATCHv4 0/3] Enable Quark X1000 support in gpio-sch
Hi all, This is a revised version for enabling Quark X1000 support in gpio-sch. This version of patch series had changed according to the feedback provided by Alexandre and Mika. Change log for V4: Patch 1: - Removed redundant/duplicated functions of sch_gpio_register_set() and sch_gpio_register_clear(). The function call had been replaced by sch_gpio_reg_set(gc, gpio, reg, 1) in place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for sch_gpio_register_clear(). - Resolved double spinlock issue caught by Alexandre. Patch 3: - Dropped the usage of "if" block that checking irq_data struct - Restructured the irq detect by using platform_get_irq(pdev, 0) instead of platform_get_resource(pdev, IORESOURCE_IRQ, 0) to get IRQ resources from MFD LPS-SCH. The patches need to be patched on top of Mika Westerberg's commit at: gpio: sch: Consolidate core and resume banks http://marc.info/?l=linux-kernel=141392647225885=2 The patches has been verifed and tested working on Galileo Board. GPIO sysfs was able to export gpio pins and changing pin direction. GPIO values were able to controlled and interrupt was enabled. Please help to review the patches so that we can make it to upstream kernel as soon as possible. Thanks for all the review comments during this period of review cycle. Regards, Rebecca Change log for V3: Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 316 ++- 2 files changed, 266 insertions(+), 61 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv4 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 81 ---
1 file changed, 28 insertions(+), 53 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..7967f14 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -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 curr_dirs;
- 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);
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- spin_unlock(sch-lock);
+ return curr_dirs;
}
-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 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
curr_dirs = inb(sch-iobase + offset);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ if (val)
+ outb(curr_dirs | BIT(bit), sch-iobase + offset);
+ else
+ outb((curr_dirs ~BIT(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(sch, 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(sch, 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, 8, GEN, 1);
+ sch_gpio_reg_set(sch, 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, 13, GEN, 1);
break;
case PCI_DEVICE_ID_INTEL_ITC_LPC:
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ
[PATCHv4 0/3] Enable Quark X1000 support in gpio-sch
Hi all, This is a revised version for enabling Quark X1000 support in gpio-sch. This version of patch series had changed according to the feedback provided by Alexandre and Mika. Change log for V4: Patch 1: - Removed redundant/duplicated functions of sch_gpio_register_set() and sch_gpio_register_clear(). The function call had been replaced by sch_gpio_reg_set(gc, gpio, reg, 1) in place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for sch_gpio_register_clear(). - Resolved double spinlock issue caught by Alexandre. Patch 3: - Dropped the usage of if block that checking irq_data struct - Restructured the irq detect by using platform_get_irq(pdev, 0) instead of platform_get_resource(pdev, IORESOURCE_IRQ, 0) to get IRQ resources from MFD LPS-SCH. The patches need to be patched on top of Mika Westerberg's commit at: gpio: sch: Consolidate core and resume banks http://marc.info/?l=linux-kernelm=141392647225885w=2 The patches has been verifed and tested working on Galileo Board. GPIO sysfs was able to export gpio pins and changing pin direction. GPIO values were able to controlled and interrupt was enabled. Please help to review the patches so that we can make it to upstream kernel as soon as possible. Thanks for all the review comments during this period of review cycle. Regards, Rebecca Change log for V3: Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 316 ++- 2 files changed, 266 insertions(+), 61 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv4 3/3] gpio: sch: Enable IRQ support for Quark X1000
ntel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 231 +--
1 file changed, 224 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 1a465bb..498746c 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include linux/pci_ids.h
#include linux/gpio.h
+#include linux/interrupt.h
+#include linux/irq.h
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq;
+ int irq_base;
+ bool irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -98,10 +111,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(sch, gpio_num, GIO, 1);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
return 0;
}
@@ -113,20 +127,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(sch, gpio_num, GIO, 0);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -141,6 +157,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch-irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = sch_gpio,
.owner = THIS_MODULE,
@@ -148,12 +171,155 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_reg_set(sch, gpio_num, GGPE, 1);
+}
+
+static void sch_gpio_irq_disable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_reg_set(sch, gpio_num, GGPE, 0);
+}
+
+static void sch_gpio_irq_ack(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_reg_set((sch-chip), gpio_num, GTS, 1);
+}
+
+static int sch_gpio_irq_type(struct irq_data *d, unsigned type)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ unsigned long flags;
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+
+ spin_lock_irqsave(sch-lock, flags);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 1);
+ sch_gpio_reg_set(sch, gpio_num, GTNE, 0);
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ sch_gpio_reg_set(sch, gpio_num, GTNE, 1);
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 0);
+ break;
+
+ case IRQ_TYPE_EDGE_BOTH:
+ sch_gpio_reg_set(sch, gpio_num, GTPE, 1
[PATCHv4 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 414d055..24c4f83 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -394,25 +394,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 7967f14..1a465bb 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -205,6 +205,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch-core_base = 0; + sch-resume_base = 2; + sch-chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
Sorry for the late reply, was working on something else.
> -Original Message-
> From: Westerberg, Mika
> Sent: 16 October, 2014 6:19 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; GPIO Subsystem Mailing List; Linux Kernel Mailing List;
> Denis
> Turischev
> Subject: Re: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
>
> On Thu, Oct 16, 2014 at 11:51:15AM +0800, Chang Rebecca Swee Fun wrote:
> > Intel Quark X1000 GPIO controller supports interrupt handling for both
> > core power well and resume power well. This patch is to enable the IRQ
> > support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
> > driver.
> >
> > This piece of work is derived from Dan O'Donovan's initial work for
> > Quark X1000 enabling.
> >
> > Signed-off-by: Chang Rebecca Swee Fun
> >
>
> In addition to what Varka Bhadram pointed out, I have few comments. See
> below.
>
> Overall, looks good.
>
> > ---
> > drivers/gpio/gpio-sch.c | 257
> > ---
> > 1 file changed, 245 insertions(+), 12 deletions(-)
> >
(...)
> > +static void sch_gpio_irq_disable(struct irq_data *d) {
> > + struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
> > + u32 gpio_num;
> > +
> > + gpio_num = d->irq - sch->irq_base;
> > + sch_gpio_register_clear(sch, gpio_num, GGPE); }
> > +
> > +static void sch_gpio_irq_ack(struct irq_data *d) {
> > + struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
> > + u32 gpio_num;
> > +
> > + gpio_num = d->irq - sch->irq_base;
> > + sch_gpio_reg_set(&(sch->chip), gpio_num, GTS, 1);
>
> Maybe use the new sch_gpio_register_set() here instead?
According to Alexandre's review feedback, sch_gpio_register_set() and
sch_gpio_register_clear() actually having similar block of codes with
sch_gpio_reg_set(). He suggested to call sch_gpio_reg_set(gc, gpio, reg, 1) in
place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for
sch_gpio_register_clear(). I'm now in the progress of reworking the patches in
that direction.
>
> > +}
> > +
> > +static int sch_gpio_irq_type(struct irq_data *d, unsigned type) {
> > + struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
> > + unsigned long flags;
> > + u32 gpio_num;
> > +
> > + if (d == NULL)
> > + return -EINVAL;
>
> How can that happen?
This is just to ensure the irq_data struct contains data. If you think this is
no needed, I will remove it in next submission.
(...)
> > +
> > +static void sch_gpio_irqs_init(struct sch_gpio *sch, unsigned int
> > +num) {
> > + unsigned int i;
> > +
> > + for (i = 0; i < num; i++) {
> > + irq_set_chip_data(i + sch->irq_base, sch);
> > + irq_set_chip_and_handler_name(i + sch->irq_base,
> > + _irq_chip,
> > + handle_simple_irq,
> > + "sch_gpio_irq_chip");
>
> Hmm, I wonder if this
>
> irq_set_chip_and_handler_name(i + sch->irq_base,
> _irq_chip,
> handle_simple_irq,
> "sch_gpio_irq_chip");
>
> looks better. Up to you.
>
I think I will take what you've suggested. :)
(...)
> > static int sch_gpio_probe(struct platform_device *pdev) {
> > struct sch_gpio *sch;
> > - struct resource *res;
> > + struct resource *res, *irq;
> > + int err;
> >
> > sch = devm_kzalloc(>dev, sizeof(*sch), GFP_KERNEL);
> > if (!sch)
> > @@ -203,6 +376,17 @@ static int sch_gpio_probe(struct platform_device
> *pdev)
> > pdev->name))
> > return -EBUSY;
> >
> > + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + sch->irq_support = !!irq;
> > + if (sch->irq_support) {
> > + sch->irq_num = irq->start;
> > + if (sch->irq_num < 0) {
>
> Is this really possible? I mean can't you just detect the irq support by
> looking
> sch->irq_num? If it is > 0 then it has interrupt support. I think that irq 0
> is not
> valid.
Yes, I can do that, but the irq_num was being passed from MFD LPC_SCH. This
implementation is to avoid enabling irq support if LPC_SCH passed in a negative
value.
As for irq 0, I think it is valid. Based on my observation in "cat
/proc/interrupt" there is a 0 irq line available.
>
> > + de
RE: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
Sorry for the late reply, was working on something else.
-Original Message-
From: Westerberg, Mika
Sent: 16 October, 2014 6:19 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; GPIO Subsystem Mailing List; Linux Kernel Mailing List;
Denis
Turischev
Subject: Re: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
On Thu, Oct 16, 2014 at 11:51:15AM +0800, Chang Rebecca Swee Fun wrote:
Intel Quark X1000 GPIO controller supports interrupt handling for both
core power well and resume power well. This patch is to enable the IRQ
support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com
In addition to what Varka Bhadram pointed out, I have few comments. See
below.
Overall, looks good.
---
drivers/gpio/gpio-sch.c | 257
---
1 file changed, 245 insertions(+), 12 deletions(-)
(...)
+static void sch_gpio_irq_disable(struct irq_data *d) {
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_register_clear(sch, gpio_num, GGPE); }
+
+static void sch_gpio_irq_ack(struct irq_data *d) {
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_reg_set((sch-chip), gpio_num, GTS, 1);
Maybe use the new sch_gpio_register_set() here instead?
According to Alexandre's review feedback, sch_gpio_register_set() and
sch_gpio_register_clear() actually having similar block of codes with
sch_gpio_reg_set(). He suggested to call sch_gpio_reg_set(gc, gpio, reg, 1) in
place of sch_gpio_register_set() and sch_gpio_reg_set(gc, gpio, reg, 0) for
sch_gpio_register_clear(). I'm now in the progress of reworking the patches in
that direction.
+}
+
+static int sch_gpio_irq_type(struct irq_data *d, unsigned type) {
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ unsigned long flags;
+ u32 gpio_num;
+
+ if (d == NULL)
+ return -EINVAL;
How can that happen?
This is just to ensure the irq_data struct contains data. If you think this is
no needed, I will remove it in next submission.
(...)
+
+static void sch_gpio_irqs_init(struct sch_gpio *sch, unsigned int
+num) {
+ unsigned int i;
+
+ for (i = 0; i num; i++) {
+ irq_set_chip_data(i + sch-irq_base, sch);
+ irq_set_chip_and_handler_name(i + sch-irq_base,
+ sch_irq_chip,
+ handle_simple_irq,
+ sch_gpio_irq_chip);
Hmm, I wonder if this
irq_set_chip_and_handler_name(i + sch-irq_base,
sch_irq_chip,
handle_simple_irq,
sch_gpio_irq_chip);
looks better. Up to you.
I think I will take what you've suggested. :)
(...)
static int sch_gpio_probe(struct platform_device *pdev) {
struct sch_gpio *sch;
- struct resource *res;
+ struct resource *res, *irq;
+ int err;
sch = devm_kzalloc(pdev-dev, sizeof(*sch), GFP_KERNEL);
if (!sch)
@@ -203,6 +376,17 @@ static int sch_gpio_probe(struct platform_device
*pdev)
pdev-name))
return -EBUSY;
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ sch-irq_support = !!irq;
+ if (sch-irq_support) {
+ sch-irq_num = irq-start;
+ if (sch-irq_num 0) {
Is this really possible? I mean can't you just detect the irq support by
looking
sch-irq_num? If it is 0 then it has interrupt support. I think that irq 0
is not
valid.
Yes, I can do that, but the irq_num was being passed from MFD LPC_SCH. This
implementation is to avoid enabling irq support if LPC_SCH passed in a negative
value.
As for irq 0, I think it is valid. Based on my observation in cat
/proc/interrupt there is a 0 irq line available.
+ dev_warn(pdev-dev,
+failed to obtain irq number for device\n);
+ sch-irq_support = false;
+ }
+ }
+
spin_lock_init(sch-lock);
sch-iobase = res-start;
sch-chip = sch_gpio_chip;
@@ -251,15 +435,64 @@ static int sch_gpio_probe(struct platform_device
*pdev)
return -ENODEV;
}
+ err = gpiochip_add(sch-chip);
+ if (err 0)
+ goto err_sch_gpio;
+
+ if (sch-irq_support) {
+ sch-irq_base = irq_alloc_descs(-1, 0, sch-chip.ngpio,
+ NUMA_NO_NODE);
+ if (sch-irq_base 0) {
+ dev_err(pdev-dev,
+ failed to allocate GPIO IRQ
RE: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
> > +
> > +static int sch_gpio_irq_type(struct irq_data *d, unsigned type)
> > +{
> > + struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
> > + unsigned long flags;
> > + u32 gpio_num;
> > +
> > + if (d == NULL)
> > + return -EINVAL;
> > +
> > + gpio_num = d->irq - sch->irq_base;
> > +
> > + spin_lock_irqsave(>lock, flags);
> > +
> > + switch (type) {
> > + case IRQ_TYPE_EDGE_RISING:
> > + sch_gpio_register_set(sch, gpio_num, GTPE);
> > + sch_gpio_register_clear(sch, gpio_num, GTNE);
>
> You will have the same problem as I pointed out in patch 1/3 that
> sch_gpio_register_set/clear() will try to acquire the already-locked
> sch->lock. No way this can ever work, or I am under a serious
> misapprehension.
>
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_FALLING:
> > + sch_gpio_register_set(sch, gpio_num, GTNE);
> > + sch_gpio_register_clear(sch, gpio_num, GTPE);
> > + break;
> > +
> > + case IRQ_TYPE_EDGE_BOTH:
> > + sch_gpio_register_set(sch, gpio_num, GTPE);
> > + sch_gpio_register_set(sch, gpio_num, GTNE);
> > + break;
> > +
> > + case IRQ_TYPE_NONE:
> > + sch_gpio_register_clear(sch, gpio_num, GTPE);
> > + sch_gpio_register_clear(sch, gpio_num, GTNE);
> > + break;
> > +
> > + default:
> > + spin_unlock_irqrestore(>lock, flags);
> > + return -EINVAL;
> > + }
> > +
> > + spin_unlock_irqrestore(>lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static struct irq_chip sch_irq_chip = {
> > + .irq_enable = sch_gpio_irq_enable,
> > + .irq_disable= sch_gpio_irq_disable,
> > + .irq_ack= sch_gpio_irq_ack,
> > + .irq_set_type = sch_gpio_irq_type,
> > +};
> > +
> > +static void sch_gpio_irqs_init(struct sch_gpio *sch, unsigned int num)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < num; i++) {
> > + irq_set_chip_data(i + sch->irq_base, sch);
> > + irq_set_chip_and_handler_name(i + sch->irq_base,
> > + _irq_chip,
> > + handle_simple_irq,
> > + "sch_gpio_irq_chip");
> > + }
> > +}
> > +
> > +static void sch_gpio_irqs_deinit(struct sch_gpio *sch, unsigned int num)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < num; i++) {
> > + irq_set_chip_data(i + sch->irq_base, 0);
> > + irq_set_chip_and_handler_name(i + sch->irq_base, 0, 0, 0);
> > + }
> > +}
> > +
> > +static void sch_gpio_irq_disable_all(struct sch_gpio *sch, unsigned int
> > num)
> > +{
> > + unsigned long flags;
> > + unsigned int gpio_num;
> > +
> > + spin_lock_irqsave(>lock, flags);
> > +
> > + for (gpio_num = 0; gpio_num < num; gpio_num++) {
> > + sch_gpio_register_clear(sch, gpio_num, GTPE);
> > + sch_gpio_register_clear(sch, gpio_num, GTNE);
> > + sch_gpio_register_clear(sch, gpio_num, GGPE);
> > + sch_gpio_register_clear(sch, gpio_num, GSMI);
>
> Same here.
Alright. I should have noticed this double locking issue earlier. Thanks. I
think the next version will be much cleaner. Thanks for spending time and
effort to review.
Rebecca
RE: [PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
Thanks for the review comments. Please check my reply below.
> -Original Message-
> From: Alexandre Courbot [mailto:gnu...@gmail.com]
> Sent: 17 October, 2014 4:44 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel
> Mailing List; Denis Turischev
> Subject: Re: [PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
>
> On Thu, Oct 16, 2014 at 12:51 PM, 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
> >
> > ---
> > drivers/gpio/gpio-sch.c | 95
> > ++
> -
> > 1 file changed, 53 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index
> > 99720c8..6e89be9 100644
> > --- a/drivers/gpio/gpio-sch.c
> > +++ b/drivers/gpio/gpio-sch.c
> > @@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
> > + unsigned reg)
> > {
> > unsigned short offset, bit;
> > u8 enable;
> >
> > spin_lock(>lock);
>
> ...
>
> >
> > - 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);
>
> I see identical blocks of code in sch_gpio_register_clear(),
> sch_gpio_reg_get() and sch_gpio_reg_set() (and possibly other functions?).
> Maybe this could be factorized into an inline function that would return the
> "enable" variable?
>
> Also, "enable" looks like the wrong name here. The exact same result is later
> called "disable" and "curr_dirs" later.
Referring to your comments below, this will be get rid of after the
sch_gpio_register_set() and sch_gpio_register_clear() being replaced by
sch_gpio_reg_set(gc, gpio, reg, 1) and sch_gpio_reg_set(gc, gpio, reg, 0).
There will be less identical block of "offset", "bit", and "enable" nor
"disable". So there is no need to factorize the identical blocks into inline
function.
>
> > - if (!(enable & (1 << bit)))
> > - outb(enable | (1 << bit), sch->iobase + offset);
> > + if (!(enable & BIT(bit)))
> > + outb(enable | BIT(bit), sch->iobase + offset);
> >
> > spin_unlock(>lock);
> > }
> >
> > -static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned
> > gpio_num)
> > +static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
> > + unsigned reg)
> > {
> > - struct sch_gpio *sch = to_sch_gpio(gc);
> > - u8 curr_dirs;
> > unsigned short offset, bit;
> > + u8 disable;
> >
> > spin_lock(>lock);
> >
> > - offset = sch_gpio_offset(sch, gpio_num, GIO);
> > - bit = sch_gpio_bit(sch, gpio_num);
> > -
> > - curr_dirs = inb(sch->iobase + offset);
> > + offset = sch_gpio_offset(sch, gpio, reg);
> > + bit = sch_gpio_bit(sch, gpio);
> >
> > - if (!(curr_dirs & (1 << bit)))
> > - outb(curr_dirs | (1 << bit), sch->iobase + offset);
> > + disable = inb(sch->iobase + offset);
> > + if (disable & BIT(bit))
> > + outb(disable & ~BIT(bit), sch->iobase + offset);
> >
> > spin_unlock(>lock);
> > - return 0;
> > }
> >
> > -static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
> > +static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio,
> > +unsigned reg)
> > {
> > struct sch_gpio *sch = to_sch_gpio(gc);
> > - int res;
> > unsigned short offset, bit;
> > + u8 curr_dirs;
> >
> > - offset = sch_gpio_offset(sch, gpio_num, GLV);
> > - bit = sch_gpio_bit(sch, gpio_num);
> > + offset = sch_gpio_offset(sch, gpio, reg);
> > + bit = sch_gpio_bit(sch, gpio);
> >
> > - res = !!(inb(sch->iobase + offset) & (1 << bit));
> > + curr_dirs = !!(inb(sch->io
RE: [PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
Thanks for the review comments. Please check my reply below.
-Original Message-
From: Alexandre Courbot [mailto:gnu...@gmail.com]
Sent: 17 October, 2014 4:44 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel
Mailing List; Denis Turischev
Subject: Re: [PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
On Thu, Oct 16, 2014 at 12:51 PM, Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@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.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95
++
-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index
99720c8..6e89be9 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(sch-lock);
...
- 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);
I see identical blocks of code in sch_gpio_register_clear(),
sch_gpio_reg_get() and sch_gpio_reg_set() (and possibly other functions?).
Maybe this could be factorized into an inline function that would return the
enable variable?
Also, enable looks like the wrong name here. The exact same result is later
called disable and curr_dirs later.
Referring to your comments below, this will be get rid of after the
sch_gpio_register_set() and sch_gpio_register_clear() being replaced by
sch_gpio_reg_set(gc, gpio, reg, 1) and sch_gpio_reg_set(gc, gpio, reg, 0).
There will be less identical block of offset, bit, and enable nor
disable. So there is no need to factorize the identical blocks into inline
function.
- if (!(enable (1 bit)))
- outb(enable | (1 bit), sch-iobase + offset);
+ if (!(enable BIT(bit)))
+ outb(enable | BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned
gpio_num)
+static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
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);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ disable = inb(sch-iobase + offset);
+ if (disable BIT(bit))
+ outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio,
+unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch-iobase + offset) (1 bit));
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int
val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch-iobase + offset);
+ curr_dirs = inb(sch-iobase + offset);
if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
+ outb(curr_dirs | BIT(bit), sch-iobase + offset
RE: [PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
+
+static int sch_gpio_irq_type(struct irq_data *d, unsigned type)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ unsigned long flags;
+ u32 gpio_num;
+
+ if (d == NULL)
+ return -EINVAL;
+
+ gpio_num = d-irq - sch-irq_base;
+
+ spin_lock_irqsave(sch-lock, flags);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ sch_gpio_register_set(sch, gpio_num, GTPE);
+ sch_gpio_register_clear(sch, gpio_num, GTNE);
You will have the same problem as I pointed out in patch 1/3 that
sch_gpio_register_set/clear() will try to acquire the already-locked
sch-lock. No way this can ever work, or I am under a serious
misapprehension.
+ break;
+
+ case IRQ_TYPE_EDGE_FALLING:
+ sch_gpio_register_set(sch, gpio_num, GTNE);
+ sch_gpio_register_clear(sch, gpio_num, GTPE);
+ break;
+
+ case IRQ_TYPE_EDGE_BOTH:
+ sch_gpio_register_set(sch, gpio_num, GTPE);
+ sch_gpio_register_set(sch, gpio_num, GTNE);
+ break;
+
+ case IRQ_TYPE_NONE:
+ sch_gpio_register_clear(sch, gpio_num, GTPE);
+ sch_gpio_register_clear(sch, gpio_num, GTNE);
+ break;
+
+ default:
+ spin_unlock_irqrestore(sch-lock, flags);
+ return -EINVAL;
+ }
+
+ spin_unlock_irqrestore(sch-lock, flags);
+
+ return 0;
+}
+
+static struct irq_chip sch_irq_chip = {
+ .irq_enable = sch_gpio_irq_enable,
+ .irq_disable= sch_gpio_irq_disable,
+ .irq_ack= sch_gpio_irq_ack,
+ .irq_set_type = sch_gpio_irq_type,
+};
+
+static void sch_gpio_irqs_init(struct sch_gpio *sch, unsigned int num)
+{
+ unsigned int i;
+
+ for (i = 0; i num; i++) {
+ irq_set_chip_data(i + sch-irq_base, sch);
+ irq_set_chip_and_handler_name(i + sch-irq_base,
+ sch_irq_chip,
+ handle_simple_irq,
+ sch_gpio_irq_chip);
+ }
+}
+
+static void sch_gpio_irqs_deinit(struct sch_gpio *sch, unsigned int num)
+{
+ unsigned int i;
+
+ for (i = 0; i num; i++) {
+ irq_set_chip_data(i + sch-irq_base, 0);
+ irq_set_chip_and_handler_name(i + sch-irq_base, 0, 0, 0);
+ }
+}
+
+static void sch_gpio_irq_disable_all(struct sch_gpio *sch, unsigned int
num)
+{
+ unsigned long flags;
+ unsigned int gpio_num;
+
+ spin_lock_irqsave(sch-lock, flags);
+
+ for (gpio_num = 0; gpio_num num; gpio_num++) {
+ sch_gpio_register_clear(sch, gpio_num, GTPE);
+ sch_gpio_register_clear(sch, gpio_num, GTNE);
+ sch_gpio_register_clear(sch, gpio_num, GGPE);
+ sch_gpio_register_clear(sch, gpio_num, GSMI);
Same here.
Alright. I should have noticed this double locking issue earlier. Thanks. I
think the next version will be much cleaner. Thanks for spending time and
effort to review.
Rebecca
[PATCHv3 0/3] Enable Quark X1000 support in gpio-sch
Hi, This is a revised version for enabling Quark X1000 in gpio-sch. Change log for V3: The patches had been rebased on "devel" branch and it has dependency on Mika Westerberg's commit at: https://lkml.org/lkml/2014/9/16/213 Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. The changes had been verified by passing build test and functional test on Intel Galileo board. Thank you. Regards, Rebecca Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 353 --- 2 files changed, 311 insertions(+), 53 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 0959ca9..0e60f93 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -371,25 +371,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate "Intel SCH/TunnelCreek/Centerton GPIO" + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" depends on PCI && X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate "Intel ICH GPIO" depends on PCI && X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 6e89be9..952990f 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch->chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch->core_base = 0; + sch->resume_base = 2; + sch->chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 257 ---
1 file changed, 245 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 952990f..dd84b1f 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include
#include
+#include
+#include
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ bool irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -66,10 +79,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -78,16 +92,17 @@ static void sch_gpio_register_set(struct sch_gpio *sch,
unsigned gpio,
if (!(enable & BIT(bit)))
outb(enable | BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_register_clear(struct sch_gpio *sch,
unsigned gpio,
if (disable & BIT(bit))
outb(disable & ~BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_register_set(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
return 0;
}
@@ -149,20 +165,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_register_clear(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -177,6 +195,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch->irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = "sch_gpio",
.owner = THIS_MODULE,
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
};
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio
[PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..6e89be9 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(>lock);
- 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);
+ if (!(enable & BIT(bit)))
+ outb(enable | BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
spin_lock(>lock);
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_dirs = inb(sch->iobase + offset);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), sch->iobase + offset);
+ disable = inb(sch->iobase + offset);
+ if (disable & BIT(bit))
+ outb(disable & ~BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch->iobase + offset) & (1 << bit));
+ curr_dirs = !!(inb(sch->iobase + offset) & BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(>lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch->iobase + offset);
+ curr_dirs = inb(sch->iobase + offset);
if (val)
- outb(curr_vals | (1 << bit), sch->iobase + offset);
+ outb(curr_dirs | BIT(bit), sch->iobase + offset);
else
- outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
+ outb((curr_dirs & ~BIT(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(>lock);
+ sch_gpio_register_set(sch, gpio_num, GIO);
spin_unlock(>lock);
+ return 0;
}
-static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
- int val)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+{
+ 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_dirs;
- unsigned short offset, bit;
spin_lock(>lock);
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
+ spin_unlock(>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);
+static int sch_gpio_direction_out(struct gpio_chip *gc,
RE: [PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
> -Original Message-
> From: Linus Walleij [mailto:linus.wall...@linaro.org]
> Sent: 15 October, 2014 3:13 PM
> To: Chang, Rebecca Swee Fun; Denis Turischev
> Cc: Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel Mailing List
> Subject: Re: [PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
>
> On Fri, Sep 26, 2014 at 12:07 PM, Chang Rebecca Swee Fun
> wrote:
>
> > Intel Quark X1000 GPIO controller supports interrupt handling for both
> > core power well and resume power well. This patch is to enable the IRQ
> > support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
> > driver.
> >
> > This piece of work is derived from Dan O'Donovan's initial work for
> > Quark X1000 enabling.
> >
> > Signed-off-by: Chang Rebecca Swee Fun
> >
> (...)
>
> This patch needs to be rebased on the gpio git "devel" branch or Torvalds'
> HEAD before I can apply it.
I will rebase and resend with the fixes below.
>
> > #define GEN0x00
> > #define GIO0x04
> > #define GLV0x08
> > +#define GTPE 0x0C
> > +#define GTNE 0x10
> > +#define GGPE 0x14
> > +#define GSMI 0x18
> > +#define GTS0x1C
> > +#define CGNMIEN0x40
> > +#define RGNMIEN0x44
>
> So the initial SCH driver for the Intel Poulsbo was submitted by Denis
> Turischev
> in 2010.
>
> Does these registers exist and work on the Poulsbo as well?
>
> Is it really enough to distinguish between these variants by checking if we're
> getting an IRQ resource on the device or not?
> Is there some version register or so?
The register values defined here are offset value, they are not the exact
register address.
They are not version register as it just carries a register offset value.
> > struct sch_gpio {
> > struct gpio_chip chip;
> > + struct irq_data data;
> > spinlock_t lock;
> > unsigned short iobase;
> > unsigned short core_base;
> > unsigned short resume_base;
> > + int irq_base;
> > + int irq_num;
> > + int irq_support;
>
> Isn't that a bool?
>
> > + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + sch->irq_support = !!irq;
>
> Yeah, it's a bool
I will change it to bool.
>
> > + if (sch->irq_support) {
> > + sch->irq_num = irq->start;
> > + if (sch->irq_num < 0) {
> > + dev_warn(>dev,
> > +"failed to obtain irq number for
> > device\n");
> > + sch->irq_support = 0;
>
> = false;
Noted
>
> > + if (sch->irq_support) {
> > + sch->irq_base = irq_alloc_descs(-1, 0, sch->chip.ngpio,
> > + NUMA_NO_NODE);
> > + if (sch->irq_base < 0) {
> > + dev_err(>dev,
> > + "failed to add GPIO IRQ descs\n");
>
> Failed to *allocate* actually...
>
> > + sch->irq_base = -1;
>
> This is overzealous. Drop it.
>
> > + goto err_sch_intr_chip;
>
> You're bailing out anyway, see.
Noted. I will change the phrase accordingly and remove the expression on next
submission.
>
> > static int sch_gpio_remove(struct platform_device *pdev) {
> > struct sch_gpio *sch = platform_get_drvdata(pdev);
> > + int err;
> >
> > - gpiochip_remove(>chip);
> > - return 0;
> > + if (sch->irq_support) {
> > + sch_gpio_irqs_deinit(sch, sch->chip.ngpio);
> > +
> > + if (sch->irq_num >= 0)
> > + free_irq(sch->irq_num, sch);
> > +
> > + irq_free_descs(sch->irq_base, sch->chip.ngpio);
> > + }
> > +
> > + err = gpiochip_remove(>chip);
> > + if (err)
> > + dev_err(>dev,
> > + "%s gpiochip_remove() failed\n", __func__);
>
> So gpiochip_remove() does *NOT* return an error in the current
> kernel. We just removed that return value from the SCH driver in the
> previous cycle for the reason that we were killing off the return type.
> commit 9f5132ae82fdbb047cc187bf689a81c8cc0de7fa
> "gpio: remove all usage of gpio_remove retval in driver/gpio"
>
> So don't reintroduce stuff we're actively trying to get rid of.
>
> Apart from this is looks OK, Mika can you ACK the end result?
Noted with thanks. I will do the changes required and resend the series.
Thanks.
Regards
Rebecca
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
RE: [PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
-Original Message-
From: Linus Walleij [mailto:linus.wall...@linaro.org]
Sent: 15 October, 2014 3:13 PM
To: Chang, Rebecca Swee Fun; Denis Turischev
Cc: Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel Mailing List
Subject: Re: [PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
On Fri, Sep 26, 2014 at 12:07 PM, Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com wrote:
Intel Quark X1000 GPIO controller supports interrupt handling for both
core power well and resume power well. This patch is to enable the IRQ
support and provide IRQ handling for Intel Quark X1000 GPIO-SCH device
driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com
(...)
This patch needs to be rebased on the gpio git devel branch or Torvalds'
HEAD before I can apply it.
I will rebase and resend with the fixes below.
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
So the initial SCH driver for the Intel Poulsbo was submitted by Denis
Turischev
in 2010.
Does these registers exist and work on the Poulsbo as well?
Is it really enough to distinguish between these variants by checking if we're
getting an IRQ resource on the device or not?
Is there some version register or so?
The register values defined here are offset value, they are not the exact
register address.
They are not version register as it just carries a register offset value.
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ int irq_support;
Isn't that a bool?
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ sch-irq_support = !!irq;
Yeah, it's a bool
I will change it to bool.
+ if (sch-irq_support) {
+ sch-irq_num = irq-start;
+ if (sch-irq_num 0) {
+ dev_warn(pdev-dev,
+failed to obtain irq number for
device\n);
+ sch-irq_support = 0;
= false;
Noted
+ if (sch-irq_support) {
+ sch-irq_base = irq_alloc_descs(-1, 0, sch-chip.ngpio,
+ NUMA_NO_NODE);
+ if (sch-irq_base 0) {
+ dev_err(pdev-dev,
+ failed to add GPIO IRQ descs\n);
Failed to *allocate* actually...
+ sch-irq_base = -1;
This is overzealous. Drop it.
+ goto err_sch_intr_chip;
You're bailing out anyway, see.
Noted. I will change the phrase accordingly and remove the expression on next
submission.
static int sch_gpio_remove(struct platform_device *pdev) {
struct sch_gpio *sch = platform_get_drvdata(pdev);
+ int err;
- gpiochip_remove(sch-chip);
- return 0;
+ if (sch-irq_support) {
+ sch_gpio_irqs_deinit(sch, sch-chip.ngpio);
+
+ if (sch-irq_num = 0)
+ free_irq(sch-irq_num, sch);
+
+ irq_free_descs(sch-irq_base, sch-chip.ngpio);
+ }
+
+ err = gpiochip_remove(sch-chip);
+ if (err)
+ dev_err(pdev-dev,
+ %s gpiochip_remove() failed\n, __func__);
So gpiochip_remove() does *NOT* return an error in the current
kernel. We just removed that return value from the SCH driver in the
previous cycle for the reason that we were killing off the return type.
commit 9f5132ae82fdbb047cc187bf689a81c8cc0de7fa
gpio: remove all usage of gpio_remove retval in driver/gpio
So don't reintroduce stuff we're actively trying to get rid of.
Apart from this is looks OK, Mika can you ACK the end result?
Noted with thanks. I will do the changes required and resend the series.
Thanks.
Regards
Rebecca
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
[PATCHv3 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..6e89be9 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(sch-lock);
- 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);
+ if (!(enable BIT(bit)))
+ outb(enable | BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
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);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ disable = inb(sch-iobase + offset);
+ if (disable BIT(bit))
+ outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch-iobase + offset) (1 bit));
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch-iobase + offset);
+ curr_dirs = inb(sch-iobase + offset);
if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
+ outb(curr_dirs | BIT(bit), sch-iobase + offset);
else
- outb((curr_vals ~(1 bit)), sch-iobase + offset);
+ outb((curr_dirs ~BIT(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_register_set(sch, gpio_num, GIO);
spin_unlock(sch-lock);
+ return 0;
}
-static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
- int val)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+{
+ 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_dirs;
- unsigned short offset, bit;
spin_lock(sch-lock);
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
+ spin_unlock(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);
+static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
+ int val)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ spin_lock(sch-lock
[PATCHv3 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 257 ---
1 file changed, 245 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 952990f..dd84b1f 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include linux/pci_ids.h
#include linux/gpio.h
+#include linux/interrupt.h
+#include linux/irq.h
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ bool irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -66,10 +79,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -78,16 +92,17 @@ static void sch_gpio_register_set(struct sch_gpio *sch,
unsigned gpio,
if (!(enable BIT(bit)))
outb(enable | BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_register_clear(struct sch_gpio *sch,
unsigned gpio,
if (disable BIT(bit))
outb(disable ~BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_register_set(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
return 0;
}
@@ -149,20 +165,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_register_clear(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -177,6 +195,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch-irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = sch_gpio,
.owner = THIS_MODULE,
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
};
+static void sch_gpio_irq_enable
[PATCHv3 0/3] Enable Quark X1000 support in gpio-sch
Hi, This is a revised version for enabling Quark X1000 in gpio-sch. Change log for V3: The patches had been rebased on devel branch and it has dependency on Mika Westerberg's commit at: https://lkml.org/lkml/2014/9/16/213 Patch 3: - Change variable type of irq_support to bool. - Update error message and remove redundant code. - Revert gpiochip_remove() to avoid it to return a value. The changes had been verified by passing build test and functional test on Intel Galileo board. Thank you. Regards, Rebecca Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: Initial version. Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 353 --- 2 files changed, 311 insertions(+), 53 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv3 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 0959ca9..0e60f93 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -371,25 +371,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 6e89be9..952990f 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch-core_base = 0; + sch-resume_base = 2; + sch-chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv2 0/3] Enable Quark X1000 support in gpio-sch
> -Original Message- > From: Alexandre Courbot [mailto:gnu...@gmail.com] > Sent: 09 October, 2014 2:29 PM > To: Chang, Rebecca Swee Fun > Cc: Linus Walleij; Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel > Mailing List > Subject: Re: [PATCHv2 0/3] Enable Quark X1000 support in gpio-sch > > On Fri, Sep 26, 2014 at 7:07 PM, Chang Rebecca Swee Fun > wrote: > > Hi, > > > > This is a revised version for gpio-sch. > > > > Change log for V2: > > Patch 1: > > - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. > > - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ > > sch_gpio_register_clear(). > > > > Patch 3: > > - Changed all sch_gpio_enable()/sch_gpio_disable() to > sch_gpio_register_set()/ > > sch_gpio_register_clear(). > > > > Version 1: > > This patch series is about enabling legacy GPIO support for Quark X1000. > > The patches were developed on top of Mika Westerberg's commit on > > consolidating core and resume banks. Please refer to the link below > > for more information about his commit. > > https://lkml.org/lkml/2014/8/17/13 > > Sorry for the late review. I tried to apply the patch you mentioned above > before > your series, and even Mika's patch won't apply on Linus' > devel branch or today's -next. This make it difficult to make a good review. > Could you rebase and resend this series once all its dependencies have been > merge by Linus W. ? Hi, I've noticed that Mika had sent a V2series on his work. Referring to his submission in: https://lkml.org/lkml/2014/9/16/213, this patch was able to patch on today's kernel tree. The patches on my series are able to patch in too. Do I need to resend since there is no changes on my side? Rebecca
RE: [PATCHv2 0/3] Enable Quark X1000 support in gpio-sch
-Original Message- From: Alexandre Courbot [mailto:gnu...@gmail.com] Sent: 09 October, 2014 2:29 PM To: Chang, Rebecca Swee Fun Cc: Linus Walleij; Westerberg, Mika; GPIO Subsystem Mailing List; Linux Kernel Mailing List Subject: Re: [PATCHv2 0/3] Enable Quark X1000 support in gpio-sch On Fri, Sep 26, 2014 at 7:07 PM, Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com wrote: Hi, This is a revised version for gpio-sch. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: This patch series is about enabling legacy GPIO support for Quark X1000. The patches were developed on top of Mika Westerberg's commit on consolidating core and resume banks. Please refer to the link below for more information about his commit. https://lkml.org/lkml/2014/8/17/13 Sorry for the late review. I tried to apply the patch you mentioned above before your series, and even Mika's patch won't apply on Linus' devel branch or today's -next. This make it difficult to make a good review. Could you rebase and resend this series once all its dependencies have been merge by Linus W. ? Hi, I've noticed that Mika had sent a V2series on his work. Referring to his submission in: https://lkml.org/lkml/2014/9/16/213, this patch was able to patch on today's kernel tree. The patches on my series are able to patch in too. Do I need to resend since there is no changes on my side? Rebecca
[PATCHv2 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 690904a..64683a9 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -356,25 +356,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate "Intel SCH/TunnelCreek/Centerton GPIO" + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" depends on PCI && X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate "Intel ICH GPIO" depends on PCI && X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 6e89be9..952990f 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch->chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch->core_base = 0; + sch->resume_base = 2; + sch->chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 267 ---
1 file changed, 253 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 952990f..332ffaf 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include
#include
+#include
+#include
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ int irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -66,10 +79,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -78,16 +92,17 @@ static void sch_gpio_register_set(struct sch_gpio *sch,
unsigned gpio,
if (!(enable & BIT(bit)))
outb(enable | BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_register_clear(struct sch_gpio *sch,
unsigned gpio,
if (disable & BIT(bit))
outb(disable & ~BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_register_set(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
return 0;
}
@@ -149,20 +165,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_register_clear(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -177,6 +195,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch->irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = "sch_gpio",
.owner = THIS_MODULE,
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio
[PATCHv2 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..6e89be9 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(>lock);
- 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);
+ if (!(enable & BIT(bit)))
+ outb(enable | BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
spin_lock(>lock);
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_dirs = inb(sch->iobase + offset);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), sch->iobase + offset);
+ disable = inb(sch->iobase + offset);
+ if (disable & BIT(bit))
+ outb(disable & ~BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch->iobase + offset) & (1 << bit));
+ curr_dirs = !!(inb(sch->iobase + offset) & BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(>lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch->iobase + offset);
+ curr_dirs = inb(sch->iobase + offset);
if (val)
- outb(curr_vals | (1 << bit), sch->iobase + offset);
+ outb(curr_dirs | BIT(bit), sch->iobase + offset);
else
- outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
+ outb((curr_dirs & ~BIT(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(>lock);
+ sch_gpio_register_set(sch, gpio_num, GIO);
spin_unlock(>lock);
+ return 0;
}
-static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
- int val)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+{
+ 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_dirs;
- unsigned short offset, bit;
spin_lock(>lock);
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
+ spin_unlock(>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);
+static int sch_gpio_direction_out(struct gpio_chip *gc,
[PATCHv2 0/3] Enable Quark X1000 support in gpio-sch
Hi, This is a revised version for gpio-sch. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: This patch series is about enabling legacy GPIO support for Quark X1000. The patches were developed on top of Mika Westerberg's commit on consolidating core and resume banks. Please refer to the link below for more information about his commit. https://lkml.org/lkml/2014/8/17/13 The patches are generally enable GPIO support for Intel Quark X1000 SoC. In the first patch of the series, I've also done some consolidating work as there are similar algorithms that can be merged and generalized. The second patch is about adding Quark X1000 pci ids and gpio pins supported in the legacy gpio bridge. The last patch in the series is about enable IRQ handling for gpio-sch. Intel Quark X1000's legacy GPIO is an IRQ based GPIO. The IRQ resources will be provided by MFD's lpc_sch.c. The changes in MFD (lpc_sch.c) required in order to support gpio-sch has been merged into linux-next.git. The patches has been built and tested working on Intel Galileo board. Thank you. Regards Rebecca Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 362 --- 2 files changed, 318 insertions(+), 55 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
> -Original Message-
> From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com]
> Sent: 26 September, 2014 5:18 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
>
> On Fri, Sep 26, 2014 at 09:14:48AM +0000, Chang, Rebecca Swee Fun wrote:
> > > > @@ -203,6 +376,17 @@ static int sch_gpio_probe(struct
> > > > platform_device
> > > *pdev)
> > > > pdev->name))
> > > > return -EBUSY;
> > > >
> > > > + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > > > + sch->irq_support = !!irq;
> > > > + if (sch->irq_support) {
> > > > + sch->irq_num = irq->start;
> > > > + if (sch->irq_num < 0) {
> > > > + dev_warn(>dev,
> > > > +"failed to obtain irq number for
> > > > device\n");
> > > > + sch->irq_support = 0;
> > > > + }
> > > > + }
> > > > +
> > > > spin_lock_init(>lock);
> > > > sch->iobase = res->start;
> > > > sch->chip = sch_gpio_chip;
> > > > @@ -251,17 +435,72 @@ static int sch_gpio_probe(struct
> > > > platform_device
> > > *pdev)
> > > > return -ENODEV;
> > > > }
> > > >
> > > > + err = gpiochip_add(>chip);
> > > > + if (err < 0)
> > > > + goto err_sch_gpio;
> > > > +
> > > > + if (sch->irq_support) {
> > > > + sch->irq_base = irq_alloc_descs(-1, 0, sch->chip.ngpio,
> > > > + NUMA_NO_NODE);
> > > > + if (sch->irq_base < 0) {
> > > > + dev_err(>dev,
> > > > + "failed to add GPIO IRQ descs\n");
> > > > + sch->irq_base = -1;
> > > > + goto err_sch_intr_chip;
> > > > + }
> > >
> > > Was there some reason why you can't use gpiochip_irqchip_* helpers here?
> >
> > I will look into the helpers function and see what I can change here.
> >
> > >
> > > > +
> > > > + /* disable interrupts */
> > > > + sch_gpio_irq_disable_all(sch, sch->chip.ngpio);
> > > > +
> > > > + err = request_irq(sch->irq_num, sch_gpio_irq_handler,
> > > > + IRQF_SHARED, KBUILD_MODNAME, sch);
> > >
> > > This seems weird, typically irqchip drivers don't call request_irq()
> > > directly but instead irq_set_chained_handler() or similar. With
> > > gpiochip_irqchip_* stuff you don't need even that.
> > >
> > Regarding this, gpio-sch is actually using shared interrupts and the
> > IRQ resources are from ACPI SCI. As per my understanding, resources
> > from ACPI SCI might be shared for power management purposes. In this
> > case, irq_set_chained_handler() might not be able to use here. What do
> > you think?
>
> I think you are right. And then you can't use gpiochip_irqchip_* helpers
> either :-
> (
Then I shall submit the changes for the first patch in V2 series for further
review. Thanks.
Rebecca
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
> > @@ -203,6 +376,17 @@ static int sch_gpio_probe(struct platform_device
> *pdev)
> > pdev->name))
> > return -EBUSY;
> >
> > + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + sch->irq_support = !!irq;
> > + if (sch->irq_support) {
> > + sch->irq_num = irq->start;
> > + if (sch->irq_num < 0) {
> > + dev_warn(>dev,
> > +"failed to obtain irq number for device\n");
> > + sch->irq_support = 0;
> > + }
> > + }
> > +
> > spin_lock_init(>lock);
> > sch->iobase = res->start;
> > sch->chip = sch_gpio_chip;
> > @@ -251,17 +435,72 @@ static int sch_gpio_probe(struct platform_device
> *pdev)
> > return -ENODEV;
> > }
> >
> > + err = gpiochip_add(>chip);
> > + if (err < 0)
> > + goto err_sch_gpio;
> > +
> > + if (sch->irq_support) {
> > + sch->irq_base = irq_alloc_descs(-1, 0, sch->chip.ngpio,
> > + NUMA_NO_NODE);
> > + if (sch->irq_base < 0) {
> > + dev_err(>dev,
> > + "failed to add GPIO IRQ descs\n");
> > + sch->irq_base = -1;
> > + goto err_sch_intr_chip;
> > + }
>
> Was there some reason why you can't use gpiochip_irqchip_* helpers here?
I will look into the helpers function and see what I can change here.
>
> > +
> > + /* disable interrupts */
> > + sch_gpio_irq_disable_all(sch, sch->chip.ngpio);
> > +
> > + err = request_irq(sch->irq_num, sch_gpio_irq_handler,
> > + IRQF_SHARED, KBUILD_MODNAME, sch);
>
> This seems weird, typically irqchip drivers don't call request_irq() directly
> but
> instead irq_set_chained_handler() or similar. With
> gpiochip_irqchip_* stuff you don't need even that.
>
Regarding this, gpio-sch is actually using shared interrupts and the IRQ
resources are from ACPI SCI. As per my understanding, resources from ACPI SCI
might be shared for power management purposes. In this case,
irq_set_chained_handler() might not be able to use here. What do you think?
Rebecca
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
@@ -203,6 +376,17 @@ static int sch_gpio_probe(struct platform_device
*pdev)
pdev-name))
return -EBUSY;
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ sch-irq_support = !!irq;
+ if (sch-irq_support) {
+ sch-irq_num = irq-start;
+ if (sch-irq_num 0) {
+ dev_warn(pdev-dev,
+failed to obtain irq number for device\n);
+ sch-irq_support = 0;
+ }
+ }
+
spin_lock_init(sch-lock);
sch-iobase = res-start;
sch-chip = sch_gpio_chip;
@@ -251,17 +435,72 @@ static int sch_gpio_probe(struct platform_device
*pdev)
return -ENODEV;
}
+ err = gpiochip_add(sch-chip);
+ if (err 0)
+ goto err_sch_gpio;
+
+ if (sch-irq_support) {
+ sch-irq_base = irq_alloc_descs(-1, 0, sch-chip.ngpio,
+ NUMA_NO_NODE);
+ if (sch-irq_base 0) {
+ dev_err(pdev-dev,
+ failed to add GPIO IRQ descs\n);
+ sch-irq_base = -1;
+ goto err_sch_intr_chip;
+ }
Was there some reason why you can't use gpiochip_irqchip_* helpers here?
I will look into the helpers function and see what I can change here.
+
+ /* disable interrupts */
+ sch_gpio_irq_disable_all(sch, sch-chip.ngpio);
+
+ err = request_irq(sch-irq_num, sch_gpio_irq_handler,
+ IRQF_SHARED, KBUILD_MODNAME, sch);
This seems weird, typically irqchip drivers don't call request_irq() directly
but
instead irq_set_chained_handler() or similar. With
gpiochip_irqchip_* stuff you don't need even that.
Regarding this, gpio-sch is actually using shared interrupts and the IRQ
resources are from ACPI SCI. As per my understanding, resources from ACPI SCI
might be shared for power management purposes. In this case,
irq_set_chained_handler() might not be able to use here. What do you think?
Rebecca
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
-Original Message-
From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com]
Sent: 26 September, 2014 5:18 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
On Fri, Sep 26, 2014 at 09:14:48AM +, Chang, Rebecca Swee Fun wrote:
@@ -203,6 +376,17 @@ static int sch_gpio_probe(struct
platform_device
*pdev)
pdev-name))
return -EBUSY;
+ irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ sch-irq_support = !!irq;
+ if (sch-irq_support) {
+ sch-irq_num = irq-start;
+ if (sch-irq_num 0) {
+ dev_warn(pdev-dev,
+failed to obtain irq number for
device\n);
+ sch-irq_support = 0;
+ }
+ }
+
spin_lock_init(sch-lock);
sch-iobase = res-start;
sch-chip = sch_gpio_chip;
@@ -251,17 +435,72 @@ static int sch_gpio_probe(struct
platform_device
*pdev)
return -ENODEV;
}
+ err = gpiochip_add(sch-chip);
+ if (err 0)
+ goto err_sch_gpio;
+
+ if (sch-irq_support) {
+ sch-irq_base = irq_alloc_descs(-1, 0, sch-chip.ngpio,
+ NUMA_NO_NODE);
+ if (sch-irq_base 0) {
+ dev_err(pdev-dev,
+ failed to add GPIO IRQ descs\n);
+ sch-irq_base = -1;
+ goto err_sch_intr_chip;
+ }
Was there some reason why you can't use gpiochip_irqchip_* helpers here?
I will look into the helpers function and see what I can change here.
+
+ /* disable interrupts */
+ sch_gpio_irq_disable_all(sch, sch-chip.ngpio);
+
+ err = request_irq(sch-irq_num, sch_gpio_irq_handler,
+ IRQF_SHARED, KBUILD_MODNAME, sch);
This seems weird, typically irqchip drivers don't call request_irq()
directly but instead irq_set_chained_handler() or similar. With
gpiochip_irqchip_* stuff you don't need even that.
Regarding this, gpio-sch is actually using shared interrupts and the
IRQ resources are from ACPI SCI. As per my understanding, resources
from ACPI SCI might be shared for power management purposes. In this
case, irq_set_chained_handler() might not be able to use here. What do
you think?
I think you are right. And then you can't use gpiochip_irqchip_* helpers
either :-
(
Then I shall submit the changes for the first patch in V2 series for further
review. Thanks.
Rebecca
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 0/3] Enable Quark X1000 support in gpio-sch
Hi, This is a revised version for gpio-sch. Change log for V2: Patch 1: - Move sch_gpio_get() and sch_gpio_set() to avoid forward declaration. - Changed sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Patch 3: - Changed all sch_gpio_enable()/sch_gpio_disable() to sch_gpio_register_set()/ sch_gpio_register_clear(). Version 1: This patch series is about enabling legacy GPIO support for Quark X1000. The patches were developed on top of Mika Westerberg's commit on consolidating core and resume banks. Please refer to the link below for more information about his commit. https://lkml.org/lkml/2014/8/17/13 The patches are generally enable GPIO support for Intel Quark X1000 SoC. In the first patch of the series, I've also done some consolidating work as there are similar algorithms that can be merged and generalized. The second patch is about adding Quark X1000 pci ids and gpio pins supported in the legacy gpio bridge. The last patch in the series is about enable IRQ handling for gpio-sch. Intel Quark X1000's legacy GPIO is an IRQ based GPIO. The IRQ resources will be provided by MFD's lpc_sch.c. The changes in MFD (lpc_sch.c) required in order to support gpio-sch has been merged into linux-next.git. The patches has been built and tested working on Intel Galileo board. Thank you. Regards Rebecca Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 362 --- 2 files changed, 318 insertions(+), 55 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..6e89be9 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -63,94 +63,105 @@ 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 void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(sch-lock);
- 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);
+ if (!(enable BIT(bit)))
+ outb(enable | BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
+ unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
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);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ disable = inb(sch-iobase + offset);
+ if (disable BIT(bit))
+ outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch-iobase + offset) (1 bit));
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch-iobase + offset);
+ curr_dirs = inb(sch-iobase + offset);
if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
+ outb(curr_dirs | BIT(bit), sch-iobase + offset);
else
- outb((curr_vals ~(1 bit)), sch-iobase + offset);
+ outb((curr_dirs ~BIT(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_register_set(sch, gpio_num, GIO);
spin_unlock(sch-lock);
+ return 0;
}
-static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
- int val)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+{
+ 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_dirs;
- unsigned short offset, bit;
spin_lock(sch-lock);
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
+ spin_unlock(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);
+static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
+ int val)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ spin_lock(sch-lock
[PATCHv2 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 267 ---
1 file changed, 253 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 952990f..332ffaf 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include linux/pci_ids.h
#include linux/gpio.h
+#include linux/interrupt.h
+#include linux/irq.h
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ int irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -66,10 +79,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_register_set(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -78,16 +92,17 @@ static void sch_gpio_register_set(struct sch_gpio *sch,
unsigned gpio,
if (!(enable BIT(bit)))
outb(enable | BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static void sch_gpio_register_clear(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_register_clear(struct sch_gpio *sch,
unsigned gpio,
if (disable BIT(bit))
outb(disable ~BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_register_set(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
return 0;
}
@@ -149,20 +165,22 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_register_clear(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -177,6 +195,13 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch-irq_base + offset;
+}
+
static struct gpio_chip sch_gpio_chip = {
.label = sch_gpio,
.owner = THIS_MODULE,
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable
[PATCHv2 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 690904a..64683a9 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -356,25 +356,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 6e89be9..952990f 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch-core_base = 0; + sch-resume_base = 2; + sch-chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
> -Original Message- > From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com] > Sent: 24 September, 2014 5:51 PM > To: Chang, Rebecca Swee Fun > Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms > > On Wed, Sep 24, 2014 at 12:55:07AM +0000, Chang, Rebecca Swee Fun wrote: > > > > The register values are required when it comes to IRQ handling. By > > > > passing in the registers values, we can make full use of the > > > > algorithms without introducing extra/similar algorithms to compute > > > > other register offset values. > > > > For example, we have other offset values to handle such as:- > > > > GTPE0x0C > > > > GTNE0x10 > > > > GGPE0x14 > > > > GSMI0x18 > > > > GTS 0x1C > > > > CGNMIEN 0x40 > > > > RGNMIEN 0x44 > > > > > > Well, can we at least call it something else than sch_gpio_enable()? > > > Perhaps sch_gpio_set_value() or so? > > > > sch_gpio_set_value() sounds good. After think twice, I intend to merge > > sch_gpio_enable() and sch_gpio_disable() into one functions. Using > > variable "enable" as an indicator, I can control whether to enable or > > disable when calling the function. Here is my draft: > > Actually sch_gpio_set_value() is too close to sch_gpio_set() which sets the > GPIO > to 1 or 0. How about sch_gpio_register_set() or something along those lines? > > And I don't think it is good idea to add yet another functionality, like > enable > there. Please leave sch_gpio_enable()/sch_gpio_disable() as is. Alright, I will change the sch_gpio_enable()/sch_gpio_disable() into sch_gpio_register_set()/sch_gpio_register_clear(). Thanks. Rebecca -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
-Original Message- From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com] Sent: 24 September, 2014 5:51 PM To: Chang, Rebecca Swee Fun Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms On Wed, Sep 24, 2014 at 12:55:07AM +, Chang, Rebecca Swee Fun wrote: The register values are required when it comes to IRQ handling. By passing in the registers values, we can make full use of the algorithms without introducing extra/similar algorithms to compute other register offset values. For example, we have other offset values to handle such as:- GTPE0x0C GTNE0x10 GGPE0x14 GSMI0x18 GTS 0x1C CGNMIEN 0x40 RGNMIEN 0x44 Well, can we at least call it something else than sch_gpio_enable()? Perhaps sch_gpio_set_value() or so? sch_gpio_set_value() sounds good. After think twice, I intend to merge sch_gpio_enable() and sch_gpio_disable() into one functions. Using variable enable as an indicator, I can control whether to enable or disable when calling the function. Here is my draft: Actually sch_gpio_set_value() is too close to sch_gpio_set() which sets the GPIO to 1 or 0. How about sch_gpio_register_set() or something along those lines? And I don't think it is good idea to add yet another functionality, like enable there. Please leave sch_gpio_enable()/sch_gpio_disable() as is. Alright, I will change the sch_gpio_enable()/sch_gpio_disable() into sch_gpio_register_set()/sch_gpio_register_clear(). Thanks. Rebecca -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
> -Original Message-
> From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com]
> Sent: 22 September, 2014 5:25 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
>
> On Mon, Sep 22, 2014 at 05:43:36AM +0000, Chang, Rebecca Swee Fun wrote:
> > Replied inline. :)
> >
> > > -Original Message-
> > > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> > > Sent: 18 September, 2014 7:17 PM
> > > To: Chang, Rebecca Swee Fun
> > > Cc: Linus Walleij; linux-g...@vger.kernel.org;
> > > linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
> > >
> > > On Wed, Sep 17, 2014 at 04:49:03PM +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
> > > >
> > > > ---
> > > > drivers/gpio/gpio-sch.c | 95
> > > > ++---
> -
> > > -
> > > > 1 file changed, 53 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
> > > > index
> > > > 99720c8..2189c22 100644
> > > > --- a/drivers/gpio/gpio-sch.c
> > > > +++ b/drivers/gpio/gpio-sch.c
> > > > @@ -43,6 +43,8 @@ struct sch_gpio {
> > > >
> > > > #define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
> > > >
> > > > +static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num,
> > > > +int val);
> > > > +
> > >
> > > Is it possible to move the sch_gpio_set() function here instead of
> > > forward declaring it?
> >
> > Yes, it is possible. There is a reason I submitted the code in this
> > structure. By putting the sch_gpio_set() function below will makes the
> > diff patch looks neat and easy for review. If it doesn't make sense
> > to make the patch for easy reviewing, I can change by moving the
> > function above.
>
> I think that we are interested in the end result (e.g) the driver and if we
> can
> avoid forward declarations the better.
Alright. I can move it up to avoid the forward declaration.
>
> > >
> > > > static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
> > > > unsigned reg)
> > > > {
> > > > @@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio,
> > > > +unsigned reg)
> > >
> > > I don't see much value in doing this.
> > >
> > > To me sch_gpio_enable(sch, gpio) is more intuitive than
> > > sch_gpio_enable(sch, gpio, GEN). Why do I need to pass register to enable
> function in the first place?
> > > It should know better how to enable the GPIO, right?
> > >
> > > Same goes for others.
> >
> > The register values are required when it comes to IRQ handling. By
> > passing in the registers values, we can make full use of the
> > algorithms without introducing extra/similar algorithms to compute
> > other register offset values.
> > For example, we have other offset values to handle such as:-
> > GTPE0x0C
> > GTNE0x10
> > GGPE0x14
> > GSMI0x18
> > GTS 0x1C
> > CGNMIEN 0x40
> > RGNMIEN 0x44
>
> Well, can we at least call it something else than sch_gpio_enable()?
> Perhaps sch_gpio_set_value() or so?
sch_gpio_set_value() sounds good. After think twice, I intend to merge
sch_gpio_enable() and sch_gpio_disable() into one functions. Using variable
"enable" as an indicator, I can control whether to enable or disable when
calling the function. Here is my draft:
static void sch_gpio_set_value(struct sch_gpio *sch, unsigned gpio, unsigned
reg, unsigned int enable)
{
unsigned long flags;
unsigned short offset, bit;
u8 set;
spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
set = inb(sch->iobase + offset);
if (enable)
outb(set | BIT(bit), sch->iobase + offset);
else
outb(disable & ~BIT(bit), sch->iobase + offset);
spin_unlock_irqrestore(>lock, flags);
}
Do you think this make sense? I am in the progress of implementing and testing.
Rebecca
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
-Original Message-
From: 'Mika Westerberg' [mailto:mika.westerb...@linux.intel.com]
Sent: 22 September, 2014 5:25 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
On Mon, Sep 22, 2014 at 05:43:36AM +, Chang, Rebecca Swee Fun wrote:
Replied inline. :)
-Original Message-
From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
Sent: 18 September, 2014 7:17 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; linux-g...@vger.kernel.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
On Wed, Sep 17, 2014 at 04:49:03PM +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.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95
++---
-
-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index
99720c8..2189c22 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -43,6 +43,8 @@ struct sch_gpio {
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num,
+int val);
+
Is it possible to move the sch_gpio_set() function here instead of
forward declaring it?
Yes, it is possible. There is a reason I submitted the code in this
structure. By putting the sch_gpio_set() function below will makes the
diff patch looks neat and easy for review. If it doesn't make sense
to make the patch for easy reviewing, I can change by moving the
function above.
I think that we are interested in the end result (e.g) the driver and if we
can
avoid forward declarations the better.
Alright. I can move it up to avoid the forward declaration.
static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
@@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio,
+unsigned reg)
I don't see much value in doing this.
To me sch_gpio_enable(sch, gpio) is more intuitive than
sch_gpio_enable(sch, gpio, GEN). Why do I need to pass register to enable
function in the first place?
It should know better how to enable the GPIO, right?
Same goes for others.
The register values are required when it comes to IRQ handling. By
passing in the registers values, we can make full use of the
algorithms without introducing extra/similar algorithms to compute
other register offset values.
For example, we have other offset values to handle such as:-
GTPE0x0C
GTNE0x10
GGPE0x14
GSMI0x18
GTS 0x1C
CGNMIEN 0x40
RGNMIEN 0x44
Well, can we at least call it something else than sch_gpio_enable()?
Perhaps sch_gpio_set_value() or so?
sch_gpio_set_value() sounds good. After think twice, I intend to merge
sch_gpio_enable() and sch_gpio_disable() into one functions. Using variable
enable as an indicator, I can control whether to enable or disable when
calling the function. Here is my draft:
static void sch_gpio_set_value(struct sch_gpio *sch, unsigned gpio, unsigned
reg, unsigned int enable)
{
unsigned long flags;
unsigned short offset, bit;
u8 set;
spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
set = inb(sch-iobase + offset);
if (enable)
outb(set | BIT(bit), sch-iobase + offset);
else
outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock_irqrestore(sch-lock, flags);
}
Do you think this make sense? I am in the progress of implementing and testing.
Rebecca
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
Replied inline. :)
> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: 18 September, 2014 7:17 PM
> To: Chang, Rebecca Swee Fun
> Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
>
> On Wed, Sep 17, 2014 at 04:49:03PM +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
> >
> > ---
> > drivers/gpio/gpio-sch.c | 95
> > ++
> -
> > 1 file changed, 53 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index
> > 99720c8..2189c22 100644
> > --- a/drivers/gpio/gpio-sch.c
> > +++ b/drivers/gpio/gpio-sch.c
> > @@ -43,6 +43,8 @@ struct sch_gpio {
> >
> > #define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
> >
> > +static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int
> > +val);
> > +
>
> Is it possible to move the sch_gpio_set() function here instead of forward
> declaring it?
Yes, it is possible. There is a reason I submitted the code in this structure.
By putting the sch_gpio_set() function below will makes the diff patch looks
neat and easy for review.
If it doesn't make sense to make the patch for easy reviewing, I can change by
moving the function above.
>
> > static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
> > unsigned reg)
> > {
> > @@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio,
> > +unsigned reg)
>
> I don't see much value in doing this.
>
> To me sch_gpio_enable(sch, gpio) is more intuitive than sch_gpio_enable(sch,
> gpio, GEN). Why do I need to pass register to enable function in the first
> place?
> It should know better how to enable the GPIO, right?
>
> Same goes for others.
The register values are required when it comes to IRQ handling. By passing in
the registers values, we can make full use of the algorithms without
introducing extra/similar algorithms to compute other register offset values.
For example, we have other offset values to handle such as:-
GTPE0x0C
GTNE0x10
GGPE0x14
GSMI0x18
GTS 0x1C
CGNMIEN 0x40
RGNMIEN 0x44
Regards
Rebecca
>
> > {
> > unsigned short offset, bit;
> > u8 enable;
> >
> > spin_lock(>lock);
> >
> > - 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);
> > + if (!(enable & BIT(bit)))
> > + outb(enable | BIT(bit), sch->iobase + offset);
> >
> > spin_unlock(>lock);
> > }
> >
> > -static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned
> > gpio_num)
> > +static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio,
> > +unsigned reg)
> > {
> > - struct sch_gpio *sch = to_sch_gpio(gc);
> > - u8 curr_dirs;
> > unsigned short offset, bit;
> > + u8 disable;
> >
> > spin_lock(>lock);
> >
> > - offset = sch_gpio_offset(sch, gpio_num, GIO);
> > - bit = sch_gpio_bit(sch, gpio_num);
> > -
> > - curr_dirs = inb(sch->iobase + offset);
> > + offset = sch_gpio_offset(sch, gpio, reg);
> > + bit = sch_gpio_bit(sch, gpio);
> >
> > - if (!(curr_dirs & (1 << bit)))
> > - outb(curr_dirs | (1 << bit), sch->iobase + offset);
> > + disable = inb(sch->iobase + offset);
> > + if (disable & BIT(bit))
> > + outb(disable & ~BIT(bit), sch->iobase + offset);
> >
> > spin_unlock(>lock);
> > - return 0;
> > }
> >
> > -static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
> > +static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio,
> > +unsigned reg)
> > {
> > struct sch_gpio *sch = to_sch_gpio(gc);
> > - int res;
> > unsigned short offset, bit;
> > + u8 curr_dirs;
> >
> >
RE: [PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
> -Original Message- > From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] > Sent: 18 September, 2014 7:23 PM > To: Chang, Rebecca Swee Fun > Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC > > On Wed, Sep 17, 2014 at 04:49:04PM +0800, Chang Rebecca Swee Fun wrote: > > Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split > > between the legacy I/O bridge and the GPIO controller. > > > > GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. > > Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 > > from the suspend power well. > > > > This piece of work is derived from Dan O'Donovan's initial work for > > Quark > > X1000 enabling. > > > > Signed-off-by: Chang Rebecca Swee Fun > > > > One question, see below. But in general looks good, so > > Reviewed-by: Mika Westerberg > > > --- > > drivers/gpio/Kconfig| 11 +-- > > drivers/gpio/gpio-sch.c |6 ++ > > 2 files changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index > > 690904a..64683a9 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -356,25 +356,32 @@ config GPIO_VR41XX > > Say yes here to support the NEC VR4100 series General-purpose I/O > > Uint > > > > config GPIO_SCH > > - tristate "Intel SCH/TunnelCreek/Centerton GPIO" > > + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" > > depends on PCI && X86 > > select MFD_CORE > > select LPC_SCH > > help > > Say yes here to support GPIO interface on Intel Poulsbo SCH, > > - Intel Tunnel Creek processor or Intel Centerton processor. > > + Intel Tunnel Creek processor, Intel Centerton processor or > > + Intel Quark X1000 SoC. > > + > > The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are > > powered by the core power rail and are turned off during sleep > > modes (S3 and higher). The remaining four GPIOs are powered by > > the Intel SCH suspend power supply. These GPIOs remain > > active during S3. The suspend powered GPIOs can be used to wake the > > system from the Suspend-to-RAM state. > > + > > The Intel Tunnel Creek processor has 5 GPIOs powered by the > > core power rail and 9 from suspend power supply. > > + > > The Intel Centerton processor has a total of 30 GPIO pins. > > Twenty-one are powered by the core power rail and 9 from the > > suspend power supply. > > > > + The Intel Quark X1000 SoC has 2 GPIOs powered by the core > > + power well and 6 from the suspend power well. > > + > > config GPIO_ICH > > tristate "Intel ICH GPIO" > > depends on PCI && X86 > > diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index > > 2189c22..38d6e74 100644 > > --- a/drivers/gpio/gpio-sch.c > > +++ b/drivers/gpio/gpio-sch.c > > @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device > *pdev) > > sch->chip.ngpio = 30; > > break; > > > > + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: > > Is this PCI ID provided by some other patch series? Yes, this PCI ID has been submitted to upstream through MFD subsystem tree. Rebecca -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
-Original Message- From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com] Sent: 18 September, 2014 7:23 PM To: Chang, Rebecca Swee Fun Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC On Wed, Sep 17, 2014 at 04:49:04PM +0800, Chang Rebecca Swee Fun wrote: Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com One question, see below. But in general looks good, so Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 690904a..64683a9 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -356,25 +356,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 2189c22..38d6e74 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: Is this PCI ID provided by some other patch series? Yes, this PCI ID has been submitted to upstream through MFD subsystem tree. Rebecca -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
Replied inline. :)
-Original Message-
From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
Sent: 18 September, 2014 7:17 PM
To: Chang, Rebecca Swee Fun
Cc: Linus Walleij; linux-g...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] gpio: sch: Consolidate similar algorithms
On Wed, Sep 17, 2014 at 04:49:03PM +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.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95
++
-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index
99720c8..2189c22 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -43,6 +43,8 @@ struct sch_gpio {
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int
+val);
+
Is it possible to move the sch_gpio_set() function here instead of forward
declaring it?
Yes, it is possible. There is a reason I submitted the code in this structure.
By putting the sch_gpio_set() function below will makes the diff patch looks
neat and easy for review.
If it doesn't make sense to make the patch for easy reviewing, I can change by
moving the function above.
static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
@@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio,
+unsigned reg)
I don't see much value in doing this.
To me sch_gpio_enable(sch, gpio) is more intuitive than sch_gpio_enable(sch,
gpio, GEN). Why do I need to pass register to enable function in the first
place?
It should know better how to enable the GPIO, right?
Same goes for others.
The register values are required when it comes to IRQ handling. By passing in
the registers values, we can make full use of the algorithms without
introducing extra/similar algorithms to compute other register offset values.
For example, we have other offset values to handle such as:-
GTPE0x0C
GTNE0x10
GGPE0x14
GSMI0x18
GTS 0x1C
CGNMIEN 0x40
RGNMIEN 0x44
Regards
Rebecca
{
unsigned short offset, bit;
u8 enable;
spin_lock(sch-lock);
- 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);
+ if (!(enable BIT(bit)))
+ outb(enable | BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned
gpio_num)
+static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio,
+unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
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);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ disable = inb(sch-iobase + offset);
+ if (disable BIT(bit))
+ outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio,
+unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch-iobase + offset) (1 bit));
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int
val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit
[PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 267 ---
1 file changed, 253 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 38d6e74..c6c64a5 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include
#include
+#include
+#include
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ int irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -67,10 +80,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -79,15 +93,16 @@ static void sch_gpio_enable(struct sch_gpio *sch, unsigned
gpio, unsigned reg)
if (!(enable & BIT(bit)))
outb(enable | BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_disable(struct sch_gpio *sch, unsigned
gpio, unsigned reg)
if (disable & BIT(bit))
outb(disable & ~BIT(bit), sch->iobase + offset);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_enable(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
return 0;
}
@@ -145,10 +161,11 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_disable(sch, gpio_num, GIO);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -171,10 +188,18 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(>lock);
+ spin_lock_irqsave(>lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(>lock);
+ spin_unlock_irqrestore(>lock, flags);
+}
+
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch->irq_base + offset;
}
static struct gpio_chip sch_gpio_chip = {
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d->irq - sch->irq_base;
+ sch_gpio_enable(sch, gpio_num, GGPE);
+}
+
+static void sch_gpio_irq_disable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, st
[PATCH 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..2189c22 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -43,6 +43,8 @@ struct sch_gpio {
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val);
+
static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
@@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(>lock);
- 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);
+ if (!(enable & BIT(bit)))
+ outb(enable | BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
spin_lock(>lock);
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_dirs = inb(sch->iobase + offset);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs & (1 << bit)))
- outb(curr_dirs | (1 << bit), sch->iobase + offset);
+ disable = inb(sch->iobase + offset);
+ if (disable & BIT(bit))
+ outb(disable & ~BIT(bit), sch->iobase + offset);
spin_unlock(>lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch->iobase + offset) & (1 << bit));
+ curr_dirs = !!(inb(sch->iobase + offset) & BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(>lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch->iobase + offset);
+ curr_dirs = inb(sch->iobase + offset);
if (val)
- outb(curr_vals | (1 << bit), sch->iobase + offset);
+ outb(curr_dirs | BIT(bit), sch->iobase + offset);
else
- outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
+ outb((curr_dirs & ~BIT(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(>lock);
+ sch_gpio_enable(sch, gpio_num, GIO);
spin_unlock(>lock);
+ return 0;
}
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(>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_disable(sch, gpio_num, GIO);
spin_unlock(>lock);
/*
@@ -166,6 +163,20 @@ static
[PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 690904a..64683a9 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -356,25 +356,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate "Intel SCH/TunnelCreek/Centerton GPIO" + tristate "Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO" depends on PCI && X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate "Intel ICH GPIO" depends on PCI && X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 2189c22..38d6e74 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch->chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch->core_base = 0; + sch->resume_base = 2; + sch->chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] Enable Quark X1000 support in gpio-sch
Hi, This patch series is about enabling legacy GPIO support for Quark X1000. The patches were developed on top of Mika Westerberg's commit on consolidating core and resume banks. Please refer to the link below for more information about his commit. https://lkml.org/lkml/2014/8/17/13 The patches are generally enable GPIO support for Intel Quark X1000 SoC. In the first patch of the series, I've also done some consolidating work as there are similar algorithms that can be merged and generalized. The second patch is about adding Quark X1000 pci ids and gpio pins supported in the legacy gpio bridge. The last patch in the series is about enable IRQ handling for gpio-sch. Intel Quark X1000's legacy GPIO is an IRQ based GPIO. The IRQ resources will be provided by MFD's lpc_sch.c. The changes in MFD (lpc_sch.c) required in order to support gpio-sch has been merged into linux-next.git. The patches has been built and tested working on Intel Galileo board. Thank you. Regards Rebecca Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 362 --- 2 files changed, 318 insertions(+), 55 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] Enable Quark X1000 support in gpio-sch
Hi, This patch series is about enabling legacy GPIO support for Quark X1000. The patches were developed on top of Mika Westerberg's commit on consolidating core and resume banks. Please refer to the link below for more information about his commit. https://lkml.org/lkml/2014/8/17/13 The patches are generally enable GPIO support for Intel Quark X1000 SoC. In the first patch of the series, I've also done some consolidating work as there are similar algorithms that can be merged and generalized. The second patch is about adding Quark X1000 pci ids and gpio pins supported in the legacy gpio bridge. The last patch in the series is about enable IRQ handling for gpio-sch. Intel Quark X1000's legacy GPIO is an IRQ based GPIO. The IRQ resources will be provided by MFD's lpc_sch.c. The changes in MFD (lpc_sch.c) required in order to support gpio-sch has been merged into linux-next.git. The patches has been built and tested working on Intel Galileo board. Thank you. Regards Rebecca Chang Rebecca Swee Fun (3): gpio: sch: Consolidate similar algorithms gpio: sch: Add support for Intel Quark X1000 SoC gpio: sch: Enable IRQ support for Quark X1000 drivers/gpio/Kconfig| 11 +- drivers/gpio/gpio-sch.c | 362 --- 2 files changed, 318 insertions(+), 55 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] gpio: sch: Add support for Intel Quark X1000 SoC
Intel Quark X1000 provides a total of 16 GPIOs. The GPIOs are split between the legacy I/O bridge and the GPIO controller. GPIO-SCH is the GPIO pins on legacy bridge for Intel Quark SoC. Intel Quark X1000 has 2 GPIOs powered by the core power well and 6 from the suspend power well. This piece of work is derived from Dan O'Donovan's initial work for Quark X1000 enabling. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com --- drivers/gpio/Kconfig| 11 +-- drivers/gpio/gpio-sch.c |6 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 690904a..64683a9 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -356,25 +356,32 @@ config GPIO_VR41XX Say yes here to support the NEC VR4100 series General-purpose I/O Uint config GPIO_SCH - tristate Intel SCH/TunnelCreek/Centerton GPIO + tristate Intel SCH/TunnelCreek/Centerton/Quark X1000 GPIO depends on PCI X86 select MFD_CORE select LPC_SCH help Say yes here to support GPIO interface on Intel Poulsbo SCH, - Intel Tunnel Creek processor or Intel Centerton processor. + Intel Tunnel Creek processor, Intel Centerton processor or + Intel Quark X1000 SoC. + The Intel SCH contains a total of 14 GPIO pins. Ten GPIOs are powered by the core power rail and are turned off during sleep modes (S3 and higher). The remaining four GPIOs are powered by the Intel SCH suspend power supply. These GPIOs remain active during S3. The suspend powered GPIOs can be used to wake the system from the Suspend-to-RAM state. + The Intel Tunnel Creek processor has 5 GPIOs powered by the core power rail and 9 from suspend power supply. + The Intel Centerton processor has a total of 30 GPIO pins. Twenty-one are powered by the core power rail and 9 from the suspend power supply. + The Intel Quark X1000 SoC has 2 GPIOs powered by the core + power well and 6 from the suspend power well. + config GPIO_ICH tristate Intel ICH GPIO depends on PCI X86 diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c index 2189c22..38d6e74 100644 --- a/drivers/gpio/gpio-sch.c +++ b/drivers/gpio/gpio-sch.c @@ -241,6 +241,12 @@ static int sch_gpio_probe(struct platform_device *pdev) sch-chip.ngpio = 30; break; + case PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB: + sch-core_base = 0; + sch-resume_base = 2; + sch-chip.ngpio = 8; + break; + default: return -ENODEV; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] gpio: sch: Consolidate similar algorithms
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 95 ++-
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 99720c8..2189c22 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -43,6 +43,8 @@ struct sch_gpio {
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val);
+
static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
unsigned reg)
{
@@ -63,94 +65,89 @@ 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 void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
unsigned short offset, bit;
u8 enable;
spin_lock(sch-lock);
- 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);
+ if (!(enable BIT(bit)))
+ outb(enable | BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
}
-static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
- struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
unsigned short offset, bit;
+ u8 disable;
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);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- if (!(curr_dirs (1 bit)))
- outb(curr_dirs | (1 bit), sch-iobase + offset);
+ disable = inb(sch-iobase + offset);
+ if (disable BIT(bit))
+ outb(disable ~BIT(bit), sch-iobase + offset);
spin_unlock(sch-lock);
- return 0;
}
-static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- int res;
unsigned short offset, bit;
+ u8 curr_dirs;
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- res = !!(inb(sch-iobase + offset) (1 bit));
+ curr_dirs = !!(inb(sch-iobase + offset) BIT(bit));
- return res;
+ return curr_dirs;
}
-static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+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_vals;
unsigned short offset, bit;
+ u8 curr_dirs;
- spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
+ offset = sch_gpio_offset(sch, gpio, reg);
+ bit = sch_gpio_bit(sch, gpio);
- curr_vals = inb(sch-iobase + offset);
+ curr_dirs = inb(sch-iobase + offset);
if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
+ outb(curr_dirs | BIT(bit), sch-iobase + offset);
else
- outb((curr_vals ~(1 bit)), sch-iobase + offset);
+ outb((curr_dirs ~BIT(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_enable(sch, gpio_num, GIO);
spin_unlock(sch-lock);
+ return 0;
}
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_disable(sch, gpio_num, GIO);
spin_unlock(sch-lock);
/*
@@ -166,6 +163,20 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
return 0;
}
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
+{
+ return
[PATCH 3/3] gpio: sch: Enable IRQ support for Quark X1000
Intel Quark X1000 GPIO controller supports interrupt handling for
both core power well and resume power well. This patch is to enable
the IRQ support and provide IRQ handling for Intel Quark X1000
GPIO-SCH device driver.
This piece of work is derived from Dan O'Donovan's initial work for
Quark X1000 enabling.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
---
drivers/gpio/gpio-sch.c | 267 ---
1 file changed, 253 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 38d6e74..c6c64a5 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -28,17 +28,30 @@
#include linux/pci_ids.h
#include linux/gpio.h
+#include linux/interrupt.h
+#include linux/irq.h
#define GEN0x00
#define GIO0x04
#define GLV0x08
+#define GTPE 0x0C
+#define GTNE 0x10
+#define GGPE 0x14
+#define GSMI 0x18
+#define GTS0x1C
+#define CGNMIEN0x40
+#define RGNMIEN0x44
struct sch_gpio {
struct gpio_chip chip;
+ struct irq_data data;
spinlock_t lock;
unsigned short iobase;
unsigned short core_base;
unsigned short resume_base;
+ int irq_base;
+ int irq_num;
+ int irq_support;
};
#define to_sch_gpio(c) container_of(c, struct sch_gpio, chip)
@@ -67,10 +80,11 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned
gpio)
static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 enable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -79,15 +93,16 @@ static void sch_gpio_enable(struct sch_gpio *sch, unsigned
gpio, unsigned reg)
if (!(enable BIT(bit)))
outb(enable | BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static void sch_gpio_disable(struct sch_gpio *sch, unsigned gpio, unsigned reg)
{
+ unsigned long flags;
unsigned short offset, bit;
u8 disable;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
offset = sch_gpio_offset(sch, gpio, reg);
bit = sch_gpio_bit(sch, gpio);
@@ -96,7 +111,7 @@ static void sch_gpio_disable(struct sch_gpio *sch, unsigned
gpio, unsigned reg)
if (disable BIT(bit))
outb(disable ~BIT(bit), sch-iobase + offset);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
}
static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
@@ -134,10 +149,11 @@ static void sch_gpio_reg_set(struct gpio_chip *gc,
unsigned gpio, unsigned reg,
static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_enable(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
return 0;
}
@@ -145,10 +161,11 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_disable(sch, gpio_num, GIO);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
/*
* according to the datasheet, writing to the level register has no
@@ -171,10 +188,18 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned
gpio_num)
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
+ unsigned long flags;
- spin_lock(sch-lock);
+ spin_lock_irqsave(sch-lock, flags);
sch_gpio_reg_set(gc, gpio_num, GLV, val);
- spin_unlock(sch-lock);
+ spin_unlock_irqrestore(sch-lock, flags);
+}
+
+static int sch_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+
+ return sch-irq_base + offset;
}
static struct gpio_chip sch_gpio_chip = {
@@ -184,12 +209,160 @@ static struct gpio_chip sch_gpio_chip = {
.get= sch_gpio_get,
.direction_output = sch_gpio_direction_out,
.set= sch_gpio_set,
+ .to_irq = sch_gpio_to_irq,
+};
+
+static void sch_gpio_irq_enable(struct irq_data *d)
+{
+ struct sch_gpio *sch = container_of(d, struct sch_gpio, data);
+ u32 gpio_num;
+
+ gpio_num = d-irq - sch-irq_base;
+ sch_gpio_enable(sch, gpio_num, GGPE);
+}
+
+static void sch_gpio_irq_disable(struct irq_data *d
RE: [PATCH v1 2/5] mfd: lpc_sch: better code manageability with chipset info struct
> -Original Message- > From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com] > Sent: 01 September, 2014 6:25 PM > To: Lee Jones > Cc: Bjorn Helgaas; linux-kernel@vger.kernel.org; Samuel Ortiz; Chang, Rebecca > Swee Fun > Subject: Re: [PATCH v1 2/5] mfd: lpc_sch: better code manageability with > chipset info struct > > On Mon, 2014-09-01 at 10:16 +0100, Lee Jones wrote: > > On Fri, 22 Aug 2014, Andy Shevchenko wrote: > > > > > Introduce additional struct to hold chipset info. This chipset info > > > will be used to store features that are supported by specific > > > processor or chipset. LPC_SCH supports SMBUS, GPIO and WDT features. > > > As this code base might expand further to support more processors, > > > this implementation will help to keep code base clean and manageable. > > > > > > Signed-off-by: Chang Rebecca Swee Fun > > > > > > Tested-by: Chang Rebecca Swee Fun > > > Signed-off-by: Andy Shevchenko > > [] > > > > The first patch would look a great deal cleaner if it had these > > changes in too. Unless you have a really good reason not to, please > > consider squashing them. > > The only reason behind is that this patch (in other form) was written by > Rebecca in the first place. I recommended to clean up before, and actually did > that clean up and amended Rebecca's patch. > > So, if Rebecca has now objections I could squash it. I have no objections. Thanks. > > > -- > Andy Shevchenko Intel Finland Oy
RE: [PATCH v1 3/5] pci_ids: add support for Intel Quark ILB
> -Original Message- > From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com] > Sent: 01 September, 2014 5:14 PM > To: Bjorn Helgaas > Cc: Lee Jones; linux-kernel@vger.kernel.org; Samuel Ortiz; Chang, Rebecca > Swee Fun; Ahmad, Josef > Subject: Re: [PATCH v1 3/5] pci_ids: add support for Intel Quark ILB > > On Fri, 2014-08-29 at 08:27 -0600, Bjorn Helgaas wrote: > > On Fri, Aug 22, 2014 at 4:58 AM, Andy Shevchenko > > wrote: > > > From: Josef Ahmad > > > > > > This patch adds the PCI id for Intel Quark ILB. > > > It will be used for GPIO and Multifunction device driver. > > > > > > Signed-off-by: Josef Ahmad > > > Signed-off-by: Andy Shevchenko > > > > Assuming that this will actually be used in more than one place: > > Yes, look for the other IDs in the same driver (lpc_sch). > > The question is will we have the update for sch_gpio in time to be included to > v3.18? I think Rebecca can answer to this one. Yes, we are working on updating sch_gpio but I can't give you a confirm answer whether it will be included in v3.18. Sch_gpio has been scheduled as my 2nd priority work currently. However, I will try to make it in time if possible. Rebecca > > > > > Acked-by: Bjorn Helgaas > > > > > --- > > > include/linux/pci_ids.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index > > > 6ed0bb7..4e82195 100644 > > > --- a/include/linux/pci_ids.h > > > +++ b/include/linux/pci_ids.h > > > @@ -2557,6 +2557,7 @@ > > > #define PCI_DEVICE_ID_INTEL_MFD_EMMC0 0x0823 #define > > > PCI_DEVICE_ID_INTEL_MFD_EMMC1 0x0824 > > > #define PCI_DEVICE_ID_INTEL_MRST_SD2 0x084F > > > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB0x095E > > > #define PCI_DEVICE_ID_INTEL_I960 0x0960 > > > #define PCI_DEVICE_ID_INTEL_I960RM 0x0962 > > > #define PCI_DEVICE_ID_INTEL_CENTERTON_ILB 0x0c60 > > > -- > > > 2.1.0 > > > > > > -- > Andy Shevchenko Intel Finland Oy
RE: [PATCH v1 3/5] pci_ids: add support for Intel Quark ILB
-Original Message- From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com] Sent: 01 September, 2014 5:14 PM To: Bjorn Helgaas Cc: Lee Jones; linux-kernel@vger.kernel.org; Samuel Ortiz; Chang, Rebecca Swee Fun; Ahmad, Josef Subject: Re: [PATCH v1 3/5] pci_ids: add support for Intel Quark ILB On Fri, 2014-08-29 at 08:27 -0600, Bjorn Helgaas wrote: On Fri, Aug 22, 2014 at 4:58 AM, Andy Shevchenko andriy.shevche...@linux.intel.com wrote: From: Josef Ahmad josef.ah...@intel.com This patch adds the PCI id for Intel Quark ILB. It will be used for GPIO and Multifunction device driver. Signed-off-by: Josef Ahmad josef.ah...@intel.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com Assuming that this will actually be used in more than one place: Yes, look for the other IDs in the same driver (lpc_sch). The question is will we have the update for sch_gpio in time to be included to v3.18? I think Rebecca can answer to this one. Yes, we are working on updating sch_gpio but I can't give you a confirm answer whether it will be included in v3.18. Sch_gpio has been scheduled as my 2nd priority work currently. However, I will try to make it in time if possible. Rebecca Acked-by: Bjorn Helgaas bhelg...@google.com --- include/linux/pci_ids.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 6ed0bb7..4e82195 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2557,6 +2557,7 @@ #define PCI_DEVICE_ID_INTEL_MFD_EMMC0 0x0823 #define PCI_DEVICE_ID_INTEL_MFD_EMMC1 0x0824 #define PCI_DEVICE_ID_INTEL_MRST_SD2 0x084F +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB0x095E #define PCI_DEVICE_ID_INTEL_I960 0x0960 #define PCI_DEVICE_ID_INTEL_I960RM 0x0962 #define PCI_DEVICE_ID_INTEL_CENTERTON_ILB 0x0c60 -- 2.1.0 -- Andy Shevchenko andriy.shevche...@intel.com Intel Finland Oy
RE: [PATCH v1 2/5] mfd: lpc_sch: better code manageability with chipset info struct
-Original Message- From: Andy Shevchenko [mailto:andriy.shevche...@linux.intel.com] Sent: 01 September, 2014 6:25 PM To: Lee Jones Cc: Bjorn Helgaas; linux-kernel@vger.kernel.org; Samuel Ortiz; Chang, Rebecca Swee Fun Subject: Re: [PATCH v1 2/5] mfd: lpc_sch: better code manageability with chipset info struct On Mon, 2014-09-01 at 10:16 +0100, Lee Jones wrote: On Fri, 22 Aug 2014, Andy Shevchenko wrote: Introduce additional struct to hold chipset info. This chipset info will be used to store features that are supported by specific processor or chipset. LPC_SCH supports SMBUS, GPIO and WDT features. As this code base might expand further to support more processors, this implementation will help to keep code base clean and manageable. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com Tested-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com [] The first patch would look a great deal cleaner if it had these changes in too. Unless you have a really good reason not to, please consider squashing them. The only reason behind is that this patch (in other form) was written by Rebecca in the first place. I recommended to clean up before, and actually did that clean up and amended Rebecca's patch. So, if Rebecca has now objections I could squash it. I have no objections. Thanks. -- Andy Shevchenko andriy.shevche...@intel.com Intel Finland Oy
