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&m=141392647225885&w=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
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
Reviewed-by: Alexandre Courbot <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
Reviewed-by: Alexandre Courbot <[email protected]>
---
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
On Mon, Dec 8, 2014 at 6:38 PM, Chang Rebecca Swee Fun
<[email protected]> wrote:
> 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&m=141392647225885&w=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.
I think these patches are good to go, if Linus is ok with it. Nice job
making this driver simpler and more legible. Looking forward to seeing
the IRQ implementation!