LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 03/21]arch:powerpc:eeh.c remove one to many l's in the word.
From: Justin P. Mattock @ 2011-02-25  6:10 UTC (permalink / raw)
  To: trivial; +Cc: paulus, linuxppc-dev, Justin P. Mattock, linux-kernel

The patch below removes an extra "l" in the word.

Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 arch/powerpc/platforms/pseries/eeh.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
index 17a11c8..3cc4d10 100644
--- a/arch/powerpc/platforms/pseries/eeh.c
+++ b/arch/powerpc/platforms/pseries/eeh.c
@@ -876,7 +876,7 @@ void eeh_restore_bars(struct pci_dn *pdn)
  *
  * Save the values of the device bars. Unlike the restore
  * routine, this routine is *not* recursive. This is because
- * PCI devices are added individuallly; but, for the restore,
+ * PCI devices are added individually; but, for the restore,
  * an entire slot is reset at a time.
  */
 static void eeh_save_bars(struct pci_dn *pdn)
-- 
1.7.4.1

^ permalink raw reply related

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Peter Zijlstra @ 2011-02-24 21:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, Jeremy Fitzhardinge, Hugh Dickins, linux-kernel
In-Reply-To: <1298582606.8833.509.camel@pasglop>

On Fri, 2011-02-25 at 08:23 +1100, Benjamin Herrenschmidt wrote:
> 
> I don't think that's needed here as there shall be no batching happening
> on the vmalloc space, but it can't hurt to merge it regardless :-)

Ah, due to the !batch->active thing? OK, then yeah Hugh's bit is
sufficient.

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Benjamin Herrenschmidt @ 2011-02-24 21:23 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linuxppc-dev, Jeremy Fitzhardinge, Hugh Dickins, linux-kernel
In-Reply-To: <1298581675.5226.840.camel@laptop>

On Thu, 2011-02-24 at 22:07 +0100, Peter Zijlstra wrote:
> 
> Lovely problem :-), benh mentioned it on IRC, but I never got around
> to
> finding the email thread, thanks for the CC.
> 
> > What would be better for 2.6.38 and 2.6.37-stable?  Moving that call
> to
> > vunmap_page_range back under vb->lock, or the partial-Peter-patch
> below?
> > And then what should be done for 2.6.39?
> 
> I think you'll also need the arch/powerpc/kernel/process.c changes
> that
> cause context switches to flush the tlb_batch queues.

I don't think that's needed here as there shall be no batching happening
on the vmalloc space, but it can't hurt to merge it regardless :-)

Cheers,
Ben.

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Benjamin Herrenschmidt @ 2011-02-24 21:12 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Jeremy Fitzhardinge, linuxppc-dev, Peter Zijlstra, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1102241216420.1708@sister.anvils>

On Thu, 2011-02-24 at 12:47 -0800, Hugh Dickins wrote:

> Reading back, I see Jeremy suggested moving vb_free()'s call to
> vunmap_page_range() back inside vb->lock: it certainly was his moving
> the call out from under that lock that brought the issue to my notice;
> but it looked as if there were other paths which would give preemptible
> PowerPC the same issue, just paths I happen not to go down myself. I'm
> not sure, I didn't take the time to follow it up properly, expecting
> further insight to arrive shortly from Ben!

Yeah, sorry, I've been too over extended lately...

> And, as threatened, Jeremy has further vmalloc changes queued up in
> mmotm, which certainly make the patch below inadequate, and I imagine
> the vunmap_page_range() movement too.  I'm currently (well, I think most
> recent mmotm doesn't even boot on my ppc) having to disable preemption
> in the kernel case of apply_to_pte_range().
> 
> What would be better for 2.6.38 and 2.6.37-stable?  Moving that call to
> vunmap_page_range back under vb->lock, or the partial-Peter-patch below?
> And then what should be done for 2.6.39?

Patch is fine. I should send it to Linus. It's not like we have a batch
on the vmalloc space anyways since it doesnt do the arch_lazy_mmu stuff,
so it's really about protecting the per-cpu variable.

Cheers,
Ben.

> --- 2.6.38-rc5/arch/powerpc/mm/tlb_hash64.c	2010-02-24 10:52:17.000000000 -0800
> +++ linux/arch/powerpc/mm/tlb_hash64.c	2011-02-15 23:27:21.000000000 -0800
> @@ -38,13 +38,11 @@ DEFINE_PER_CPU(struct ppc64_tlb_batch, p
>   * neesd to be flushed. This function will either perform the flush
>   * immediately or will batch it up if the current CPU has an active
>   * batch on it.
> - *
> - * Must be called from within some kind of spinlock/non-preempt region...
>   */
>  void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
>  		     pte_t *ptep, unsigned long pte, int huge)
>  {
> -	struct ppc64_tlb_batch *batch = &__get_cpu_var(ppc64_tlb_batch);
> +	struct ppc64_tlb_batch *batch = &get_cpu_var(ppc64_tlb_batch);
>  	unsigned long vsid, vaddr;
>  	unsigned int psize;
>  	int ssize;
> @@ -99,6 +97,7 @@ void hpte_need_flush(struct mm_struct *m
>  	 */
>  	if (!batch->active) {
>  		flush_hash_page(vaddr, rpte, psize, ssize, 0);
> +		put_cpu_var(ppc64_tlb_batch);
>  		return;
>  	}
>  
> @@ -127,6 +126,7 @@ void hpte_need_flush(struct mm_struct *m
>  	batch->index = ++i;
>  	if (i >= PPC64_TLB_BATCH_NR)
>  		__flush_tlb_pending(batch);
> +	put_cpu_var(ppc64_tlb_batch);
>  }
>  
>  /*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Peter Zijlstra @ 2011-02-24 21:07 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: Jeremy Fitzhardinge, linuxppc-dev, linux-kernel
In-Reply-To: <alpine.LSU.2.00.1102241216420.1708@sister.anvils>

On Thu, 2011-02-24 at 12:47 -0800, Hugh Dickins wrote:

Lovely problem :-), benh mentioned it on IRC, but I never got around to
finding the email thread, thanks for the CC.

> What would be better for 2.6.38 and 2.6.37-stable?  Moving that call to
> vunmap_page_range back under vb->lock, or the partial-Peter-patch below?
> And then what should be done for 2.6.39?

I think you'll also need the arch/powerpc/kernel/process.c changes that
cause context switches to flush the tlb_batch queues.

> --- 2.6.38-rc5/arch/powerpc/mm/tlb_hash64.c     2010-02-24 10:52:17.000000000 -0800
> +++ linux/arch/powerpc/mm/tlb_hash64.c  2011-02-15 23:27:21.000000000 -0800
> @@ -38,13 +38,11 @@ DEFINE_PER_CPU(struct ppc64_tlb_batch, p
>   * neesd to be flushed. This function will either perform the flush
>   * immediately or will batch it up if the current CPU has an active
>   * batch on it.
> - *
> - * Must be called from within some kind of spinlock/non-preempt region...
>   */
>  void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
>                      pte_t *ptep, unsigned long pte, int huge)
>  {
> -       struct ppc64_tlb_batch *batch = &__get_cpu_var(ppc64_tlb_batch);
> +       struct ppc64_tlb_batch *batch = &get_cpu_var(ppc64_tlb_batch);
>         unsigned long vsid, vaddr;
>         unsigned int psize;
>         int ssize;
> @@ -99,6 +97,7 @@ void hpte_need_flush(struct mm_struct *m
>          */
>         if (!batch->active) {
>                 flush_hash_page(vaddr, rpte, psize, ssize, 0);
> +               put_cpu_var(ppc64_tlb_batch);
>                 return;
>         }
>  
> @@ -127,6 +126,7 @@ void hpte_need_flush(struct mm_struct *m
>         batch->index = ++i;
>         if (i >= PPC64_TLB_BATCH_NR)
>                 __flush_tlb_pending(batch);
> +       put_cpu_var(ppc64_tlb_batch);
>  }
>  
>  /* 

^ permalink raw reply

* Re: PowerPC BUG: using smp_processor_id() in preemptible code
From: Hugh Dickins @ 2011-02-24 20:47 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Jeremy Fitzhardinge, linuxppc-dev, Peter Zijlstra, linux-kernel
In-Reply-To: <1293705910.17779.60.camel@pasglop>

On Thu, 30 Dec 2010, Benjamin Herrenschmidt wrote:
> On Wed, 2010-12-29 at 14:54 -0800, Hugh Dickins wrote:
> > With recent 2.6.37-rc, with CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y
> > on the PowerPC G5, I get spammed by BUG warnings each time I swapoff:
> > 
> > BUG: using smp_processor_id() in preemptible [00000000] code: swapoff/3974
> > caller is .hpte_need_flush+0x4c/0x2e8
> > Call Trace:
> > [c0000001b4a3f830] [c00000000000f3cc] .show_stack+0x6c/0x16c (unreliable)
> > [c0000001b4a3f8e0] [c00000000023eda0] .debug_smp_processor_id+0xe4/0x11c
> > [c0000001b4a3f970] [c00000000002f2f4] .hpte_need_flush+0x4c/0x2e8
> > [c0000001b4a3fa30] [c0000000000e7ef8] .vunmap_pud_range+0x148/0x200
> > [c0000001b4a3fb10] [c0000000000e8058] .vunmap_page_range+0xa8/0xd4
> > [c0000001b4a3fbb0] [c0000000000e80a4] .free_unmap_vmap_area+0x20/0x38
> > [c0000001b4a3fc40] [c0000000000e8138] .remove_vm_area+0x7c/0xb4
> > [c0000001b4a3fcd0] [c0000000000e8308] .__vunmap+0x50/0x104
> > [c0000001b4a3fd60] [c0000000000ef3fc] .SyS_swapoff+0x59c/0x6a8
> > [c0000001b4a3fe30] [c0000000000075a8] syscall_exit+0x0/0x40
> > 
> > I notice hpte_need_flush() itself acknowledges
> >  * Must be called from within some kind of spinlock/non-preempt region...
> 
> Yes, we assume that the PTE lock is always held when modifying page
> tables...

Right, not for the kernel page tables.  I remember when doing the
pte_offset_map_lock() stuff, that it appeared that the kernel would
already be in trouble if it did not have higher serialization for
its pte updates, so no point in using a page_table_lock for them.

> 
> > Though I didn't actually bisect, I believe this is since Jeremy's
> > 64141da587241301ce8638cc945f8b67853156ec "vmalloc: eagerly clear ptes
> > on vunmap", which moves a call to vunmap_page_range() from one place
> > (which happened to be inside a spinlock) to another (where it's not).
> > 
> > I guess my warnings would be easily silenced by moving that call to
> > vunmap_page_range() down just inside the spinlock below it; but I'm
> > dubious that that's the right fix - it looked as if there are other
> > paths through vmalloc.c where vunmap_page_range() has been getting
> > called without preemption disabled, long before Jeremy's change,
> > just paths that I never happen to go down in my limited testing.
> > 
> > For the moment I'm using the obvious patch below to keep it quiet;
> > but I doubt that this is the right patch either.  I'm hoping that
> > ye who understand the importance of hpte_need_flush() will be best
> > able to judge what to do.  Or might there be other architectures
> > expecting to be unpreemptible there?
> 
> Well, it looks like our kernel mappings tend to take some nasty
> shortcuts with the PTE locking, which I suppose are legit but do break
> some of my assumptions there. I need to have a closer look. Thanks for
> the report !

None of us have progressed this since late in 2.6.37-rc, and now
it's late in 2.6.38-rc and we're still in the same situation.

The patch I'm currently using to suppress all the noise (I suppose
I could just turn off DEBUG_PREEMPT but that would be cheating) is an
extract from Peter's preemptible mmu_gather patches, below, but I don't
really know if it's valid to extract it in this way.

Reading back, I see Jeremy suggested moving vb_free()'s call to
vunmap_page_range() back inside vb->lock: it certainly was his moving
the call out from under that lock that brought the issue to my notice;
but it looked as if there were other paths which would give preemptible
PowerPC the same issue, just paths I happen not to go down myself. I'm
not sure, I didn't take the time to follow it up properly, expecting
further insight to arrive shortly from Ben!

And, as threatened, Jeremy has further vmalloc changes queued up in
mmotm, which certainly make the patch below inadequate, and I imagine
the vunmap_page_range() movement too.  I'm currently (well, I think most
recent mmotm doesn't even boot on my ppc) having to disable preemption
in the kernel case of apply_to_pte_range().

What would be better for 2.6.38 and 2.6.37-stable?  Moving that call to
vunmap_page_range back under vb->lock, or the partial-Peter-patch below?
And then what should be done for 2.6.39?

Hugh

--- 2.6.38-rc5/arch/powerpc/mm/tlb_hash64.c	2010-02-24 10:52:17.000000000 -0800
+++ linux/arch/powerpc/mm/tlb_hash64.c	2011-02-15 23:27:21.000000000 -0800
@@ -38,13 +38,11 @@ DEFINE_PER_CPU(struct ppc64_tlb_batch, p
  * neesd to be flushed. This function will either perform the flush
  * immediately or will batch it up if the current CPU has an active
  * batch on it.
- *
- * Must be called from within some kind of spinlock/non-preempt region...
  */
 void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
 		     pte_t *ptep, unsigned long pte, int huge)
 {
-	struct ppc64_tlb_batch *batch = &__get_cpu_var(ppc64_tlb_batch);
+	struct ppc64_tlb_batch *batch = &get_cpu_var(ppc64_tlb_batch);
 	unsigned long vsid, vaddr;
 	unsigned int psize;
 	int ssize;
@@ -99,6 +97,7 @@ void hpte_need_flush(struct mm_struct *m
 	 */
 	if (!batch->active) {
 		flush_hash_page(vaddr, rpte, psize, ssize, 0);
+		put_cpu_var(ppc64_tlb_batch);
 		return;
 	}
 
@@ -127,6 +126,7 @@ void hpte_need_flush(struct mm_struct *m
 	batch->index = ++i;
 	if (i >= PPC64_TLB_BATCH_NR)
 		__flush_tlb_pending(batch);
+	put_cpu_var(ppc64_tlb_batch);
 }
 
 /*

^ permalink raw reply

* Re: Open Firmware and interrupt trigger
From: Benjamin Herrenschmidt @ 2011-02-24 20:46 UTC (permalink / raw)
  To: Robert Thorhuus; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9081694521850B4D91387C8C2E612B591062A0D5CA@ESESSCMS0361.eemea.ericsson.se>

On Wed, 2011-02-23 at 22:18 +0100, Robert Thorhuus wrote:
> Hello!
> 
> I'm quite new to linux and Open Firmware.
> 
> I have a PPC processor. To this I have a Compact Flash connected. The Compact Flash is using external interrupt 0 of the processor.
> In my DTS file I have specified a Compact Flash node and within it I have an interrupt element:
> interrupt = <0 2 0 0>;
> 
> Here I thought the first number was the ID of the interrupt and the second one should be a number indicating how the interrupt is triggered (high, low, raising, falling).
> 
> The interrupt is active low.
> 
> But I could not get it to work which ever value I chose.
> 
> Looking in the code I found this in function __devinit pata_of_platform_probe in file pata_of_platform.c:
> 
> 	ret = of_irq_to_resource(dn, 0, &irq_res);
> 	if (ret == NO_IRQ)
> 		irq_res.start = irq_res.end = 0;
> 	else
> 		irq_res.flags = 0;
> 
> Here "flags" will be zero whatever I do in the DTS. As far as I can understand the flags are defined in interrupts.h:
> #define IRQF_TRIGGER_NONE       0x00000000
> #define IRQF_TRIGGER_RISING     0x00000001
> #define IRQF_TRIGGER_FALLING    0x00000002
> #define IRQF_TRIGGER_HIGH       0x00000004
> #define IRQF_TRIGGER_LOW        0x00000008

Actually, the .dts flags depend on the specific interrupt controller you
are using. For example, MPIC uses a different mapping scheme (for
historical reasons). Check booting-without-of.txt.

> So modifying the code to:
> 	else
> 		irq_res.flags = 2;
> 
> I get it to work.
> 
> Could someone please explain to me why the "flags" parameter is hardcoded zero or just point in a good direction.

That does indeed look odd. Might be worth trying to figure out with the
git history who came up with that code in the first place and ask that
person. Without answer, I think it's valid to patch that out.

Cheers,
Ben.

> Thank you
> 
> BR
> Robert
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: mpic_alloc: Differences between of_address_to_resource() and of_get_property()+of_translate_address()
From: Benjamin Herrenschmidt @ 2011-02-24 20:43 UTC (permalink / raw)
  To: Moffett, Kyle D; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <031BE41A-C26B-419E-A515-F88C0BF4CA5D@boeing.com>

On Thu, 2011-02-24 at 11:43 -0600, Moffett, Kyle D wrote:
> Hello everyone,
> 
> I'm currently cleaning up a new P2020 (mpc85xx) board port for submission and I was noticing a lot of commonalities between the various ports.
> 
> In particular, at least 80% of the mpic_alloc() callers seem to do something like this (with more error-checking):
> 
> struct resource r;
> of_address_to_resource(np, 0, &r);
> mpic_alloc(np, r.start, [...]);
> 
> But mpic_alloc() itself seems to have some logic for digging the base address out of OpenFirmware already:
> 
> if (paddr == 0 && !(mpic->flags & MPIC_USES_DCR)) {
>       const u32 *reg = of_get_property(node, "reg", NULL);
>       BUG_ON(reg == NULL);
>       paddr = of_translate_address(node, reg);
>       BUG_ON(paddr == OF_BAD_ADDR);
> }
> 
> As far as I can tell, the code in mpic_alloc() is a very limited form of of_address_to_resource() without a
> lot of the special cases, but I can't tell what the effect of those special cases would be on the boards
> relying on mpic_alloc(np, 0, [...]);

Probably none, ie, it's probably historical and could probably be
converted.

> I'd like to just convert mpic_alloc() to do the of_address_to_resource() internally (instead of the existing logic),
> but I'd be afraid of breaking other systems.
> 
> Any comments/advice?

I think it can be tried :-) The risk should be reasonably low.

Cheers,
Ben.

> Cheers,
> Kyle Moffett
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: Flushing data cache on PPC405 in Linux
From: Dan Malek @ 2011-02-24 20:15 UTC (permalink / raw)
  To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <119fc163-ae81-4492-955a-94d1234ab6d9@VA3EHSMHS021.ehs.local>


On Feb 24, 2011, at 6:43 AM, John Linn wrote:

> It seems like this also depends on that fact that __GFP_COLD will  
> work,
> otherwise some of the data could
> already be in the cache such that you're not guaranteed to get
> everything out of the cache.

I wouldn't count on GFP_COLD as a guarantee the data isn't
in the cache.  It's likely, but the generic MM functions just
ensures they are old pages, based on an indication from the
caller that freed the pages.


	-- Dan

^ permalink raw reply

* Re: rtc on PowerMac7,3
From: kevin diggs @ 2011-02-24 18:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Linux PPC Development
In-Reply-To: <1298506192.8833.222.camel@pasglop>

Hi,

Thanks for taking some of your valuable time to reply.

Now I can't get it to fail. I don't know what I did wrong??? These
things are tryin' to push me over the edge!

Part of the problem may be the /dev/rtc (10:135 or whatever the PC
numbers are) PC device that gets put into /dev/ (udev) on YDL 6.0.
Should probably figure out who is adding it and nuke it.

Sorry for the noise!?!

kevin

On Wed, Feb 23, 2011 at 6:09 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> Not sure, I haven't looked at that RTC stuff for ages. Basically the
> platform code provides generic RTC hooks (in that G5 it's going to be
> via the via-pmu) and it should "just work". That code hasn't been
> touched for eons.
>
> Cheers,
> Ben.
>

^ permalink raw reply

* mpic_alloc: Differences between of_address_to_resource() and of_get_property()+of_translate_address()
From: Moffett, Kyle D @ 2011-02-24 17:43 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org

Hello everyone,

I'm currently cleaning up a new P2020 (mpc85xx) board port for submission a=
nd I was noticing a lot of commonalities between the various ports.

In particular, at least 80% of the mpic_alloc() callers seem to do somethin=
g like this (with more error-checking):

struct resource r;
of_address_to_resource(np, 0, &r);
mpic_alloc(np, r.start, [...]);

But mpic_alloc() itself seems to have some logic for digging the base addre=
ss out of OpenFirmware already:

if (paddr =3D=3D 0 && !(mpic->flags & MPIC_USES_DCR)) {
      const u32 *reg =3D of_get_property(node, "reg", NULL);
      BUG_ON(reg =3D=3D NULL);
      paddr =3D of_translate_address(node, reg);
      BUG_ON(paddr =3D=3D OF_BAD_ADDR);
}

As far as I can tell, the code in mpic_alloc() is a very limited form of of=
_address_to_resource() without a lot of the special cases, but I can't tell=
 what the effect of those special cases would be on the boards relying on m=
pic_alloc(np, 0, [...]);

I'd like to just convert mpic_alloc() to do the of_address_to_resource() in=
ternally (instead of the existing logic), but I'd be afraid of breaking oth=
er systems.

Any comments/advice?

Cheers,
Kyle Moffett

^ permalink raw reply

* Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Scott Wood @ 2011-02-24 17:27 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Mike Frysinger, Russell King, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linuxppc-dev, linux-kernel,
	David Miller, Paul Mackerras, linux-arm-kernel, netdev,
	John Stultz, Thomas Gleixner, Rodolfo Giometti, Christoph Lameter,
	Alan Cox, Krzysztof Halasa
In-Reply-To: <20110224165004.GB15234@riccoc20.at.omicron.at>

On Thu, 24 Feb 2011 17:50:04 +0100
Richard Cochran <richardcochran@gmail.com> wrote:

> On Wed, Feb 23, 2011 at 01:24:44PM -0600, Scott Wood wrote:
> > Whatever string is used should be written into a binding document.
> > 
> > fsl,etsec-v1.6-ptp seems like it would be just as good for that purpose.
> > 
> > Even just fsl,etsec-ptp will identify the binding, though it's lacking in
> > identifying the hardware (in the absence of access to the eTSEC ID
> > registers).
> 
> I read the conversation, and I don't mind admitting that I do not
> understand what you both are arguing/discussing about.
> 
> How should I set the strings?  Like this?
> 
> arch/powerpc/boot/dts/mpc8313erdb.dts:
> 	ptp_clock@24E00 {
> 		compatible = "fsl,mpc8313-etsec-ptp";
> 	}
> arch/powerpc/boot/dts/mpc8572ds.dts:
> 	ptp_clock@24E00 {
> 		compatible = "fsl,mpc8572-etsec-ptp";
> 	} 
> arch/powerpc/boot/dts/p2020ds.dts:
> 	ptp_clock@24E00 {
> 		compatible = "fsl,p2020ds-etsec-ptp";
> 	} 
> arch/powerpc/boot/dts/p2020rdb.dts:
> 	ptp_clock@24E00 {
> 		compatible = "fsl,p2020rdb-etsec-ptp";
> 	} 
> 
> drivers/net/gianfar_ptp.c:
> 
> static struct of_device_id match_table[] = {
> 	{ .compatible = "fsl,mpc8313-etsec-ptp" },
> 	{ .compatible = "fsl,mpc8572-etsec-ptp" },
> 	{ .compatible = "fsl,p2020ds-etsec-ptp" },
> 	{ .compatible = "fsl,p2020rdb-etsec-ptp" },
> 	{},
> };

Those last two are boards, not chips.  I don't think even Grant is asking
to take things that far.

My vote, if it goes in a separate node at all, is "fsl,etsec-ptp", and let
the driver use SVR.  Even encoding an etsec version in the compatible
string would be difficult, unless fixed up by u-boot, as it appears to
differ based on chip revision (and the chip manuals seem to often not match
the hardware regarding the advertised eTSEC revision) and we don't normally
have separate dts files for different revisions of the same chip.  Plus,
our docs (at least the public ones) don't seem to be very helpful in
determining what version of eTSEC implies what.

If you want to use chip-based compatibles instead, then use the actual name
of the chip.  You'll need to verify 100% compatibility if you want to claim
compatibility with another chip; it's probably easier/safer to just list
every single Freescale chip that has this type of PTP in a huge compatible
table, like PCI drivers do.

-Scott

^ permalink raw reply

* Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Richard Cochran @ 2011-02-24 17:26 UTC (permalink / raw)
  To: Grant Likely
  Cc: Thomas Gleixner, Rodolfo Giometti, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linux-kernel, Russell King,
	Paul Mackerras, John Stultz, Alan Cox, netdev, Mike Frysinger,
	Christoph Lameter, linuxppc-dev, David Miller, linux-arm-kernel,
	Krzysztof Halasa
In-Reply-To: <20110223165058.GE14597@angua.secretlab.ca>

On Wed, Feb 23, 2011 at 09:50:58AM -0700, Grant Likely wrote:
> On Wed, Feb 23, 2011 at 11:38:17AM +0100, Richard Cochran wrote:
> > +Clock Properties:
> > +
> > +  - tclk-period  Timer reference clock period in nanoseconds.
> > +  - tmr-prsc     Prescaler, divides the output clock.
> > +  - tmr-add      Frequency compensation value.
> > +  - cksel        0= external clock, 1= eTSEC system clock, 3= RTC clock input.
> > +                 Currently the driver only supports choice "1".
> 
> I'd be hesitant about defining something that isn't actually
> implemented yet.  You may find the binding to be insufficient at a
> later date.

Okay, I'll remove it.
We never got the external VCO working anyhow.

> > +  - tmr-fiper1   Fixed interval period pulse generator.
> > +  - tmr-fiper2   Fixed interval period pulse generator.
> > +  - max-adj      Maximum frequency adjustment in parts per billion.
> 
> These are all custom properties (not part of any shared binding) so
> they should probably be prefixed with 'fsl,'.

Okay, fine.

> > +  The calculation for tmr_fiper2 is the same as for tmr_fiper1. The
> > +  driver expects that tmr_fiper1 will be correctly set to produce a 1
> > +  Pulse Per Second (PPS) signal, since this will be offered to the PPS
> > +  subsystem to synchronize the Linux clock.
> 
> Good documentation, thanks.  Question though, how many of these values
> will the end user (or board builder) be likely to want to change.  It
> is risky encoding the calculation results into the device tree when
> they aren't the actually parameters that will be manipulated, or at
> least very user-unfriendly.

The whole thing is pretty opaque, and my explanation is (IMHO) way
better that Freescale's documentation of how the fipers work.

The board designer / system designer will want to set these carefully,
but never change them. Basically, for a given input clock, there is
only one optimal setting.

I think the device tree is the right place for that kind of setting.

The fiper1 signal should always be a 1 PPS.  We could make fiper2 run
time programmable via PHC ioctls, but I think this can wait.


> > +	etsects->irq = irq_of_parse_and_map(node, 0);
> 
> Use platform_get_irq().

Okay.

> > +	etsects->regs = of_iomap(node, 0);
> 
> Use platform_get_resource(), and don't forget to request the
> resources.

Okay, but didn't you tell me before to do this way?

   http://marc.info/?l=linux-netdev&m=127662247203659&w=4

> > +static struct of_platform_driver gianfar_ptp_driver = {
> 
> Use a platform_driver instead.  of_platform_driver is deprecated and
> being removed.

Ja, should have noticed that myself, sorry.

> > +++ b/drivers/net/gianfar_ptp_reg.h
> 
> This data is only used by gianfar_ptp.c, so there is no need for a
> separate include file.  Move the contents of gianfar_ptp_reg.h into
> gianfar_ptp.c

You are right, of course, since private #defines and declarations
should simply stay in their .c files. Some people think that all
#defines and declarations must go into a header file.

I am not one of those people, but in this case, I generated the file
from a little tool I wrote and so kept it separate.

Still, it is no trouble to combine the header into the driver .c file.

Thanks for your review,

Richard

^ permalink raw reply

* Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Scott Wood @ 2011-02-24 17:08 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Mike Frysinger, Russell King, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linuxppc-dev, linux-kernel,
	David Miller, Paul Mackerras, linux-arm-kernel, netdev,
	John Stultz, Thomas Gleixner, Rodolfo Giometti, Christoph Lameter,
	Alan Cox, Krzysztof Halasa
In-Reply-To: <20110224163944.GA15234@riccoc20.at.omicron.at>

On Thu, 24 Feb 2011 17:39:44 +0100
Richard Cochran <richardcochran@gmail.com> wrote:

> On Wed, Feb 23, 2011 at 10:54:59AM -0700, Grant Likely wrote:
> > On Wed, Feb 23, 2011 at 11:26:12AM -0600, Scott Wood wrote:
> 
> > > The eTSEC revision is probeable as well, but due the way PTP is described as
> > > a separate node, the driver doesn't have straightforward access to those
> > > registers.
> > 
> > Ignorant question: Should the ptp be described as a separate node?
> 
> Well, the PTP Hardware Clock function is logically separate from the
> MAC function.

The eTSEC node doesn't describe the MAC function, it describes the whole
device (or at least it should... we make an exception for MDIO, which
should probably have been a subnode instead).

> PHCs can be implemented in the MAC, in the PHY, or in
> between in an FPGA on MII bus.
> 
> If the PHC is in the MAC, then it might be wise to implement one
> driver that offers both the MAC and the PHC.
> 
> In the case of gianfar, it is not really necessary to combine the PHC
> into the gianfar driver, since the registers are pretty well
> separated.

How the drivers are structured in Linux is a separate concern from how the
devices are described in the device tree.  The tree is supposed to be an
OS-independent representation of hardware.

If Linux has multiple drivers that correspond to portions of one node, a
toplevel driver can register platform devices for the components, adding
in any additional information like versioning that it gets from the
toplevel registers.

-Scott

^ permalink raw reply

* Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Richard Cochran @ 2011-02-24 16:50 UTC (permalink / raw)
  To: Scott Wood
  Cc: Mike Frysinger, Russell King, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linuxppc-dev, linux-kernel,
	David Miller, Paul Mackerras, linux-arm-kernel, netdev,
	John Stultz, Thomas Gleixner, Rodolfo Giometti, Christoph Lameter,
	Alan Cox, Krzysztof Halasa
In-Reply-To: <20110223132444.65dfdda4@schlenkerla>

On Wed, Feb 23, 2011 at 01:24:44PM -0600, Scott Wood wrote:
> Whatever string is used should be written into a binding document.
> 
> fsl,etsec-v1.6-ptp seems like it would be just as good for that purpose.
> 
> Even just fsl,etsec-ptp will identify the binding, though it's lacking in
> identifying the hardware (in the absence of access to the eTSEC ID
> registers).

I read the conversation, and I don't mind admitting that I do not
understand what you both are arguing/discussing about.

How should I set the strings?  Like this?

arch/powerpc/boot/dts/mpc8313erdb.dts:
	ptp_clock@24E00 {
		compatible = "fsl,mpc8313-etsec-ptp";
	}
arch/powerpc/boot/dts/mpc8572ds.dts:
	ptp_clock@24E00 {
		compatible = "fsl,mpc8572-etsec-ptp";
	} 
arch/powerpc/boot/dts/p2020ds.dts:
	ptp_clock@24E00 {
		compatible = "fsl,p2020ds-etsec-ptp";
	} 
arch/powerpc/boot/dts/p2020rdb.dts:
	ptp_clock@24E00 {
		compatible = "fsl,p2020rdb-etsec-ptp";
	} 

drivers/net/gianfar_ptp.c:

static struct of_device_id match_table[] = {
	{ .compatible = "fsl,mpc8313-etsec-ptp" },
	{ .compatible = "fsl,mpc8572-etsec-ptp" },
	{ .compatible = "fsl,p2020ds-etsec-ptp" },
	{ .compatible = "fsl,p2020rdb-etsec-ptp" },
	{},
};

Please let me know if this is what you meant.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Richard Cochran @ 2011-02-24 16:39 UTC (permalink / raw)
  To: Grant Likely
  Cc: Mike Frysinger, Russell King, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linuxppc-dev, linux-kernel,
	David Miller, Paul Mackerras, linux-arm-kernel, netdev,
	Scott Wood, John Stultz, Thomas Gleixner, Rodolfo Giometti,
	Christoph Lameter, Alan Cox, Krzysztof Halasa
In-Reply-To: <20110223175459.GH14597@angua.secretlab.ca>

On Wed, Feb 23, 2011 at 10:54:59AM -0700, Grant Likely wrote:
> On Wed, Feb 23, 2011 at 11:26:12AM -0600, Scott Wood wrote:

> > The eTSEC revision is probeable as well, but due the way PTP is described as
> > a separate node, the driver doesn't have straightforward access to those
> > registers.
> 
> Ignorant question: Should the ptp be described as a separate node?

Well, the PTP Hardware Clock function is logically separate from the
MAC function. PHCs can be implemented in the MAC, in the PHY, or in
between in an FPGA on MII bus.

If the PHC is in the MAC, then it might be wise to implement one
driver that offers both the MAC and the PHC.

In the case of gianfar, it is not really necessary to combine the PHC
into the gianfar driver, since the registers are pretty well
separated. Also, given the size and complexity (and churn over time)
of the gianfar driver, I decided to keep the PHC separate.

Right now, the driver correctly handles all the clock revisions in the
boards that I have (mpc8313, mpc8572, p2020ds, p2020rdb).

If checking the revision becomes important, then we can always export
a function from gianfar to provide this.

Thanks,

Richard

^ permalink raw reply

* Re: [RFC PATCH 13/15] dt/serial: Eliminate users of of_platform_{, un}register_driver
From: Arnd Bergmann @ 2011-02-24 16:34 UTC (permalink / raw)
  To: devicetree-discuss; +Cc: sfr, linux-kernel, sparclinux, linuxppc-dev, davem
In-Reply-To: <20110223043442.20795.4037.stgit@localhost6.localdomain6>

On Wednesday 23 February 2011, Grant Likely wrote:
> Get rid of users of of_platform_driver in drivers/serial.  The
> of_platform_{,un}register_driver functions are going away, so the
> users need to be converted to using the platform_bus_type directly.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

of_serial changes look good.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [RFC PATCH 01/15] dt/powerpc: move of_bus_type infrastructure to ibmebus
From: Arnd Bergmann @ 2011-02-24 14:46 UTC (permalink / raw)
  To: devicetree-discuss; +Cc: sfr, linux-kernel, sparclinux, linuxppc-dev, davem
In-Reply-To: <20110223043340.20795.84024.stgit@localhost6.localdomain6>

On Wednesday 23 February 2011, Grant Likely wrote:
> arch/powerpc/kernel/ibmebus.c is the only remaining user of the
> of_bus_type support code for initializing the bus and registering
> drivers.  All others have either been switched to the vanilla platform
> bus or already have their own infrastructure.
> 
> This patch moves the functionality that ibmebus is using out of
> drivers/of/{platform,device}.c and into ibmebus.c where it is actually
> used.  Also renames the moved symbols from of_platform_* to
> ibmebus_bus_* to reflect the actual usage.
> 
> This patch is part of moving all of the of_platform_bus_type users
> over to the platform_bus_type.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

The ibmebus is essentially the platform bus of the IBM Power Systems (a.k.a.
pSeries a.k.a. System p), I think it would make a lot of sense to convert
the two drivers (ehca and ehea) on this bus into platform drivers as well.

The original reason for this bus was to provide a different IOMMU for them
than what is used on the PCI devices. This should now be possible in simpler
ways.
 
>
> +static void ibmebus_bus_device_shutdown(struct device *dev)
> +{
> +	struct platform_device *of_dev = to_platform_device(dev);
> +	struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +
> +	if (dev->driver && drv->shutdown)
> +		drv->shutdown(of_dev);
> +}

neither of the drivers provides a shutdown function.

> +#ifdef CONFIG_PM_SLEEP
> +static int ibmebus_bus_legacy_suspend(struct device *dev, pm_message_t mesg)
> +{
> +	struct platform_device *of_dev = to_platform_device(dev);
> +	struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +	int ret = 0;
> +
> +	if (dev->driver && drv->suspend)
> +		ret = drv->suspend(of_dev, mesg);
> +	return ret;
> +}
> +
> +static int ibmebus_bus_legacy_resume(struct device *dev)
> +{
> +	struct platform_device *of_dev = to_platform_device(dev);
> +	struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +	int ret = 0;
> +
> +	if (dev->driver && drv->resume)
> +		ret = drv->resume(of_dev);
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_prepare(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (drv && drv->pm && drv->pm->prepare)
> +		ret = drv->pm->prepare(dev);
> +
> +	return ret;
> +}
> +
> +static void ibmebus_bus_pm_complete(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +
> +	if (drv && drv->pm && drv->pm->complete)
> +		drv->pm->complete(dev);
> +}
> +
> +#ifdef CONFIG_SUSPEND
> +
> +static int ibmebus_bus_pm_suspend(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->suspend)
> +			ret = drv->pm->suspend(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_suspend(dev, PMSG_SUSPEND);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_suspend_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->suspend_noirq)
> +			ret = drv->pm->suspend_noirq(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_resume(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->resume)
> +			ret = drv->pm->resume(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_resume(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_resume_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->resume_noirq)
> +			ret = drv->pm->resume_noirq(dev);
> +	}
> +
> +	return ret;
> +}

These are also unused in the drivers.

> +#ifdef CONFIG_HIBERNATION
> +
> +static int ibmebus_bus_pm_freeze(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->freeze)
> +			ret = drv->pm->freeze(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_suspend(dev, PMSG_FREEZE);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_freeze_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->freeze_noirq)
> +			ret = drv->pm->freeze_noirq(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_thaw(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->thaw)
> +			ret = drv->pm->thaw(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_resume(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_thaw_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->thaw_noirq)
> +			ret = drv->pm->thaw_noirq(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_poweroff(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->poweroff)
> +			ret = drv->pm->poweroff(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_suspend(dev, PMSG_HIBERNATE);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_poweroff_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->poweroff_noirq)
> +			ret = drv->pm->poweroff_noirq(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_restore(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->restore)
> +			ret = drv->pm->restore(dev);
> +	} else {
> +		ret = ibmebus_bus_legacy_resume(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ibmebus_bus_pm_restore_noirq(struct device *dev)
> +{
> +	struct device_driver *drv = dev->driver;
> +	int ret = 0;
> +
> +	if (!drv)
> +		return 0;
> +
> +	if (drv->pm) {
> +		if (drv->pm->restore_noirq)
> +			ret = drv->pm->restore_noirq(dev);
> +	}
> +
> +	return ret;
> +}

And these, too.

	Arnd

^ permalink raw reply

* RE: Flushing data cache on PPC405 in Linux
From: John Linn @ 2011-02-24 14:43 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-dev
In-Reply-To: <B5790C8E-AC59-4588-B4C5-A636A4EC1F98@digitaldans.com>

> -----Original Message-----
> From: John Linn
> Sent: Thursday, February 24, 2011 7:15 AM
> To: 'Dan Malek'
> Cc: linuxppc-dev@ozlabs.org; grant.likely@secretlab.ca
> Subject: RE: Flushing data cache on PPC405 in Linux
> =

> > -----Original Message-----
> > From: Dan Malek [mailto:ppc6dev@digitaldans.com]
> > Sent: Wednesday, February 23, 2011 8:39 PM
> > To: John Linn
> > Cc: linuxppc-dev@ozlabs.org
> > Subject: Re: Flushing data cache on PPC405 in Linux
> >
> >
> > Hi John.
> >
> > On Feb 23, 2011, at 5:04 PM, John Linn wrote:
> >
> > > Any thoughts?
> >
> > I can come up with two methods, but before I describe them
> > ensure you consider the actual implementation of your 405 core.
> > My comments are based on the "standard" ppc405 processor,
> > but since you can configure the embedded cores to your liking,
> > the cache size may be different that the 16K bytes on the old
> > processor.
> >
> > Method one.  Just 'memset()' 16K of data that will replace all cache
> > lines and push out modified lines.  This 16K must be physically
> > contiguous, so allocate in your favorite way, with with the cache
> > coherent DMA functions, bootmem, reserved, whatever.  Just
> > ensure that this 16K is also cached, as I've implemented
> > dma_cache_coherent into uncached VM space in the past :-)
> =

> Awesome Dan, that seems reasonable to me and much easier than I was
thinking :)
> =

> I was obviously making it harder than it had to be as usual. Seems
like I can just kmalloc the memory
> then memset it.

It seems like this also depends on that fact that __GFP_COLD will work,
otherwise some of the data could =

already be in the cache such that you're not guaranteed to get
everything out of the cache.

Do we know that works on arch/powerpc, or I'm not understanding it
right?  I'm doing some digging to make sure.

Thanks,
John

> =

> Appreciate the help and suggestions.
> =

> -- John
> =

> >
> > Method two.  Use 'dcread' to read the cache tags.  If the line is
> > modified, form an EA that will match that line and 'dcbf' the line.
> > This is a little more tricky because the tags are physical
addresses,
> > so you would need to do this with the MMU disabled to ensure
> > the physical EA you generated also hits the cache.  The upside
> > is you only perform the actual required flush operations, and you
> > didn't blow the cache away requiring a refill latency.
> >
> > Have fun!
> >
> > 	-- Dan
> >


This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* RE: Flushing data cache on PPC405 in Linux
From: John Linn @ 2011-02-24 14:15 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-dev
In-Reply-To: <B5790C8E-AC59-4588-B4C5-A636A4EC1F98@digitaldans.com>

> -----Original Message-----
> From: Dan Malek [mailto:ppc6dev@digitaldans.com]
> Sent: Wednesday, February 23, 2011 8:39 PM
> To: John Linn
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: Flushing data cache on PPC405 in Linux
> =

> =

> Hi John.
> =

> On Feb 23, 2011, at 5:04 PM, John Linn wrote:
> =

> > Any thoughts?
> =

> I can come up with two methods, but before I describe them
> ensure you consider the actual implementation of your 405 core.
> My comments are based on the "standard" ppc405 processor,
> but since you can configure the embedded cores to your liking,
> the cache size may be different that the 16K bytes on the old
> processor.
> =

> Method one.  Just 'memset()' 16K of data that will replace all cache
> lines and push out modified lines.  This 16K must be physically
> contiguous, so allocate in your favorite way, with with the cache
> coherent DMA functions, bootmem, reserved, whatever.  Just
> ensure that this 16K is also cached, as I've implemented
> dma_cache_coherent into uncached VM space in the past :-)

Awesome Dan, that seems reasonable to me and much easier than I was
thinking :)

I was obviously making it harder than it had to be as usual. Seems like
I can just kmalloc the memory then memset it. =


Appreciate the help and suggestions.

-- John

> =

> Method two.  Use 'dcread' to read the cache tags.  If the line is
> modified, form an EA that will match that line and 'dcbf' the line.
> This is a little more tricky because the tags are physical addresses,
> so you would need to do this with the MMU disabled to ensure
> the physical EA you generated also hits the cache.  The upside
> is you only perform the actual required flush operations, and you
> didn't blow the cache away requiring a refill latency.
> =

> Have fun!
> =

> 	-- Dan
> =



This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* [PATCH] fsl_pci: Add support for FSL PCIe controllers v2.x
From: Prabhakar Kushwaha @ 2011-02-24  9:35 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: meet2prabhu, Prabhakar Kushwaha

FSL PCIe controller v2.1:
     - New MSI inbound window
     - Same Inbound windows address as PCIe controller v1.x

Added new pit_t member(pmit) to struct ccsr_pci for MSI inbound window

FSL PCIe controller v2.2 and v2.3:
     - Different addresses for PCIe inbound window 3,2,1
     - Exposed PCIe inbound window 0
     - New PCIe interrupt status register

Added new config and interrupt Status register to struct ccsr_pci & updated
pit_t array size to reflect the 4 inbound windows.

Device tree is used to maintain backward compatibility i.e. update inbound
window 1 index depending upon "compatible" field witin PCIE node.

Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>
Acked-by: Roy Zang <tie-fei.zang@freescale.com>
Acked-by: Kumar Gala <kumar.gala@freescale.com>
---

 Based upon git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git (branch master )

 arch/powerpc/sysdev/fsl_pci.c |   15 +++++++++++----
 arch/powerpc/sysdev/fsl_pci.h |   17 ++++++++++-------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 818f7c6..f8f7f28 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -1,7 +1,7 @@
 /*
  * MPC83xx/85xx/86xx PCI/PCIE support routing.
  *
- * Copyright 2007-2010 Freescale Semiconductor, Inc.
+ * Copyright 2007-2011 Freescale Semiconductor, Inc.
  * Copyright 2008-2009 MontaVista Software, Inc.
  *
  * Initial author: Xianghua Xiao <x.xiao@freescale.com>
@@ -99,7 +99,7 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
 				  struct resource *rsrc)
 {
 	struct ccsr_pci __iomem *pci;
-	int i, j, n, mem_log, win_idx = 2;
+	int i, j, n, mem_log, win_idx = 3, start_idx = 1, end_idx = 4;
 	u64 mem, sz, paddr_hi = 0;
 	u64 paddr_lo = ULLONG_MAX;
 	u32 pcicsrbar = 0, pcicsrbar_sz;
@@ -109,6 +109,13 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
 
 	pr_debug("PCI memory map start 0x%016llx, size 0x%016llx\n",
 		    (u64)rsrc->start, (u64)rsrc->end - (u64)rsrc->start + 1);
+
+	if (of_device_is_compatible(hose->dn, "fsl,qoriq-pcie-v2.2")) {
+		win_idx = 2;
+		start_idx = 0;
+		end_idx = 3;
+	}
+
 	pci = ioremap(rsrc->start, rsrc->end - rsrc->start + 1);
 	if (!pci) {
 	    dev_err(hose->parent, "Unable to map ATMU registers\n");
@@ -118,7 +125,7 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
 	/* Disable all windows (except powar0 since it's ignored) */
 	for(i = 1; i < 5; i++)
 		out_be32(&pci->pow[i].powar, 0);
-	for(i = 0; i < 3; i++)
+	for (i = start_idx; i < end_idx; i++)
 		out_be32(&pci->piw[i].piwar, 0);
 
 	/* Setup outbound MEM window */
@@ -204,7 +211,7 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
 			mem_log++;
 		}
 
-		piwar |= (mem_log - 1);
+		piwar |= ((mem_log - 1) & PIWAR_SZ_MASK);
 
 		/* Setup inbound memory window */
 		out_be32(&pci->piw[win_idx].pitar,  0x00000000);
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index 8ad72a1..a39ed5c 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -1,7 +1,7 @@
 /*
  * MPC85xx/86xx PCI Express structure define
  *
- * Copyright 2007 Freescale Semiconductor, Inc
+ * Copyright 2007,2011 Freescale Semiconductor, Inc
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -21,6 +21,7 @@
 #define PIWAR_TGI_LOCAL		0x00f00000	/* target - local memory */
 #define PIWAR_READ_SNOOP	0x00050000
 #define PIWAR_WRITE_SNOOP	0x00005000
+#define PIWAR_SZ_MASK          0x0000003f
 
 /* PCI/PCI Express outbound window reg */
 struct pci_outbound_window_regs {
@@ -49,7 +50,9 @@ struct ccsr_pci {
 	__be32	int_ack;		/* 0x.008 - PCI Interrupt Acknowledge Register */
 	__be32	pex_otb_cpl_tor;	/* 0x.00c - PCIE Outbound completion timeout register */
 	__be32	pex_conf_tor;		/* 0x.010 - PCIE configuration timeout register */
-	u8	res2[12];
+	__be32	pex_config;		/* 0x.014 - PCIE CONFIG Register */
+	__be32	pex_int_status;		/* 0x.018 - PCIE interrupt status */
+	u8	res2[4];
 	__be32	pex_pme_mes_dr;		/* 0x.020 - PCIE PME and message detect register */
 	__be32	pex_pme_mes_disr;	/* 0x.024 - PCIE PME and message disable register */
 	__be32	pex_pme_mes_ier;	/* 0x.028 - PCIE PME and message interrupt enable register */
@@ -62,14 +65,14 @@ struct ccsr_pci {
  * in all of the other outbound windows.
  */
 	struct pci_outbound_window_regs pow[5];
-
-	u8	res14[256];
-
-/* PCI/PCI Express inbound window 3-1
+	u8	res14[96];
+	struct pci_inbound_window_regs	pmit;	/* 0xd00 - 0xd9c Inbound MSI */
+	u8	res6[96];
+/* PCI/PCI Express inbound window 3-0
  * inbound window 1 supports only a 32-bit base address and does not
  * define an inbound window base extended address register.
  */
-	struct pci_inbound_window_regs piw[3];
+	struct pci_inbound_window_regs piw[4];
 
 	__be32	pex_err_dr;		/* 0x.e00 - PCI/PCIE error detect register */
 	u8	res21[4];
-- 
1.7.3

^ permalink raw reply related

* Re: Flushing data cache on PPC405 in Linux
From: Dan Malek @ 2011-02-24  3:38 UTC (permalink / raw)
  To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <ef11a825-8824-460f-91c0-82cb8029ed81@VA3EHSMHS029.ehs.local>


Hi John.

On Feb 23, 2011, at 5:04 PM, John Linn wrote:

> Any thoughts?

I can come up with two methods, but before I describe them
ensure you consider the actual implementation of your 405 core.
My comments are based on the "standard" ppc405 processor,
but since you can configure the embedded cores to your liking,
the cache size may be different that the 16K bytes on the old
processor.

Method one.  Just 'memset()' 16K of data that will replace all cache
lines and push out modified lines.  This 16K must be physically
contiguous, so allocate in your favorite way, with with the cache
coherent DMA functions, bootmem, reserved, whatever.  Just
ensure that this 16K is also cached, as I've implemented
dma_cache_coherent into uncached VM space in the past :-)

Method two.  Use 'dcread' to read the cache tags.  If the line is
modified, form an EA that will match that line and 'dcbf' the line.
This is a little more tricky because the tags are physical addresses,
so you would need to do this with the MMU disabled to ensure
the physical EA you generated also hits the cache.  The upside
is you only perform the actual required flush operations, and you
didn't blow the cache away requiring a refill latency.

Have fun!

	-- Dan

^ permalink raw reply

* Flushing data cache on PPC405 in Linux
From: John Linn @ 2011-02-24  1:04 UTC (permalink / raw)
  To: linuxppc-dev

I have a situation that requires a flush the data cache at specific time
periods to help with memory scrubbing.

On the 405, I don't see any easy way to do this since you don't know
what the cache has in it and there's not an instruction to flush the
whole cache.  It looks like a kernel driver is needed to map pages then
flush that data cache for the pages. =


I'm concerned there will be side affects I don't realize as I'm assuming
the driver would slowly go thru all of physical memory mapping pages
(cached or non-cached) and then flushing all addresses in the page(s).

Any thoughts?

Thanks,
John

This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* Re: rtc on PowerMac7,3
From: Benjamin Herrenschmidt @ 2011-02-24  0:09 UTC (permalink / raw)
  To: kevin diggs; +Cc: Linux PPC Development
In-Reply-To: <AANLkTinB+CLhJsiu3SBygC7TW+vnOH3e7==h_7nEGWts@mail.gmail.com>

On Wed, 2011-02-23 at 17:24 -0600, kevin diggs wrote:
> It probably does on everyone else's 7,3. The 8600 probably infected
> it. ... I hear the two of them out there late at night. Mumbling to
> each other ... plotting ...

Not sure, I haven't looked at that RTC stuff for ages. Basically the
platform code provides generic RTC hooks (in that G5 it's going to be
via the via-pmu) and it should "just work". That code hasn't been
touched for eons.

Cheers,
Ben.

^ permalink raw reply

* Re: powerpc/ptrace: Fix bug in signal handling
From: Michael Neuling @ 2011-02-24  0:02 UTC (permalink / raw)
  To: mjw, Michael Wolf; +Cc: linuxppc-dev, anton
In-Reply-To: <1298406645.9824.3.camel@w500>

Mike,

> Subject: powerpc/ptrace: Fix bug in signal handling

Maybe something more descriptive here like:
  powerpc/ptrace: remove BUG_ON when full register set not available

> In some cases during a threaded core dump not all 
> the threads will have a full register set.  This
> will cause problems when the sigkill is sent to
> the thread

Please add a description for how you are solving this.  Something like
"Solve this by putting poison values in the registers".  Similar inline
comments would be good too.

Also, I'm not convinced this is the right solution... not that I have a
have a better one to suggest :-)

<snip>

> +static inline int user_regset_copyout_poison(unsigned int *pos,
> +					   unsigned int *count,
> +					   void **kbuf, void __user **ubuf,
> +					   const int start_pos,
> +					   const int end_pos)
> +{
> +	long poison_data[17] = { [0 ... 16] = 0xdeadbeefdeadbeefUL };

Why only poison 17 registers?

Mikey

^ 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