All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-04-22 19:54 ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-04-22 19:54 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nick Desaulniers, Nathan Chancellor, Joe Perches,
	Benjamin Herrenschmidt, Paul Mackerras, Oliver O'Halloran,
	Alexey Kardashevskiy, linuxppc-dev, linux-kernel

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.

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Joe Perches <joe@perches.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/platforms/powernv/pci.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 9b9bca169275..591480a37b05 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,6 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-#if CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = NULL;
@@ -734,12 +733,6 @@ static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 
 	return true;
 }
-#else
-static inline pnv_pci_cfg_check(struct pci_dn *pdn)
-{
-	return true;
-}
-#endif /* CONFIG_EEH */
 
 static int pnv_pci_read_config(struct pci_bus *bus,
 			       unsigned int devfn,

base-commit: 16fc44d6387e260f4932e9248b985837324705d8
prerequisite-patch-id: 950233069fb22099a8ff8990f620f5c3586a3cbd
-- 
2.31.1.498.g6c1eba8ee3d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-04-22 19:54 ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-04-22 19:54 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Alexey Kardashevskiy, Nick Desaulniers, linux-kernel,
	Nathan Chancellor, Oliver O'Halloran, Joe Perches,
	Paul Mackerras, linuxppc-dev

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.

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Joe Perches <joe@perches.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/platforms/powernv/pci.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 9b9bca169275..591480a37b05 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,6 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-#if CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = NULL;
@@ -734,12 +733,6 @@ static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 
 	return true;
 }
