RE: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend & Resume PM enabling
>
> > +/* Store GPIO context across system-wide suspend/resume transitions
> > +*/ static struct dwapb_context {
> > + u32 data[DWAPB_MAX_PORTS];
> > + u32 dir[DWAPB_MAX_PORTS];
> > + u32 ext[DWAPB_MAX_PORTS];
> > + u32 int_en;
> > + u32 int_mask;
> > + u32 int_type;
> > + u32 int_pol;
> > + u32 int_deb;
> > +} dwapb_context;
>
> NAK, this should STILL be part of the device state container. Not a local
> variable.
>
> I've said this before. Please fix this, thanks.
>
Linus, please review Version 5 that I sent on Sep.17th. I have fixed it in the
v5.
RE: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend Resume PM enabling
+/* Store GPIO context across system-wide suspend/resume transitions
+*/ static struct dwapb_context {
+ u32 data[DWAPB_MAX_PORTS];
+ u32 dir[DWAPB_MAX_PORTS];
+ u32 ext[DWAPB_MAX_PORTS];
+ u32 int_en;
+ u32 int_mask;
+ u32 int_type;
+ u32 int_pol;
+ u32 int_deb;
+} dwapb_context;
NAK, this should STILL be part of the device state container. Not a local
variable.
I've said this before. Please fix this, thanks.
Linus, please review Version 5 that I sent on Sep.17th. I have fixed it in the
v5.
Re: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend & Resume PM enabling
On Fri, Sep 5, 2014 at 7:53 AM, Weike Chen wrote:
> This patch enables suspend and resume mode for the power management, and
> it is based on Josef Ahmad's previous work.
>
> Reviewed-by: Hock Leong Kweh
> Reviewed-by: Shevchenko, Andriy
> Signed-off-by: Weike Chen
(...)
> +/* Store GPIO context across system-wide suspend/resume transitions */
> +static struct dwapb_context {
> + u32 data[DWAPB_MAX_PORTS];
> + u32 dir[DWAPB_MAX_PORTS];
> + u32 ext[DWAPB_MAX_PORTS];
> + u32 int_en;
> + u32 int_mask;
> + u32 int_type;
> + u32 int_pol;
> + u32 int_deb;
> +} dwapb_context;
NAK, this should STILL be part of the device state container. Not
a local variable.
I've said this before. Please fix this, thanks.
Yours,
Linus Walleij
--
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 v2] GPIO: gpio-dwapb: Suspend Resume PM enabling
On Fri, Sep 5, 2014 at 7:53 AM, Weike Chen alvin.c...@intel.com wrote:
This patch enables suspend and resume mode for the power management, and
it is based on Josef Ahmad's previous work.
Reviewed-by: Hock Leong Kweh hock.leong.k...@intel.com
Reviewed-by: Shevchenko, Andriy andriy.shevche...@intel.com
Signed-off-by: Weike Chen alvin.c...@intel.com
(...)
+/* Store GPIO context across system-wide suspend/resume transitions */
+static struct dwapb_context {
+ u32 data[DWAPB_MAX_PORTS];
+ u32 dir[DWAPB_MAX_PORTS];
+ u32 ext[DWAPB_MAX_PORTS];
+ u32 int_en;
+ u32 int_mask;
+ u32 int_type;
+ u32 int_pol;
+ u32 int_deb;
+} dwapb_context;
NAK, this should STILL be part of the device state container. Not
a local variable.
I've said this before. Please fix this, thanks.
Yours,
Linus Walleij
--
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 v2] GPIO: gpio-dwapb: Suspend & Resume PM enabling
> > On Fri, 2014-09-05 at 07:53 -0700, Weike Chen wrote: > > This patch enables suspend and resume mode for the power management, > > and it is based on Josef Ahmad's previous work. > > > > Reviewed-by: Hock Leong Kweh > > Reviewed-by: Shevchenko, Andriy > > I have to recall my reviwed-by tag since patch is quite changed and as I > understood Linus is continuing to be changed. OK.
Re: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend & Resume PM enabling
On Fri, 2014-09-05 at 07:53 -0700, Weike Chen wrote:
> This patch enables suspend and resume mode for the power management, and
> it is based on Josef Ahmad's previous work.
>
> Reviewed-by: Hock Leong Kweh
> Reviewed-by: Shevchenko, Andriy
I have to recall my reviwed-by tag since patch is quite changed and as I
understood Linus is continuing to be changed.
> Signed-off-by: Weike Chen
> ---
> drivers/gpio/gpio-dwapb.c | 102
> +
> 1 file changed, 102 insertions(+)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 6db7501..a103def 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -54,6 +54,7 @@ struct dwapb_gpio_port {
> struct bgpio_chip bgc;
> boolis_registered;
> struct dwapb_gpio *gpio;
> + unsigned intidx;
> };
>
> struct dwapb_gpio {
> @@ -376,6 +377,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
>
> port = >ports[offs];
> port->gpio = gpio;
> + port->idx = pp->idx;
>
> dat = gpio->regs + GPIO_EXT_PORTA + (pp->idx * GPIO_EXT_PORT_SIZE);
> set = gpio->regs + GPIO_SWPORTA_DR + (pp->idx * GPIO_SWPORT_DR_SIZE);
> @@ -594,10 +596,110 @@ static const struct of_device_id dwapb_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, dwapb_of_match);
>
> +#ifdef CONFIG_PM_SLEEP
> +/* Store GPIO context across system-wide suspend/resume transitions */
> +static struct dwapb_context {
> + u32 data[DWAPB_MAX_PORTS];
> + u32 dir[DWAPB_MAX_PORTS];
> + u32 ext[DWAPB_MAX_PORTS];
> + u32 int_en;
> + u32 int_mask;
> + u32 int_type;
> + u32 int_pol;
> + u32 int_deb;
> +} dwapb_context;
> +
> +static int dwapb_gpio_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
> + struct bgpio_chip *bgc = >ports[0].bgc;
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(>lock, flags);
> + for (i = 0; i < gpio->nr_ports; i++) {
> + unsigned int offset;
> + unsigned int idx = gpio->ports[i].idx;
> +
> + offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
> + dwapb_context.dir[i]= dwapb_read(gpio, offset);
> +
> + offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
> + dwapb_context.data[i] = dwapb_read(gpio, offset);
> +
> + offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
> + dwapb_context.ext[i]= dwapb_read(gpio, offset);
> +
> + if (idx == 0) {
> + dwapb_context.int_mask = dwapb_read(gpio, GPIO_INTMASK);
> + dwapb_context.int_en = dwapb_read(gpio, GPIO_INTEN);
> + dwapb_context.int_pol =
> + dwapb_read(gpio, GPIO_INT_POLARITY);
> + dwapb_context.int_type =
> + dwapb_read(gpio, GPIO_INTTYPE_LEVEL);
> + dwapb_context.int_deb =
> + dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
> +
> + /* Mask out interrupts */
> + dwapb_write(gpio, GPIO_INTMASK, 0x);
> + }
> + }
> + spin_unlock_irqrestore(>lock, flags);
> +
> + return 0;
> +}
> +
> +static int dwapb_gpio_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
> + struct bgpio_chip *bgc = >ports[0].bgc;
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(>lock, flags);
> + for (i = 0; i < gpio->nr_ports; i++) {
> + unsigned int offset;
> + unsigned int idx = gpio->ports[i].idx;
> +
> + offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
> + dwapb_write(gpio, offset, dwapb_context.data[i]);
> +
> + offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
> + dwapb_write(gpio, offset, dwapb_context.dir[i]);
> +
> + offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
> + dwapb_write(gpio, offset, dwapb_context.ext[i]);
> +
> + if (idx == 0) {
> + dwapb_write(gpio, GPIO_INTTYPE_LEVEL,
> + dwapb_context.int_type);
> + dwapb_write(gpio, GPIO_INT_POLARITY,
> + dwapb_context.int_pol);
> + dwapb_write(gpio, GPIO_PORTA_DEBOUNCE,
> + dwapb_context.int_deb);
> + dwapb_write(gpio, GPIO_INTEN, dwapb_context.int_en);
> + dwapb_write(gpio, GPIO_INTMASK, dwapb_context.int_mask);
> +
> + /* Clear out spurious interrupts */
> +
Re: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend Resume PM enabling
On Fri, 2014-09-05 at 07:53 -0700, Weike Chen wrote:
This patch enables suspend and resume mode for the power management, and
it is based on Josef Ahmad's previous work.
Reviewed-by: Hock Leong Kweh hock.leong.k...@intel.com
Reviewed-by: Shevchenko, Andriy andriy.shevche...@intel.com
I have to recall my reviwed-by tag since patch is quite changed and as I
understood Linus is continuing to be changed.
Signed-off-by: Weike Chen alvin.c...@intel.com
---
drivers/gpio/gpio-dwapb.c | 102
+
1 file changed, 102 insertions(+)
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 6db7501..a103def 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -54,6 +54,7 @@ struct dwapb_gpio_port {
struct bgpio_chip bgc;
boolis_registered;
struct dwapb_gpio *gpio;
+ unsigned intidx;
};
struct dwapb_gpio {
@@ -376,6 +377,7 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
port = gpio-ports[offs];
port-gpio = gpio;
+ port-idx = pp-idx;
dat = gpio-regs + GPIO_EXT_PORTA + (pp-idx * GPIO_EXT_PORT_SIZE);
set = gpio-regs + GPIO_SWPORTA_DR + (pp-idx * GPIO_SWPORT_DR_SIZE);
@@ -594,10 +596,110 @@ static const struct of_device_id dwapb_of_match[] = {
};
MODULE_DEVICE_TABLE(of, dwapb_of_match);
+#ifdef CONFIG_PM_SLEEP
+/* Store GPIO context across system-wide suspend/resume transitions */
+static struct dwapb_context {
+ u32 data[DWAPB_MAX_PORTS];
+ u32 dir[DWAPB_MAX_PORTS];
+ u32 ext[DWAPB_MAX_PORTS];
+ u32 int_en;
+ u32 int_mask;
+ u32 int_type;
+ u32 int_pol;
+ u32 int_deb;
+} dwapb_context;
+
+static int dwapb_gpio_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
+ struct bgpio_chip *bgc = gpio-ports[0].bgc;
+ unsigned long flags;
+ int i;
+
+ spin_lock_irqsave(bgc-lock, flags);
+ for (i = 0; i gpio-nr_ports; i++) {
+ unsigned int offset;
+ unsigned int idx = gpio-ports[i].idx;
+
+ offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
+ dwapb_context.dir[i]= dwapb_read(gpio, offset);
+
+ offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
+ dwapb_context.data[i] = dwapb_read(gpio, offset);
+
+ offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
+ dwapb_context.ext[i]= dwapb_read(gpio, offset);
+
+ if (idx == 0) {
+ dwapb_context.int_mask = dwapb_read(gpio, GPIO_INTMASK);
+ dwapb_context.int_en = dwapb_read(gpio, GPIO_INTEN);
+ dwapb_context.int_pol =
+ dwapb_read(gpio, GPIO_INT_POLARITY);
+ dwapb_context.int_type =
+ dwapb_read(gpio, GPIO_INTTYPE_LEVEL);
+ dwapb_context.int_deb =
+ dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
+
+ /* Mask out interrupts */
+ dwapb_write(gpio, GPIO_INTMASK, 0x);
+ }
+ }
+ spin_unlock_irqrestore(bgc-lock, flags);
+
+ return 0;
+}
+
+static int dwapb_gpio_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct dwapb_gpio *gpio = platform_get_drvdata(pdev);
+ struct bgpio_chip *bgc = gpio-ports[0].bgc;
+ unsigned long flags;
+ int i;
+
+ spin_lock_irqsave(bgc-lock, flags);
+ for (i = 0; i gpio-nr_ports; i++) {
+ unsigned int offset;
+ unsigned int idx = gpio-ports[i].idx;
+
+ offset = GPIO_SWPORTA_DR + (idx * GPIO_SWPORT_DR_SIZE);
+ dwapb_write(gpio, offset, dwapb_context.data[i]);
+
+ offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
+ dwapb_write(gpio, offset, dwapb_context.dir[i]);
+
+ offset = GPIO_EXT_PORTA + (idx * GPIO_EXT_PORT_SIZE);
+ dwapb_write(gpio, offset, dwapb_context.ext[i]);
+
+ if (idx == 0) {
+ dwapb_write(gpio, GPIO_INTTYPE_LEVEL,
+ dwapb_context.int_type);
+ dwapb_write(gpio, GPIO_INT_POLARITY,
+ dwapb_context.int_pol);
+ dwapb_write(gpio, GPIO_PORTA_DEBOUNCE,
+ dwapb_context.int_deb);
+ dwapb_write(gpio, GPIO_INTEN, dwapb_context.int_en);
+ dwapb_write(gpio, GPIO_INTMASK, dwapb_context.int_mask);
+
+ /* Clear out spurious interrupts */
+ dwapb_write(gpio, GPIO_PORTA_EOI, 0x);
+
RE: [PATCH 3/3 v2] GPIO: gpio-dwapb: Suspend Resume PM enabling
On Fri, 2014-09-05 at 07:53 -0700, Weike Chen wrote: This patch enables suspend and resume mode for the power management, and it is based on Josef Ahmad's previous work. Reviewed-by: Hock Leong Kweh hock.leong.k...@intel.com Reviewed-by: Shevchenko, Andriy andriy.shevche...@intel.com I have to recall my reviwed-by tag since patch is quite changed and as I understood Linus is continuing to be changed. OK.
