Re: [PATCH edk2-platforms] Silicon/Bcm2836: add random number generator driver

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Thu, 31 Jan 2019 at 19:35, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Jan 31, 2019 at 06:14:45PM +0100, Ard Biesheuvel wrote:
> > On Thu, 31 Jan 2019 at 16:05, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Wed, Jan 30, 2019 at 08:39:43PM +0100, Ard Biesheuvel wrote:
> > > > Expose the SoC's RNG peripheral via the EFI_RNG_PROTOCOL.
> > > > This is used by Linux to seed the KASLR routines.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >
> > > Not tested, but looks fine. Only question: could we add those few
> > > #defines to IndustryStandard/Bcm2836.h (should that really be
> > > #IndustryStandard, btw?) rather than creating a tiny standalone one?
> > > (more below)
> > >
> >
> > Sure.
> >
> > Re IndustryStandard/, I deliberately chose something idiomatic for
> > EDK2, and this is the least inappropriate one. I could live with
> > Chipset/ as well, but dumping headers under Include/ directly is not
> > the solution IMO.
>
> I disagree. Dumping the main SoC header under the top-level SoC
> directory (and same pattern for platform) is idiomatic.
> Dumping all kinds of random files there isn't, I agree (although that
> happens too).
>
> BeagleBoardPkg/Include/BeagleBoard.h
> OvmfPkg/Include/OvmfPlatforms.h
> Vlv2TbltDevicePkg/Include/Platform.h
>
> Silicon/Hisilicon/Hi6220/Include/Hi6220.h
>
> An alternative pattern is an include directory named after the
> SoC/Platform.
>
> Omap35xxPkg/Include/Omap3530/
>
> You used
>
> Silicon/Socionext/SynQuacer/Include/Platform/
>
> which I also don't mind.
>
> If you'd prefer it, I'd be happy with Platform/ and Silicon/.
>
> But we'd better settle on something before Pete changes too much based
> on my feedback.
>

Yeah. I'll park this until Pete's stuff is merged.