Re: [PATCH] gpib: agilent_82357a: Support some 82357b clones

[Dave Penkler]

On Tue, Sep 01, 2026 at 11:36:40PM +0000, Tom Keller wrote:
> Some adapters marked as genuine Agilent/Keysight 82357B devices are in
> fact fake. These clones have their own EEPROM instead of loading
> firmware on power up, and only respond to the initialization sequence
> sent by the vendor drivers. Detect clone adapters that don't respond to
> the 0xA0 firmware load request and run the alternate init sequence
> against them.
> 
> Signed-off-by: Tom Keller <tom@xxxxxxxxxxx>
> ---
> Because I do not have a genuine 82357B, I can't test whether or not this
> patch breaks working adapters, but I did confirm that the clone adapter
> doesn't respond to a firmware load 0xA0 request, and the adapter
> enumerates as 0957:0718 without loading firmware. A CPUCS read would be
> answered by real hardware, but it isn't by the clone.

Hi,
There are a bunch of different clones out there which all work with
the current driver. Some require a firmware load, others don't. Those
that don't, enumerate directly as 0957:0718 and those that do as
0957:0518. For the latter the userspace udev scripts load the firmware
with fxload after which they also enumerate as 0957:0718. I am
reticent to introduce clone specific code. What are the failure
symptoms of your clone with the current driver ? What would be the
minimal changes needed to get it to work ? For the initialisation
sequence, do the register writes have to be in the exact order as in
the pcap trace ? Try removing the undocumented writes. Functionally
they are not needed unless indeed your clone is fingerprinting the
sequence.