-#else
-static inline pnv_pci_cfg_check(struct pci_dn *pdn)
-{
-	return true;
-}
-#endif /* CONFIG_EEH */
 
 static int pnv_pci_read_config(struct pci_bus *bus,
 			       unsigned int devfn,

base-commit: 16fc44d6387e260f4932e9248b985837324705d8
prerequisite-patch-id: 950233069fb22099a8ff8990f620f5c3586a3cbd
-- 
2.31.1.498.g6c1eba8ee3d-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
  2021-04-22 19:54 ` Nick Desaulniers
@ 2021-04-22 23:09   ` Daniel Axtens
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2021-04-22 23:09 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Nick Desaulniers, Nathan Chancellor, Joe Perches,
	Benjamin Herrenschmidt, Paul Mackerras, Oliver O'Halloran,
	Alexey Kardashevskiy, linuxppc-dev, linux-kernel

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?

Kind regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-04-22 23:09   ` Daniel Axtens
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2021-04-22 23:09 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Alexey Kardashevskiy, Nick Desaulniers, linux-kernel,
	Nathan Chancellor, Oliver O'Halloran, Joe Perches,
	Paul Mackerras, linuxppc-dev

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?

Kind regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
  2021-04-22 23:09   ` Daniel Axtens
@ 2021-04-23  1:13     ` Oliver O'Halloran
  -1 siblings, 0 replies; 16+ messages in thread
From: Oliver O'Halloran @ 2021-04-23  1:13 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: Nick Desaulniers, Michael Ellerman, Nathan Chancellor,
	Joe Perches, Benjamin Herrenschmidt, Paul Mackerras,
	Alexey Kardashevskiy, linuxppc-dev, Linux Kernel Mailing List

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.

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-04-23  1:13     ` Oliver O'Halloran
  0 siblings, 0 replies; 16+ messages in thread
From: Oliver O'Halloran @ 2021-04-23  1:13 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: Alexey Kardashevskiy, Nick Desaulniers,
	Linux Kernel Mailing List, Nathan Chancellor, Paul Mackerras,
	Joe Perches, linuxppc-dev

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.

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
  2021-04-23  1:13     ` Oliver O'Halloran
@ 2021-05-18  0:16       ` Nick Desaulniers
  -1 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-05-18  0:16 UTC (permalink / raw)
  To: Oliver O'Halloran, Daniel Axtens, Michael Ellerman
  Cc: Nathan Chancellor, Joe Perches, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, linuxppc-dev,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-05-18  0:16       ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-05-18  0:16 UTC (permalink / raw)
  To: Oliver O'Halloran, Daniel Axtens, Michael Ellerman
  Cc: Alexey Kardashevskiy, Linux Kernel Mailing List,
	Nathan Chancellor, Paul Mackerras, Joe Perches, linuxppc-dev

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
  2021-05-18  0:16       ` Nick Desaulniers
@ 2021-05-18  6:13         ` Michael Ellerman
  -1 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2021-05-18  6:13 UTC (permalink / raw)
  To: Nick Desaulniers, Oliver O'Halloran, Daniel Axtens
  Cc: Nathan Chancellor, Joe Perches, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, linuxppc-dev,
	Linux Kernel Mailing List

Nick Desaulniers <ndesaulniers@google.com> writes:
> 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?

Yeah, please.

I'm not sure I agree with oohal that untangling pseries/powernv from
EEH is something we should do, but let's kick that can down the road by
just fixing up the ifdef.

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
@ 2021-05-18  6:13         ` Michael Ellerman
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2021-05-18  6:13 UTC (permalink / raw)
  To: Nick Desaulniers, Oliver O'Halloran, Daniel Axtens
  Cc: Alexey Kardashevskiy, Linux Kernel Mailing List,
	Nathan Chancellor, Paul Mackerras, Joe Perches, linuxppc-dev

Nick Desaulniers <ndesaulniers@google.com> writes:
> 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?

Yeah, please.

I'm not sure I agree with oohal that untangling pseries/powernv from
EEH is something we should do, but let's kick that can down the road by
just fixing up the ifdef.

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2] powerpc/powernv/pci: fix header guard
  2021-05-18  6:13         ` Michael Ellerman
@ 2021-05-18 20:40           ` Nick Desaulniers
  -1 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-05-18 20:40 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Oliver O'Halloran, Nick Desaulniers, Nathan Chancellor,
	Joe Perches, Benjamin Herrenschmidt, Paul Mackerras,
	Greg Kroah-Hartman, Alexey Kardashevskiy, linuxppc-dev,
	linux-kernel

While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
possible candidate to convert to #ifdef CONFIG_EEH.

It seems that based on Kconfig dependencies it's not possible to build
this file without CONFIG_EEH enabled, but based on upstream discussion,
it's not clear yet that CONFIG_EEH should be enabled by default.

For now, simply fix the -Wundef warning.

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Joe Perches <joe@perches.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
Link: https://lore.kernel.org/lkml/CAOSf1CGoN5R0LUrU=Y=UWho1Z_9SLgCX8s3SbFJXwJXc5BYz4A@mail.gmail.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/platforms/powernv/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b18468dc31ff..6bb3c52633fb 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,7 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-#if CONFIG_EEH
+#ifdef CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = NULL;
-- 
2.31.1.751.gd2f1c929bd-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2] powerpc/powernv/pci: fix header guard
@ 2021-05-18 20:40           ` Nick Desaulniers
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-05-18 20:40 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Alexey Kardashevskiy, Nick Desaulniers, linux-kernel,
	Nathan Chancellor, Paul Mackerras, Greg Kroah-Hartman,
	Joe Perches, Oliver O'Halloran, linuxppc-dev

While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
possible candidate to convert to #ifdef CONFIG_EEH.

It seems that based on Kconfig dependencies it's not possible to build
this file without CONFIG_EEH enabled, but based on upstream discussion,
it's not clear yet that CONFIG_EEH should be enabled by default.

For now, simply fix the -Wundef warning.

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Joe Perches <joe@perches.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
Link: https://lore.kernel.org/lkml/CAOSf1CGoN5R0LUrU=Y=UWho1Z_9SLgCX8s3SbFJXwJXc5BYz4A@mail.gmail.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/platforms/powernv/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b18468dc31ff..6bb3c52633fb 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,7 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-#if CONFIG_EEH
+#ifdef CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = NULL;
-- 
2.31.1.751.gd2f1c929bd-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] powerpc/powernv/pci: fix header guard
  2021-05-18 20:40           ` Nick Desaulniers
@ 2021-05-18 20:41             ` Nathan Chancellor
  -1 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2021-05-18 20:41 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Oliver O'Halloran, Joe Perches, Benjamin Herrenschmidt,
	Paul Mackerras, Greg Kroah-Hartman, Alexey Kardashevskiy,
	linuxppc-dev, linux-kernel

On 5/18/2021 1:40 PM, Nick Desaulniers wrote:
> While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> possible candidate to convert to #ifdef CONFIG_EEH.
> 
> It seems that based on Kconfig dependencies it's not possible to build
> this file without CONFIG_EEH enabled, but based on upstream discussion,
> it's not clear yet that CONFIG_EEH should be enabled by default.
> 
> For now, simply fix the -Wundef warning.
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Joe Perches <joe@perches.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/570
> Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
> Link: https://lore.kernel.org/lkml/CAOSf1CGoN5R0LUrU=Y=UWho1Z_9SLgCX8s3SbFJXwJXc5BYz4A@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Makes sense, thanks for the patch!

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>   arch/powerpc/platforms/powernv/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b18468dc31ff..6bb3c52633fb 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -711,7 +711,7 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
>   	return PCIBIOS_SUCCESSFUL;
>   }
>   
> -#if CONFIG_EEH
> +#ifdef CONFIG_EEH
>   static bool pnv_pci_cfg_check(struct pci_dn *pdn)
>   {
>   	struct eeh_dev *edev = NULL;
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] powerpc/powernv/pci: fix header guard
@ 2021-05-18 20:41             ` Nathan Chancellor
  0 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2021-05-18 20:41 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Alexey Kardashevskiy, linux-kernel, Oliver O'Halloran,
	Greg Kroah-Hartman, Joe Perches, Paul Mackerras, linuxppc-dev

On 5/18/2021 1:40 PM, Nick Desaulniers wrote:
> While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> possible candidate to convert to #ifdef CONFIG_EEH.
> 
> It seems that based on Kconfig dependencies it's not possible to build
> this file without CONFIG_EEH enabled, but based on upstream discussion,
> it's not clear yet that CONFIG_EEH should be enabled by default.
> 
> For now, simply fix the -Wundef warning.
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Joe Perches <joe@perches.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/570
> Link: https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.camel@perches.com/
> Link: https://lore.kernel.org/lkml/CAOSf1CGoN5R0LUrU=Y=UWho1Z_9SLgCX8s3SbFJXwJXc5BYz4A@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Makes sense, thanks for the patch!

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>   arch/powerpc/platforms/powernv/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b18468dc31ff..6bb3c52633fb 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -711,7 +711,7 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
>   	return PCIBIOS_SUCCESSFUL;
>   }
>   
> -#if CONFIG_EEH
> +#ifdef CONFIG_EEH
>   static bool pnv_pci_cfg_check(struct pci_dn *pdn)
>   {
>   	struct eeh_dev *edev = NULL;
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] powerpc/powernv/pci: fix header guard
  2021-05-18 20:40           ` Nick Desaulniers
@ 2021-06-06 12:08             ` Michael Ellerman
  -1 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2021-06-06 12:08 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Greg Kroah-Hartman, Alexey Kardashevskiy, linux-kernel,
	Oliver O'Halloran, Nathan Chancellor, linuxppc-dev,
	Joe Perches, Paul Mackerras

On Tue, 18 May 2021 13:40:41 -0700, Nick Desaulniers wrote:
> While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> possible candidate to convert to #ifdef CONFIG_EEH.
> 
> It seems that based on Kconfig dependencies it's not possible to build
> this file without CONFIG_EEH enabled, but based on upstream discussion,
> it's not clear yet that CONFIG_EEH should be enabled by default.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/powernv/pci: fix header guard
      https://git.kernel.org/powerpc/c/73e6e4e01134c9ee97433ad1f470c71b0748b08f

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] powerpc/powernv/pci: fix header guard
@ 2021-06-06 12:08             ` Michael Ellerman
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2021-06-06 12:08 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Alexey Kardashevskiy, Greg Kroah-Hartman, linux-kernel,
	Nathan Chancellor, Oliver O'Halloran, Joe Perches,
	Paul Mackerras, linuxppc-dev

On Tue, 18 May 2021 13:40:41 -0700, Nick Desaulniers wrote:
> While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> possible candidate to convert to #ifdef CONFIG_EEH.
> 
> It seems that based on Kconfig dependencies it's not possible to build
> this file without CONFIG_EEH enabled, but based on upstream discussion,
> it's not clear yet that CONFIG_EEH should be enabled by default.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/powernv/pci: fix header guard
      https://git.kernel.org/powerpc/c/73e6e4e01134c9ee97433ad1f470c71b0748b08f

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-06-06 12:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 19:54 [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH Nick Desaulniers
2021-04-22 19:54 ` Nick Desaulniers
2021-04-22 23:09 ` Daniel Axtens
2021-04-22 23:09   ` Daniel Axtens
2021-04-23  1:13   ` Oliver O'Halloran
2021-04-23  1:13     ` Oliver O'Halloran
2021-05-18  0:16     ` Nick Desaulniers
2021-05-18  0:16       ` Nick Desaulniers
2021-05-18  6:13       ` Michael Ellerman
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:40           ` Nick Desaulniers
2021-05-18 20:41           ` Nathan Chancellor
2021-05-18 20:41             ` Nathan Chancellor
2021-06-06 12:08           ` Michael Ellerman
2021-06-06 12:08             ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.