From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::241; helo=mail-wr0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EACD720965DD4 for ; Thu, 31 May 2018 14:03:29 -0700 (PDT) Received: by mail-wr0-x241.google.com with SMTP id i14-v6so34277216wre.2 for ; Thu, 31 May 2018 14:03:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=NUpWyiKJFXlHOGWmjv+k2Q6vBTPRq9RPyoX/wv8ZhTU=; b=M6HsjOT7Fe4mb240qABoyo3dgTNPGTHTwCRxzXMjeuR8h0bIEIXys9LsoL6HtEgENP 5d9WplkhC7iA+th14dNHLLw/L8ffwmVKrcbBgmGkRR+r9y4gmHjDsuedVbAlcyC53ub0 sNQ8LCFYenLLLyeLMkOBfIgT5NxsMydqoeL40= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=NUpWyiKJFXlHOGWmjv+k2Q6vBTPRq9RPyoX/wv8ZhTU=; b=gvvhqUoElFmU9vNmBlKMBeOnxKN6RjWE98MTWe68HJCRrikaI+0zL3gonUKtJBGCi8 9ErkZi1/Wytn+QFzBM//DuEFpgSz4ec36r4OHurdbdxpeL/940Ft3hxMlmnglX3/qwxR PRkgGCoW42zzXB+OqZv1lXwewh1zAq0cgZpazYuCP3SYjbkxeTTNEbnY7/TZ6gloMwH2 CRnO68DIERTqEpcyhJMK8DgzE4mMW/yLsDP/x7FyA4HZaGvK6fJ0b25CTGmFVoDOFnh6 Eyp8fH9Qg4F79nfI8l6UwszXgBIXrkBVLQdxXHxFG56lsnffudEqrBnE2Xfzwiy4PSjQ TtBw== X-Gm-Message-State: ALKqPweBERgmerjgmUOrwdJ1yW7rjpxCNhdpbE4ZJ3e+X+NradDCPt2Z zj0/JRQoYaJhMm/MtzhHlclsEA== X-Google-Smtp-Source: ADUXVKL9yU7fCluC02lsuoXxqHbn8X1Chequr1xJHOhhpgHv59ue4PvPMEIm94FFXHeePqP7HdkNiA== X-Received: by 2002:adf:9341:: with SMTP id 59-v6mr2473173wro.80.1527800608523; Thu, 31 May 2018 14:03:28 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id i26-v6sm367145wmb.19.2018.05.31.14.03.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 31 May 2018 14:03:27 -0700 (PDT) Date: Thu, 31 May 2018 22:03:26 +0100 From: Leif Lindholm To: Haojian Zhuang Cc: edk2-devel@lists.01.org, Ard Biesheuvel Message-ID: <20180531210325.uabqe54rqqosqthj@bivouac.eciton.net> References: <1527736210-17133-1-git-send-email-haojian.zhuang@linaro.org> <1527736210-17133-3-git-send-email-haojian.zhuang@linaro.org> <20180531210151.wsix7pnnd7tqwn7u@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: <20180531210151.wsix7pnnd7tqwn7u@bivouac.eciton.net> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH v7 edk-platforms 2/6] Platform/HiKey960: do basic initialization X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 May 2018 21:03:30 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Oh, and please resubmit only this patch. The rest are good to go, but I'll bring them in together. / Leif On Thu, May 31, 2018 at 10:01:51PM +0100, Leif Lindholm wrote: > This fixes the headers, but ignores the most important bit of feedback > on v6: > > On Thu, May 31, 2018 at 11:10:06AM +0800, Haojian Zhuang wrote: > > Do some basic initliazation on peripherals, such as pins and > > regulators. > > > > The hardcoding code is taken from non-open reference code. > > Can't fix it for lack of documents. > > > > Cc: Leif Lindholm > > Cc: Ard Biesheuvel > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Haojian Zhuang > > --- > > > diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c > > new file mode 100644 > > index 000000000000..e88bad2167c6 > > --- /dev/null > > +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c > ... > > +/** > > + Notification function of the event defined as belonging to the > > + EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in > > + the entry point of the driver. > > + > > + This function is called when an event belonging to the > > + EFI_END_OF_DXE_EVENT_GROUP_GUID event group is signalled. Such an > > + event is signalled once at the end of the dispatching of all > > + drivers (end of the so called DXE phase). > > + > > + @param[in] Event Event declared in the entry point of the driver whose > > + notification function is being invoked. > > + @param[in] Context NULL > > +**/ > > +STATIC > > +VOID > > +OnEndOfDxe ( > > + IN EFI_EVENT Event, > > + IN VOID *Context > > + ) > > +{ > > + UINT32 BootMode; > > + CHAR8 Buf[64]; > > + UINTN Count; > > + > > + BootMode = MmioRead32 (SCTRL_BAK_DATA0) & BOOT_MODE_MASK; > > + if (BootMode == BOOT_MODE_RECOVERY) { > > + Count = AsciiSPrint ( > > + Buf, > > + 64, > > + "WARNING: CAN NOT BOOT KERNEL IN RECOVERY MODE!\n" > > + ); > > + SerialPortWrite ((UINT8 *)Buf, Count); > > + Count = AsciiSPrint ( > > + Buf, > > + 64, > > + "Switch to normal boot mode, then reboot to boot kernel.\n" > > + ); > > I gave the following feedback on v6, which has not been addressed at all: > --- > I asked for the hand-coded values to be removed, and instead they have > been replaced by a static buffer which may or may not be able to hold > updated versions of the stringsm and require changes in three places > if that happens. This is not an improvement. > > The length or size of a compile time constant should always be > described through StrLen/AsciiStrLen, or where possible sizeof. > > In this instance, I would suggest doing something like: > > VOID *RecoveryString; > VOID *SwitchString; > > RecoveryString = "WARNING: CAN NOT BOOT KERNEL IN RECOVERY MODE!\n" > SwitchString = "Switch to normal boot mode, then reboot to boot > kernel.\n"; > > You can then use SerialPortWrite (RecoveryString, > AsciiStrLen(*RecoveryString)). > > I think we should also in future look into how we can get the console > working before Dxe. Both HiKey boards are generally intended to be > used with displays, so these messages will be invisible to many users. > --- > > > + SerialPortWrite ((UINT8 *)Buf, Count); > > + } > > +} > > / > Leif