All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hongbo Zhang <hongbo.zhang@freescale.com>
To: Scott Wood <scottwood@freescale.com>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] DTS: DMA: Fix DMA3 interrupts
Date: Thu, 12 Dec 2013 17:46:14 +0800	[thread overview]
Message-ID: <52A985E6.2040403@freescale.com> (raw)
In-Reply-To: <1386700403.10013.109.camel@snotra.buserror.net>


On 12/11/2013 02:33 AM, Scott Wood wrote:
> On Tue, 2013-12-10 at 18:33 +0800, Hongbo Zhang wrote:
>> Scott,
>> This issue is due to the non-continuous MPIC register, I think there is
>> two ways to fix it.
>>
>> The first one is as what we are discussing, in fact the Bman/Qman DT
>> author had introduced this way, and I had to follow it, it is a trick,
>> adding 208 is a bit ugly I think, and even difficult to explain it to
>> customers etc, but this way changes less codes.
>>
>> The second one is editing MPIC related codes without adding 208 to high
>> interrupts. The point of translate interrupt number to MPIC register
>> address is a so called 'isu' mechanism, we can do like the following
>> example codes, then the tricky adding 208 isn't needed any more.
>>
>> Which one do you prefer?
>> In fact I myself prefer the second, if the idea is acceptable, I will
>> send a patch instead of this one. (and also alone with the internal
>> patch decreasing 208 for the Bman/Qman)
>>
>> void __init corenet_ds_pic_init(void)
>> {
>>       ......
>>
>>       mpic = mpic_alloc(NULL, 0, flags, 0, 512, "OpenPIC");
>>       BUG_ON(mpic == NULL);
>>
>> // Add this start
>>       for (i = 0; i < 17; i++) {
>>           if (i < 11)
>>               addr_off = 0x10000 + 0x20 * 16 * i;
>>           else
>>               addr_off = 0x13000 + 0x20 * 16 * (i - 11);  /* scape the
>> address not for interrupts */
>>           mpic_assign_isu(mpic, i, mpic->paddr + addr_off);
>>       }
>> // Add this end
>>
>>       mpic_init(mpic);
>> }
> NACK
>
> We already have a binding that states that the interrupt number is based
> on the register offset, rather than whatever arbitrary numbers hardware
> documenters decide to use next week.
>
> While I'm not terribly happy with the usability of this, especially now
> that it's not a simple "add 16", redefining the existing binding is not
> OK (and in any case the code above seems obfuscatory).  If we decide to
> do something other than continue with register offset divided by 32,
> then we need to define a new interrupt type (similar to current defined
> types of error interrupt, timer, and IPI) for the new numberspace -- and
> it should be handled when decoding that type of interrupt specifier,
> rather than with the isu mechanism.
>
> -Scott
>
>
Scott,
Thanks for your comments. Since the second way isn't so good, let's 
choose the original one.
But we meet a small accident now.
My patch is based on the http://patchwork.ozlabs.org/patch/291553/, 
which had been superseded, so this thread can be closed now.

And Shenzhou has already sent a complete dma3 dtsi patch including 
correct interrupt numbers, http://patchwork.ozlabs.org/patch/300026/, so 
let's focus on this patch, and I will forward your first comments of my 
patch there.

Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Hongbo Zhang <hongbo.zhang@freescale.com>
To: Scott Wood <scottwood@freescale.com>
Cc: <benh@kernel.crashing.org>, <linuxppc-dev@lists.ozlabs.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] DTS: DMA: Fix DMA3 interrupts
Date: Thu, 12 Dec 2013 17:46:14 +0800	[thread overview]
Message-ID: <52A985E6.2040403@freescale.com> (raw)
In-Reply-To: <1386700403.10013.109.camel@snotra.buserror.net>


On 12/11/2013 02:33 AM, Scott Wood wrote:
> On Tue, 2013-12-10 at 18:33 +0800, Hongbo Zhang wrote:
>> Scott,
>> This issue is due to the non-continuous MPIC register, I think there is
>> two ways to fix it.
>>
>> The first one is as what we are discussing, in fact the Bman/Qman DT
>> author had introduced this way, and I had to follow it, it is a trick,
>> adding 208 is a bit ugly I think, and even difficult to explain it to
>> customers etc, but this way changes less codes.
>>
>> The second one is editing MPIC related codes without adding 208 to high
>> interrupts. The point of translate interrupt number to MPIC register
>> address is a so called 'isu' mechanism, we can do like the following
>> example codes, then the tricky adding 208 isn't needed any more.
>>
>> Which one do you prefer?
>> In fact I myself prefer the second, if the idea is acceptable, I will
>> send a patch instead of this one. (and also alone with the internal
>> patch decreasing 208 for the Bman/Qman)
>>
>> void __init corenet_ds_pic_init(void)
>> {
>>       ......
>>
>>       mpic = mpic_alloc(NULL, 0, flags, 0, 512, "OpenPIC");
>>       BUG_ON(mpic == NULL);
>>
>> // Add this start
>>       for (i = 0; i < 17; i++) {
>>           if (i < 11)
>>               addr_off = 0x10000 + 0x20 * 16 * i;
>>           else
>>               addr_off = 0x13000 + 0x20 * 16 * (i - 11);  /* scape the
>> address not for interrupts */
>>           mpic_assign_isu(mpic, i, mpic->paddr + addr_off);
>>       }
>> // Add this end
>>
>>       mpic_init(mpic);
>> }
> NACK
>
> We already have a binding that states that the interrupt number is based
> on the register offset, rather than whatever arbitrary numbers hardware
> documenters decide to use next week.
>
> While I'm not terribly happy with the usability of this, especially now
> that it's not a simple "add 16", redefining the existing binding is not
> OK (and in any case the code above seems obfuscatory).  If we decide to
> do something other than continue with register offset divided by 32,
> then we need to define a new interrupt type (similar to current defined
> types of error interrupt, timer, and IPI) for the new numberspace -- and
> it should be handled when decoding that type of interrupt specifier,
> rather than with the isu mechanism.
>
> -Scott
>
>
Scott,
Thanks for your comments. Since the second way isn't so good, let's 
choose the original one.
But we meet a small accident now.
My patch is based on the http://patchwork.ozlabs.org/patch/291553/, 
which had been superseded, so this thread can be closed now.

And Shenzhou has already sent a complete dma3 dtsi patch including 
correct interrupt numbers, http://patchwork.ozlabs.org/patch/300026/, so 
let's focus on this patch, and I will forward your first comments of my 
patch there.

Thanks.




  reply	other threads:[~2013-12-12  9:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-29  8:07 [PATCH] DTS: DMA: Fix DMA3 interrupts hongbo.zhang
2013-11-29  8:07 ` hongbo.zhang
2013-12-06 19:21 ` Scott Wood
2013-12-06 19:21   ` Scott Wood
2013-12-10 10:33   ` Hongbo Zhang
2013-12-10 10:33     ` Hongbo Zhang
2013-12-10 18:33     ` Scott Wood
2013-12-10 18:33       ` Scott Wood
2013-12-12  9:46       ` Hongbo Zhang [this message]
2013-12-12  9:46         ` Hongbo Zhang
2013-12-17  8:48       ` Li Yang
2013-12-17  8:48         ` Li Yang
2013-12-17 19:45         ` Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A985E6.2040403@freescale.com \
    --to=hongbo.zhang@freescale.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=scottwood@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.