* Re: [PATCH v6 3/3] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-23 0:01 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537DC9AD.3090202@suse.de>
On Thu, May 22, 2014 at 11:55:57AM +0200, Alexander Graf wrote:
>
>On 22.05.14 10:23, Gavin Shan wrote:
>>If we detects frozen state on PE that has been passed through to somebody
>>else. we needn't handle it. Instead, we rely on the device's owner to
>>detect and recover it. The patch avoid EEH event on the frozen passed PE so
>>that the device's owner can have chance to handle that.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
>I think you want to fold this with patch 1/3.
>
Yes, I'll do it in next revision.
Thanks,
Gavin
>>---
>> arch/powerpc/kernel/eeh.c | 8 ++++++++
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index b90a474..aee6cc5 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -403,6 +403,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>> if (ret > 0)
>> return ret;
>>+ /*
>>+ * If the PE isn't owned by us, we shouldn't check the
>>+ * state. Instead, let the owner handle it if the PE has
>>+ * been frozen.
>>+ */
>>+ if (eeh_pe_passed(pe))
>>+ return 0;
>>+
>> /* If we already have a pending isolation event for this
>> * slot, we know it's bad already, we don't need to check.
>> * Do this checking under a lock; as multiple PCI devices
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 1b5982f..03a3ed2 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -890,7 +890,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>> opal_pci_eeh_freeze_clear(phb->opal_id, frozen_pe_no,
>> OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
>> ret = EEH_NEXT_ERR_NONE;
>>- } else if ((*pe)->state & EEH_PE_ISOLATED) {
>>+ } else if ((*pe)->state & EEH_PE_ISOLATED ||
>>+ eeh_pe_passed(*pe)) {
>> ret = EEH_NEXT_ERR_NONE;
>> } else {
>> pr_err("EEH: Frozen PHB#%x-PE#%x (%s) detected\n",
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Cody P Schafer @ 2014-05-22 23:54 UTC (permalink / raw)
To: Stephen Rothwell
Cc: peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <20140523094929.15f4613c@canb.auug.org.au>
On 05/22/2014 04:49 PM, Stephen Rothwell wrote:
> Hi Cody,
>
> On Thu, 22 May 2014 15:44:25 -0700 Cody P Schafer <cody@linux.vnet.ibm.com> wrote:
>>
>> if (ret) {
>> if (success_expected)
>> pr_err_ratelimited("hcall failed: %d %#x %#x %d => 0x%lx (%ld) detail=0x%x failing ix=%x\n",
>> domain, offset, ix, lpar,
>> ret, ret,
>> - result_buffer.buf.detailed_rc,
>> - result_buffer.buf.failing_request_ix);
>> - return ret;
>> + result_buffer->buf.detailed_rc,
>> + result_buffer->buf.failing_request_ix);
>> + goto out_hcall;
>> }
>>
>> - *res = be64_to_cpu(result_buffer.result);
>> + *res = be64_to_cpu(result_buffer->result);
>
> not a biggie, but this last bit could be (remove the goto out_hcall and
> teh label and then)
>
> } else {
> *res = be64_to_cpu(result_buffer->result);
> }
>
I've got a slight preference toward keeping it as is, which lets all of
the non-error path code stay outside of if/else blocks (and the error
handling is kept ever so slightly more consistent).
>> +out_hcall:
>> + kfree(result_buffer);
>> +out_resb:
>> + kfree(request_buffer);
>> +out_reqb:
>> return ret;
>> }
>>
>
> otherwise looks good to me.
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Stephen Rothwell @ 2014-05-22 23:49 UTC (permalink / raw)
To: Cody P Schafer
Cc: peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <1400798673-27522-1-git-send-email-cody@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
Hi Cody,
On Thu, 22 May 2014 15:44:25 -0700 Cody P Schafer <cody@linux.vnet.ibm.com> wrote:
>
> if (ret) {
> if (success_expected)
> pr_err_ratelimited("hcall failed: %d %#x %#x %d => 0x%lx (%ld) detail=0x%x failing ix=%x\n",
> domain, offset, ix, lpar,
> ret, ret,
> - result_buffer.buf.detailed_rc,
> - result_buffer.buf.failing_request_ix);
> - return ret;
> + result_buffer->buf.detailed_rc,
> + result_buffer->buf.failing_request_ix);
> + goto out_hcall;
> }
>
> - *res = be64_to_cpu(result_buffer.result);
> + *res = be64_to_cpu(result_buffer->result);
not a biggie, but this last bit could be (remove the goto out_hcall and
teh label and then)
} else {
*res = be64_to_cpu(result_buffer->result);
}
> +out_hcall:
> + kfree(result_buffer);
> +out_resb:
> + kfree(request_buffer);
> +out_reqb:
> return ret;
> }
>
otherwise looks good to me.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 1/1][v3] powerpc/perf: Adjust callchain based on DWARF debug info
From: Sukadev Bhattiprolu @ 2014-05-22 23:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: linuxppc-dev, Anton Blanchard, linux-kernel, michael,
Ulrich.Weigand, Maynard Johnson
powerpc/perf: Adjust callchain based on DWARF debug info
When saving the callchain on Power, the kernel conservatively saves excess
entries in the callchain. A few of these entries are needed in some cases
but not others. We should use the DWARF debug information to determine
when the entries are needed.
Eg: the value in the link register (LR) is needed only when it holds the
return address of a function. At other times it must be ignored.
If the unnecessary entries are not ignored, we end up with duplicate arcs
in the call-graphs.
Use the DWARF debug information to determine if any callchain entries
should be ignored when building call-graphs.
Callgraph before the patch:
14.67% 2234 sprintft libc-2.18.so [.] __random
|
--- __random
|
|--61.12%-- __random
| |
| |--97.15%-- rand
| | do_my_sprintf
| | main
| | generic_start_main.isra.0
| | __libc_start_main
| | 0x0
| |
| --2.85%-- do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--38.88%-- rand
|
|--94.01%-- rand
| do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--5.99%-- do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
0x0
Callgraph after the patch:
14.67% 2234 sprintft libc-2.18.so [.] __random
|
--- __random
|
|--95.93%-- rand
| do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--4.07%-- do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
0x0
TODO: For split-debug info objects like glibc, we can only determine
the call-frame-address only when both .eh_frame and .debug_info
sections are available. We should be able to determin the CFA
even without the .eh_frame section.
Fix suggested by Anton Blanchard.
Thanks to valuable input on DWARF debug information from Ulrich Weigand.
Reported-by: Maynard Johnson <maynard@us.ibm.com>
Tested-by: Maynard Johnson <maynard@us.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
Changelog[v3]
[Jiri Olsa] Rename function to arch_skip_callchain_idx() to be
consistent with behavior.
[Jiri Olsa] Add '__maybe_unused' tags for unused parameters.
Changelog[v2]:
- Add missing dwfl_end()
- Fix merge conflicts due to some unwind code
tools/perf/arch/powerpc/Makefile | 1 +
tools/perf/arch/powerpc/util/skip-callchain-idx.c | 266 +++++++++++++++++++++
tools/perf/config/Makefile | 4 +
tools/perf/util/callchain.h | 13 +
tools/perf/util/machine.c | 16 +-
5 files changed, 298 insertions(+), 2 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/skip-callchain-idx.c
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 744e629..b92219b 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
endif
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o
diff --git a/tools/perf/arch/powerpc/util/skip-callchain-idx.c b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
new file mode 100644
index 0000000..a7c23a4
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/skip-callchain-idx.c
@@ -0,0 +1,266 @@
+/*
+ * Use DWARF Debug information to skip unnecessary callchain entries.
+ *
+ * Copyright (C) 2014 Sukadev Bhattiprolu, IBM Corporation.
+ * Copyright (C) 2014 Ulrich Weigand, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <inttypes.h>
+#include <dwarf.h>
+#include <elfutils/libdwfl.h>
+
+#include "util/thread.h"
+#include "util/callchain.h"
+
+/*
+ * When saving the callchain on Power, the kernel conservatively saves
+ * excess entries in the callchain. A few of these entries are needed
+ * in some cases but not others. If the unnecessary entries are not
+ * ignored, we end up with duplicate arcs in the call-graphs. Use
+ * DWARF debug information to skip over any unnecessary callchain
+ * entries.
+ *
+ * See function header for arch_adjust_callchain() below for more details.
+ *
+ * The libdwfl code in this file is based on code from elfutils
+ * (libdwfl/argp-std.c, libdwfl/tests/addrcfi.c, etc).
+ */
+static char *debuginfo_path;
+
+static const Dwfl_Callbacks offline_callbacks = {
+ .debuginfo_path = &debuginfo_path,
+ .find_debuginfo = dwfl_standard_find_debuginfo,
+ .section_address = dwfl_offline_section_address,
+};
+
+
+/*
+ * Use the DWARF expression for the Call-frame-address and determine
+ * if return address is in LR and if a new frame was allocated.
+ */
+static int check_return_reg(int ra_regno, Dwarf_Frame *frame)
+{
+ Dwarf_Op ops_mem[2];
+ Dwarf_Op dummy;
+ Dwarf_Op *ops = &dummy;
+ size_t nops;
+ int result;
+
+ result = dwarf_frame_register(frame, ra_regno, ops_mem, &ops, &nops);
+ if (result < 0) {
+ pr_debug("dwarf_frame_register() %s\n", dwarf_errmsg(-1));
+ return -1;
+ }
+
+ /*
+ * Check if return address is on the stack.
+ */
+ if (nops != 0 || ops != NULL)
+ return 0;
+
+ /*
+ * Return address is in LR. Check if a frame was allocated
+ * but not-yet used.
+ */
+ result = dwarf_frame_cfa(frame, &ops, &nops);
+ if (result < 0) {
+ pr_debug("dwarf_frame_cfa() returns %d, %s\n", result,
+ dwarf_errmsg(-1));
+ return -1;
+ }
+
+ /*
+ * If call frame address is in r1, no new frame was allocated.
+ */
+ if (nops == 1 && ops[0].atom == DW_OP_bregx && ops[0].number == 1 &&
+ ops[0].number2 == 0)
+ return 1;
+
+ /*
+ * A new frame was allocated but has not yet been used.
+ */
+ return 2;
+}
+
+/*
+ * Get the DWARF frame from the .eh_frame section.
+ */
+static Dwarf_Frame *get_eh_frame(Dwfl_Module *mod, Dwarf_Addr pc)
+{
+ int result;
+ Dwarf_Addr bias;
+ Dwarf_CFI *cfi;
+ Dwarf_Frame *frame;
+
+ cfi = dwfl_module_eh_cfi(mod, &bias);
+ if (!cfi) {
+ pr_debug("%s(): no CFI - %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ result = dwarf_cfi_addrframe(cfi, pc, &frame);
+ if (result) {
+ pr_debug("%s(): %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ return frame;
+}
+
+/*
+ * Get the DWARF frame from the .debug_frame section.
+ */
+static Dwarf_Frame *get_dwarf_frame(Dwfl_Module *mod, Dwarf_Addr pc)
+{
+ Dwarf_CFI *cfi;
+ Dwarf_Addr bias;
+ Dwarf_Frame *frame;
+ int result;
+
+ cfi = dwfl_module_dwarf_cfi(mod, &bias);
+ if (!cfi) {
+ pr_debug("%s(): no CFI - %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ result = dwarf_cfi_addrframe(cfi, pc, &frame);
+ if (result) {
+ pr_debug("%s(): %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ return frame;
+}
+
+/*
+ * Return:
+ * 0 if return address for the program counter @pc is on stack
+ * 1 if return address is in LR and no new stack frame was allocated
+ * 2 if return address is in LR and a new frame was allocated (but not
+ * yet used)
+ * -1 in case of errors
+ */
+static int check_return_addr(const char *exec_file, Dwarf_Addr pc)
+{
+ int rc = -1;
+ Dwfl *dwfl;
+ Dwfl_Module *mod;
+ Dwarf_Frame *frame;
+ int ra_regno;
+ Dwarf_Addr start = pc;
+ Dwarf_Addr end = pc;
+ bool signalp;
+
+ dwfl = dwfl_begin(&offline_callbacks);
+ if (!dwfl) {
+ pr_debug("dwfl_begin() failed: %s\n", dwarf_errmsg(-1));
+ return -1;
+ }
+
+ if (dwfl_report_offline(dwfl, "", exec_file, -1) == NULL) {
+ pr_debug("dwfl_report_offline() failed %s\n", dwarf_errmsg(-1));
+ goto out;
+ }
+
+ mod = dwfl_addrmodule(dwfl, pc);
+ if (!mod) {
+ pr_debug("dwfl_addrmodule() failed, %s\n", dwarf_errmsg(-1));
+ goto out;
+ }
+
+ /*
+ * To work with split debug info files (eg: glibc), check both
+ * .eh_frame and .debug_frame sections of the ELF header.
+ */
+ frame = get_eh_frame(mod, pc);
+ if (!frame) {
+ frame = get_dwarf_frame(mod, pc);
+ if (!frame)
+ goto out;
+ }
+
+ ra_regno = dwarf_frame_info(frame, &start, &end, &signalp);
+ if (ra_regno < 0) {
+ pr_debug("Return address register unavailable: %s\n",
+ dwarf_errmsg(-1));
+ goto out;
+ }
+
+ rc = check_return_reg(ra_regno, frame);
+
+out:
+ dwfl_end(dwfl);
+ return rc;
+}
+
+/*
+ * The callchain saved by the kernel always includes the link register (LR).
+ *
+ * 0: PERF_CONTEXT_USER
+ * 1: Program counter (Next instruction pointer)
+ * 2: LR value
+ * 3: Caller's caller
+ * 4: ...
+ *
+ * The value in LR is only needed when it holds a return address. If the
+ * return address is on the stack, we should ignore the LR value.
+ *
+ * Further, when the return address is in the LR, if a new frame was just
+ * allocated but the LR was not saved into it, then the LR contains the
+ * caller, slot 4: contains the caller's caller and the contents of slot 3:
+ * (chain->ips[3]) is undefined and must be ignored.
+ *
+ * Use DWARF debug information to determine if any entries need to be skipped.
+ *
+ * Return:
+ * index: of callchain entry that needs to be ignored (if any)
+ * -1 if no entry needs to be ignored or in case of errors
+ */
+int arch_skip_callchain_idx(struct machine *machine, struct thread *thread,
+ struct ip_callchain *chain)
+{
+ struct addr_location al;
+ struct dso *dso = NULL;
+ int rc;
+ u64 ip;
+ u64 skip_slot = -1;
+
+ if (chain->nr < 3)
+ return skip_slot;
+
+ ip = chain->ips[2];
+
+ thread__find_addr_location(thread, machine, PERF_RECORD_MISC_USER,
+ MAP__FUNCTION, ip, &al);
+
+ if (al.map)
+ dso = al.map->dso;
+
+ if (!dso) {
+ pr_debug("%" PRIx64 " dso is NULL\n", ip);
+ return skip_slot;
+ }
+
+ rc = check_return_addr(dso->long_name, ip);
+
+ pr_debug("DSO %s, nr %" PRIx64 ", ip 0x%" PRIx64 "rc %d\n",
+ dso->long_name, chain->nr, ip, rc);
+
+ if (rc == 0) {
+ /*
+ * Return address on stack. Ignore LR value in callchain
+ */
+ skip_slot = 2;
+ } else if (rc == 2) {
+ /*
+ * New frame allocated but return address still in LR.
+ * Ignore the caller's caller entry in callchain.
+ */
+ skip_slot = 3;
+ }
+ return skip_slot;
+}
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 729bbdf..8d1417d 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -48,6 +48,10 @@ ifneq ($(ARCH),$(filter $(ARCH),x86 arm))
NO_LIBDW_DWARF_UNWIND := 1
endif
+ifeq ($(ARCH),powerpc)
+ CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
+endif
+
ifeq ($(LIBUNWIND_LIBS),)
NO_LIBUNWIND := 1
else
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index bde2b0c..5176599 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -165,4 +165,17 @@ int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *samp
extern const char record_callchain_help[];
int parse_callchain_report_opt(const char *arg);
+
+#ifdef HAVE_SKIP_CALLCHAIN_IDX
+extern int arch_skip_callchain_idx(struct machine *machine,
+ struct thread *thread, struct ip_callchain *chain);
+#else
+static inline int arch_skip_callchain_idx(struct machine *machine __maybe_unused,
+ struct thread *thread __maybe_unused,
+ struct ip_callchain *chain __maybe_unused)
+{
+ return -1;
+}
+#endif
+
#endif /* __PERF_CALLCHAIN_H */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7409ac8..bc93279 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1290,6 +1290,7 @@ static int machine__resolve_callchain_sample(struct machine *machine,
int chain_nr = min(max_stack, (int)chain->nr);
int i;
int err;
+ int skip_slot;
callchain_cursor_reset(&callchain_cursor);
@@ -1298,14 +1299,25 @@ static int machine__resolve_callchain_sample(struct machine *machine,
return 0;
}
+ /*
+ * Based on DWARF debug information, some architectures skip
+ * some of the callchain entries saved by the kernel.
+ */
+ skip_slot = arch_skip_callchain_idx(machine, thread, chain);
+
for (i = 0; i < chain_nr; i++) {
u64 ip;
struct addr_location al;
- if (callchain_param.order == ORDER_CALLEE)
+ if (callchain_param.order == ORDER_CALLEE) {
+ if (i == skip_slot)
+ continue;
ip = chain->ips[i];
- else
+ } else {
+ if ((int)(chain->nr - i - 1) == skip_slot)
+ continue;
ip = chain->ips[chain->nr - i - 1];
+ }
if (ip >= PERF_CONTEXT_MAX) {
switch (ip) {
--
1.7.9.5
^ permalink raw reply related
* Re: powerpc: remove checks for CONFIG_BOOK3E_MMU_TLB_STATS
From: Scott Wood @ 2014-05-22 22:37 UTC (permalink / raw)
To: Paul Bolle; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1400608550.4912.48.camel@x220>
On Tue, May 20, 2014 at 07:55:50PM +0200, Paul Bolle wrote:
> Three checks for CONFIG_BOOK3E_MMU_TLB_STATS were added in v2.6.32. But
> the related Kconfig symbol was never added. These checks have always
> evaluated to false. Remove them.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
>
> ---
> Untested.
>
> A second order cleanup is now possible (ie, TLB_MISS_PROLOG_STATS and
> friends are, well, defined away). Perhaps it is preferable to include
> that cleanup in this patch.
>
> arch/powerpc/include/asm/exception-64e.h | 38 --------------------------------
> arch/powerpc/mm/tlb_low_64e.S | 10 ---------
> 2 files changed, 48 deletions(-)
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S: Assembler messages:
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:89: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:238: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:269: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:281: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:441: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:510: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:881: Error: unrecognized opcode: `tlb_miss_prolog_stats'
/home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:918: Error: unrecognized opcode: `tlb_miss_prolog_stats'
-Scott
^ permalink raw reply
* Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
From: Scott Wood @ 2014-05-22 22:51 UTC (permalink / raw)
To: Shengzhou Liu; +Cc: linuxppc-dev
In-Reply-To: <1400666751-11459-2-git-send-email-Shengzhou.Liu@freescale.com>
On Wed, May 21, 2014 at 06:05:51PM +0800, Shengzhou Liu wrote:
> Enable RTC support for DS1307, DS1374, DS3232, which is
> needed on some corenet boards.
>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>
> ---
> arch/powerpc/configs/corenet64_smp_defconfig | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/configs/corenet64_smp_defconfig b/arch/powerpc/configs/corenet64_smp_defconfig
> index 63508dd..e9c9f86 100644
> --- a/arch/powerpc/configs/corenet64_smp_defconfig
> +++ b/arch/powerpc/configs/corenet64_smp_defconfig
> @@ -125,6 +125,11 @@ CONFIG_USB_EHCI_FSL=y
> CONFIG_USB_STORAGE=y
> CONFIG_MMC=y
> CONFIG_MMC_SDHCI=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_CMOS=y
> +CONFIG_RTC_DRV_DS1307=y
> +CONFIG_RTC_DRV_DS1374=y
> +CONFIG_RTC_DRV_DS3232=y
> CONFIG_EDAC=y
> CONFIG_EDAC_MM_EDAC=y
> CONFIG_DMADEVICES=y
Why only corenet64 and not corenet32?
-Scott
^ permalink raw reply
* [PATCH v2] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Cody P Schafer @ 2014-05-22 22:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Cody P Schafer, Michael Ellerman
Cc: sfr, peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <537E78AD.1040409@linux.vnet.ibm.com>
Ian pointed out the use of __aligned(4096) caused rather large stack
consumption in single_24x7_request(), so use the kmem_cache
hv_page_cache (which we've already got set up for other allocations)
insead of allocating locally.
Reported-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
In v2:
- remove duplicate exit path
arch/powerpc/perf/hv-24x7.c | 48 +++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index e0766b8..998863b 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -294,7 +294,7 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
u16 lpar, u64 *res,
bool success_expected)
{
- unsigned long ret;
+ unsigned long ret = -ENOMEM;
/*
* request_buffer and result_buffer are not required to be 4k aligned,
@@ -304,7 +304,27 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
struct reqb {
struct hv_24x7_request_buffer buf;
struct hv_24x7_request req;
- } __packed __aligned(4096) request_buffer = {
+ } __packed *request_buffer;
+ struct resb {
+ struct hv_24x7_data_result_buffer buf;
+ struct hv_24x7_result res;
+ struct hv_24x7_result_element elem;
+ __be64 result;
+ } __packed *result_buffer;
+
+ BUILD_BUG_ON(sizeof(*request_buffer) > 4096);
+ BUILD_BUG_ON(sizeof(*result_buffer) > 4096);
+
+ request_buffer = kmem_cache_alloc(hv_page_cache, GFP_USER);
+
+ if (!request_buffer)
+ goto out_reqb;
+
+ result_buffer = kmem_cache_zalloc(hv_page_cache, GFP_USER);
+ if (!result_buffer)
+ goto out_resb;
+
+ *request_buffer = (struct reqb) {
.buf = {
.interface_version = HV_24X7_IF_VERSION_CURRENT,
.num_requests = 1,
@@ -320,28 +340,26 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
}
};
- struct resb {
- struct hv_24x7_data_result_buffer buf;
- struct hv_24x7_result res;
- struct hv_24x7_result_element elem;
- __be64 result;
- } __packed __aligned(4096) result_buffer = {};
-
ret = plpar_hcall_norets(H_GET_24X7_DATA,
- virt_to_phys(&request_buffer), sizeof(request_buffer),
- virt_to_phys(&result_buffer), sizeof(result_buffer));
+ virt_to_phys(request_buffer), sizeof(*request_buffer),
+ virt_to_phys(result_buffer), sizeof(*result_buffer));
if (ret) {
if (success_expected)
pr_err_ratelimited("hcall failed: %d %#x %#x %d => 0x%lx (%ld) detail=0x%x failing ix=%x\n",
domain, offset, ix, lpar,
ret, ret,
- result_buffer.buf.detailed_rc,
- result_buffer.buf.failing_request_ix);
- return ret;
+ result_buffer->buf.detailed_rc,
+ result_buffer->buf.failing_request_ix);
+ goto out_hcall;
}
- *res = be64_to_cpu(result_buffer.result);
+ *res = be64_to_cpu(result_buffer->result);
+out_hcall:
+ kfree(result_buffer);
+out_resb:
+ kfree(request_buffer);
+out_reqb:
return ret;
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Cody P Schafer @ 2014-05-22 22:43 UTC (permalink / raw)
To: Stephen Rothwell
Cc: peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <20140523083847.4e039ddb@canb.auug.org.au>
On 05/22/2014 03:38 PM, Stephen Rothwell wrote:
> Hi Cody,
>
> On Thu, 22 May 2014 15:29:08 -0700 Cody P Schafer <cody@linux.vnet.ibm.com> wrote:
>>
>> - *res = be64_to_cpu(result_buffer.result);
>> + *res = be64_to_cpu(result_buffer->result);
>> + kfree(result_buffer);
>> + kfree(request_buffer);
>> + return ret;
>
> Why not just fall through here by removing the above 3 lines?
No reason except me not noticing it.
>> +
>> +out_hcall:
>> + kfree(result_buffer);
>> +out_resb:
>> + kfree(request_buffer);
>> +out_reqb:
>> return ret;
>> }
>
^ permalink raw reply
* Re: [PATCH] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Stephen Rothwell @ 2014-05-22 22:38 UTC (permalink / raw)
To: Cody P Schafer
Cc: peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <1400797756-6057-1-git-send-email-cody@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
Hi Cody,
On Thu, 22 May 2014 15:29:08 -0700 Cody P Schafer <cody@linux.vnet.ibm.com> wrote:
>
> - *res = be64_to_cpu(result_buffer.result);
> + *res = be64_to_cpu(result_buffer->result);
> + kfree(result_buffer);
> + kfree(request_buffer);
> + return ret;
Why not just fall through here by removing the above 3 lines?
> +
> +out_hcall:
> + kfree(result_buffer);
> +out_resb:
> + kfree(request_buffer);
> +out_reqb:
> return ret;
> }
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] powerpc/perf/hv-24x7: use kmem_cache instead of aligned stack allocations
From: Cody P Schafer @ 2014-05-22 22:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Cody P Schafer, Michael Ellerman
Cc: sfr, peterz, linux-kernel, michael, mingo, paulus, imunsie, acme,
scottwood, linuxppc-dev
In-Reply-To: <537E78AD.1040409@linux.vnet.ibm.com>
Ian pointed out the use of __aligned(4096) caused rather large stack
consumption in single_24x7_request(), so use the kmem_cache
hv_page_cache (which we've already got set up for other allocations)
insead of allocating locally.
Reported-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 52 ++++++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index e0766b8..9a7a830 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -294,7 +294,7 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
u16 lpar, u64 *res,
bool success_expected)
{
- unsigned long ret;
+ unsigned long ret = -ENOMEM;
/*
* request_buffer and result_buffer are not required to be 4k aligned,
@@ -304,7 +304,27 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
struct reqb {
struct hv_24x7_request_buffer buf;
struct hv_24x7_request req;
- } __packed __aligned(4096) request_buffer = {
+ } __packed *request_buffer;
+ struct resb {
+ struct hv_24x7_data_result_buffer buf;
+ struct hv_24x7_result res;
+ struct hv_24x7_result_element elem;
+ __be64 result;
+ } __packed *result_buffer;
+
+ BUILD_BUG_ON(sizeof(*request_buffer) > 4096);
+ BUILD_BUG_ON(sizeof(*result_buffer) > 4096);
+
+ request_buffer = kmem_cache_alloc(hv_page_cache, GFP_USER);
+
+ if (!request_buffer)
+ goto out_reqb;
+
+ result_buffer = kmem_cache_zalloc(hv_page_cache, GFP_USER);
+ if (!result_buffer)
+ goto out_resb;
+
+ *request_buffer = (struct reqb) {
.buf = {
.interface_version = HV_24X7_IF_VERSION_CURRENT,
.num_requests = 1,
@@ -320,28 +340,30 @@ static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
}
};
- struct resb {
- struct hv_24x7_data_result_buffer buf;
- struct hv_24x7_result res;
- struct hv_24x7_result_element elem;
- __be64 result;
- } __packed __aligned(4096) result_buffer = {};
-
ret = plpar_hcall_norets(H_GET_24X7_DATA,
- virt_to_phys(&request_buffer), sizeof(request_buffer),
- virt_to_phys(&result_buffer), sizeof(result_buffer));
+ virt_to_phys(request_buffer), sizeof(*request_buffer),
+ virt_to_phys(result_buffer), sizeof(*result_buffer));
if (ret) {
if (success_expected)
pr_err_ratelimited("hcall failed: %d %#x %#x %d => 0x%lx (%ld) detail=0x%x failing ix=%x\n",
domain, offset, ix, lpar,
ret, ret,
- result_buffer.buf.detailed_rc,
- result_buffer.buf.failing_request_ix);
- return ret;
+ result_buffer->buf.detailed_rc,
+ result_buffer->buf.failing_request_ix);
+ goto out_hcall;
}
- *res = be64_to_cpu(result_buffer.result);
+ *res = be64_to_cpu(result_buffer->result);
+ kfree(result_buffer);
+ kfree(request_buffer);
+ return ret;
+
+out_hcall:
+ kfree(result_buffer);
+out_resb:
+ kfree(request_buffer);
+out_reqb:
return ret;
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v4 09/11] powerpc/perf: add support for the hv 24x7 interface
From: Cody P Schafer @ 2014-05-22 22:22 UTC (permalink / raw)
To: Ian Munsie
Cc: Stephen Rothwell, Peter Zijlstra, LKML, Michael Ellerman,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo, scottwood,
linuxppc-dev
In-Reply-To: <1400744950-sup-4804@delenn.ozlabs.ibm.com>
On 05/22/2014 01:19 AM, Ian Munsie wrote:
> Hi Cody,
>
> I just tried building this with gcc 4.5, which failed with the following
> warning (treated as an error):
>
> cc1: warnings being treated as errors
> arch/powerpc/perf/hv-24x7.c: In function 'single_24x7_request':
> arch/powerpc/perf/hv-24x7.c:346:1: error: the frame size of 8192 bytes is larger than 2048 bytes
> make[3]: *** [arch/powerpc/perf/hv-24x7.o] Error 1
> make[2]: *** [arch/powerpc/perf] Error 2
>
> My .config has CONFIG_FRAME_WARN=2048 (default on 64bit), but the
> alignment constraints in this function may require 8K on the stack -
> possibly a bit large?
>
Yep, it is a bit large. In other places in hv-24x7 that use similar
firmware interfaces (with similar alignment requirements), I've used a
kmem_cache (hv_page_cache). Testing out a patch that uses that here as well.
>
> Notably for some reason this warning no longer seems to trigger on gcc
> 4.8 (or at least somewhere between 4.5-4.8), though the assembly does
> still show it aligning the buffers.
That's a bit concerning (and might be why I didn't pick it up, using gcc
4.9.0 over here). Looking at the gcc docs, it seems to indicate that
alloca() and VLAs aren't counted for -Wframe-larger-than. Perhaps gcc
decided to move locally defined structures with alignment requirements
into that same bucket? (while size of the structures is statically
determinable, the stack consumption due to alignment is [to some degree]
variable).
^ permalink raw reply
* [PATCH] arch: powerpc: kernel: fadump.c: Cleaning up inconsistent NULL checks
From: Rickard Strandqvist @ 2014-05-22 22:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel, Rickard Strandqvist
Cleaning up inconsistent NULL checks.
There is otherwise a risk of a possible null pointer dereference.
Was largely found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
arch/powerpc/kernel/fadump.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 2230fd0..a76e7a4 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -645,7 +645,7 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
}
/* Lower 4 bytes of reg_value contains logical cpu id */
cpu = reg_entry->reg_value & FADUMP_CPU_ID_MASK;
- if (!cpumask_test_cpu(cpu, &fdh->cpu_online_mask)) {
+ if (fdh && !cpumask_test_cpu(cpu, &fdh->cpu_online_mask)) {
SKIP_TO_NEXT_CPU(reg_entry);
continue;
}
@@ -662,9 +662,11 @@ static int __init fadump_build_cpu_notes(const struct fadump_mem_struct *fdm)
}
fadump_final_note(note_buf);
- pr_debug("Updating elfcore header (%llx) with cpu notes\n",
+ if (fdh) {
+ pr_debug("Updating elfcore header (%llx) with cpu notes\n",
fdh->elfcorehdr_addr);
- fadump_update_elfcore_header((char *)__va(fdh->elfcorehdr_addr));
+ fadump_update_elfcore_header((char *)__va(fdh->elfcorehdr_addr));
+ }
return 0;
error_out:
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] powerpc: Don't corrupt user registers on 32-bit
From: shiva7 @ 2014-05-22 21:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20131023084002.GA8325@iris.ozlabs.ibm.com>
Paul Mackerras wrote
> Commit de79f7b9f6 ("powerpc: Put FP/VSX and VR state into structures")
> modified load_up_fpu() and load_up_altivec() in such a way that they
> now use r7 and r8. Unfortunately, the callers of these functions on
> 32-bit machines then return to userspace via fast_exception_return,
> which doesn't restore all of the volatile GPRs, but only r1, r3 -- r6
> and r9 -- r12. This was causing userspace segfaults and other
> userspace misbehaviour on 32-bit machines.
>
> This fixes the problem by changing the register usage of load_up_fpu()
> and load_up_altivec() to avoid using r7 and r8 and instead use r6 and
> r10. This also adds comments to those functions saying which registers
> may be used.
>
> Signed-off-by: Paul Mackerras <
> paulus@
> >
> ---
> arch/powerpc/kernel/fpu.S | 14 ++++++++------
> arch/powerpc/kernel/vector.S | 15 +++++++++------
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
> index 4dca05e..f7f5b8b 100644
> --- a/arch/powerpc/kernel/fpu.S
> +++ b/arch/powerpc/kernel/fpu.S
> @@ -106,6 +106,8 @@ _GLOBAL(store_fp_state)
> * and save its floating-point registers in its thread_struct.
> * Load up this task's FP registers from its thread_struct,
> * enable the FPU for the current task and return to the task.
> + * Note that on 32-bit this can only use registers that will be
> + * restored by fast_exception_return, i.e. r3 - r6, r10 and r11.
> */
> _GLOBAL(load_up_fpu)
> mfmsr r5
> @@ -131,10 +133,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> beq 1f
> toreal(r4)
> addi r4,r4,THREAD /* want last_task_used_math->thread */
> - addi r8,r4,THREAD_FPSTATE
> - SAVE_32FPVSRS(0, R5, R8)
> + addi r10,r4,THREAD_FPSTATE
> + SAVE_32FPVSRS(0, R5, R10)
> mffs fr0
> - stfd fr0,FPSTATE_FPSCR(r8)
> + stfd fr0,FPSTATE_FPSCR(r10)
> PPC_LL r5,PT_REGS(r4)
> toreal(r5)
> PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5)
> @@ -157,10 +159,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> or r12,r12,r4
> std r12,_MSR(r1)
> #endif
> - addi r7,r5,THREAD_FPSTATE
> - lfd fr0,FPSTATE_FPSCR(r7)
> + addi r10,r5,THREAD_FPSTATE
> + lfd fr0,FPSTATE_FPSCR(r10)
> MTFSF_L(fr0)
> - REST_32FPVSRS(0, R4, R7)
> + REST_32FPVSRS(0, R4, R10)
> #ifndef CONFIG_SMP
> subi r4,r5,THREAD
> fromreal(r4)
> diff --git a/arch/powerpc/kernel/vector.S b/arch/powerpc/kernel/vector.S
> index eacda4e..0458a9a 100644
> --- a/arch/powerpc/kernel/vector.S
> +++ b/arch/powerpc/kernel/vector.S
> @@ -64,6 +64,9 @@ _GLOBAL(store_vr_state)
> * Enables the VMX for use in the kernel on return.
> * On SMP we know the VMX is free, since we give it up every
> * switch (ie, no lazy save of the vector registers).
> + *
> + * Note that on 32-bit this can only use registers that will be
> + * restored by fast_exception_return, i.e. r3 - r6, r10 and r11.
> */
> _GLOBAL(load_up_altivec)
> mfmsr r5 /* grab the current MSR */
> @@ -89,11 +92,11 @@ _GLOBAL(load_up_altivec)
> /* Save VMX state to last_task_used_altivec's THREAD struct */
> toreal(r4)
> addi r4,r4,THREAD
> - addi r7,r4,THREAD_VRSTATE
> - SAVE_32VRS(0,r5,r7)
> + addi r6,r4,THREAD_VRSTATE
> + SAVE_32VRS(0,r5,r6)
> mfvscr vr0
> li r10,VRSTATE_VSCR
> - stvx vr0,r10,r7
> + stvx vr0,r10,r6
> /* Disable VMX for last_task_used_altivec */
> PPC_LL r5,PT_REGS(r4)
> toreal(r5)
> @@ -125,13 +128,13 @@ _GLOBAL(load_up_altivec)
> oris r12,r12,MSR_VEC@h
> std r12,_MSR(r1)
> #endif
> - addi r7,r5,THREAD_VRSTATE
> + addi r6,r5,THREAD_VRSTATE
> li r4,1
> li r10,VRSTATE_VSCR
> stw r4,THREAD_USED_VR(r5)
> - lvx vr0,r10,r7
> + lvx vr0,r10,r6
> mtvscr vr0
> - REST_32VRS(0,r4,r7)
> + REST_32VRS(0,r4,r6)
> #ifndef CONFIG_SMP
> /* Update last_task_used_altivec to 'current' */
> subi r4,r5,THREAD /* Back to 'current' */
> --
> 1.8.4.rc3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@.ozlabs
> https://lists.ozlabs.org/listinfo/linuxppc-dev
By any chance, same corruption is happening for DEBUG_DEBUG exception ?
because I could see similar SEGV but dont have any code/program to prove it
:(
--
View this message in context: http://linuxppc.10917.n7.nabble.com/PATCH-powerpc-Don-t-corrupt-user-registers-on-32-bit-tp77443p82590.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH 2/2] powerpc/e6500: hw tablewalk: fix recursive tlb lock on cpu 0
From: Scott Wood @ 2014-05-22 21:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <1400795101-8737-1-git-send-email-scottwood@freescale.com>
Commit 82d86de25b9c99db546e17c6f7ebf9a691da557e "TLB lock recursive"
introduced a bug whereby cpu 0 uses the same value for "lock held" as
is used to indicate that the lock is free. This means that cpu 1 can
acquire the lock whenever it wants, regardless of whether cpu 0 has it
locked, which in turn means we can get duplicate TLB entries.
Add one to the CPU value to ensure we do not use zero as a "lock held"
value.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/mm/tlb_low_64e.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
index 3298d10..ba3ba3c 100644
--- a/arch/powerpc/mm/tlb_low_64e.S
+++ b/arch/powerpc/mm/tlb_low_64e.S
@@ -309,6 +309,7 @@ tlb_miss_common_e6500:
lhz r10,PACAPACAINDEX(r13)
cmpdi r15,0
cmpdi cr1,r15,1 /* set cr1.eq = 0 for non-recursive */
+ addi r10,r10,1
bne 2f
stbcx. r10,0,r11
bne 1b
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/e6500: hw tablewalk: clear TID in kernel indirect entries
From: Scott Wood @ 2014-05-22 21:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Scott Wood
Previously TID was being cleared before the tlbsx, but not after. This
can lead to a multiway hit between a TLB entry with TID=0 (previously
inserted when PID=0) and a TLB entry with TID!=0 that matches PID.
This can theoretically result in undefined behavior, though we probably
get lucky due to the details of the overlap. It also results in the
inability to use multihit detection to detect other conflicting TLB
entries, as well as poorer TLB utilization due to duplicating kernel
TLB entries.
Rather than try to patch up MAS1 after tlbsx, the entire value is
saved/restored as with MAS2.
I observed a slight improvement in TLB miss performance with this patch
applied.
Signed-off-by: Scott Wood <scottwood@freescale.com>
Reported-by: Ed Swarthout <ed.swarthout@freescale.com>
---
arch/powerpc/mm/tlb_low_64e.S | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
index 356e8b4..3298d10 100644
--- a/arch/powerpc/mm/tlb_low_64e.S
+++ b/arch/powerpc/mm/tlb_low_64e.S
@@ -322,19 +322,17 @@ tlb_miss_common_e6500:
b 1b
.previous
- mfspr r15,SPRN_MAS2
+ mfspr r15,SPRN_MAS1
+ mfspr r10,SPRN_MAS2
tlbsx 0,r16
+ mtspr SPRN_MAS2,r10
mfspr r10,SPRN_MAS1
+ mtspr SPRN_MAS1,r15
+
andis. r10,r10,MAS1_VALID@h
bne tlb_miss_done_e6500
- /* Undo MAS-damage from the tlbsx */
- mfspr r10,SPRN_MAS1
- oris r10,r10,MAS1_VALID@h
- mtspr SPRN_MAS1,r10
- mtspr SPRN_MAS2,r15
-
/* Now, we need to walk the page tables. First check if we are in
* range.
*/
--
1.9.1
^ permalink raw reply related
* Re: NUMA topology question wrt. d4edc5b6
From: Srivatsa S. Bhat @ 2014-05-22 20:48 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Dave Hansen, Srikar Dronamraju,
linuxppc-dev@lists.ozlabs.org list, Linux MM, Aneesh Kumar K.V,
nfont, Cody P Schafer, Anton Blanchard
In-Reply-To: <20140521200451.GB5755@linux.vnet.ibm.com>
[ Adding a few more CC's ]
On 05/22/2014 01:34 AM, Nishanth Aravamudan wrote:
> Hi Srivatsa,
>
> After d4edc5b6 ("powerpc: Fix the setup of CPU-to-Node mappings during
> CPU online"), cpu_to_node() looks like:
>
> static inline int cpu_to_node(int cpu)
> {
> int nid;
>
> nid = numa_cpu_lookup_table[cpu];
>
> /*
> * During early boot, the numa-cpu lookup table might not have been
> * setup for all CPUs yet. In such cases, default to node 0.
> */
> return (nid < 0) ? 0 : nid;
> }
>
> However, I'm curious if this is correct in all cases. I have seen
> several LPARs that do not have any CPUs on node 0. In fact, because node
> 0 is statically set online in the initialization of the N_ONLINE
> nodemask, 0 is always present to Linux, whether it is present on the
> system. I'm not sure what the best thing to do here is, but I'm curious
> if you have any ideas? I would like to remove the static initialization
> of node 0, as it's confusing to users to see an empty node (particularly
> when it's completely separate in the numbering from other nodes), but
> we trip a panic (refer to:
> http://www.spinics.net/lists/linux-mm/msg73321.html).
>
Ah, I see. I didn't have any particular reason to default it to zero.
I just did that because the existing code before this patch did the same
thing. (numa_cpu_lookup_table[] is a global array, so it will be initialized
with zeros. So if we access it before populating it via numa_setup_cpu(),
it would return 0. So I retained that behaviour with the above conditional).
Will something like the below [totally untested] patch solve the boot-panic?
I understand that as of today first_online_node will still pick 0 since
N_ONLINE is initialized statically, but with your proposed change to that
init code, I guess the following patch should avoid the boot panic.
[ But note that first_online_node is hard-coded to 0, if MAX_NUMNODES is = 1.
So we'll have to fix that if powerpc can have a single node system whose node
is numbered something other than 0. Can that happen as well? ]
And regarding your question about what is the best way to fix this whole Linux
MM's assumption about node0, I'm not really sure.. since I am not really aware
of the extent to which the MM subsystem is intertwined with this assumption
and what it would take to cure that :-(
Regards,
Srivatsa S. Bhat
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index c920215..58e6469 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -18,6 +18,7 @@ struct device_node;
*/
#define RECLAIM_DISTANCE 10
+#include <linux/nodemask.h>
#include <asm/mmzone.h>
static inline int cpu_to_node(int cpu)
@@ -30,7 +31,7 @@ static inline int cpu_to_node(int cpu)
* During early boot, the numa-cpu lookup table might not have been
* setup for all CPUs yet. In such cases, default to node 0.
*/
- return (nid < 0) ? 0 : nid;
+ return (nid < 0) ? first_online_node : nid;
}
#define parent_node(node) (node)
^ permalink raw reply related
* Re: [PATCH net-next v2 0/9] net: of_phy_connect_fixed_link removal
From: David Miller @ 2014-05-22 19:18 UTC (permalink / raw)
To: f.fainelli
Cc: thomas.petazzoni, devicetree, aida.mynzhasova, sergei.shtylyov,
netdev, richardcochran, linux-kernel, claudiu.manoil, vbordug,
grant.likely, linuxppc-dev
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 22 May 2014 09:47:42 -0700
> This patch set removes of_phy_connect_fixed_link() from the tree now that
> we have a better solution for dealing with fixed PHY (emulated PHY) devices
> for drivers that require them.
>
> First two patches update the 'fixed-link' Device Tree binding and drivers to
> refere to it.
>
> Patches 3 to 7 update the in-tree network drivers that use
> of_phy_connect_fixed_link()
>
> Patch 8 removes of_phy_connect_fixed_link
>
> Patch 9 removes the PowerPC code that parsed the 'fixed-link' property.
>
> Patch 9 can be merged via the net-next tree if the PowerPC folks ack it,
> but it really has to be merged after the first 8 patches in order to avoid
> breakage.
Series applied, if someone doesn't like patch #9 we can fix it up with a
followup patch or revert.
^ permalink raw reply
* Re: [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Sergei Shtylyov @ 2014-05-22 17:21 UTC (permalink / raw)
To: Florian Fainelli
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
netdev, Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., David Miller
In-Reply-To: <CAGVrzcbBQFhw-dUh+tFm8H5m9-fmVgutChS8wUG-F6sTBoRq-g@mail.gmail.com>
Hello.
On 05/22/2014 02:02 AM, Florian Fainelli wrote:
>>> of_phy_connect_fixed_link() is becoming obsolete, and also required
>>> platform code to register the fixed PHYs at the specified addresses for
>>> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
>>> plus of_phy_register_fixed_link() helpers to transition over the new
>>> scheme.
>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>> ---
>>> drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
>>> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
>>> 2 files changed, 16 insertions(+), 2 deletions(-)
>>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>>> index d40c5b969e9e..dc708a888f80 100644
>>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>>> @@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
>>> /* Read CRC forward */
>>> priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
>>>
>>> - priv->phydev = of_phy_connect_fixed_link(dev,
>>> bcm_sysport_adj_link,
>>> -
>>> priv->phy_interface);
>>> + priv->phydev = of_phy_connect(dev, priv->phy_dn,
>>> bcm_sysport_adj_link,
>>> + 0, priv->phy_interface);
>> The continuation line should start on the next character after ( on the
>> above line, according to the networking coding style.
> Unless I am once again not following the coding style, the patch in
> patchwork has this correctly, and so does my file locally:
> http://patchwork.ozlabs.org/patch/351323/
Don't know about your local file but the patchwork still has the
indentation wrong. -- you've used only tabs while the last several characters
should have been spaces. However, if David himself finds it correct, I guess I
just need to shut up. :-)
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/perf: Adjust callchain based on DWARF debug
From: Sukadev Bhattiprolu @ 2014-05-22 17:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: linuxppc-dev, Anton Blanchard, linux-kernel,
Arnaldo Carvalho de Melo, michael, Ulrich.Weigand,
Maynard Johnson
In-Reply-To: <20140522100045.GA4789@krava.brq.redhat.com>
Jiri Olsa [jolsa@redhat.com] wrote:
|
| yep, that sounds more clear to me.. something like below?
|
| calling callchain_dup from within arch_adjust_callchain in case
| you want to change it and returning != 0 in this case, so
| we could free the new callchain
Agree.
|
| but it might be to much overhead in case you have the support to skip
| just one entry now right? is there a plan for more cleaning? ;)
Its only one entry AFACIT. No plans for other changes, so will rename
the function as you mention in the other message and will resend.
Thanks,
Sukadev
^ permalink raw reply
* Re: [PATCH] powerpc: fix typo 'CONFIG_PMAC'
From: Andreas Schwab @ 2014-05-22 16:47 UTC (permalink / raw)
To: Paul Bolle; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1400742620.21254.22.camel@x220>
Paul Bolle <pebolle@tiscali.nl> writes:
> Do you want to know how to test this patch on a 32 bit powermac? Ie, see
> if it has any effect, and whether that effect improves things or make
> things worse.
Yes.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH net-next v2 9/9] powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
Parsing and registration of fixed PHY devices was needed with the use of
of_phy_connect_fixed_link() because this function was using the
designated PHY address identifier (first cell of the property) as the
address to bind the PHY on the emulated bus.
Since commit 3be2a49e5c08d268f8af0dd4fe89a24ea8cdc339 ("of: provide a
binding for fixed link PHYs") a new pair of functions has been
introduced which allows for dynamic address allocation of these fixed
PHY devices, but also parses the old 'fixed-link' 5-digit property.
Registration of fixed PHY early in platform code was needed because we
could not issue a fixed MDIO bus re-scan within network drivers. The
fixed PHYs had to be registered before the network drivers would call
of_phy_connect_fixed_link(). All of these caveats are solved now, such
that we can safely remove of_add_fixed_phys() now.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
arch/powerpc/sysdev/fsl_soc.c | 32 --------------------------------
1 file changed, 32 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 228cf91b91c1..ffd1169ebaab 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -25,7 +25,6 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
#include <linux/spi/spi.h>
#include <linux/fsl_devices.h>
#include <linux/fs_enet_pd.h>
@@ -178,37 +177,6 @@ u32 get_baudrate(void)
EXPORT_SYMBOL(get_baudrate);
#endif /* CONFIG_CPM2 */
-#ifdef CONFIG_FIXED_PHY
-static int __init of_add_fixed_phys(void)
-{
- int ret;
- struct device_node *np;
- u32 *fixed_link;
- struct fixed_phy_status status = {};
-
- for_each_node_by_name(np, "ethernet") {
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link)
- continue;
-
- status.link = 1;
- status.duplex = fixed_link[1];
- status.speed = fixed_link[2];
- status.pause = fixed_link[3];
- status.asym_pause = fixed_link[4];
-
- ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
- if (ret) {
- of_node_put(np);
- return ret;
- }
- }
-
- return 0;
-}
-arch_initcall(of_add_fixed_phys);
-#endif /* CONFIG_FIXED_PHY */
-
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
static __be32 __iomem *rstcr;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 8/9] of: mdio: remove of_phy_connect_fixed_link
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
All in-tree drivers have been converted to use the new pair of
functions: of_is_fixed_phy_link() plus of_phy_register_fixed_link(), we
can now safely remove of_phy_connect_fixed_link.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/of/of_mdio.c | 38 --------------------------------------
include/linux/of_mdio.h | 10 ----------
2 files changed, 48 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1def0bb5cb37..4c1e01ed16dc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -246,44 +246,6 @@ struct phy_device *of_phy_connect(struct net_device *dev,
EXPORT_SYMBOL(of_phy_connect);
/**
- * of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
- * @dev: pointer to net_device claiming the phy
- * @hndlr: Link state callback for the network device
- * @iface: PHY data interface type
- *
- * This function is a temporary stop-gap and will be removed soon. It is
- * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
- * not call this function from new drivers.
- */
-struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- struct device_node *net_np;
- char bus_id[MII_BUS_ID_SIZE + 3];
- struct phy_device *phy;
- const __be32 *phy_id;
- int sz;
-
- if (!dev->dev.parent)
- return NULL;
-
- net_np = dev->dev.parent->of_node;
- if (!net_np)
- return NULL;
-
- phy_id = of_get_property(net_np, "fixed-link", &sz);
- if (!phy_id || sz < sizeof(*phy_id))
- return NULL;
-
- sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
-
- phy = phy_connect(dev, bus_id, hndlr, iface);
- return IS_ERR(phy) ? NULL : phy;
-}
-EXPORT_SYMBOL(of_phy_connect_fixed_link);
-
-/**
* of_phy_attach - Attach to a PHY without starting the state machine
* @dev: pointer to net_device claiming the phy
* @phy_np: Node pointer for the PHY
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 0aa367e316cb..d449018d0726 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -22,9 +22,6 @@ extern struct phy_device *of_phy_connect(struct net_device *dev,
struct phy_device *of_phy_attach(struct net_device *dev,
struct device_node *phy_np, u32 flags,
phy_interface_t iface);
-extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface);
extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
@@ -59,13 +56,6 @@ static inline struct phy_device *of_phy_attach(struct net_device *dev,
return NULL;
}
-static inline struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- return NULL;
-}
-
static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
{
return NULL;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 7/9] ucc_geth: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/freescale/ucc_geth.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index c8299c31b21f..fab39e295441 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1728,9 +1728,6 @@ static int init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
- if (!phydev)
- phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- priv->phy_interface);
if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
@@ -3790,6 +3787,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!ug_info->phy_node) {
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ return err;
+ }
+ ug_info->phy_node = np;
+ }
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 6/9] gianfar: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/freescale/gianfar.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e2d42475b006..282674027c92 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -889,6 +889,17 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ goto err_grp_init;
+
+ priv->phy_node = np;
+ }
+
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -1660,9 +1671,6 @@ static int init_phy(struct net_device *dev)
priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
interface);
- if (!priv->phydev)
- priv->phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- interface);
if (!priv->phydev) {
dev_err(&dev->dev, "could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 5/9] fs_enet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
- merge if conditions on the same line as suggested by Sergei
- add comment explaining the device_node pointer assignment
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index dc80db41d6b3..cfaf17b70f3f 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -792,10 +792,6 @@ static int fs_init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, fep->fpi->phy_node, &fs_adjust_link, 0,
iface);
if (!phydev) {
- phydev = of_phy_connect_fixed_link(dev, &fs_adjust_link,
- iface);
- }
- if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
}
@@ -1029,9 +1025,16 @@ static int fs_enet_probe(struct platform_device *ofdev)
fpi->use_napi = 1;
fpi->napi_weight = 17;
fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
- if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
- NULL)))
- goto out_free_fpi;
+ if (!fpi->phy_node && of_phy_is_fixed_link(ofdev->dev.of_node)) {
+ err = of_phy_register_fixed_link(ofdev->dev.of_node);
+ if (err)
+ goto out_free_fpi;
+
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ fpi->phy_node = ofdev->dev.of_node;
+ }
if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) {
phy_connection_type = of_get_property(ofdev->dev.of_node,
--
1.9.1
^ 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