public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Marcin Wojtas <mw@semihalf.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Grzegorz Jaszczyk <jaz@semihalf.com>,
	Grzegorz Bernacki <gjb@semihalf.com>,
	Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
	alon.rotman@solid-run.com
Subject: Re: [edk2-platforms PATCH 0/4] Marvell readmes
Date: Wed, 22 Sep 2021 13:46:21 +0100	[thread overview]
Message-ID: <20210922124621.orqr5wj26zqfzac5@leviathan> (raw)
In-Reply-To: <CAMj1kXF6VO31SzzX5ov1ncuTA7ciU7jNcFN8ggW6Ccc1ZS5YjQ@mail.gmail.com>

On Wed, Sep 22, 2021 at 14:20:31 +0200, Ard Biesheuvel wrote:
> On Wed, 22 Sept 2021 at 14:14, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Wed, Sep 22, 2021 at 13:24:07 +0200, Ard Biesheuvel wrote:
> > > On Tue, 21 Sept 2021 at 13:02, Marcin Wojtas <mw@semihalf.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > This short patchset adds README files for the platforms
> > > > based on Marvell SoC's, which have already been
> > > > SystemReady ES certified. It also bumps the firmware
> > > > revision after the PCI / IcuLib rework and addition of
> > > > the CN913x CEx7 Evaluation Board support.
> > > >
> > > > The patches are available on a public branch:
> > > > https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/marvell-howtos-upstream-r20210921
> > > >
> > >
> > > Thanks
> > >
> > > Pushed as 64394fd2b48f..d1d4dd2c39e7
> >
> > Ugh, wish I'd taken the time to respond yesterday.
> >
> > This is more proliferation of "include all very specific build
> > steps for a very specific target and build environment" documentation.
> >
> > I don't see this as an improvement.
> >
> > Platform docs should document what one needs to know specific to the
> > platform. If a vendor wishes to provide full recipes, they should host
> > that on a wiki somewhere.
> >
> 
> Fair point.
> 
> Marcin, when you find the time, could you please do a pass over these
> files with Leif's critique in mind?

Some suggestions/requests:

The top-level Readme.md should contain all relevant build system and
cross compilation information, so please refer to that instead of
duplicating information. If it is missing some generically useful
information, please add it.
(That file, for example, points out the appropriate location to get
arm cross-toolchains from if your distro doesn't provide them, and
Linaro stopped working on GCC years ago.)

Please *do* document the repositories required to build (and hence
need to specify in PACKAGES_PATH). The example build command line for
CN913x CEx7 requires edk2-non-osi.

Please don't document the inclusion of the Shell TFTP command by
default. Mention it as an option if you must.

Document the builds in chronological order, don't interleave TF-A and
EDK2 instructions. Ideally, cut TF-A instructions down to a bare
minimum (e.g. "pass the generated EDK2 image to the TF-A build with
`$ export BL33=...`) and link to aforementioned wiki page for the full
recipe.

Best Regards,

Leif

      parent reply	other threads:[~2021-09-22 12:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 11:00 [edk2-platforms PATCH 0/4] Marvell readmes Marcin Wojtas
2021-09-21 11:00 ` [edk2-platforms PATCH 1/4] SolidRun/Cn913xCEx7Eval: Add Readme.md Marcin Wojtas
2021-09-21 11:00 ` [edk2-platforms PATCH 2/4] SolidRun/Armada80x0McBin: " Marcin Wojtas
2021-09-21 11:00 ` [edk2-platforms PATCH 3/4] Marvell/Cn913xDbA: " Marcin Wojtas
2021-09-21 11:00 ` [edk2-platforms PATCH 4/4] Marvell: Armada7k8k/OcteonTx: Bump firmware to "EDK2 SH 1.1" revision Marcin Wojtas
2021-09-22 11:24 ` [edk2-platforms PATCH 0/4] Marvell readmes Ard Biesheuvel
2021-09-22 12:14   ` Leif Lindholm
2021-09-22 12:20     ` Ard Biesheuvel
2021-09-22 12:46       ` Marcin Wojtas
2021-09-23 10:26         ` Leif Lindholm
2021-09-22 12:46       ` Leif Lindholm [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210922124621.orqr5wj26zqfzac5@leviathan \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox