LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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 &lt;

> paulus@

> &gt;
> ---
>  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

* [PATCH net-next v2 4/9] net: systemport: 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/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);
 	if (!priv->phydev) {
 		netdev_err(dev, "could not attach to PHY\n");
 		return -ENODEV;
@@ -1551,6 +1551,19 @@ static int bcm_sysport_probe(struct platform_device *pdev)
 	if (priv->phy_interface < 0)
 		priv->phy_interface = PHY_INTERFACE_MODE_GMII;
 
+	/* 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(dn)) {
+		ret = of_phy_register_fixed_link(dn);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to register fixed PHY\n");
+			goto err;
+		}
+
+		priv->phy_dn = dn;
+	}
+
 	/* Initialize netdevice members */
 	macaddr = of_get_mac_address(dn);
 	if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@ struct bcm_sysport_priv {
 	unsigned int		rx_c_index;
 
 	/* PHY device */
+	struct device_node	*phy_dn;
 	struct phy_device	*phydev;
 	phy_interface_t		phy_interface;
 	int			old_pause;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 3/9] net: bcmgenet: 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/broadcom/genet/bcmmii.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4608673beaff..add8d8596084 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -298,6 +298,7 @@ int bcmgenet_mii_config(struct net_device *dev)
 static int bcmgenet_mii_probe(struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
+	struct device_node *dn = priv->pdev->dev.of_node;
 	struct phy_device *phydev;
 	unsigned int phy_flags;
 	int ret;
@@ -307,15 +308,19 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 		return 0;
 	}
 
-	if (priv->phy_dn)
-		phydev = of_phy_connect(dev, priv->phy_dn,
-					bcmgenet_mii_setup, 0,
-					priv->phy_interface);
-	else
-		phydev = of_phy_connect_fixed_link(dev,
-					bcmgenet_mii_setup,
-					priv->phy_interface);
+	/* 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(dn)) {
+		ret = of_phy_register_fixed_link(dn);
+		if (ret)
+			return ret;
+
+		priv->phy_dn = dn;
+	}
 
+	phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup, 0,
+				priv->phy_interface);
 	if (!phydev) {
 		pr_err("could not attach to PHY\n");
 		return -ENODEV;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 2/9] Documentation: devicetree: net: refer to fixed-link.txt
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>

Update the Freescale TSEC PHY, Broadcom GENET & SYSTEMPORT Device Tree
binding documentation to refer to the fixed-link Device Tree binding in
fixed-link.txt.

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2

 Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt   | 2 +-
 Documentation/devicetree/bindings/net/broadcom-systemport.txt | 2 +-
 Documentation/devicetree/bindings/net/fsl-tsec-phy.txt        | 5 +----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index f2febb94550e..451fef26b4df 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -24,7 +24,7 @@ Optional properties:
 - fixed-link: When the GENET interface is connected to a MoCA hardware block or
   when operating in a RGMII to RGMII type of connection, or when the MDIO bus is
   voluntarily disabled, this property should be used to describe the "fixed link".
-  See Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for information on
+  See Documentation/devicetree/bindings/net/fixed-link.txt for information on
   the property specifics
 
 Required child nodes:
diff --git a/Documentation/devicetree/bindings/net/broadcom-systemport.txt b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
index 1b7600e022dd..c183ea90d9bc 100644
--- a/Documentation/devicetree/bindings/net/broadcom-systemport.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
@@ -8,7 +8,7 @@ Required properties:
 - local-mac-address: Ethernet MAC address (48 bits) of this adapter
 - phy-mode: Should be a string describing the PHY interface to the
   Ethernet switch/PHY, see Documentation/devicetree/bindings/net/ethernet.txt
-- fixed-link: see Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for
+- fixed-link: see Documentation/devicetree/bindings/net/fixed-link.txt for
   the property specific details
 
 Optional properties:
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 737cdef4f903..be6ea8960f20 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -42,10 +42,7 @@ Properties:
     interrupt.  For TSEC and eTSEC devices, the first interrupt is
     transmit, the second is receive, and the third is error.
   - phy-handle : See ethernet.txt file in the same directory.
-  - fixed-link : <a b c d e> where a is emulated phy id - choose any,
-    but unique to the all specified fixed-links, b is duplex - 0 half,
-    1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
-    pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
+  - fixed-link : See fixed-link.txt in the same directory.
   - phy-connection-type : See ethernet.txt file in the same directory.
     This property is only really needed if the connection is of type
     "rgmii-id", as all other connection types are detected by hardware.
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 1/9] Documentation: devicetree: add old and deprecated '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>

Update the fixed-link Device Tree binding documentation to contain
information about the old and deprecated 5-digit 'fixed-link' property.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- fixed typos and spelling mistakes as spotted by Sergei

 Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index e956de1be935..82bf7e0f47b6 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -18,6 +18,18 @@ properties:
 * 'asym-pause' (boolean, optional), to indicate that asym_pause should
   be enabled.
 
+Old, deprecated 'fixed-link' binding:
+
+* A 'fixed-link' property in the Ethernet MAC node, with 5 cells, of the
+  form <a b c d e> with the following accepted values:
+  - a: emulated PHY ID, choose any but but unique to the all specified
+    fixed-links, from 0 to 31
+  - b: duplex configuration: 0 for half duplex, 1 for full duplex
+  - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
+  - d: pause configuration: 0 for no pause, 1 for pause
+  - e: asymmetric pause configuration: 0 for no asymmetric pause, 1 for
+    asymmetric pause
+
 Example:
 
 ethernet@0 {
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 0/9] net: of_phy_connect_fixed_link removal
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

Hi all,

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.

Florian Fainelli (9):
  Documentation: devicetree: add old and deprecated 'fixed-link'
  Documentation: devicetree: net: refer to fixed-link.txt
  net: bcmgenet: use the new fixed PHY helpers
  net: systemport: use the new fixed PHY helpers
  fs_enet: use the new fixed PHY helpers
  gianfar: use the new fixed PHY helpers
  ucc_geth: use the new fixed PHY helpers
  of: mdio: remove of_phy_connect_fixed_link
  powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code

 .../devicetree/bindings/net/broadcom-bcmgenet.txt  |  2 +-
 .../bindings/net/broadcom-systemport.txt           |  2 +-
 .../devicetree/bindings/net/fixed-link.txt         | 12 +++++++
 .../devicetree/bindings/net/fsl-tsec-phy.txt       |  5 +--
 arch/powerpc/sysdev/fsl_soc.c                      | 32 ------------------
 drivers/net/ethernet/broadcom/bcmsysport.c         | 17 ++++++++--
 drivers/net/ethernet/broadcom/bcmsysport.h         |  1 +
 drivers/net/ethernet/broadcom/genet/bcmmii.c       | 21 +++++++-----
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 17 ++++++----
 drivers/net/ethernet/freescale/gianfar.c           | 14 ++++++--
 drivers/net/ethernet/freescale/ucc_geth.c          | 14 ++++++--
 drivers/of/of_mdio.c                               | 38 ----------------------
 include/linux/of_mdio.h                            | 10 ------
 13 files changed, 76 insertions(+), 109 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH RESEND 0/9 net-next] net: of_phy_connect_fixed_link removal
From: David Miller @ 2014-05-22 15:53 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: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>


Please address Sergei's feedback, except the indentation one which
as you stated is correct.

^ permalink raw reply


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