linux-kernel - Re: [PATCH v4 4/6] irqchip: irq-mvebu-icu: new driver for Marvell ICU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170621153042.05237000@windsurf.home>
Date:   Wed, 21 Jun 2017 15:30:42 +0200
From:   Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Ian Campbell <ijc+devicetree@...lion.org.uk>,
        Pawel Moll <pawel.moll@....com>,
        Mark Rutland <mark.rutland@....com>,
        Kumar Gala <galak@...eaurora.org>,
        Andrew Lunn <andrew@...n.ch>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Gregory Clement <gregory.clement@...e-electrons.com>,
        linux-arm-kernel@...ts.infradead.org,
        Nadav Haklai <nadavh@...vell.com>,
        Hanna Hawa <hannah@...vell.com>,
        Yehuda Yitschak <yehuday@...vell.com>,
        Antoine Tenart <antoine.tenart@...e-electrons.com>,
        Miquèl Raynal 
        <miquel.raynal@...e-electrons.com>
Subject: Re: [PATCH v4 4/6] irqchip: irq-mvebu-icu: new driver for Marvell
 ICU

Hello,

On Wed, 21 Jun 2017 08:56:24 +0100, Marc Zyngier wrote:

> > +static void mvebu_icu_write_msg(struct msi_desc *desc, struct msi_msg *msg)
> > +{
> > +	struct irq_data *d = irq_get_irq_data(desc->irq);
> > +	struct mvebu_icu_irq_data *icu_irqd = d->chip_data;
> > +	struct mvebu_icu *icu = icu_irqd->icu;
> > +	unsigned int icu_int;
> > +
> > +	if (msg->address_lo) {  
> 
> As mention in the previous thread, this needs to check address_hi as well.

Fixed in v5.

> > +	gicp_dn = of_parse_phandle(node, "msi-parent", 0);
> > +	if (!gicp_dn) {
> > +		dev_err(&pdev->dev, "Missing marvell,gicp property.\n");  
> 
> The property name doesn't match here. But overall, there is little point
> in reparsing the msi-parent, since that's what of_msi_get_domain does.
> Instead, consider using:
> 
> 	gicp_dn = irq_domain_get_of_node(pdev->dev.msi_domain);
> 
> which will do the right thing.

Fixed in v5.


> With the two minor changes above, and for patch #3 as well:
> 
> Reviewed-by: Marc Zyngier <marc.zyngier@....com>

Thanks! Tags added in v5.

> I can either perform the changes when applying the patches, or let you
> do it so that you can test it first (and solve the issue with the DT
> include fine with Gregory).

I've sent a v5 that fixes the different issues you pointed out, and
also fixes the DT include problem. Tested on HW, it still works as
expected.

Thanks again for the review!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com