LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Erroneous double irq_eoi() on CPM IRQ in MPC8xx
From: Christophe Leroy @ 2013-04-18  5:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Vitaly Bordug,
	Marcelo Tosatti, Thomas Gleixner
  Cc: linuxppc-dev, linux-kernel

irq_eoi() is already called by generic_handle_irq() so 
it shall not be called a again

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

Index: linux/arch/powerpc/platforms/8xx/m8xx_setup.c
===================================================================
--- linux/arch/powerpc/platforms/8xx/m8xx_setup.c	(revision 4802)
+++ linux/arch/powerpc/platforms/8xx/m8xx_setup.c	(working copy)
@@ -218,19 +218,12 @@
 
 static void cpm_cascade(unsigned int irq, struct irq_desc *desc)
 {
-	struct irq_chip *chip;
-	int cascade_irq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int cascade_irq = cpm_get_irq();
 
-	if ((cascade_irq = cpm_get_irq()) >= 0) {
-		struct irq_desc *cdesc = irq_to_desc(cascade_irq);
-
+	if (cascade_irq >= 0)
 		generic_handle_irq(cascade_irq);
 
-		chip = irq_desc_get_chip(cdesc);
-		chip->irq_eoi(&cdesc->irq_data);
-	}
-
-	chip = irq_desc_get_chip(desc);
 	chip->irq_eoi(&desc->irq_data);
 }
 

^ permalink raw reply

* [Suggestion] PowerPC: kernel: memory access violation when rtas_data_buf contents are more than 1026
From: Chen Gang @ 2013-04-18  4:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, paulus@samba.org, Al Viro
  Cc: sfr@canb.auug.org.au, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org

Hello Maintainers:


in arch/powerpc/kernel/lparcfg.c, parse_system_parameter_string()

  need set '\0' for 'local_buffer'.

  the reason is:
    SPLPAR_MAXLENGTH is 1026, RTAS_DATA_BUF_SIZE is 4096
    the contents of rtas_data_buf may truncated in memcpy (line 301).

    if contents are truncated.
      the splpar_strlen is more than 1026 (line 321)
      the while loop checking will not find the end of buffer (line 326)
      it will cause memory access violation.


  I find it by reading code, so please help check.

  thanks.

gchen.

-------------------------related fix patch--------------------------------------

diff --git a/arch/powerpc/kernel/lparcfg.c b/arch/powerpc/kernel/lparcfg.c
index 801a757..d92f387 100644
--- a/arch/powerpc/kernel/lparcfg.c
+++ b/arch/powerpc/kernel/lparcfg.c
@@ -299,6 +299,7 @@ static void parse_system_parameter_string(struct seq_file *m)
 				__pa(rtas_data_buf),
 				RTAS_DATA_BUF_SIZE);
 	memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
+	local_buffer[SPLPAR_MAXLENGTH - 1] = '\0';
 	spin_unlock(&rtas_data_buf_lock);
 
 	if (call_status != 0) {



-------------------------related source code------------------------------------


283 static void parse_system_parameter_string(struct seq_file *m)
284 {
285         int call_status;
286 
287         unsigned char *local_buffer = kmalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
288         if (!local_buffer) {
289                 printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
290                        __FILE__, __func__, __LINE__);
291                 return;
292         }
293 
294         spin_lock(&rtas_data_buf_lock);
295         memset(rtas_data_buf, 0, SPLPAR_MAXLENGTH);
296         call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
297                                 NULL,
298                                 SPLPAR_CHARACTERISTICS_TOKEN,
299                                 __pa(rtas_data_buf),
300                                 RTAS_DATA_BUF_SIZE);
301         memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
302         spin_unlock(&rtas_data_buf_lock);
303         
304         if (call_status != 0) {
305                 printk(KERN_INFO
306                        "%s %s Error calling get-system-parameter (0x%x)\n",
307                        __FILE__, __func__, call_status);
308         } else {       
309                 int splpar_strlen;
310                 int idx, w_idx;
311                 char *workbuffer = kzalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
312                 if (!workbuffer) { 
313                         printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
314                                __FILE__, __func__, __LINE__);
315                         kfree(local_buffer);
316                         return;
317                 }       
318 #ifdef LPARCFG_DEBUG
319                 printk(KERN_INFO "success calling get-system-parameter\n");
320 #endif
321                 splpar_strlen = local_buffer[0] * 256 + local_buffer[1];
322                 local_buffer += 2;      /* step over strlen value */
323 
324                 w_idx = 0;
325                 idx = 0;
326                 while ((*local_buffer) && (idx < splpar_strlen)) {
327                         workbuffer[w_idx++] = local_buffer[idx++];
328                         if ((local_buffer[idx] == ',')
329                             || (local_buffer[idx] == '\0')) {
330                                 workbuffer[w_idx] = '\0';
331                                 if (w_idx) {
332                                         /* avoid the empty string */
333                                         seq_printf(m, "%s\n", workbuffer);
334                                 }
335                                 memset(workbuffer, 0, SPLPAR_MAXLENGTH);
336                                 idx++;  /* skip the comma */
337                                 w_idx = 0;
338                         } else if (local_buffer[idx] == '=') {
339                                 /* code here to replace workbuffer contents
340                                    with different keyword strings */
341                                 if (0 == strcmp(workbuffer, "MaxEntCap")) {
342                                         strcpy(workbuffer,
343                                                "partition_max_entitled_capacity");
344                                         w_idx = strlen(workbuffer);
345                                 }
346                                 if (0 == strcmp(workbuffer, "MaxPlatProcs")) {
347                                         strcpy(workbuffer,
348                                                "system_potential_processors");
349                                         w_idx = strlen(workbuffer);
350                                 }
351                         }
352                 }
353                 kfree(workbuffer);
354                 local_buffer -= 2;      /* back up over strlen value */
355         }
356         kfree(local_buffer);
357 }

^ permalink raw reply related

* Re: [PATCH] powerpc: Add HWCAP2 aux entry
From: Michael Neuling @ 2013-04-18  3:41 UTC (permalink / raw)
  To: akpm
  Cc: vda.linux, Nishanth Aravamudan, Steve Munroe, linux-kernel,
	paulus, viro, Ryan Arnold, linuxppc-dev
In-Reply-To: <31546.1366255991@ale.ozlabs.ibm.com>

akpm,

If you're happy with this, is it something you can take in your tree?

Mikey

