On Wed, Feb 5, 2014 at 1:38 PM, Michael Kenney <mfkenney@xxxxxxxxx> wrote: > On Wed, Feb 5, 2014 at 1:22 PM, Martyn Welch <martyn@xxxxxxxxxxxx> wrote: >> >> >> >> On 5 February 2014 17:41, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >>> >>> On Tue, Feb 04, 2014 at 06:34:13PM +0000, Martyn Welch wrote: >>> > On 04/02/14 16:34, Michael Kenney wrote: >>> > >Hi Martyn, >>> > > >>> > >On Tue, Feb 4, 2014 at 7:28 AM, Martyn Welch <martyn.welch@xxxxxx >>> > ><mailto:martyn.welch@xxxxxx>> wrote: >>> > > >>> > > On 28/12/13 00:34, Michael Kenney wrote: >>> > > >>> > > Hi Martyn, >>> > > >>> > > On Fri, Dec 27, 2013 at 4:23 PM, Martyn Welch >>> > > <martyn@xxxxxxxxxxxx <mailto:martyn@xxxxxxxxxxxx>> wrote: >>> > > >>> > > On 27/12/13 20:15, Michael Kenney wrote: >>> > > >>> > > >>> > > We are using the vme_tsi148 bridge driver along with >>> > > the >>> > > vme_user >>> > > driver to access the VME boards. The A/D board requires >>> > > D32 bus cycles >>> > > and the VME master window is configured accordingly, >>> > > however, when >>> > > monitoring the bus cycles with a logic analyzer, we >>> > > noticed that the >>> > > CPU is transferring one byte at a time (i.e. four D8 >>> > > transfers rather >>> > > than one D32). >>> > > >>> > > Is this the expected behavior of the tsi148 driver? >>> > > >>> > > >>> > > Hi Mike, >>> > > >>> > > This is certainly not the expected behaviour - if the >>> > > window >>> > > is configured >>> > > for D32 then it should do 32 bit transfers where possible. >>> > > >>> > > I've heard of this happening recently, but haven't yet been >>> > > able to >>> > > replicate it. Which VME board are you running Linux on and >>> > > which flavour of >>> > > Linux? >>> > > >>> > > >>> > > I'm running Debian 7.2 with kernel 3.2 on a Fastwel CPC600 >>> > > (Pentium M >>> > > based CPU board). >>> > > >>> > > >>> > > I haven't forgotten about this, still not sure exactly what is >>> > > happening. >>> > > >>> > > Is your install/kernel 32 or 64 bit? >>> > > >>> > > Are you doing single 32-bit transfers, or are you seeing this on >>> > > longer transfers (i.e. copying a buffer full of data)? >>> > > >>> > > >>> > >Thanks for getting back to me. >>> > > >>> > >I'm running a 32-bit kernel and I see this behavior on all transfers >>> > >regardless of buffer size. >>> > > >>> > >>> > Gah! Thought I could see what may be causing it in 64-bit kernels, but >>> > not >>> > 32-bit (my x86 asm is not particularly hot). >>> > >>> > I think we /may/ be hitting issues with how the memcpy function gets >>> > implemented on specific architectures. The tsi148 is a PCI/X to VME >>> > bridge, >>> > if it receives a series of 8-bit reads or writes, it translates these to >>> > 8-bit reads or writes on the VME bus, which is not necessarily what we >>> > want. >>> > >>> > According to the data sheet, the card you are using has an Intel 6300ESB >>> > ICH, which seems to be PCI/X (so we can rule out PCIe to PCI/X bridges >>> > or >>> > something like that doing nasty things). >>> > >>> > I think (if I follow everything correctly) then the memcpy for 32-bit is >>> > handled by __memcpy in arch/x86/include/asm/string_32.h: >>> > >>> > static __always_inline void *__memcpy(void *to, const void *from, size_t >>> > n) >>> > { >>> > int d0, d1, d2; >>> > asm volatile("rep ; movsl\n\t" >>> > "movl %4,%%ecx\n\t" >>> > "andl $3,%%ecx\n\t" >>> > "jz 1f\n\t" >>> > "rep ; movsb\n\t" >>> > "1:" >>> > : "=&c" (d0), "=&D" (d1), "=&S" (d2) >>> > : "0" (n / 4), "g" (n), "1" ((long)to), "2" >>> > ((long)from) >>> > : "memory"); >>> > return to; >>> > } >>> > >>> > I'd expected this function to use movl (32-bit moves) where possible, >>> > but >>> > movsb to get to naturally aligned moves (which is something that we deal >>> > with in the VME code already to use 16-bit reads where we can). >>> > >>> > Greg (as co-maintainer of the VME subsystem :-) ), Am I reading this >>> > right? >>> >>> Yes, but why would it matter if we copy by bytes, bits, or dwords to a >>> memory-backed bus? By definition, that shouldn't matter. If it does >>> matter, then we shouldn't be using memcpy, or relying on the compiler to >>> do the copying, but rather we need to use a write() function instead as >>> this would be IO memory that does care about this type of thing. >>> >> >> The drivers are using the functions memcpy_toio and memcpy_fromio for the >> bulk of the transfer, these seem to equate to memcpy on x86. >> >> In this instance it does matter - some devices can only accept certain width >> accesses and the VME bridge will translate a 8-bit PCI transfer to a 8-bit >> transfer on the VME bus. >> >> So (and I haven't even compile tested this yet), this?: >> >> diff --git a/drivers/vme/bridges/vme_ca91cx42.c >> b/drivers/vme/bridges/vme_ca91cx42.c >> index 1425d22c..0d87ebd 100644 >> --- a/drivers/vme/bridges/vme_ca91cx42.c >> +++ b/drivers/vme/bridges/vme_ca91cx42.c >> @@ -894,9 +894,9 @@ static ssize_t ca91cx42_master_read(struct >> vme_master_resource *image, >> } >> >> count32 = (count - done) & ~0x3; >> - if (count32 > 0) { >> - memcpy_fromio(buf + done, addr + done, (unsigned int)count); >> - done += count32; >> + while (done < count32) { >> + *(u32 *)(buf + done) = ioread32(addr + done); >> + done += 4; >> } >> >> if ((count - done) & 0x2) { >> @@ -948,9 +948,9 @@ static ssize_t ca91cx42_master_write(struct >> vme_master_resource *image, >> } >> >> count32 = (count - done) & ~0x3; >> - if (count32 > 0) { >> - memcpy_toio(addr + done, buf + done, count32); >> - done += count32; >> + while (done < count32) { >> + iowrite32(*(u32 *)(buf + done), addr + done); >> + done += 4; >> } >> >> if ((count - done) & 0x2) { >> diff --git a/drivers/vme/bridges/vme_tsi148.c >> b/drivers/vme/bridges/vme_tsi148.c >> index 5fbd08f..bc82991 100644 >> --- a/drivers/vme/bridges/vme_tsi148.c >> +++ b/drivers/vme/bridges/vme_tsi148.c >> @@ -1297,9 +1297,9 @@ static ssize_t tsi148_master_read(struct >> vme_master_resource *image, void *buf, >> } >> >> count32 = (count - done) & ~0x3; >> - if (count32 > 0) { >> - memcpy_fromio(buf + done, addr + done, count32); >> - done += count32; >> + if (done < count32) { >> + *(u32 *)(buf + done) = ioread32(addr + done); >> + done += 4; >> } >> >> if ((count - done) & 0x2) { >> @@ -1379,9 +1379,9 @@ static ssize_t tsi148_master_write(struct >> vme_master_resource *image, void *buf, >> } >> >> count32 = (count - done) & ~0x3; >> - if (count32 > 0) { >> - memcpy_toio(addr + done, buf + done, count32); >> - done += count32; >> + if (done < count32) { >> + iowrite32(*(u32 *)(buf + done), addr + done); >> + done += 4; >> } >> >> if ((count - done) & 0x2) { >> >> >>> >>> Does that help? >>> >> >> Yep :-) >> >> Thanks Greg. > > That certainly looks promising. I can give that patch a try later this week. > > Thanks. Good news Martyn. I had a bit of time today to test your patch and am happy to say it worked! No more bus errors. Now I can work on getting the actual data acquisition working :-) Thanks again for all your help. --Mike _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
