[fixed top-posted reply]

On Wed, Feb 9, 2011 at 3:16 AM, Henk Stegeman [off-list ref] wro=
te:

On Mon, Feb 7, 2011 at 12:05 AM, Benjamin Herrenschmidt
[off-list ref] wrote:

quoted

On Sat, 2011-01-15 at 02:28 +0100, Henk Stegeman wrote:

quoted

When using the GPT as interrupt controller interupts were missing.
It turned out the IrqEn bit of the GPT is not a mask bit, but it also
disables interrupt generation. This modification masks interrupt one
level down the cascade. Note that masking one level down the cascade
is only valid here because the GPT as interrupt ontroller only serves
one IRQ.

I'm not too sure here... You shouldn't implemen t both mask/unmask and
enable/disable on the same irq_chip and certainly not cal
enable_irq/disable_irq from a mask or an unmask callback...

Now, I'm not familiar with the HW here, can you tell me more about what
exactly is happening, how things are layed out and what you are trying
to fix ?

[...]

Because the old code in the unmask/mask function did enable/disable
and I didn't want to just drop that code, I provided it via the
enable/disable function.
What is wrong by implementing & registering both mask/unmask and
enable/disable for the same irq_chip?
If it is wrong it would be nice to let the kernel print a big fat
warning when this is registered.

After some digging, yes Ben is right.  It doesn't make much sense to
provide an enable/disable function along with the mask/unmask.  I
think you can safely drop the old enable/disable code.  I'm going to
drop this patch from my tree and you can respin and retest.

g.

Cheers,

Henk

quoted

quoted

Signed-off-by: Henk Stegeman <redacted>
---
=A0arch/powerpc/platforms/52xx/mpc52xx_gpt.c | =A0 25 +++++++++++++++++=

+++++---

quoted

quoted

=A01 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/p=

latforms/52xx/mpc52xx_gpt.c

quoted

quoted

index 6f8ebe1..9ae2045 100644

--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c

@@ -91,7 +91,7 @@ struct mpc52xx_gpt_priv {

=A0 =A0 =A0 struct irq_host *irqhost;
=A0 =A0 =A0 u32 ipb_freq;
=A0 =A0 =A0 u8 wdt_mode;
-
+ =A0 =A0 int cascade_virq;
=A0#if defined(CONFIG_GPIOLIB)
=A0 =A0 =A0 struct of_gpio_chip of_gc;
=A0#endif

@@ -136,18 +136,35 @@ DEFINE_MUTEX(mpc52xx_gpt_list_mutex);

=A0static void mpc52xx_gpt_irq_unmask(unsigned int virq)
=A0{
=A0 =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
+
+ =A0 =A0 enable_irq(gpt->cascade_virq);
+
+}
+
+static void mpc52xx_gpt_irq_mask(unsigned int virq)
+{
+ =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
+
+ =A0 =A0 disable_irq(gpt->cascade_virq);
+}
+
+static void mpc52xx_gpt_irq_enable(unsigned int virq)
+{
+ =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
=A0 =A0 =A0 unsigned long flags;

+ =A0 =A0 dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
=A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
=A0 =A0 =A0 setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
=A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
=A0}

-static void mpc52xx_gpt_irq_mask(unsigned int virq)
+static void mpc52xx_gpt_irq_disable(unsigned int virq)
=A0{
=A0 =A0 =A0 struct mpc52xx_gpt_priv *gpt =3D get_irq_chip_data(virq);
=A0 =A0 =A0 unsigned long flags;

+ =A0 =A0 dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
=A0 =A0 =A0 spin_lock_irqsave(&gpt->lock, flags);
=A0 =A0 =A0 clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
=A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);

@@ -184,6 +201,8 @@ static struct irq_chip mpc52xx_gpt_irq_chip =3D {

=A0 =A0 =A0 .name =3D "MPC52xx GPT",
=A0 =A0 =A0 .unmask =3D mpc52xx_gpt_irq_unmask,
=A0 =A0 =A0 .mask =3D mpc52xx_gpt_irq_mask,
+ =A0 =A0 .enable =3D mpc52xx_gpt_irq_enable,
+ =A0 =A0 .disable =3D mpc52xx_gpt_irq_disable,
=A0 =A0 =A0 .ack =3D mpc52xx_gpt_irq_ack,
=A0 =A0 =A0 .set_type =3D mpc52xx_gpt_irq_set_type,
=A0};

@@ -268,7 +287,7 @@ mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt,=

 struct device_node *node)

quoted

quoted

=A0 =A0 =A0 if ((mode & MPC52xx_GPT_MODE_MS_MASK) =3D=3D 0)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(&gpt->regs->mode, mode | MPC52xx_G=

PT_MODE_MS_IC);

quoted

quoted

=A0 =A0 =A0 spin_unlock_irqrestore(&gpt->lock, flags);
-
+ =A0 =A0 gpt->cascade_virq =3D cascade_virq;
=A0 =A0 =A0 dev_dbg(gpt->dev, "%s() complete. virq=3D%i\n", __func__, c=

ascade_virq);

quoted

quoted

=A0}

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.