From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web09.22441.1574176066623355112 for ; Tue, 19 Nov 2019 07:07:47 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=H0cDx5JM; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f65.google.com with SMTP id s5so24271736wrw.2 for ; Tue, 19 Nov 2019 07:07:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=XbsON4bugg71M+Qo4SPsUtpSAmUhDDnwe/k9JjbsdlY=; b=H0cDx5JMAz6wOgwGy1u2+fZWYz5BgLrrEloJRUfmtuwiEp8wfSu5UMj+SL9rRRH2JN xM3O3wHFnIabHz8pcQyFkU6qXxV4mJqFPDuSwrIRFqxXIx8p/YeUA01afBnv56UEm76/ GqhJJ7DRR9WPaO5vXBDSqZQGLsWgsOt/emcqR6Db+46MIZLSoQLAUo2L5X8TyVVW0lDU J86SxFYWPpFyM2v+QSOQdKOolfofHkwThb34vSKNYXV56saoIEKYcYQJT3ebIip8Bc9t NRiYZ6FvByDscx8Gs70XjtRnyy3VwOJjQgD/T1UqSnxXyILW2lyC0BZeONjiFBpf8QYI NACQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=XbsON4bugg71M+Qo4SPsUtpSAmUhDDnwe/k9JjbsdlY=; b=smkrmW3Hte8i7z/xhN0h2MeAxmmgAaktDoipJezN4xmi/Ph806LIch8dFhAiKkwpzh 4+1krXGI0rhef7uubCrAJKjzhwMI8BDXCPIlrtL15tIRsp8wrT6TFcaqG9z5IEPrRaFC gjns0q8ZDn5GCohhRLmA7R3qk7iBd3FzVy7PFo3FwTI1BXDaokskUEgbOUOmvCVbMk1Q MWNDwxAKZIJTLFxT5quua+zBcUYRUTY6fGnvj+qYJxeZiF5SaJIDfbtB53mCvWCls4pD ogYqF1VyxpXUEPhyeh9MMB29VmMdQeszHUdtyt68lrOrlZVc/Y7/A8Ch5QsBDfmjNYB9 DiSQ== X-Gm-Message-State: APjAAAUqi7DcAtNS4iZaowmbsZlwCA0qBcTg+xgsA1eK164PXYRgQ8T5 gjUgDlvXzQUuyvPsD9xspAtYp+scY/EwvI6uVoNUKg== X-Google-Smtp-Source: APXvYqwa7MN8y/rlCMs1Z4Led2XJafwQ/uePzW6TrhmZ1wt4ybRyCFMxIL7FnXhCPmzClx6VtcYerSoSwByQaZC+YTI= X-Received: by 2002:adf:f743:: with SMTP id z3mr36439866wrp.200.1574176065074; Tue, 19 Nov 2019 07:07:45 -0800 (PST) MIME-Version: 1.0 References: <20191114160740.10072-1-pete@akeo.ie> <20191114160740.10072-2-pete@akeo.ie> <20191118175156.GX7323@bivouac.eciton.net> <99b30bf5-a9c6-62aa-f7e2-7db9c2bc9848@akeo.ie> <20191118180521.GY7323@bivouac.eciton.net> <3e51f090-9391-1c59-5a64-c2927011ccb1@akeo.ie> In-Reply-To: <3e51f090-9391-1c59-5a64-c2927011ccb1@akeo.ie> From: "Ard Biesheuvel" Date: Tue, 19 Nov 2019 16:07:34 +0100 Message-ID: Subject: Re: [edk2-platforms][PATCH 1/8] Platform/RPi: Add model family detection To: Pete Batard Cc: Leif Lindholm , edk2-devel-groups-io , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: text/plain; charset="UTF-8" On Mon, 18 Nov 2019 at 19:32, Pete Batard wrote: > > On 2019.11.18 18:05, Leif Lindholm wrote: > > 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 > >>>> > >>>> 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... > > Well, this is exactly what I would point out as an example of the strive > for commit atomicity getting in the way of a more readable codebase as > well as overall user experience (the users here being the developers who > are dealing with the code). The reason I'm pointing this out is that, in > the past, I have been dealing with projects that seemed to care more > about keeping a squeaky clean commit history than they seemed to care > about making the underlying code as good as it could possibly get, which > resulted in increased pain for the developers having to contend with > said codebase and ultimately end-users of the software produced from > that codebase. > > Again, I would assert that there has to exist a middle ground between > keeping a super-clean commit history and improving the source where it > can indeed be improved at little cost, by not always defaulting to > people having to devote extra time splitting patches. > > But I understand this is not my choice to make here. Thus I'll stay away > from reordering that doesn't have to do with new PCDs being introduced. > Please keep in mind that when open source maintainers take ownership of your code, they assume the responsibility to ensure that it doesn't get broken by future updates elsewhere in the codebase, often way beyond the commercial lifetime of the product that is supported by that code. This is a sizable effort, and an important part of managing that effort is ensuring that the code is in an acceptable shape to begin with, and what 'acceptable' means differs between different maintainers. Not being able to revert a patch easily because it touches unrelated code may make our lives more difficult years after you have stopped caring about this platform entirely.