changeset 252:68069d97828b

BUG-id: DIO-16: SPISlaveImpl.transfer method does not begin transaction with zero skip parameter Summary: SPISlaveImpl is completely re-implemented. All write and read operation performed through the SPICombinedMessage which starts and ends transaction Reviewed-by: snazarki Contributed-by: amironenko
author onazarkina
date Tue, 15 Mar 2016 17:17:36 +0300
parents 19eec3548675
children 49f2db0470a5
files src/se/classes/com/oracle/dio/utils/ExceptionMessage.java src/share/classes/com/oracle/dio/impl/Transaction.java src/share/classes/com/oracle/dio/spibus/impl/SPICompositeMessageImpl.java src/share/classes/com/oracle/dio/spibus/impl/SPISlaveImpl.java
diffstat 4 files changed, 146 insertions(+), 180 deletions(-) [+]
line wrap: on
line diff
--- a/src/se/classes/com/oracle/dio/utils/ExceptionMessage.java	Wed Mar 09 16:17:38 2016 +0300
+++ b/src/se/classes/com/oracle/dio/utils/ExceptionMessage.java	Tue Mar 15 17:17:36 2016 +0300
@@ -152,7 +152,8 @@
     public static final int SPIBUS_FIRST = PWM_LAST + 1;
     public static final int SPIBUS_SLAVE_WORD_LENGTH                 = SPIBUS_FIRST + 0;
     public static final int SPIBUS_BYTE_NUMBER_BELIES_WORD_LENGTH    = SPIBUS_FIRST + 1;
-    public static final int SPIBUS_LAST = SPIBUS_BYTE_NUMBER_BELIES_WORD_LENGTH;
+    public static final int SPIBUS_BYTE_BUFFER_MODIFICATION          = SPIBUS_FIRST + 2;
+    public static final int SPIBUS_LAST = SPIBUS_BYTE_BUFFER_MODIFICATION;
 
     public static final int UART_FIRST = SPIBUS_LAST + 1;
     public static final int UART_CANT_GET_PORT_NAME                  = UART_FIRST + 0;
@@ -277,6 +278,7 @@
         // spi bus messages
         "Slave Word Length is %d",
         "the number of bytes to receive/send belies word length",
+        "The original read buffer was modified after append",
 
         // uart messages
         "Cannot get serial port name",
--- a/src/share/classes/com/oracle/dio/impl/Transaction.java	Wed Mar 09 16:17:38 2016 +0300
+++ b/src/share/classes/com/oracle/dio/impl/Transaction.java	Tue Mar 15 17:17:36 2016 +0300
@@ -36,10 +36,6 @@
  */
 public abstract class Transaction<P extends Device<P>> extends PowerManagedBase<P>{
 
-    public static final int REGULAR_MESSAGE = -1;
-
-    protected int transaction = REGULAR_MESSAGE;
-
     protected Transaction(DeviceDescriptor<P> dscr, int mode) {
         super(dscr, mode);
     }
--- a/src/share/classes/com/oracle/dio/spibus/impl/SPICompositeMessageImpl.java	Wed Mar 09 16:17:38 2016 +0300
+++ b/src/share/classes/com/oracle/dio/spibus/impl/SPICompositeMessageImpl.java	Tue Mar 15 17:17:36 2016 +0300
@@ -27,9 +27,10 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Vector;
 
 import com.oracle.dio.utils.ExceptionMessage;
+import java.nio.BufferOverflowException;
+import java.util.ConcurrentModificationException;
 
 import jdk.dio.ClosedDeviceException;
 import jdk.dio.UnavailableDeviceException;
@@ -44,7 +45,7 @@
     private boolean isAlreadyTransferedOnce;
 
     /* Owner of the message */
-    private SPISlaveImpl device;
+    private final SPISlaveImpl device;
 
     // delay between operations
     private int delay;
@@ -53,7 +54,7 @@
     private int rxMsgs;
 
     private class Message {
-        ByteBuffer tx, rx;
+        ByteBuffer tx, rx, newTx, newRx;
         int skip, delay;
         public Message(ByteBuffer tx, int skip, ByteBuffer rx, int delay) {
             this.rx = rx;
@@ -162,6 +163,52 @@
     public SPIDevice getTargetedDevice() {
         return device;
     }
+    
+    /**
+     * Returns buffers are suitable for low level SPI operations 
+     * New src buffer is located at index 0, dst buffer is at index 1
+     * The both buffers are direct to avoid native resource allocations in low levels
+     * The both buffers are the same length
+     *
+     * @param originalSrc 
+     *            original array to be sent
+     * @param originalDst 
+     *            The buffer into which bytes are to be transferred
+     *
+     * @return New buffers in the array. newSrcBuf = array[0], newDstBuf = array[1]
+     */    
+    private ByteBuffer[] getBuffersForTransfer(ByteBuffer originalSrc, int skip, ByteBuffer originalDst) {
+        int bytesInSrc = originalSrc == null ? 0 : originalSrc.remaining();
+        int bytesInDst = originalDst == null ? 0 : originalDst.remaining();
+                
+        int newRequiredSizeOfBuffers = bytesInSrc < (skip + bytesInDst) ?
+                                            skip + bytesInDst :
+                                            bytesInSrc;
+        
+        ByteBuffer[] array = new ByteBuffer[2];
+        
+        if(originalSrc == null || !originalSrc.isDirect() || originalSrc.remaining() < newRequiredSizeOfBuffers) {
+            array[0] = (ByteBuffer) ByteBuffer.allocateDirect(newRequiredSizeOfBuffers);
+            if(originalSrc != null) {
+                array[0].put(originalSrc);
+            }
+            array[0].rewind();
+        } else {
+            //Can not use originalSrc as is, because caller code can change position and limit
+            //after calling read/write/append operations
+            array[0] = originalSrc.slice();
+        }
+        
+        if(originalDst == null || !originalDst.isDirect() || originalDst.remaining() < newRequiredSizeOfBuffers) {
+            array[1] = ByteBuffer.allocateDirect(newRequiredSizeOfBuffers);
+        } else {
+            //Can not use originalDst as is, because caller code can change position and limit
+            //after calling read/write/append operations
+            array[1] = originalDst.slice();
+        }
+        
+        return array;
+    }
 
     @Override
     public int[] transfer() throws IOException, UnavailableDeviceException, ClosedDeviceException {
@@ -172,20 +219,57 @@
             if (0 == messageList.size()) {
                 return new int[0];
             }
-            int transaction = device.beginTransaction();
+
+            device.beginTransaction();
+            
             try {
                 final int size = messageList.size();
                 int[] bytesRead = new int[rxMsgs];
-                for (int i = 0, j = 0; i < size; i++) {
+                for (int i = 0; i < size; i++) {
                     Message message = messageList.get(i);
-                    int res = device.transfer(message.tx, message.skip, message.rx, transaction);
-                    if (null != message.rx) {
-                        bytesRead[j++] = res;
+                    ByteBuffer[] newBuffers = getBuffersForTransfer(message.tx, message.skip, message.rx);
+                    message.newTx = newBuffers[0];
+                    message.newRx = newBuffers[1];
+                    
+                    //New buffers have the same length and to avoid a dummy native call
+                    //just make tranfers of non-empty buffers
+                    if(message.newRx.remaining() > 0) {
+                        device.transferWithLock(message.newTx, message.newRx);
                     }
                 }
+                
+                device.endTransaction();
+                int j = 0;
+                for(Message message : messageList) {
+                    if(message.tx != null) {
+                        message.tx.position(message.tx.limit());
+                    }
+                    
+                    if(message.rx != null) {
+                        message.newRx.position(message.skip);
+                        try {
+                            message.newRx.limit(message.skip + message.rx.remaining());
+
+                            bytesRead[j++] = message.newRx.remaining();
+                        
+                            message.rx.put(message.newRx);
+                        } catch(BufferOverflowException | IllegalArgumentException ex) {
+                            //IAE for newRx.limit and BOE is for rx.put
+                            throw new ConcurrentModificationException(
+                                ExceptionMessage.format(
+                                        ExceptionMessage.SPIBUS_BYTE_BUFFER_MODIFICATION));
+                                    
+                        }
+                    }
+                    
+                    message.newRx = null;
+                    message.newTx = null;
+                }
+                
                 return bytesRead;
+            
             } finally {
-                device.endTransaction(transaction);
+                device.endTransaction();
             }
         }
     }
--- a/src/share/classes/com/oracle/dio/spibus/impl/SPISlaveImpl.java	Wed Mar 09 16:17:38 2016 +0300
+++ b/src/share/classes/com/oracle/dio/spibus/impl/SPISlaveImpl.java	Tue Mar 15 17:17:36 2016 +0300
@@ -26,18 +26,13 @@
 package com.oracle.dio.spibus.impl;
 
 import java.io.IOException;
-import java.lang.Runnable;
 import java.nio.ByteBuffer;
 import java.nio.Buffer;
 import java.security.AccessController;
-import java.util.Objects;
-import java.util.Vector;
 
 import com.oracle.dio.impl.Transaction;
-import com.oracle.dio.power.impl.PowerManagedBase;
 import com.oracle.dio.utils.Constants;
 import com.oracle.dio.utils.ExceptionMessage;
-import com.oracle.dio.utils.Logging;
 
 import jdk.dio.ClosedDeviceException;
 import jdk.dio.DeviceConfig;
@@ -46,11 +41,9 @@
 import jdk.dio.DeviceNotFoundException;
 import jdk.dio.DevicePermission;
 import jdk.dio.InvalidDeviceConfigException;
-import jdk.dio.InvalidDeviceConfigException;
-import jdk.dio.Transactional;
 import jdk.dio.UnavailableDeviceException;
 import jdk.dio.spibus.InvalidWordLengthException;
-import jdk.dio.spibus.InvalidWordLengthException;
+import jdk.dio.spibus.SPICompositeMessage;
 import jdk.dio.spibus.SPIDevice;
 import jdk.dio.spibus.SPIDeviceConfig;
 import jdk.dio.spibus.SPIPermission;
@@ -63,8 +56,7 @@
 
     //every call checkWordLen updates these two variables
     private int byteNum;
-    private int bitNum;
-
+    private int bitNum;   
 
     public SPISlaveImpl(DeviceDescriptor<SPIDevice> dscr, int mode) throws
             IOException, DeviceNotFoundException, InvalidDeviceConfigException {
@@ -162,8 +154,8 @@
     @Override
     public int read() throws IOException, UnavailableDeviceException,
             ClosedDeviceException {
-        ByteBuffer dst = ByteBuffer.allocateDirect(byteNum);
-        transfer(null, 0, dst);
+        ByteBuffer dst = ByteBuffer.allocateDirect(byteNum);        
+        transferInternal(null, 0, dst);
         return byteArray2int(dst);
     }
 
@@ -229,14 +221,14 @@
             throw new IllegalArgumentException();
         }
         checkBuffer(dst);
-        return transfer(null, skip, dst);
+        return transferInternal(null, skip, dst);
     }
 
     @Override
     public int write(ByteBuffer src) throws IOException,
             UnavailableDeviceException, ClosedDeviceException {
         checkBuffer(src);
-        return transfer(src, 0, null);
+        return transferInternal(src, 0, null);
     }
 
     @Override
@@ -261,14 +253,14 @@
         }
         checkBuffer(src);
         checkBuffer(dst);
-        return transfer(src, skip, dst);
+        return transferInternal(src, skip, dst);
     }
 
     @Override
     public int writeAndRead(int txData) throws IOException, UnavailableDeviceException, ClosedDeviceException{
         ByteBuffer tx = int2byteArray(txData);
         ByteBuffer rx = tx.slice();
-        transfer(tx, 0, rx);
+        transferInternal(tx, 0, rx);
         return byteArray2int(rx);
     }
 
@@ -318,164 +310,56 @@
         }
         return retI;
     }
-
-    private int transfer(ByteBuffer src, int skip, ByteBuffer dst) throws IOException {
-        return transfer(src, skip, dst, transaction);
+    
+    /* Returns number of recevied bytes if dst is not NULL, or number of sent bytes otherwise */
+    private int transferInternal(ByteBuffer src, int skip, ByteBuffer dst) throws IOException {
+        int returnCount = dst != null ? dst.remaining() : src.remaining();
+        SPICompositeMessage message = this.createCompositeMessage();
+        
+        if(src == null && dst != null) {
+            message.appendRead(skip, dst);
+        } else if(dst == null && src != null) {
+            message.appendWrite(src);
+        } else {
+            message.appendWriteAndRead(src, skip, dst);            
+        }
+              
+        message.transfer();
+        
+        return returnCount;  
+    }
+    
+    
+    /**
+     * Performs a SPI transfer operation with locking the SPI peripheral
+     * The both buffers should be direct and same length
+     *
+     * @param src 
+     *            Direct byte buffer which will be sent to the slave device
+     * @param dst 
+     *            Direct byte buffer which will used for the saving all the 
+     *            data from slave 
+     *
+     */    
+    void transferWithLock(ByteBuffer src, ByteBuffer dst) throws IOException {
+        try {
+            conditionalLock();
+            
+            writeAndRead0(src, dst);
+            
+        } finally  {
+            conditionalUnlock();
+        }        
     }
 
-    /* Returns number of recevied bytes if dst is not NULL, or number of sent bytes otherwise */
-    int transfer(ByteBuffer src, int skip, ByteBuffer dst, int transaction) throws IOException {
-
-        synchronized(this) {
-            checkPowerState();
-        }
-
-        int xfered = 0;
-        final boolean count_recv = null != dst;
-        final boolean combined = (0 != skip || (null != dst && null != src && dst.remaining() != src.remaining()));
-        Vector<Runnable> localActions = null;
-
-        /* synchronized allows to avoid IllegaStateException for the case when transfer()
-           is called while previous operation is incomplete.
-           sync on handle to avoid dead lock on close() since close is synchronized on this instance.
-        */
-        synchronized(handle){
-
-        final boolean trStart = combined && transaction == Transaction.REGULAR_MESSAGE;
-
-        if (trStart) {
-            // can throw ISE
-            transaction = beginTransaction();
-            if (!(this instanceof Transactional)) {
-                localActions = new Vector<>();
-            }
-        }
-
-        try {
-            do {
-                // convert tries to align toSend and toRecv buffer length if src or dst are nondirect.
-                ByteBuffer toRecv = convert(dst, src);
-                ByteBuffer toSend = convert(src, toRecv);
-
-
-                // if there is nothing to send, use recv buffer as dummy send data
-                if (null == toSend) {
-                    if (null == toRecv) {
-                        // both empty buffers
-                        return xfered;
-                    }
-                    toSend = toRecv.slice();
-                }
-
-                if (null != toRecv) {
-                    // always align send and recv buffers len,
-                    // or recv to NULL buffer
-                    if (toSend.remaining() <= skip) {
-                        toRecv = null;
-                    }
-                }
-
-
-                try {
-                    conditionalLock();
-                    writeAndRead0(toSend, toRecv);
-                } finally {
-                    conditionalUnlock();
-                }
-
-                if (null != src) {
-
-                    if (null != localActions) {
-                        final ByteBuffer ref = toSend;
-                        localActions.add(new Runnable() {
-                                public void run() {
-                                    // dummy action to retain object refence
-                                    ref.remaining();
-                                };
-                            });
-                    }
-
-                    if (!count_recv) {
-                        xfered += toSend.remaining();
-                    }
-                    shiftBufferPosition(src, src.position() + toSend.remaining());
-                }
-
-                if (skip > 0) {
-                    if (null != toRecv) {
-                        // ability to fit 'skip' bytes was checked above (see if (toSend.remaining() <= skip) )
-                        toRecv.position(skip);
-                        skip = 0;
-                    } else {
-                        skip-=toSend.remaining();
-                    }
-                }
-                if (null != toRecv) {
-                    xfered += toRecv.remaining();
-                    // linux and similar accumulate packets and transfer them after endTrasaction call
-                    // transaction requires postponed reverse copying
-                    if (null != localActions) {
-                        final ByteBuffer to = dst.slice();
-                        final ByteBuffer from = toRecv.slice();
-                        localActions.add(new Runnable() {
-                                public void run() {
-                                    to.put(from);
-                                };
-                            });
-
-                        try {
-                            dst.position(dst.position() + toRecv.remaining());
-                        }catch (IllegalArgumentException e){
-                            // the buffer was updated in parallel
-                            Logging.reportWarning(ExceptionMessage.format(ExceptionMessage.BUFFER_IS_MODIFIED));
-                            //
-                            dst.position(dst.limit());
-                        }
-
-                    } else {
-                        dst.put(toRecv);
-                    }
-                }
-
-            } while ((null != dst && dst.hasRemaining()) || (null != src && src.hasRemaining()));
-
-        } finally  {
-            if (trStart) {
-                try {
-                    endTransaction(transaction);
-                    if (null != localActions) {
-                        for (Runnable toRun: localActions) {
-                            toRun.run();
-                        }
-                    }
-                } catch (IllegalStateException e) {
-                    // intentionally skip
-                }
-            }
-        }
-        }//synchronized(handle)
-        return xfered;
-    }
-
-    // transaction demarcator
-    private static int trans_counter;
-
-    protected int beginTransaction() throws IOException, IllegalStateException {
+    void beginTransaction() throws IOException, IllegalStateException {
         // interapp lock
         conditionalLock();
 
         begin0();
-        // it is enough to separate transactions in scope of current application enviroment.
-        // the rest is cared by javacall
-        synchronized(SPISlaveImpl.class) {
-            return (transaction = trans_counter++);
-        }
     }
 
-    protected void endTransaction(int transaction) throws IllegalStateException {
-        if (transaction != this.transaction) {
-            throw new IllegalStateException();
-        }
+    void endTransaction() throws IllegalStateException {
 
         end0();