LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/2] spi: fsl-spi: Allow dynamic allocation of CPM1 parameter RAM
From: Scott Wood @ 2014-10-03 19:51 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Mark Brown, linuxppc-dev, linux-kernel, linux-spi
In-Reply-To: <20141003164917.397871AB276@localhost.localdomain>

On Fri, 2014-10-03 at 18:49 +0200, Christophe Leroy wrote:
> On CPM1, the SPI parameter RAM has a default location. In order to use SPI while
> using SCC2 with features like QMC or Ethernet, it is necessary to relocate SPI
> parameter RAM in a free location in the CPM dual port RAM.

Please explain in more detail why it's necessary.

>  With this patch,
> when "fsl,cpm1-spi-reloc" instead of "fsl,cpm1-spi" compatible is set in the
> device tree, the parameter RAM for SPI is dynamically allocated
> with cpm_muram_alloc().
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
> ---
> Changes from v1 to v2: using OF compatible instead of compile time option
> 
>  drivers/spi/spi-fsl-cpm.c   | 3 +++
>  drivers/spi/spi-fsl-lib.c   | 2 ++
>  include/linux/fsl_devices.h | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
> index 0f3a912..048ca5f 100644
> --- a/drivers/spi/spi-fsl-cpm.c
> +++ b/drivers/spi/spi-fsl-cpm.c
> @@ -261,6 +261,9 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
>  	if (mspi->flags & SPI_CPM2) {
>  		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
>  		out_be16(spi_base, pram_ofs);
> +	} else if (mspi->flags & SPI_CPM1_RELOC) {
> +		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 32);
> +		out_be16(spi_base, pram_ofs);
>  	} else {
>  		u16 rpbase = in_be16(spi_base);
>  
> diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c
> index c3f7802..e9db4c5 100644
> --- a/drivers/spi/spi-fsl-lib.c
> +++ b/drivers/spi/spi-fsl-lib.c
> @@ -227,6 +227,8 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev)
>  		pdata->flags = SPI_CPM_MODE | SPI_CPM2;
>  	else if (of_device_is_compatible(np, "fsl,cpm1-spi"))
>  		pdata->flags = SPI_CPM_MODE | SPI_CPM1;
> +	else if (of_device_is_compatible(np, "fsl,cpm1-spi-reloc"))
> +		pdata->flags = SPI_CPM_MODE | SPI_CPM1 | SPI_CPM1_RELOC;

Where's the binding and device tree update for this?  What specific
aspect of hardware does it describe?

-Scott

^ permalink raw reply

* [PATCH v2 2/2] spi: fsl-spi: Allow dynamic allocation of CPM1 parameter RAM
From: Christophe Leroy @ 2014-10-03 16:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi

On CPM1, the SPI parameter RAM has a default location. In order to use SPI while
using SCC2 with features like QMC or Ethernet, it is necessary to relocate SPI
parameter RAM in a free location in the CPM dual port RAM. With this patch,
when "fsl,cpm1-spi-reloc" instead of "fsl,cpm1-spi" compatible is set in the
device tree, the parameter RAM for SPI is dynamically allocated
with cpm_muram_alloc().

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
Changes from v1 to v2: using OF compatible instead of compile time option

 drivers/spi/spi-fsl-cpm.c   | 3 +++
 drivers/spi/spi-fsl-lib.c   | 2 ++
 include/linux/fsl_devices.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 0f3a912..048ca5f 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -261,6 +261,9 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
 	if (mspi->flags & SPI_CPM2) {
 		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
 		out_be16(spi_base, pram_ofs);
+	} else if (mspi->flags & SPI_CPM1_RELOC) {
+		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 32);
+		out_be16(spi_base, pram_ofs);
 	} else {
 		u16 rpbase = in_be16(spi_base);
 
diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c
index c3f7802..e9db4c5 100644
--- a/drivers/spi/spi-fsl-lib.c
+++ b/drivers/spi/spi-fsl-lib.c
@@ -227,6 +227,8 @@ int of_mpc8xxx_spi_probe(struct platform_device *ofdev)
 		pdata->flags = SPI_CPM_MODE | SPI_CPM2;
 	else if (of_device_is_compatible(np, "fsl,cpm1-spi"))
 		pdata->flags = SPI_CPM_MODE | SPI_CPM1;
+	else if (of_device_is_compatible(np, "fsl,cpm1-spi-reloc"))
+		pdata->flags = SPI_CPM_MODE | SPI_CPM1 | SPI_CPM1_RELOC;
 
 	return 0;
 }
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index a82296a..417ad52 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -123,6 +123,7 @@ struct fsl_spi_platform_data {
 #define SPI_CPM1		(1 << 2) /* SPI unit is in CPM1 block */
 #define SPI_CPM2		(1 << 3) /* SPI unit is in CPM2 block */
 #define SPI_QE			(1 << 4) /* SPI unit is in QE block */
+#define SPI_CPM1_RELOC		(1 << 5) /* SPI pram has to be relocated */
 	/* board specific information */
 	u16	max_chipselect;
 	void	(*cs_control)(struct spi_device *spi, bool on);
-- 
2.1.0

^ permalink raw reply related

* [PATCH v2 1/2] spi: fsl-spi: Fix parameter ram offset setup for CPM1
From: Christophe Leroy @ 2014-10-03 16:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi

On CPM1, the SPI parameter RAM has a default location. In fsl_spi_cpm_get_pram()
there was a confusion between the SPI_BASE register and the base of the SPI
parameter RAM. Fortunatly, it was working properly with MPC866 and MPC885
because they do set SPI_BASE, but on MPC860 and other old MPC8xx that doesn't
set SPI_BASE, pram_ofs was not properly set. This patch fixes this confusion.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
Changes from v1 to v2: none

 drivers/spi/spi-fsl-cpm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..0f3a912 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -262,15 +262,14 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
 		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
 		out_be16(spi_base, pram_ofs);
 	} else {
-		struct spi_pram __iomem *pram = spi_base;
-		u16 rpbase = in_be16(&pram->rpbase);
+		u16 rpbase = in_be16(spi_base);
 
-		/* Microcode relocation patch applied? */
+		/* Microcode relocation patch applied | rpbase set by default */
 		if (rpbase) {
 			pram_ofs = rpbase;
 		} else {
-			pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
-			out_be16(spi_base, pram_ofs);
+			pram_ofs = offsetof(cpm8xx_t, cp_dparam[PROFF_SPI]) -
+				   offsetof(cpm8xx_t, cp_dpmem[0]);
 		}
 	}
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH v2 0/2] fsl: fsl-spi: Fix CMP1 pram allocation and allow dynamic allocation
From: Christophe Leroy @ 2014-10-03 16:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi

This patchset:
1) Fix parameter ram offset setup for CPM1
2) Allow dynamic allocation of CPM1 parameter RAM

Tested on MPC885.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
Changes from v1 to v2: using OF compatible instead of compile time option

 drivers/spi/spi-fsl-cpm.c   | 12 +++++++-----
 drivers/spi/spi-fsl-lib.c   |  2 ++
 include/linux/fsl_devices.h |  1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

^ permalink raw reply

* Re: [PATCH] ASoC: fsl_spdif: Remove unused includes of linux/clk-private.h
From: Mark Brown @ 2014-10-03 16:19 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: alsa-devel, Mike Turquette, Liam Girdwood, Takashi Iwai,
	linux-kernel, Timur Tabi, Jaroslav Kysela, Nicolin Chen, Xiubo Li,
	linuxppc-dev
In-Reply-To: <1412351653-29592-1-git-send-email-tomeu.vizoso@collabora.com>

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

On Fri, Oct 03, 2014 at 05:54:13PM +0200, Tomeu Vizoso wrote:
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* [PATCH] ASoC: fsl_spdif: Remove unused includes of linux/clk-private.h
From: Tomeu Vizoso @ 2014-10-03 15:54 UTC (permalink / raw)
  Cc: alsa-devel, Mike Turquette, Tomeu Vizoso, Takashi Iwai,
	linux-kernel, Mark Brown, Timur Tabi, Liam Girdwood, Nicolin Chen,
	Xiubo Li, Jaroslav Kysela, linuxppc-dev

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 sound/soc/fsl/fsl_spdif.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 70acfe4..5bda323 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -15,7 +15,6 @@
 
 #include <linux/bitrev.h>
 #include <linux/clk.h>
-#include <linux/clk-private.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH 2/2] spi: fsl-spi: Allow dynamic allocation of CPM1 parameter RAM
From: Mark Brown @ 2014-10-03 14:44 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Marcelo Tosatti, linux-kernel, linux-spi, Paul Mackerras,
	scottwood, linuxppc-dev
In-Reply-To: <20141003125609.5BEA11AB276@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Fri, Oct 03, 2014 at 02:56:09PM +0200, Christophe Leroy wrote:

> +config CPM1_RELOCSPI
> +	bool "Dynamic SPI relocation"
> +	default n
> +	help
> +	  On recent MPC8xx (at least MPC866 and MPC885) SPI can be relocated
> +	  without micropatch. This activates relocation to a dynamically
> +	  allocated area in the CPM Dual port RAM.
> +	  When combined with SPI relocation patch (for older MPC8xx) it avoids
> +	  the "loss" of additional Dual port RAM space just above the patch,
> +	  which might be needed for example when using the CPM QMC.

Something like this shouldn't be a compile time option.  Either it
should be unconditional or it should be triggered in some system
specific manner (from DT, from knowing about other users or similar).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/4] sound/radeon: Move 64-bit MSI quirk from arch to driver
From: Alex Deucher @ 2014-10-03 13:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, Dave Airlie, Linux PCI, Anton Blanchard, Brian King,
	Yijing Wang, Takashi Iwai, Bjorn Helgaas
In-Reply-To: <1412313820.30859.2.camel@pasglop>

On Fri, Oct 3, 2014 at 1:23 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2014-10-01 at 22:13 -0400, Alex Deucher wrote:
>
>> The attached updated patch only flags the affected asics.
>
> I need formal (email :-) permission to add your s-o-b (and From: while
> at it since you wrote most of it) for this patch. I'll then include
> it in my new series.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

^ permalink raw reply

* [PATCH 2/2] spi: fsl-spi: Allow dynamic allocation of CPM1 parameter RAM
From: Christophe Leroy @ 2014-10-03 12:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Mark Brown,
	Marcelo Tosatti, Vitaly Bordug
  Cc: scottwood, linuxppc-dev, linux-kernel, linux-spi

On CPM1, the SPI parameter RAM has a default location. In order to use SPI while
using SCC2 with features like QMC or Ethernet, it is necessary to relocate SPI
parameter RAM in a free location in the CPM dual port RAM. With this patch,
when CONFIG_CPM1_RELOCSPI is set, the parameter RAM for SPI is dynamically
allocated with cpm_muram_alloc().

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 arch/powerpc/platforms/8xx/Kconfig | 11 +++++++++++
 drivers/spi/spi-fsl-cpm.c          |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/arch/powerpc/platforms/8xx/Kconfig b/arch/powerpc/platforms/8xx/Kconfig
index 7c1690b..3b76a3f 100644
--- a/arch/powerpc/platforms/8xx/Kconfig
+++ b/arch/powerpc/platforms/8xx/Kconfig
@@ -203,4 +203,15 @@ config UCODE_PATCH
 	default y
 	depends on !NO_UCODE_PATCH
 
+config CPM1_RELOCSPI
+	bool "Dynamic SPI relocation"
+	default n
+	help
+	  On recent MPC8xx (at least MPC866 and MPC885) SPI can be relocated
+	  without micropatch. This activates relocation to a dynamically
+	  allocated area in the CPM Dual port RAM.
+	  When combined with SPI relocation patch (for older MPC8xx) it avoids
+	  the "loss" of additional Dual port RAM space just above the patch,
+	  which might be needed for example when using the CPM QMC.
+
 endmenu
diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 0f3a912..f33820e 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -262,6 +262,10 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
 		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
 		out_be16(spi_base, pram_ofs);
 	} else {
+#ifdef CONFIG_CPM1_RELOCSPI
+		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 32);
+		out_be16(spi_base, pram_ofs);
+#else
 		u16 rpbase = in_be16(spi_base);
 
 		/* Microcode relocation patch applied | rpbase set by default */
@@ -271,6 +275,7 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
 			pram_ofs = offsetof(cpm8xx_t, cp_dparam[PROFF_SPI]) -
 				   offsetof(cpm8xx_t, cp_dpmem[0]);
 		}
+#endif
 	}
 
 	iounmap(spi_base);
-- 
2.1.0

^ permalink raw reply related

* [PATCH 1/2] spi: fsl-spi: Fix parameter ram offset setup for CPM1
From: Christophe Leroy @ 2014-10-03 12:56 UTC (permalink / raw)
  To: Mark Brown
  Cc: Marcelo Tosatti, linux-kernel, linux-spi, Paul Mackerras,
	scottwood, linuxppc-dev

On CPM1, the SPI parameter RAM has a default location. In fsl_spi_cpm_get_pram()
there was a confusion between the SPI_BASE register and the base of the SPI
parameter RAM. Fortunatly, it was working properly with MPC866 and MPC885
because they do set SPI_BASE, but on MPC860 and other old MPC8xx that doesn't
set SPI_BASE, pram_ofs was not properly set. This patch fixes this confusion.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 drivers/spi/spi-fsl-cpm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..0f3a912 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -262,15 +262,14 @@ static unsigned long fsl_spi_cpm_get_pram(struct mpc8xxx_spi *mspi)
 		pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
 		out_be16(spi_base, pram_ofs);
 	} else {
-		struct spi_pram __iomem *pram = spi_base;
-		u16 rpbase = in_be16(&pram->rpbase);
+		u16 rpbase = in_be16(spi_base);
 
-		/* Microcode relocation patch applied? */
+		/* Microcode relocation patch applied | rpbase set by default */
 		if (rpbase) {
 			pram_ofs = rpbase;
 		} else {
-			pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
-			out_be16(spi_base, pram_ofs);
+			pram_ofs = offsetof(cpm8xx_t, cp_dparam[PROFF_SPI]) -
+				   offsetof(cpm8xx_t, cp_dpmem[0]);
 		}
 	}
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH 0/2] fsl: fsl-spi: Fix CMP1 pram allocation and allow dynamic allocation
From: Christophe Leroy @ 2014-10-03 12:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Mark Brown,
	Marcelo Tosatti, Vitaly Bordug
  Cc: scottwood, linuxppc-dev, linux-kernel, linux-spi

This patchset:
1) Fix parameter ram offset setup for CPM1
2) Allow dynamic allocation of CPM1 parameter RAM

Tested on MPC885.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 linux/arch/powerpc/platforms/8xx/Kconfig | 11 +++++++++++
 linux/drivers/spi/spi-fsl-cpm.c          | 14 +++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

^ permalink raw reply

* Re: [PATCH] powerpc: fix sys_call_table declaration
From: Romeo Cane @ 2014-10-03 10:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1412285674.28143.35.camel@pasglop>

