Re: [EXT] Re: [PATCH v2 07/28] Silicon/NXP: Implement SerialUartClockLib

[Varun Sethi <V.Sethi@nxp.com>]

Hi Leif,
Please find my comments inline.

Regards
Varun

-----Original Message-----
From: Leif Lindholm <leif@nuviainc.com> 
Sent: Monday, April 6, 2020 4:55 PM
To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Michael D Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io; Varun Sethi <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Jon Nettleton <jon@solid-run.com>
Subject: [EXT] Re: [PATCH v2 07/28] Silicon/NXP: Implement SerialUartClockLib

Caution: EXT Email

On Mon, Apr 06, 2020 at 06:33:40 +0000, Pankaj Bansal (OSS) wrote:
>
>
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: Wednesday, April 1, 2020 3:23 PM
> > To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Michael D 
> > Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io; Varun 
> > Sethi <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj- 
> > Mahmoud@arm.com>; Jon Nettleton <jon@solid-run.com>
> > Subject: Re: [PATCH v2 07/28] Silicon/NXP: Implement 
> > SerialUartClockLib
> >
> > On Fri, Mar 20, 2020 at 20:05:22 +0530, Pankaj Bansal wrote:
> > > From: Pankaj Bansal <pankaj.bansal@nxp.com>
> > >
> > > Implement SerialUartClockLib for all NXP Layerscape Platforms.
> > >
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > ---
> > >  .../SerialUartClockLib/SerialUartClockLib.c   | 27 +++++++++++++++++++
> > >  .../SerialUartClockLib/SerialUartClockLib.inf | 26 
> > > ++++++++++++++++++
> >
> > I requested after the initial submission that you "either follow the 
> > manual git setup steps from
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > thub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FLaszlo%2527s-unk
> > empt-&amp;data=02%7C01%7CV.Sethi%40nxp.com%7C147ef25a5d6e4a0c899408d
> > 7da1d2288%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6372176909755
> > 41422&amp;sdata=6A%2Bg1jAmhpu9p%2FISIddejL0dhzQs%2FS%2BqbYCXtgPAUfk%
> > 3D&amp;reserved=0 git-guide-for-edk2-contributors-and-maintainers
> > or execute edk2/BaseTools/Scripts/SetupGit.py in each of the 
> > tianocore repositories"
> >
> > Now, neither appears to have happened, as .c still comes before .inf 
> > and paths are still truncated.
>
> I had executed the SetupGit.py script, I will check again.
>
> >
> > But in addition to that, we realised that git happily ignores 
> > settings for --stat. So, please, execute aforementioned script, but 
> > then generate v3 with --stat=1000 --stat-graph-width=20.
> >
>
> Ok. I will follow these steps as well.
>
> > For this particular patch, the side discussion on the edk2 portion 
> > may make it redundant, so I'm deferring review.
>
> Right now, I am yet to work on the review comments of other edk2 patch 
> and since it touches other platforms as well, it requires thorough 
> vetting from other platform maintainers. It seems like that would take 
> more time. If I send this patch series without removing DuartLib (i.e. 
> drop patches 7, 8, 9 from this series), can you please merge. 
> Parallelly, I will work on edk2 patch and once that is accepted, I will send patches 7, 8, 9 after that in new series ?

>See comment in my previous email on merging code only to remove it shortly afterwards.

We will certainly take of this going forward.

>As you have seen on that thread, a couple of proposals have been made for how to fix this without affecting other platforms (the HOB variant[1] and the separate .inf[2] variant). I will keep chasing on that >occasionally, but please also contribute to that conversation yourself.

Pankaj would be responding to this thread and make changes accordingly. Please allow us to pursue this patchset as a separate thread. We need the base patches to be merged asap so that we can submit patches for LX2160A and LS1046A. We have urgent releases planned for these platforms.