Re: [edk2-platforms][PATCH 1/8] Platform/RPi: Add model family detection

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Mon, Nov 18, 2019 at 05:58:05PM +0000, Pete Batard wrote:
> On 2019.11.18 17:51, Leif Lindholm wrote:
> > On Thu, Nov 14, 2019 at 04:07:33PM +0000, Pete Batard wrote:
> > > From: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
> > > 
> > > Add GetModelFamily to RASPBERRY_PI_FIRMWARE_PROTOCOL.
> > > 
> > > This uses the board revision to return a numeric value representing
> > > the RPi family (1=RPi, 2=RPi2, 3=RPi3 and 4=RPi4).
> > > 
> > > Knowing the Pi family will help us set the SD card routing when we
> > > introduce support for the Pi 4 and should also be easier to maintain
> > > than if using individual model detection.
> > > 
> > > Also add a missing entry for the "Raspberry Pi Compute Module 3+" in
> > > RpiFirmwareGetModelName ().
> > 
> > Can you drop the above line and include the below as 1/? in v2?
> 
> Okay.
> 
> Note that since you requested alphabetical for PCDs, I'm going to have an
> "Also" in 2/ (now 3/) since the existing PCDs in
> Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf are out of
> alphabetical order.

Actually, I try to never request reordering of existing lines, so I
would be quite happy for you to skip the changes that would motivate
the use of the "also".

I tend to apply a rule of trying to insert *new* (or moved) lines in a
way that will improve the existing order - or in messy cases at least
not make it worse.

I have had it pointed out to me that this is maybe not entirely
obvious...

Regards,

Leif

> I sure hope you're not going to ask me to split this extra reordering into a
> separate commit...
> 
> Regards,
> 
> /Pete
> 
> > 
> > /
> >      Leif
> > 
> >  From 59f01ff36ac7918e9ce166acbd3e963f638ab4b1 Mon Sep 17 00:00:00 2001
> > From: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
> > Date: Mon, 18 Nov 2019 17:47:06 +0000
> > Subject: [PATCH edk2-platforms 1/1] Platform/RPi: Add missing model name
> > 
> > add a missing entry for the "Raspberry Pi Compute Module 3+" in
> > RpiFirmwareGetModelName ().
> > 
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> >   Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > index 9b4aa068857c..dcb434fabefe 100644
> > --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > @@ -1,5 +1,6 @@
> >   /** @file
> >    *
> > + *  Copyright (c) 2019, ARM Limited. All rights reserved.
> >    *  Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> >    *  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> >    *
> > @@ -595,6 +596,8 @@ RpiFirmwareGetModelName (
> >       return "Raspberry Pi 3 Model B+";
> >     case 0x0E:
> >       return "Raspberry Pi 3 Model A+";
> > +  case 0x10:
> > +    return "Raspberry Pi Compute Module 3+";
> >     case 0x11:
> >       return "Raspberry Pi 4 Model B";
> >     default:
> > 
>