Re: vme_tsi148 question

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/02/14 23:21, Michael Kenney wrote:
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 Mike.

Guess you didn't spot the little bug then :-)

For the tsi148 I forgot to change the conditional from "if" to "while".

I'll submit a proper patch in a minute,

Martyn

Thanks again for all your help.

--Mike


--
--
Martyn Welch (Lead Software Engineer)  | Registered in England and Wales
GE Intelligent Platforms               | (3828642) at 100 Barbirolli Square
T +44(0)1327322748                     | Manchester, M2 3AB
E martyn.welch@xxxxxx                  | VAT:GB 927559189
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux