* [PATCHv5 2/7] ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
From: Yann Droneaud @ 2014-01-05 20:36 UTC (permalink / raw)
To: Jeremy Kerr, Arnd Bergmann, Benjamin Herrenschmidt,
Paul Mackerras
Cc: Yann Droneaud, cbe-oss-dev, linux-kernel, Al Viro, Andrew Morton,
linuxppc-dev
In-Reply-To: <cover.1388952061.git.ydroneaud@opteya.com>
Macro get_unused_fd() is used to allocate a file descriptor with
default flags. Those default flags (0) can be "unsafe":
O_CLOEXEC must be used by default to not leak file descriptor
across exec().
Instead of macro get_unused_fd(), function get_unused_fd_flags()
(or anon_inode_getfd()) should be used with flags given by userspace.
If not possible, flags should be set to O_CLOEXEC to provide userspace
with a default safe behavor.
In a further patch, get_unused_fd() will be removed so that new code
start using get_unused_fd_flags() (or anon_inode_getfd()) with correct
flags.
This patch replaces calls to get_unused_fd() with equivalent call to
get_unused_fd_flags(0) to preserve current behavor for existing code.
The hard coded flag value (0) should be reviewed on a per-subsystem
basis, and, if possible, set to O_CLOEXEC.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Link: http://lkml.kernel.org/r/cover.1388952061.git.ydroneaud@opteya.com
---
arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 87ba7cf99cd7..51effcec30d8 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -301,7 +301,7 @@ static int spufs_context_open(struct path *path)
int ret;
struct file *filp;
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
return ret;
@@ -518,7 +518,7 @@ static int spufs_gang_open(struct path *path)
int ret;
struct file *filp;
- ret = get_unused_fd();
+ ret = get_unused_fd_flags(0);
if (ret < 0)
return ret;
--
1.8.4.2
^ permalink raw reply related
* [PATCH v5 0/7] Getting rid of get_unused_fd() / enable close-on-exec
From: Yann Droneaud @ 2014-01-05 20:36 UTC (permalink / raw)
To: Tony Luck, Fenghua Yu, Al Viro, linux-ia64, Jeremy Kerr,
Arnd Bergmann, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, cbe-oss-dev, linux-fsdevel, Eric Paris,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Andrew Morton
Cc: Yann Droneaud, Ulrich Drepper, linux-kernel
Hi,
Please find the fifth revision of my patchset to remove get_unused_fd()
macro in order to help subsystems to use get_unused_fd_flags() (or
anon_inode_getfd()) with flags either provided by the userspace or
set to O_CLOEXEC by default where appropriate.
Without get_unused_fd() macro, more subsystems are likely to use
get_unused_fd_flags() (or anon_inode_getfd()) and be teached to
provide an API that let userspace choose the opening flags
of the file descriptor.
Not allowing userspace to provide the "open" flags or not using O_CLOEXEC
by default should be considered bad practice from security point of view:
in most case O_CLOEXEC must be used to not leak file descriptor across
exec().
Not allowing userspace to atomically set close-on-exec flag and not using
O_CLOEXEC should be avoided to protect multi-threaded program
from race condition when it tried to set close-on-exec flag using
fcntl(fd, F_SETFD, FD_CLOEXEC) after opening the file descriptor.
Example:
int fd;
int ret;
fd = open(filename, O_RDONLY);
if (fd < 0) {
perror("open");
return -1;
}
/*
* window opened for another thread to call fork(),
* then the new process can call exec() at any time
* and the file descriptor would be inherited
*/
ret = fcntl(fd, F_SETFD, FD_CLOEXEC)
if (ret < 0) {
perror("fnctl()");
close(fd);
return -1;
}
vs.:
int fd;
fd = open(filaneme, O_RDONLY | O_CLOEXEC);
if (fd < 0) {
perror("open");
return -1;
}
Using O_CLOEXEC by default when flags are not (eg. cannot be) provided
by userspace is the safest bet as it allows userspace to choose, if, when
and where the file descriptor is going to be inherited across exec():
userspace is free to call fcntl() to remove FD_CLOEXEC flag in the child
process that will call exec().
Unfortunately, O_CLOEXEC cannot be made the default for most existing system
calls as it's not the default behavior for POSIX / Unix. Reader interested
in this issue could have a look at "Ghosts of Unix past, part 2: Conflated
designs" [1] article by Neil Brown.
FAQ:
- Why do one want close-on-exec ?
Setting close-on-exec flag on file descriptor ensure it won't be inherited
silently by child, child of child, etc. when executing another program.
If the file descriptor is not closed, some kernel resources can be locked
until the last process with the opened file descriptor exit.
If the file descriptor is not closed, this can lead to a security issue,
eg. making resources available to a less privileged program allowing
information leak and/or deny of service.
- Why do one need atomic close-on-exec ?
Even if it's possible to set close-on-exec flag through call to fcntl()
as shown previously, it introduces a race condition in multi-threaded
process, where a thread call fork() / exec() while another thread
is between call to open() and fcntl().
Additionally, using close-on-exec free the programmer from tracking manually
which file descriptor is to close in a child before calling exec():
in a program using multiple third-party libraries, it's difficult to know
which file descriptor must be closed.
AFAIK, while there's a atexit(), pthread_atfork(), there's no atexec()
userspace function in libc to allow libraries to register a handler in
order to close its opened file descriptor before exec().
- Why default to close-on-exec ?
Some kernel interfaces don't allow userspace to pass a O_CLOEXEC-like
flag when creating a new file descriptor.
In such cases, if possible (see below), O_CLOEXEC must be made
the default so that userspace doesn't have to call fcntl()
which, as demonstrated previously, is open to race condition in
multi-threaded program.
- How to choose between flag 0 or O_CLOEXEC in call to
get_unused_fd_flags() (or anon_inode_getfd()) ?
Short: Will it break existing application ? Will it break kernel ABI ?
If answer is no, use O_CLOEXEC.
If answer is yes, use 0.
Long: If userspace expect to retrieve a file descriptor with plain
old Unix(tm) semantics, O_CLOEXEC must not be the default, as it
could break some applications expecting that the file descriptor
will be inherited across exec().
But for some subsystems, such as InfiniBand, KVM, VFIO, it makes
no sense to have file descriptors inherited across exec() since
those are tied to resources that will vanish when a another program
will replace the current one by mean of exec(), so it's safe to use
O_CLOEXEC in such cases.
For others, like XFS, the file descriptor is retrieved by one
program and will be used by a different program, executed as a
child. In this case, setting O_CLOEXEC would break existing
application which do not expect to have to call fcntl(fd,
F_SETFD, 0) to make it available across exec().
If file descriptor created by a subsystem is not tied to the
current process resources, it's likely legal to use it in a
different process context, thus O_CLOEXEC must not be the default.
If one, as a subsystem maintainer, cannot tell for sure that
no userspace program ever rely current behavior, eg. file descriptor
being inherited across exec(), then the default flag *must*
be kept 0 to not break application.
- This subsystem cannot be turned to use O_CLOEXEC by default:
If O_CLOEXEC cannot be made the default, it would be interesting
to think to extend the API to have a (set of) function(s) taking
a flag parameter so that userspace can atomically request close-on-exec
if it need it (and it should need it).
- Background:
One might want to read "Secure File Descriptor Handling" [2] by
Ulrich Drepper who is responsible of adding O_CLOEXEC flag on open(),
and flags alike on other syscalls.
One might also want to read PEP-446 "Make newly created file descriptors
non-inheritable" [3] by Victor Stinner since it has lot more background
information on file descriptor leaking.
One also like to read "Excuse me son, but your code is leaking !!!" [4]
by Dan Walsh for advice.
[1] http://lwn.net/Articles/412131/
[2] http://udrepper.livejournal.com/20407.html
[3] http://www.python.org/dev/peps/pep-0446/
[4] http://danwalsh.livejournal.com/53603.html
- Statistics:
In linux-next tag 20131224, they're currently:
- 32 calls to fd_install()
with one call part of anon_inode_getfd()
- 24 calls to get_unused_fd_flags()
with one call part of anon_inode_getfd()
with another part of get_unused_fd() macro
- 11 calls to anon_inode_getfd()
- 8 calls to anon_inode_getfile()
with one call part of anon_inode_getfd()
- 7 calls to get_unused_fd()
Changes from patchset v4 [PATCHSETv4]:
- rewrote cover letter following discussion with perf maintainer.
Thanks to Peter Zijlstra.
- modified a bit some commit messages.
- events: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: replaced by following patch
- perf: introduce a flag to enable close-on-exec in perf_event_open()
NEW: instead of hard coding the flags to 0, this patch
allows userspace to specify close-on-exec flag.
- fanotify: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: replaced by following patch
- fanotify: enable close-on-exec on events' fd when requested in
fanotify_init()
NEW: instead of hard coding the flags to 0, this patch
enable close-on-exec if userspace request it.
Changes from patchset v3 [PATCHSETv3]:
- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
DROPPED: applied upstream
Changes from patchset v2 [PATCHSETv2]:
- android/sw_sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
DROPPED: applied upstream
- android/sync: use get_unused_fd_flags(O_CLOEXEC) instead of get_unused_fd()
DROPPED: applied upstream
- vfio: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: applied upstream.
Additionally subsystem maintainer applied another patch on top
to set the flags to O_CLOEXEC.
- industrialio: use anon_inode_getfd() with O_CLOEXEC flag
NEW: propose to use O_CLOEXEC as default flag.
Changes from patchset v1 [PATCHSETv1]:
- explicitly added subsystem maintainers as mail recepients.
- infiniband: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: subsystem maintainer applied another patch
using get_unused_fd_flags(O_CLOEXEC) as suggested.
- android/sw_sync: use get_unused_fd_flags(0) instead of get_unused_fd()
MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested
- android/sync: use get_unused_fd_flags(0) instead of get_unused_fd()
MODIFIED: use get_unused_fd_flags(O_CLOEXEC) as suggested
- xfs: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: applied asis by subsystem maintainer.
- sctp: use get_unused_fd_flags(0) instead of get_unused_fd()
DROPPED: applied asis by subsystem maintainer.
Links:
[PATCHSETv4]
http://lkml.kernel.org/r/cover.1383121137.git.ydroneaud@opteya.com
[PATCHSETv3]
http://lkml.kernel.org/r/cover.1378460926.git.ydroneaud@opteya.com
[PATCHSETv2]
http://lkml.kernel.org/r/cover.1376327678.git.ydroneaud@opteya.com
[PATCHSETv1]
http://lkml.kernel.org/r/cover.1372777600.git.ydroneaud@opteya.com
PS: Happy new (gregorian calendar's) year 2014 and best wishes ;)
Yann Droneaud (7):
ia64: use get_unused_fd_flags(0) instead of get_unused_fd()
ppc/cell: use get_unused_fd_flags(0) instead of get_unused_fd()
binfmt_misc: use get_unused_fd_flags(0) instead of get_unused_fd()
file: use get_unused_fd_flags(0) instead of get_unused_fd()
fanotify: enable close-on-exec on events' fd when requested in
fanotify_init()
perf: introduce a flag to enable close-on-exec in perf_event_open()
file: remove macro get_unused_fd()
arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/platforms/cell/spufs/inode.c | 4 ++--
fs/binfmt_misc.c | 2 +-
fs/file.c | 2 +-
fs/notify/fanotify/fanotify_user.c | 2 +-
include/linux/file.h | 1 -
include/uapi/linux/perf_event.h | 1 +
kernel/events/core.c | 12 +++++++++---
8 files changed, 16 insertions(+), 10 deletions(-)
--
1.8.4.2
^ permalink raw reply
* [PATCH] ASoC: fsl_ssi: Fix printing return code on clk error
From: Alexander Shiyan @ 2014-01-05 6:21 UTC (permalink / raw)
To: alsa-devel
Cc: Alexander Shiyan, Liam Girdwood, Takashi Iwai, Timur Tabi,
Jaroslav Kysela, Mark Brown, linuxppc-dev
Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
sound/soc/fsl/fsl_ssi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2101fc5..3d74477a 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1192,7 +1192,8 @@ static int fsl_ssi_probe(struct platform_device *pdev)
*/
ssi_private->baudclk = devm_clk_get(&pdev->dev, "baud");
if (IS_ERR(ssi_private->baudclk))
- dev_warn(&pdev->dev, "could not get baud clock: %d\n", ret);
+ dev_warn(&pdev->dev, "could not get baud clock: %d\n",
+ PTR_ERR(ssi_private->baudclk));
else
clk_prepare_enable(ssi_private->baudclk);
--
1.8.3.2
^ permalink raw reply related
* 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
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