[fixed top-posted reply]
On Wed, Feb 9, 2011 at 3:16 AM, Henk Stegeman <henk.stege...@gmail.com> wrote:
> On Mon, Feb 7, 2011 at 12:05 AM, Benjamin Herrenschmidt
> <b...@kernel.crashing.org> wrote:
>> On Sat, 2011-01-15 at 02:28 +0100, Henk Stegeman wrote:
>>> 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
>
>>
>>> Signed-off-by: Henk Stegeman <henk.stege...@gmail.com>
>>> ---
>>> arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 25 ++++++++++++++++++++++---
>>> 1 files changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
>>> b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
>>> 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 {
>>> struct irq_host *irqhost;
>>> u32 ipb_freq;
>>> u8 wdt_mode;
>>> -
>>> + int cascade_virq;
>>> #if defined(CONFIG_GPIOLIB)
>>> struct of_gpio_chip of_gc;
>>> #endif
>>> @@ -136,18 +136,35 @@ DEFINE_MUTEX(mpc52xx_gpt_list_mutex);
>>> static void mpc52xx_gpt_irq_unmask(unsigned int virq)
>>> {
>>> struct mpc52xx_gpt_priv *gpt = get_irq_chip_data(virq);
>>> +
>>> + enable_irq(gpt->cascade_virq);
>>> +
>>> +}
>>> +
>>> +static void mpc52xx_gpt_irq_mask(unsigned int virq)
>>> +{
>>> + struct mpc52xx_gpt_priv *gpt = get_irq_chip_data(virq);
>>> +
>>> + disable_irq(gpt->cascade_virq);
>>> +}
>>> +
>>> +static void mpc52xx_gpt_irq_enable(unsigned int virq)
>>> +{
>>> + struct mpc52xx_gpt_priv *gpt = get_irq_chip_data(virq);
>>> unsigned long flags;
>>>
>>> + dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
>>> spin_lock_irqsave(&gpt->lock, flags);
>>> setbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>>> spin_unlock_irqrestore(&gpt->lock, flags);
>>> }
>>>
>>> -static void mpc52xx_gpt_irq_mask(unsigned int virq)
>>> +static void mpc52xx_gpt_irq_disable(unsigned int virq)
>>> {
>>> struct mpc52xx_gpt_priv *gpt = get_irq_chip_data(virq);
>>> unsigned long flags;
>>>
>>> + dev_dbg(gpt->dev, "%s %d\n", __func__, virq);
>>> spin_lock_irqsave(&gpt->lock, flags);
>>> clrbits32(&gpt->regs->mode, MPC52xx_GPT_MODE_IRQ_EN);
>>> spin_unlock_irqrestore(&gpt->lock, flags);
>>> @@ -184,6 +201,8 @@ static struct irq_chip mpc52xx_gpt_irq_chip = {
>>> .name = "MPC52xx GPT",
>>> .unmask = mpc52xx_gpt_irq_unmask,
>>> .mask = mpc52xx_gpt_irq_mask,
>>> + .enable = mpc52xx_gpt_irq_enable,
>>> + .disable = mpc52xx_gpt_irq_disable,
>>> .ack = mpc52xx_gpt_irq_ack,
>>> .set_type = mpc52xx_gpt_irq_set_type,
>>> };
>>> @@ -268,7 +287,7 @@ mpc52xx_gpt_irq_setup(struct mpc52xx_gpt_priv *gpt,
>>> struct device_node *node)
>>> if ((mode & MPC52xx_GPT_MODE_MS_MASK) == 0)
>>> out_be32(&gpt->regs->mode, mode | MPC52xx_GPT_MODE_MS_IC);
>>> spin_unlock_irqrestore(&gpt->lock, flags);
>>> -
>>> + gpt->cascade_virq = cascade_virq;
>>> dev_dbg(gpt->dev, "%s() complete. virq=%i\n", __func__, cascade_virq);
>>> }
>>>
>>
>>
>>
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