On Fri, Oct 03, 2014 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2014-10-02 at 15:41 +0100, Romeo Cane wrote:
> > Declaring sys_call_table as a pointer causes the compiler to generate the wrong lookup code in arch_syscall_addr
> 
> Care to elaborate ?
> 
> Ben.
> 
> > Signed-off-by: Romeo Cane <romeo.cane.ext@coriant.com>
> > ---
> >  arch/powerpc/include/asm/syscall.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> > index b54b2ad..528ba9d 100644
> > --- a/arch/powerpc/include/asm/syscall.h
> > +++ b/arch/powerpc/include/asm/syscall.h
> > @@ -17,7 +17,7 @@
> >  
> >  /* ftrace syscalls requires exporting the sys_call_table */
> >  #ifdef CONFIG_FTRACE_SYSCALLS
> > -extern const unsigned long *sys_call_table;
> > +extern const unsigned long sys_call_table[];
> >  #endif /* CONFIG_FTRACE_SYSCALLS */
> >  
> >  static inline long syscall_get_nr(struct task_struct *task,
> 
>

Hi Ben,

this is the arch_syscall_addr function from kernel/trace/trace_syscalls.c:

unsigned long __init __weak arch_syscall_addr(int nr)
{
    return (unsigned long)sys_call_table[nr];
}

on my platform (E500MC) the generated assembly code is as follows:

without the patch:
  <arch_syscall_addr>:
     lis     r9,-16384
     rlwinm  r3,r3,2,0,29
     lwz     r11,30640(r9)
     lwzx    r3,r11,r3
     blr

with the patch:
  <arch_syscall_addr>:
     lis     r9,-16384
     rlwinm  r3,r3,2,0,29
     addi    r9,r9,30640
     lwzx    r3,r9,r3
     blr


the goal of the function is to retrieve the n-th element of the table (i.e. the address of a syscall)
Without the patch, the returned value is in fact the memory content pointed by the address of the first syscall plus an offset, that is not what we want.
The consequence is that ftrace of syscalls doesn't work.

That table has always been declared as a pointer since the support for syscalls tracing has been introduced for powerpc years ago, so I'm wondering why nobody else had this problem before.
Other architectures are not affected since in their includes the table is already declared as an array.


Romeo
 

^ permalink raw reply

* [PATCH v2] powerpc/vphn: fix endian issue in NUMA device node code
From: Greg Kurz @ 2014-10-03  9:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nishanth Aravamudan

The associativity domain numbers are obtained from the hypervisor through
registers and written into memory by the guest: the packed array passed to
vphn_unpack_associativity() is then native-endian, unlike what was assumed
in the following commit:

commit b08a2a12e44eaec5024b2b969f4fcb98169d1ca3
Author: Alistair Popple <alistair@popple.id.au>
Date:   Wed Aug 7 02:01:44 2013 +1000

    powerpc: Make NUMA device node code endian safe

If a CPU home node changes, the topology gets filled with
bogus values. This leads to severe performance breakdowns.

This patch does two things:
- extract values from the packed array with shifts, in order to be endian
  neutral
- convert the resulting values to be32 as expected

Suggested-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---

Changes in v2:
- removed the left out __be16 *field declaration
- removed the left out be16_to_cpup() call
- updated the comment of the magic formula

Thanks again Nish... the two left outs probably explain why PowerVM wasn't
happy that patch. :P

--
Greg

 arch/powerpc/mm/numa.c |   64 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b835bf0..4547f91 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1369,38 +1369,78 @@ static int update_cpu_associativity_changes_mask(void)
 #define VPHN_ASSOC_BUFSIZE (6*sizeof(u64)/sizeof(u32) + 1)
 
 /*
+ * The associativity values are either 16-bit (VPHN_FIELD_MSB) or 32-bit (data
+ * or VPHN_FIELD_UNUSED). We hence need to parse the packed array into 16-bit
+ * chunks. Let's do that with bit shifts to be endian neutral.
+ *
+ *    --- 16-bit chunks -->
+ *  _________________________
+ *  |  0  |  1  |  2  |  3  |   packed[0]
+ *  -------------------------
+ *  _________________________
+ *  |  4  |  5  |  6  |  7  |   packed[1]
+ *  -------------------------
+ *            ...
+ *  _________________________
+ *  | 20  | 21  | 22  | 23  |   packed[5]
+ *  -------------------------
+ *       48    32    16     0
+ *    <------ bits -------- 
+ *
+ * We need 48-bit shift for chunks 0,4,8,16,20
+ *         32-bit shift for chunks 1,5,9,17,21
+ *         16-bit shift for chunks 2,6,10,18,22
+ *             no shift for chunks 3,7,11,19,23
+ *
+ * The 2 lo bits of the chunk index multiplied by 16 give the shift.
+ * The remaining hi bits divided by 4 give the index in packed[].
+ *
+ * For example:
+ * chunk  0 = packed[0/4]  >> (~0b00000 & 0b11) * 16 = bits 48..63 in packed[0]
+ * chunk  5 = packed[5/4]  >> (~0b00101 & 0b11) * 16 = bits 32..47 in packed[1]
+ * chunk 22 = packed[22/4] >> (~0b10110 & 0b11) * 16 = bits 16..31 in packed[5]
+ */
+static inline u16 read_vphn_chunk(const long *packed, unsigned int i)
+{
+	return packed[i >> 2] >> ((~i & 3) << 4);
+}
+
+/*
  * Convert the associativity domain numbers returned from the hypervisor
  * to the sequence they would appear in the ibm,associativity property.
  */
 static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
 {
-	int i, nr_assoc_doms = 0;
-	const __be16 *field = (const __be16 *) packed;
+	unsigned int i, j, nr_assoc_doms = 0;
 
 #define VPHN_FIELD_UNUSED	(0xffff)
 #define VPHN_FIELD_MSB		(0x8000)
 #define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
 
-	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
-		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
+	for (i = 1, j = 0; i < VPHN_ASSOC_BUFSIZE; i++) {
+		u16 field = read_vphn_chunk(packed, j);
+
+		if (field == VPHN_FIELD_UNUSED) {
 			/* All significant fields processed, and remaining
 			 * fields contain the reserved value of all 1's.
 			 * Just store them.
 			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
-		} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
+			unpacked[i] = (VPHN_FIELD_UNUSED << 16 |
+				       VPHN_FIELD_UNUSED);
+			j += 2;
+		} else if (field & VPHN_FIELD_MSB) {
 			/* Data is in the lower 15 bits of this field */
-			unpacked[i] = cpu_to_be32(
-				be16_to_cpup(field) & VPHN_FIELD_MASK);
-			field++;
+			unpacked[i] = cpu_to_be32(field & VPHN_FIELD_MASK);
+			j++;
 			nr_assoc_doms++;
 		} else {
 			/* Data is in the lower 15 bits of this field
 			 * concatenated with the next 16 bit field
 			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
+			unpacked[i] =
+				cpu_to_be32((u32) field << 16 |
+					    read_vphn_chunk(packed, j + 1));
+			j += 2;
 			nr_assoc_doms++;
 		}
 	}

^ permalink raw reply related

* Re: [PATCH 1/3] powerpc: Don't build powernv for other platform defconfigs
From: Michael Ellerman @ 2014-10-03  8:04 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20140924163410.4f1ad720@canb.auug.org.au>

On Wed, 2014-09-24 at 16:34 +1000, Stephen Rothwell wrote:
> Hi Michael,
> 
> On Wed, 24 Sep 2014 15:57:10 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Because powernv arrived after these other platforms, the defconfigs
> > didn't have PPC_POWERNV disabled, and being default y it gets turned on.
> 
> Well, that raises the question of why PPC_POWERNV is default y at all?

I think that makes sense. With a bare config, you want all the platforms that
can be built to build, so that the kernel you produce is able to boot on as
many things as possible.

If you want a stripped down config for a particular target then we have
defconfigs that do that.

cheers

^ permalink raw reply

* Re: powerpc/book3s: Fix flush_tlb cpu_spec hook to take a generic argument.
From: Benjamin Herrenschmidt @ 2014-10-03  8:04 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Mahesh Salgaonkar, Paul Mackerras, linuxppc-dev
In-Reply-To: <20141003073622.3F0B8140180@ozlabs.org>

On Fri, 2014-10-03 at 17:36 +1000, Michael Ellerman wrote:
> It seems that the only detail we are abstracting here is that power7 has 128
> TLB sets and power8 has 512. Is that right?

Today ...

> It seems like a lot of code to deal with that one detail.
> 
> I guess the other thing it does is cur_cpu_spec->flush_tlb() is only
> implemented on power7 and power8, ie. not power6. But it seems that the two
> callers can only ever run on power7/power8 anway.
> 
> So couldn't we just do:
>  - add the number of tlb_sets to cpu_spec
>  - use that in flush_tlb()

Provided the form of tlbiel we use remains ... things are different from
embedded for example. I've asked Mahesh to add this hook with the idea
somewhat to migrate existing/older code to use it as well, and on older
chips or embedded the TLB flush is quite different.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 3/3] powerpc/kdump: crash_dump.c needs to include io.h
From: Michael Ellerman @ 2014-10-03  8:04 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20140924163657.74b23785@canb.auug.org.au>

On Wed, 2014-09-24 at 16:36 +1000, Stephen Rothwell wrote:
> Hi Michael,
> 
> On Wed, 24 Sep 2014 15:57:12 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > For __ioremap().
> 
> So does that mean that you really want this patch before 2/3 so that you
> don't introduce an unnecessary bisection breakage in ppc64_defconfig?

Yeah I guess it does.

cheers

^ permalink raw reply

* Re: powerpc/book3s: Fix flush_tlb cpu_spec hook to take a generic argument.
From: Michael Ellerman @ 2014-10-03  7:36 UTC (permalink / raw)
  To: Mahesh Salgaonkar, linuxppc-dev, Paul Mackerras,
	Benjamin Herrenschmidt
In-Reply-To: <20140923035330.6097.43515.stgit@mars>

On Tue, 2014-23-09 at 03:53:54 UTC, Mahesh Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The flush_tlb hook in cpu_spec was introduced as a generic function hook
> to invalidate TLBs. But the current implementation of flush_tlb hook
> takes IS (invalidation selector) as an argument which is architecture
> dependent. Hence, It is not right to have a generic routine where caller
> has to pass non-generic argument.
> 
> This patch fixes this and makes flush_tlb hook as high level API.
> 
> The old code used to call flush_tlb hook with IS=0 (single page) resulting
> partial invalidation of TLBs which is not right. This fix now makes
> sure that whole TLB is invalidated to be able to successfully recover from
> TLB and ERAT errors.

Hi Mahesh,

It seems that the only detail we are abstracting here is that power7 has 128
TLB sets and power8 has 512. Is that right?

It seems like a lot of code to deal with that one detail.

I guess the other thing it does is cur_cpu_spec->flush_tlb() is only
implemented on power7 and power8, ie. not power6. But it seems that the two
callers can only ever run on power7/power8 anway.

So couldn't we just do:
 - add the number of tlb_sets to cpu_spec
 - use that in flush_tlb()

cheers

^ permalink raw reply

* [PATCH] powerpc/powernv: Fix endian bug in LPC bus debugfs accessors
From: Benjamin Herrenschmidt @ 2014-10-03  7:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Anton Blanchard

When reading from the LPC, the OPAL FW calls return the value via pointer
to a uint32_t which is always returned big endian. Our internal inb/outb
implementation byteswaps that fine but our debugfs code is still broken.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---

diff --git a/arch/powerpc/platforms/powernv/opal-lpc.c b/arch/powerpc/platforms/powernv/opal-lpc.c
index ad4b31d..dd2c285 100644
--- a/arch/powerpc/platforms/powernv/opal-lpc.c
+++ b/arch/powerpc/platforms/powernv/opal-lpc.c
@@ -191,6 +191,7 @@ static ssize_t lpc_debug_read(struct file *filp, char __user *ubuf,
 {
 	struct lpc_debugfs_entry *lpc = filp->private_data;
 	u32 data, pos, len, todo;
+	__be32 bedata;
 	int rc;
 
 	if (!access_ok(VERIFY_WRITE, ubuf, count))
@@ -213,9 +214,10 @@ static ssize_t lpc_debug_read(struct file *filp, char __user *ubuf,
 				len = 2;
 		}
 		rc = opal_lpc_read(opal_lpc_chip_id, lpc->lpc_type, pos,
-				   &data, len);
+				   &bedata, len);
 		if (rc)
 			return -ENXIO;
+		data = be32_to_cpu(bedata);
 		switch(len) {
 		case 4:
 			rc = __put_user((u32)data, (u32 __user *)ubuf);

^ permalink raw reply related

* Re: [PATCH] [RFC] powerpc/vphn: fix endian issue in NUMA device node code
From: Greg Kurz @ 2014-10-03  6:57 UTC (permalink / raw)
  To: Nishanth Aravamudan; +Cc: linuxppc-dev
In-Reply-To: <20141002234340.GB20262@linux.vnet.ibm.com>

On Thu, 2 Oct 2014 16:43:41 -0700
Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:

> On 02.10.2014 [23:59:15 +0200], Greg Kurz wrote:
> > The associativity domain numbers are obtained from the hypervisor through
> > registers and written into memory by the guest: the packed array passed to
> > vphn_unpack_associativity() is then native-endian, unlike what was assumed
> > in the following commit:
> > 
> > commit b08a2a12e44eaec5024b2b969f4fcb98169d1ca3
> > Author: Alistair Popple <alistair@popple.id.au>
> > Date:   Wed Aug 7 02:01:44 2013 +1000
> > 
> >     powerpc: Make NUMA device node code endian safe
> > 
> > If a CPU home node changes, the topology gets filled with
> > bogus values. This leads to severe performance breakdowns.
> > 
> > This patch does two things:
> > - extract values from the packed array with shifts, in order to be endian
> >   neutral
> > - convert the resulting values to be32 as expected
> > 
> > Suggested-by: Anton Blanchard <anton@samba.org>
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> > 
> > I could test this code in a userland program and obtain the same
> > result in little and big endian. If the 64-bit packed values
> > are:
> > 
> > 0x8001ffffffff0000
> > 0x112200003344ffff
> > 0xffff000055660000
> > 0x7788000099aaffff
> > 0xffff8002ffffffff
> > 0xffffffffffffffff
> > 
> > then the unpacked array is:
> > 
> > 0x00000007
> > 0x00000001
> > 0xffffffff
> > 0x00001122
> > 0x00003344
> > 0xffffffff
> > 0x00005566
> > 0x00007788
> > 0x000099aa
> > 0xffffffff
> > 0x00000002
> > 0xffffffff
> > 0xffffffff
> > 
> > I could not test in a PowerVM guest though, hence the RFC.
> > 
> > --
> > Greg
> > 
> >  arch/powerpc/mm/numa.c |   50 ++++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 40 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index b835bf0..06af179 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -1369,38 +1369,68 @@ static int update_cpu_associativity_changes_mask(void)
> >  #define VPHN_ASSOC_BUFSIZE (6*sizeof(u64)/sizeof(u32) + 1)
> >  
> >  /*
> > + * The associativity values are either 16-bit (VPHN_FIELD_MSB) or 32-bit (data
> > + * or VPHN_FIELD_UNUSED). We hence need to parse the packed array into 16-bit
> > + * chunks. Let's do that with bit shifts to be endian neutral.
> > + *
> > + *    --- 16-bit chunks -->
> > + *  _________________________
> > + *  |  0  |  1  |  2  |  3  |   packed[0]
> > + *  -------------------------
> > + *  _________________________
> > + *  |  4  |  5  |  6  |  7  |   packed[1]
> > + *  -------------------------
> > + *            ...
> > + *  _________________________
> > + *  | 20  | 21  | 22  | 23  |   packed[5]
> > + *  -------------------------
> > + *
> > + *   >>48  >>32  >>16  >>0      <-- needed bit shift
> > + *
> > + * We only care for the 2 lower bits of the chunk index to compute the shift.
> > + */
> > +static inline u16 read_vphn_chunk(const long *packed, unsigned int i)
> > +{
> > +	return packed[i >> 2] >> ((~i & 3) << 4);
> 
> This is some excellent magic and the comment *should* be sufficient, but
> maybe an example would be good? i is the index we want to read from in
> the packed array?
> 

Yes, i is the index of the 16-bit chunk we want to read. I will add
numerical examples:

chunk #0 is (u16) packed[0] >> 48
chunk #1 is (u16) packed[0] >> 32
chunk #2 is (u16) packed[0] >> 16
chunk #3 is (u16) packed[0] >> 0
chunk #4 is (u16) packed[1] >> 48
chunk #5 is (u16) packed[1] >> 32
...
chunk #22 is (u16) packed[1] >> 16
chunk #23 is (u16) packed[1] >> 0


> > +}
> > +
> > +/*
> >   * Convert the associativity domain numbers returned from the hypervisor
> >   * to the sequence they would appear in the ibm,associativity property.
> >   */
> >  static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
> >  {
> > -	int i, nr_assoc_doms = 0;
> > +	unsigned int i, j, nr_assoc_doms = 0;
> >  	const __be16 *field = (const __be16 *) packed;
> >  
> >  #define VPHN_FIELD_UNUSED	(0xffff)
> >  #define VPHN_FIELD_MSB		(0x8000)
> >  #define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
> >  
> > -	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
> > -		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
> > +	for (i = 1, j = 0; i < VPHN_ASSOC_BUFSIZE; i++) {
> > +		u16 field = read_vphn_chunk(packed, j);
> 
> Maybe I'm super dense here, but isn't there are already a variable named
> field in this context? With a different annotation?
> 

Oops ! The u16 one is supposed to supersede the __be16 * one :)

-       const __be16 *field = (const __be16 *) packed;

> > +
> > +		if (field == VPHN_FIELD_UNUSED) {
> >  			/* All significant fields processed, and remaining
> >  			 * fields contain the reserved value of all 1's.
> >  			 * Just store them.
> >  			 */
> > -			unpacked[i] = *((__be32 *)field);
> > -			field += 2;
> > +			unpacked[i] = (VPHN_FIELD_UNUSED << 16 |
> > +				       VPHN_FIELD_UNUSED);
> > +			j += 2;
> >  		} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
> 
> Why do we need to do be16_to_cpup(field) here if field is now a u16? Or
> more explicitly, if we don't performe be16_to_cpup(field) anywhere else
> in this function now, why do we need to here?

Same as above ^^

-               } else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
+               } else if (field & VPHN_FIELD_MSB) {

Bad copy/paste from the prototype code... :\

Thanks for the review.

> 
> >  			/* Data is in the lower 15 bits of this field */
> > -			unpacked[i] = cpu_to_be32(
> > -				be16_to_cpup(field) & VPHN_FIELD_MASK);
> > -			field++;
> > +			unpacked[i] = cpu_to_be32(field & VPHN_FIELD_MASK);
> > +			j++;
> >  			nr_assoc_doms++;
> >  		} else {
> >  			/* Data is in the lower 15 bits of this field
> >  			 * concatenated with the next 16 bit field
> >  			 */
> > -			unpacked[i] = *((__be32 *)field);
> > -			field += 2;
> > +			unpacked[i] =
> > +				cpu_to_be32((u32) field << 16 |
> > +					    read_vphn_chunk(packed, j + 1));
> > +			j += 2;
> >  			nr_assoc_doms++;
> >  		}
> >  	}
> > 
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [1/4] powerpc/powernv: Sync header with firmware
From: Gavin Shan @ 2014-10-03  6:40 UTC (permalink / raw)
  To: Stewart Smith; +Cc: qiudayu, linuxppc-dev, Gavin Shan
In-Reply-To: <m3a95dhg94.fsf@oc8180480414.ibm.com>

On Fri, Oct 03, 2014 at 03:30:31PM +1000, Stewart Smith wrote:
>Michael Ellerman <mpe@ellerman.id.au> writes:
>>> +	OpalEjtIoaDmaWriteMemTarget	= 19,
>>> +};
>>
>> I realise these come from the skiboot source, but they're just too ugly.
>>
>> Please use kernel style naming, like most of the rest of the file, eg:
>>
>> 	OPAL_ERR_INJECT_IOA_BUS_ERR
>
>You know what, I think I'd feel better if we changed skiboot source to
>be like this too. Many of the other enums in skiboot are kernel style
>and not camel. So, I'm going to go do that, then kernel and firmware can
>match.

Yes, for the PCI error types/functions, I sent one patch to make skiboot
looks same to kernel yesterday.

Thanks,
Gavin

^ permalink raw reply

* Re: [1/4] powerpc/powernv: Sync header with firmware
From: Stewart Smith @ 2014-10-03  5:30 UTC (permalink / raw)
  To: Michael Ellerman, Gavin Shan, linuxppc-dev; +Cc: qiudayu, Gavin Shan
In-Reply-To: <20140925042747.8E2AE14016A@ozlabs.org>

Michael Ellerman <mpe@ellerman.id.au> writes:
>> +	OpalEjtIoaDmaWriteMemTarget	= 19,
>> +};
>
> I realise these come from the skiboot source, but they're just too ugly.
>
> Please use kernel style naming, like most of the rest of the file, eg:
>
> 	OPAL_ERR_INJECT_IOA_BUS_ERR