> 
> Vendor init sequences were obtained by running the latest vendor control
> bundle (21.3.293) on a Windows 11 VM and capturing the USB packets with
> pcap. I confirmed that this change does make my clone function, and as 
> far as I can tell th
> e clone is fingerprinting the specific sequences. The
> clone has poor error handling in hardware and can freeze on a failed or
> aborted transaction; in that case, power-cycling the adapter is required.
> 
>  drivers/gpib/agilent_82357a/agilent_82357a.c | 151 ++++++++++++++++++-
>  drivers/gpib/agilent_82357a/agilent_82357a.h |   2 +
>  2 files changed, 152 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpib/agilent_82357a/agilent_82357a.c b/drivers/gpib/agilent_82357a/agilent_82357a.c
> index 2468a471d..b5c0d34f0 100644
> --- a/drivers/gpib/agilent_82357a/agilent_82357a.c
> +++ b/drivers/gpib/agilent_82357a/agilent_82357a.c
> @@ -566,8 +566,11 @@ static ssize_t agilent_82357a_generic_write(struct gpib_board *board,
>  	out_data[i++] = 0; // primary address when AWF_NO_ADDRESS is not set
>  	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)
> +	if (send_commands) {
>  		out_data[i] |= A
> WF_ATN | AWF_NO_FAST_TALKER;
> +		if (a_priv->is_clone_82357b)
> +			out_data[i] &= ~AWF_NO_FAST_TALKER_FIRST_BYTE;

Is this really necessary? AWF_NO_FAST_TALKER is already set so
clearing the AWF_NO_FAST_TALKER_FIRST_BYTE bit should not be needed,
although having it set is redundant. I checked that clearing the
AWF_NO_FAST_TALKER_FIRST_BYTE bit on commands also works with my adaptors.
If setting the bit on your clone does not work I'm OK with not setting it
for all adaptors when sending commands.

> +	}
>  	if (send_eoi)
>  		out_data[i] |= AWF_SEND_EOI;
>  	++i;
> @@ -652,6 +655,15 @@ static ssize_t agilent_82357a_generic_write(struct gpib_board *board,
>  		return -ETIMEDOUT;
>  	}
>  
> +	/*
> +	 * The write-complete interrupt carries the number of bytes transferred
> +	 * in bytes 2..5. Clone adapters do not properly answer XFER_STATUS
> +	 */
> +	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 +1126,10 @@ 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 = transfer_
> buffer[2] |
> +			(transfer_buffer[3] << 8) | (transfer_buffer[4] << 16) |
> +			((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))
> @@ -1214,6 +1230,135 @@ static void agilent_82357a_free_private(struct gpib_board *board)
>  }
>  
>  #define INIT_NUM_REG_WRITES 18
> +/* Some clone adapters contain firmware already. Genuine adapters only
> + * enumerate as 0x0718 after firmware load; clone adapters enumerate as 0x0718
> + * right away and refuse a firmware load request.
> + */
> +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 *data;
> +	int retval;
> +
> +	data = kmalloc(1, GFP_KERNEL);
> +	if (!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, data, 1, 100);
> +	kfree(data);
> +	if (retval < 0) {
> +		a_priv->is_clone_82357b = 1;
> +		dev_info(&usb_dev->dev, "Clone 82357B detected (firmware load request returned %i), using vendor quirks\n",
> +			 retval);
> +	}
> +}
> +
> +/*
> + * From usb captures of vendor software. Clone adapters require this specific
> + * sequence to function.
> + */
> +static int agilent_82357a_init_82357b_clone(struct gpib_board *board)
> +{
> +	struct agilent_82357a_priv *a_priv = board->private_data;
> +	struct usb_device *usb_dev = interface_to_usbdev(a_priv->bus_interface);
> +	struct agilent_82357a_register_pairlet hw_control;
> +	struct agilent_82357a_register_pairlet writes[19];
> +	int retval;
> +
> +	writes[0].address = RESET_TO_POWERUP;
> +	writes[0].value = RESET_SPACEBALL;
> +	retval = agilent_82357a_write_registers(a_priv, writes, 1);
> +	if (retval) {
> +		dev_err(&usb_dev->dev, "write_registers() returned error\n");
> +		return -EIO;
> +	}
> +	// Write init sequence
> +	set_current_state(T
> ASK_INTERRUPTIBLE);
> +	if (schedule_timeout(usec_to_jiffies(2000)))
> +		return -ERESTARTSYS;
> +	writes[0].address = HW_CONTROL;
> +	writes[0].value = NOT_TI_RESET | SYSTEM_CONTROLLER | NOT_PARALLEL_POLL;
> +	writes[1].address = LED_CONTROL;
> +	writes[1].value = FIRMWARE_LED_CONTROL;
> +	writes[2].address = AUXCR;
> +	writes[2].value = AUX_CS | AUX_CHIP_RESET;
> +	writes[3].address = IMR0;
> +	writes[3].value = HR_BOIE;
> +	writes[4].address = IMR1;
> +	writes[4].value = HR_SRQIE;
> +	writes[5].address = AUXCR;
> +	writes[5].value = AUX_NBAF;
> +	writes[6].address = AUXCR;
> +	writes[6].value = AUX_HLDE;
> +	writes[7].address = ADR;
> +	writes[7].value = board->pad & ADDRESS_MASK;
> +	writes[8].address = PPR;
> +	writes[8].value = 0;
> +	writes[9].address = AUXCR;
> +	writes[9].value = AUX_CS | AUX_STDL;
> +	writes[10].address = AUXCR;
> +	writes[10].value = AUX_LON;
> +	writes[11].address = AUXCR;
> +	writes[11].value = AUX_TON;
> +	writes[12].address = SPMR;
> +	writes[12].value = 0;
> +	writes[13].address = 
> AUXCR;
> +	writes[13].value = AUX_RSV2;
> +	writes[14].address = AUXCR;
> +	writes[14].value = 0x20; // undocumented, sent by the vendor driver
> +	writes[15].address = AUXCR;
> +	writes[15].value = AUX_INVAL;
> +	writes[16].address = AUXCR;
> +	writes[16].value = AUX_CHIP_RESET;
> +	writes[17].address = AUXCR;
> +	writes[17].value = AUX_TCA;
> +	writes[18].address = AUXCR;
> +	writes[18].value = AUX_CS | AUX_SIC;
> +	retval = agilent_82357a_write_registers(a_priv, writes, 19);
> +	if (retval) {
> +		dev_err(&usb_dev->dev, "write_registers() returned error\n");
> +		return -EIO;
> +	}
> +
> +	set_current_state(TASK_INTERRUPTIBLE);
> +	if (schedule_timeout(usec_to_jiffies(15000)))
> +		return -ERESTARTSYS;
> +	writes[0].address = AUXCR;
> +	writes[0].value = AUX_CS | AUX_SRE;
> +	writes[1].address = AUXCR;
> +	writes[1].value = AUX_RPP;
> +	writes[2].address = AUXCR;
> +	writes[2].value = AUX_SIC;
> +	writes[3].address = 9;  // undocumented, sent by the vendor driver
> +	writes[3].value = 3;
> +	writes[4].address =
>  PROTOCOL_CONTROL;
> +	writes[4].value = WRITE_COMPLETE_INTERRUPT_EN;
> +	retval = agilent_82357a_write_registers(a_priv, writes, 5);
> +	if (retval) {
> +		dev_err(&usb_dev->dev, "write_registers() returned error\n");
> +		return -EIO;
> +	}
> +
> +	writes[0].address = FAST_TALKER_T1;
> +	writes[0].value = 0x27;
> +	board->t1_nano_sec = 0x27 * 21;
> +	retval = agilent_82357a_write_registers(a_priv, writes, 1);
> +	if (retval) {
> +		dev_err(&usb_dev->dev, "write_registers() returned error\n");
> +		return -EIO;
> +	}
> +	hw_control.address = HW_CONTROL;
> +	retval = agilent_82357a_read_registers(a_priv, &hw_control, 1, 1);
> +	if (retval) {
> +		dev_err(&usb_dev->dev, "read_registers() returned error\n");
> +		return -EIO;
> +	}
> +	a_priv->hw_control_bits = (hw_control.value & ~0x7) | NOT_TI_RESET | NOT_PARALLEL_POLL;
> +
> +	return 0;
> +}
> +
>  static int agilent_82357a_init(struct gpib_board *board)
>  {
>  	struct agilent_82357a_priv *a_priv = board->private_data;
> @@ -1223,6 +1368,9 @@ static int agilent_8
> 2357a_init(struct gpib_board *board)
>  	int retval;
>  	unsigned int nanosec;
>  
> +	if (a_priv->is_clone_82357b)
> +		return agilent_82357a_init_82357b_clone(board);
> +
>  	writes[0].address = LED_CONTROL;
>  	writes[0].value = FAIL_LED_ON;
>  	writes[1].address = RESET_TO_POWERUP;
> @@ -1346,6 +1494,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");
> diff --git a/drivers/gpib/agilent_82357a/agilent_82357a.h b/drivers/gpib/agilent_82357a/agilent_82357a.h
> index 33ac558e5..950d8d4ce 100644
> --- a/drivers/gpib/agilent_82357a/agilent_82357a.h
> +++ b/drivers/gpib/agilent_82357a/agilent_82357a.h
> @@ -126,6 +126,7 @@ struct agilent_82357a_priv {
>  	unsigned
>  short eos_mode;
>  	unsigned short hw_control_bits;
>  	unsigned long interrupt_flags;
> +	u32 write_complete_count;               // bytes transferred
>  	struct urb *bulk_urb;
>  	struct urb *interrupt_urb;
>  	u8 *interrupt_buffer;
> @@ -139,6 +140,7 @@ struct agilent_82357a_priv {
>  	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
>