Bug 96743 - [sk] [patch] broken 32-bit register operations
Summary: [sk] [patch] broken 32-bit register operations
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 6.1-BETA4
Hardware: Any Any
: Normal Affects Only Me
Assignee: Pyun YongHyeon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-04 03:10 UTC by leres
Modified: 2010-10-25 22:51 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (1.58 KB, patch)
2006-05-04 03:10 UTC, leres
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description leres 2006-05-04 03:10:19 UTC
	While attempting to apply a local patch that implements
	receive hardware timestamps (nice for research and intrusion
	detection applications) with the syskonnect SK-9843, I
	noticed that some 32-bit register operations are broken.
	Most of the registers, including XM_MODE and XM_TSTAMP_READ,
	must be accessed 16-bits at a time. One step in enabling
	timestamps is to set the XM_MODE_TIMESTAMP bit in the XM_MODE
	register but it was not possible to access the high bytes
	of the register.

	I developed my timestamp patch with 4.8-RELEASE using version
	1.8.2.1 of if_skreg.h which defines SK_XM_READ_4() and
	SK_XM_WRITE_4() macros that use sk_win_read_2() and
	sk_win_read_2() to read and write 16 bits at a time, as
	required by the hardware.

	Version 1.8.2.2 of the include file change the macros to
	use sk_win_read_4() and sk_win_write_4() which only return
	the low order 16-bits.

	This problem probably wasn't detected before now because
	the current driver only seems to ever access bits in the
	low bytes of the registers (e.g. XM_MODE_RX_PROMISC in
	XM_MODE).

Fix: The appended patch (vs. 1.29.2.1 of if_skreg.h) re-enables
	the 2-byte versions if the SK_XM_READ_4() and SK_XM_WRITE_4()
	macros. It also protects the SK_XM_WRITE_4() macro.

	After booting the new kernel, the debugging printout
	shows the complete device id value:

	    sk0: XM_DEVID 0xe0ae20 (0xe0/0xae20)

	It looks like this patch would also apply to the -current
	version (1.35).

==============================================================================
How-To-Repeat: 	This patch helps demonstrate the problem:

==============================================================================

*** if_sk.c.virgin	Wed May  3 17:56:30 2006
--- if_sk.c	Wed May  3 17:56:37 2006
***************
*** 2532,2537 ****
--- 2532,2540 ----
  
  	/* Save the XMAC II revision */
  	sc_if->sk_xmac_rev = XM_XMAC_REV(SK_XM_READ_4(sc_if, XM_DEVID));
+ printf("sk%d: XM_DEVID 0x%x (0x%x/0x%x)\n",
+     sc_if->sk_unit, SK_XM_READ_4(sc_if, XM_DEVID),
+     SK_XM_READ_2(sc_if, XM_DEVID_HI), SK_XM_READ_2(sc_if, XM_DEVID_LO));
  
  	/*
  	 * Perform additional initialization for external PHYs,

==============================================================================

	After applying the patch and building and booting the kernel,
	the driver reports:

	    skc0: <SysKonnect Gigabit Ethernet (V1.0)> port 0x3000-0x30ff mem 0xdd300000-0xd
	    d303fff irq 48 at device 1.0 on pci3
	    skc0: SysKonnect SK-NET Gigabit Ethernet Adapter SK-9843 SX rev. (0x0)
	    sk0: <XaQti Corp. XMAC II> on skc0
	    sk0: Ethernet address: 00:00:5a:9a:80:18
	    sk0: XM_DEVID 0xae20 (0xe0/0xae20)

	Notice that SK_XM_READ_4() returned the same value as the
	low bytes SK_XM_READ_2() and that the high bytes have 0xe0,
	not zero.
Comment 1 Pyun YongHyeon freebsd_committer 2006-07-27 12:03:49 UTC
Responsible Changed
From-To: freebsd-bugs->yongari

I'll handle this.
Comment 2 Pyun YongHyeon freebsd_committer 2006-07-27 12:15:26 UTC
State Changed
From-To: open->feedback

XMAC documentation says that NP(node processor) interface supports 
both 16bit/32bit modes. Your results shows it uses 16bit mode only. 
I don't have SysKonnect SK-NET hardware so it's hard to verify this. 
As multicast filter setup uses SK_XM_WRITE_4 macros, I think 
multicasting filtering didn't work on your hardware without using 
two 16bit accesses. 
Would you verify that the hardware really works for multicasting 
packet after applying your patch?
Comment 3 Pyun YongHyeon freebsd_committer 2010-10-25 22:50:57 UTC
State Changed
From-To: feedback->closed

Close(feedback timed out). 
I double checked XaQti XQ11800FP 1000Mbps Gigabit Ethernet 
Controller data sheet and it clearly says NP supports both 16bits 
and 32bits operation. I also checked Linux and Vendor's driver and 
they also used 32bits register access for Genesis controllers. The 
only guess I have at this moment is you may have wrong access 
pattern to the register. To access 32bits quantity with two 16bits 
operation, you may have to access low address first and then high 
address. Just accessing high 16bits of a 32bit register would yield 
incorrect result. 
Please check data sheet to see which register supports both 16bits 
and 32bits and which register supports 16bits only(pp. 40).