Re: [PATCH v3] gpib: agilent_82357a: Support some 82357B clones

[Dave Penkler <dpenkler@gmail.com>]

On Thu, Sep 10, 2026 at 08:24:03AM +0000, Tom Keller wrote:
> Some adapters marked as Agilent/Keysight 82357B are clones that contain
> their own firmware. They do not support XFER_STATUS and need slightly
> relaxed timing. Detect clone adapters that fail a firmware load and work
> around their quirks.
> 
> Signed-off-by: Tom Keller <tom@tompkel.net>

Acked-by: Dave Penkler <dpenkler@gmail.com>
Tested-by: Dave Penkler <dpenkler@gmail.com>

> ---
> Changes since v2:
> - Use 819 ns as the default FAST_TALKER_T1 for all adapters and drop
>   the clone-specific initialization.
> - Reword the clone detection message.
> Changes since v1:
> - Keep the existing init sequence, add a small increase to FAST_TALKER_T1
>   on clones.
> - Clear AWF_NO_FAST_TALKER_FIRST_BYTE on command writes for all
>   adapters.
> v2: https://lore.kernel.org/all/20260902224109.2130225-1-tom@tompkel.net/
> v1: https://lore.kernel.org/all/20260901233621.1823280-1-tom@tompkel.net/
>  drivers/gpib/agilent_82357a/agilent_82357a.c | 41 +++++++++++++++++++-
>  drivers/gpib/agilent_82357a/agilent_82357a.h |  2 +
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpib/agilent_82357a/agilent_82357a.c b/drivers/gpib/agilent_82357a/agilent_82357a.c
> index 2468a471d1755..5b34fe7e13662 100644
> --- a/drivers/gpib/agilent_82357a/agilent_82357a.c
> +++ b/drivers/gpib/agilent_82357a/agilent_82357a.c
> @@ -567,7 +567,7 @@ static ssize_t agilent_82357a_generic_write(struct gpib_board *board,
>  	out_data[i++] = 0; // secondary address when AWF_NO_ADDRESS is not set
>  	out_data[i] = AWF_NO_ADDRESS | AWF_NO_FAST_TALKER_FIRST_BYTE;
>  	if (send_commands)
> -		out_data[i] |= AWF_ATN | AWF_NO_FAST_TALKER;
> +		out_data[i] = AWF_ATN | AWF_NO_ADDRESS | AWF_NO_FAST_TALKER;
>  	if (send_eoi)
>  		out_data[i] |= AWF_SEND_EOI;
>  	++i;
> @@ -652,6 +652,16 @@ static ssize_t agilent_82357a_generic_write(struct gpib_board *board,
>  		return -ETIMEDOUT;
>  	}
>  
> +	/*
> +	 * Clone adapters do not answer to XFER_STATUS, so use the
> +	 * bytes_written from the write complete interrupt.
> +	 */
> +	if (a_priv->is_clone_82357b) {
> +		mutex_unlock(&a_priv->bulk_transfer_lock);
> +		*bytes_written = a_priv->write_complete_count;
> +		return 0;
> +	}
> +
>  	status_data = kmalloc(STATUS_DATA_LEN, GFP_KERNEL);
>  	if (!status_data) {
>  		mutex_unlock(&a_priv->bulk_transfer_lock);
> @@ -1114,6 +1124,12 @@ static void agilent_82357a_interrupt_complete(struct urb *urb)
>  	}
>  
>  	interrupt_flags = transfer_buffer[0];
> +	if (urb->actual_length >= 6 && test_bit(AIF_WRITE_COMPLETE_BN, &interrupt_flags)) {
> +		a_priv->write_complete_count  = (u32)transfer_buffer[2];
> +		a_priv->write_complete_count |= (u32)transfer_buffer[3] << 8;
> +		a_priv->write_complete_count |= (u32)transfer_buffer[4] << 16;
> +		a_priv->write_complete_count |= (u32)transfer_buffer[5] << 24;
> +	}
>  	if (test_bit(AIF_READ_COMPLETE_BN, &interrupt_flags))
>  		set_bit(AIF_READ_COMPLETE_BN, &a_priv->interrupt_flags);
>  	if (test_bit(AIF_WRITE_COMPLETE_BN, &interrupt_flags))
> @@ -1213,6 +1229,26 @@ static void agilent_82357a_free_private(struct gpib_board *board)
>  	board->private_data = NULL;
>  }
>  
> +static void agilent_82357a_detect_clone(struct agilent_82357a_priv *a_priv)
> +{
> +	struct usb_device *usb_dev = interface_to_usbdev(a_priv->bus_interface);
> +	u8 *ret_data;
> +	int retval;
> +
> +	ret_data = kmalloc(1, GFP_KERNEL);
> +	if (!ret_data)
> +		return;
> +	retval = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0), 0xA0,
> +				 USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +				 0xE600, 0, ret_data, 1, 100);
> +	kfree(ret_data);
> +	if (retval < 0) {
> +		a_priv->is_clone_82357b = 1;
> +		dev_info(&usb_dev->dev, "82357B clone with firmware detected (CPUCS read returned %i)\n",
> +			 retval);
> +	}
> +}
> +
>  #define INIT_NUM_REG_WRITES 18
>  static int agilent_82357a_init(struct gpib_board *board)
>  {
> @@ -1346,6 +1382,7 @@ static int agilent_82357a_attach(struct gpib_board *board, const struct gpib_boa
>  	case USB_DEVICE_ID_AGILENT_82357B:
>  		a_priv->bulk_out_endpoint = AGILENT_82357B_BULK_OUT_ENDPOINT;
>  		a_priv->interrupt_in_endpoint = AGILENT_82357B_INTERRUPT_IN_ENDPOINT;
> +		agilent_82357a_detect_clone(a_priv);
>  		break;
>  	default:
>  		dev_err(&usb_dev->dev, "bug, unhandled product_id in switch?\n");
> @@ -1359,7 +1396,7 @@ static int agilent_82357a_attach(struct gpib_board *board, const struct gpib_boa
>  
>  	timer_setup(&a_priv->bulk_timer, agilent_82357a_timeout_handler, 0);
>  
> -	board->t1_nano_sec = 800;
> +	board->t1_nano_sec = 819;
>  
>  	retval = agilent_82357a_init(board);
>  
> diff --git a/drivers/gpib/agilent_82357a/agilent_82357a.h b/drivers/gpib/agilent_82357a/agilent_82357a.h
> index 33ac558e55528..2013163446965 100644
> --- a/drivers/gpib/agilent_82357a/agilent_82357a.h
> +++ b/drivers/gpib/agilent_82357a/agilent_82357a.h
> @@ -133,12 +133,14 @@ struct agilent_82357a_priv {
>  	struct mutex bulk_alloc_lock;		// bulk transfer allocation lock
>  	struct mutex interrupt_alloc_lock;	// interrupt allocation lock
>  	struct mutex control_alloc_lock;	// control message allocation lock
> +	u32 write_complete_count;               // bytes transferred in last write
>  	struct timer_list bulk_timer;
>  	struct agilent_82357a_urb_ctx context;
>  	unsigned int bulk_out_endpoint;
>  	unsigned int interrupt_in_endpoint;
>  	unsigned is_cic : 1;
>  	unsigned ren_state : 1;
> +	unsigned is_clone_82357b : 1;
>  };
>  
>  struct agilent_82357a_register_pairlet {
> -- 
> 2.55.0
> 
>