LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Andreas Schwab @ 2006-06-02 14:41 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linuxppc-dev, Johannes Berg, alsa-devel
In-Reply-To: <s5hslmn4p00.wl%tiwai@suse.de>

Takashi Iwai <tiwai@suse.de> writes:

> At Fri, 02 Jun 2006 16:07:05 +0200,
> Johannes Berg wrote:
>> 
>> On Fri, 2006-06-02 at 15:53 +0200, Takashi Iwai wrote:
>> > > +	if (strlen(sdev->modalias)) {
>> > > +		length = snprintf (buf, 34, "%s\n", sdev->modalias);
>> > 
>> > sizeof(sdev->modalias) + 2?  But still strange if modalias has no
>> > nul-end.
>> 
>> Hm, how would I handle that if modalias has no nul?
>
> Well, I'm not sure whether modalias is always nul-terminated.

If modalias is not nul-terminated then strlen will already fail.

> Otherwise (if not nul-terminated), I'd implement like:
>
> 	strlcpy(buf, sdev->modalias, sizeof(sdev->modalias) + 1);
> 	strcpy(buf, "\n");

strcat?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [Alsa-devel] [RFC 0/8] snd-aoa: add snd-aoa
From: Takashi Iwai @ 2006-06-02 14:43 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20060601115844.343214000@sipsolutions.net>

At Thu, 01 Jun 2006 13:58:44 +0200,
Johannes Berg wrote:
> 
> The following patches would add snd-aoa to the Linux source tree. I'm
> posting them here in the spirit of getting review fairly early, it works
> fine for those machines it handles and even implements headphone/line-out
> detection now.
> 
> One thing that still needs to be done is fix this on the mac-mini where
> apparently some things are missing from the device-tree, and on the
> PowerMac7,2/7,3 where the device tree has a bug that prevents the i2sbus
> module from working properly.
> 
> Other than that, it replaces snd-powermac for all layout-id machines, and
> handles many more already, so once those two issues are fixed I'll also add
> a patch to this patchset that removes support for any machines with a
> layout-id from snd-powermac to allow snd-aoa and snd-powermac to coexist
> nicely.
> 
> Any comments are appreciated.

As already posted, there are small glitches, but it looks otherwise
good enough to merge.

Would you like to commit the patches as one changeset or separate?
Anyway, please provide a changelog and a signed-off-by line for proper
commit.


thanks,

Takashi

^ permalink raw reply

* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Johannes Berg @ 2006-06-02 14:44 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Takashi Iwai, linuxppc-dev, alsa-devel
In-Reply-To: <jeu073aad1.fsf@sykes.suse.de>

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

On Fri, 2006-06-02 at 16:41 +0200, Andreas Schwab wrote:

> If modalias is not nul-terminated then strlen will already fail.

Yeah, I'll just have to make sure elsewhere that it is.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 793 bytes --]

^ permalink raw reply

* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Takashi Iwai @ 2006-06-02 14:45 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: linuxppc-dev, Johannes Berg, alsa-devel
In-Reply-To: <jeu073aad1.fsf@sykes.suse.de>

At Fri, 02 Jun 2006 16:41:14 +0200,
Andreas Schwab wrote:
> 
> Takashi Iwai <tiwai@suse.de> writes:
> 
> > At Fri, 02 Jun 2006 16:07:05 +0200,
> > Johannes Berg wrote:
> >> 
> >> On Fri, 2006-06-02 at 15:53 +0200, Takashi Iwai wrote:
> >> > > +	if (strlen(sdev->modalias)) {
> >> > > +		length = snprintf (buf, 34, "%s\n", sdev->modalias);
> >> > 
> >> > sizeof(sdev->modalias) + 2?  But still strange if modalias has no
> >> > nul-end.
> >> 
> >> Hm, how would I handle that if modalias has no nul?
> >
> > Well, I'm not sure whether modalias is always nul-terminated.
> 
> If modalias is not nul-terminated then strlen will already fail.

Oh yes.  It should be simply
	if (*sdev->modalias) {
		...

> > Otherwise (if not nul-terminated), I'd implement like:
> >
> > 	strlcpy(buf, sdev->modalias, sizeof(sdev->modalias) + 1);
> > 	strcpy(buf, "\n");
> 
> strcat?

Argh, yes.


Takashi

^ permalink raw reply

* Re: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Kumar Gala @ 2006-06-02 14:50 UTC (permalink / raw)
  To: Zang Roy-r61911
  Cc: Alexandre.Bounine, Liu Dave-r63238, linuxppc-dev list,
	Paul Mackerras, Yang Xin-Xin-r48390
In-Reply-To: <9FCDBA58F226D911B202000BDBAD4673066CF774@zch01exm40.ap.freescale.net>


On Jun 2, 2006, at 3:20 AM, Zang Roy-r61911 wrote:

>
>> How much RAM do you have ? Maybe you are running out of
>> virtual space ?
>>
>> Ben.
>>
>>
>
> 512MBytes

If I understand things, there is a 16M region for PCI config space.   
You could look at just doing the ioremap for each chunk when its  
needed via the low level calls.

- kumar

^ permalink raw reply

* Re: [Alsa-devel] [RFC 0/8] snd-aoa: add snd-aoa
From: Johannes Berg @ 2006-06-02 14:56 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <s5hodxb4o00.wl%tiwai@suse.de>

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On Fri, 2006-06-02 at 16:43 +0200, Takashi Iwai wrote:

> As already posted, there are small glitches, but it looks otherwise
> good enough to merge.

Right, I'll post updates (also have a few small changes locally)

> Would you like to commit the patches as one changeset or separate?

I guess it doesn't make much sense to commit them in separate changesets
as they depend on each other, I just split it up for making it easier to
review.

> Anyway, please provide a changelog and a signed-off-by line for proper
> commit.

Obviously, but I didn't think that was necessary for review.

Also, what with further development? I have this in a git tree and will
continue using it, should I just push patches regularly?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 793 bytes --]

^ permalink raw reply

* [PATCH] fix make rpm for powerpc
From: Mike Wolf @ 2006-06-02 14:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev

The default target for most powerpc platforms is zImage.  The
zImage however is in arch/powerpc/boot and the mkspec script
was set up to get the kernel from the top level of the kernel 
tree.  This patch copies vmlinux to arch/powerpc/boot and then
copies the kernel to the tmp directory so the rpm can be made.

Signed-off-by: Mike Wolf <mjw@us.ibm.com>


=============

--- linux.orig/scripts/package/mkspec	2006-05-21 23:22:53.000000000 -0500
+++ linux/scripts/package/mkspec	2006-05-22 15:13:56.000000000 -0500
@@ -73,8 +73,13 @@
 echo 'cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/efi/vmlinuz-$KERNELRELEASE"
 echo 'ln -s '"efi/vmlinuz-$KERNELRELEASE" '$RPM_BUILD_ROOT'"/boot/"
 echo "%else"
+echo "%ifarch ppc64"
+echo "cp vmlinux arch/powerpc/boot"
+echo "cp arch/powerpc/boot/"'$KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
+echo "%else"
 echo 'cp $KBUILD_IMAGE $RPM_BUILD_ROOT'"/boot/vmlinuz-$KERNELRELEASE"
 echo "%endif"
+echo "%endif"
 
 echo 'cp System.map $RPM_BUILD_ROOT'"/boot/System.map-$KERNELRELEASE"
 

^ permalink raw reply

* 8xx spi completion sometime doesn't generate an interrupt
From: Antonio Di Bacco @ 2006-06-02 15:08 UTC (permalink / raw)
  To: linuxppc-embedded

As far I understood the spi bus device of mpc8xx works independently from the 
attached device (in my case a dataflash m25pe80). Thus I don't understand why 
sometime when I start an SPI transfer I don't receive an interrupt for the 
completion of this operation. I wait 50 ms for this interrupt and sometime it 
doesn't happen. Anyone had a similar problem?

Bye,
Antonio.

^ permalink raw reply

* [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Adam Litke @ 2006-06-02 14:08 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: linux-mm, linux-kernel, David Gibson

[PATCH] powerpc: Close hugetlb regions when unmapping VMAs

On powerpc, each segment can contain pages of only one size.  When a
hugetlb mapping is requested, a segment is located and marked for use
with huge pages.  This is a uni-directional operation -- hugetlb
segments are never marked for use again with normal pages.  For long
running processes which make use of a combination of normal and hugetlb
mappings, this behavior can unduly constrain the virtual address space.

The following patch introduces a architecture-specific vm_ops.close()
hook.  For all architectures besides powerpc, this is a no-op.  On
powerpc, the low and high segments are scanned to locate empty hugetlb
segments which can be made available for normal mappings.  Comments?

Signed-off-by: Adam Litke <agl@us.ibm.com>
---
 arch/powerpc/mm/hugetlbpage.c |   39 ++++++++++++++++++++++++++++++++++++++-
 include/asm-powerpc/pgtable.h |    1 +
 include/linux/hugetlb.h       |    6 ++++++
 mm/hugetlb.c                  |    1 +
 4 files changed, 46 insertions(+), 1 deletion(-)
diff -upN reference/arch/powerpc/mm/hugetlbpage.c current/arch/powerpc/mm/hugetlbpage.c
--- reference/arch/powerpc/mm/hugetlbpage.c
+++ current/arch/powerpc/mm/hugetlbpage.c
@@ -297,7 +297,6 @@ void hugetlb_free_pgd_range(struct mmu_g
 	start = addr;
 	pgd = pgd_offset((*tlb)->mm, addr);
 	do {
-		BUG_ON(! in_hugepage_area((*tlb)->mm->context, addr));
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd))
 			continue;
@@ -494,6 +493,44 @@ static int open_high_hpage_areas(struct 
 	return 0;
 }
 
+/*
+ * Called when tearing down a hugetlb vma.  See if we can free up any
+ * htlb areas so normal pages can be mapped there again.
+ */
+void arch_hugetlb_close_vma(struct vm_area_struct *vma)
+{
+	struct mm_struct *mm = vma->vm_mm;
+	unsigned long i;
+	struct slb_flush_info fi;
+	u16 inuse, hiflush, loflush;
+
+	if (!mm)
+		return;
+
+	inuse = mm->context.low_htlb_areas;
+	for (i = 0; i < NUM_LOW_AREAS; i++)
+		if (prepare_low_area_for_htlb(mm, i) == 0)
+			inuse &= ~(1 << i);
+	loflush = inuse ^ mm->context.low_htlb_areas;
+	mm->context.low_htlb_areas = inuse;
+
+	inuse = mm->context.high_htlb_areas;
+	for (i = 0; i < NUM_HIGH_AREAS; i++)
+		if (prepare_high_area_for_htlb(mm, i) == 0)
+			inuse &= ~(1 << i);
+	hiflush = inuse ^ mm->context.high_htlb_areas;
+	mm->context.high_htlb_areas = inuse;
+
+	/* the context changes must make it to memory before the flush,
+	 * so that further SLB misses do the right thing. */
+	mb();
+	fi.mm = mm;
+	if ((fi.newareas = loflush))
+		on_each_cpu(flush_low_segments, &fi, 0, 1);
+	if ((fi.newareas = hiflush))
+		on_each_cpu(flush_high_segments, &fi, 0, 1);
+}
+
 int prepare_hugepage_range(unsigned long addr, unsigned long len)
 {
 	int err = 0;
diff -upN reference/include/asm-powerpc/pgtable.h current/include/asm-powerpc/pgtable.h
--- reference/include/asm-powerpc/pgtable.h
+++ current/include/asm-powerpc/pgtable.h
@@ -155,6 +155,7 @@ extern unsigned long empty_zero_page[PAG
 
 #define HAVE_ARCH_UNMAPPED_AREA
 #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
+#define ARCH_HAS_HUGETLB_CLOSE_VMA
 
 #endif
 
diff -upN reference/include/linux/hugetlb.h current/include/linux/hugetlb.h
--- reference/include/linux/hugetlb.h
+++ current/include/linux/hugetlb.h
@@ -85,6 +85,12 @@ pte_t huge_ptep_get_and_clear(struct mm_
 void hugetlb_prefault_arch_hook(struct mm_struct *mm);
 #endif
 
+#ifndef ARCH_HAS_HUGETLB_CLOSE_VMA
+#define arch_hugetlb_close_vma(x)	0
+#else
+void arch_hugetlb_close_vma(struct vm_area_struct *vma);
+#endif
+
 #else /* !CONFIG_HUGETLB_PAGE */
 
 static inline int is_vm_hugetlb_page(struct vm_area_struct *vma)
diff -upN reference/mm/hugetlb.c current/mm/hugetlb.c
--- reference/mm/hugetlb.c
+++ current/mm/hugetlb.c
@@ -399,6 +399,7 @@ static struct page *hugetlb_nopage(struc
 
 struct vm_operations_struct hugetlb_vm_ops = {
 	.nopage = hugetlb_nopage,
+	.close = arch_hugetlb_close_vma,
 };
 
 static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Dave Hansen @ 2006-06-02 15:17 UTC (permalink / raw)
  To: Adam Litke; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <1149257287.9693.6.camel@localhost.localdomain>

On Fri, 2006-06-02 at 09:08 -0500, Adam Litke wrote:
>  #define HAVE_ARCH_UNMAPPED_AREA
>  #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
> +#define ARCH_HAS_HUGETLB_CLOSE_VMA
>  
>  #endif
>  
> diff -upN reference/include/linux/hugetlb.h
> current/include/linux/hugetlb.h
> --- reference/include/linux/hugetlb.h
> +++ current/include/linux/hugetlb.h
> @@ -85,6 +85,12 @@ pte_t huge_ptep_get_and_clear(struct mm_
>  void hugetlb_prefault_arch_hook(struct mm_struct *mm);
>  #endif
>  
> +#ifndef ARCH_HAS_HUGETLB_CLOSE_VMA
> +#define arch_hugetlb_close_vma(x)      0
> +#else
> +void arch_hugetlb_close_vma(struct vm_area_struct *vma);
> +#endif

Please don't do this ARCH_HAS stuff.  Use Kconfig at the very least.
You could also have an arch-specific htlb vma init function that could
be used for other things in the future. 

> @@ -297,7 +297,6 @@ void hugetlb_free_pgd_range(struct mmu_g
>         start = addr;
>         pgd = pgd_offset((*tlb)->mm, addr);
>         do {
> -               BUG_ON(! in_hugepage_area((*tlb)->mm->context, addr));
>                 next = pgd_addr_end(addr, end);
>                 if (pgd_none_or_clear_bad(pgd))
>                         continue;

Why does this BUG() go away?

> +/*
> + * Called when tearing down a hugetlb vma.  See if we can free up any
> + * htlb areas so normal pages can be mapped there again.
> + */
> +void arch_hugetlb_close_vma(struct vm_area_struct *vma)
> +{
> +       struct mm_struct *mm = vma->vm_mm;
> +       unsigned long i;
> +       struct slb_flush_info fi;
> +       u16 inuse, hiflush, loflush;
> +
> +       if (!mm)
> +               return;

Why is this check necessary?  Do kernel threads use vmas? ;)

> +       inuse = mm->context.low_htlb_areas;
> +       for (i = 0; i < NUM_LOW_AREAS; i++)
> +               if (prepare_low_area_for_htlb(mm, i) == 0)
> +                       inuse &= ~(1 << i);

Why check _all_ the areas?  Shouldn't the check just be for the current
VMA's area?  Also, prepare_low_area_for_htlb() is a pretty silly
function name, especially for its use here.  Especially because you are
tearing down a htlb area.  low_area_contains_vma() is a bit more apt.  

My first thought about what this function is that it should probably be
asking the question, "is the VMA that I'm closing right now that last
one in this segment?"

> +       loflush = inuse ^ mm->context.low_htlb_areas;
> +       mm->context.low_htlb_areas = inuse;

This bit fiddling should really be done in some helper functions.  It
isn't immediately and completely obvious what this is doing.  

> +       inuse = mm->context.high_htlb_areas;

Are you re-using "inuse"?  How about a different variable name for a
different use?

> +       for (i = 0; i < NUM_HIGH_AREAS; i++)
> +               if (prepare_high_area_for_htlb(mm, i) == 0)
> +                       inuse &= ~(1 << i);
> +       hiflush = inuse ^ mm->context.high_htlb_areas;
> +       mm->context.high_htlb_areas = inuse;

This, combined with the other loop, completely rebuild the mm->context's
view into htlb state, right?  Isn't that a bit excessive?

> +       /* the context changes must make it to memory before the flush,
> +        * so that further SLB misses do the right thing. */
> +       mb();
> +       fi.mm = mm;
> +       if ((fi.newareas = loflush))
> +               on_each_cpu(flush_low_segments, &fi, 0, 1);
> +       if ((fi.newareas = hiflush))
> +               on_each_cpu(flush_high_segments, &fi, 0, 1);
> +}

Yikes!  Think about a pathological program here.  It mmap()s 1 htlb
area, then unmaps it quickly, over and over.  What will that perform
like here?  

-- Dave

^ permalink raw reply

* Re: [Alsa-devel] [RFC 0/8] snd-aoa: add snd-aoa
From: Takashi Iwai @ 2006-06-02 15:23 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <1149260165.11092.32.camel@johannes.berg>

At Fri, 02 Jun 2006 16:56:05 +0200,
Johannes Berg wrote:
> 
> On Fri, 2006-06-02 at 16:43 +0200, Takashi Iwai wrote:
> 
> > As already posted, there are small glitches, but it looks otherwise
> > good enough to merge.
> 
> Right, I'll post updates (also have a few small changes locally)
> 
> > Would you like to commit the patches as one changeset or separate?
> 
> I guess it doesn't make much sense to commit them in separate changesets
> as they depend on each other, I just split it up for making it easier to
> review.

OK.

> > Anyway, please provide a changelog and a signed-off-by line for proper
> > commit.
> 
> Obviously, but I didn't think that was necessary for review.
> 
> Also, what with further development? I have this in a git tree and will
> continue using it, should I just push patches regularly?

Please post patches to alsa-devel ML.  It's easier for us to review.


Takashi

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Hugh Dickins @ 2006-06-02 16:43 UTC (permalink / raw)
  To: Adam Litke; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <1149257287.9693.6.camel@localhost.localdomain>

On Fri, 2 Jun 2006, Adam Litke wrote:
> 
> On powerpc, each segment can contain pages of only one size.  When a
> hugetlb mapping is requested, a segment is located and marked for use
> with huge pages.  This is a uni-directional operation -- hugetlb
> segments are never marked for use again with normal pages.  For long
> running processes which make use of a combination of normal and hugetlb
> mappings, this behavior can unduly constrain the virtual address space.
> 
> The following patch introduces a architecture-specific vm_ops.close()
> hook.  For all architectures besides powerpc, this is a no-op.  On
> powerpc, the low and high segments are scanned to locate empty hugetlb
> segments which can be made available for normal mappings.  Comments?

Wouldn't hugetlb_free_pgd_range be a better place to do that kind of
thing, all within arch/powerpc, no need for arch_hugetlb_close_vma etc?

Hugh

^ permalink raw reply

* RE: MPC85xx PCI transfer disconnect
From: Martin, Tim @ 2006-06-02 16:45 UTC (permalink / raw)
  To: Liu Dave-r63238, linuxppc-embedded

> The 85xx cacheline is 32 bytes.

Yes, so it is. =20

> Did you enable Memory Read Multiple command of your PCI master?

Thanks for the clue.  I'm trying to figure out how to do this.  The PCI
master is a Tundra TSI148 VME-PCI bridge.  The documentation says it
supports the PCI read multiple cycle, but so far I haven't found a
register to specifically configure it.  I'll check the C/BE lines to see
if a PCI read multiple cycle is being issued.

I have confirmed that 64 bytes are being transferred before the MPC85xx
disconnects the transfer.  If the PCI master initiates a 32 byte or 64
byte read, the MPC85xx does not disconnect the transfer.

Tim

> -----Original Message-----
> From: Liu Dave-r63238 [mailto:DaveLiu@freescale.com]
> Sent: Thursday, June 01, 2006 8:52 PM
> To: Martin, Tim; linuxppc-embedded@ozlabs.org
> Subject: RE: MPC85xx PCI transfer disconnect
>=20
>=20
> > I know this is probably a question for Freescale directly,
> > but I thought I would post here to see if anyone else has
> > seen similar issues with the MPC85xx PCI under Linux.
> >
> > I'm using a Freescale MPC85xx (8541) processor and seeing an
> > extreme slowness to the PCI bus.
> >
> > I'm attemping to do 2048 byte PCI burst reads from an
> > external PCI master.  So an external chip is the PCI master
> > ("initiator") and the MPC85xx is the PCI slave ("target")
> >
> > When initiating the PCI burst read, there is a 270 ns delay
> > from the MPC85xx claiming the transaction (DEVSEL# going low)
> > to being ready (TRDY# going low).  Then 64 bytes (a single
> > cacheline) are transferred, and TRDY# goes inactive.  Next,
> > STOP# goes low, terminating the transfer.  I believe this is
> > considered a PCI DISCONNECT type 1 since IRDY# is active the
> > entire time.
> >
>=20
> The 85xx cacheline is 32 bytes.
> Did you enable Memory Read Multiple command of your PCI master?
>=20
> > What I would expect is that the entire 2048 bytes are
> > transferred in one PCI bus mastership, but instead there are
> > several re-arbitrations and long delays in between several 64
> > byte transfers.
> >
> > Additional info:
> > The PCI bus is 64bit running at 66 MHz.
> > I'm running a modified Linux 2.6.9 kernel (elinos-111).
> > The PCI CCSR piwar1 =3D 0xa0f4401d.
> >
> > Freescale has an FAQ 21021 that describes having a PCI
> > DISCONNECT 2, which sounds similar to my problem, but I've
> > already confirmed I'm doing what the FAQ suggests (enable
> > prefetching in the piwar register)
> > (http://www.freescale.com/webapp/sps/utils/SingleFaq.jsp?FAQ-2
> 1021.xml)
>=20
> Any thoughts/suggestions would be appreciated,
> Tim
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-
> embedded

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Adam Litke @ 2006-06-02 16:47 UTC (permalink / raw)
  To: Dave Hansen; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <1149261452.16665.86.camel@localhost.localdomain>

On Fri, 2006-06-02 at 08:17 -0700, Dave Hansen wrote:
> On Fri, 2006-06-02 at 09:08 -0500, Adam Litke wrote:
> >  #define HAVE_ARCH_UNMAPPED_AREA
> >  #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
> > +#define ARCH_HAS_HUGETLB_CLOSE_VMA
> >  
> >  #endif
> >  
> > diff -upN reference/include/linux/hugetlb.h
> > current/include/linux/hugetlb.h
> > --- reference/include/linux/hugetlb.h
> > +++ current/include/linux/hugetlb.h
> > @@ -85,6 +85,12 @@ pte_t huge_ptep_get_and_clear(struct mm_
> >  void hugetlb_prefault_arch_hook(struct mm_struct *mm);
> >  #endif
> >  
> > +#ifndef ARCH_HAS_HUGETLB_CLOSE_VMA
> > +#define arch_hugetlb_close_vma(x)      0
> > +#else
> > +void arch_hugetlb_close_vma(struct vm_area_struct *vma);
> > +#endif
> 
> Please don't do this ARCH_HAS stuff.  Use Kconfig at the very least.
> You could also have an arch-specific htlb vma init function that could
> be used for other things in the future. 

That's how the rest of the hugetlb arch hooks are implemented.

> > @@ -297,7 +297,6 @@ void hugetlb_free_pgd_range(struct mmu_g
> >         start = addr;
> >         pgd = pgd_offset((*tlb)->mm, addr);
> >         do {
> > -               BUG_ON(! in_hugepage_area((*tlb)->mm->context, addr));
> >                 next = pgd_addr_end(addr, end);
> >                 if (pgd_none_or_clear_bad(pgd))
> >                         continue;
> 
> Why does this BUG() go away?

Since the area is 'closed' to huge pages before the page tables are torn
down, it is no longer a bug to have huge ptes in a non-hugetlb region.

> > +/*
> > + * Called when tearing down a hugetlb vma.  See if we can free up any
> > + * htlb areas so normal pages can be mapped there again.
> > + */
> > +void arch_hugetlb_close_vma(struct vm_area_struct *vma)
> > +{
> > +       struct mm_struct *mm = vma->vm_mm;
> > +       unsigned long i;
> > +       struct slb_flush_info fi;
> > +       u16 inuse, hiflush, loflush;
> > +
> > +       if (!mm)
> > +               return;
> 
> Why is this check necessary?  Do kernel threads use vmas? ;)

Paranoia got the best of me here.  I have a habit of checking for null
before dereferencing pointers.  But as you suggest, it should be safe to
remove.

> > +       inuse = mm->context.low_htlb_areas;
> > +       for (i = 0; i < NUM_LOW_AREAS; i++)
> > +               if (prepare_low_area_for_htlb(mm, i) == 0)
> > +                       inuse &= ~(1 << i);
> 
> Why check _all_ the areas?  Shouldn't the check just be for the current
> VMA's area?  Also, prepare_low_area_for_htlb() is a pretty silly
> function name, especially for its use here.  Especially because you are
> tearing down a htlb area.  low_area_contains_vma() is a bit more apt.

Checking all the areas does make the code simpler (if a fair bit less
efficient).  I suppose I could only check htlb-enabled areas as a simple
optimization.  But checking only those regions affected by this vma
might not be that bad.

Yes I agree about the function names.  Originally I was planning to
rename these in a different patch, but I suppose those changes can be
folded into this already small patch.

> My first thought about what this function is that it should probably be
> asking the question, "is the VMA that I'm closing right now that last
> one in this segment?"
> 
> > +       loflush = inuse ^ mm->context.low_htlb_areas;
> > +       mm->context.low_htlb_areas = inuse;
> 
> This bit fiddling should really be done in some helper functions.  It
> isn't immediately and completely obvious what this is doing.  
> 
> > +       inuse = mm->context.high_htlb_areas;
> 
> Are you re-using "inuse"?  How about a different variable name for a
> different use?
> 
> > +       for (i = 0; i < NUM_HIGH_AREAS; i++)
> > +               if (prepare_high_area_for_htlb(mm, i) == 0)
> > +                       inuse &= ~(1 << i);
> > +       hiflush = inuse ^ mm->context.high_htlb_areas;
> > +       mm->context.high_htlb_areas = inuse;
> 
> This, combined with the other loop, completely rebuild the mm->context's
> view into htlb state, right?  Isn't that a bit excessive?

Ok.  These bit flipping operations might benefit from some abstraction
to share more code with the 'open' cases.  Point conceded.

> > +       /* the context changes must make it to memory before the flush,
> > +        * so that further SLB misses do the right thing. */
> > +       mb();
> > +       fi.mm = mm;
> > +       if ((fi.newareas = loflush))
> > +               on_each_cpu(flush_low_segments, &fi, 0, 1);
> > +       if ((fi.newareas = hiflush))
> > +               on_each_cpu(flush_high_segments, &fi, 0, 1);
> > +}
> 
> Yikes!  Think about a pathological program here.  It mmap()s 1 htlb
> area, then unmaps it quickly, over and over.  What will that perform
> like here?  

Well, it will only flush segments on cpus currently executing on the
same mm.  So said pathological program would only be slowing itself down
(with the exception of the interrupt overhead).

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Adam Litke @ 2006-06-02 16:49 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <Pine.LNX.4.64.0606021737310.26864@blonde.wat.veritas.com>

On Fri, 2006-06-02 at 17:43 +0100, Hugh Dickins wrote:
> On Fri, 2 Jun 2006, Adam Litke wrote:
> > 
> > On powerpc, each segment can contain pages of only one size.  When a
> > hugetlb mapping is requested, a segment is located and marked for use
> > with huge pages.  This is a uni-directional operation -- hugetlb
> > segments are never marked for use again with normal pages.  For long
> > running processes which make use of a combination of normal and hugetlb
> > mappings, this behavior can unduly constrain the virtual address space.
> > 
> > The following patch introduces a architecture-specific vm_ops.close()
> > hook.  For all architectures besides powerpc, this is a no-op.  On
> > powerpc, the low and high segments are scanned to locate empty hugetlb
> > segments which can be made available for normal mappings.  Comments?
> 
> Wouldn't hugetlb_free_pgd_range be a better place to do that kind of
> thing, all within arch/powerpc, no need for arch_hugetlb_close_vma etc?

Hmm.  Interesting idea.  I'll take a look.

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

^ permalink raw reply

* RE: MPC85xx PCI transfer disconnect
From: Martin, Tim @ 2006-06-02 17:33 UTC (permalink / raw)
  To: Liu Dave-r63238, linuxppc-embedded

More information...

> > Did you enable Memory Read Multiple command of your PCI master?
>=20
> Thanks for the clue.  I'm trying to figure out how to do this.  The
PCI
> master is a Tundra TSI148 VME-PCI bridge.  The documentation says it
> supports the PCI read multiple cycle, but so far I haven't found a
> register to specifically configure it.  I'll check the C/BE lines to
see
> if a PCI read multiple cycle is being issued.
>=20

After grepping through the Tundra documentation more closely, I found
this tidbit:

"Memory read multiple: The memory read multiple command is used when the
requested byte count is greater than 32 bytes."


> I have confirmed that 64 bytes are being transferred before the
MPC85xx
> disconnects the transfer.  If the PCI master initiates a 32 byte or 64
> byte read, the MPC85xx does not disconnect the transfer.
>=20

After scaning more logic analyzer captures, I noticed that occasionally
the MPC85xx inserts additional wait states (by deasserting TRDY#) after
only 32 bytes have been transferred.  So it definitely appears that (the
way I have things configured now) there's a 20-80ns delay after 1 cache
line is read, and the MPC85xx disconnects transfers that are larger than
2 cache lines.

Tim

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Christoph Lameter @ 2006-06-02 20:06 UTC (permalink / raw)
  To: Adam Litke; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <1149257287.9693.6.camel@localhost.localdomain>

On Fri, 2 Jun 2006, Adam Litke wrote:

> The following patch introduces a architecture-specific vm_ops.close()
> hook.  For all architectures besides powerpc, this is a no-op.  On
> powerpc, the low and high segments are scanned to locate empty hugetlb
> segments which can be made available for normal mappings.  Comments?

IA64 has similar issues and uses the hook suggested by Hugh. However, we 
have a permanently reserved memory area. I am a bit surprised about the 
need to make address space available for normal mappings. Is this for 32 
bit powerpc support?

void hugetlb_free_pgd_range(struct mmu_gather **tlb,
                        unsigned long addr, unsigned long end,
                        unsigned long floor, unsigned long ceiling)
{
        /*
         * This is called to free hugetlb page tables.
         *
         * The offset of these addresses from the base of the hugetlb
         * region must be scaled down by HPAGE_SIZE/PAGE_SIZE so that
         * the standard free_pgd_range will free the right page tables.
         *
         * If floor and ceiling are also in the hugetlb region, they
         * must likewise be scaled down; but if outside, left unchanged.
         */

        addr = htlbpage_to_page(addr);
        end  = htlbpage_to_page(end);
        if (REGION_NUMBER(floor) == RGN_HPAGE)
                floor = htlbpage_to_page(floor);
        if (REGION_NUMBER(ceiling) == RGN_HPAGE)
                ceiling = htlbpage_to_page(ceiling);

        free_pgd_range(tlb, addr, end, floor, ceiling);
}

^ permalink raw reply

* [PATCH 2.6.16.16] sata_sil24: SII3124 sata driver endian problem
From: Rune Torgersen @ 2006-06-02 20:19 UTC (permalink / raw)
  To: jgarzik; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <DCEAAC0833DD314AB0B58112AD99B93B0189DDFF@ismail.innsys.innovsys.com>

[-- Attachment #1: Type: text/plain, Size: 2350 bytes --]

> -----Original Message-----
> From: Rune Torgersen
> Sent: Thursday, June 01, 2006 16:10
> To: linuxppc-dev@ozlabs.org
> Subject: SII3124-2
> 
> Has anybody been successful in getting a SII3124-2 based SATA 
> controller
> to work under PPC?
> 
> I have a eval board that I tried on two different freescale boards (a
> MPC8266ADS board and a MPC8560ADS board).
> Kernel 2.6.16.16.
> 
> Here is the relevant output from the kernel.
> 
> ata1: SATA max UDMA/100 cmd 0xD1010000 ctl 0x0 bmdma 0x0 irq 115
> ata2: SATA max UDMA/100 cmd 0xD1012000 ctl 0x0 bmdma 0x0 irq 115
> ata3: SATA max UDMA/100 cmd 0xD1014000 ctl 0x0 bmdma 0x0 irq 115
> ata4: SATA max UDMA/100 cmd 0xD1016000 ctl 0x0 bmdma 0x0 irq 115
> ata1: SATA link down (SStatus 0)
> scsi0 : sata_sil24
> ata2: SATA link up 3.0 Gbps (SStatus 123)
> sata_sil24 ata2: SRST failed, disabling port
> scsi1 : sata_sil24
> ata3: SATA link down (SStatus 0)
> scsi2 : sata_sil24
> ata4: SATA link down (SStatus 0)
> scsi3 : sata_sil24
> 
> I added debug output to see the content of the Command Error register.
> It is set to 26 which according to the datasheet for the 3124-1 (I am
> running a -2), is PLDCMDERRORMASTERABORT, "A PCI Master Abort occurred
> while the SiI3124 was fetching a Port Request Block (PRB) from host
> memory."

There is an endian issue in the sil24 driver. 
The follwing pathc seems to fix it for me. (it is also attached in case
the mailer borks it for me)

Signed-off-by: Rune Torgersen <runet@innovsys.com>

Index: linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c
===================================================================
--- linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c	(revision 101)
+++ linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c	(working copy)
@@ -446,7 +446,7 @@
 	 */
 	msleep(10);
 
-	prb->ctrl = PRB_CTRL_SRST;
+	prb->ctrl = cpu_to_le16(PRB_CTRL_SRST);
 	prb->fis[1] = 0; /* no PM yet */
 
 	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
@@ -537,9 +537,9 @@
 
 		if (qc->tf.protocol != ATA_PROT_ATAPI_NODATA) {
 			if (qc->tf.flags & ATA_TFLAG_WRITE)
-				prb->ctrl = PRB_CTRL_PACKET_WRITE;
+				prb->ctrl =
cpu_to_le16(PRB_CTRL_PACKET_WRITE);
 			else
-				prb->ctrl = PRB_CTRL_PACKET_READ;
+				prb->ctrl =
cpu_to_le16(PRB_CTRL_PACKET_READ);
 		} else
 			prb->ctrl = 0;
 

[-- Attachment #2: sil24_endian_patch --]
[-- Type: application/octet-stream, Size: 795 bytes --]

Index: linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c
===================================================================
--- linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c	(revision 101)
+++ linux-innsys-2.6.16.16/drivers/scsi/sata_sil24.c	(working copy)
@@ -446,7 +446,7 @@
 	 */
 	msleep(10);
 
-	prb->ctrl = PRB_CTRL_SRST;
+	prb->ctrl = cpu_to_le16(PRB_CTRL_SRST);
 	prb->fis[1] = 0; /* no PM yet */
 
 	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
@@ -537,9 +537,9 @@
 
 		if (qc->tf.protocol != ATA_PROT_ATAPI_NODATA) {
 			if (qc->tf.flags & ATA_TFLAG_WRITE)
-				prb->ctrl = PRB_CTRL_PACKET_WRITE;
+				prb->ctrl = cpu_to_le16(PRB_CTRL_PACKET_WRITE);
 			else
-				prb->ctrl = PRB_CTRL_PACKET_READ;
+				prb->ctrl = cpu_to_le16(PRB_CTRL_PACKET_READ);
 		} else
 			prb->ctrl = 0;
 

^ permalink raw reply

* [PATCH] reorg RTAS delay code
From: John Rose @ 2006-06-02 20:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: External List, Paul Mackerras
In-Reply-To: <1149200718.15158.0.camel@sinatra.austin.ibm.com>

This patch attempts to handle RTAS "busy" return codes in a more simple
and consistent manner.  Typical callers of RTAS shouldn't have to
manage wait times and delay calls.

This patch also changes the kernel to use msleep() rather than udelay()
when a runtime delay is necessary.  This will avoid CPU soft lockups
for extended delay conditions.

Signed-off-by: John Rose <johnrose@austin.ibm.com>

---

This obsoletes the patch in the parent thread, "use msleep for RTAS 
delays".  Sniff tested on POWER5.

Thanks-
John

 2_6_linus-johnrose/arch/powerpc/kernel/rtas-rtc.c   |   30 +++---
 2_6_linus-johnrose/arch/powerpc/kernel/rtas.c       |   86 ++++++++------------
 2_6_linus-johnrose/arch/powerpc/kernel/rtas_flash.c |   25 -----
 2_6_linus-johnrose/include/asm-powerpc/rtas.h       |    8 -
 4 files changed, 57 insertions(+), 92 deletions(-)

diff -puN arch/powerpc/kernel/rtas.c~rtas_delay_reorg arch/powerpc/kernel/rtas.c
--- 2_6_linus/arch/powerpc/kernel/rtas.c~rtas_delay_reorg	2006-06-02 15:09:43.000000000 -0500
+++ 2_6_linus-johnrose/arch/powerpc/kernel/rtas.c	2006-06-02 15:22:37.000000000 -0500
@@ -370,24 +370,35 @@ int rtas_call(int token, int nargs, int 
 	return ret;
 }
 
-/* Given an RTAS status code of 990n compute the hinted delay of 10^n
- * (last digit) milliseconds.  For now we bound at n=5 (100 sec).
+/* For RTAS_BUSY (-2), delay for 1 millisecond.  For an extended busy status
+ * code of 990n, perform the hinted delay of 10^n (last digit) milliseconds.
  */
-unsigned int rtas_extended_busy_delay_time(int status)
+unsigned int rtas_busy_delay_time(int status)
 {
-	int order = status - 9900;
-	unsigned long ms;
+	int order;
+	unsigned int ms = 0;
 
-	if (order < 0)
-		order = 0;	/* RTC depends on this for -2 clock busy */
-	else if (order > 5)
-		order = 5;	/* bound */
+	if (status == RTAS_BUSY) {
+		ms = 1;
+	} else if (status >= 9900 && status <= 9905) {
+		order = status - 9900;
+		for (ms = 1; order > 0; order--)
+			ms *= 10;
+	}
+
+	return ms;
+}
 
-	/* Use microseconds for reasonable accuracy */
-	for (ms = 1; order > 0; order--)
-		ms *= 10;
+/* For an RTAS busy status code, perform the hinted delay. */
+unsigned int rtas_busy_delay(int status)
+{
+	unsigned int ms;
 
-	return ms; 
+	ms = rtas_busy_delay_time(status);
+	if (ms)
+		msleep(ms);
+
+	return ms;
 }
 
 int rtas_error_rc(int rtas_rc)
@@ -438,22 +449,14 @@ int rtas_get_power_level(int powerdomain
 int rtas_set_power_level(int powerdomain, int level, int *setlevel)
 {
 	int token = rtas_token("set-power-level");
-	unsigned int wait_time;
 	int rc;
 
 	if (token == RTAS_UNKNOWN_SERVICE)
 		return -ENOENT;
 
-	while (1) {
+	do
 		rc = rtas_call(token, 2, 2, setlevel, powerdomain, level);
-		if (rc == RTAS_BUSY)
-			udelay(1);
-		else if (rtas_is_extended_busy(rc)) {
-			wait_time = rtas_extended_busy_delay_time(rc);
-			udelay(wait_time * 1000);
-		} else
-			break;
-	}
+	while (rtas_busy_delay(rc));
 
 	if (rc < 0)
 		return rtas_error_rc(rc);
@@ -463,22 +466,14 @@ int rtas_set_power_level(int powerdomain
 int rtas_get_sensor(int sensor, int index, int *state)
 {
 	int token = rtas_token("get-sensor-state");
-	unsigned int wait_time;
 	int rc;
 
 	if (token == RTAS_UNKNOWN_SERVICE)
 		return -ENOENT;
 
-	while (1) {
+	do
 		rc = rtas_call(token, 2, 2, state, sensor, index);
-		if (rc == RTAS_BUSY)
-			udelay(1);
-		else if (rtas_is_extended_busy(rc)) {
-			wait_time = rtas_extended_busy_delay_time(rc);
-			udelay(wait_time * 1000);
-		} else
-			break;
-	}
+	while (rtas_busy_delay(rc));
 
 	if (rc < 0)
 		return rtas_error_rc(rc);
@@ -488,23 +483,14 @@ int rtas_get_sensor(int sensor, int inde
 int rtas_set_indicator(int indicator, int index, int new_value)
 {
 	int token = rtas_token("set-indicator");
-	unsigned int wait_time;
 	int rc;
 
 	if (token == RTAS_UNKNOWN_SERVICE)
 		return -ENOENT;
 
-	while (1) {
+	do
 		rc = rtas_call(token, 3, 1, NULL, indicator, index, new_value);
-		if (rc == RTAS_BUSY)
-			udelay(1);
-		else if (rtas_is_extended_busy(rc)) {
-			wait_time = rtas_extended_busy_delay_time(rc);
-			udelay(wait_time * 1000);
-		}
-		else
-			break;
-	}
+	while (rtas_busy_delay(rc));
 
 	if (rc < 0)
 		return rtas_error_rc(rc);
@@ -552,16 +538,14 @@ void rtas_os_term(char *str)
 
 	snprintf(rtas_os_term_buf, 2048, "OS panic: %s", str);
 
-	do {
+	do
 		status = rtas_call(rtas_token("ibm,os-term"), 1, 1, NULL,
 				   __pa(rtas_os_term_buf));
+	while (rtas_busy_delay(status));
 
-		if (status == RTAS_BUSY)
-			udelay(1);
-		else if (status != 0)
-			printk(KERN_EMERG "ibm,os-term call failed %d\n",
+	if (status != 0)
+		printk(KERN_EMERG "ibm,os-term call failed %d\n",
 			       status);
-	} while (status == RTAS_BUSY);
 }
 
 static int ibm_suspend_me_token = RTAS_UNKNOWN_SERVICE;
@@ -789,7 +773,7 @@ EXPORT_SYMBOL(rtas_token);
 EXPORT_SYMBOL(rtas_call);
 EXPORT_SYMBOL(rtas_data_buf);
 EXPORT_SYMBOL(rtas_data_buf_lock);
-EXPORT_SYMBOL(rtas_extended_busy_delay_time);
+EXPORT_SYMBOL(rtas_busy_delay_time);
 EXPORT_SYMBOL(rtas_get_sensor);
 EXPORT_SYMBOL(rtas_get_power_level);
 EXPORT_SYMBOL(rtas_set_power_level);
diff -puN arch/powerpc/kernel/rtas-rtc.c~rtas_delay_reorg arch/powerpc/kernel/rtas-rtc.c
--- 2_6_linus/arch/powerpc/kernel/rtas-rtc.c~rtas_delay_reorg	2006-06-02 15:09:43.000000000 -0500
+++ 2_6_linus-johnrose/arch/powerpc/kernel/rtas-rtc.c	2006-06-02 15:09:43.000000000 -0500
@@ -14,19 +14,20 @@
 unsigned long __init rtas_get_boot_time(void)
 {
 	int ret[8];
-	int error, wait_time;
+	int error;
+	unsigned int wait_time;
 	u64 max_wait_tb;
 
 	max_wait_tb = get_tb() + tb_ticks_per_usec * 1000 * MAX_RTC_WAIT;
 	do {
 		error = rtas_call(rtas_token("get-time-of-day"), 0, 8, ret);
-		if (error == RTAS_CLOCK_BUSY || rtas_is_extended_busy(error)) {
-			wait_time = rtas_extended_busy_delay_time(error);
+
+		wait_time = rtas_busy_delay_time(error);
+		if (wait_time) {
 			/* This is boot time so we spin. */
 			udelay(wait_time*1000);
-			error = RTAS_CLOCK_BUSY;
 		}
-	} while (error == RTAS_CLOCK_BUSY && (get_tb() < max_wait_tb));
+	} while (wait_time && (get_tb() < max_wait_tb));
 
 	if (error != 0 && printk_ratelimit()) {
 		printk(KERN_WARNING "error: reading the clock failed (%d)\n",
@@ -44,24 +45,25 @@ unsigned long __init rtas_get_boot_time(
 void rtas_get_rtc_time(struct rtc_time *rtc_tm)
 {
         int ret[8];
-	int error, wait_time;
+	int error;
+	unsigned int wait_time;
 	u64 max_wait_tb;
 
 	max_wait_tb = get_tb() + tb_ticks_per_usec * 1000 * MAX_RTC_WAIT;
 	do {
 		error = rtas_call(rtas_token("get-time-of-day"), 0, 8, ret);
-		if (error == RTAS_CLOCK_BUSY || rtas_is_extended_busy(error)) {
+
+		wait_time = rtas_busy_delay_time(error);
+		if (wait_time) {
 			if (in_interrupt() && printk_ratelimit()) {
 				memset(rtc_tm, 0, sizeof(struct rtc_time));
 				printk(KERN_WARNING "error: reading clock"
 				       " would delay interrupt\n");
 				return;	/* delay not allowed */
 			}
-			wait_time = rtas_extended_busy_delay_time(error);
 			msleep(wait_time);
-			error = RTAS_CLOCK_BUSY;
 		}
-	} while (error == RTAS_CLOCK_BUSY && (get_tb() < max_wait_tb));
+	} while (wait_time && (get_tb() < max_wait_tb));
 
         if (error != 0 && printk_ratelimit()) {
                 printk(KERN_WARNING "error: reading the clock failed (%d)\n",
@@ -88,14 +90,14 @@ int rtas_set_rtc_time(struct rtc_time *t
 				  tm->tm_year + 1900, tm->tm_mon + 1,
 				  tm->tm_mday, tm->tm_hour, tm->tm_min,
 				  tm->tm_sec, 0);
-		if (error == RTAS_CLOCK_BUSY || rtas_is_extended_busy(error)) {
+
+		wait_time = rtas_busy_delay_time(error);
+		if (wait_time) {
 			if (in_interrupt())
 				return 1;	/* probably decrementer */
-			wait_time = rtas_extended_busy_delay_time(error);
 			msleep(wait_time);
-			error = RTAS_CLOCK_BUSY;
 		}
-	} while (error == RTAS_CLOCK_BUSY && (get_tb() < max_wait_tb));
+	} while (wait_time && (get_tb() < max_wait_tb));
 
         if (error != 0 && printk_ratelimit())
                 printk(KERN_WARNING "error: setting the clock failed (%d)\n",
diff -puN include/asm-powerpc/rtas.h~rtas_delay_reorg include/asm-powerpc/rtas.h
--- 2_6_linus/include/asm-powerpc/rtas.h~rtas_delay_reorg	2006-06-02 15:09:43.000000000 -0500
+++ 2_6_linus-johnrose/include/asm-powerpc/rtas.h	2006-06-02 15:09:43.000000000 -0500
@@ -177,12 +177,8 @@ extern unsigned long rtas_get_boot_time(
 extern void rtas_get_rtc_time(struct rtc_time *rtc_time);
 extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
 
-/* Given an RTAS status code of 9900..9905 compute the hinted delay */
-unsigned int rtas_extended_busy_delay_time(int status);
-static inline int rtas_is_extended_busy(int status)
-{
-	return status >= 9900 && status <= 9909;
-}
+extern unsigned int rtas_busy_delay_time(int status);
+extern unsigned int rtas_busy_delay(int status);
 
 extern void pSeries_log_error(char *buf, unsigned int err_type, int fatal);
 
diff -puN arch/powerpc/kernel/rtas_flash.c~rtas_delay_reorg arch/powerpc/kernel/rtas_flash.c
--- 2_6_linus/arch/powerpc/kernel/rtas_flash.c~rtas_delay_reorg	2006-06-02 15:09:43.000000000 -0500
+++ 2_6_linus-johnrose/arch/powerpc/kernel/rtas_flash.c	2006-06-02 15:09:43.000000000 -0500
@@ -365,20 +365,12 @@ static int rtas_excl_release(struct inod
 
 static void manage_flash(struct rtas_manage_flash_t *args_buf)
 {
-	unsigned int wait_time;
 	s32 rc;
 
-	while (1) {
+	do
 		rc = rtas_call(rtas_token("ibm,manage-flash-image"), 1, 
 			       1, NULL, args_buf->op);
-		if (rc == RTAS_RC_BUSY)
-			udelay(1);
-		else if (rtas_is_extended_busy(rc)) {
-			wait_time = rtas_extended_busy_delay_time(rc);
-			udelay(wait_time * 1000);
-		} else
-			break;
-	}
+	while (rtas_busy_delay(rc));
 
 	args_buf->status = rc;
 }
@@ -451,27 +443,18 @@ static ssize_t manage_flash_write(struct
 static void validate_flash(struct rtas_validate_flash_t *args_buf)
 {
 	int token = rtas_token("ibm,validate-flash-image");
-	unsigned int wait_time;
 	int update_results;
 	s32 rc;	
 
 	rc = 0;
-	while(1) {
+	do {
 		spin_lock(&rtas_data_buf_lock);
 		memcpy(rtas_data_buf, args_buf->buf, VALIDATE_BUF_SIZE);
 		rc = rtas_call(token, 2, 2, &update_results, 
 			       (u32) __pa(rtas_data_buf), args_buf->buf_size);
 		memcpy(args_buf->buf, rtas_data_buf, VALIDATE_BUF_SIZE);
 		spin_unlock(&rtas_data_buf_lock);
-			
-		if (rc == RTAS_RC_BUSY)
-			udelay(1);
-		else if (rtas_is_extended_busy(rc)) {
-			wait_time = rtas_extended_busy_delay_time(rc);
-			udelay(wait_time * 1000);
-		} else
-			break;
-	}
+	} while (rtas_busy_delay(rc));
 
 	args_buf->status = rc;
 	args_buf->update_results = update_results;

_

^ permalink raw reply

* Re: 8xx spi completion sometime doesn't generate an interrupt
From: Wolfgang Denk @ 2006-06-02 20:45 UTC (permalink / raw)
  To: Antonio Di Bacco; +Cc: linuxppc-embedded
In-Reply-To: <200606021708.48814.antonio.dibacco@aruba.it>

In message <200606021708.48814.antonio.dibacco@aruba.it> you wrote:
> As far I understood the spi bus device of mpc8xx works independently from the 
> attached device (in my case a dataflash m25pe80). Thus I don't understand why 
> sometime when I start an SPI transfer I don't receive an interrupt for the 
> completion of this operation. I wait 50 ms for this interrupt and sometime it 
> doesn't happen. Anyone had a similar problem?

Yes, this is known problem, especially if you are runnign the SPI bus
with higher data transfer rates and/or high CPM load. Check  the  FSL
knowledge base; it contains pretty clear statements about what SPI is
*not*  designed  for  - especially, it was not designed for any high-
bandwidth data transfers.

See for example  FAQ-8992:

        The physical clocking speed of the SPI can be up to 12 MHz.
        However, it only has a 16-bit holding register. Thus, the 12
        Mbit/sec rate can only be sustained for 16 bits. If you need
        to transmit more than 2-bytes of data at that clocking rate,
        you must put the data into separate BDs and set the data
        length to 2 and set the L bit in each BD. If you are using a
        character length of 16-bits, the maximum clocking rate is 3.1
        Mbit/sec. If you are using a character length of 8 bits, the
        maximum is 500 Kbits/sec. Note that 500 Kbits/sec is the
        maximum throughput when no other peripherals (SCCs, SMCs) are
        being used. Load on those peripherals will further reduce the
        maximum data rate through the SPI. See the question in the
        SCC area related to maximum data rate calculations.

 
Ok, this was for a 25 MHz (?) 68360 so you get somewhat better  rates
with an 8xx at higher CPU/CPM clocks - but the problem is essentially
still present.

See also FAQ-10566. And especially FAQ-10335, which comes to a point:

        The SPI data rate is based upon the load of the CPM. The SPI
        was not designed to be a high-bandwidth channel. It can run
	    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        very quickly for bursts of up to 16-bits. But the peripheral
        has no FIFO and low priority in the MPC860 and thus you
        cannot burst lots of data quickly through the interface.


Been there before, and yes, we've been bitten, too. No way to fix.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
In a business, marketroids, salespukes, and  lawyers  have  different
goals from those who actually do work and produce something. Usually,
is  is the former who triumph over the latter, due to the simple rule
that those who print the money make the rules.
         -- Tom Christiansen in <5jdcls$b04$2@csnews.cs.colorado.edu>

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Adam Litke @ 2006-06-02 20:57 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <Pine.LNX.4.64.0606021301300.5492@schroedinger.engr.sgi.com>

On Fri, 2006-06-02 at 13:06 -0700, Christoph Lameter wrote:
> On Fri, 2 Jun 2006, Adam Litke wrote:
> 
> > The following patch introduces a architecture-specific vm_ops.close()
> > hook.  For all architectures besides powerpc, this is a no-op.  On
> > powerpc, the low and high segments are scanned to locate empty hugetlb
> > segments which can be made available for normal mappings.  Comments?
> 
> IA64 has similar issues and uses the hook suggested by Hugh. However, we 
> have a permanently reserved memory area. I am a bit surprised about the 
> need to make address space available for normal mappings. Is this for 32 
> bit powerpc support?

I now have a working implementation using Hugh's suggestion and
incorporating some suggestions from David Hansen... (attaching for
reference).

The real reason I want to "close" hugetlb regions (even on 64bit
platforms) is so a process can replace a previous hugetlb mapping with
normal pages when huge pages become scarce.  An example would be the
hugetlb morecore (malloc) feature in libhugetlbfs :)

[PATCH] powerpc: Close hugetlb regions when unmapping VMAs

On powerpc, each segment can contain pages of only one size.  When a hugetlb
mapping is requested, a segment is located and marked for use with huge pages.
This is a uni-directional operation -- hugetlb segments are never marked for
use again with normal pages.  For long running processes which make use of a
combination of normal and hugetlb mappings, this behavior can unduly constrain
the virtual address space.

Changes since V1:
 * Modifications limited to arch-specific code (hugetlb_free_pgd_range)
 * Only scan segments covered by the range to be unmapped

Signed-off-by: Adam Litke <agl@us.ibm.com>
---
 hugetlbpage.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 49 insertions(+)
diff -upN reference/arch/powerpc/mm/hugetlbpage.c current/arch/powerpc/mm/hugetlbpage.c
--- reference/arch/powerpc/mm/hugetlbpage.c
+++ current/arch/powerpc/mm/hugetlbpage.c
@@ -52,6 +52,7 @@
 typedef struct { unsigned long pd; } hugepd_t;
 
 #define hugepd_none(hpd)	((hpd).pd == 0)
+void close_hugetlb_areas(struct mm_struct *mm);
 
 static inline pte_t *hugepd_page(hugepd_t hpd)
 {
@@ -303,6 +304,8 @@ void hugetlb_free_pgd_range(struct mmu_g
 			continue;
 		hugetlb_free_pud_range(*tlb, pgd, addr, next, floor, ceiling);
 	} while (pgd++, addr = next, addr != end);
+
+	close_hugetlb_areas((*tlb)->mm);
 }
 
 void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
@@ -518,6 +521,52 @@ int prepare_hugepage_range(unsigned long
 	return 0;
 }
 
+void close_hugetlb_areas(struct mm_struct *mm, unsigned long start,
+		unsigned long end)
+{
+	unsigned long i;
+	struct slb_flush_info fi;
+	u16 inuse, hiflush, loflush, mask;
+
+	if (!mm)
+		return;
+
+	if (start < 0x100000000UL) {
+		mask = LOW_ESID_MASK(start, end - start);
+		inuse = mm->context.low_htlb_areas;
+		for (i = 0; i < NUM_LOW_AREAS; i++) {
+			if (!(mask & (1 << i)))
+				continue;
+			if (prepare_low_area_for_htlb(mm, i) == 0)
+				inuse &= ~(1 << i);
+		}
+		loflush = inuse ^ mm->context.low_htlb_areas;
+		mm->context.low_htlb_areas = inuse;
+	}
+
+	if (end > 0x100000000UL) {
+		mask = HTLB_AREA_MASK(start, end - start);
+		inuse = mm->context.high_htlb_areas;
+		for (i = 0; i < NUM_HIGH_AREAS; i++) {
+			if (!(mask & (1 << i)))
+				continue;
+			if (prepare_high_area_for_htlb(mm, i) == 0)
+				inuse &= ~(1 << i);
+		}
+		hiflush = inuse ^ mm->context.high_htlb_areas;
+		mm->context.high_htlb_areas = inuse;
+	}
+
+	/* the context changes must make it to memory before the flush,
+	 * so that further SLB misses do the right thing. */
+	mb();
+	fi.mm = mm;
+	if ((fi.newareas = loflush))
+		on_each_cpu(flush_low_segments, &fi, 0, 1);
+	if ((fi.newareas = hiflush))
+		on_each_cpu(flush_high_segments, &fi, 0, 1);
+}
+
 struct page *
 follow_huge_addr(struct mm_struct *mm, unsigned long address, int write)
 {

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH] hugetlb: powerpc: Actively close unused htlb regions on vma close
From: Christoph Lameter @ 2006-06-02 21:08 UTC (permalink / raw)
  To: Adam Litke; +Cc: linuxppc-dev, David Gibson, linux-kernel, linux-mm
In-Reply-To: <1149281841.9693.39.camel@localhost.localdomain>

On Fri, 2 Jun 2006, Adam Litke wrote:

> The real reason I want to "close" hugetlb regions (even on 64bit
> platforms) is so a process can replace a previous hugetlb mapping with
> normal pages when huge pages become scarce.  An example would be the
> hugetlb morecore (malloc) feature in libhugetlbfs :)

Well that approach wont work on IA64 it seems.

^ permalink raw reply

* RE: [PATCH/2.6.17-rc4 4/10]Powerpc:  Add tsi108 pic support
From: Alexandre Bounine @ 2006-06-02 21:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev list, Paul Mackerras, Yang Xin-Xin-r48390

The items 1 and 2 not needed. I moved them from the tsi108_pic.c but =
really they have been leftovers from the old code there. Originally code =
ppc/syslib/open_pic.c was used as a template, which was good fit for the =
blocking output mode of the tsi108 PIC (plus some extra tweaks). In that =
mode EOI has to be signaled to PIC to deactivate interrupt line to the =
CPU before it re-enables local interrupts. This is why we have got ack =
routine. We changed mode later and removed most of workarounds except =
these two.

I removed code for 1 and 2 and will send a patch to Roy after retesting =
(probably this weekend
with some other stuff).

Alex.
             =20

-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
Sent: Thursday, June 01, 2006 6:06 PM
To: Alexandre Bounine
Cc: Kumar Gala; Zang Roy-r61911; linuxppc-dev list; Yang Xin-Xin-r48390;
Paul Mackerras
Subject: RE: [PATCH/2.6.17-rc4 4/10]Powerpc: Add tsi108 pic support


On Thu, 2006-06-01 at 16:45 -0400, Alexandre Bounine wrote:
> All differences in the Tsi108/109 PIC code from the standard MPIC are =
caused by the HW behavior. The Tsi108/109 PIC looks like standard =
OpenPIC but, in fact, is different in registers mapping and behavior. =
Its logic is close but not exactly as MPIC. =20
>=20
> Here are replies on comments to the code:
>=20
> 1.Why do you have to check if its a LEVEL irq?
>=20
> Check for LEVEL irqs is required in the ack/end pair to enable nested =
interrupt servicing and
> does not hang when core (local) interrupts are re-enabled in the ISR. =
Otherwise we have to use=20
> SA_INTERRUPT flag for all level signaled interrupts.

Can you be more precise about what exactly happens and why ? Unless you
EOI handling is broken of course, there should be no need to do anything
other than a single eoi in end(), period.

> 2. if the PIC works like other openpic's you dont need an 'ack' we =20
> handle it via 'end'
>=20
> Tsi108/109 needs it.

What for ? Please, give the low level details.

> 3. why the changes to where we do mpic_eoi for TSI108?
> The Tsi108 PIC requires EOI for spurious interrupt (as all other =
interrupt sources).

Ok, that is acceptable.

> The do_IRQ() does not call end routine for spurious interrupts. =20
>=20
> The MPIC code patch for Tsi108/109 demonstrates HW differences and why =
we originally considered having separate code for Tsi108 pic.

Please tell me more about the HW differences :)

Ben.
=20
>=20
>=20
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, May 30, 2006 3:18 PM
> To: Zang Roy-r61911
> Cc: Benjamin Herrenschmidt; linuxppc-dev list; Yang Xin-Xin-r48390; =
Paul
> Mackerras; Alexandre Bounine
> Subject: Re: [PATCH/2.6.17-rc4 4/10]Powerpc: Add tsi108 pic support
>=20
>=20
>=20
> On May 29, 2006, at 10:28 PM, Zang Roy-r61911 wrote:
>=20
> >>
> >> On Wed, 2006-05-17 at 18:14 +0800, Zang Roy-r61911 wrote:
> >>> Add Tundra Semiconductor tsi108 host bridge interrupt
> >> controller support.
> >>
> >> It looks a bit like an hacked up MPIC... Is it different
> >> enough to justify a separate driver ? Or would it be possible
> >> to define a TSI108 flag to pass the current mpic driver and
> >> add the necessary bits to it ?
> >>
> >
> > Tsi108 implementation of MPIC has many differences form the =20
> > original one,  the following
> > code implements it with mpic. Any comment? The following patch is =20
> > just based on
> > my previous send out patches.
>=20
> In the future please provide it against the base, much easier to read.
>=20
> > Integrate Tundra Semiconductor tsi108 host bridge interrupt =
controller
> > to mpic arch.
> >
> > Signed-off-by: Alexandre Bounine <alexandreb@tundra.com>
> >
>=20
> [snip]
>=20
> >
> > --- linux-2.6.17-rc4-tun/arch/powerpc/sysdev/mpic.c	2006-03-20 =20
> > 00:53:29.000000000 -0500
> > +++ linux-2.6.17-rc4-hpc2/arch/powerpc/sysdev/mpic.c	2006-05-29 =20
> > 16:07:03.000000000 -0400
> > @@ -81,7 +81,7 @@ static inline void _mpic_write(unsigned
> >  static inline u32 _mpic_ipi_read(struct mpic *mpic, unsigned int =
ipi)
> >  {
> >  	unsigned int be =3D (mpic->flags & MPIC_BIG_ENDIAN) !=3D 0;
> > -	unsigned int offset =3D MPIC_GREG_IPI_VECTOR_PRI_0 + (ipi * 0x10);
> > +	unsigned int offset =3D MPIC_GREG_IPI_VECTOR_PRI_0 + (ipi * =20
> > MPIC_GREG_IPI_STRIDE);
> >
> >  	if (mpic->flags & MPIC_BROKEN_IPI)
> >  		be =3D !be;
> > @@ -90,7 +90,7 @@ static inline u32 _mpic_ipi_read(struct
> >
> >  static inline void _mpic_ipi_write(struct mpic *mpic, unsigned int  =

> > ipi, u32 value)
> >  {
> > -	unsigned int offset =3D MPIC_GREG_IPI_VECTOR_PRI_0 + (ipi * 0x10);
> > +	unsigned int offset =3D MPIC_GREG_IPI_VECTOR_PRI_0 + (ipi * =20
> > MPIC_GREG_IPI_STRIDE);
> >
> >  	_mpic_write(mpic->flags & MPIC_BIG_ENDIAN, mpic->gregs, offset, =20
> > value);
> >  }
> > @@ -393,7 +393,11 @@ static inline struct mpic * mpic_from_ir
> >  static inline void mpic_eoi(struct mpic *mpic)
> >  {
> >  	mpic_cpu_write(MPIC_CPU_EOI, 0);
> > +#ifndef CONFIG_TSI108_BRIDGE
> >  	(void)mpic_cpu_read(MPIC_CPU_WHOAMI);
> > +#else
> > +	(void)mpic_cpu_read(MPIC_CPU_OUTPUT);
> > +#endif
> >  }
> >
> >  #ifdef CONFIG_SMP
> > @@ -514,9 +518,26 @@ static void mpic_end_irq(unsigned int ir
> >  	}
> >  #endif /* CONFIG_MPIC_BROKEN_U3 */
> >
> > +#ifdef CONFIG_TSI108_BRIDGE
> > +	if ((irq_desc[irq].status & IRQ_LEVEL) !=3D 0)
> > +#endif
>=20
> Why do you have to check if its a LEVEL irq?
>=20
> >  	mpic_eoi(mpic);
> >  }
> >
> > +#ifdef CONFIG_TSI108_BRIDGE
> > +static void mpic_ack_irq(unsigned int irq)
> > +{
> > +	struct mpic *mpic =3D mpic_from_irq(irq);
> > +
> > +#ifdef DEBUG_IRQ
> > +	DBG("%s: ack_irq: %d\n", mpic->name, irq);
> > +#endif
> > +
> > +	if ((irq_desc[irq].status & IRQ_LEVEL) =3D=3D 0)
> > +		mpic_eoi(mpic);
> > +}
> > +#endif /* CONFIG_TSI108_BRIDGE */
> > +
>=20
> if the PIC works like other openpic's you dont need an 'ack' we =20
> handle it via 'end'
>=20
> >  #ifdef CONFIG_SMP
> >
> >  static void mpic_enable_ipi(unsigned int irq)
> > @@ -596,6 +617,9 @@ struct mpic * __init mpic_alloc(unsigned
> >  	mpic->hc_irq.enable =3D mpic_enable_irq;
> >  	mpic->hc_irq.disable =3D mpic_disable_irq;
> >  	mpic->hc_irq.end =3D mpic_end_irq;
> > +#ifdef CONFIG_TSI108_BRIDGE
> > +	mpic->hc_irq.ack =3D mpic_ack_irq;
> > +#endif
> >  	if (flags & MPIC_PRIMARY)
> >  		mpic->hc_irq.set_affinity =3D mpic_set_affinity;
> >  #ifdef CONFIG_SMP
> > @@ -955,8 +979,13 @@ void mpic_send_ipi(unsigned int ipi_no,
> >  	DBG("%s: send_ipi(ipi_no: %d)\n", mpic->name, ipi_no);
> >  #endif
> >
> > +#ifndef CONFIG_TSI108_BRIDGE
> >  	mpic_cpu_write(MPIC_CPU_IPI_DISPATCH_0 + ipi_no * 0x10,
> >  		       mpic_physmask(cpu_mask & cpus_addr(cpu_online_map)[0]));
> > +#else /* CONFIG_TSI108_BRIDGE */
> > +	mpic_write(mpic->gregs, MPIC_CPU_IPI_DISPATCH,
> > +		       mpic_physmask(cpu_mask & cpus_addr(cpu_online_map)[0]));
> > +#endif /* !CONFIG_TSI108_BRIDGE */
> >  }
> >
> >  int mpic_get_one_irq(struct mpic *mpic, struct pt_regs *regs)
> > @@ -972,11 +1001,20 @@ int mpic_get_one_irq(struct mpic *mpic,
> >  		DBG("%s: cascading ...\n", mpic->name);
> >  #endif
> >  		irq =3D mpic->cascade(regs, mpic->cascade_data);
> > +#ifdef DEBUG_LOW
> > +		DBG("%s: cascaded irq: %d\n", mpic->name, irq);
> > +#endif
> > +#ifndef CONFIG_TSI108_BRIDGE
> >  		mpic_eoi(mpic);
> > +#endif
> >  		return irq;
> >  	}
> > -	if (unlikely(irq =3D=3D MPIC_VEC_SPURRIOUS))
> > +	if (unlikely(irq =3D=3D MPIC_VEC_SPURRIOUS)) {
> > +#ifdef CONFIG_TSI108_BRIDGE
> > +		mpic_eoi(mpic);
> > +#endif
> >  		return -1;
> > +	}
>=20
> why the changes to where we do mpic_eoi for TSI108?
>=20
> >  	if (irq < MPIC_VEC_IPI_0) {
> >  #ifdef DEBUG_IRQ
> >  		DBG("%s: irq %d\n", mpic->name, irq + mpic->irq_offset);
>=20
>=20
>=20
> [snip]

^ permalink raw reply

* Re: [PATCH] reorg RTAS delay code
From: Nathan Lynch @ 2006-06-02 21:33 UTC (permalink / raw)
  To: John Rose; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <1149280229.18052.3.camel@sinatra.austin.ibm.com>

> +/* For an RTAS busy status code, perform the hinted delay. */
> +unsigned int rtas_busy_delay(int status)
> +{
> +	unsigned int ms;
>  
> -	return ms; 
> +	ms = rtas_busy_delay_time(status);
> +	if (ms)
> +		msleep(ms);
> +
> +	return ms;
>  }

Can you put a might_sleep() at the beginning of this function so that
we can reliably catch unsafe uses of it?  Otherwise we'll get warnings
only when a delay is actually executed.


> @@ -438,22 +449,14 @@ int rtas_get_power_level(int powerdomain
>  int rtas_set_power_level(int powerdomain, int level, int *setlevel)
>  {
>  	int token = rtas_token("set-power-level");
> -	unsigned int wait_time;
>  	int rc;
>  
>  	if (token == RTAS_UNKNOWN_SERVICE)
>  		return -ENOENT;
>  
> -	while (1) {
> +	do
>  		rc = rtas_call(token, 2, 2, setlevel, powerdomain, level);
> -		if (rc == RTAS_BUSY)
> -			udelay(1);
> -		else if (rtas_is_extended_busy(rc)) {
> -			wait_time = rtas_extended_busy_delay_time(rc);
> -			udelay(wait_time * 1000);
> -		} else
> -			break;
> -	}
> +	while (rtas_busy_delay(rc));

Coding style nit -- am I alone in thinking that do/while without the
brackets looks weird?  Given the single-statement body of the loop, I
guess the brackets aren't necessary, but omitting the brackets doesn't
save lines in this case.

^ permalink raw reply

* Re: 8xx spi completion sometime doesn't generate an interrupt
From: Antonio Di Bacco @ 2006-06-02 22:21 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: linuxppc-embedded
In-Reply-To: <20060602204540.BE50D353A57@atlas.denx.de>

> Yes, this is known problem, especially if you are runnign the SPI bus
> with higher data transfer rates and/or high CPM load ....

I lowered the SPI bus frequency, the interrupt is now always happening but I 
have always problems.
I did further investigations, I'm doing spi transfers of 256 bytes both 
writing and reading (256 is the size of a page of the chip). I write one time 
and then read two times the complete 8Mbit chip. The two reading give me 
always the same result, then, the writing is failing in my case. Probably I 
have to double check the writing procedure. But it is also worth to note that 
changing the PM (prescaler module) and lowering the SPI clock even more the 
writing problem is happening less frequently. 

Thank you,
Bye,
Antonio.

^ 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