Re: [PATCH v4 edk2-platforms 01/23] Silicon/Broadcom/Bcm282x: Add interrupt driver

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

Please assume that for now.

If you're ready to resubmit and we change our mind, I'll do the
rework.

Regards,

Leif

On Fri, Feb 01, 2019 at 10:28:32AM +0000, Pete Batard wrote:
> All,
> 
> From the feedback below, I'm going to assert that the current consensus is
> to keep Bcm2836.h in IndustryStandard/ and therefore, outside of adding an
> additional description with regards to its purpose, I will keep this patch
> as-is for v5.
> 
> If this is not what we want, please let me know.
> 
> Regards,
> 
> /Pete
> 
> On 2019.02.01 08:43, Laszlo Ersek wrote:
> > On 01/31/19 22:01, Andrew Fish wrote:
> > > 
> > > 
> > > > On Jan 31, 2019, at 11:57 AM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > > 
> > > > +Andrew, Laszlo, Mike.
> > > > 
> > > > On Thu, Jan 31, 2019 at 06:19:48PM +0100, Ard Biesheuvel wrote:
> > > > > On Thu, 31 Jan 2019 at 16:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > > > > 
> > > > > > On Tue, Jan 29, 2019 at 04:26:33PM +0000, Pete Batard wrote:
> > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > > Signed-off-by: Pete Batard <pete@akeo.ie>
> > > > > > > 
> > > > > > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > > ---
> > > > > > > Silicon/Broadcom/Bcm283x/Bcm283x.dec                           |  23 ++
> > > > > > > Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.c   | 367 ++++++++++++++++++++
> > > > > > > Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf |  48 +++
> > > > > > > Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h    |  72 ++++
> > > > > > 
> > > > > > Another generic comment: "IndustryStandard" is something like ACPI,
> > > > > > SMBIOS, PCI, USB, MMC, ... (also including SoC/platform-specific
> > > > > > additions to the same).
> > > > > 
> > > > > Is that your interpretation? Or is this documented somewhere?
> > > > 
> > > > Only in asmuch as it is a clearly descriptive name.
> > > > 
> > > > > I could live with Chipset/, and I'm open to other suggestions, but the
> > > > > Library vs Protocol vs IndustryStandard distinction is very useful
> > > > > imo.
> > > > 
> > > > It is useful because it is descriptive.
> > > > Pretending that an SoC hardware description or a platform description
> > > > header is an "Industry Standard" is disingenuous.
> > > > 
> > > > > > I would be more comfortable with SoC-specific and Platform-specific
> > > > > > include files living directly in Include/.
> > > > > 
> > > > > No, don't drop headers in Include/ please. The namespacing is one of
> > > > > the things EDK2 actually gets right (assuming you define the paths
> > > > > correctly in the package .dec file), and I'd hate to start dumping
> > > > > headers at the root level because we cannot make up our minds what to
> > > > > call the enclosing folder.
> > > > 
> > > > Mike, Andrew - what is your take on this?
> > > > Is there a formal definition of not only what goes in
> > > > IndustryStandard, but where chipset and platform headers should live
> > > > in the namespace?
> > > > 
> > > 
> > > Leif,
> > > 
> > > I kind of think IndustryStandard as things that have a public spec
> > 
> > I think the same. I think any device / interface headers can go under
> > Include/IndustryStandard as long as the interface was explicitly
> > designed for external consumption, and is promised to be stable.
> > 
> > I realize some packages have Include/Register too... I find that a bit
> > redundant.
> > 
> > Thanks
> > Laszlo
> > 
>