Clean up W=1 for the powernv platform

Clean up W=1 for the powernv platform

[Oliver O'Halloran]

Fixes the (mostly) suprious errors we get when building powernv with
W=1. More work is required to build all of powerpc with W=1, let alone
W=2.

[PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Oliver O'Halloran]

When building with W=1 we get the following warning:

 arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
 arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
 	empty body in an ‘if’ statement [-Werror=empty-body]
   276 |      cpu, srr1);
       |                ^
 cc1: all warnings being treated as errors

The full context is this block:

 if (srr1 && !generic_check_cpu_restart(cpu))
 	DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
 			cpu, srr1);

When building with DEBUG undefined DBG() expands to nothing and GCC emits
the warning due to the lack of braces around an empty statement.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
We could add the braces too. That might even be better since it's a multi-line
if block even though it's only a single statement.
---
 arch/powerpc/platforms/powernv/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index b2ba3e95bda7..bbf361f23ae8 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -43,7 +43,7 @@
 #include <asm/udbg.h>
 #define DBG(fmt...) udbg_printf(fmt)
 #else
-#define DBG(fmt...)
+#define DBG(fmt...) do { } while (0)
 #endif
 
 static void pnv_smp_setup_cpu(int cpu)
-- 
2.26.2

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Joel Stanley]

On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> When building with W=1 we get the following warning:
>
>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
>         empty body in an ‘if’ statement [-Werror=empty-body]
>    276 |      cpu, srr1);
>        |                ^
>  cc1: all warnings being treated as errors
>
> The full context is this block:
>
>  if (srr1 && !generic_check_cpu_restart(cpu))
>         DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
>                         cpu, srr1);
>
> When building with DEBUG undefined DBG() expands to nothing and GCC emits
> the warning due to the lack of braces around an empty statement.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> We could add the braces too. That might even be better since it's a multi-line
> if block even though it's only a single statement.

Or you could put it all on one line, now that our 120 line overlords
have taken over.

Reviewed-by: Joel Stanley <joel@jms.id.au>

Messy:

$ git grep "define DBG(" arch/powerpc/ |grep -v print
arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
arch/powerpc/kernel/iommu.c:#define DBG(...)
arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/kernel/prom.c:#define DBG(fmt...)
arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
arch/powerpc/kernel/smp.c:#define DBG(fmt...)
arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
do { } while(0)
arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/maple/time.c:#define DBG(x...)
arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)


> ---
>  arch/powerpc/platforms/powernv/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index b2ba3e95bda7..bbf361f23ae8 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -43,7 +43,7 @@
>  #include <asm/udbg.h>
>  #define DBG(fmt...) udbg_printf(fmt)
>  #else
> -#define DBG(fmt...)
> +#define DBG(fmt...) do { } while (0)
>  #endif
>
>  static void pnv_smp_setup_cpu(int cpu)
> --
> 2.26.2
>

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Oliver O'Halloran]

On Tue, Aug 4, 2020 at 12:07 PM Joel Stanley <joel@jms.id.au> wrote:
>
> Messy:
>
> $ git grep "define DBG(" arch/powerpc/ |grep -v print
> arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
> arch/powerpc/kernel/iommu.c:#define DBG(...)
> arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/kernel/prom.c:#define DBG(fmt...)
> arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
> arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
> arch/powerpc/kernel/smp.c:#define DBG(fmt...)
> arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
> arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
> arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
> arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
> arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
> do { } while(0)
> arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
> arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
> arch/powerpc/platforms/maple/time.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
> arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
> arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
> arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
> arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
> arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
> arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)

I started off writing a patch that fixed all these too. When I went to
test it I discovered there's a giant pile of other W=1 warnings from
other parts of arch/powerpc/ so I figured I'd start with something
less ambitious.

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Michael Ellerman]

Joel Stanley <joel@jms.id.au> writes:
> On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran <oohall@gmail.com> wrote:
>>
>> When building with W=1 we get the following warning:
>>
>>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
>>         empty body in an ‘if’ statement [-Werror=empty-body]
>>    276 |      cpu, srr1);
>>        |                ^
>>  cc1: all warnings being treated as errors
>>
>> The full context is this block:
>>
>>  if (srr1 && !generic_check_cpu_restart(cpu))
>>         DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
>>                         cpu, srr1);
>>
>> When building with DEBUG undefined DBG() expands to nothing and GCC emits
>> the warning due to the lack of braces around an empty statement.
>>
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> ---
>> We could add the braces too. That might even be better since it's a multi-line
>> if block even though it's only a single statement.
>
> Or you could put it all on one line, now that our 120 line overlords
> have taken over.

Yeah I think that one may as well be one line.

> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> Messy:
>
> $ git grep "define DBG(" arch/powerpc/ |grep -v print
> arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
> arch/powerpc/kernel/iommu.c:#define DBG(...)
> arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/kernel/prom.c:#define DBG(fmt...)
..

Yeah, gross old cruft.

The vast majority of those should just be replaced with pr_devel()
and/or pr_debug().

The pnv_smp_cpu_kill_self() case is one where we probably do want to
stick with udbg_printf(), because I don't think it's kosher to call
printk() from an offline CPU.

cheers

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

On Tue, 4 Aug 2020 10:54:05 +1000, Oliver O'Halloran wrote:
> When building with W=1 we get the following warning:
> 
>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
>  	empty body in an ‘if’ statement [-Werror=empty-body]
>    276 |      cpu, srr1);
>        |                ^
>  cc1: all warnings being treated as errors
> 
> [...]

Applied to powerpc/next.

[1/6] powerpc/powernv/smp: Fix spurious DBG() warning
      https://git.kernel.org/powerpc/c/f6bac19cf65c5be21d14a0c9684c8f560f2096dd
[2/6] powerpc/powernv: Include asm/powernv.h from the local powernv.h
      https://git.kernel.org/powerpc/c/8471c1dd93de9a2278d41c527b76291e4ace8f1c
[3/6] powerpc/powernv: Staticify functions without prototypes
      https://git.kernel.org/powerpc/c/3b70464aa78917e88c1d4bfc2100c344c0eda8e0
[4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c
      https://git.kernel.org/powerpc/c/fb248c3121af713f31736af359608491544cfc23
[5/6] powerpc/powernv: Remove set but not used variable 'parent'
      https://git.kernel.org/powerpc/c/18102e4bcc47f5b5ac70e2e4461d022c1ee6df24
[6/6] powerpc/nx: Don't pack struct coprocessor_request_block
      https://git.kernel.org/powerpc/c/3ced132a055c4e5046d21732393ae6848ff309e0

cheers

[PATCH 2/6] powerpc/powernv: Include asm/powernv.h from the local powernv.h

[Oliver O'Halloran]

The asm/powernv.h header provides prototypes for functions which need to be
called by non-powernv platform code. Also include it in the powernv.h
that's local to the platform directory to squash some warnings about
non-static functions missing prototypes.

Also include powernv.h since from opal-memcons.c since it has the
prototypes for the memcons wrangling functions which are used for the opal
and ultravisor msglog.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/opal-msglog.c | 2 ++
 arch/powerpc/platforms/powernv/powernv.h     | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index d26da19a611f..d3b6e135c18b 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -12,6 +12,8 @@
 #include <linux/types.h>
 #include <asm/barrier.h>
 
+#include "powernv.h"
+
 /* OPAL in-memory console. Defined in OPAL source at core/console.c */
 struct memcons {
 	__be64 magic;
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 1aa51c4fa904..11df4e16a1cc 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h
@@ -2,6 +2,13 @@
 #ifndef _POWERNV_H
 #define _POWERNV_H
 
+/*
+ * There's various hacks scattered throughout the generic powerpc arch code
+ * that needs to call into powernv platform stuff. The prototypes for those
+ * functions are in asm/powernv.h
+ */
+#include <asm/powernv.h>
+
 #ifdef CONFIG_SMP
 extern void pnv_smp_init(void);
 #else
-- 
2.26.2

[PATCH 3/6] powerpc/powernv: Staticify functions without prototypes

[Oliver O'Halloran]

There's a few scattered in the powernv platform.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 4 ++--
 arch/powerpc/platforms/powernv/rng.c         | 2 +-
 arch/powerpc/platforms/powernv/vas-window.c  | 9 ++++-----
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 9af8c3b98853..663bd69ac51b 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -38,7 +38,7 @@
 
 static int eeh_event_irq = -EINVAL;
 
-void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
+static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	dev_dbg(&pdev->dev, "EEH: Setting up device\n");
 	eeh_probe_device(pdev);
@@ -190,7 +190,7 @@ PNV_EEH_DBGFS_ENTRY(inbB, 0xE10);
 
 #endif /* CONFIG_DEBUG_FS */
 
-void pnv_eeh_enable_phbs(void)
+static void pnv_eeh_enable_phbs(void)
 {
 	struct pci_controller *hose;
 	struct pnv_phb *phb;
diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c
index 8035caf6e297..72c25295c1c2 100644
--- a/arch/powerpc/platforms/powernv/rng.c
+++ b/arch/powerpc/platforms/powernv/rng.c
@@ -65,7 +65,7 @@ int powernv_get_random_real_mode(unsigned long *v)
 	return 1;
 }
 
-int powernv_get_random_darn(unsigned long *v)
+static int powernv_get_random_darn(unsigned long *v)
 {
 	unsigned long val;
 
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 6434f9cb5aed..5f5fe63a3d1c 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -186,7 +186,7 @@ static void unmap_winctx_mmio_bars(struct vas_window *window)
  * OS/User Window Context (UWC) MMIO Base Address Region for the given window.
  * Map these bus addresses and save the mapped kernel addresses in @window.
  */
-int map_winctx_mmio_bars(struct vas_window *window)
+static int map_winctx_mmio_bars(struct vas_window *window)
 {
 	int len;
 	u64 start;
@@ -214,7 +214,7 @@ int map_winctx_mmio_bars(struct vas_window *window)
  *	 registers are not sequential. And, we can only write to offsets
  *	 with valid registers.
  */
-void reset_window_regs(struct vas_window *window)
+static void reset_window_regs(struct vas_window *window)
 {
 	write_hvwc_reg(window, VREG(LPID), 0ULL);
 	write_hvwc_reg(window, VREG(PID), 0ULL);
@@ -357,7 +357,8 @@ static void init_rsvd_tx_buf_count(struct vas_window *txwin,
  *	as a one-time task? That could work for NX but what about other
  *	receivers?  Let the receivers tell us the rx-fifo buffers for now.
  */
-int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
+static void init_winctx_regs(struct vas_window *window,
+			     struct vas_winctx *winctx)
 {
 	u64 val;
 	int fifo_size;
@@ -499,8 +500,6 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
 	val = SET_FIELD(VAS_WINCTL_NX_WIN, val, winctx->nx_win);
 	val = SET_FIELD(VAS_WINCTL_OPEN, val, 1);
 	write_hvwc_reg(window, VREG(WINCTL), val);
-
-	return 0;
 }
 
 static void vas_release_window_id(struct ida *ida, int winid)
-- 
2.26.2

Re: [PATCH 3/6] powerpc/powernv: Staticify functions without prototypes

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:01, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> There's a few scattered in the powernv platform.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -38,7 +38,7 @@
>
>  static int eeh_event_irq = -EINVAL;
>
> -void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> +static void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  {
>         dev_dbg(&pdev->dev, "EEH: Setting up device\n");
>         eeh_probe_device(pdev);

This one could even be deleted as eeh_probe_device has it's own dev_dbg.

Reviewed-by: Joel Stanley <joel@jms.id.au>

[PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c

[Oliver O'Halloran]

Comments opening with /** are parsed by kerneldoc and this causes the
following warning to be printed:

	arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot understand
	function prototype: 'struct opal_prd_msg_queue_item '

opal_prd_mesg_queue_item is an internal data structure so there's no real
need for it to be documented at all. Fix up the comment to squash the
warning.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/opal-prd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c
index 45f4223a790f..deddaebf8c14 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -24,7 +24,7 @@
 #include <linux/uaccess.h>
 
 
-/**
+/*
  * The msg member must be at the end of the struct, as it's followed by the
  * message data.
  */
-- 
2.26.2

Re: [PATCH 4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:03, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> Comments opening with /** are parsed by kerneldoc and this causes the
> following warning to be printed:
>
>         arch/powerpc/platforms/powernv/opal-prd.c:31: warning: cannot understand
>         function prototype: 'struct opal_prd_msg_queue_item '
>
> opal_prd_mesg_queue_item is an internal data structure so there's no real
> need for it to be documented at all. Fix up the comment to squash the
> warning.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

[PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable

[Oliver O'Halloran]

The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
anything anymore and can be dropped.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index c9c25fb0783c..6d48155bd885 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -894,7 +894,6 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 
 int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 {
-	struct pci_dev *parent;
 	uint8_t bcomp, dcomp, fcomp;
 	long rc, rid_end, rid;
 
@@ -904,7 +903,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 
 		dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
 		fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
-		parent = pe->pbus->self;
 		if (pe->flags & PNV_IODA_PE_BUS_ALL)
 			count = resource_size(&pe->pbus->busn_res);
 		else
@@ -925,12 +923,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 		}
 		rid_end = pe->rid + (count << 8);
 	} else {
-#ifdef CONFIG_PCI_IOV
-		if (pe->flags & PNV_IODA_PE_VF)
-			parent = pe->parent_dev;
-		else
-#endif /* CONFIG_PCI_IOV */
-			parent = pe->pdev->bus->self;
 		bcomp = OpalPciBusAll;
 		dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
 		fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
-- 
2.26.2

Re: [PATCH 5/6] powerpc/powernv/pci: Drop unused parent variable

[Joel Stanley]

On Tue, 4 Aug 2020 at 01:06, Oliver O'Halloran <oohall@gmail.com> wrote:
>
> The "parent" variable in pnv_pci_ioda_configure_pe() isn't used for
> anything anymore and can be dropped.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

[PATCH 6/6] powerpc/nx: Don't pack struct coprocessor_request_block

[Oliver O'Halloran]

Building with W=1 results in the following warning:

In file included from arch/powerpc/platforms/powernv/vas-fault.c:16:
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
	coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
  159 | } __packed;
      | ^
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
	coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
	coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
./arch/powerpc/include/asm/icswx.h:159:1: error: alignment 1 of ‘struct
	coprocessor_request_block’ is less than 16 [-Werror=packed-not-aligned]
cc1: all warnings being treated as errors

This happens because coprocessor_request_block includes several
sub-structures with an alignment specified using the __aligned(XX)
attribute. The problem comes from coprocessor_request_block having the
__packed attribute. Packing the structure causes the preferred alignment of
the nested structures to be ignored and we get the warnings as a result.

This isn't a problem in practice since the struct is defined with explicit
padding in the form of reserved fields, but we'd like to get rid of the
spurious warnings. The simplest solution is to remove the packed attribute
and use a BUILD_BUG_ON() to ensure the struct is the correct (expected by
HW) size compile time.

Also add a __aligned(128) to the request block structure since Book4 for P8
suggests the HW requires it to be aligned to a 128 byte boundary. There's a
similar requirement for P9 since the COPY and PASTE instructions used to
invoke VAS/NX accelerators operates on a cache line boundary.

Cc: Dan Streetman <ddstreet@ieee.org>
Cc: Haren Myneni <haren@linux.vnet.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/include/asm/icswx.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index b0c70a35fd0e..f6599ccb3012 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -156,8 +156,7 @@ struct coprocessor_request_block {
 	u8 reserved[32];
 
 	struct coprocessor_status_block csb;
-} __packed;
-
+} __aligned(128);
 
 /* RFC02167 Initiate Coprocessor Instructions document
  * Chapter 8.2.1.1.1 RS
@@ -188,6 +187,9 @@ static inline int icswx(__be32 ccw, struct coprocessor_request_block *crb)
 	__be64 ccw_reg = ccw;
 	u32 cr;
 
+	/* NB: the same structures are used by VAS-NX */
+	BUILD_BUG_ON(sizeof(*crb) != 128);
+
 	__asm__ __volatile__(
 	PPC_ICSWX(%1,0,%2) "\n"
 	"mfcr %0\n"
-- 
2.26.2