You know what, I think I'd feel better if we changed skiboot source to
be like this too. Many of the other enums in skiboot are kernel style
and not camel. So, I'm going to go do that, then kernel and firmware can
match.

^ permalink raw reply

* Re: [PATCH v2 3/4] sound/radeon: Move 64-bit MSI quirk from arch to driver
From: Benjamin Herrenschmidt @ 2014-10-03  5:23 UTC (permalink / raw)
  To: Alex Deucher
  Cc: linuxppc-dev, Dave Airlie, Linux PCI, Anton Blanchard, Brian King,
	Yijing Wang, Takashi Iwai, Bjorn Helgaas
In-Reply-To: <CADnq5_PhuFbRPgzAa_hEA1czTQjY9xdLFOWhL393DN4CYDnFng@mail.gmail.com>

On Wed, 2014-10-01 at 22:13 -0400, Alex Deucher wrote:

> The attached updated patch only flags the affected asics.

I need formal (email :-) permission to add your s-o-b (and From: while
at it since you wrote most of it) for this patch. I'll then include
it in my new series.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] powerpc/eeh: Block CFG upon frozen Shiner adapter
From: Gavin Shan @ 2014-10-03  4:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

The Broadcom Shiner 2-ports 10G ethernet adapter has same problem
commit 6f20bda0 ("powerpc/eeh: Block PCI config access upon frozen
PE") fixes. Put it to the black list as well.

   # lspci -s 0004:01:00.0
   0004:01:00.0 Ethernet controller: Broadcom Corporation \
                NetXtreme II BCM57810 10 Gigabit Ethernet (rev 10)
   # lspci -n -s 0004:01:00.0
   0004:01:00.0 0200: 14e4:168e (rev 10)

Reported-by: John Walthour <jwalthour@us.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 443ce96..1d19e79 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -182,9 +182,10 @@ static int powernv_eeh_dev_probe(struct pci_dev *dev, void *flag)
 	 * that PE to block its config space.
 	 *
 	 * Broadcom Austin 4-ports NICs (14e4:1657)
+	 * Broadcom Shiner 2-ports 10G NICs (14e4:168e)
 	 */
-	if (dev->vendor == PCI_VENDOR_ID_BROADCOM &&
-	    dev->device == 0x1657)
+	if ((dev->vendor == PCI_VENDOR_ID_BROADCOM && dev->device == 0x1657) ||
+	    (dev->vendor == PCI_VENDOR_ID_BROADCOM && dev->device == 0x168e))
 		edev->pe->state |= EEH_PE_CFG_RESTRICTED;
 
 	/*
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off
From: Michael Ellerman @ 2014-10-03  4:42 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Arnd Bergmann, Geoff Levand, Alistair Popple, Scott Wood,
	Anatolij Gustschin, linuxppc-dev
In-Reply-To: <1412170086-57971-1-git-send-email-agraf@suse.de>

On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
> The generic Linux framework to power off the machine is a function pointer
> called pm_power_off. The trick about this pointer is that device drivers can
> potentially implement it rather than board files.
> 
> Today on PowerPC we set pm_power_off to invoke our generic full machine power
> off logic which then calls ppc_md.power_off to invoke machine specific power
> off.
> 
> To fix this up, let's get rid of the ppc_md.power_off logic and just always use
> pm_power_off as was intended. Then individual drivers such as the GPIO power off
> driver can implement power off logic via that function pointer.

This looks OK to me with one caveat.

In several of the patches you're replacing a static initialisation with a
runtime one, and you're doing the runtime initialisation in xxx_setup_arch().
That's reasonably late, so I'd prefer you did it in xxx_probe().

If you resend with that changed I'll put it in next.

cheers

^ permalink raw reply

* Re: [PATCH v3] powerpc/iommu/ddw: Fix endianness
From: Anton Blanchard @ 2014-10-03  4:21 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: devicetree, linuxppc-dev, linux-kernel
In-Reply-To: <1411627167-22927-1-git-send-email-aik@ozlabs.ru>

Hi Alexey,

> rtas_call() accepts and returns values in CPU endianness.
> The ddw_query_response and ddw_create_response structs members are
> defined and treated as BE but as they are passed to rtas_call() as
> (u32 *) and they get byteswapped automatically, the data is
> CPU-endian. This fixes ddw_query_response and ddw_create_response
> definitions and use.
> 
> of_read_number() is designed to work with device tree cells - it
> assumes the input is big-endian and returns data in CPU-endian.
> However due to the ddw_create_response struct fix, create.addr_hi/lo
> are already CPU-endian so do not byteswap them.
> 
> ddw_avail is a pointer to the "ibm,ddw-applicable" property which
> contains 3 cells which are big-endian as it is a device tree.
> rtas_call() accepts a RTAS token in CPU-endian. This makes use of
> of_property_read_u32_array to byte swap and avoid the need for a
> number of be32_to_cpu calls.
> 
> Cc: stable@vger.kernel.org # v3.13
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Anton Blanchard <anton@samba.org>
> [aik: folded Anton's patch with of_property_read_u32_array]
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Thanks for updating, looks good. Could we make it clear the bug is
present in 3.13-3.17 with:

Cc: stable@vger.kernel.org # v3.13+

Acked-by: Anton Blanchard <anton@samba.org>

Anton

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox