linux-kernel - Re: [PATCH] ARM: socfpga: dts: add fpga manager

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1510132142250.2247@atull-730U3E-740U3E>
Date:	Tue, 13 Oct 2015 21:46:15 +0200
From:	atull <atull@...nsource.altera.com>
To:	Dinh Nguyen <dinguyen@...nsource.altera.com>
CC:	Steffen Trumtrar <s.trumtrar@...gutronix.de>,
	Mark Rutland <mark.rutland@....com>,
	<devicetree@...r.kernel.org>, Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	<linux-kernel@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>,
	Kumar Gala <galak@...eaurora.org>,
	<delicious.quinoa@...il.com>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ARM: socfpga: dts: add fpga manager

On Tue, 13 Oct 2015, Dinh Nguyen wrote:

> On Tue, 13 Oct 2015, Steffen Trumtrar wrote:
> 
> > Hi Alan!
> > 
> > On Tue, Oct 13, 2015 at 01:28:20PM -0500, atull@...nsource.altera.com wrote:
> > > From: Alan Tull <atull@...nsource.altera.com>
> > > 
> > > Add FPGA manager to device tree for SoCFPGA.
> > > 
> > > Signed-off-by: Alan Tull <atull@...nsource.altera.com>
> > > ---
> > >  arch/arm/boot/dts/socfpga.dtsi | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> > > index 314e589..e37ba11 100644
> > > --- a/arch/arm/boot/dts/socfpga.dtsi
> > > +++ b/arch/arm/boot/dts/socfpga.dtsi
> > > @@ -834,5 +834,12 @@
> > >  			compatible = "altr,sys-mgr", "syscon";
> > >  			reg = <0xffd08000 0x4000>;
> > >  		};
> > > +
> > > +		hps_0_fpgamgr: fpgamgr@...f706000 {
> > > +			compatible = "altr,socfpga-fpga-mgr";
> > > +			reg = <0xFF706000 0x1000
> > > +			       0xFFB90000 0x1000>;
> > > +			interrupts = <0 175 4>;
> > > +		};
> > >  	};
> > >  };
> > 
> > Please keep the format in line with the rest of the file:
> > Hexvalues are lowercase and no 0x after the "fpgamgr@".
> > I'd say "And please put the node in its proper position
> > according to its address", but I see just now, that the
> > dtsi is all messed up right now anyway :-(
> > 

Hi Steffen,

Thanks for the review.  I'll fix these things you pointed out.

"all messed up" seems kind of unclear and overstated here.  From the 
context, I assume you mean that you are pointing out that the nodes
are in more or less alphatetical order and not in order of address.  Is 
that what you mean or is the dtsi really all messed up?

Alan

> 
> How so? Patches are welcomed!
> 
> Please fix the "fpgamgr@" and lower case the hex values. But I would prefer the
> nodes to be in alphabetical order.
> 
> Also, you need to document the "altr,socfpga-fpga-mgr".
> 
> Thanks,
> Dinh
> > Regards,
> > Steffen
> > 
> > -- 
> > Pengutronix e.K.                           |                             |
> > Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> > 
> 
> BR,
> Dinh
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/