From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web10.518.1585774343884348837 for ; Wed, 01 Apr 2020 13:52:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=Vsuf0R2z; spf=pass (domain: nuviainc.com, ip: 209.85.128.66, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f66.google.com with SMTP id c195so4406756wme.1 for ; Wed, 01 Apr 2020 13:52:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=IL8sn5iQP4AqlnJA8eIzkcT5/MLNNKb2OsIE8OlIyW4=; b=Vsuf0R2zqOiSCUjn5AgiE6NMlu4I/oT4rhzjdG3PF1l5tzknZp/j67LSKHOBkNNORn ih+bNXimkNy0axRIR78wptcfZskcoA+tuw8u2QHczkH3OxO6inuerTmr8Kasj5MA5pnd OubmQ/vEuWeJDivrD61FjJtsI3MHhqhPtudMCeRlyOBnQdq/EL3gaH6EYvX1Ax+pIdDj 8OV569ib/waizKS+CHch5EbLWe3w13K3vW0TbI6cIkvRTO0qkUGeA0au8rLI1v8ILU0P DXcOW8RSN8jee8AW5BHD5ySYCUwu5nM0TH8WUlcVyXM+dTyRgpb7xF5s4gWbRVpfsMLP 7ojw== 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:content-transfer-encoding :in-reply-to:user-agent; bh=IL8sn5iQP4AqlnJA8eIzkcT5/MLNNKb2OsIE8OlIyW4=; b=Fh+eSzmF8z7z4L9/MW0LDcFDpDPNiM8FZ/9GuwFxI+iQU9O6C+5Wqj/FC/nweA+tLH wXbZLTII0gCXQ8LA24msqLr8BkUrz5BJTNtKCmJPFjyUFSg+o9Gw13paeryaao7FKxfL +6MxxX+YEpuYweQXMs6Gt4JZSEhEA0eE2jCkHuj27t8Hx07rPv+fTLJkgis3zfGHsbXz WPbrx51eaYwEF4kyYeVMQiuuTNtQMAIDyHbVmLE/wecLTHNNBS1CEQCeEgvTKfsgUTvN 4oYp7o3R7DpKC3L4sbjmVxhpgI1m/zDyYVNANlRyBIl3Yq9w2ZydU2gud4feQ93Mm72Z YCLg== X-Gm-Message-State: AGi0PubTzPhtkG3t4f4LTmVE6IrEADievDSzx5sqQdFg/bwyjk/7k9p8 HCVY4xhQJD/hfdUeTPKsYEoGVg== X-Google-Smtp-Source: APiQypI12xwn262Bfnlz/s1qhu8NYweeFY5B7GZ/Vu7p57thOkimdJlV5zMSqKx/wbODfO0sXQ3DGw== X-Received: by 2002:a1c:195:: with SMTP id 143mr279628wmb.0.1585774342340; Wed, 01 Apr 2020 13:52:22 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id p10sm4398309wrm.6.2020.04.01.13.52.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2020 13:52:21 -0700 (PDT) Date: Wed, 1 Apr 2020 21:52:19 +0100 From: "Leif Lindholm" To: Pankaj Bansal Cc: Meenakshi Aggarwal , Michael D Kinney , devel@edk2.groups.io, Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton Subject: Re: [PATCH v2 00/28] Add PEI phase to LS1043ARDB Platform Message-ID: <20200401205219.GK7468@vanye> References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> MIME-Version: 1.0 In-Reply-To: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit Hi Pankaj, I have now finished my review of individual patches. Beyond that, the build fails at the patch 10/28 "Silicon/NXP: remove print information from Soc lib" with the error edk2-platforms/Silicon/NXP/Library/SocLib/Chassis.c:209:1: error: ‘CpuMaskNext’ defined but not used [-Werror=unused-function] The function is deleted in the subsequent patch, 11/28 "Silicon/NXP: remove not needed components". Please move that deletion to the now failing patch. Regards, Leif On Fri, Mar 20, 2020 at 20:05:15 +0530, Pankaj Bansal wrote: > From: Pankaj Bansal > > This patch series adds PEI phase to NXP LS1043ARDB Platform. > The previous attempt at this feature can be referred here: > https://edk2.groups.io/g/devel/message/54006 > > I have taken care of the review comments received on v1 and have > broken down the patches further to make review easier. > > That is why the number of patches have increased from 19 in v1 to > 28 in v2. > > As such the v1 and v2 patches have diverged, which is why i am not > putting version specific changes in each indivisual patch. > > i have created v2 series in a way that the changes feel more organic > and not abrupt. > Only the patch "12/28 remove not needed components" would seem too > invasive. But, as i have noted in patch description, i am not removing > anything which is needed for booting LS1043ARDB as of now. i have done > this to keep the code simple and introduce the components as and when > needed for new features. This makes code review simpler too. > > Pankaj Bansal (28): > Silicon/NXP: Add I2c lib > Silicon/NXP: changes to use I2clib in i2cdxe > Silicon/NXP/I2cDxe: Fix I2c Timeout with RTC > Silicon/Maxim: Fix bug in RtcWrite in Ds1307RtcLib > Silicon/Maxim: Add comments in Ds1307RtcLib > NXP/LS1043aRdb: Move Soc specific components to soc files > Silicon/NXP: Implement SerialUartClockLib > Silicon/NXP/LS1043A: Use BaseSerialPortLib16550 as SerialPortLib > Silicon/NXP: Drop DUartPortLib > Silicon/NXP: remove print information from Soc lib > Silicon/NXP: remove not needed components > Silicon/NXP: Remove unnecessary PCDs > Silicon/NXP: Move dsc file > Platform/NXP: rename the ArmPlatformLib as per ArmPlatformPkg > Silicon/NXP: Move RAM retrieval from SocLib > Platform/NXP/LS1043aRdbPkg: Add Clock retrieval APIs > Silicon/NXP: Use Clock retrieval PPI in modules > Silicon/NXP: Add Chassis2 Package > Silicon/NXP/LS1043A: Use ChassisLib from Chassis2 Pkg > Silicon/NXP/LS1043A: Move SocLib to Soc Package > Slicon/NXP: Add PlatformPei Lib > NXP/LS1043aRdbPkg/ArmPlatformLib: Use default ArmPlatformHelper.S > NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool > NXP/LS1043aRdbPkg/ArmPlatformLib: Remove extern SocInit > Platform/NXP: Modify FV rules > Platform/NXP/LS1043aRdbPkg: Add VarStore > Silicon/NXP: move MemoryInitPeiLib as per PEIM structures > Platform/NXP/LS1043aRdbPkg: Add PEI Phase > > Platform/NXP/FVRules.fdf.inc | 59 +- > .../Drivers/PlatformDxe/PlatformDxe.c | 15 +- > .../Drivers/PlatformDxe/PlatformDxe.inf | 11 +- > Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 26 +- > Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 21 +- > .../AArch64/ArmPlatformHelper.S | 45 ++ > .../ArmPlatformLib.c | 61 +- > .../Library/ArmPlatformLib/ArmPlatformLib.inf | 42 ++ > .../ArmPlatformLibMem.c} | 84 ++- > .../Library/PlatformLib/ArmPlatformLib.inf | 55 -- > .../Library/PlatformLib/NxpQoriqLsHelper.S | 31 - > Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc | 91 +++ > .../Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.c | 23 +- > Silicon/NXP/Chassis2/Chassis2.dec | 23 + > Silicon/NXP/Chassis2/Chassis2.dsc.inc | 10 + > Silicon/NXP/Chassis2/Include/Chassis.h | 34 ++ > .../Chassis2/Library/ChassisLib/ChassisLib.c | 97 +++ > .../Library/ChassisLib/ChassisLib.inf | 34 ++ > Silicon/NXP/Drivers/I2cDxe/I2cDxe.c | 533 +--------------- > Silicon/NXP/Drivers/I2cDxe/I2cDxe.h | 50 +- > Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf | 14 +- > Silicon/NXP/Include/Chassis2/LsSerDes.h | 62 -- > Silicon/NXP/Include/Chassis2/NxpSoc.h | 361 ----------- > Silicon/NXP/Include/DramInfo.h | 38 -- > Silicon/NXP/Include/Library/ChassisLib.h | 51 ++ > Silicon/NXP/Include/Library/I2cLib.h | 120 ++++ > Silicon/NXP/Include/Library/SocLib.h | 52 ++ > Silicon/NXP/Include/Ppi/NxpPlatformGetClock.h | 53 ++ > Silicon/NXP/LS1043A/Include/Soc.h | 55 ++ > Silicon/NXP/LS1043A/Include/SocSerDes.h | 51 -- > Silicon/NXP/LS1043A/LS1043A.dsc.inc | 51 +- > Silicon/NXP/LS1043A/Library/SocLib/SocLib.c | 77 +++ > Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf | 27 + > Silicon/NXP/Library/DUartPortLib/DUart.h | 122 ---- > .../NXP/Library/DUartPortLib/DUartPortLib.c | 364 ----------- > .../NXP/Library/DUartPortLib/DUartPortLib.inf | 34 -- > Silicon/NXP/Library/I2cLib/I2cLib.c | 576 ++++++++++++++++++ > Silicon/NXP/Library/I2cLib/I2cLib.inf | 31 + > Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 105 ++++ > .../Library/MemoryInitPei/MemoryInitPeiLib.c | 140 ----- > .../MemoryInitPeiLib/MemoryInitPeiLib.c | 224 +++++++ > .../MemoryInitPeiLib/MemoryInitPeiLib.h | 25 + > .../MemoryInitPeiLib.inf | 10 +- > .../Library/PlatformPeiLib/PlatformPeiLib.c | 30 + > .../Library/PlatformPeiLib/PlatformPeiLib.inf | 41 ++ > .../SerialUartClockLib/SerialUartClockLib.c | 22 + > .../SerialUartClockLib/SerialUartClockLib.inf | 26 + > Silicon/NXP/Library/SocLib/Chassis.c | 495 --------------- > Silicon/NXP/Library/SocLib/Chassis2/Soc.c | 162 ----- > Silicon/NXP/Library/SocLib/LS1043aSocLib.inf | 45 -- > Silicon/NXP/Library/SocLib/NxpChassis.h | 136 ----- > Silicon/NXP/Library/SocLib/SerDes.c | 268 -------- > Silicon/NXP/NxpQoriqLs.dec | 95 +-- > {Platform => Silicon}/NXP/NxpQoriqLs.dsc.inc | 74 ++- > 54 files changed, 2181 insertions(+), 3201 deletions(-) > create mode 100644 Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S > rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib => ArmPlatformLib}/ArmPlatformLib.c (51%) > create mode 100644 Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf > rename Platform/NXP/LS1043aRdbPkg/Library/{PlatformLib/NxpQoriqLsMem.c => ArmPlatformLib/ArmPlatformLibMem.c} (51%) > delete mode 100644 Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf > delete mode 100644 Platform/NXP/LS1043aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S > create mode 100644 Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc > create mode 100644 Silicon/NXP/Chassis2/Chassis2.dec > create mode 100644 Silicon/NXP/Chassis2/Chassis2.dsc.inc > create mode 100644 Silicon/NXP/Chassis2/Include/Chassis.h > create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.c > create mode 100644 Silicon/NXP/Chassis2/Library/ChassisLib/ChassisLib.inf > delete mode 100644 Silicon/NXP/Include/Chassis2/LsSerDes.h > delete mode 100644 Silicon/NXP/Include/Chassis2/NxpSoc.h > delete mode 100644 Silicon/NXP/Include/DramInfo.h > create mode 100644 Silicon/NXP/Include/Library/ChassisLib.h > create mode 100644 Silicon/NXP/Include/Library/I2cLib.h > create mode 100644 Silicon/NXP/Include/Library/SocLib.h > create mode 100644 Silicon/NXP/Include/Ppi/NxpPlatformGetClock.h > create mode 100644 Silicon/NXP/LS1043A/Include/Soc.h > delete mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h > create mode 100644 Silicon/NXP/LS1043A/Library/SocLib/SocLib.c > create mode 100644 Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUart.h > delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUartPortLib.c > delete mode 100644 Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.c > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLib.inf > create mode 100644 Silicon/NXP/Library/I2cLib/I2cLibInternal.h > delete mode 100644 Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c > create mode 100644 Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c > create mode 100644 Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h > rename Silicon/NXP/Library/{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.inf (74%) > create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c > create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf > create mode 100644 Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.c > create mode 100644 Silicon/NXP/Library/SerialUartClockLib/SerialUartClockLib.inf > delete mode 100644 Silicon/NXP/Library/SocLib/Chassis.c > delete mode 100644 Silicon/NXP/Library/SocLib/Chassis2/Soc.c > delete mode 100644 Silicon/NXP/Library/SocLib/LS1043aSocLib.inf > delete mode 100644 Silicon/NXP/Library/SocLib/NxpChassis.h > delete mode 100644 Silicon/NXP/Library/SocLib/SerDes.c > rename {Platform => Silicon}/NXP/NxpQoriqLs.dsc.inc (84%) > > -- > 2.17.1 >