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:c09::243; helo=mail-wm0-x243.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) (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 575E4203B8CF9 for ; Thu, 31 May 2018 14:01:55 -0700 (PDT) Received: by mail-wm0-x243.google.com with SMTP id 18-v6so50785677wml.2 for ; Thu, 31 May 2018 14:01:55 -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=1XJ0xskcFPzgbUT9EY0NLQSXZoYitWIIqDpbGojb9PE=; b=kmwBLAcvcxddYSCwXSVhQyMM/y90tZTwIF730I6rm1mrYK6NTO3kJkxJ+WudzKgRLo Qi1BP+QIuvL58JCoLqDX7odhNKhqGxfZJi78SHrcmUtbcAL5OSInyteN+lVFj36aiua/ FXz+2PwnoFpyMF9PLaRBF6x33eWSXLT2AeBDo= 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=1XJ0xskcFPzgbUT9EY0NLQSXZoYitWIIqDpbGojb9PE=; b=XCgbSR7y/r4+U2/h+hGJUF8CC99VPOb2vavxbvCB4u2qUHogx0szn9HoxFnN8grO/p Ay7eTk4Bu4DfJ+Q7gOUVNfUZD5cimivcC9xwAZJEFf6fMw/2dSiAcQofr2l8heewd0Z4 sUrQYb7FursK3WlrcNaHOuYiByuvNQHObg4+SvCej722EbTne79Uu5m+5deZjOoGm8gM V3UXrwF5kcFCWjJFtA/Q5EAne7X5xSJpO9SgTCpzVb/pNVjBmQgAkTajYCvDlzsjo2kO mYSFst7P3M8DGdyhzDhQp8O/99z/0pf3Bxhx7uNvxvXSdwhvUieB47jPkSlMR1GzMLeI EQ0g== X-Gm-Message-State: APt69E1HE+vSpe42vleKvktOq18uo6qnE56cBxtQa9Lo81N3n3xiZAyc 63ysrgLORaq2NkuzHeryvkPbGQ== X-Google-Smtp-Source: ADUXVKIf47GCTd0W+BzH65ut3qYDquBXkhqmH3oW6gT/xhGO3iYniiYqeZSxzoLyLrwbA+ROjnqb+g== X-Received: by 2002:a1c:aa0c:: with SMTP id t12-v6mr925237wme.56.1527800513826; Thu, 31 May 2018 14:01:53 -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 o138-v6sm424871wmg.10.2018.05.31.14.01.52 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 31 May 2018 14:01:52 -0700 (PDT) Date: Thu, 31 May 2018 22:01:51 +0100 From: Leif Lindholm To: Haojian Zhuang Cc: edk2-devel@lists.01.org, Ard Biesheuvel Message-ID: <20180531210151.wsix7pnnd7tqwn7u@bivouac.eciton.net> References: <1527736210-17133-1-git-send-email-haojian.zhuang@linaro.org> <1527736210-17133-3-git-send-email-haojian.zhuang@linaro.org> MIME-Version: 1.0 In-Reply-To: <1527736210-17133-3-git-send-email-haojian.zhuang@linaro.org> 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:01:55 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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