| Message ID | 1368724352-10849-3-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote: > From: Prathyush K <prathyush.k@samsung.com> > > Add the irq_set_wake function for exynos pinctrl to configure the > external interrupt wakeup mask register. > > [dianders: minor nit fixes; port to ToT] > > Signed-off-by: Prathyush K <prathyush.k@samsung.com> > Signed-off-by: Doug Anderson <dianders@chromium.org> > --- > drivers/pinctrl/pinctrl-exynos.c | 45 > ++++++++++++++++++++++++++++----------- > drivers/pinctrl/pinctrl-exynos.h | 3 ++- > drivers/pinctrl/pinctrl-samsung.h | 2 ++ > 3 files changed, 37 insertions(+), 13 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-exynos.c > b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 > --- a/drivers/pinctrl/pinctrl-exynos.c > +++ b/drivers/pinctrl/pinctrl-exynos.c > @@ -30,6 +30,8 @@ > #include <linux/spinlock.h> > #include <linux/err.h> > > +#include <plat/pm.h> > + This is not going to work with CONFIG_MULTIPLATFORM. Now this raises a question what is the preferred way to pass some data from generic driver to platform code. I would suggest adding a function called exynos_pinctrl_get_eintmask() (or whatever) that would return the wake-up mask configured in the driver and then modify platform code to use it. > #include "pinctrl-samsung.h" > #include "pinctrl-exynos.h" > > @@ -326,6 +328,24 @@ static int exynos_wkup_irq_set_type(struct irq_data > *irqd, unsigned int type) return 0; > } > > +static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int > state) +{ > + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); > + const int eint_num = bank->eint_base + irqd->hwirq; > + unsigned long bit = 1L << eint_num; > + > + pr_info("wake %s for eint %d / %s[%ld]\n", > + state ? "enabled" : "disabled", > + eint_num, bank->name, irqd->hwirq); > + > + if (!state) > + s3c_irqwake_eintmask |= bit; > + else > + s3c_irqwake_eintmask &= ~bit; > + > + return 0; > +} > + > /* > * irq_chip for wakeup interrupts > */ > @@ -335,6 +355,7 @@ static struct irq_chip exynos_wkup_irq_chip = { > .irq_mask = exynos_wkup_irq_mask, > .irq_ack = exynos_wkup_irq_ack, > .irq_set_type = exynos_wkup_irq_set_type, > + .irq_set_wake = exynos_wkup_irq_set_wake, > }; > > /* interrupt handler for wakeup interrupts 0..15 */ > @@ -543,10 +564,10 @@ static struct samsung_pin_bank > exynos4210_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, "gpy4"), > EXYNOS_PIN_BANK_EINTN(8, 0x1C0, "gpy5"), > EXYNOS_PIN_BANK_EINTN(8, 0x1E0, "gpy6"), > - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), > - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), > - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), > - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), > + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), > + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), > + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), > + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), > }; > > /* pin banks of exynos4210 pin-controller 2 */ > @@ -629,10 +650,10 @@ static struct samsung_pin_bank > exynos4x12_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, "gpy4"), > EXYNOS_PIN_BANK_EINTN(8, 0x1C0, "gpy5"), > EXYNOS_PIN_BANK_EINTN(8, 0x1E0, "gpy6"), > - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), > - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), > - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), > - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), > + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), > + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), > + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), > + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), > }; > > /* pin banks of exynos4x12 pin-controller 2 */ > @@ -724,10 +745,10 @@ static struct samsung_pin_bank > exynos5250_pin_banks0[] = { EXYNOS_PIN_BANK_EINTN(8, 0x220, "gpy4"), > EXYNOS_PIN_BANK_EINTN(8, 0x240, "gpy5"), > EXYNOS_PIN_BANK_EINTN(8, 0x260, "gpy6"), > - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), > - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), > - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), > - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), > + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), > + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), > + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), > + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), > }; > > /* pin banks of exynos5250 pin-controller 1 */ > diff --git a/drivers/pinctrl/pinctrl-exynos.h > b/drivers/pinctrl/pinctrl-exynos.h index 9b1f77a..d98e9ff 100644 > --- a/drivers/pinctrl/pinctrl-exynos.h > +++ b/drivers/pinctrl/pinctrl-exynos.h > @@ -65,13 +65,14 @@ > .name = id \ > } > > -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ > +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ > { \ > .type = &bank_type_alive, \ > .pctl_offset = reg, \ > .nr_pins = pins, \ > .eint_type = EINT_TYPE_WKUP, \ > .eint_offset = offs, \ > + .eint_base = base, \ I can't look at my patch at the moment, but I think I have managed to get EINT index without adding this extra field. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tomasz, On Thu, May 16, 2013 at 12:26 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote: >> From: Prathyush K <prathyush.k@samsung.com> >> >> Add the irq_set_wake function for exynos pinctrl to configure the >> external interrupt wakeup mask register. >> >> [dianders: minor nit fixes; port to ToT] >> >> Signed-off-by: Prathyush K <prathyush.k@samsung.com> >> Signed-off-by: Doug Anderson <dianders@chromium.org> >> --- >> drivers/pinctrl/pinctrl-exynos.c | 45 >> ++++++++++++++++++++++++++++----------- >> drivers/pinctrl/pinctrl-exynos.h | 3 ++- >> drivers/pinctrl/pinctrl-samsung.h | 2 ++ >> 3 files changed, 37 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/pinctrl/pinctrl-exynos.c >> b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 >> --- a/drivers/pinctrl/pinctrl-exynos.c >> +++ b/drivers/pinctrl/pinctrl-exynos.c >> @@ -30,6 +30,8 @@ >> #include <linux/spinlock.h> >> #include <linux/err.h> >> >> +#include <plat/pm.h> >> + > > This is not going to work with CONFIG_MULTIPLATFORM. Hmm, this sounds like it might be a bit of a long path, especially since I haven't been keeping up with what's been going on with MULTIPLATFORM and I'm currently midway through making 3.8 work (which has no MULTIPLATFORM). Perhaps for this patch it makes more sense for you to post your version and I can review it? We may end up just keeping our version of this patch for 3.8 and pick up yours when we do our next rebase. Does that sound OK? >> -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ >> +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ >> { \ >> .type = &bank_type_alive, \ >> .pctl_offset = reg, \ >> .nr_pins = pins, \ >> .eint_type = EINT_TYPE_WKUP, \ >> .eint_offset = offs, \ >> + .eint_base = base, \ > > I can't look at my patch at the moment, but I think I have managed to get > EINT index without adding this extra field. It looks like this is always 2 * eint_offset in the code above. Maybe you just multiplied? The multiplication works fine although I think specifying eint_base like this might be more generic and handle future chips better? Ya never know... -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 16 of May 2013 15:25:15 Doug Anderson wrote: > Tomasz, > > On Thu, May 16, 2013 at 12:26 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > > On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote: > >> From: Prathyush K <prathyush.k@samsung.com> > >> > >> Add the irq_set_wake function for exynos pinctrl to configure the > >> external interrupt wakeup mask register. > >> > >> [dianders: minor nit fixes; port to ToT] > >> > >> Signed-off-by: Prathyush K <prathyush.k@samsung.com> > >> Signed-off-by: Doug Anderson <dianders@chromium.org> > >> --- > >> > >> drivers/pinctrl/pinctrl-exynos.c | 45 > >> > >> ++++++++++++++++++++++++++++----------- > >> drivers/pinctrl/pinctrl-exynos.h | 3 ++- > >> > >> drivers/pinctrl/pinctrl-samsung.h | 2 ++ > >> 3 files changed, 37 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/pinctrl/pinctrl-exynos.c > >> b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 > >> --- a/drivers/pinctrl/pinctrl-exynos.c > >> +++ b/drivers/pinctrl/pinctrl-exynos.c > >> @@ -30,6 +30,8 @@ > >> > >> #include <linux/spinlock.h> > >> #include <linux/err.h> > >> > >> +#include <plat/pm.h> > >> + > > > > This is not going to work with CONFIG_MULTIPLATFORM. > > Hmm, this sounds like it might be a bit of a long path, especially > since I haven't been keeping up with what's been going on with > MULTIPLATFORM and I'm currently midway through making 3.8 work (which > has no MULTIPLATFORM). Well, to make long story short, including headers from plat/ and mach/ from files outside plat/ or mach/ is no longer valid with CONFIG_MULTIPLATFORM, because more than one plat and/or mach can be enabled at the same time. In addition to this, care must be taken for code to not break platforms other than written for, when compiled into the resulting kernel. > Perhaps for this patch it makes more sense for you to post your > version and I can review it? We may end up just keeping our version > of this patch for 3.8 and pick up yours when we do our next rebase. > Does that sound OK? Fine. I will also send a patch adding save and restore for several EINT registers that need it. > >> -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ > >> +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ > >> > >> { \ > >> > >> .type = &bank_type_alive, \ > >> .pctl_offset = reg, \ > >> .nr_pins = pins, \ > >> .eint_type = EINT_TYPE_WKUP, \ > >> .eint_offset = offs, \ > >> > >> + .eint_base = base, \ > > > > I can't look at my patch at the moment, but I think I have managed to > > get EINT index without adding this extra field. > > It looks like this is always 2 * eint_offset in the code above. Maybe > you just multiplied? The multiplication works fine although I think > specifying eint_base like this might be more generic and handle future > chips better? Ya never know... Since EINT handling is highly SoC-specific (i.e. done in pinctrl-exynos, not pinctrl-samsung), such assumption wouldn't be a problem. Let me see how I solved this problem in my version tomorrow at work. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tomasz, On Thu, May 16, 2013 at 3:37 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > Well, to make long story short, including headers from plat/ and mach/ > from files outside plat/ or mach/ is no longer valid with > CONFIG_MULTIPLATFORM, because more than one plat and/or mach can be > enabled at the same time. In addition to this, care must be taken for code > to not break platforms other than written for, when compiled into the > resulting kernel. Right. That makes sense. That's also why it would be a bit of a longshot for me to get this done right now. I'd imagine that there would be a number of changes to the samsung pm infrastructure that are needed to make this work and I don't have all of those in my tree right now. We've already picked back a lot to 3.8, but multiplatform seems too much. >> Perhaps for this patch it makes more sense for you to post your >> version and I can review it? We may end up just keeping our version >> of this patch for 3.8 and pick up yours when we do our next rebase. >> Does that sound OK? > > Fine. I will also send a patch adding save and restore for several EINT > registers that need it. OK, sounds good. I was trying to figure out why we didn't seem to have those in our 3.4 stuff and that it seems to work without saving/restoring. I assumed that maybe higher level code was masking/unmasking interrupts but didn't dig. > Since EINT handling is highly SoC-specific (i.e. done in pinctrl-exynos, > not pinctrl-samsung), such assumption wouldn't be a problem. Let me see > how I solved this problem in my version tomorrow at work. Fair enough. :) Looking forward to seeing your patch! -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 --- a/drivers/pinctrl/pinctrl-exynos.c +++ b/drivers/pinctrl/pinctrl-exynos.c @@ -30,6 +30,8 @@ #include <linux/spinlock.h> #include <linux/err.h> +#include <plat/pm.h> + #include "pinctrl-samsung.h" #include "pinctrl-exynos.h" @@ -326,6 +328,24 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, unsigned int type) return 0; } +static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int state) +{ + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); + const int eint_num = bank->eint_base + irqd->hwirq; + unsigned long bit = 1L << eint_num; + + pr_info("wake %s for eint %d / %s[%ld]\n", + state ? "enabled" : "disabled", + eint_num, bank->name, irqd->hwirq); + + if (!state) + s3c_irqwake_eintmask |= bit; + else + s3c_irqwake_eintmask &= ~bit; + + return 0; +} + /* * irq_chip for wakeup interrupts */ @@ -335,6 +355,7 @@ static struct irq_chip exynos_wkup_irq_chip = { .irq_mask = exynos_wkup_irq_mask, .irq_ack = exynos_wkup_irq_ack, .irq_set_type = exynos_wkup_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, }; /* interrupt handler for wakeup interrupts 0..15 */ @@ -543,10 +564,10 @@ static struct samsung_pin_bank exynos4210_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, "gpy4"), EXYNOS_PIN_BANK_EINTN(8, 0x1C0, "gpy5"), EXYNOS_PIN_BANK_EINTN(8, 0x1E0, "gpy6"), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), }; /* pin banks of exynos4210 pin-controller 2 */ @@ -629,10 +650,10 @@ static struct samsung_pin_bank exynos4x12_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, "gpy4"), EXYNOS_PIN_BANK_EINTN(8, 0x1C0, "gpy5"), EXYNOS_PIN_BANK_EINTN(8, 0x1E0, "gpy6"), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), }; /* pin banks of exynos4x12 pin-controller 2 */ @@ -724,10 +745,10 @@ static struct samsung_pin_bank exynos5250_pin_banks0[] = { EXYNOS_PIN_BANK_EINTN(8, 0x220, "gpy4"), EXYNOS_PIN_BANK_EINTN(8, 0x240, "gpy5"), EXYNOS_PIN_BANK_EINTN(8, 0x260, "gpy6"), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, "gpx0", 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, "gpx1", 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, "gpx2", 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, "gpx3", 0x0c, 24), }; /* pin banks of exynos5250 pin-controller 1 */ diff --git a/drivers/pinctrl/pinctrl-exynos.h b/drivers/pinctrl/pinctrl-exynos.h index 9b1f77a..d98e9ff 100644 --- a/drivers/pinctrl/pinctrl-exynos.h +++ b/drivers/pinctrl/pinctrl-exynos.h @@ -65,13 +65,14 @@ .name = id \ } -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ { \ .type = &bank_type_alive, \ .pctl_offset = reg, \ .nr_pins = pins, \ .eint_type = EINT_TYPE_WKUP, \ .eint_offset = offs, \ + .eint_base = base, \ .name = id \ } diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h index c9a7b6e..1e033ae 100644 --- a/drivers/pinctrl/pinctrl-samsung.h +++ b/drivers/pinctrl/pinctrl-samsung.h @@ -120,6 +120,7 @@ struct samsung_pin_bank_type { * @eint_func: function to set in CON register to configure pin as EINT. * @eint_type: type of the external interrupt supported by the bank. * @eint_mask: bit mask of pins which support EINT function. + * @eint_base: number of external wakeup interrupts from start to this bank. * @name: name to be prefixed for each pin in this pin bank. * @of_node: OF node of the bank. * @drvdata: link to controller driver data @@ -138,6 +139,7 @@ struct samsung_pin_bank { enum eint_type eint_type; u32 eint_mask; u32 eint_offset; + u32 eint_base; char *name; struct device_node *of_node; struct samsung_pinctrl_drv_data *drvdata;