From: Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
Cc: Greentime <greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>,
Linux Kernel Mailing List
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
linux-arch <linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
netdev <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Vincent Chen <deanbo422-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
DTML <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
Daniel Lezcano
<daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rick Chen <rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org>
Subject: Re: [PATCH v2 27/35] irqchip: Andestech Internal Vector Interrupt Controller driver
Date: Wed, 29 Nov 2017 23:23:34 +0800 [thread overview]
Message-ID: <CAEbi=3eXP1cfNDhu9YjZbjPghgxm9xmEtjEBO=KVOnWZyPqVbw@mail.gmail.com> (raw)
In-Reply-To: <c7447c93-9905-2840-e2d8-01837b9fdecd-5wv7dgnIgG8@public.gmane.org>
Hi, Marc:
2017-11-28 17:37 GMT+08:00 Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>:
> On 27/11/17 12:28, Greentime Hu wrote:
>> +static void ativic32_ack_irq(struct irq_data *data)
>> +{
>> + __nds32__mtsr_dsb(1 << data->hwirq, NDS32_SR_INT_PEND2);
>
> Consider writing (1 << data->hwirq) as BIT(data->hwirq).
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>
> Same here.
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_ack_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb((1 << data->hwirq), NDS32_SR_INT_PEND2);
>
> This is effectively MASK+ACK, so you're better off just writing it as
> such. And since there is no advantage in your implementation in having
> MASK_ACK over MASK+ACK, I suggest you remove this function completely,
> and rely on the core code which will call them in sequence.
I think mask_ack is still better than mask + ack because we don't need
to do two function call.
We can save a prologue and a epilogue. It will benefit interrupt latency.
>> +
>> +}
>> +
>> +static void ativic32_unmask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 | (1 << data->hwirq), NDS32_SR_INT_MASK2);
>
> Same BIT() here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static struct irq_chip ativic32_chip = {
>> + .name = "ativic32",
>> + .irq_ack = ativic32_ack_irq,
>> + .irq_mask = ativic32_mask_irq,
>> + .irq_mask_ack = ativic32_mask_ack_irq,
>> + .irq_unmask = ativic32_unmask_irq,
>> +};
>> +
>> +static unsigned int __initdata nivic_map[6] = { 6, 2, 10, 16, 24, 32 };
>> +
>> +static struct irq_domain *root_domain;
>> +static int ativic32_irq_domain_map(struct irq_domain *id, unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> +
>> + unsigned long int_trigger_type;
>> + int_trigger_type = __nds32__mfsr(NDS32_SR_INT_TRIGGER);
>> + if (int_trigger_type & (1 << hw))
>
> And here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_edge_irq);
>> + else
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_level_irq);
>
> Since you do not express the trigger in DT, you need to tell the core
> about it by calling irqd_set_trigger_type() with the right setting.
>
Since the comments say so, I will add ativic32_set_type() for irq_set_type()
in the next version patch.
/*
* Must only be called inside irq_chip.irq_set_type() functions.
*/
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)
{
__irqd_to_state(d) &= ~IRQD_TRIGGER_MASK;
__irqd_to_state(d) |= type & IRQD_TRIGGER_MASK;
}
It will be like this.
static int ativic32_set_type(struct irq_data *data, unsigned int flow_type)
{
irqd_set_trigger_type(data, flow_type);
return IRQ_SET_MASK_OK;
}
>> +
>> + return 0;
>> +}
>> +
>> +static struct irq_domain_ops ativic32_ops = {
>> + .map = ativic32_irq_domain_map,
>> + .xlate = irq_domain_xlate_onecell
>> +};
>> +
>> +static int get_intr_src(void)
>
> I'm not sure "int" is the best return type here. I suspect something
> unsigned would be preferable, or even the irq_hw_number_t type.
Thanks for this suggestion. I will modify it in the next version patch.
>> +{
>> + return ((__nds32__mfsr(NDS32_SR_ITYPE)&ITYPE_mskVECTOR) >> ITYPE_offVECTOR)
>
> Spaces around '&'.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + - NDS32_VECTOR_offINTERRUPT;
>> +}
>> +
>> +asmlinkage void asm_do_IRQ(struct pt_regs *regs)
>> +{
>> + int hwirq = get_intr_src();
>
> irq_hw_number_t.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + handle_domain_irq(root_domain, hwirq, regs);
>> +}
>> +
>> +int __init ativic32_init_irq(struct device_node *node, struct device_node *parent)
>> +{
>> + unsigned long int_vec_base, nivic;
>> +
>> + if (WARN(parent, "non-root ativic32 are not supported"))
>> + return -EINVAL;
>> +
>> + int_vec_base = __nds32__mfsr(NDS32_SR_IVB);
>> +
>> + if (((int_vec_base & IVB_mskIVIC_VER) >> IVB_offIVIC_VER) == 0)
>> + panic("Unable to use atcivic32 for this cpu.\n");
>> +
>> + nivic = (int_vec_base & IVB_mskNIVIC) >> IVB_offNIVIC;
>> + if (nivic >= (sizeof nivic_map / sizeof nivic_map[0]))
>
> This should be:
> if (nivic >= ARRAY_SIZE(NIVIC_MAP))
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + panic("The number of input for ativic32 is not supported.\n");
>> +
>> + nivic = nivic_map[nivic];
>
> I'd rather you use another variable (nr_ints?).
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +
>> + root_domain = irq_domain_add_linear(node, nivic,
>> + &ativic32_ops, NULL);
>> +
>> + if (!root_domain)
>> + panic("%s: unable to create IRQ domain\n", node->full_name);
>> +
>> + return 0;
>> +}
>> +IRQCHIP_DECLARE(ativic32, "andestech,ativic32", ativic32_init_irq);
>>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2017-11-29 15:23 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-27 12:27 [PATCH v2 00/35] Andes(nds32) Linux Kernel Port Greentime Hu
2017-11-27 12:27 ` [PATCH v2 01/35] nds32: Assembly macros and definitions Greentime Hu
2017-11-27 12:27 ` [PATCH v2 02/35] nds32: Kernel booting and initialization Greentime Hu
2017-11-27 12:27 ` [PATCH v2 03/35] nds32: Exception handling Greentime Hu
2017-11-27 12:27 ` [PATCH v2 04/35] nds32: MMU definitions Greentime Hu
2017-11-27 12:27 ` [PATCH v2 05/35] nds32: MMU initialization Greentime Hu
2017-11-27 12:27 ` [PATCH v2 06/35] nds32: MMU fault handling and page table management Greentime Hu
[not found] ` <ba92adae5d20d99c7c18e75146642a2ccbd5d047.1511785528.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-27 13:51 ` Mark Rutland
[not found] ` <20171127135136.3gnguzaf6d52tcpd-agMKViyK24J5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org>
2017-11-29 7:24 ` Greentime Hu
2017-12-07 16:40 ` Al Viro
[not found] ` <20171207164040.GD21978-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-12-08 5:26 ` Greentime Hu
2017-11-27 12:27 ` [PATCH v2 07/35] nds32: Cache and TLB routines Greentime Hu
2017-11-27 12:27 ` [PATCH v2 08/35] nds32: Process management Greentime Hu
2017-12-07 16:45 ` Al Viro
2017-12-08 5:27 ` Greentime Hu
2017-11-27 12:27 ` [PATCH v2 09/35] nds32: IRQ handling Greentime Hu
2017-11-27 12:27 ` [PATCH v2 11/35] nds32: Device specific operations Greentime Hu
2017-11-27 14:51 ` Arnd Bergmann
2017-11-27 12:27 ` [PATCH v2 12/35] nds32: DMA mapping API Greentime Hu
2017-11-27 12:28 ` [PATCH v2 13/35] nds32: ELF definitions Greentime Hu
2017-11-27 12:28 ` [PATCH v2 14/35] nds32: System calls handling Greentime Hu
2017-11-27 14:46 ` Arnd Bergmann
2017-11-28 2:18 ` Vincent Chen
2017-11-28 9:23 ` Arnd Bergmann
2017-11-27 12:28 ` [PATCH v2 15/35] nds32: VDSO support Greentime Hu
2017-11-27 12:28 ` [PATCH v2 16/35] nds32: Signal handling support Greentime Hu
[not found] ` <21cfd623872d4377ba5064cb7302bff49ebf917e.1511785528.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-27 14:37 ` Arnd Bergmann
[not found] ` <CAK8P3a1sqhLwz3WqM0Qx4w0SBWqFWMuXVgX4p9StpacfWdSnUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-28 2:21 ` Vincent Chen
2017-11-27 12:28 ` [PATCH v2 17/35] nds32: Library functions Greentime Hu
2017-11-27 12:28 ` [PATCH v2 18/35] nds32: Debugging support Greentime Hu
2017-11-27 14:34 ` Arnd Bergmann
[not found] ` <CAK8P3a2czU7=jECXFOvtRNhrq3zyX7gV7sa3OFPQ-8A4U8iH0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-28 2:21 ` Vincent Chen
2017-11-27 12:28 ` [PATCH v2 19/35] nds32: L2 cache support Greentime Hu
2017-11-27 14:33 ` Arnd Bergmann
2017-11-29 11:53 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 20/35] nds32: Loadable modules Greentime Hu
2017-11-27 12:28 ` [PATCH v2 21/35] nds32: Generic timers support Greentime Hu
2017-11-27 12:28 ` [PATCH v2 22/35] nds32: Device tree support Greentime Hu
2017-11-27 14:30 ` Arnd Bergmann
[not found] ` <CAK8P3a3nczwuuna8BGRQU11hhOFZMqGQqn9i_7D=Tzrc1PizFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-28 6:54 ` Greentime Hu
2017-11-27 19:07 ` Rob Herring
2017-11-27 19:14 ` Rob Herring
[not found] ` <CAL_Jsq+c4vt4-royBuTxAj+AY2wFHMugyyy41S5YP-QXyF2gbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-12-02 16:47 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 24/35] nds32: defconfig Greentime Hu
2017-11-27 14:27 ` Arnd Bergmann
[not found] ` <cover.1511785528.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-27 12:27 ` [PATCH v2 10/35] nds32: Atomic operations Greentime Hu
2017-11-27 13:57 ` Mark Rutland
2017-11-28 4:24 ` Vincent Chen
2017-11-27 12:28 ` [PATCH v2 23/35] nds32: Miscellaneous header files Greentime Hu
2017-11-27 12:28 ` [PATCH v2 25/35] nds32: Build infrastructure Greentime Hu
2017-11-27 14:21 ` Arnd Bergmann
2017-11-29 8:39 ` Greentime Hu
2017-11-29 8:58 ` Arnd Bergmann
2017-11-29 9:10 ` Geert Uytterhoeven
2017-11-29 9:25 ` Arnd Bergmann
2017-11-29 11:39 ` Greentime Hu
2017-11-29 11:57 ` Arnd Bergmann
2017-11-29 14:10 ` Greentime Hu
2017-11-29 20:27 ` Arnd Bergmann
2017-11-30 5:48 ` Greentime Hu
2017-11-30 7:52 ` Geert Uytterhoeven
2017-11-30 9:29 ` Greentime Hu
[not found] ` <CAEbi=3cTkbt9i7XPXMnY1D6qtbebDW1x8sFVsgqhq-nApAx5mA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-30 9:30 ` Arnd Bergmann
2017-11-30 10:01 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 26/35] dt-bindings: interrupt-controller: Andestech Internal Vector Interrupt Controller Greentime Hu
2017-11-28 14:05 ` Rob Herring
2017-11-27 12:28 ` [PATCH v2 27/35] irqchip: Andestech Internal Vector Interrupt Controller driver Greentime Hu
[not found] ` <e82831165cd9e45a7d03af9c870560a6384e1603.1511785528.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-28 9:37 ` Marc Zyngier
[not found] ` <c7447c93-9905-2840-e2d8-01837b9fdecd-5wv7dgnIgG8@public.gmane.org>
2017-11-29 15:23 ` Greentime Hu [this message]
2017-11-30 10:57 ` Marc Zyngier
2017-11-27 12:28 ` [PATCH v2 28/35] MAINTAINERS: Add nds32 Greentime Hu
2017-11-27 12:28 ` [PATCH v2 29/35] dt-bindings: nds32 CPU Bindings Greentime Hu
2017-11-27 13:42 ` Mark Rutland
[not found] ` <20171127134232.q343uymer47zt74m-agMKViyK24J5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org>
2017-11-28 3:18 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 30/35] net: faraday add nds32 support Greentime Hu
2017-11-27 14:15 ` Arnd Bergmann
[not found] ` <CAK8P3a2GJERt78uWgdDy+Azr-ZMcOcB+D6Akq99tSfwKmt2LiA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-28 2:55 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 31/35] earlycon: add reg-offset to physical address before mapping Greentime Hu
2017-11-28 14:25 ` Greg KH
2017-11-29 5:40 ` Greentime Hu
2017-11-27 12:28 ` [PATCH v2 32/35] asm-generic/io.h: move ioremap_nocache/ioremap_uc/ioremap_wc/ioremap_wt out of ifndef CONFIG_MMU Greentime Hu
2017-11-27 14:14 ` Arnd Bergmann
2017-11-27 12:28 ` [PATCH v2 33/35] clocksource/drivers/atcpit100: Add andestech atcpit100 timer Greentime Hu
2017-12-01 12:30 ` Linus Walleij
2017-12-07 8:44 ` Daniel Lezcano
2017-11-27 12:28 ` [PATCH v2 34/35] clocksource/drivers/Kconfig: Support " Greentime Hu
2017-11-27 14:11 ` Arnd Bergmann
[not found] ` <CAK8P3a2ovDuwWCq2HABZaCVGO04TX0VdgnQbK65RvHhsMEzsiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-28 2:53 ` Greentime Hu
2017-12-07 8:40 ` Daniel Lezcano
[not found] ` <1a22db002413ff60851737736a86b40c38877220.1511785528.git.green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-07 8:39 ` Daniel Lezcano
2017-11-27 12:28 ` [PATCH v2 35/35] dt-bindings: timer: Add andestech atcpit100 timer binding doc Greentime Hu
2017-12-01 12:19 ` Linus Walleij
2017-12-04 1:07 ` 陳建志
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='CAEbi=3eXP1cfNDhu9YjZbjPghgxm9xmEtjEBO=KVOnWZyPqVbw@mail.gmail.com' \
--to=green.hu-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=arnd-r2nGTMty4D4@public.gmane.org \
--cc=daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=deanbo422-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=greentime-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org \
--cc=jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org \
--cc=linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=marc.zyngier-5wv7dgnIgG8@public.gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=rick-MUIXKm3Oiri1Z/+hSey0Gg@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
--cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
--cc=will.deacon-5wv7dgnIgG8@public.gmane.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).