From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QW5kcmVhcyBCaWXDn21hbm4=?= Date: Mon, 18 Mar 2013 12:01:54 +0100 Subject: [U-Boot] [PATCH] arm: at91: at91sam9n12ek: add nandflash/spiflash/mmc/lcd support In-Reply-To: <5146E513.1010009@atmel.com> References: <1363342624-2939-1-git-send-email-josh.wu@atmel.com> <20130318065821.566A820058D@gemini.denx.de> <5146E513.1010009@atmel.com> Message-ID: <5146F422.8000309@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Josh Wu, On 03/18/2013 10:57 AM, Josh Wu wrote: > Dear Wolfgang Denk > > Thanks for your review. See my comment below: > > On 3/18/2013 2:58 PM, Wolfgang Denk wrote: >> Dear Josh Wu, >> >> In message <1363342624-2939-1-git-send-email-josh.wu@atmel.com> you >> wrote: >>> This patch adds at91sam9n12ek support, it enables: >>> - dbgu >>> - nand with pmecc >>> - spi flash >>> - mmc >>> - lcd >> It appears you are adding support for a new board - in this case the >> Subject: is misleading, plrase fix. >> >> The missing entry in MAINTAINERS has already been pointed out. Please >> also make sure to run your patch through checkpatch - it complains for >> example "WARNING: line over 80 characters". Please fix these, too. >> >> Please also make sure to keep all lists sorted. Bo shen already >> mentioned this for the Makefile, but this also applies to lists as >> here: >> >> #elif defined(CONFIG_AT91SAM9G45) || >> defined(CONFIG_AT91SAM9M10G45) \ >> - || defined(CONFIG_AT91SAM9X5) >> + || defined(CONFIG_AT91SAM9X5) || defined(CONFIG_AT91SAM9N12) >> > > I'll fix them in next version. Can we please find another identifier instead? I will not have these always growing list of SoC defines to distinguish between different access modes. Can we find some common thing which all these SoC headers export then? I mean something like 'AT91_CLOCK_IP_V2' in this case here. If we are looking forward to a single u-boot binary for different boards I would favor some runtime detection. Beside that I will do a full review of the patch later this day. Best regards Andreas Bie?mann