* Re: drivers/tty: ehv_bytechan fails to build as a module
From: Guenter Roeck @ 2014-01-04 19:15 UTC (permalink / raw)
To: Anton Blanchard; +Cc: gregkh, dvaleev, paulus, linuxppc-dev, timur
In-Reply-To: <20131209160310.7f81a1bc@kryten>
On Mon, Dec 09, 2013 at 04:03:10PM +1100, Anton Blanchard wrote:
> ehv_bytechan is marked tristate but fails to build as a module:
>
> drivers/tty/ehv_bytechan.c:363:1: error: type defaults to ‘int’ in declaration of ‘console_initcall’ [-Werror=implicit-int]
>
> It doesn't make much sense for a console driver to be built as
> a module, so change it to a bool.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
>
Any comments on this patch ? The problem still causes powerpc:allmodconfig to
fail.
Guenter
> ---
>
>
> Index: b/drivers/tty/Kconfig
> ===================================================================
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -366,7 +366,7 @@ config TRACE_SINK
> "Trace data router for MIPI P1149.7 cJTAG standard".
>
> config PPC_EPAPR_HV_BYTECHAN
> - tristate "ePAPR hypervisor byte channel driver"
> + bool "ePAPR hypervisor byte channel driver"
> depends on PPC
> select EPAPR_PARAVIRT
> help
^ permalink raw reply
* Re: [v3, 3/7] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Kevin Hao @ 2014-01-04 6:34 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc
In-Reply-To: <1388796549.11795.100.camel@snotra.buserror.net>
[-- Attachment #1: Type: text/plain, Size: 5906 bytes --]
On Fri, Jan 03, 2014 at 06:49:09PM -0600, Scott Wood wrote:
> On Fri, 2013-12-20 at 15:43 +0800, Kevin Hao wrote:
> > On Wed, Dec 18, 2013 at 05:48:25PM -0600, Scott Wood wrote:
> > > On Wed, Aug 07, 2013 at 09:18:31AM +0800, Kevin Hao wrote:
> > > > This is based on the codes in the head_44x.S. The difference is that
> > > > the init tlb size we used is 64M. With this patch we can only load the
> > > > kernel at address between memstart_addr ~ memstart_addr + 64M. We will
> > > > fix this restriction in the following patches.
> > >
> > > Which following patch fixes the restriction? With all seven patches
> > > applied, I was still only successful booting within 64M of memstart_addr.
> >
> > There is bug in this patch series when booting above the 64M. It seems
> > that I missed to test this previously. Sorry for that. With the following
> > change I can boot the kernel at 0x5000000.
>
> I tried v4 and it still doesn't work for me over 64M (without increasing
> the start of memory). I pulled the following out of the log buffer when
> booting at 0x5000000 (after cleaning up the binary goo -- is that
> something new?):
>
> Unable to handle kernel paging request for data at address 0xbffe4008
Actually there still have one limitation that we have to make sure
that the kernel and dtb are in the 64M memory mapped by the init tlb entry.
I booted the kernel successfully by using the following u-boot commands:
setenv fdt_high 0xffffffff
dhcp 6000000 128.224.162.196:/vlm-boards/p5020/uImage
tftp 6f00000 128.224.162.196:/vlm-boards/p5020/p5020ds.dtb
bootm 6000000 - 6f00000
> Faulting instruction address: 0xc16ee934
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=8
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 3.13.0-rc1-00065-g422752a #12
> task: c18f2340 ti: c192c000 task.ti: c192c000
> NIP: c16ee934 LR: c16d1860 CTR: c100e96c
> REGS: c192dee0 TRAP: 0300 Not tainted (3.13.0-rc1-00065-g422752a)
> MSR: 00021002 <CE,ME> CR: 42044022 XER: 20000000
> DEAR: bffe4008 ESR: 00000000
> GPR00: c16d1860 c192df90 c18f2340 c16eec58 00000000 00000000 05000000 00000000
> GPR08: 00000000 bffe4000 00000000 bc000000 00000000 0570ad40 ffffffff 7ffb0aec
> GPR16: 00000000 00000000 7fe4dd70 00000000 7fe4ddb0 00000000 c192c000 00000007
> GPR24: 00000000 c1940000 c16eec58 00000000 ffffffff 03fe4000 00000000 c18f0000
> NIP [c16ee934] of_scan_flat_dt+0x28/0x148
> LR [c16d1860] early_get_first_memblock_info+0x38/0x84
> Call Trace:
> [c192dfc0] [c16d1860] early_get_first_memblock_info+0x38/0x84
> [c192dfd0] [c16d4888] relocate_init+0x98/0x160
> [c192dff0] [c100045c] set_ivor+0x144/0x190
> Instruction dump:
> 7c0803a6 4e800020 9421ffd0 7c0802a6 bee1000c 3f20c194 8139a45c 7c7a1b78
> 90010034 7c9b2378 3b80ffff 3ae00007 <83e90008> 3b00fff8 7fe9fa14 48000008
> ---[ end trace 41ed10ed80b8d831 ]---
> Kernel panic - not syncing: Attempted to kill the idle task!
>
>
> > diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
> > index 048d716ae706..ce0c7d7db6c3 100644
> > --- a/arch/powerpc/mm/fsl_booke_mmu.c
> > +++ b/arch/powerpc/mm/fsl_booke_mmu.c
> > @@ -171,11 +171,10 @@ unsigned long calc_cam_sz(unsigned long ram, unsigned long virt,
> > return 1UL << camsize;
> > }
> >
> > -unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> > +static unsigned long map_mem_in_cams_addr(phys_addr_t phys, unsigned long virt,
> > + unsigned long ram, int max_cam_idx)
> > {
> > int i;
> > - unsigned long virt = PAGE_OFFSET;
> > - phys_addr_t phys = memstart_addr;
> > unsigned long amount_mapped = 0;
> >
> > /* Calculate CAM values */
> > @@ -195,6 +194,14 @@ unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> > return amount_mapped;
> > }
> >
> > +unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> > +{
> > + unsigned long virt = PAGE_OFFSET;
> > + phys_addr_t phys = memstart_addr;
> > +
> > + return map_mem_in_cams_addr(phys, virt, ram, max_cam_idx);
> > +}
> > +
> > #ifdef CONFIG_PPC32
> >
> > #if defined(CONFIG_LOWMEM_CAM_NUM_BOOL) && (CONFIG_LOWMEM_CAM_NUM >= NUM_TLBCAMS)
> > @@ -289,7 +296,11 @@ notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
> > is_second_reloc = 1;
> > n = switch_to_as1();
> > /* map a 64M area for the second relocation */
> > - map_mem_in_cams(0x4000000UL, CONFIG_LOWMEM_CAM_NUM);
> > + if (memstart_addr > start)
> > + map_mem_in_cams(0x4000000, CONFIG_LOWMEM_CAM_NUM);
> > + else
> > + map_mem_in_cams_addr(start, PAGE_OFFSET - offset,
> > + 0x4000000, CONFIG_LOWMEM_CAM_NUM);
> > restore_to_as0(n, offset, __va(dt_ptr));
> > /* We should never reach here */
> > panic("Relocation error");
> >
>
> I'm having a hard time following the logic here. What is PAGE_OFFSET -
> offset supposed to be? Why would we map anything belowe PAGE_OFFSET?
No, we don't map the address below PAGE_OFFSET.
memstart_addr is the physical start address of RAM.
start is the kernel running physical address aligned with 64M.
offset = memstart_addr - start
So if memstart_addr < start, the offset is negative. The PAGE_OFFSET - offset
is the virtual start address we should use for the init 64M map. It's above
the PAGE_OFFSET instead of below.
For example, if we boot the kernel at 0x5000000:
memstart_addr = 0x0
start = 0x4000000
offset = -0x4000000
PAGE_OFFSET - offset = 0xc4000000.
Then we should create a 64M map for the virtual address from
0xc4000000 ~ 0xc8000000. This is the final virtual address that the kernel
symbols would use.
Thanks,
Kevin
>
> -Scott
>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [v3, 3/7] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Scott Wood @ 2014-01-04 0:49 UTC (permalink / raw)
To: Kevin Hao; +Cc: linuxppc
In-Reply-To: <20131220074339.GA23977@pek-khao-d1.corp.ad.wrs.com>
On Fri, 2013-12-20 at 15:43 +0800, Kevin Hao wrote:
> On Wed, Dec 18, 2013 at 05:48:25PM -0600, Scott Wood wrote:
> > On Wed, Aug 07, 2013 at 09:18:31AM +0800, Kevin Hao wrote:
> > > This is based on the codes in the head_44x.S. The difference is that
> > > the init tlb size we used is 64M. With this patch we can only load the
> > > kernel at address between memstart_addr ~ memstart_addr + 64M. We will
> > > fix this restriction in the following patches.
> >
> > Which following patch fixes the restriction? With all seven patches
> > applied, I was still only successful booting within 64M of memstart_addr.
>
> There is bug in this patch series when booting above the 64M. It seems
> that I missed to test this previously. Sorry for that. With the following
> change I can boot the kernel at 0x5000000.
I tried v4 and it still doesn't work for me over 64M (without increasing
the start of memory). I pulled the following out of the log buffer when
booting at 0x5000000 (after cleaning up the binary goo -- is that
something new?):
Unable to handle kernel paging request for data at address 0xbffe4008
Faulting instruction address: 0xc16ee934
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 3.13.0-rc1-00065-g422752a #12
task: c18f2340 ti: c192c000 task.ti: c192c000
NIP: c16ee934 LR: c16d1860 CTR: c100e96c
REGS: c192dee0 TRAP: 0300 Not tainted (3.13.0-rc1-00065-g422752a)
MSR: 00021002 <CE,ME> CR: 42044022 XER: 20000000
DEAR: bffe4008 ESR: 00000000
GPR00: c16d1860 c192df90 c18f2340 c16eec58 00000000 00000000 05000000 00000000
GPR08: 00000000 bffe4000 00000000 bc000000 00000000 0570ad40 ffffffff 7ffb0aec
GPR16: 00000000 00000000 7fe4dd70 00000000 7fe4ddb0 00000000 c192c000 00000007
GPR24: 00000000 c1940000 c16eec58 00000000 ffffffff 03fe4000 00000000 c18f0000
NIP [c16ee934] of_scan_flat_dt+0x28/0x148
LR [c16d1860] early_get_first_memblock_info+0x38/0x84
Call Trace:
[c192dfc0] [c16d1860] early_get_first_memblock_info+0x38/0x84
[c192dfd0] [c16d4888] relocate_init+0x98/0x160
[c192dff0] [c100045c] set_ivor+0x144/0x190
Instruction dump:
7c0803a6 4e800020 9421ffd0 7c0802a6 bee1000c 3f20c194 8139a45c 7c7a1b78
90010034 7c9b2378 3b80ffff 3ae00007 <83e90008> 3b00fff8 7fe9fa14 48000008
---[ end trace 41ed10ed80b8d831 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
> diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
> index 048d716ae706..ce0c7d7db6c3 100644
> --- a/arch/powerpc/mm/fsl_booke_mmu.c
> +++ b/arch/powerpc/mm/fsl_booke_mmu.c
> @@ -171,11 +171,10 @@ unsigned long calc_cam_sz(unsigned long ram, unsigned long virt,
> return 1UL << camsize;
> }
>
> -unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> +static unsigned long map_mem_in_cams_addr(phys_addr_t phys, unsigned long virt,
> + unsigned long ram, int max_cam_idx)
> {
> int i;
> - unsigned long virt = PAGE_OFFSET;
> - phys_addr_t phys = memstart_addr;
> unsigned long amount_mapped = 0;
>
> /* Calculate CAM values */
> @@ -195,6 +194,14 @@ unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> return amount_mapped;
> }
>
> +unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
> +{
> + unsigned long virt = PAGE_OFFSET;
> + phys_addr_t phys = memstart_addr;
> +
> + return map_mem_in_cams_addr(phys, virt, ram, max_cam_idx);
> +}
> +
> #ifdef CONFIG_PPC32
>
> #if defined(CONFIG_LOWMEM_CAM_NUM_BOOL) && (CONFIG_LOWMEM_CAM_NUM >= NUM_TLBCAMS)
> @@ -289,7 +296,11 @@ notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
> is_second_reloc = 1;
> n = switch_to_as1();
> /* map a 64M area for the second relocation */
> - map_mem_in_cams(0x4000000UL, CONFIG_LOWMEM_CAM_NUM);
> + if (memstart_addr > start)
> + map_mem_in_cams(0x4000000, CONFIG_LOWMEM_CAM_NUM);
> + else
> + map_mem_in_cams_addr(start, PAGE_OFFSET - offset,
> + 0x4000000, CONFIG_LOWMEM_CAM_NUM);
> restore_to_as0(n, offset, __va(dt_ptr));
> /* We should never reach here */
> panic("Relocation error");
>
I'm having a hard time following the logic here. What is PAGE_OFFSET -
offset supposed to be? Why would we map anything belowe PAGE_OFFSET?
-Scott
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/fsl-booke: Add support for T2080/T2081 SoC
From: Scott Wood @ 2014-01-03 22:55 UTC (permalink / raw)
To: Shengzhou Liu; +Cc: linuxppc-dev
In-Reply-To: <1387966018-10131-3-git-send-email-Shengzhou.Liu@freescale.com>
On Wed, 2013-12-25 at 18:06 +0800, Shengzhou Liu wrote:
> +/* controller at 0x270000 */
> +&pci3 {
> + compatible = "fsl,t208x-pcie", "fsl,qoriq-pcie";
Don't use wildcards in compatible strings.
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /*
> + * Temporarily add next-level-cache info in each cpu node so
> + * that uboot can do L2 cache fixup. This can be removed once
> + * u-boot can create cpu node with cache info.
> + */
This sort of thing is why these dts files belong in U-Boot and not the
Linux tree.
-Scott
^ permalink raw reply
* Re: [PATCH 01/12][v3] pci: fsl: derive the common PCI driver to drivers/pci/host
From: Scott Wood @ 2014-01-03 22:37 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911, linux-pci
In-Reply-To: <20131125230116.GA3819@google.com>
On Mon, 2013-11-25 at 16:01 -0700, Bjorn Helgaas wrote:
> On Wed, Oct 23, 2013 at 06:41:23PM +0800, Minghuan Lian wrote:
> > The Freescale's Layerscape series processors will use ARM cores.
> > The LS1's PCIe controllers is the same as T4240's. So it's better
> > the PCIe controller driver can support PowerPC and ARM
> > simultaneously. This patch is for this purpose. It derives
> > the common functions from arch/powerpc/sysdev/fsl_pci.c to
> > drivers/pci/host/pci-fsl-common.c and leaves the architecture
> > specific functions which should be implemented in arch related files.
> >
> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>
> It doesn't look like we have a consensus on how to proceed here, so I'm
> ignoring this series for now. Let me know if there really *is* agreement,
> or if there's some subset of this work that everybody can agree on. It'd
> be good to make forward progress, even if it's not completely perfect yet.
I'd like to see what this would look like with actual ARM support,
rather than just some theoretical attempts to make it less PPC-dependent
that involve copying PPC stuff that may or may not work well with what
ARM code will expect.
When I replied to Kumar's comment saying that this seemed like a
reasonable interim approach, that was before I saw how much was copied
from places other than fsl_pci files.
-Scott
^ permalink raw reply
* Re: [03/12,v3] pci: fsl: add PCI indirect access support
From: Scott Wood @ 2014-01-03 22:33 UTC (permalink / raw)
To: Minghuan Lian; +Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <1382524894-15164-3-git-send-email-Minghuan.Lian@freescale.com>
On Wed, Oct 23, 2013 at 06:41:25PM +0800, Minghuan Lian wrote:
> The patch adds PCI indirect read/write functions. The main code
> is ported from arch/powerpc/sysdev/indirect_pci.c. We use general
> IO API iowrite32be/ioread32be instead of out_be32/in_be32, and
> use structure fsl_Pci instead of PowerPC's pci_controller.
> The patch also provides fsl_pcie_check_link() to check PCI link.
> The weak function fsl_arch_pci_exclude_device() is provided to
> call ppc_md.pci_exclude_device() for PowerPC architecture.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>
> ---
> change log:
> v1-v3:
> Derived from http://patchwork.ozlabs.org/patch/278965/
>
> Based on upstream master.
> Based on the discussion of RFC version here
> http://patchwork.ozlabs.org/patch/274487/
>
> drivers/pci/host/pci-fsl-common.c | 169 ++++++++++++++++++++++++++++++++------
> include/linux/fsl/pci-common.h | 6 ++
> 2 files changed, 151 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pci/host/pci-fsl-common.c b/drivers/pci/host/pci-fsl-common.c
> index 69d338b..8bc9a64 100644
> --- a/drivers/pci/host/pci-fsl-common.c
> +++ b/drivers/pci/host/pci-fsl-common.c
> @@ -35,52 +35,173 @@
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>
> -static int fsl_pcie_check_link(struct pci_controller *hose)
> +/* Indirect type */
> +#define INDIRECT_TYPE_EXT_REG 0x00000002
> +#define INDIRECT_TYPE_SURPRESS_PRIMARY_BUS 0x00000004
> +#define INDIRECT_TYPE_NO_PCIE_LINK 0x00000008
> +#define INDIRECT_TYPE_BIG_ENDIAN 0x00000010
> +#define INDIRECT_TYPE_FSL_CFG_REG_LINK 0x00000040
Why are these here rather than in the header, given that you have
indirect_type in the struct in the header?
> +int __weak fsl_arch_pci_exclude_device(struct fsl_pci *pci, u8 bus, u8 devfn)
> +{
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int fsl_pci_read_config(struct fsl_pci *pci, int bus, int devfn,
> + int offset, int len, u32 *val)
> +{
> + u32 bus_no, reg, data;
> +
> + if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
> + if (bus != pci->first_busno)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + if (devfn != 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }
A lot of this seems duplicated from arch/powerpc/sysdev/indirect_pci.c.
How generally applicable is that file to non-PPC implementations? At a
minimum I see a similar file in arch/microblaze. It should probably
eventually be moved to common code, rather than duplicated again. A
prerequisite for that would be making common the dependencies it has on
the rest of what is currently arch PCI infrastructure; until then, it's
probably better to just have the common fsl-pci code know how to
interface with the appropriate PPC/ARM code rather than trying to copy
the infrastructure as well.
-Scott
^ permalink raw reply
* Re: [02/12,v3] pci: fsl: add structure fsl_pci
From: Scott Wood @ 2014-01-03 22:19 UTC (permalink / raw)
To: Minghuan Lian; +Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <1382524894-15164-2-git-send-email-Minghuan.Lian@freescale.com>
On Wed, Oct 23, 2013 at 06:41:24PM +0800, Minghuan Lian wrote:
> PowerPC uses structure pci_controller to describe PCI controller,
> but ARM uses structure pci_sys_data. In order to support PowerPC
> and ARM simultaneously, the patch adds a structure fsl_pci that
> contains most of the members of the pci_controller and pci_sys_data.
> Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
> be implemented in architecture-specific PCI controller driver to
> convert pci_controller or pci_sys_data to fsl_pci.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>
> ---
> change log:
> v1-v3:
> Derived from http://patchwork.ozlabs.org/patch/278965/
>
> Based on upstream master.
> Based on the discussion of RFC version here
> http://patchwork.ozlabs.org/patch/274487/
>
> include/linux/fsl/pci-common.h | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
> index 5e4f683..e56a040 100644
> --- a/include/linux/fsl/pci-common.h
> +++ b/include/linux/fsl/pci-common.h
> @@ -102,5 +102,46 @@ struct ccsr_pci {
>
> };
>
> +/*
> + * Structure of a PCI controller (host bridge)
> + */
> +struct fsl_pci {
> + struct list_head node;
> + bool is_pcie;
> + struct device_node *dn;
> + struct device *dev;
> +
> + int first_busno;
> + int last_busno;
> + int self_busno;
> + struct resource busn;
> +
> + struct pci_ops *ops;
> + struct ccsr_pci __iomem *regs;
> +
> + u32 indirect_type;
> +
> + struct resource io_resource;
> + resource_size_t io_base_phys;
> + resource_size_t pci_io_size;
> +
> + struct resource mem_resources[3];
> + resource_size_t mem_offset[3];
> +
> + int global_number; /* PCI domain number */
> +
> + resource_size_t dma_window_base_cur;
> + resource_size_t dma_window_size;
> +
> + void *sys;
> +};
I don't like the extent to which this duplicates (not moves) PPC's struct
pci_controller. Also this leaves some fields like "indirect_type"
unexplained (PPC_INDIRECT_TYPE_xxx is only in the PPC header).
Does the arch-independent part of the driver really need all this? Given
how closely this tracks the PPC code, how would this work on ARM?
-Scott
^ permalink raw reply
* Re: [11/12,v3] pci: fsl: update PCI EDAC driver
From: Scott Wood @ 2014-01-03 22:16 UTC (permalink / raw)
To: Minghuan Lian; +Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <1382524894-15164-11-git-send-email-Minghuan.Lian@freescale.com>
On Wed, Oct 23, 2013 at 06:41:33PM +0800, Minghuan Lian wrote:
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 40d2e1d..4a03e1a 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -236,6 +236,7 @@ void fsl_arch_pci_sys_remove(struct fsl_pci *pci)
> if (!hose)
> return;
>
> + mpc85xx_pci_err_remove(to_platform_device(pci->dev));
> pcibios_free_controller(hose);
This causes a linker error if the EDAC driver is enabled, due to commit
0f1741c74aa6794b1c7fbdd19f26a4f2377a11c6 ("edac/85xx: Remove
mpc85xx_pci_err_remove").
-Scott
^ permalink raw reply
* P1020 implementation crashes with "Data Cache Parity Error" on board derived from Freescale P1020Wlan eval board.
From: John Clark @ 2014-01-03 22:10 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
I have a problem when booting the linux kernel using two cores of the =
P1020 Freescale ppc implementation.
I have found on the Freescale site one other person who has a similar =
problem and we both are using designed driver from the P1020Wlan 'eval' =
board.
I would like to know if others have this 'problem', and used the =
P1020Wlan as their hardware starting point, and what they did to =
overcome this problem.
Single core works fine. Further the original P1020Wlan board boots my =
'new' kernel, 3.10.15 from OpenWRT sources, 'just fine'.
The problem may be that we used a different DDR3 chip than the original =
eval, and some subtle difference in timing that needs some 'adjustment' =
has eluded me.
Thanks for any info/insight,
John Clark.
^ permalink raw reply
* Re: [PATCH RFC v6 4/5] dma: mpc512x: register for device tree channel lookup
From: Alexander Popov @ 2014-01-03 20:54 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine, devicetree
In-Reply-To: <20131226124828.GK8064@book.gsilab.sittig.org>
Hello Gerhard.
Thanks for your review.
2013/12/26 Gerhard Sittig <gsi@denx.de>:
> [ dropping devicetree, we're DMA specific here ]
>
> On Tue, Dec 24, 2013 at 16:06 +0400, Alexander Popov wrote:
>>
>> --- a/drivers/dma/mpc512x_dma.c
>> +++ b/drivers/dma/mpc512x_dma.c
>> [ ... ]
>> @@ -950,6 +951,7 @@ static int mpc_dma_probe(struct platform_device *op)
>> INIT_LIST_HEAD(&dma->channels);
>> dma_cap_set(DMA_MEMCPY, dma->cap_mask);
>> dma_cap_set(DMA_SLAVE, dma->cap_mask);
>> + dma_cap_set(DMA_PRIVATE, dma->cap_mask);
>>
>> for (i = 0; i < dma->chancnt; i++) {
>> mchan = &mdma->channels[i];
>
> What are the implications of this? Is a comment due?
I've involved DMA_PRIVATE flag because new of_dma_xlate_by_chan_id()
uses dma_get_slave_channel() instead of dma_request_channel()
(PATCH RFC v6 3/5). This flag is implicitly set in dma_request_channel(),
but is not set in dma_get_slave_channel().
There are only two places in the mainline kernel, where
dma_get_slave_channel() is used. I've picked up the idea
at one of these places. Please look at this patch:
http://www.spinics.net/lists/arm-kernel/msg268718.html
> I haven't found documentation about the DMA_PRIVATE flag, only
> saw commit 59b5ec21446b9 "dmaengine: introduce
> dma_request_channel and private channels".
Unfortunately I didn't find any description of DMA_PRIVATE flag too.
But the comment at the beginning of drivers/dma/dmaengine.c
may give a clue. Quotation:
* subsystem can get access to a channel by calling dmaengine_get() followed
* by dma_find_channel(), or if it has need for an exclusive channel
it can call
* dma_request_channel(). Once a channel is allocated a reference is taken
* against its corresponding driver to disable removal.
DMA_PRIVATE capability flag might indicate that the DMA controller
can provide exclusive channels to its clients. Please correct me if I'm wrong.
> Alex, unless I'm
> missing something this one-line change is quite a change in
> semantics, and has dramatic influence on the code's behaviour
> (ignores the DMA controller when looking for channels that can do
> mem-to-mem transfers)
Excuse me, Gerhard, I don't see what you mean.
Could you point to the corresponding code?
> Consider the fact that this driver
> handles both MPC5121 as well as MPC8308 hardware.
Ah, yes, sorry. I should certainly fix this, if setting of DMA_PRIVATE flag
is needed at all.
Thanks!
Best regards,
Alexander
^ permalink raw reply
* Re: [PATCH v2] mtd: m25p80: Add Power Management support
From: Brian Norris @ 2014-01-03 19:07 UTC (permalink / raw)
To: Hou Zhiqiang
Cc: Marek Vasut, linuxppc-dev, linux-mtd, scottwood, Mingkai.Hu,
dwmw2
In-Reply-To: <1386826716-20476-1-git-send-email-b48286@freescale.com>
On Thu, Dec 12, 2013 at 01:38:36PM +0800, Hou Zhiqiang wrote:
> Add PM support using callback function suspend and resume in
> .driver.pm of spi_driver.
>
> Signed-off-by: Hou Zhiqiang <b48286@freescale.com>
> ---
> v2:
> - Replace .driver.suspend and .driver.resume with .driver.pm
> - Use CONFIG_PM_SLEEP instead of CONFIG_PM
Just noticed v2... My v1 comments still apply.
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -1128,11 +1130,46 @@ static int m25p_remove(struct spi_device *spi)
...
> +static const struct dev_pm_ops m25p_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(m25p_suspend, m25p_resume)
> +};
This could just be:
static SIMPLE_DEV_PM_OPS(m25p_pm_ops, m25p_suspend, m25p_resume);
>
> static struct spi_driver m25p80_driver = {
> .driver = {
> .name = "m25p80",
> .owner = THIS_MODULE,
> + .pm = &m25p_pm,
> },
> .id_table = m25p_ids,
> .probe = m25p_probe,
Brian
^ permalink raw reply
* Re: [PATCH] mtd: m25p80: Add Power Management support
From: Brian Norris @ 2014-01-03 19:00 UTC (permalink / raw)
To: Hou Zhiqiang
Cc: Marek Vasut, linuxppc-dev, linux-mtd, scottwood, Mingkai.Hu,
dwmw2
In-Reply-To: <1386749970-22021-1-git-send-email-b48286@freescale.com>
On Wed, Dec 11, 2013 at 04:19:30PM +0800, Hou Zhiqiang wrote:
> Add PM support using callback function suspend and resume in .driver of
> spi_driver.
>
> Signed-off-by: Hou Zhiqiang <b48286@freescale.com>
> ---
> drivers/mtd/devices/m25p80.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 7eda71d..b0c2b8c 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -66,6 +66,8 @@
>
> /* Used for Spansion flashes only. */
> #define OPCODE_BRWR 0x17 /* Bank register write */
> +#define OPCODE_DP 0xb9 /* Enter deep power down mode */
> +#define OPCODE_RES 0xab /* Exit deep power down mode */
Where did you get these opcodes from? They are not in the Spansion
datasheets I'm reading. And in fact, they are overloaded as RES (Read
Electronic Signature, 0xab) and Bank Register Access (0xb9) in the
datasheet I'm looking at. So this patch is wrong.
Also, can you describe the purpose of these "deep power down" modes?
I've never seen PM states where the *flash* needs to be put into a lower
power mode. Typically the flash is pretty low-power when idle, and it
may even be powered off completely when the system enters a lower-power
state. Anyway, please describe why this patch is needed.
>
> /* Status Register bits. */
> #define SR_WIP 1 /* Write in progress */
> @@ -1128,11 +1130,46 @@ static int m25p_remove(struct spi_device *spi)
> return mtd_device_unregister(&flash->mtd);
> }
>
> +#ifdef CONFIG_PM
> +static int m25p_suspend(struct device *dev, pm_message_t mesg)
> +{
> + struct m25p *flash = dev_get_drvdata(dev);
> + int ret;
> +
> + flash->command[0] = OPCODE_DP;
As mentioned above, this opcode is not recognized by many flash
supported in this driver. So we might want one or more of the following:
(1) to assign different suspend/resume opcodes for use in
m25p_suspend/resume
(2) to provide over-loadable callbacks so that different flash could
use different suspend/resume routines
And of course, we need to avoid sending these commands at all to
unsupported flash.
> + mutex_lock(&flash->lock);
> + /* Wait until finished previous write/erase command. */
> + ret = wait_till_ready(flash);
> + if (ret) {
> + mutex_unlock(&flash->lock);
> + return ret;
> + }
> + ret = spi_write(flash->spi, flash->command, 1);
> + mutex_unlock(&flash->lock);
> +
> + return ret;
> +}
> +
> +static int m25p_resume(struct device *dev)
> +{
> + struct m25p *flash = dev_get_drvdata(dev);
> + int ret;
> +
> + flash->command[0] = OPCODE_RES;
> + ret = spi_write(flash->spi, flash->command, 1);
> +
> + return ret;
> +}
> +#endif /* CONFIG_PM */
>
> static struct spi_driver m25p80_driver = {
> .driver = {
> .name = "m25p80",
> .owner = THIS_MODULE,
> +#ifdef CONFIG_PM
> + .suspend = m25p_suspend,
> + .resume = m25p_resume,
> +#endif
> },
> .id_table = m25p_ids,
> .probe = m25p_probe,
Brian
^ permalink raw reply
* Re: [PATCH v3 REPOST 2/4] i2c: i2c-ibm-iic: perform the transfer in the interrupt handler
From: Jean-Jacques Hiblot @ 2014-01-03 15:57 UTC (permalink / raw)
To: Wolfram Sang
Cc: Gregory CLEMENT, linuxppc-dev, jean-jacques hiblot, linux-i2c
In-Reply-To: <20140103150416.GA7132@katana>
2014/1/3 Wolfram Sang <wsa@the-dreams.de>:
>
> Hi,
>
> thanks for the submission!
You're welcome :) Thanks for reviewing this.
>
>> --- a/drivers/i2c/busses/i2c-ibm_iic.c
>> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
>> @@ -58,6 +58,8 @@ static bool iic_force_fast;
>> module_param(iic_force_fast, bool, 0);
>> MODULE_PARM_DESC(iic_force_fast, "Force fast mode (400 kHz)");
>>
>> +#define FIFO_FLUSH_TIMEOUT 100
>
> 100 what? The unit is missing.
The actual value of this timeout isn't very important. This is used
only to avoid a never ending loop in case the hardware goes into a
really bad state.
>
>> @@ -167,8 +170,8 @@ static void iic_dev_init(struct ibm_iic_private* dev)
>> /* Clear control register */
>> out_8(&iic->cntl, 0);
>>
>> - /* Enable interrupts if possible */
>> - iic_interrupt_mode(dev, dev->irq >= 0);
>> + /* Start with each individual interrupt masked*/
>
> Space at the end of comment missing
ok
>
>> static irqreturn_t iic_handler(int irq, void *dev_id)
>> {
>> - struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
>> - struct iic_regs __iomem *iic = dev->vaddr;
>> -
>> - DBG2(dev, "irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
>> - in_8(&iic->sts), in_8(&iic->extsts));
>> -
>> - /* Acknowledge IRQ and wakeup iic_wait_for_tc */
>> - out_8(&iic->sts, STS_IRQA | STS_SCMP);
>> - wake_up_interruptible(&dev->wq);
>> -
>> + struct ibm_iic_private *dev = (struct ibm_iic_private *) dev_id;
>> + iic_xfer_bytes(dev);
>
> Is iic_xfer_bytes later used when polling, too? Otherwise it could be
> simply inserted here.
Yes it'll be used for polling (implemented in a later patch)
>
>
>> + if ((status & STS_ERR) ||
>> + (ext_status & (EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA))) {
>> + DBG(dev, "status 0x%x\n", status);
>> + DBG(dev, "extended status 0x%x\n", ext_status);
>> + if (status & STS_ERR)
>> + ERR(dev, "Error detected\n");
>> + if (ext_status & EXTSTS_LA)
>> + DBG(dev, "Lost arbitration\n");
>> + if (ext_status & EXTSTS_ICT)
>> + ERR(dev, "Incomplete transfer\n");
>> + if (ext_status & EXTSTS_XFRA)
>> + ERR(dev, "Transfer aborted\n");
>> +
>> + dev->status = -EIO;
>
> You could consider returning different fault codes for the different
> states. See Documentation/i2c/fault-codes for a guide.
I'll have a look at it.
>
>> + if (dev->msgs == NULL) {
>> + DBG(dev, "spurious !!!!!\n");
>> + dev->status = -EINVAL;
>> + return dev->status;
>> + }
>
> Does that really happen?
Not in my test cases (going through i2c dev). But it could if the data
passed to i2c_transfer is malformed.
Should I remove this ?
>
> And it introduces a build warning:
>
> drivers/i2c/busses/i2c-ibm_iic.c:410:12: warning: 'iic_wait_for_tc' defined but not used [-Wunused-function]
>
I posted this some time ago, but I believe I kept this in to help git
produce a diff readable by a human.
It's removed in a later patch
Jean-Jacques
^ permalink raw reply
* Re: [PATCH v3 REPOST 3/4] i2c: i2c-ibm-iic: Implements transfer abortion
From: Jean-Jacques Hiblot @ 2014-01-03 16:14 UTC (permalink / raw)
To: Wolfram Sang
Cc: Gregory CLEMENT, linuxppc-dev, jean-jacques hiblot, linux-i2c
In-Reply-To: <20140103150948.GB7132@katana>
2014/1/3 Wolfram Sang <wsa@the-dreams.de>:
> On Fri, Dec 20, 2013 at 04:12:55PM +0100, jean-jacques hiblot wrote:
>> From: jean-jacques hiblot <jjhiblot@gmail.com>
>>
>> Clean-up properly when a transfer fails for whatever reason.
>> Cancel the transfer when the process is signaled.
>
> Please describe what you do a little. I wonder how you can remove so
> much code while keeping the functionality?
Well there are 2 reasons why so much code went away.
1) The iic_wait_for_tc() function wasn't used anymore (it should have
disappeared in an earlier patch but the diff was terrible to read)
2) the whole abortion scheme is different. It's now done as a part of
the data transfer. The reason is that the controller doesn't react
properly to abortion when it's not done at the right moment.
The idea here is to abort the transfer right after sending the next
byte to keep the controller happy. If the abortion is asked at the
wrong moment, the controller may not set the abortion complete flag
and the next transfer may fail.
>
>>
>> Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
>
>> - out_8(&iic->cntl, CNTL_HMT);
>> + DBG(dev, "aborting transfer\n");
>> + /* transfer should be aborted within 10ms */
>> + end = jiffies + 10;
>
> Eeks, msecs_to_jiffies() macro please!
>
> And please consider running checkpatch and sparse over your code. Sparse
> gives, for example:
>
> drivers/i2c/busses/i2c-ibm_iic.c:418:24: warning: incorrect type in argument 1 (different address spaces)
> drivers/i2c/busses/i2c-ibm_iic.c:418:24: expected unsigned char const volatile [noderef] [usertype] <asn:2>*addr
> drivers/i2c/busses/i2c-ibm_iic.c:418:24: got unsigned char *<noident>
>
> (This probably due to patch 1 or 2, I'd guess)
>
^ permalink raw reply
* Re: [PATCH v3 REPOST 4/4] i2c: i2c-ibm-iic: Implements a polling mode
From: Wolfram Sang @ 2014-01-03 15:13 UTC (permalink / raw)
To: jean-jacques hiblot
Cc: gregory.clement, linuxppc-dev, linux-i2c, jean-jacques hiblot
In-Reply-To: <1387552376-12986-5-git-send-email-jjhiblot@traphandler.com>
[-- Attachment #1: Type: text/plain, Size: 666 bytes --]
On Fri, Dec 20, 2013 at 04:12:56PM +0100, jean-jacques hiblot wrote:
> From: jean-jacques hiblot <jjhiblot@gmail.com>
>
> When no valid interrupt is defined for the controller, use polling to handle
> the transfers.
> The polling mode can also be forced with the "iic_force_poll" module parameter.
>
> Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
Do I understand correctly: Polling was not available after patch 2 until patch 4?
Has the signal handling been tested extensively? Most drivers do not
handle signals because it is usually cumbersome to cancel a transfer
gracefully. That being said, it might work well for you here.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 REPOST 3/4] i2c: i2c-ibm-iic: Implements transfer abortion
From: Wolfram Sang @ 2014-01-03 15:09 UTC (permalink / raw)
To: jean-jacques hiblot
Cc: gregory.clement, linuxppc-dev, linux-i2c, jean-jacques hiblot
In-Reply-To: <1387552376-12986-4-git-send-email-jjhiblot@traphandler.com>
[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]
On Fri, Dec 20, 2013 at 04:12:55PM +0100, jean-jacques hiblot wrote:
> From: jean-jacques hiblot <jjhiblot@gmail.com>
>
> Clean-up properly when a transfer fails for whatever reason.
> Cancel the transfer when the process is signaled.
Please describe what you do a little. I wonder how you can remove so
much code while keeping the functionality?
>
> Signed-off-by: jean-jacques hiblot <jjhiblot@traphandler.com>
> - out_8(&iic->cntl, CNTL_HMT);
> + DBG(dev, "aborting transfer\n");
> + /* transfer should be aborted within 10ms */
> + end = jiffies + 10;
Eeks, msecs_to_jiffies() macro please!
And please consider running checkpatch and sparse over your code. Sparse
gives, for example:
drivers/i2c/busses/i2c-ibm_iic.c:418:24: warning: incorrect type in argument 1 (different address spaces)
drivers/i2c/busses/i2c-ibm_iic.c:418:24: expected unsigned char const volatile [noderef] [usertype] <asn:2>*addr
drivers/i2c/busses/i2c-ibm_iic.c:418:24: got unsigned char *<noident>
(This probably due to patch 1 or 2, I'd guess)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 REPOST 2/4] i2c: i2c-ibm-iic: perform the transfer in the interrupt handler
From: Wolfram Sang @ 2014-01-03 15:04 UTC (permalink / raw)
To: jean-jacques hiblot
Cc: gregory.clement, linuxppc-dev, linux-i2c, jean-jacques hiblot
In-Reply-To: <1387552376-12986-3-git-send-email-jjhiblot@traphandler.com>
[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]
Hi,
thanks for the submission!
> --- a/drivers/i2c/busses/i2c-ibm_iic.c
> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
> @@ -58,6 +58,8 @@ static bool iic_force_fast;
> module_param(iic_force_fast, bool, 0);
> MODULE_PARM_DESC(iic_force_fast, "Force fast mode (400 kHz)");
>
> +#define FIFO_FLUSH_TIMEOUT 100
100 what? The unit is missing.
> @@ -167,8 +170,8 @@ static void iic_dev_init(struct ibm_iic_private* dev)
> /* Clear control register */
> out_8(&iic->cntl, 0);
>
> - /* Enable interrupts if possible */
> - iic_interrupt_mode(dev, dev->irq >= 0);
> + /* Start with each individual interrupt masked*/
Space at the end of comment missing
> static irqreturn_t iic_handler(int irq, void *dev_id)
> {
> - struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
> - struct iic_regs __iomem *iic = dev->vaddr;
> -
> - DBG2(dev, "irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
> - in_8(&iic->sts), in_8(&iic->extsts));
> -
> - /* Acknowledge IRQ and wakeup iic_wait_for_tc */
> - out_8(&iic->sts, STS_IRQA | STS_SCMP);
> - wake_up_interruptible(&dev->wq);
> -
> + struct ibm_iic_private *dev = (struct ibm_iic_private *) dev_id;
> + iic_xfer_bytes(dev);
Is iic_xfer_bytes later used when polling, too? Otherwise it could be
simply inserted here.
> + if ((status & STS_ERR) ||
> + (ext_status & (EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA))) {
> + DBG(dev, "status 0x%x\n", status);
> + DBG(dev, "extended status 0x%x\n", ext_status);
> + if (status & STS_ERR)
> + ERR(dev, "Error detected\n");
> + if (ext_status & EXTSTS_LA)
> + DBG(dev, "Lost arbitration\n");
> + if (ext_status & EXTSTS_ICT)
> + ERR(dev, "Incomplete transfer\n");
> + if (ext_status & EXTSTS_XFRA)
> + ERR(dev, "Transfer aborted\n");
> +
> + dev->status = -EIO;
You could consider returning different fault codes for the different
states. See Documentation/i2c/fault-codes for a guide.
> + if (dev->msgs == NULL) {
> + DBG(dev, "spurious !!!!!\n");
> + dev->status = -EINVAL;
> + return dev->status;
> + }
Does that really happen?
And it introduces a build warning:
drivers/i2c/busses/i2c-ibm_iic.c:410:12: warning: 'iic_wait_for_tc' defined but not used [-Wunused-function]
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mtsprg
From: Dongsheng.Wang @ 2014-01-03 10:32 UTC (permalink / raw)
To: Scott Wood, Benjamin Herrenschmidt
Cc: Anton Vorontsov, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1388702270-21550-1-git-send-email-scottwood@freescale.com>
Looks good. I will test it as soon as possible.=20
BTW, there is only SPRG3 need to save.
32bit: SPRG0-SPRG1, SPRG2-SPRG7, SPRG9 be use to deal with exception,
those register not need to save.(SPRG8 not be used) Only SPRG3 be used
to save current thread_info pointer.
-Dongsheng
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, January 03, 2014 6:38 AM
> To: Benjamin Herrenschmidt
> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Wang Dongsheng-B405=
34;
> Anton Vorontsov
> Subject: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mts=
prg
>=20
> This fixes a build break that was probably introduced with the removal
> of -Wa,-me500 (commit f49596a4cf4753d13951608f24f939a59fdcc653), where
> the assembler refuses to recognize SPRG4-7 with a generic PPC target.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Cc: Dongsheng Wang <dongsheng.wang@freescale.com>
> Cc: Anton Vorontsov <avorontsov@mvista.com>
> ---
> Dongsheng, please test.
> ---
> arch/powerpc/kernel/swsusp_booke.S | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/swsusp_booke.S
> b/arch/powerpc/kernel/swsusp_booke.S
> index 0f20405..553c140 100644
> --- a/arch/powerpc/kernel/swsusp_booke.S
> +++ b/arch/powerpc/kernel/swsusp_booke.S
> @@ -74,21 +74,21 @@ _GLOBAL(swsusp_arch_suspend)
> bne 1b
>=20
> /* Save SPRGs */
> - mfsprg r4,0
> + mfspr r4,SPRN_SPRG0
> stw r4,SL_SPRG0(r11)
> - mfsprg r4,1
> + mfspr r4,SPRN_SPRG1
> stw r4,SL_SPRG1(r11)
> - mfsprg r4,2
> + mfspr r4,SPRN_SPRG2
> stw r4,SL_SPRG2(r11)
> - mfsprg r4,3
> + mfspr r4,SPRN_SPRG3
> stw r4,SL_SPRG3(r11)
> - mfsprg r4,4
> + mfspr r4,SPRN_SPRG4
> stw r4,SL_SPRG4(r11)
> - mfsprg r4,5
> + mfspr r4,SPRN_SPRG5
> stw r4,SL_SPRG5(r11)
> - mfsprg r4,6
> + mfspr r4,SPRN_SPRG6
> stw r4,SL_SPRG6(r11)
> - mfsprg r4,7
> + mfspr r4,SPRN_SPRG7
> stw r4,SL_SPRG7(r11)
>=20
> /* Call the low level suspend stuff (we should probably have made
> @@ -150,21 +150,21 @@ _GLOBAL(swsusp_arch_resume)
> bl _tlbil_all
>=20
> lwz r4,SL_SPRG0(r11)
> - mtsprg 0,r4
> + mtspr SPRN_SPRG0,r4
> lwz r4,SL_SPRG1(r11)
> - mtsprg 1,r4
> + mtspr SPRN_SPRG1,r4
> lwz r4,SL_SPRG2(r11)
> - mtsprg 2,r4
> + mtspr SPRN_SPRG2,r4
> lwz r4,SL_SPRG3(r11)
> - mtsprg 3,r4
> + mtspr SPRN_SPRG3,r4
> lwz r4,SL_SPRG4(r11)
> - mtsprg 4,r4
> + mtspr SPRN_SPRG4,r4
> lwz r4,SL_SPRG5(r11)
> - mtsprg 5,r4
> + mtspr SPRN_SPRG5,r4
> lwz r4,SL_SPRG6(r11)
> - mtsprg 6,r4
> + mtspr SPRN_SPRG6,r4
> lwz r4,SL_SPRG7(r11)
> - mtsprg 7,r4
> + mtspr SPRN_SPRG7,r4
>=20
> /* restore the MSR */
> lwz r3,SL_MSR(r11)
> --
> 1.8.3.2
^ permalink raw reply
* [PATCH v2 2/2] powerpc/eeh: Reinit error reporting
From: Gavin Shan @ 2014-01-03 9:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1388742433-7328-1-git-send-email-shangw@linux.vnet.ibm.com>
The patch implements the EEH operation backend restore_config()
for PowerNV platform. That relies on OPAL API opal_pci_reinit()
where we reinitialize the error reporting properly after PE or
PHB reset. The patch also extends opal_pci_reinit() to have one
additional parameter to carry more information like PCI device
address.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/opal.h | 8 ++++++--
arch/powerpc/platforms/powernv/eeh-powernv.c | 23 ++++++++++++++++++++++-
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 7bdcf34..25f6454 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -311,12 +311,16 @@ enum OpalMveEnableAction {
OPAL_ENABLE_MVE = 1
};
-enum OpalPciResetAndReinitScope {
+enum OpalPciResetScope {
OPAL_PHB_COMPLETE = 1, OPAL_PCI_LINK = 2, OPAL_PHB_ERROR = 3,
OPAL_PCI_HOT_RESET = 4, OPAL_PCI_FUNDAMENTAL_RESET = 5,
OPAL_PCI_IODA_TABLE_RESET = 6,
};
+enum OpalPciReinitScope {
+ OPAL_PCI_DEV = 1
+};
+
enum OpalPciResetState {
OPAL_DEASSERT_RESET = 0,
OPAL_ASSERT_RESET = 1
@@ -710,7 +714,7 @@ int64_t opal_pci_get_phb_diag_data(uint64_t phb_id, void *diag_buffer,
int64_t opal_pci_get_phb_diag_data2(uint64_t phb_id, void *diag_buffer,
uint64_t diag_buffer_len);
int64_t opal_pci_fence_phb(uint64_t phb_id);
-int64_t opal_pci_reinit(uint64_t phb_id, uint8_t reinit_scope);
+int64_t opal_pci_reinit(uint64_t phb_id, uint8_t reinit_scope, uint32_t data);
int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
int64_t opal_set_slot_led_status(uint64_t phb_id, uint64_t slot_id, uint8_t led_type, uint8_t led_action);
int64_t opal_get_epow_status(__be64 *status);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index ab91e6a..a0d4d9a 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -344,6 +344,27 @@ static int powernv_eeh_next_error(struct eeh_pe **pe)
return -EEXIST;
}
+static int powernv_eeh_restore_config(struct device_node *dn)
+{
+ struct eeh_dev *edev = of_node_to_eeh_dev(dn);
+ struct pnv_phb *phb;
+ s64 ret;
+
+ if (!edev)
+ return -EEXIST;
+
+ phb = edev->phb->private_data;
+ ret = opal_pci_reinit(phb->opal_id,
+ OPAL_PCI_DEV, edev->config_addr);
+ if (ret) {
+ pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
+ __func__, edev->config_addr, ret);
+ return -EIO;
+ }
+
+ return 0;
+}
+
static struct eeh_ops powernv_eeh_ops = {
.name = "powernv",
.init = powernv_eeh_init,
@@ -360,7 +381,7 @@ static struct eeh_ops powernv_eeh_ops = {
.read_config = pnv_pci_cfg_read,
.write_config = pnv_pci_cfg_write,
.next_error = powernv_eeh_next_error,
- .restore_config = NULL
+ .restore_config = powernv_eeh_restore_config
};
/**
--
1.7.10.4
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/eeh: Add restore_config operation
From: Gavin Shan @ 2014-01-03 9:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
After reset on the specific PE or PHB, we never configure AER
correctly on PowerNV platform. We needn't care it on pSeries
platform. The patch introduces additional EEH operation eeh_ops::
restore_config() so that we have chance to configure AER correctly
for PowerNV platform.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 1 +
arch/powerpc/kernel/eeh_pe.c | 3 +++
arch/powerpc/platforms/powernv/eeh-powernv.c | 3 ++-
arch/powerpc/platforms/pseries/eeh_pseries.c | 4 +++-
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d3e5e9b..7f8adc8 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -157,6 +157,7 @@ struct eeh_ops {
int (*read_config)(struct device_node *dn, int where, int size, u32 *val);
int (*write_config)(struct device_node *dn, int where, int size, u32 val);
int (*next_error)(struct eeh_pe **pe);
+ int (*restore_config)(struct device_node *dn);
};
extern struct eeh_ops *eeh_ops;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index f945053..b60e11a 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -737,6 +737,9 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
else
eeh_restore_device_bars(edev, dn);
+ if (eeh_ops->restore_config)
+ eeh_ops->restore_config(dn);
+
return NULL;
}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 73b9814..ab91e6a 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -359,7 +359,8 @@ static struct eeh_ops powernv_eeh_ops = {
.configure_bridge = powernv_eeh_configure_bridge,
.read_config = pnv_pci_cfg_read,
.write_config = pnv_pci_cfg_write,
- .next_error = powernv_eeh_next_error
+ .next_error = powernv_eeh_next_error,
+ .restore_config = NULL
};
/**
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ccb633e..9ef3cc8 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -689,7 +689,9 @@ static struct eeh_ops pseries_eeh_ops = {
.get_log = pseries_eeh_get_log,
.configure_bridge = pseries_eeh_configure_bridge,
.read_config = pseries_eeh_read_config,
- .write_config = pseries_eeh_write_config
+ .write_config = pseries_eeh_write_config,
+ .next_error = NULL,
+ .restore_config = NULL
};
/**
--
1.7.10.4
^ permalink raw reply related
* [PATCH] powerpc: add SATA_MV to ppc64_defconfig
From: Olof Johansson @ 2014-01-03 8:24 UTC (permalink / raw)
To: benh; +Cc: Olof Johansson, linuxppc-dev
This makes ppc64_defconfig bootable without initrd on pasemi systems,
most of whom have MV SATA controllers. Some have SIL24, but that driver
is already enabled.
Signed-off-by: Olof Johansson <olof@lixom.net>
---
arch/powerpc/configs/ppc64_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 581a3bc..e015896 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -186,6 +186,7 @@ CONFIG_SCSI_DH_RDAC=m
CONFIG_SCSI_DH_ALUA=m
CONFIG_ATA=y
CONFIG_SATA_SIL24=y
+CONFIG_SATA_MV=y
CONFIG_SATA_SVW=y
CONFIG_MD=y
CONFIG_BLK_DEV_MD=y
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] powerpc: Fix alignment of secondary cpu spin vars
From: Olof Johansson @ 2014-01-03 8:12 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: chzigotzky, linuxppc-dev, linux-kernel@vger.kernel.org,
Anton Blanchard
In-Reply-To: <CAOesGMgsY0R6KBXXH_ibTBYb1i_auWESwnoDkvi-esb58Pc+2A@mail.gmail.com>
On Thu, Jan 02, 2014 at 11:56:04PM -0800, Olof Johansson wrote:
> This makes things interesting though. The BE/LE trampoline code
> assumes at least 3 consecutive instructions. What was the reasoning
> behind entering the kernel LE instead of keeping the old boot protocol
> and just switching to LE once kernel is loaded? Is it actually used on
> some platforms or is this just a theoretical thing?
Actually, adding a little hack that zeroes out the memory once we're done
executing it will work just fine too. I know this is sort of icky, but maybe
it'll be good enough for now?
Of course, main worry is that this is just hiding some latent NULL deref in
the kernel now... :-/
-Olof
--- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< --- 8< ---
>From 4d003186cae546900cefc9e51b0ed4e65f775be1 Mon Sep 17 00:00:00 2001
From: Olof Johansson <olof@lixom.net>
Date: Fri, 3 Jan 2014 00:09:28 -0800
Subject: [PATCH] powerpc: set some low memory contents to 0 early
The little-endian code adds some code path to __start, which essentially ends
up adding memory contents in low memory that didn't use to be there.
That seems to have triggered a latent bug, either in firmware or kernel, where
the 64-bit word located at physical address 8 needs to be 0.
The simple hack for this right now is to write it to 0 after we're done
executing it, which is what this patch does. Unfortunately I no longer
seem to have a working JTAG setup nor firmware sources, so debugging
this down to root cause might be more trouble than it's worth given the
relatively simple workaround.
Signed-off-by: Olof Johansson <olof@lixom.net>
---
arch/powerpc/kernel/head_64.S | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 2ae41ab..437d8bd 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -69,6 +69,13 @@ _GLOBAL(__start)
/* NOP this out unconditionally */
BEGIN_FTR_SECTION
FIXUP_ENDIAN
+ /* Hack for PWRficient platforms: Due to CFE(?) bug, the 64-bit
+ * word at 0x8 needs to be set to 0. Patch it up here once we're
+ * done executing it (we can be lazy and avoid invalidating
+ * icache)
+ */
+ li r0,0
+ std 0,8(0)
b .__start_initialization_multiplatform
END_FTR_SECTION(0, 1)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] powerpc: Fix alignment of secondary cpu spin vars
From: Olof Johansson @ 2014-01-03 7:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Olof Johansson, chzigotzky, linuxppc-dev,
linux-kernel@vger.kernel.org, Anton Blanchard
In-Reply-To: <CAOesGMhHw-G-T7ZfNsVU92ftOucCNe9shNJtA7SWzqX=dQBQng@mail.gmail.com>
On Sat, Dec 28, 2013 at 1:05 PM, Olof Johansson <olof@lixom.net> wrote:
> Sigh, it's not this after all. I did a clean build with this applied
> and still see failures. Something else is (also?) going on here.
Ok, so after some more digging I actually think that this isn't about
the new code added as much as it is about having more code in low
memory.
Before, there were only two instuctions in __start:
b .__start_initialization_multiplatform
trap
Now, there's a whole bunch:
c000000000000000 <.__start>:
c000000000000000: 08 00 00 48 tdi 0,r0,72
c000000000000004: 48 00 00 24 b c000000000000028 <.__start+0x28>
c000000000000008: 05 00 9f 42 .long 0x5009f42
c00000000000000c: a6 02 48 7d lhzu r16,18557(r2)
c000000000000010: 1c 00 4a 39 mulli r0,r0,19001
c000000000000014: a6 00 60 7d lhzu r16,24701(0)
c000000000000018: 01 00 6b 69 .long 0x1006b69
c00000000000001c: a6 03 5a 7d lhzu r16,23165(r3)
c000000000000020: a6 03 7b 7d lhzu r16,31613(r3)
c000000000000024: 24 00 00 4c dozi r0,r0,76
c000000000000028: 48 00 95 84 b c0000000000095ac
<.__start_initialization_multiplatform>
c00000000000002c: 7f e0 00 08 trap
And indeed, by replacing some of the LE hand-converted code with 0x0,
it seems that what's really making things blow up here is that 0x8-0xc
contain something else than 0x0.
Where/why this comes from I'm less certain of -- and since I seem to
no longer have a usable JTAG setup, I can't break in and see where the
code gets stuck and call paths, etc. So it's pure speculation, but I'm
guessing it's a null pointer dereference somewhere with a chained
pointer as the second member in a struct, i.e. with NULL the stray
null ptr deref does no harm.
Since it doesn't seem to impact pSeries, there's a chance that the bug
is in firmware, not in the kernel, since this seems to happen during
fairly early boot, i.e. possibly while grabbing the DT contents out.
This makes things interesting though. The BE/LE trampoline code
assumes at least 3 consecutive instructions. What was the reasoning
behind entering the kernel LE instead of keeping the old boot protocol
and just switching to LE once kernel is loaded? Is it actually used on
some platforms or is this just a theoretical thing?
-Olof
^ permalink raw reply
* Re: [PATCH -V2] powerpc: thp: Fix crash on mremap
From: Aneesh Kumar K.V @ 2014-01-03 5:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aarcange, linuxppc-dev, paulus, Kirill A. Shutemov, linux-mm
In-Reply-To: <1388665786.4373.48.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> On Thu, 2014-01-02 at 16:22 +0530, Aneesh Kumar K.V wrote:
>> > Just use config option directly:
>> >
>> > if (new_ptl != old_ptl ||
>> > IS_ENABLED(CONFIG_ARCH_THP_MOVE_PMD_ALWAYS_WITHDRAW))
>>
>>
>> I didn't like that. I found the earlier one easier for reading.
>> If you and others strongly feel about this, I can redo the patch.
>> Please let me know
>
> Yes, use IS_ENABLED, no need to have two indirections of #define's
>
> Another option is to have
>
> if (pmd_move_must_withdraw(new,old)) {
> }
>
> With in a generic header:
>
> #ifndef pmd_move_must_withdraw
> static inline bool pmd_move_must_withdraw(spinlock_t *new_ptl, ...)
> {
> return new_ptl != old_ptl;
> }
> #endif
>
> And in powerpc:
>
> static inline bool pmd_move_must_withdraw(spinlock_t *new_ptl, ...)
> {
> return true;
> }
> #define pmd_move_must_withdraw pmd_move_must_withdraw
This is better i guess. It is also in-line with rest of transparent
hugepage functions. I will do this.
-aneesh
^ permalink raw reply
* Re: commit e38c0a1f breaks powerpc boards with uli1575 chip
From: Scott Wood @ 2014-01-03 0:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree@vger.kernel.org, Arnd Bergmann, Dmitry Krivoschekov,
Nikita Yushchenko, Thierry Reding, linux-kernel, Rob Herring,
Grant Likely, linuxppc-dev, Alexey Lugovskoy
In-Reply-To: <1388373200.4373.25.camel@pasglop>
On Mon, 2013-12-30 at 14:13 +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2013-12-19 at 08:42 +0400, Nikita Yushchenko wrote:
> > No, this does not help.
> >
> > I've dumped the actual content of 'range' and 'addr' at the failure
> > point
> > (i.e. ar point that returns error with e38c0a1f but passes without
> > e38c0a1f ):
> >
> > OF: default map, cp=0, s=10000, da=70
> > range: 01 00 00 00 00 00 00 00 00 00 00 00
> > addr: 00 00 00 00 00 00 00 00 00 00 00 70
>
> Something that has a #address-cells larger than 2, or more generally,
> an address field that contains more than a single number, must have
> a specific translation backend, like we have for PCI.
>
> This is a bit annoying but originates from the original OFW stuff on
> which this stuff is based where the bus node would provide the methods
> for translation.
I can maybe see that for PCI which has a special encoding, but why is it
always needed? E.g. if Freescale localbus had a 64-bit offset instead
of 32-bit, the child nodes would have 3 address cells, but
straightforward use of ranges would bring it down to 2 for the final
physical address. Existing localbus nodes already have "an address
field that contains more than a single number"; it's just a simple
enough encoding that it works to treat it as if it were a single large
number.
-Scott
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox