Message ID | 20180517170336.8426-5-guido@kiener-muenchen.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, May 17, 2018 at 07:03:28PM +0200, Guido Kiener wrote: > - add USBTMC488_IOCTL_TRIGGER to send TRIGGER Bulk-OUT header > according to Subclass USB488 Specification > > The usbtmc trigger command is equivalent to the IEEE 488 GET (Group > Execute Trigger) action. While the "*TRG" command can be sent as > data to perform the same operation, in some situations an instrument > will be busy and unable to process the data immediately in which > case the USBTMC488_IOCTL_TRIGGER can be used to trigger the > instrument with lower latency. > > - add USBTMC_IOCTL_EOM_ENABLE to specify EOM bit for next write() > call. Sets Bit 0 of field 'bmTransferAttributes' of DEV_DEP_MSG_OUT > Bulk-OUT Header. > > Allows fine grained control over end of message handling on a > per file descriptor basis. > > - add USBTMC_IOCTL_CONFIG_TERMCHAR to control TermChar handling > for next read(). Controls field 'TermChar' and Bit 1 of field > 'bmTransferAttributes' of REQUEST_DEV_DEP_MSG_IN BULK-OUT header. > > Allows enabling/disabling of terminating a read on reception of > term_char individually for each read request. Why isn't this 3 patches? Please only do "one thing per patch". thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Zitat von Greg KH <gregkh@linuxfoundation.org>: > On Thu, May 17, 2018 at 07:03:28PM +0200, Guido Kiener wrote: >> - add USBTMC488_IOCTL_TRIGGER to send TRIGGER Bulk-OUT header >> according to Subclass USB488 Specification >> >> The usbtmc trigger command is equivalent to the IEEE 488 GET (Group >> Execute Trigger) action. While the "*TRG" command can be sent as >> data to perform the same operation, in some situations an instrument >> will be busy and unable to process the data immediately in which >> case the USBTMC488_IOCTL_TRIGGER can be used to trigger the >> instrument with lower latency. >> >> - add USBTMC_IOCTL_EOM_ENABLE to specify EOM bit for next write() >> call. Sets Bit 0 of field 'bmTransferAttributes' of DEV_DEP_MSG_OUT >> Bulk-OUT Header. >> >> Allows fine grained control over end of message handling on a >> per file descriptor basis. >> >> - add USBTMC_IOCTL_CONFIG_TERMCHAR to control TermChar handling >> for next read(). Controls field 'TermChar' and Bit 1 of field >> 'bmTransferAttributes' of REQUEST_DEV_DEP_MSG_IN BULK-OUT header. >> >> Allows enabling/disabling of terminating a read on reception of >> term_char individually for each read request. > > Why isn't this 3 patches? Please only do "one thing per patch". I just thought it is clearly arranged. Please let me know when you want to see a breakup of other patches as well. However I would be happy when I could save some extra work here. In the middle of next week I start my holiday. > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 18, 2018 at 12:08:16PM +0000, guido@kiener-muenchen.de wrote: > > Zitat von Greg KH <gregkh@linuxfoundation.org>: > > > On Thu, May 17, 2018 at 07:03:28PM +0200, Guido Kiener wrote: > > > - add USBTMC488_IOCTL_TRIGGER to send TRIGGER Bulk-OUT header > > > according to Subclass USB488 Specification > > > > > > The usbtmc trigger command is equivalent to the IEEE 488 GET (Group > > > Execute Trigger) action. While the "*TRG" command can be sent as > > > data to perform the same operation, in some situations an instrument > > > will be busy and unable to process the data immediately in which > > > case the USBTMC488_IOCTL_TRIGGER can be used to trigger the > > > instrument with lower latency. > > > > > > - add USBTMC_IOCTL_EOM_ENABLE to specify EOM bit for next write() > > > call. Sets Bit 0 of field 'bmTransferAttributes' of DEV_DEP_MSG_OUT > > > Bulk-OUT Header. > > > > > > Allows fine grained control over end of message handling on a > > > per file descriptor basis. > > > > > > - add USBTMC_IOCTL_CONFIG_TERMCHAR to control TermChar handling > > > for next read(). Controls field 'TermChar' and Bit 1 of field > > > 'bmTransferAttributes' of REQUEST_DEV_DEP_MSG_IN BULK-OUT header. > > > > > > Allows enabling/disabling of terminating a read on reception of > > > term_char individually for each read request. > > > > Why isn't this 3 patches? Please only do "one thing per patch". > > I just thought it is clearly arranged. Please let me know when you > want to see a breakup of other patches as well. I only got this far in the patch series and I stopped. As you know this has to be broken up, please do that and we can iterate from there. Worse case, I take the first few patches and you redo the later ones, that's a very common process. > However I would be happy when I could save some extra work here. > In the middle of next week I start my holiday. No rush on my end, there's no deadlines in kernel development, send them when you have them ready :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: > + retval = usb_bulk_msg(data->usb_dev, > + usb_sndbulkpipe(data->usb_dev, > + data->bulk_out), > + buffer, USBTMC_HEADER_SIZE, > + &actual, file_data->timeout); > + > + /* Store bTag (in case we need to abort) */ > + data->bTag_last_write = data->bTag; > + > + /* Increment bTag -- and increment again if zero */ > + data->bTag++; > + if (!data->bTag) > + data->bTag++; > + Independent of whether this needs to be split up, do you really want to do this regardless of usb_bulk_msg() returning an error? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Zitat von Oliver Neukum <oneukum@suse.com>: > Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: >> + retval = usb_bulk_msg(data->usb_dev, >> + usb_sndbulkpipe(data->usb_dev, >> + data->bulk_out), >> + buffer, USBTMC_HEADER_SIZE, >> + &actual, file_data->timeout); >> + >> + /* Store bTag (in case we need to abort) */ >> + data->bTag_last_write = data->bTag; >> + >> + /* Increment bTag -- and increment again if zero */ >> + data->bTag++; >> + if (!data->bTag) >> + data->bTag++; >> + > > Independent of whether this needs to be split up, do you really > want to do this regardless of usb_bulk_msg() returning an error? > > Regards > Oliver I think it is ok. Our devices do not care much about the right order of sequence numbers. It is just a hint to abort the last messages. And the client should know the last message numbers itself. The current implementation does it the same way. https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/class/usbtmc.c#L835 Regards Guido -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c index ad7872003420..152e2daa9644 100644 --- a/drivers/usb/class/usbtmc.c +++ b/drivers/usb/class/usbtmc.c @@ -126,6 +126,7 @@ struct usbtmc_file_data { u8 TermChar; bool TermCharEnabled; bool auto_abort; + u8 eom_val; }; /* Forward declarations */ @@ -170,6 +171,7 @@ static int usbtmc_open(struct inode *inode, struct file *filp) file_data->TermChar = data->TermChar; file_data->TermCharEnabled = data->TermCharEnabled; file_data->auto_abort = data->auto_abort; + file_data->eom_val = 1; INIT_LIST_HEAD(&file_data->file_elem); spin_lock_irq(&data->dev_lock); @@ -577,6 +579,51 @@ static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data, return rv; } +/* + * Sends a TRIGGER Bulk-OUT command message + * See the USBTMC-USB488 specification, Table 2. + * + * Also updates bTag_last_write. + */ +static int usbtmc488_ioctl_trigger(struct usbtmc_file_data *file_data) +{ + struct usbtmc_device_data *data = file_data->data; + int retval; + u8 *buffer; + int actual; + + buffer = kzalloc(USBTMC_HEADER_SIZE, GFP_KERNEL); + if (!buffer) + return -ENOMEM; + + buffer[0] = 128; + buffer[1] = data->bTag; + buffer[2] = ~data->bTag; + + retval = usb_bulk_msg(data->usb_dev, + usb_sndbulkpipe(data->usb_dev, + data->bulk_out), + buffer, USBTMC_HEADER_SIZE, + &actual, file_data->timeout); + + /* Store bTag (in case we need to abort) */ + data->bTag_last_write = data->bTag; + + /* Increment bTag -- and increment again if zero */ + data->bTag++; + if (!data->bTag) + data->bTag++; + + kfree(buffer); + if (retval < 0) { + dev_err(&data->intf->dev, "%s returned %d\n", + __func__, retval); + return retval; + } + + return 0; +} + /* * Sends a REQUEST_DEV_DEP_MSG_IN message on the Bulk-OUT endpoint. * @transfer_size: number of bytes to request from the device. @@ -829,7 +876,7 @@ static ssize_t usbtmc_write(struct file *filp, const char __user *buf, buffer[8] = 0; } else { this_part = remaining; - buffer[8] = 1; + buffer[8] = file_data->eom_val; } /* Setup IO buffer for DEV_DEP_MSG_OUT message */ @@ -1251,6 +1298,47 @@ static int usbtmc_ioctl_set_timeout(struct usbtmc_file_data *file_data, return 0; } +/* + * enables/disables sending EOM on write + */ +static int usbtmc_ioctl_eom_enable(struct usbtmc_file_data *file_data, + void __user *arg) +{ + __u8 eom_enable; + + if (copy_from_user(&eom_enable, arg, sizeof(eom_enable))) + return -EFAULT; + + if (eom_enable > 1) + return -EINVAL; + + file_data->eom_val = eom_enable; + + return 0; +} + +/* + * Configure TermChar and TermCharEnable + */ +static int usbtmc_ioctl_config_termc(struct usbtmc_file_data *file_data, + void __user *arg) +{ + struct usbtmc_termchar termc; + + if (copy_from_user(&termc, arg, sizeof(termc))) + return -EFAULT; + + if ((termc.term_char_enabled > 1) || + (termc.term_char_enabled && + !(file_data->data->capabilities.device_capabilities & 1))) + return -EINVAL; + + file_data->TermChar = termc.term_char; + file_data->TermCharEnabled = termc.term_char_enabled; + + return 0; +} + static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct usbtmc_file_data *file_data; @@ -1301,12 +1389,19 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg) (void __user *)arg); break; + case USBTMC_IOCTL_EOM_ENABLE: + retval = usbtmc_ioctl_eom_enable(file_data, + (void __user *)arg); + break; + + case USBTMC_IOCTL_CONFIG_TERMCHAR: + retval = usbtmc_ioctl_config_termc(file_data, + (void __user *)arg); + break; + case USBTMC488_IOCTL_GET_CAPS: - retval = copy_to_user((void __user *)arg, - &data->usb488_caps, - sizeof(data->usb488_caps)); - if (retval) - retval = -EFAULT; + retval = put_user(data->usb488_caps, + (unsigned char __user *)arg); break; case USBTMC488_IOCTL_READ_STB: @@ -1328,6 +1423,10 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg) retval = usbtmc488_ioctl_simple(data, (void __user *)arg, USBTMC488_REQUEST_LOCAL_LOCKOUT); break; + + case USBTMC488_IOCTL_TRIGGER: + retval = usbtmc488_ioctl_trigger(file_data); + break; } skip_io_on_zombie: diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h index d6de192150f4..60369825691c 100644 --- a/include/uapi/linux/usb/tmc.h +++ b/include/uapi/linux/usb/tmc.h @@ -16,6 +16,8 @@ #ifndef __LINUX_USB_TMC_H #define __LINUX_USB_TMC_H +#include <linux/types.h> /* __u8 etc */ + /* USB TMC status values */ #define USBTMC_STATUS_SUCCESS 0x01 #define USBTMC_STATUS_PENDING 0x02 @@ -38,6 +40,11 @@ #define USBTMC488_REQUEST_GOTO_LOCAL 161 #define USBTMC488_REQUEST_LOCAL_LOCKOUT 162 +struct usbtmc_termchar { + __u8 term_char; + __u8 term_char_enabled; // bool +} __attribute__ ((packed)); + /* Request values for USBTMC driver's ioctl entry point */ #define USBTMC_IOC_NR 91 #define USBTMC_IOCTL_INDICATOR_PULSE _IO(USBTMC_IOC_NR, 1) @@ -48,11 +55,15 @@ #define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7) #define USBTMC_IOCTL_GET_TIMEOUT _IOR(USBTMC_IOC_NR, 9, unsigned int) #define USBTMC_IOCTL_SET_TIMEOUT _IOW(USBTMC_IOC_NR, 10, unsigned int) +#define USBTMC_IOCTL_EOM_ENABLE _IOW(USBTMC_IOC_NR, 11, __u8) +#define USBTMC_IOCTL_CONFIG_TERMCHAR _IOW(USBTMC_IOC_NR, 12, struct usbtmc_termchar) + #define USBTMC488_IOCTL_GET_CAPS _IOR(USBTMC_IOC_NR, 17, unsigned char) #define USBTMC488_IOCTL_READ_STB _IOR(USBTMC_IOC_NR, 18, unsigned char) #define USBTMC488_IOCTL_REN_CONTROL _IOW(USBTMC_IOC_NR, 19, unsigned char) #define USBTMC488_IOCTL_GOTO_LOCAL _IO(USBTMC_IOC_NR, 20) #define USBTMC488_IOCTL_LOCAL_LOCKOUT _IO(USBTMC_IOC_NR, 21) +#define USBTMC488_IOCTL_TRIGGER _IO(USBTMC_IOC_NR, 22) /* Driver encoded usb488 capabilities */ #define USBTMC488_CAPABILITY_TRIGGER 1