Michael Neuling <mikey@neuling.org> wrote:
> We are currently out of free bits in AT_HWCAP. With POWER8, we have
> several hardware features that we need to advertise. 
> 
> Tested on POWER and x86.
> 
> Signed-off-by: Michael Neuling <michael@neuling.org>
> Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
> ---
> 
> > Wouldn't it be safer to not emit AT_HWCAP2 unless it is defined by the arch?
> > 
> > That way the change would only impact powerpc.
> 
> Should be addressed with this version.
> 
> Mikey
> 
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index fb3245e..ccadad6 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -52,6 +52,7 @@ struct cpu_spec {
>  	char		*cpu_name;
>  	unsigned long	cpu_features;		/* Kernel features */
>  	unsigned int	cpu_user_features;	/* Userland features */
> +	unsigned int	cpu_user_features2;	/* Userland features v2 */
>  	unsigned int	mmu_features;		/* MMU features */
>  
>  	/* cache line sizes */
> diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
> index ac9790f..cc0655a 100644
> --- a/arch/powerpc/include/asm/elf.h
> +++ b/arch/powerpc/include/asm/elf.h
> @@ -61,6 +61,7 @@ typedef elf_vrregset_t elf_fpxregset_t;
>     instruction set this cpu supports.  This could be done in userspace,
>     but it's not easy, and we've already done it here.  */
>  # define ELF_HWCAP	(cur_cpu_spec->cpu_user_features)
> +# define ELF_HWCAP2	(cur_cpu_spec->cpu_user_features2)
>  
>  /* This yields a string that ld.so will use to load implementation
>     specific libraries for optimization.  This is more specific in
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 3939829..1f8b5d5 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -240,6 +240,9 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
>  	NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
>   	NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
>  	NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
> +#ifdef ELF_HWCAP2
> +	NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
> +#endif
>  	NEW_AUX_ENT(AT_EXECFN, bprm->exec);
>  	if (k_platform) {
>  		NEW_AUX_ENT(AT_PLATFORM,
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index 9c13e02..bf2381d 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -483,7 +483,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
>  	size_t platform_len = 0, len;
>  	char *k_platform, *k_base_platform;
>  	char __user *u_platform, *u_base_platform, *p;
> -	long hwcap;
>  	int loop;
>  	int nr;	/* reset for each csp adjustment */
>  
> @@ -502,8 +501,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
>  		return -EFAULT;
>  #endif
>  
> -	hwcap = ELF_HWCAP;
> -
>  	/*
>  	 * If this architecture has a platform capability string, copy it
>  	 * to userspace.  In some cases (Sparc), this info is impossible
> @@ -617,7 +614,10 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
>  
>  	nr = 0;
>  	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
> -	NEW_AUX_ENT(AT_HWCAP,	hwcap);
> +	NEW_AUX_ENT(AT_HWCAP,	ELF_HWCAP);
> +#ifdef ELF_HWCAP2
> +	NEW_AUX_ENT(AT_HWCAP2,	ELF_HWCAP2);
> +#endif
>  	NEW_AUX_ENT(AT_PAGESZ,	PAGE_SIZE);
>  	NEW_AUX_ENT(AT_CLKTCK,	CLOCKS_PER_SEC);
>  	NEW_AUX_ENT(AT_PHDR,	exec_params->ph_addr);
> diff --git a/include/uapi/linux/auxvec.h b/include/uapi/linux/auxvec.h
> index 61594d5..835c065 100644
> --- a/include/uapi/linux/auxvec.h
> +++ b/include/uapi/linux/auxvec.h
> @@ -28,6 +28,7 @@
>  #define AT_BASE_PLATFORM 24	/* string identifying real platform, may
>  				 * differ from AT_PLATFORM. */
>  #define AT_RANDOM 25	/* address of 16 random bytes */
> +#define AT_HWCAP2 26	/* extension of AT_HWCAP */
>  
>  #define AT_EXECFN  31	/* filename of program */
>  
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* [PATCH] powerpc: Add HWCAP2 aux entry
From: Michael Neuling @ 2013-04-18  3:33 UTC (permalink / raw)
  To: Michael Ellerman, Nishanth Aravamudan
  Cc: vda.linux, Steve Munroe, linux-kernel, paulus, viro, Ryan Arnold,
	linuxppc-dev, akpm
In-Reply-To: <27144.1366001074@ale.ozlabs.ibm.com>

We are currently out of free bits in AT_HWCAP. With POWER8, we have
several hardware features that we need to advertise. 

Tested on POWER and x86.

Signed-off-by: Michael Neuling <michael@neuling.org>
Signed-off-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
---

> Wouldn't it be safer to not emit AT_HWCAP2 unless it is defined by the arch?
> 
> That way the change would only impact powerpc.

Should be addressed with this version.

Mikey

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index fb3245e..ccadad6 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -52,6 +52,7 @@ struct cpu_spec {
 	char		*cpu_name;
 	unsigned long	cpu_features;		/* Kernel features */
 	unsigned int	cpu_user_features;	/* Userland features */
+	unsigned int	cpu_user_features2;	/* Userland features v2 */
 	unsigned int	mmu_features;		/* MMU features */
 
 	/* cache line sizes */
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index ac9790f..cc0655a 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -61,6 +61,7 @@ typedef elf_vrregset_t elf_fpxregset_t;
    instruction set this cpu supports.  This could be done in userspace,
    but it's not easy, and we've already done it here.  */
 # define ELF_HWCAP	(cur_cpu_spec->cpu_user_features)
+# define ELF_HWCAP2	(cur_cpu_spec->cpu_user_features2)
 
 /* This yields a string that ld.so will use to load implementation
    specific libraries for optimization.  This is more specific in
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 3939829..1f8b5d5 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -240,6 +240,9 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
 	NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
  	NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
 	NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
+#ifdef ELF_HWCAP2
+	NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
+#endif
 	NEW_AUX_ENT(AT_EXECFN, bprm->exec);
 	if (k_platform) {
 		NEW_AUX_ENT(AT_PLATFORM,
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 9c13e02..bf2381d 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -483,7 +483,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 	size_t platform_len = 0, len;
 	char *k_platform, *k_base_platform;
 	char __user *u_platform, *u_base_platform, *p;
-	long hwcap;
 	int loop;
 	int nr;	/* reset for each csp adjustment */
 
@@ -502,8 +501,6 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 		return -EFAULT;
 #endif
 
-	hwcap = ELF_HWCAP;
-
 	/*
 	 * If this architecture has a platform capability string, copy it
 	 * to userspace.  In some cases (Sparc), this info is impossible
@@ -617,7 +614,10 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
 
 	nr = 0;
 	csp -= DLINFO_ITEMS * 2 * sizeof(unsigned long);
-	NEW_AUX_ENT(AT_HWCAP,	hwcap);
+	NEW_AUX_ENT(AT_HWCAP,	ELF_HWCAP);
+#ifdef ELF_HWCAP2
+	NEW_AUX_ENT(AT_HWCAP2,	ELF_HWCAP2);
+#endif
 	NEW_AUX_ENT(AT_PAGESZ,	PAGE_SIZE);
 	NEW_AUX_ENT(AT_CLKTCK,	CLOCKS_PER_SEC);
 	NEW_AUX_ENT(AT_PHDR,	exec_params->ph_addr);
diff --git a/include/uapi/linux/auxvec.h b/include/uapi/linux/auxvec.h
index 61594d5..835c065 100644
--- a/include/uapi/linux/auxvec.h
+++ b/include/uapi/linux/auxvec.h
@@ -28,6 +28,7 @@
 #define AT_BASE_PLATFORM 24	/* string identifying real platform, may
 				 * differ from AT_PLATFORM. */
 #define AT_RANDOM 25	/* address of 16 random bytes */
+#define AT_HWCAP2 26	/* extension of AT_HWCAP */
 
 #define AT_EXECFN  31	/* filename of program */
 

^ permalink raw reply related

* AUTO: Michael Barry is out of the office (returning 24/04/2013)
From: Michael Barry @ 2013-04-18  3:01 UTC (permalink / raw)
  To: linuxppc-dev


I am out of the office until 24/04/2013.




Note: This is an automated response to your message  "Linuxppc-dev Digest,
Vol 104, Issue 108" sent on 17/04/2013 21:17:28.

This is the only notification you will receive while this person is away.

^ permalink raw reply

* Re: [PATCH V2 5/5] powerpc, perf: Enable branch stack sampling framework support with BHRB
From: Anshuman Khandual @ 2013-04-18  2:58 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, mikey, linux-kernel
In-Reply-To: <516E9096.70906@linux.vnet.ibm.com>

On 04/17/2013 05:37 PM, Anshuman Khandual wrote:
> On 04/17/2013 12:38 PM, Michael Ellerman wrote:
>> On Tue, Apr 16, 2013 at 09:24:10PM +0530, Anshuman Khandual wrote:
>>> This patch provides basic enablement for perf branch stack sampling framework
>>> on POWER8 processor with a new PMU feature called BHRB (Branch History Rolling
>>> Buffer).
>>>
>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>> ---
>>>  arch/powerpc/perf/core-book3s.c     | 96 +++++++++++++++++++++++++++++++++++--
>>>  arch/powerpc/perf/perf_event_bhrb.c | 85 ++++++++++++++++++++++++++++++++
>>>  2 files changed, 178 insertions(+), 3 deletions(-)
>>>  create mode 100644 arch/powerpc/perf/perf_event_bhrb.c
>>>
>>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>>> index 4ac6e64..f4d1347 100644
>>> --- a/arch/powerpc/perf/core-book3s.c
>>> +++ b/arch/powerpc/perf/core-book3s.c
>>> @@ -19,6 +19,8 @@
>>>  #include <asm/firmware.h>
>>>  #include <asm/ptrace.h>
>>>  
>>> +#define BHRB_MAX_ENTRIES	32
>>> +
>>>  struct cpu_hw_events {
>>>  	int n_events;
>>>  	int n_percpu;
>>> @@ -38,11 +40,21 @@ struct cpu_hw_events {
>>>  
>>>  	unsigned int group_flag;
>>>  	int n_txn_start;
>>> +
>>> +	/* BHRB bits */
>>> +	u64				bhrb_filter;	/* BHRB HW branch filter */
>>> +	int				bhrb_users;
>>> +	void				*bhrb_context;
>>> +	struct	perf_branch_stack	bhrb_stack;
>>> +	struct	perf_branch_entry	bhrb_entries[BHRB_MAX_ENTRIES];
>>>  };
>>> +
>>>  DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);
>>>  
>>>  struct power_pmu *ppmu;
>>>  
>>> +#include "perf_event_bhrb.c"
>>> +
>>
>> Um, why are you doing that?
>>
> 
> There was no specific reason for that.
> 

Ahh, I remember it now. The function in the new file uses "cpu_hw_events" structure
which is passed during "record_and_restart" data capture phase. Right now cpu_hw_events is
not defined in the header file but inside core-book3s.c itself. Solution to this problem
could be any of these.

(0) Move all the code from the new file perf_event_bhrb.c into core-book3s.c
(1) Move cpu_hw_events structure to perf_event_server.h
(2) Create additional BHRB processing function inside struct power_pmu and
    define it for P8 inside power8_pmu.c

Regards
Anshuman

^ permalink raw reply

* Re: [RFC PATCH powerpc] make CONFIG_NUMA depends on CONFIG_SMP
From: Michael Ellerman @ 2013-04-18  1:46 UTC (permalink / raw)
  To: Li Zhong; +Cc: PowerPC email list, Paul Mackerras
In-Reply-To: <1338370318.3637.12.camel@ThinkPad-T420>

On Wed, May 30, 2012 at 05:31:58PM +0800, Li Zhong wrote:
> I'm not sure whether it makes sense to add this dependency to avoid
> CONFI_NUMA && !CONFIG_SMP. 
> 
> I want to do this because I saw some build errors on next-tree when
> compiling with CONFIG_SMP disabled, and it seems they are caused by some
> codes under the CONFIG_NUMA #ifdefs.  

This seems to make sense to me. Can you please repost with a better
changelog and a description of the actual build error you were seeing.

cheers

^ permalink raw reply

* [PATCH] Erroneous double irq_eoi() on CPM IRQ in MPC8xx
From: Christophe Leroy @ 2013-04-17 22:04 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Vitaly Bordug,
	Marcelo Tosatti, Thomas Gleixner
  Cc: linuxppc-dev, linux-kernel

irq_eoi() is already called by generic_handle_irq() so 
it shall not be called a again

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

Index: linux/arch/powerpc/platforms/8xx/m8xx_setup.c
===================================================================
--- linux/arch/powerpc/platforms/8xx/m8xx_setup.c	(revision 4802)
+++ linux/arch/powerpc/platforms/8xx/m8xx_setup.c	(working copy)
@@ -218,19 +218,12 @@
 
 static void cpm_cascade(unsigned int irq, struct irq_desc *desc)
 {
-	struct irq_chip *chip;
-	int cascade_irq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int cascade_irq = cpm_get_irq());
 
-	if ((cascade_irq = cpm_get_irq()) >= 0) {
-		struct irq_desc *cdesc = irq_to_desc(cascade_irq);
-
+	if (cascade_irq >= 0)
 		generic_handle_irq(cascade_irq);
 
-		chip = irq_desc_get_chip(cdesc);
-		chip->irq_eoi(&cdesc->irq_data);
-	}
-
-	chip = irq_desc_get_chip(desc);
 	chip->irq_eoi(&desc->irq_data);
 }
 

^ permalink raw reply

* BUG: branch trace support for 64-bit Book-E (was Re: questions around Book III-E and branch trace)
From: Chris Friesen @ 2013-04-17 23:30 UTC (permalink / raw)
  To: Kumar Gala, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras
In-Reply-To: <516EED8D.3040304@genband.com>

On 04/17/2013 12:44 PM, Chris Friesen wrote:
> Hi,
>
> I'm trying to wrap my head around how linux handles branch tracing on
> Book III-E. I think I understand how we set MSR[DE] and DBCR0[IDM|BT],
> and how we handle fixing things up if an instruction being traced causes
> an exception.

While poking around looking for answers to my previous questions I seem 
to have stumbled over a bug in branch tracing for 64-bit Book-E.

Commit ec097c8 added support for branch tracing for 32-bit code, but 
didn't do the 64-bit path.  As it stands, debug_crit/debug_debug in 
exceptions-64e.S only check DBSR_IC, so branch tracing will not get 
fixed up and will die a horrible death (typically in a tbr exception 
handler).

I was banging my head trying to figure out why this wasn't working when 
I finally clued in to the fact that head_booke.h only applied to the 
32-bit implementation.  This might be something useful to put in a 
comment up at the top of the file.

Chris

^ permalink raw reply

* Re: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()
From: Grant Likely @ 2013-04-17 22:00 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Tang Yuantian-B29983, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	rob.herring@calxeda.com
In-Reply-To: <CAOZdJXVnppPczrJi8jcNWLvabHFWP++Ex1QiEadbyLfApCR85g@mail.gmail.com>

On Wed, Apr 17, 2013 at 10:52 PM, Timur Tabi <timur@tabi.org> wrote:
> On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>>
>>> I really preciate if you can spend some times to review this patch.
>>
>> Applied, thanks.
>
> Pff.  Why do I bother?

Relax Timur:

http://git.secretlab.ca/?p=linux.git;a=commitdiff;h=b855f16b05a697ac1863adabe99bfba56e6d3199

g.



--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH v2] of/base: release the node correctly in of_parse_phandle_with_args()
From: Timur Tabi @ 2013-04-17 21:52 UTC (permalink / raw)
  To: Grant Likely
  Cc: Tang Yuantian-B29983, devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	rob.herring@calxeda.com
In-Reply-To: <20130417145719.F194E3E2B73@localhost>

On Wed, Apr 17, 2013 at 9:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> I really preciate if you can spend some times to review this patch.
>
> Applied, thanks.

Pff.  Why do I bother?

^ permalink raw reply

* Re: [PATCH 2/3] serial/mpc52xx_uart: add PSC UART support for MPC5125 platforms.
From: Anatolij Gustschin @ 2013-04-17 21:24 UTC (permalink / raw)
  To: Matteo Facchinetti; +Cc: gregkh, linuxppc-dev
In-Reply-To: <1363801314-16967-3-git-send-email-matteo.facchinetti@sirius-es.it>

On Wed, 20 Mar 2013 18:41:53 +0100
Matteo Facchinetti <matteo.facchinetti@sirius-es.it> wrote:

> MPC5125 PSC controller has different registers than MPC5121.
> 
> This patch was originally created by Vladimir Ermakov
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2011-March/088954.html
> 
> Signed-off-by: Vladimir Ermakov <vooon341@gmail.com>
> 
> Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
> ---
>  arch/powerpc/include/asm/mpc52xx_psc.h |   49 ++++
>  drivers/tty/serial/mpc52xx_uart.c      |  407 ++++++++++++++++++++++++++++----
>  2 files changed, 414 insertions(+), 42 deletions(-)

This patch should be better split to add SoC specific register accessors
first, then we can add mpc5125 changes by separate patch. I've just done
this splitting and added more descriptive commit log.

Thanks,

Anatolij

^ permalink raw reply

* [PATCH v2 1/2] serial/mpc52xx_uart: prepare for adding MPC5125 PSC UART support
From: Anatolij Gustschin @ 2013-04-17 21:21 UTC (permalink / raw)
  To: linux-serial, Greg Kroah-Hartman
  Cc: Matteo Facchinetti, Anatolij Gustschin, linuxppc-dev,
	Vladimir Ermakov
In-Reply-To: <1363801314-16967-3-git-send-email-matteo.facchinetti@sirius-es.it>

From: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>

MPC5125 PSC controller has different register layout than MPC5121.
To support MPC5125 PSC in this driver we have to provide further
psc_ops functions for SoC specific register accesses.

Add new register access functions to the psc_ops structure and
provide MPC52xx and MPC512x specific implementation for them.
Then replace remaining direct register accesses in the driver by
appropriate psc_ops function calls. The subsequent patch can now
add MPC5125 specific set of psc_ops functions.

Signed-off-by: Vladimir Ermakov <vooon341@gmail.com>
Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---

Greg, with your Acked-by I can push these patches to my mpc5xxx tree.
But it is fine with me if you prefer to apply them to tty tree.

Thanks,

Anatolij

Changes in v2:
 - split into two patches to simplify review
 - minor coding style changes
 - revise commit log

 drivers/tty/serial/mpc52xx_uart.c |  161 +++++++++++++++++++++++++++----------
 1 file changed, 119 insertions(+), 42 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 018bad9..5aa87ac 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -122,6 +122,15 @@ struct psc_ops {
 	void		(*fifoc_uninit)(void);
 	void		(*get_irq)(struct uart_port *, struct device_node *);
 	irqreturn_t	(*handle_irq)(struct uart_port *port);
+	u16		(*get_status)(struct uart_port *port);
+	u8		(*get_ipcr)(struct uart_port *port);
+	void		(*command)(struct uart_port *port, u8 cmd);
+	void		(*set_mode)(struct uart_port *port, u8 mr1, u8 mr2);
+	void		(*set_rts)(struct uart_port *port, int state);
+	void		(*enable_ms)(struct uart_port *port);
+	void		(*set_sicr)(struct uart_port *port, u32 val);
+	void		(*set_imr)(struct uart_port *port, u16 val);
+	u8		(*get_mr1)(struct uart_port *port);
 };
 
 /* setting the prescaler and divisor reg is common for all chips */
@@ -134,6 +143,65 @@ static inline void mpc52xx_set_divisor(struct mpc52xx_psc __iomem *psc,
 	out_8(&psc->ctlr, divisor & 0xff);
 }
 
+static u16 mpc52xx_psc_get_status(struct uart_port *port)
+{
+	return in_be16(&PSC(port)->mpc52xx_psc_status);
+}
+
+static u8 mpc52xx_psc_get_ipcr(struct uart_port *port)
+{
+	return in_8(&PSC(port)->mpc52xx_psc_ipcr);
+}
+
+static void mpc52xx_psc_command(struct uart_port *port, u8 cmd)
+{
+	out_8(&PSC(port)->command, cmd);
+}
+
+static void mpc52xx_psc_set_mode(struct uart_port *port, u8 mr1, u8 mr2)
+{
+	out_8(&PSC(port)->command, MPC52xx_PSC_SEL_MODE_REG_1);
+	out_8(&PSC(port)->mode, mr1);
+	out_8(&PSC(port)->mode, mr2);
+}
+
+static void mpc52xx_psc_set_rts(struct uart_port *port, int state)
+{
+	if (state)
+		out_8(&PSC(port)->op1, MPC52xx_PSC_OP_RTS);
+	else
+		out_8(&PSC(port)->op0, MPC52xx_PSC_OP_RTS);
+}
+
+static void mpc52xx_psc_enable_ms(struct uart_port *port)
+{
+	struct mpc52xx_psc __iomem *psc = PSC(port);
+
+	/* clear D_*-bits by reading them */
+	in_8(&psc->mpc52xx_psc_ipcr);
+	/* enable CTS and DCD as IPC interrupts */
+	out_8(&psc->mpc52xx_psc_acr, MPC52xx_PSC_IEC_CTS | MPC52xx_PSC_IEC_DCD);
+
+	port->read_status_mask |= MPC52xx_PSC_IMR_IPC;
+	out_be16(&psc->mpc52xx_psc_imr, port->read_status_mask);
+}
+
+static void mpc52xx_psc_set_sicr(struct uart_port *port, u32 val)
+{
+	out_be32(&PSC(port)->sicr, val);
+}
+
+static void mpc52xx_psc_set_imr(struct uart_port *port, u16 val)
+{
+	out_be16(&PSC(port)->mpc52xx_psc_imr, val);
+}
+
+static u8 mpc52xx_psc_get_mr1(struct uart_port *port)
+{
+	out_8(&PSC(port)->command, MPC52xx_PSC_SEL_MODE_REG_1);
+	return in_8(&PSC(port)->mode);
+}
+
 #ifdef CONFIG_PPC_MPC52xx
 #define FIFO_52xx(port) ((struct mpc52xx_psc_fifo __iomem *)(PSC(port)+1))
 static void mpc52xx_psc_fifo_init(struct uart_port *port)
@@ -304,6 +372,15 @@ static struct psc_ops mpc52xx_psc_ops = {
 	.set_baudrate = mpc5200_psc_set_baudrate,
 	.get_irq = mpc52xx_psc_get_irq,
 	.handle_irq = mpc52xx_psc_handle_irq,
+	.get_status = mpc52xx_psc_get_status,
+	.get_ipcr = mpc52xx_psc_get_ipcr,
+	.command = mpc52xx_psc_command,
+	.set_mode = mpc52xx_psc_set_mode,
+	.set_rts = mpc52xx_psc_set_rts,
+	.enable_ms = mpc52xx_psc_enable_ms,
+	.set_sicr = mpc52xx_psc_set_sicr,
+	.set_imr = mpc52xx_psc_set_imr,
+	.get_mr1 = mpc52xx_psc_get_mr1,
 };
 
 static struct psc_ops mpc5200b_psc_ops = {
@@ -325,6 +402,15 @@ static struct psc_ops mpc5200b_psc_ops = {
 	.set_baudrate = mpc5200b_psc_set_baudrate,
 	.get_irq = mpc52xx_psc_get_irq,
 	.handle_irq = mpc52xx_psc_handle_irq,
+	.get_status = mpc52xx_psc_get_status,
+	.get_ipcr = mpc52xx_psc_get_ipcr,
+	.command = mpc52xx_psc_command,
+	.set_mode = mpc52xx_psc_set_mode,
+	.set_rts = mpc52xx_psc_set_rts,
+	.enable_ms = mpc52xx_psc_enable_ms,
+	.set_sicr = mpc52xx_psc_set_sicr,
+	.set_imr = mpc52xx_psc_set_imr,
+	.get_mr1 = mpc52xx_psc_get_mr1,
 };
 
 #endif /* CONFIG_MPC52xx */
@@ -595,8 +681,18 @@ static struct psc_ops mpc512x_psc_ops = {
 	.fifoc_uninit = mpc512x_psc_fifoc_uninit,
 	.get_irq = mpc512x_psc_get_irq,
 	.handle_irq = mpc512x_psc_handle_irq,
+	.get_status = mpc52xx_psc_get_status,
+	.get_ipcr = mpc52xx_psc_get_ipcr,
+	.command = mpc52xx_psc_command,
+	.set_mode = mpc52xx_psc_set_mode,
+	.set_rts = mpc52xx_psc_set_rts,
+	.enable_ms = mpc52xx_psc_enable_ms,
+	.set_sicr = mpc52xx_psc_set_sicr,
+	.set_imr = mpc52xx_psc_set_imr,
+	.get_mr1 = mpc52xx_psc_get_mr1,
 };
-#endif
+#endif /* CONFIG_PPC_MPC512x */
+
 
 static const struct psc_ops *psc_ops;
 
@@ -613,17 +709,14 @@ mpc52xx_uart_tx_empty(struct uart_port *port)
 static void
 mpc52xx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
-	if (mctrl & TIOCM_RTS)
-		out_8(&PSC(port)->op1, MPC52xx_PSC_OP_RTS);
-	else
-		out_8(&PSC(port)->op0, MPC52xx_PSC_OP_RTS);
+	psc_ops->set_rts(port, mctrl & TIOCM_RTS);
 }
 
 static unsigned int
 mpc52xx_uart_get_mctrl(struct uart_port *port)
 {
 	unsigned int ret = TIOCM_DSR;
-	u8 status = in_8(&PSC(port)->mpc52xx_psc_ipcr);
+	u8 status = psc_ops->get_ipcr(port);
 
 	if (!(status & MPC52xx_PSC_CTS))
 		ret |= TIOCM_CTS;
@@ -673,15 +766,7 @@ mpc52xx_uart_stop_rx(struct uart_port *port)
 static void
 mpc52xx_uart_enable_ms(struct uart_port *port)
 {
-	struct mpc52xx_psc __iomem *psc = PSC(port);
-
-	/* clear D_*-bits by reading them */
-	in_8(&psc->mpc52xx_psc_ipcr);
-	/* enable CTS and DCD as IPC interrupts */
-	out_8(&psc->mpc52xx_psc_acr, MPC52xx_PSC_IEC_CTS | MPC52xx_PSC_IEC_DCD);
-
-	port->read_status_mask |= MPC52xx_PSC_IMR_IPC;
-	out_be16(&psc->mpc52xx_psc_imr, port->read_status_mask);
+	psc_ops->enable_ms(port);
 }
 
 static void
@@ -691,9 +776,9 @@ mpc52xx_uart_break_ctl(struct uart_port *port, int ctl)
 	spin_lock_irqsave(&port->lock, flags);
 
 	if (ctl == -1)
-		out_8(&PSC(port)->command, MPC52xx_PSC_START_BRK);
+		psc_ops->command(port, MPC52xx_PSC_START_BRK);
 	else
-		out_8(&PSC(port)->command, MPC52xx_PSC_STOP_BRK);
+		psc_ops->command(port, MPC52xx_PSC_STOP_BRK);
 
 	spin_unlock_irqrestore(&port->lock, flags);
 }
@@ -701,7 +786,6 @@ mpc52xx_uart_break_ctl(struct uart_port *port, int ctl)
 static int
 mpc52xx_uart_startup(struct uart_port *port)
 {
-	struct mpc52xx_psc __iomem *psc = PSC(port);
 	int ret;
 
 	if (psc_ops->clock) {
@@ -717,15 +801,15 @@ mpc52xx_uart_startup(struct uart_port *port)
 		return ret;
 
 	/* Reset/activate the port, clear and enable interrupts */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
-	out_8(&psc->command, MPC52xx_PSC_RST_TX);
+	psc_ops->command(port, MPC52xx_PSC_RST_RX);
+	psc_ops->command(port, MPC52xx_PSC_RST_TX);
 
-	out_be32(&psc->sicr, 0);	/* UART mode DCD ignored */
+	psc_ops->set_sicr(port, 0);	/* UART mode DCD ignored */
 
 	psc_ops->fifo_init(port);
 
-	out_8(&psc->command, MPC52xx_PSC_TX_ENABLE);
-	out_8(&psc->command, MPC52xx_PSC_RX_ENABLE);
+	psc_ops->command(port, MPC52xx_PSC_TX_ENABLE);
+	psc_ops->command(port, MPC52xx_PSC_RX_ENABLE);
 
 	return 0;
 }
@@ -733,15 +817,13 @@ mpc52xx_uart_startup(struct uart_port *port)
 static void
 mpc52xx_uart_shutdown(struct uart_port *port)
 {
-	struct mpc52xx_psc __iomem *psc = PSC(port);
-
 	/* Shut down the port.  Leave TX active if on a console port */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
+	psc_ops->command(port, MPC52xx_PSC_RST_RX);
 	if (!uart_console(port))
-		out_8(&psc->command, MPC52xx_PSC_RST_TX);
+		psc_ops->command(port, MPC52xx_PSC_RST_TX);
 
 	port->read_status_mask = 0;
-	out_be16(&psc->mpc52xx_psc_imr, port->read_status_mask);
+	psc_ops->set_imr(port, port->read_status_mask);
 
 	if (psc_ops->clock)
 		psc_ops->clock(port, 0);
@@ -754,7 +836,6 @@ static void
 mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
 			 struct ktermios *old)
 {
-	struct mpc52xx_psc __iomem *psc = PSC(port);
 	unsigned long flags;
 	unsigned char mr1, mr2;
 	unsigned int j;
@@ -818,13 +899,11 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
 			"Some chars may have been lost.\n");
 
 	/* Reset the TX & RX */
-	out_8(&psc->command, MPC52xx_PSC_RST_RX);
-	out_8(&psc->command, MPC52xx_PSC_RST_TX);
+	psc_ops->command(port, MPC52xx_PSC_RST_RX);
+	psc_ops->command(port, MPC52xx_PSC_RST_TX);
 
 	/* Send new mode settings */
-	out_8(&psc->command, MPC52xx_PSC_SEL_MODE_REG_1);
-	out_8(&psc->mode, mr1);
-	out_8(&psc->mode, mr2);
+	psc_ops->set_mode(port, mr1, mr2);
 	baud = psc_ops->set_baudrate(port, new, old);
 
 	/* Update the per-port timeout */
@@ -834,8 +913,8 @@ mpc52xx_uart_set_termios(struct uart_port *port, struct ktermios *new,
 		mpc52xx_uart_enable_ms(port);
 
 	/* Reenable TX & RX */
-	out_8(&psc->command, MPC52xx_PSC_TX_ENABLE);
-	out_8(&psc->command, MPC52xx_PSC_RX_ENABLE);
+	psc_ops->command(port, MPC52xx_PSC_TX_ENABLE);
+	psc_ops->command(port, MPC52xx_PSC_RX_ENABLE);
 
 	/* We're all set, release the lock */
 	spin_unlock_irqrestore(&port->lock, flags);
@@ -963,7 +1042,7 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port)
 		flag = TTY_NORMAL;
 		port->icount.rx++;
 
-		status = in_be16(&PSC(port)->mpc52xx_psc_status);
+		status = psc_ops->get_status(port);
 
 		if (status & (MPC52xx_PSC_SR_PE |
 			      MPC52xx_PSC_SR_FE |
@@ -983,7 +1062,7 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port)
 			}
 
 			/* Clear error condition */
-			out_8(&PSC(port)->command, MPC52xx_PSC_RST_ERR_STAT);
+			psc_ops->command(port, MPC52xx_PSC_RST_ERR_STAT);
 
 		}
 		tty_insert_flip_char(tport, ch, flag);
@@ -1066,7 +1145,7 @@ mpc5xxx_uart_process_int(struct uart_port *port)
 		if (psc_ops->tx_rdy(port))
 			keepgoing |= mpc52xx_uart_int_tx_chars(port);
 
-		status = in_8(&PSC(port)->mpc52xx_psc_ipcr);
+		status = psc_ops->get_ipcr(port);
 		if (status & MPC52xx_PSC_D_DCD)
 			uart_handle_dcd_change(port, !(status & MPC52xx_PSC_DCD));
 
@@ -1107,14 +1186,12 @@ static void __init
 mpc52xx_console_get_options(struct uart_port *port,
 			    int *baud, int *parity, int *bits, int *flow)
 {
-	struct mpc52xx_psc __iomem *psc = PSC(port);
 	unsigned char mr1;
 
 	pr_debug("mpc52xx_console_get_options(port=%p)\n", port);
 
 	/* Read the mode registers */
-	out_8(&psc->command, MPC52xx_PSC_SEL_MODE_REG_1);
-	mr1 = in_8(&psc->mode);
+	mr1 = psc_ops->get_mr1(port);
 
 	/* CT{U,L}R are write-only ! */
 	*baud = CONFIG_SERIAL_MPC52xx_CONSOLE_BAUD;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 2/2] serial/mpc52xx_uart: add MPC5125 PSC support
From: Anatolij Gustschin @ 2013-04-17 21:21 UTC (permalink / raw)
  To: linux-serial, Greg Kroah-Hartman
  Cc: Matteo Facchinetti, Anatolij Gustschin, linuxppc-dev,
	Vladimir Ermakov
In-Reply-To: <1366233702-20689-1-git-send-email-agust@denx.de>

From: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>

Add MPC5125 PSC register layout structure, MPC5125 specific
psc_ops function set and the compatible string.

Signed-off-by: Vladimir Ermakov <vooon341@gmail.com>
Signed-off-by: Matteo Facchinetti <matteo.facchinetti@sirius-es.it>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Changes in v2:
 - split into two patches to simplify review
 - minor coding style changes 
 - revise commit log

 arch/powerpc/include/asm/mpc52xx_psc.h |   49 +++++++
 drivers/tty/serial/mpc52xx_uart.c      |  241 ++++++++++++++++++++++++++++++++
 2 files changed, 290 insertions(+)

diff --git a/arch/powerpc/include/asm/mpc52xx_psc.h b/arch/powerpc/include/asm/mpc52xx_psc.h
index 2966df6..d0ece25 100644
--- a/arch/powerpc/include/asm/mpc52xx_psc.h
+++ b/arch/powerpc/include/asm/mpc52xx_psc.h
@@ -299,4 +299,53 @@ struct mpc512x_psc_fifo {
 #define rxdata_32 rxdata.rxdata_32
 };
 
+struct mpc5125_psc {
+	u8		mr1;			/* PSC + 0x00 */
+	u8		reserved0[3];
+	u8		mr2;			/* PSC + 0x04 */
+	u8		reserved1[3];
+	struct {
+		u16		status;		/* PSC + 0x08 */
+		u8		reserved2[2];
+		u8		clock_select;	/* PSC + 0x0c */
+		u8		reserved3[3];
+	} sr_csr;
+	u8		command;		/* PSC + 0x10 */
+	u8		reserved4[3];
+	union {					/* PSC + 0x14 */
+		u8		buffer_8;
+		u16		buffer_16;
+		u32		buffer_32;
+	} buffer;
+	struct {
+		u8		ipcr;		/* PSC + 0x18 */
+		u8		reserved5[3];
+		u8		acr;		/* PSC + 0x1c */
+		u8		reserved6[3];
+	} ipcr_acr;
+	struct {
+		u16		isr;		/* PSC + 0x20 */
+		u8		reserved7[2];
+		u16		imr;		/* PSC + 0x24 */
+		u8		reserved8[2];
+	} isr_imr;
+	u8		ctur;			/* PSC + 0x28 */
+	u8		reserved9[3];
+	u8		ctlr;			/* PSC + 0x2c */
+	u8		reserved10[3];
+	u32		ccr;			/* PSC + 0x30 */
+	u32		ac97slots;		/* PSC + 0x34 */
+	u32		ac97cmd;		/* PSC + 0x38 */
+	u32		ac97data;		/* PSC + 0x3c */
+	u8		reserved11[4];
+	u8		ip;			/* PSC + 0x44 */
+	u8		reserved12[3];
+	u8		op1;			/* PSC + 0x48 */
+	u8		reserved13[3];
+	u8		op0;			/* PSC + 0x4c */
+	u8		reserved14[3];
+	u32		sicr;			/* PSC + 0x50 */
+	u8		reserved15[4];	/* make eq. sizeof(mpc52xx_psc) */
+};
+
 #endif  /* __ASM_MPC52xx_PSC_H__ */
diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 5aa87ac..9c3eab5 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -658,6 +658,246 @@ static void mpc512x_psc_get_irq(struct uart_port *port, struct device_node *np)
 	port->irqflags = IRQF_SHARED;
 	port->irq = psc_fifoc_irq;
 }
+#endif
+
+#ifdef CONFIG_PPC_MPC512x
+
+#define PSC_5125(port) ((struct mpc5125_psc __iomem *)((port)->membase))
+#define FIFO_5125(port) ((struct mpc512x_psc_fifo __iomem *)(PSC_5125(port)+1))
+
+static void mpc5125_psc_fifo_init(struct uart_port *port)
+{
+	/* /32 prescaler */
+	out_8(&PSC_5125(port)->mpc52xx_psc_clock_select, 0xdd);
+
+	out_be32(&FIFO_5125(port)->txcmd, MPC512x_PSC_FIFO_RESET_SLICE);
+	out_be32(&FIFO_5125(port)->txcmd, MPC512x_PSC_FIFO_ENABLE_SLICE);
+	out_be32(&FIFO_5125(port)->txalarm, 1);
+	out_be32(&FIFO_5125(port)->tximr, 0);
+
+	out_be32(&FIFO_5125(port)->rxcmd, MPC512x_PSC_FIFO_RESET_SLICE);
+	out_be32(&FIFO_5125(port)->rxcmd, MPC512x_PSC_FIFO_ENABLE_SLICE);
+	out_be32(&FIFO_5125(port)->rxalarm, 1);
+	out_be32(&FIFO_5125(port)->rximr, 0);
+
+	out_be32(&FIFO_5125(port)->tximr, MPC512x_PSC_FIFO_ALARM);
+	out_be32(&FIFO_5125(port)->rximr, MPC512x_PSC_FIFO_ALARM);
+}
+
+static int mpc5125_psc_raw_rx_rdy(struct uart_port *port)
+{
+	return !(in_be32(&FIFO_5125(port)->rxsr) & MPC512x_PSC_FIFO_EMPTY);
+}
+
+static int mpc5125_psc_raw_tx_rdy(struct uart_port *port)
+{
+	return !(in_be32(&FIFO_5125(port)->txsr) & MPC512x_PSC_FIFO_FULL);
+}
+
+static int mpc5125_psc_rx_rdy(struct uart_port *port)
+{
+	return in_be32(&FIFO_5125(port)->rxsr) &
+	       in_be32(&FIFO_5125(port)->rximr) & MPC512x_PSC_FIFO_ALARM;
+}
+
+static int mpc5125_psc_tx_rdy(struct uart_port *port)
+{
+	return in_be32(&FIFO_5125(port)->txsr) &
+	       in_be32(&FIFO_5125(port)->tximr) & MPC512x_PSC_FIFO_ALARM;
+}
+
+static int mpc5125_psc_tx_empty(struct uart_port *port)
+{
+	return in_be32(&FIFO_5125(port)->txsr) & MPC512x_PSC_FIFO_EMPTY;
+}
+
+static void mpc5125_psc_stop_rx(struct uart_port *port)
+{
+	unsigned long rx_fifo_imr;
+
+	rx_fifo_imr = in_be32(&FIFO_5125(port)->rximr);
+	rx_fifo_imr &= ~MPC512x_PSC_FIFO_ALARM;
+	out_be32(&FIFO_5125(port)->rximr, rx_fifo_imr);
+}
+
+static void mpc5125_psc_start_tx(struct uart_port *port)
+{
+	unsigned long tx_fifo_imr;
+
+	tx_fifo_imr = in_be32(&FIFO_5125(port)->tximr);
+	tx_fifo_imr |= MPC512x_PSC_FIFO_ALARM;
+	out_be32(&FIFO_5125(port)->tximr, tx_fifo_imr);
+}
+
+static void mpc5125_psc_stop_tx(struct uart_port *port)
+{
+	unsigned long tx_fifo_imr;
+
+	tx_fifo_imr = in_be32(&FIFO_5125(port)->tximr);
+	tx_fifo_imr &= ~MPC512x_PSC_FIFO_ALARM;
+	out_be32(&FIFO_5125(port)->tximr, tx_fifo_imr);
+}
+
+static void mpc5125_psc_rx_clr_irq(struct uart_port *port)
+{
+	out_be32(&FIFO_5125(port)->rxisr, in_be32(&FIFO_5125(port)->rxisr));
+}
+
+static void mpc5125_psc_tx_clr_irq(struct uart_port *port)
+{
+	out_be32(&FIFO_5125(port)->txisr, in_be32(&FIFO_5125(port)->txisr));
+}
+
+static void mpc5125_psc_write_char(struct uart_port *port, unsigned char c)
+{
+	out_8(&FIFO_5125(port)->txdata_8, c);
+}
+
+static unsigned char mpc5125_psc_read_char(struct uart_port *port)
+{
+	return in_8(&FIFO_5125(port)->rxdata_8);
+}
+
+static void mpc5125_psc_cw_disable_ints(struct uart_port *port)
+{
+	port->read_status_mask =
+		in_be32(&FIFO_5125(port)->tximr) << 16 |
+		in_be32(&FIFO_5125(port)->rximr);
+	out_be32(&FIFO_5125(port)->tximr, 0);
+	out_be32(&FIFO_5125(port)->rximr, 0);
+}
+
+static void mpc5125_psc_cw_restore_ints(struct uart_port *port)
+{
+	out_be32(&FIFO_5125(port)->tximr,
+		(port->read_status_mask >> 16) & 0x7f);
+	out_be32(&FIFO_5125(port)->rximr, port->read_status_mask & 0x7f);
+}
+
+static inline void mpc5125_set_divisor(struct mpc5125_psc __iomem *psc,
+		u8 prescaler, unsigned int divisor)
+{
+	/* select prescaler */
+	out_8(&psc->mpc52xx_psc_clock_select, prescaler);
+	out_8(&psc->ctur, divisor >> 8);
+	out_8(&psc->ctlr, divisor & 0xff);
+}
+
+static unsigned int mpc5125_psc_set_baudrate(struct uart_port *port,
+					     struct ktermios *new,
+					     struct ktermios *old)
+{
+	unsigned int baud;
+	unsigned int divisor;
+
+	/*
+	 * Calculate with a /16 prescaler here.
+	 */
+
+	/* uartclk contains the ips freq */
+	baud = uart_get_baud_rate(port, new, old,
+				  port->uartclk / (16 * 0xffff) + 1,
+				  port->uartclk / 16);
+	divisor = (port->uartclk + 8 * baud) / (16 * baud);
+
+	/* enable the /16 prescaler and set the divisor */
+	mpc5125_set_divisor(PSC_5125(port), 0xdd, divisor);
+	return baud;
+}
+
+/*
+ * MPC5125 have compatible PSC FIFO Controller.
+ * Special init not needed.
+ */
+static u16 mpc5125_psc_get_status(struct uart_port *port)
+{
+	return in_be16(&PSC_5125(port)->mpc52xx_psc_status);
+}
+
+static u8 mpc5125_psc_get_ipcr(struct uart_port *port)
+{
+	return in_8(&PSC_5125(port)->mpc52xx_psc_ipcr);
+}
+
+static void mpc5125_psc_command(struct uart_port *port, u8 cmd)
+{
+	out_8(&PSC_5125(port)->command, cmd);
+}
+
+static void mpc5125_psc_set_mode(struct uart_port *port, u8 mr1, u8 mr2)
+{
+	out_8(&PSC_5125(port)->mr1, mr1);
+	out_8(&PSC_5125(port)->mr2, mr2);
+}
+
+static void mpc5125_psc_set_rts(struct uart_port *port, int state)
+{
+	if (state & TIOCM_RTS)
+		out_8(&PSC_5125(port)->op1, MPC52xx_PSC_OP_RTS);
+	else
+		out_8(&PSC_5125(port)->op0, MPC52xx_PSC_OP_RTS);
+}
+
+static void mpc5125_psc_enable_ms(struct uart_port *port)
+{
+	struct mpc5125_psc __iomem *psc = PSC_5125(port);
+
+	/* clear D_*-bits by reading them */
+	in_8(&psc->mpc52xx_psc_ipcr);
+	/* enable CTS and DCD as IPC interrupts */
+	out_8(&psc->mpc52xx_psc_acr, MPC52xx_PSC_IEC_CTS | MPC52xx_PSC_IEC_DCD);
+
+	port->read_status_mask |= MPC52xx_PSC_IMR_IPC;
+	out_be16(&psc->mpc52xx_psc_imr, port->read_status_mask);
+}
+
+static void mpc5125_psc_set_sicr(struct uart_port *port, u32 val)
+{
+	out_be32(&PSC_5125(port)->sicr, val);
+}
+
+static void mpc5125_psc_set_imr(struct uart_port *port, u16 val)
+{
+	out_be16(&PSC_5125(port)->mpc52xx_psc_imr, val);
+}
+
+static u8 mpc5125_psc_get_mr1(struct uart_port *port)
+{
+	return in_8(&PSC_5125(port)->mr1);
+}
+
+static struct psc_ops mpc5125_psc_ops = {
+	.fifo_init = mpc5125_psc_fifo_init,
+	.raw_rx_rdy = mpc5125_psc_raw_rx_rdy,
+	.raw_tx_rdy = mpc5125_psc_raw_tx_rdy,
+	.rx_rdy = mpc5125_psc_rx_rdy,
+	.tx_rdy = mpc5125_psc_tx_rdy,
+	.tx_empty = mpc5125_psc_tx_empty,
+	.stop_rx = mpc5125_psc_stop_rx,
+	.start_tx = mpc5125_psc_start_tx,
+	.stop_tx = mpc5125_psc_stop_tx,
+	.rx_clr_irq = mpc5125_psc_rx_clr_irq,
+	.tx_clr_irq = mpc5125_psc_tx_clr_irq,
+	.write_char = mpc5125_psc_write_char,
+	.read_char = mpc5125_psc_read_char,
+	.cw_disable_ints = mpc5125_psc_cw_disable_ints,
+	.cw_restore_ints = mpc5125_psc_cw_restore_ints,
+	.set_baudrate = mpc5125_psc_set_baudrate,
+	.clock = mpc512x_psc_clock,
+	.fifoc_init = mpc512x_psc_fifoc_init,
+	.fifoc_uninit = mpc512x_psc_fifoc_uninit,
+	.get_irq = mpc512x_psc_get_irq,
+	.handle_irq = mpc512x_psc_handle_irq,
+	.get_status = mpc5125_psc_get_status,
+	.get_ipcr = mpc5125_psc_get_ipcr,
+	.command = mpc5125_psc_command,
+	.set_mode = mpc5125_psc_set_mode,
+	.set_rts = mpc5125_psc_set_rts,
+	.enable_ms = mpc5125_psc_enable_ms,
+	.set_sicr = mpc5125_psc_set_sicr,
+	.set_imr = mpc5125_psc_set_imr,
+	.get_mr1 = mpc5125_psc_get_mr1,
+};
 
 static struct psc_ops mpc512x_psc_ops = {
 	.fifo_init = mpc512x_psc_fifo_init,
@@ -1381,6 +1621,7 @@ static struct of_device_id mpc52xx_uart_of_match[] = {
 #endif
 #ifdef CONFIG_PPC_MPC512x
 	{ .compatible = "fsl,mpc5121-psc-uart", .data = &mpc512x_psc_ops, },
+	{ .compatible = "fsl,mpc5125-psc-uart", .data = &mpc5125_psc_ops, },
 #endif
 	{},
 };
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
From: Bjorn Helgaas @ 2013-04-17 20:30 UTC (permalink / raw)
  To: Alex Deucher
  Cc: DRI mailing list, Kleber Sacilotto de Souza, Brian King,
	Jerome Glisse, Thadeu Lima de Souza Cascardo,
	Lucas Kannebley Tavares, Alex Deucher, linuxppc-dev
In-Reply-To: <CADnq5_MvDfziNtWMdUxfxFV_iRuLdAhfT6diZmL9n2xf2K7WHw@mail.gmail.com>

On Wed, Apr 17, 2013 at 2:17 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Apr 17, 2013 at 4:11 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares
>>> <lucaskt@linux.vnet.ibm.com> wrote:
>>>> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote:
>>>>>
>>>>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares
>>>>> <lucaskt@linux.vnet.ibm.com>  wrote:
>>>>>>
>>>>>> radeon currently uses a drm function to get the speed capabilities for
>>>>>> the bus. However, this is a non-standard method of performing this
>>>>>> detection and this patch changes it to use the max_bus_speed attribute.
>>>>>> ---
>>>>>>   drivers/gpu/drm/radeon/evergreen.c |    9 ++-------
>>>>>>   drivers/gpu/drm/radeon/r600.c      |    8 +-------
>>>>>>   drivers/gpu/drm/radeon/rv770.c     |    8 +-------
>>>>>>   3 files changed, 4 insertions(+), 21 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c
>>>>>> b/drivers/gpu/drm/radeon/evergreen.c
>>>>>> index 305a657..3291f62 100644
>>>>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>>>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>>>>> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev)
>>>>>>
>>>>>>   void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
>>>>>>   {
>>>>>> -       u32 link_width_cntl, speed_cntl, mask;
>>>>>> -       int ret;
>>>>>> +       u32 link_width_cntl, speed_cntl;
>>>>>>
>>>>>>          if (radeon_pcie_gen2 == 0)
>>>>>>                  return;
>>>>>> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct
>>>>>> radeon_device *rdev)
>>>>>>          if (ASIC_IS_X2(rdev))
>>>>>>                  return;
>>>>>>
>>>>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>>>>> -       if (ret != 0)
>>>>>> -               return;
>>>>>> -
>>>>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>>>>
>>>>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>>>>
>>>>>
>>>>> For devices on a root bus, we previously dereferenced a NULL pointer
>>>>> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a
>>>>> root bus.  (I think this is the original problem you tripped over,
>>>>> Lucas.)
>>>>>
>>>>> These patches fix that problem.  On pseries, where the device *is* on
>>>>> a root bus, your patches set max_bus_speed so this will work as
>>>>> expected.  On most other systems, max_bus_speed for root buses will be
>>>>> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because
>>>>> most arches don't have code like the pseries code you're adding).
>>>>>
>>>>> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on
>>>>> the root bus, we'll attempt to enable Gen2 on the device even though
>>>>> we have no idea what the bus will support.
>>>>>
>>>>> That's why I originally suggested skipping the Gen2 stuff if
>>>>> "max_bus_speed == PCI_SPEED_UNKNOWN".  I was just being conservative,
>>>>> thinking that it's better to have a functional but slow GPU rather
>>>>> than the unknown (to me) effects of enabling Gen2 on a link that might
>>>>> not support it.  But I'm fine with this being either way.
>>>>
>>>>
>>>> You're right here, of course. I'll test for it being different from 5_0GT
>>>> and 8_0GT instead. Though at some point I suppose someone will want to
>>>> tackle Gen3 speeds.
>>>
>>> drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see
>>> what speeds it supported.  If the new code doesn't actually check the
>>> bridge caps then the new code does not seem like a valid replacement
>>> unless I'm missing something.
>>
>> The max_bus_speed in struct pci_bus is set in pci_set_bus_speed()
>> based on the PCIe, PCI-X, or AGP capabilities.  This happens when we
>> enumerate the bridge device.  Or, in this case, Lucas added
>> powerpc-specific code to set max_bus_speed for the root bus based on
>> platform-specific host bridge knowledge.
>>
>> So it still does check the PCI bridge capabilities, just not as
>> directly as it did before.
>
> Ah, ok.  Thanks.  The previous comments about PCI_SPEED_UNKNOWN being
> set in pci_alloc_bus() and never updated confused me.

Yeah, that's just for root buses where we don't have the host bridge
knowledge to figure it out.  The root bus case was broken in
drm_pcie_get_speed_cap_mask() anyway, because there is no upstream P2P
bridge to look at.  (That's why Lucas tripped over the null pointer
dereference in the first place.)

^ permalink raw reply

* Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
From: Alex Deucher @ 2013-04-17 20:17 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: DRI mailing list, Kleber Sacilotto de Souza, Brian King,
	Jerome Glisse, Thadeu Lima de Souza Cascardo,
	Lucas Kannebley Tavares, Alex Deucher, linuxppc-dev
In-Reply-To: <CAErSpo4TDYrNZthVMkFF0E-U5bt31=WQvO_RN6xQLEnq7yOaWw@mail.gmail.com>

On Wed, Apr 17, 2013 at 4:11 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares
>> <lucaskt@linux.vnet.ibm.com> wrote:
>>> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote:
>>>>
>>>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares
>>>> <lucaskt@linux.vnet.ibm.com>  wrote:
>>>>>
>>>>> radeon currently uses a drm function to get the speed capabilities for
>>>>> the bus. However, this is a non-standard method of performing this
>>>>> detection and this patch changes it to use the max_bus_speed attribute.
>>>>> ---
>>>>>   drivers/gpu/drm/radeon/evergreen.c |    9 ++-------
>>>>>   drivers/gpu/drm/radeon/r600.c      |    8 +-------
>>>>>   drivers/gpu/drm/radeon/rv770.c     |    8 +-------
>>>>>   3 files changed, 4 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c
>>>>> b/drivers/gpu/drm/radeon/evergreen.c
>>>>> index 305a657..3291f62 100644
>>>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>>>> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev)
>>>>>
>>>>>   void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
>>>>>   {
>>>>> -       u32 link_width_cntl, speed_cntl, mask;
>>>>> -       int ret;
>>>>> +       u32 link_width_cntl, speed_cntl;
>>>>>
>>>>>          if (radeon_pcie_gen2 == 0)
>>>>>                  return;
>>>>> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct
>>>>> radeon_device *rdev)
>>>>>          if (ASIC_IS_X2(rdev))
>>>>>                  return;
>>>>>
>>>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>>>> -       if (ret != 0)
>>>>> -               return;
>>>>> -
>>>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>>>
>>>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>>>
>>>>
>>>> For devices on a root bus, we previously dereferenced a NULL pointer
>>>> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a
>>>> root bus.  (I think this is the original problem you tripped over,
>>>> Lucas.)
>>>>
>>>> These patches fix that problem.  On pseries, where the device *is* on
>>>> a root bus, your patches set max_bus_speed so this will work as
>>>> expected.  On most other systems, max_bus_speed for root buses will be
>>>> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because
>>>> most arches don't have code like the pseries code you're adding).
>>>>
>>>> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on
>>>> the root bus, we'll attempt to enable Gen2 on the device even though
>>>> we have no idea what the bus will support.
>>>>
>>>> That's why I originally suggested skipping the Gen2 stuff if
>>>> "max_bus_speed == PCI_SPEED_UNKNOWN".  I was just being conservative,
>>>> thinking that it's better to have a functional but slow GPU rather
>>>> than the unknown (to me) effects of enabling Gen2 on a link that might
>>>> not support it.  But I'm fine with this being either way.
>>>
>>>
>>> You're right here, of course. I'll test for it being different from 5_0GT
>>> and 8_0GT instead. Though at some point I suppose someone will want to
>>> tackle Gen3 speeds.
>>
>> drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see
>> what speeds it supported.  If the new code doesn't actually check the
>> bridge caps then the new code does not seem like a valid replacement
>> unless I'm missing something.
>
> The max_bus_speed in struct pci_bus is set in pci_set_bus_speed()
> based on the PCIe, PCI-X, or AGP capabilities.  This happens when we
> enumerate the bridge device.  Or, in this case, Lucas added
> powerpc-specific code to set max_bus_speed for the root bus based on
> platform-specific host bridge knowledge.
>
> So it still does check the PCI bridge capabilities, just not as
> directly as it did before.

Ah, ok.  Thanks.  The previous comments about PCI_SPEED_UNKNOWN being
set in pci_alloc_bus() and never updated confused me.

Alex

^ permalink raw reply

* Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
From: Bjorn Helgaas @ 2013-04-17 20:11 UTC (permalink / raw)
  To: Alex Deucher
  Cc: DRI mailing list, Kleber Sacilotto de Souza, Brian King,
	Jerome Glisse, Thadeu Lima de Souza Cascardo,
	Lucas Kannebley Tavares, Alex Deucher, linuxppc-dev
In-Reply-To: <CADnq5_MA6T0Qu61_WxkHnAJc_PmR+un6W-rfHUVecjWydoXHVw@mail.gmail.com>

On Wed, Apr 17, 2013 at 2:04 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares
> <lucaskt@linux.vnet.ibm.com> wrote:
>> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote:
>>>
>>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares
>>> <lucaskt@linux.vnet.ibm.com>  wrote:
>>>>
>>>> radeon currently uses a drm function to get the speed capabilities for
>>>> the bus. However, this is a non-standard method of performing this
>>>> detection and this patch changes it to use the max_bus_speed attribute.
>>>> ---
>>>>   drivers/gpu/drm/radeon/evergreen.c |    9 ++-------
>>>>   drivers/gpu/drm/radeon/r600.c      |    8 +-------
>>>>   drivers/gpu/drm/radeon/rv770.c     |    8 +-------
>>>>   3 files changed, 4 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c
>>>> b/drivers/gpu/drm/radeon/evergreen.c
>>>> index 305a657..3291f62 100644
>>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>>> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev)
>>>>
>>>>   void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
>>>>   {
>>>> -       u32 link_width_cntl, speed_cntl, mask;
>>>> -       int ret;
>>>> +       u32 link_width_cntl, speed_cntl;
>>>>
>>>>          if (radeon_pcie_gen2 == 0)
>>>>                  return;
>>>> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct
>>>> radeon_device *rdev)
>>>>          if (ASIC_IS_X2(rdev))
>>>>                  return;
>>>>
>>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>>> -       if (ret != 0)
>>>> -               return;
>>>> -
>>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>>
>>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>>
>>>
>>> For devices on a root bus, we previously dereferenced a NULL pointer
>>> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a
>>> root bus.  (I think this is the original problem you tripped over,
>>> Lucas.)
>>>
>>> These patches fix that problem.  On pseries, where the device *is* on
>>> a root bus, your patches set max_bus_speed so this will work as
>>> expected.  On most other systems, max_bus_speed for root buses will be
>>> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because
>>> most arches don't have code like the pseries code you're adding).
>>>
>>> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on
>>> the root bus, we'll attempt to enable Gen2 on the device even though
>>> we have no idea what the bus will support.
>>>
>>> That's why I originally suggested skipping the Gen2 stuff if
>>> "max_bus_speed == PCI_SPEED_UNKNOWN".  I was just being conservative,
>>> thinking that it's better to have a functional but slow GPU rather
>>> than the unknown (to me) effects of enabling Gen2 on a link that might
>>> not support it.  But I'm fine with this being either way.
>>
>>
>> You're right here, of course. I'll test for it being different from 5_0GT
>> and 8_0GT instead. Though at some point I suppose someone will want to
>> tackle Gen3 speeds.
>
> drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see
> what speeds it supported.  If the new code doesn't actually check the
> bridge caps then the new code does not seem like a valid replacement
> unless I'm missing something.

The max_bus_speed in struct pci_bus is set in pci_set_bus_speed()
based on the PCIe, PCI-X, or AGP capabilities.  This happens when we
enumerate the bridge device.  Or, in this case, Lucas added
powerpc-specific code to set max_bus_speed for the root bus based on
platform-specific host bridge knowledge.

So it still does check the PCI bridge capabilities, just not as
directly as it did before.

Bjorn

^ permalink raw reply

* Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
From: Alex Deucher @ 2013-04-17 20:04 UTC (permalink / raw)
  To: Lucas Kannebley Tavares
  Cc: Brian King, DRI mailing list, Kleber Sacilotto de Souza,
	Alex Deucher, Jerome Glisse, Thadeu Lima de Souza Cascardo,
	Bjorn Helgaas, linuxppc-dev
In-Reply-To: <516E97A9.7050102@linux.vnet.ibm.com>

On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares
<lucaskt@linux.vnet.ibm.com> wrote:
> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote:
>>
>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares
>> <lucaskt@linux.vnet.ibm.com>  wrote:
>>>
>>> radeon currently uses a drm function to get the speed capabilities for
>>> the bus. However, this is a non-standard method of performing this
>>> detection and this patch changes it to use the max_bus_speed attribute.
>>> ---
>>>   drivers/gpu/drm/radeon/evergreen.c |    9 ++-------
>>>   drivers/gpu/drm/radeon/r600.c      |    8 +-------
>>>   drivers/gpu/drm/radeon/rv770.c     |    8 +-------
>>>   3 files changed, 4 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/evergreen.c
>>> b/drivers/gpu/drm/radeon/evergreen.c
>>> index 305a657..3291f62 100644
>>> --- a/drivers/gpu/drm/radeon/evergreen.c
>>> +++ b/drivers/gpu/drm/radeon/evergreen.c
>>> @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev)
>>>
>>>   void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
>>>   {
>>> -       u32 link_width_cntl, speed_cntl, mask;
>>> -       int ret;
>>> +       u32 link_width_cntl, speed_cntl;
>>>
>>>          if (radeon_pcie_gen2 == 0)
>>>                  return;
>>> @@ -3871,11 +3870,7 @@ void evergreen_pcie_gen2_enable(struct
>>> radeon_device *rdev)
>>>          if (ASIC_IS_X2(rdev))
>>>                  return;
>>>
>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>> -       if (ret != 0)
>>> -               return;
>>> -
>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>
>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>
>>
>> For devices on a root bus, we previously dereferenced a NULL pointer
>> in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a
>> root bus.  (I think this is the original problem you tripped over,
>> Lucas.)
>>
>> These patches fix that problem.  On pseries, where the device *is* on
>> a root bus, your patches set max_bus_speed so this will work as
>> expected.  On most other systems, max_bus_speed for root buses will be
>> PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because
>> most arches don't have code like the pseries code you're adding).
>>
>> PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on
>> the root bus, we'll attempt to enable Gen2 on the device even though
>> we have no idea what the bus will support.
>>
>> That's why I originally suggested skipping the Gen2 stuff if
>> "max_bus_speed == PCI_SPEED_UNKNOWN".  I was just being conservative,
>> thinking that it's better to have a functional but slow GPU rather
>> than the unknown (to me) effects of enabling Gen2 on a link that might
>> not support it.  But I'm fine with this being either way.
>
>
> You're right here, of course. I'll test for it being different from 5_0GT
> and 8_0GT instead. Though at some point I suppose someone will want to
> tackle Gen3 speeds.

drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see
what speeds it supported.  If the new code doesn't actually check the
bridge caps then the new code does not seem like a valid replacement
unless I'm missing something.

Alex

>
>
>>
>> It would be nice if we could get rid of drm_pcie_get_speed_cap_mask()
>> altogether.  It is exported, but I have no idea of anybody else uses
>> it.  Maybe it could at least be marked __deprecated now?
>>
>
> I'll mark it.
>
>> I don't know who should take these patches.  They don't touch
>> drivers/pci, but I'd be happy to push them, given the appropriate ACKs
>> from DRM and powerpc folks.
>>
>> Bjorn
>>
>>>                  return;
>>>
>>>          speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
>>> diff --git a/drivers/gpu/drm/radeon/r600.c
>>> b/drivers/gpu/drm/radeon/r600.c
>>> index 0740db3..64b90c0 100644
>>> --- a/drivers/gpu/drm/radeon/r600.c
>>> +++ b/drivers/gpu/drm/radeon/r600.c
>>> @@ -4351,8 +4351,6 @@ static void r600_pcie_gen2_enable(struct
>>> radeon_device *rdev)
>>>   {
>>>          u32 link_width_cntl, lanes, speed_cntl, training_cntl, tmp;
>>>          u16 link_cntl2;
>>> -       u32 mask;
>>> -       int ret;
>>>
>>>          if (radeon_pcie_gen2 == 0)
>>>                  return;
>>> @@ -4371,11 +4369,7 @@ static void r600_pcie_gen2_enable(struct
>>> radeon_device *rdev)
>>>          if (rdev->family<= CHIP_R600)
>>>                  return;
>>>
>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>> -       if (ret != 0)
>>> -               return;
>>> -
>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>
>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>>                  return;
>>>
>>>          speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
>>> diff --git a/drivers/gpu/drm/radeon/rv770.c
>>> b/drivers/gpu/drm/radeon/rv770.c
>>> index d63fe1d..c683c36 100644
>>> --- a/drivers/gpu/drm/radeon/rv770.c
>>> +++ b/drivers/gpu/drm/radeon/rv770.c
>>> @@ -1238,8 +1238,6 @@ static void rv770_pcie_gen2_enable(struct
>>> radeon_device *rdev)
>>>   {
>>>          u32 link_width_cntl, lanes, speed_cntl, tmp;
>>>          u16 link_cntl2;
>>> -       u32 mask;
>>> -       int ret;
>>>
>>>          if (radeon_pcie_gen2 == 0)
>>>                  return;
>>> @@ -1254,11 +1252,7 @@ static void rv770_pcie_gen2_enable(struct
>>> radeon_device *rdev)
>>>          if (ASIC_IS_X2(rdev))
>>>                  return;
>>>
>>> -       ret = drm_pcie_get_speed_cap_mask(rdev->ddev,&mask);
>>> -       if (ret != 0)
>>> -               return;
>>> -
>>> -       if (!(mask&  DRM_PCIE_SPEED_50))
>>>
>>> +       if (rdev->pdev->bus->max_bus_speed<  PCIE_SPEED_5_0GT)
>>>                  return;
>>>
>>>          DRM_INFO("enabling PCIE gen 2 link speeds, disable with
>>> radeon.pcie_gen2=0\n");
>>> --
>>> 1.7.4.4
>>>
>>
>
>
> --
> Lucas Kannebley Tavares
> Software Engineer
> IBM Linux Technology Center
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* questions around Book III-E and branch trace
From: Chris Friesen @ 2013-04-17 18:44 UTC (permalink / raw)
  To: Kumar Gala, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras

Hi,

I'm trying to wrap my head around how linux handles branch tracing on 
Book III-E.  I think I understand how we set MSR[DE] and DBCR0[IDM|BT], 
and how we handle fixing things up if an instruction being traced causes 
an exception.

I have a few questions though:

1) Does user_enable_block_step() have a bug in it?  The current code has

task->thread.dbcr0 = DBCR0_IDM | DBCR0_BT;

Should that be as follows (to match the singel-step case)?

task->thread.dbcr0 |= DBCR0_IDM | DBCR0_BT;


2) Why doesn't DBCR0_ACTIVE_EVENTS include DBCR0_BT?


3) In sys_debug_setcontext() why does SIG_DBG_BRANCH_TRACING return 
-EINVAL if CONFIG_PPC_ADV_DEBUG_REGS is set?  Would it not be possible 
to use DBCR0_BT?

Thanks,
Chris


-- 

Chris Friesen
Software Designer

500 Palladium Drive, Suite 2100
Ottawa, Ontario K2N 1C2, Canada
www.genband.com
office:+1.343.883.2717
chris.friesen@genband.com

^ permalink raw reply

* Re: [PATCH v7 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
From: Jason Cooper @ 2013-04-17 16:22 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-mips@linux-mips.org, siva.kallam@samsung.com,
	linux-pci@vger.kernel.org, linus.walleij@linaro.org,
	thierry.reding@avionic-design.de, Liviu Dudau, Gregory CLEMENT,
	paulus@samba.org, linux-samsung-soc@vger.kernel.org,
	linux@arm.linux.org.uk, jg1.han@samsung.com,
	jgunthorpe@obsidianresearch.com, thomas.abraham@linaro.org,
	arnd@arndb.de, devicetree-discuss@lists.ozlabs.org,
	rob.herring@calxeda.com, kgene.kim@samsung.com,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org,
	thomas.petazzoni@free-electrons.com, monstr@monstr.eu,
	linux-kernel@vger.kernel.org, Andrew Lunn,
	suren.reddy@samsung.com, Olof Johansson, Andrew Murray,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CACxGe6sNk=wNinTcMHbJa5o-56Tyh=0CnFSEW+Hk-ujpjeg2gQ@mail.gmail.com>

On Wed, Apr 17, 2013 at 05:17:48PM +0100, Grant Likely wrote:
> On Wed, Apr 17, 2013 at 5:10 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> > On Wed, Apr 17, 2013 at 05:00:15PM +0100, Grant Likely wrote:
> >> On Tue, 16 Apr 2013 11:30:06 +0100, Andrew Murray <andrew.murray@arm.com> wrote:
> >> > On Tue, Apr 16, 2013 at 11:18:26AM +0100, Andrew Murray wrote:
> >> > > The pci_process_bridge_OF_ranges function, used to parse the "ranges"
> >> > > property of a PCI host device, is found in both Microblaze and PowerPC
> >> > > architectures. These implementations are nearly identical. This patch
> >> > > moves this common code to a common place.
> >> > >
> >> > > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> >> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >> > > Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> >> > > Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> >> > > Acked-by: Michal Simek <monstr@monstr.eu>
> >> > > ---
> >> > >  arch/microblaze/include/asm/pci-bridge.h |    5 +-
> >> > >  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
> >> > >  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
> >> > >  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
> >> >
> >> > Is there anyone on linuxppc-dev/linux-mips that can help test this patchset?
> >> >
> >> > I've tested that it builds on powerpc with a variety of configs (some which
> >> > include fsl_pci.c implementation). Though I don't have hardware to verify that
> >> > it works.
> >> >
> >> > I haven't tested this builds or runs on MIPS.
> >> >
> >> > You shouldn't see any difference in behaviour or new warnings and PCI devices
> >> > should continue to operate as before.
> >>
> >> I've got through a line-by-line comparison between powerpc, microblaze,
> >> and then new code. The differences are purely cosmetic, so I have
> >> absolutely no concerns about this patch. I've applied it to my tree.
> >
> > oops.  Due to the number of dependencies the mvebu-pcie series has (this
> > being one of them, we (arm-soc/mvebu) asked if we could take this
> > through our tree.  Rob Herring agreed to this several days ago.  Is this
> > a problem for you?
> >
> > It would truly (dogs and cats living together) upset the apple cart for
> > us at this stage to pipe these through a different tree...
> 
> Not a problem at all. I'll drop it.

Great!  Thanks.

Jason.

^ permalink raw reply

* Re: [PATCH v7 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
From: Grant Likely @ 2013-04-17 16:17 UTC (permalink / raw)
  To: Jason Cooper
  Cc: linux-mips@linux-mips.org, siva.kallam@samsung.com,
	linux-pci@vger.kernel.org, linus.walleij@linaro.org,
	thierry.reding@avionic-design.de, Liviu Dudau, Gregory CLEMENT,
	paulus@samba.org, linux-samsung-soc@vger.kernel.org,
	linux@arm.linux.org.uk, jg1.han@samsung.com,
	jgunthorpe@obsidianresearch.com, thomas.abraham@linaro.org,
	arnd@arndb.de, devicetree-discuss@lists.ozlabs.org,
	rob.herring@calxeda.com, kgene.kim@samsung.com,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org,
	thomas.petazzoni@free-electrons.com, monstr@monstr.eu,
	linux-kernel@vger.kernel.org, Andrew Lunn,
	suren.reddy@samsung.com, Olof Johansson, Andrew Murray,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130417161036.GB27197@titan.lakedaemon.net>

On Wed, Apr 17, 2013 at 5:10 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> On Wed, Apr 17, 2013 at 05:00:15PM +0100, Grant Likely wrote:
>> On Tue, 16 Apr 2013 11:30:06 +0100, Andrew Murray <andrew.murray@arm.com> wrote:
>> > On Tue, Apr 16, 2013 at 11:18:26AM +0100, Andrew Murray wrote:
>> > > The pci_process_bridge_OF_ranges function, used to parse the "ranges"
>> > > property of a PCI host device, is found in both Microblaze and PowerPC
>> > > architectures. These implementations are nearly identical. This patch
>> > > moves this common code to a common place.
>> > >
>> > > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
>> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>> > > Reviewed-by: Rob Herring <rob.herring@calxeda.com>
>> > > Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
>> > > Acked-by: Michal Simek <monstr@monstr.eu>
>> > > ---
>> > >  arch/microblaze/include/asm/pci-bridge.h |    5 +-
>> > >  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
>> > >  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
>> > >  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
>> >
>> > Is there anyone on linuxppc-dev/linux-mips that can help test this patchset?
>> >
>> > I've tested that it builds on powerpc with a variety of configs (some which
>> > include fsl_pci.c implementation). Though I don't have hardware to verify that
>> > it works.
>> >
>> > I haven't tested this builds or runs on MIPS.
>> >
>> > You shouldn't see any difference in behaviour or new warnings and PCI devices
>> > should continue to operate as before.
>>
>> I've got through a line-by-line comparison between powerpc, microblaze,
>> and then new code. The differences are purely cosmetic, so I have
>> absolutely no concerns about this patch. I've applied it to my tree.
>
> oops.  Due to the number of dependencies the mvebu-pcie series has (this
> being one of them, we (arm-soc/mvebu) asked if we could take this
> through our tree.  Rob Herring agreed to this several days ago.  Is this
> a problem for you?
>
> It would truly (dogs and cats living together) upset the apple cart for
> us at this stage to pipe these through a different tree...

Not a problem at all. I'll drop it.

g.

^ permalink raw reply

* Re: [PATCH v7 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
From: Jason Cooper @ 2013-04-17 16:10 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-mips@linux-mips.org, siva.kallam@samsung.com,
	linux-pci@vger.kernel.org, linus.walleij@linaro.org,
	thierry.reding@avionic-design.de, Liviu Dudau, Gregory CLEMENT,
	paulus@samba.org, linux-samsung-soc@vger.kernel.org,
	linux@arm.linux.org.uk, jg1.han@samsung.com,
	jgunthorpe@obsidianresearch.com, thomas.abraham@linaro.org,
	arnd@arndb.de, devicetree-discuss@lists.ozlabs.org,
	rob.herring@calxeda.com, kgene.kim@samsung.com,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org,
	thomas.petazzoni@free-electrons.com, monstr@monstr.eu,
	linux-kernel@vger.kernel.org, Andrew Lunn,
	suren.reddy@samsung.com, Olof Johansson, Andrew Murray,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130417160015.777453E2B73@localhost>

On Wed, Apr 17, 2013 at 05:00:15PM +0100, Grant Likely wrote:
> On Tue, 16 Apr 2013 11:30:06 +0100, Andrew Murray <andrew.murray@arm.com> wrote:
> > On Tue, Apr 16, 2013 at 11:18:26AM +0100, Andrew Murray wrote:
> > > The pci_process_bridge_OF_ranges function, used to parse the "ranges"
> > > property of a PCI host device, is found in both Microblaze and PowerPC
> > > architectures. These implementations are nearly identical. This patch
> > > moves this common code to a common place.
> > > 
> > > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > > Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> > > Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > > Acked-by: Michal Simek <monstr@monstr.eu>
> > > ---
> > >  arch/microblaze/include/asm/pci-bridge.h |    5 +-
> > >  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
> > >  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
> > >  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
> > 
> > Is there anyone on linuxppc-dev/linux-mips that can help test this patchset?
> > 
> > I've tested that it builds on powerpc with a variety of configs (some which
> > include fsl_pci.c implementation). Though I don't have hardware to verify that
> > it works.
> > 
> > I haven't tested this builds or runs on MIPS.
> > 
> > You shouldn't see any difference in behaviour or new warnings and PCI devices
> > should continue to operate as before.
> 
> I've got through a line-by-line comparison between powerpc, microblaze,
> and then new code. The differences are purely cosmetic, so I have
> absolutely no concerns about this patch. I've applied it to my tree.

oops.  Due to the number of dependencies the mvebu-pcie series has (this
being one of them, we (arm-soc/mvebu) asked if we could take this
through our tree.  Rob Herring agreed to this several days ago.  Is this
a problem for you?

It would truly (dogs and cats living together) upset the apple cart for
us at this stage to pipe these through a different tree...

thx,

Jason.

^ permalink raw reply

* Re: [PATCH v7 1/3] of/pci: Unify pci_process_bridge_OF_ranges from Microblaze and PowerPC
From: Grant Likely @ 2013-04-17 16:00 UTC (permalink / raw)
  To: Andrew Murray, linux-mips@linux-mips.org,
	linuxppc-dev@lists.ozlabs.org
  Cc: siva.kallam@samsung.com, linux-pci@vger.kernel.org,
	linus.walleij@linaro.org, thierry.reding@avionic-design.de,
	Liviu Dudau, paulus@samba.org, linux-samsung-soc@vger.kernel.org,
	linux@arm.linux.org.uk, jg1.han@samsung.com,
	jgunthorpe@obsidianresearch.com, thomas.abraham@linaro.org,
	arnd@arndb.de, devicetree-discuss@lists.ozlabs.org,
	rob.herring@calxeda.com, kgene.kim@samsung.com,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org,
	thomas.petazzoni@free-electrons.com, monstr@monstr.eu,
	linux-kernel@vger.kernel.org, suren.reddy@samsung.com
In-Reply-To: <20130416103005.GB12726@arm.com>

On Tue, 16 Apr 2013 11:30:06 +0100, Andrew Murray <andrew.murray@arm.com> wrote:
> On Tue, Apr 16, 2013 at 11:18:26AM +0100, Andrew Murray wrote:
> > The pci_process_bridge_OF_ranges function, used to parse the "ranges"
> > property of a PCI host device, is found in both Microblaze and PowerPC
> > architectures. These implementations are nearly identical. This patch
> > moves this common code to a common place.
> > 
> > Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> > Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > Acked-by: Michal Simek <monstr@monstr.eu>
> > ---
> >  arch/microblaze/include/asm/pci-bridge.h |    5 +-
> >  arch/microblaze/pci/pci-common.c         |  192 ----------------------------
> >  arch/powerpc/include/asm/pci-bridge.h    |    5 +-
> >  arch/powerpc/kernel/pci-common.c         |  192 ----------------------------
> 
> Is there anyone on linuxppc-dev/linux-mips that can help test this patchset?
> 
> I've tested that it builds on powerpc with a variety of configs (some which
> include fsl_pci.c implementation). Though I don't have hardware to verify that
> it works.
> 
> I haven't tested this builds or runs on MIPS.
> 
> You shouldn't see any difference in behaviour or new warnings and PCI devices
> should continue to operate as before.

I've got through a line-by-line comparison between powerpc, microblaze,
and then new code. The differences are purely cosmetic, so I have
absolutely no concerns about this patch. I've applied it to my tree.

g.

^ permalink raw reply

* Re: [PATCH v7 3/3] of/pci: mips: convert to common of_pci_range_parser
From: Linus Walleij @ 2013-04-17 15:42 UTC (permalink / raw)
  To: Andrew Murray
  Cc: linux-mips, siva.kallam, linux-pci, Thierry Reding, Liviu.Dudau,
	Paul Mackerras, linux-samsung-soc, Russell King - ARM Linux,
	Jingoo Han, Jason Gunthorpe, Thomas Abraham, Arnd Bergmann,
	devicetree-discuss@lists.ozlabs.org, Rob Herring, Kukjin Kim,
	bhelgaas@google.com, linux-arm-kernel@lists.infradead.org,
	Thomas Petazzoni, Michal Simek, linux-kernel@vger.kernel.org,
	suren.reddy, linuxppc-dev@lists.ozlabs.org list
In-Reply-To: <1366107508-12672-4-git-send-email-Andrew.Murray@arm.com>

On Tue, Apr 16, 2013 at 12:18 PM, Andrew Murray <Andrew.Murray@arm.com> wrote:

> This patch converts the pci_load_of_ranges function to use the new common
> of_pci_range_parser.
>
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Reviewed-by: Rob Herring <rob.herring@calxeda.com>

Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] usb: remove redundant tdi_reset
From: Alan Stern @ 2013-04-17 15:03 UTC (permalink / raw)
  To: Shengzhou Liu; +Cc: linux-usb, linuxppc-dev, stable
In-Reply-To: <1366193026-20871-1-git-send-email-Shengzhou.Liu@freescale.com>

On Wed, 17 Apr 2013, Shengzhou Liu wrote:

> We remove the redundant tdi_reset in ehci_setup since there
> is already it in ehci_reset.
> It was observed that the duplicated tdi_reset was causing
> the PHY_CLK_VALID bit unstable.
> 
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
>  drivers/usb/host/ehci-hcd.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 416a6dc..83b5a17 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -670,9 +670,6 @@ int ehci_setup(struct usb_hcd *hcd)
>  	if (retval)
>  		return retval;
>  
> -	if (ehci_is_TDI(ehci))
> -		tdi_reset(ehci);
> -
>  	ehci_reset(ehci);
>  
>  	return 0;

Acked-by: Alan Stern <stern@rowland.harvard.edu>

This should be applied to stable kernels going back to 3.6.

In case you are wondering why that redudant call was added, I did it
because some of the PCI drivers (Intel and TDI) already had calls to
tdi_reset.  The commit removed those calls, so the new one was added 
in.

^ 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