On Thu, Aug 02, 2018 at 04:13:26PM -0700, Atish Patra wrote: >> + * which device 0 is defined as non-existant by the RISC-V Priviledged Spec. > /s/existant/existent > > /s/Priviledged/Privileged >> + * Each hart context has a vector of interupt enable bits associated with it. > /s/interupt/interrupt All fixed. >> + WARN_ON_ONCE(!handler->present); >> + >> + csr_clear(sie, SIE_STIE); > > We should clear the SIE_SEIE not SIE_STIE. Correct ? Yes, fixed. >> + if (plic_regs) { >> + pr_warning("PLIC already present.\n"); > > Got a check-patch warning. > > WARNING: Prefer pr_warn(... to pr_warning(... > #257: FILE: drivers/irqchip/irq-sifive-plic.c:191: > + pr_warning("PLIC already present.\n"); Fixed. >> + >> + if (of_irq_parse_one(node, i, &parent)) { >> + pr_err("failed to parse parent for contxt %d.\n", i); > /s/contxt/context Fixed. >> + continue; >> + } >> + >> + /* skip context holes */ >> + if (parent.args[0] == -1) >> + continue; >> + >> + cpu = plic_find_hart_id(parent.np->parent); > > Since plic_find_hart_id() is going to walk up the DT, we can pass only > parent.np instead of parent.np->parent. It does not increase any efficiency > either way. So not very necessary. Just thought of taking the advantage of > plic_find_hart_id. Yeah, I'll update this. Thanks for the review! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html