From mboxrd@z Thu Jan 1 00:00:00 1970 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Subject: Re: [PATCH 1/7] pinctrl: mediatek: emulate GPIO interrupt on both-edges Date: Tue, 10 Feb 2015 09:40:21 +0100 Message-ID: <20150210084021.GM10842@pengutronix.de> References: <1422339332-27498-1-git-send-email-chaotian.jing@mediatek.com> <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> Sender: linux-mmc-owner@vger.kernel.org To: Chaotian Jing Cc: Rob Herring , Matthias Brugger , Chris Ball , Ulf Hansson , Linus Walleij , Mark Rutland , James Liao , srv_heupstream@mediatek.com, Arnd Bergmann , devicetree@vger.kernel.org, Hongzhou Yang , Catalin Marinas , linux-mmc@vger.kernel.org, Will Deacon , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Sascha Hauer , "Joe.C" , Eddie Huang , bin.zhang@mediatek.com, linux-arm-kernel@lists.infradead.org List-Id: linux-gpio@vger.kernel.org Hello, On Tue, Jan 27, 2015 at 02:15:26PM +0800, Chaotian Jing wrote: > From: Yingjoe Chen >=20 > MTK EINT does not support generating interrupt on both edges. > Emulate this by changing edge polarity while enable irq, > set types and interrupt handling. This follows an example of > drivers/gpio/gpio-mxc.c. >=20 > Signed-off-by: Yingjoe Chen > Signed-off-by: Chaotian Jing > --- > drivers/pinctrl/mediatek/pinctrl-mt8135.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mt8173.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 76 +++++++++++++++++= ++++++++-- > drivers/pinctrl/mediatek/pinctrl-mtk-common.h | 4 ++ > 4 files changed, 83 insertions(+), 3 deletions(-) >=20 > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8135.c b/drivers/pinc= trl/mediatek/pinctrl-mt8135.c > index b6ee2b2..1296d6d 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8135.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8135.c > @@ -325,6 +325,9 @@ static const struct mtk_pinctrl_devdata mt8135_pi= nctrl_data =3D { > .sens =3D 0x140, > .sens_set =3D 0x180, > .sens_clr =3D 0x1c0, > + .soft =3D 0x200, > + .soft_set =3D 0x240, > + .soft_clr =3D 0x280, > .pol =3D 0x300, > .pol_set =3D 0x340, > .pol_clr =3D 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8173.c b/drivers/pinc= trl/mediatek/pinctrl-mt8173.c > index 444d88d..717000e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8173.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8173.c > @@ -278,6 +278,9 @@ static const struct mtk_pinctrl_devdata mt8173_pi= nctrl_data =3D { > .sens =3D 0x140, > .sens_set =3D 0x180, > .sens_clr =3D 0x1c0, > + .soft =3D 0x200, > + .soft_set =3D 0x240, > + .soft_clr =3D 0x280, > .pol =3D 0x300, > .pol_set =3D 0x340, > .pol_clr =3D 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/= pinctrl/mediatek/pinctrl-mtk-common.c > index 109a882..820ce9e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > @@ -792,6 +792,32 @@ static unsigned int mtk_eint_get_mask(struct mtk= _pinctrl *pctl, > return !!(readl(reg) & bit); > } > =20 > +static int mtk_eint_flip_edge(struct mtk_pinctrl *pctl, int hwirq) > +{ > + int start_level, curr_level; > + unsigned int reg_offset; > + const struct mtk_eint_offsets *eint_offsets =3D &(pctl->devdata->ei= nt_offsets); > + u32 mask =3D 1 << (hwirq & 0x1f); > + u32 port =3D (hwirq >> 5) & eint_offsets->port_mask; > + void __iomem *reg =3D pctl->eint_reg_base + (port << 2); > + const struct mtk_desc_pin *pin; > + > + pin =3D mtk_find_pin_by_eint_num(pctl, hwirq); > + curr_level =3D mtk_gpio_get(pctl->chip, pin->pin.number); > + do { > + start_level =3D curr_level; > + if (start_level) > + reg_offset =3D eint_offsets->pol_clr; > + else > + reg_offset =3D eint_offsets->pol_set; > + writel(mask, reg + reg_offset); > + > + curr_level =3D mtk_gpio_get(pctl->chip, pin->pin.number); > + } while (start_level !=3D curr_level); > + > + return start_level; > +} > + > static void mtk_eint_mask(struct irq_data *d) > { > struct mtk_pinctrl *pctl =3D irq_data_get_irq_chip_data(d); > @@ -814,6 +840,9 @@ static void mtk_eint_unmask(struct irq_data *d) > eint_offsets->mask_clr); > =20 > writel(mask, reg); > + > + if (pctl->eint_dual_edges[d->hwirq]) > + mtk_eint_flip_edge(pctl, d->hwirq); > } =46rom looking at the code it seems to me that there is a bug. Consider the following to happen: pin changes level, say high to low, triggers irq irq is masked by writel(mask, reg) in mtk_eint_mask mtk_eint_flip_edge gets curr_level =3D low pin goes up writel(mask, reg + eint_offsets->pol_set); oh, pin is high, so: writel(mask, reg + eint_offsets->pol_clr So now you trigger the irq the next time when the pin goes down again. But that means to missed to trigger on the "pin goes up" in the above list, right? Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig = | Industrial Linux Solutions | http://www.pengutronix.de/= | From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753962AbbBJIkw (ORCPT ); Tue, 10 Feb 2015 03:40:52 -0500 Received: from metis.ext.pengutronix.de ([92.198.50.35]:52491 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753637AbbBJIku (ORCPT ); Tue, 10 Feb 2015 03:40:50 -0500 Date: Tue, 10 Feb 2015 09:40:21 +0100 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Chaotian Jing Cc: Rob Herring , Matthias Brugger , Chris Ball , Ulf Hansson , Linus Walleij , Mark Rutland , James Liao , srv_heupstream@mediatek.com, Arnd Bergmann , devicetree@vger.kernel.org, Hongzhou Yang , Catalin Marinas , linux-mmc@vger.kernel.org, Will Deacon , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Sascha Hauer , "Joe.C" , Eddie Huang , bin.zhang@mediatek.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/7] pinctrl: mediatek: emulate GPIO interrupt on both-edges Message-ID: <20150210084021.GM10842@pengutronix.de> References: <1422339332-27498-1-git-send-email-chaotian.jing@mediatek.com> <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Tue, Jan 27, 2015 at 02:15:26PM +0800, Chaotian Jing wrote: > From: Yingjoe Chen > > MTK EINT does not support generating interrupt on both edges. > Emulate this by changing edge polarity while enable irq, > set types and interrupt handling. This follows an example of > drivers/gpio/gpio-mxc.c. > > Signed-off-by: Yingjoe Chen > Signed-off-by: Chaotian Jing > --- > drivers/pinctrl/mediatek/pinctrl-mt8135.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mt8173.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 76 +++++++++++++++++++++++++-- > drivers/pinctrl/mediatek/pinctrl-mtk-common.h | 4 ++ > 4 files changed, 83 insertions(+), 3 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8135.c b/drivers/pinctrl/mediatek/pinctrl-mt8135.c > index b6ee2b2..1296d6d 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8135.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8135.c > @@ -325,6 +325,9 @@ static const struct mtk_pinctrl_devdata mt8135_pinctrl_data = { > .sens = 0x140, > .sens_set = 0x180, > .sens_clr = 0x1c0, > + .soft = 0x200, > + .soft_set = 0x240, > + .soft_clr = 0x280, > .pol = 0x300, > .pol_set = 0x340, > .pol_clr = 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8173.c b/drivers/pinctrl/mediatek/pinctrl-mt8173.c > index 444d88d..717000e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8173.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8173.c > @@ -278,6 +278,9 @@ static const struct mtk_pinctrl_devdata mt8173_pinctrl_data = { > .sens = 0x140, > .sens_set = 0x180, > .sens_clr = 0x1c0, > + .soft = 0x200, > + .soft_set = 0x240, > + .soft_clr = 0x280, > .pol = 0x300, > .pol_set = 0x340, > .pol_clr = 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > index 109a882..820ce9e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > @@ -792,6 +792,32 @@ static unsigned int mtk_eint_get_mask(struct mtk_pinctrl *pctl, > return !!(readl(reg) & bit); > } > > +static int mtk_eint_flip_edge(struct mtk_pinctrl *pctl, int hwirq) > +{ > + int start_level, curr_level; > + unsigned int reg_offset; > + const struct mtk_eint_offsets *eint_offsets = &(pctl->devdata->eint_offsets); > + u32 mask = 1 << (hwirq & 0x1f); > + u32 port = (hwirq >> 5) & eint_offsets->port_mask; > + void __iomem *reg = pctl->eint_reg_base + (port << 2); > + const struct mtk_desc_pin *pin; > + > + pin = mtk_find_pin_by_eint_num(pctl, hwirq); > + curr_level = mtk_gpio_get(pctl->chip, pin->pin.number); > + do { > + start_level = curr_level; > + if (start_level) > + reg_offset = eint_offsets->pol_clr; > + else > + reg_offset = eint_offsets->pol_set; > + writel(mask, reg + reg_offset); > + > + curr_level = mtk_gpio_get(pctl->chip, pin->pin.number); > + } while (start_level != curr_level); > + > + return start_level; > +} > + > static void mtk_eint_mask(struct irq_data *d) > { > struct mtk_pinctrl *pctl = irq_data_get_irq_chip_data(d); > @@ -814,6 +840,9 @@ static void mtk_eint_unmask(struct irq_data *d) > eint_offsets->mask_clr); > > writel(mask, reg); > + > + if (pctl->eint_dual_edges[d->hwirq]) > + mtk_eint_flip_edge(pctl, d->hwirq); > } >>From looking at the code it seems to me that there is a bug. Consider the following to happen: pin changes level, say high to low, triggers irq irq is masked by writel(mask, reg) in mtk_eint_mask mtk_eint_flip_edge gets curr_level = low pin goes up writel(mask, reg + eint_offsets->pol_set); oh, pin is high, so: writel(mask, reg + eint_offsets->pol_clr So now you trigger the irq the next time when the pin goes down again. But that means to missed to trigger on the "pin goes up" in the above list, right? Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | From mboxrd@z Thu Jan 1 00:00:00 1970 From: u.kleine-koenig@pengutronix.de (Uwe =?iso-8859-1?Q?Kleine-K=F6nig?=) Date: Tue, 10 Feb 2015 09:40:21 +0100 Subject: [PATCH 1/7] pinctrl: mediatek: emulate GPIO interrupt on both-edges In-Reply-To: <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> References: <1422339332-27498-1-git-send-email-chaotian.jing@mediatek.com> <1422339332-27498-2-git-send-email-chaotian.jing@mediatek.com> Message-ID: <20150210084021.GM10842@pengutronix.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello, On Tue, Jan 27, 2015 at 02:15:26PM +0800, Chaotian Jing wrote: > From: Yingjoe Chen > > MTK EINT does not support generating interrupt on both edges. > Emulate this by changing edge polarity while enable irq, > set types and interrupt handling. This follows an example of > drivers/gpio/gpio-mxc.c. > > Signed-off-by: Yingjoe Chen > Signed-off-by: Chaotian Jing > --- > drivers/pinctrl/mediatek/pinctrl-mt8135.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mt8173.c | 3 ++ > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 76 +++++++++++++++++++++++++-- > drivers/pinctrl/mediatek/pinctrl-mtk-common.h | 4 ++ > 4 files changed, 83 insertions(+), 3 deletions(-) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8135.c b/drivers/pinctrl/mediatek/pinctrl-mt8135.c > index b6ee2b2..1296d6d 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8135.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8135.c > @@ -325,6 +325,9 @@ static const struct mtk_pinctrl_devdata mt8135_pinctrl_data = { > .sens = 0x140, > .sens_set = 0x180, > .sens_clr = 0x1c0, > + .soft = 0x200, > + .soft_set = 0x240, > + .soft_clr = 0x280, > .pol = 0x300, > .pol_set = 0x340, > .pol_clr = 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8173.c b/drivers/pinctrl/mediatek/pinctrl-mt8173.c > index 444d88d..717000e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mt8173.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8173.c > @@ -278,6 +278,9 @@ static const struct mtk_pinctrl_devdata mt8173_pinctrl_data = { > .sens = 0x140, > .sens_set = 0x180, > .sens_clr = 0x1c0, > + .soft = 0x200, > + .soft_set = 0x240, > + .soft_clr = 0x280, > .pol = 0x300, > .pol_set = 0x340, > .pol_clr = 0x380, > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > index 109a882..820ce9e 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c > @@ -792,6 +792,32 @@ static unsigned int mtk_eint_get_mask(struct mtk_pinctrl *pctl, > return !!(readl(reg) & bit); > } > > +static int mtk_eint_flip_edge(struct mtk_pinctrl *pctl, int hwirq) > +{ > + int start_level, curr_level; > + unsigned int reg_offset; > + const struct mtk_eint_offsets *eint_offsets = &(pctl->devdata->eint_offsets); > + u32 mask = 1 << (hwirq & 0x1f); > + u32 port = (hwirq >> 5) & eint_offsets->port_mask; > + void __iomem *reg = pctl->eint_reg_base + (port << 2); > + const struct mtk_desc_pin *pin; > + > + pin = mtk_find_pin_by_eint_num(pctl, hwirq); > + curr_level = mtk_gpio_get(pctl->chip, pin->pin.number); > + do { > + start_level = curr_level; > + if (start_level) > + reg_offset = eint_offsets->pol_clr; > + else > + reg_offset = eint_offsets->pol_set; > + writel(mask, reg + reg_offset); > + > + curr_level = mtk_gpio_get(pctl->chip, pin->pin.number); > + } while (start_level != curr_level); > + > + return start_level; > +} > + > static void mtk_eint_mask(struct irq_data *d) > { > struct mtk_pinctrl *pctl = irq_data_get_irq_chip_data(d); > @@ -814,6 +840,9 @@ static void mtk_eint_unmask(struct irq_data *d) > eint_offsets->mask_clr); > > writel(mask, reg); > + > + if (pctl->eint_dual_edges[d->hwirq]) > + mtk_eint_flip_edge(pctl, d->hwirq); > } >>From looking at the code it seems to me that there is a bug. Consider the following to happen: pin changes level, say high to low, triggers irq irq is masked by writel(mask, reg) in mtk_eint_mask mtk_eint_flip_edge gets curr_level = low pin goes up writel(mask, reg + eint_offsets->pol_set); oh, pin is high, so: writel(mask, reg + eint_offsets->pol_clr So now you trigger the irq the next time when the pin goes down again. But that means to missed to trigger on the "pin goes up" in the above list, right? Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |