From: Nick Desaulniers <ndesaulniers@google.com>
To: "Oliver O'Halloran" <oohall@gmail.com>,
Daniel Axtens <dja@axtens.net>,
Michael Ellerman <mpe@ellerman.id.au>
Cc: Nathan Chancellor <nathan@kernel.org>,
Joe Perches <joe@perches.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Alexey Kardashevskiy <aik@ozlabs.ru>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
Date: Mon, 17 May 2021 17:16:55 -0700 [thread overview]
Message-ID: <CAKwvOdmMugQkTRwC3HOEt2-em2zSfAoi7gpvJRkqfdzSDRMeEg@mail.gmail.com> (raw)
In-Reply-To: <CAOSf1CGoN5R0LUrU=Y=UWho1Z_9SLgCX8s3SbFJXwJXc5BYz4A@mail.gmail.com>
On Thu, Apr 22, 2021 at 6:13 PM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> On Fri, Apr 23, 2021 at 9:09 AM Daniel Axtens <dja@axtens.net> wrote:
> >
> > Hi Nick,
> >
> > > While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> > > possible candidate to convert to #ifdef CONFIG_EEH, but it seems that
> > > based on Kconfig dependencies it's not possible to build this file
> > > without CONFIG_EEH enabled.
> >
> > This seemed odd to me, but I think you're right:
> >
> > arch/powerpc/platforms/Kconfig contains:
> >
> > config EEH
> > bool
> > depends on (PPC_POWERNV || PPC_PSERIES) && PCI
> > default y
> >
> > It's not configurable from e.g. make menuconfig because there's no prompt.
> > You can attempt to explicitly disable it with e.g. `scripts/config -d EEH`
> > but then something like `make oldconfig` will silently re-enable it for
> > you.
> >
> > It's been forced on since commit e49f7a9997c6 ("powerpc/pseries: Rivet
> > CONFIG_EEH for pSeries platform") in 2012 which fixed it for
> > pseries. That moved out from pseries to pseries + powernv later on.
> >
> > There are other cleanups in the same vein that could be made, from the
> > Makefile (which has files only built with CONFIG_EEH) through to other
> > source files. It looks like there's one `#ifdef CONFIG_EEH` in
> > arch/powerpc/platforms/powernv/pci-ioda.c that could be pulled out, for
> > example.
> >
> > I think it's probably worth trying to rip out all of those in one patch?
>
> The change in commit e49f7a9997c6 ("powerpc/pseries: Rivet CONFIG_EEH
> for pSeries platform") never should have been made.
I'll change my patch to keep the conditionals, but use #ifdef instead
of #if then?
>
> There's no inherent reason why EEH needs to be enabled and forcing it
> on is (IMO) a large part of why EEH support is the byzantine
> clusterfuck that it is. One of the things I was working towards was
> allowing pseries and powernv to be built with !CONFIG_EEH since that
> would help define a clearer boundary between what is "eeh support" and
> what is required to support PCI on the platform. Pseries is
> particularly bad for this since PAPR says the RTAS calls needed to do
> a PCI bus reset are part of the EEH extension, but there's non-EEH
> reasons why you might want to use those RTAS calls. The PHB reset that
> we do when entering a kdump kernel is a good example since that uses
> the same RTAS calls, but it has nothing to do with the EEH recovery
> machinery enabled by CONFIG_EEH.
>
> I was looking into that largely because people were considering using
> OPAL for microwatt platforms. Breaking the assumption that
> powernv==EEH support is one of the few bits of work required to enable
> that, but even if you don't go down that road I think everyone would
> be better off if you kept a degree of separation between the two.
--
Thanks,
~Nick Desaulniers
next prev parent reply other threads:[~2021-05-18 0:17 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-22 19:54 [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH Nick Desaulniers
2021-04-22 23:09 ` Daniel Axtens
2021-04-23 1:13 ` Oliver O'Halloran
2021-05-18 0:16 ` Nick Desaulniers [this message]
2021-05-18 6:13 ` Michael Ellerman
2021-05-18 20:40 ` [PATCH v2] powerpc/powernv/pci: fix header guard Nick Desaulniers
2021-05-18 20:41 ` Nathan Chancellor
2021-06-06 12:08 ` Michael Ellerman
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-all 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=CAKwvOdmMugQkTRwC3HOEt2-em2zSfAoi7gpvJRkqfdzSDRMeEg@mail.gmail.com \
--to=ndesaulniers@google.com \
--cc=aik@ozlabs.ru \
--cc=benh@kernel.crashing.org \
--cc=dja@axtens.net \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=nathan@kernel.org \
--cc=oohall@gmail.com \
--cc=paulus@samba.org \
/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;
as well as URLs for NNTP newsgroup(s).