sfc: Remove confusing MMIO functions

efx_writed_table() uses a step of 16 bytes but efx_readd_table() uses a step of 4 bytes. Why are they different? Firstly, register access is asymmetric: - The EVQ_RPTR table and RX_INDIRECTION_TBL can (or must?) be written as dwords even though they have a step size of 16 bytes, unlike most other CSRs. - In general, a read of any width is valid for registers, so long as it does not cross register boundaries. There is also no latching behaviour in the BIU, contrary to rumour. We write to the EVQ_RPTR table with efx_writed_table() but never read it back as it's write-only. We write to the RX_INDIRECTION_TBL with efx_writed_table(), but only read it back for the register dump, where we use efx_reado_table() as for any other table with step size of 16. We read MC_TREG_SMEM with efx_readd_table() for the register dump, but normally read and write it with efx_readd() and efx_writed() using offsets calculated in bytes. Since these functions are trivial and have few callers, it's clearer to open-code them at the call sites. While we're at it, update the comments on the BIU behaviour again. Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

sfc: Remove confusing MMIO functions
efx_writed_table() uses a step of 16 bytes but efx_readd_table() uses a step of 4 bytes. Why are they different? Firstly, register access is asymmetric: - The EVQ_RPTR table and RX_INDIRECTION_TBL can (or must?) be written as dwords even though they have a step size of 16 bytes, unlike most other CSRs. - In general, a read of any width is valid for registers, so long as it does not cross register boundaries. There is also no latching behaviour in the BIU, contrary to rumour. We write to the EVQ_RPTR table with efx_writed_table() but never read it back as it's write-only. We write to the RX_INDIRECTION_TBL with efx_writed_table(), but only read it back for the register dump, where we use efx_reado_table() as for any other table with step size of 16. We read MC_TREG_SMEM with efx_readd_table() for the register dump, but normally read and write it with efx_readd() and efx_writed() using offsets calculated in bytes. Since these functions are trivial and have few callers, it's clearer to open-code them at the call sites. While we're at it, update the comments on the BIU behaviour again. Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Ben Hutchings
1 parent bbec969b7f
Showing 3 changed files with 28 additions and 36 deletions Side-by-side Diff
drivers/net/ethernet/sfc/io.h
drivers/net/ethernet/sfc/nic.c
drivers/net/ethernet/sfc/siena_sriov.c
@@ -22,23 +22,22 @@
  *
  * Notes on locking strategy:
  *
- * Most CSRs are 128-bit (oword) and therefore cannot be read or
- * written atomically.  Access from the host is buffered by the Bus
- * Interface Unit (BIU).  Whenever the host reads from the lowest
- * address of such a register, or from the address of a different such
- * register, the BIU latches the register's value.  Subsequent reads
- * from higher addresses of the same register will read the latched
- * value.  Whenever the host writes part of such a register, the BIU
- * collects the written value and does not write to the underlying
- * register until all 4 dwords have been written.  A similar buffering
- * scheme applies to host access to the NIC's 64-bit SRAM.
+ * Many CSRs are very wide and cannot be read or written atomically.
+ * Writes from the host are buffered by the Bus Interface Unit (BIU)
+ * up to 128 bits.  Whenever the host writes part of such a register,
+ * the BIU collects the written value and does not write to the
+ * underlying register until all 4 dwords have been written.  A
+ * similar buffering scheme applies to host access to the NIC's 64-bit
+ * SRAM.
  *
- * Access to different CSRs and 64-bit SRAM words must be serialised,
- * since interleaved access can result in lost writes or lost
- * information from read-to-clear fields.  We use efx_nic::biu_lock
- * for this.  (We could use separate locks for read and write, but
- * this is not normally a performance bottleneck.)
+ * Writes to different CSRs and 64-bit SRAM words must be serialised,
+ * since interleaved access can result in lost writes.  We use
+ * efx_nic::biu_lock for this.
  *
+ * We also serialise reads from 128-bit CSRs and SRAM with the same
+ * spinlock.  This may not be necessary, but it doesn't really matter
+ * as there are no such reads on the fast path.
+ *
  * The DMA descriptor pointers (RX_DESC_UPD and TX_DESC_UPD) are
  * 128-bit but are special-cased in the BIU to avoid the need for
  * locking in the host:
@@ -202,20 +201,6 @@
 				     unsigned int reg, unsigned int index)
 {
 	efx_reado(efx, value, reg + index * sizeof(efx_oword_t));
-}
-
-/* Write a 32-bit CSR forming part of a table, or 32-bit SRAM */
-static inline void efx_writed_table(struct efx_nic *efx, efx_dword_t *value,
-				       unsigned int reg, unsigned int index)
-{
-	efx_writed(efx, value, reg + index * sizeof(efx_oword_t));
-}
-
-/* Read a 32-bit CSR forming part of a table, or 32-bit SRAM */
-static inline void efx_readd_table(struct efx_nic *efx, efx_dword_t *value,
-				   unsigned int reg, unsigned int index)
-{
-	efx_readd(efx, value, reg + index * sizeof(efx_dword_t));
 }
  
 /* Page-mapped register block size */
@@ -765,8 +765,13 @@
  
 	EFX_POPULATE_DWORD_1(reg, FRF_AZ_EVQ_RPTR,
 			     channel->eventq_read_ptr & channel->eventq_mask);
-	efx_writed_table(efx, &reg, efx->type->evq_rptr_tbl_base,
-			 channel->channel);
+
+	/* For Falcon A1, EVQ_RPTR_KER is documented as having a step size
+	 * of 4 bytes, but it is really 16 bytes just like later revisions.
+	 */
+	efx_writed(efx, &reg,
+		   efx->type->evq_rptr_tbl_base +
+		   FR_BZ_EVQ_RPTR_STEP * channel->channel);
 }
  
 /* Use HW to insert a SW defined event */
@@ -1564,7 +1569,9 @@
 	for (i = 0; i < FR_BZ_RX_INDIRECTION_TBL_ROWS; i++) {
 		EFX_POPULATE_DWORD_1(dword, FRF_BZ_IT_QUEUE,
 				     efx->rx_indir_table[i]);
-		efx_writed_table(efx, &dword, FR_BZ_RX_INDIRECTION_TBL, i);
+		efx_writed(efx, &dword,
+			   FR_BZ_RX_INDIRECTION_TBL +
+			   FR_BZ_RX_INDIRECTION_TBL_STEP * i);
 	}
 }
  
  
@@ -2028,15 +2035,15 @@
  
 		for (i = 0; i < table->rows; i++) {
 			switch (table->step) {
-			case 4: /* 32-bit register or SRAM */
-				efx_readd_table(efx, buf, table->offset, i);
+			case 4: /* 32-bit SRAM */
+				efx_readd(efx, buf, table->offset + 4 * i);
 				break;
 			case 8: /* 64-bit SRAM */
 				efx_sram_readq(efx,
 					       efx->membase + table->offset,
 					       buf, i);
 				break;
-			case 16: /* 128-bit register */
+			case 16: /* 128-bit-readable register */
 				efx_reado_table(efx, buf, table->offset, i);
 				break;
 			case 32: /* 128-bit register, interleaved */
@@ -993,7 +993,7 @@
 			     FRF_AZ_EVQ_BUF_BASE_ID, buftbl);
 	efx_writeo_table(efx, &reg, FR_BZ_EVQ_PTR_TBL, abs_evq);
 	EFX_POPULATE_DWORD_1(ptr, FRF_AZ_EVQ_RPTR, 0);
-	efx_writed_table(efx, &ptr, FR_BZ_EVQ_RPTR, abs_evq);
+	efx_writed(efx, &ptr, FR_BZ_EVQ_RPTR + FR_BZ_EVQ_RPTR_STEP * abs_evq);
  
 	mutex_unlock(&vf->status_lock);
 }
...	...	@@ -22,23 +22,22 @@
22	22	*
23	23	* Notes on locking strategy:
24	24	*
25		- * Most CSRs are 128-bit (oword) and therefore cannot be read or
26		- * written atomically. Access from the host is buffered by the Bus
27		- * Interface Unit (BIU). Whenever the host reads from the lowest
28		- * address of such a register, or from the address of a different such
29		- * register, the BIU latches the register's value. Subsequent reads
30		- * from higher addresses of the same register will read the latched
31		- * value. Whenever the host writes part of such a register, the BIU
32		- * collects the written value and does not write to the underlying
33		- * register until all 4 dwords have been written. A similar buffering
34		- * scheme applies to host access to the NIC's 64-bit SRAM.
	25	+ * Many CSRs are very wide and cannot be read or written atomically.
	26	+ * Writes from the host are buffered by the Bus Interface Unit (BIU)
	27	+ * up to 128 bits. Whenever the host writes part of such a register,
	28	+ * the BIU collects the written value and does not write to the
	29	+ * underlying register until all 4 dwords have been written. A
	30	+ * similar buffering scheme applies to host access to the NIC's 64-bit
	31	+ * SRAM.
35	32	*
36		- * Access to different CSRs and 64-bit SRAM words must be serialised,
37		- * since interleaved access can result in lost writes or lost
38		- * information from read-to-clear fields. We use efx_nic::biu_lock
39		- * for this. (We could use separate locks for read and write, but
40		- * this is not normally a performance bottleneck.)
	33	+ * Writes to different CSRs and 64-bit SRAM words must be serialised,
	34	+ * since interleaved access can result in lost writes. We use
	35	+ * efx_nic::biu_lock for this.
41	36	*
	37	+ * We also serialise reads from 128-bit CSRs and SRAM with the same
	38	+ * spinlock. This may not be necessary, but it doesn't really matter
	39	+ * as there are no such reads on the fast path.
	40	+ *
42	41	* The DMA descriptor pointers (RX_DESC_UPD and TX_DESC_UPD) are
43	42	* 128-bit but are special-cased in the BIU to avoid the need for
44	43	* locking in the host:
...	...	@@ -202,20 +201,6 @@
202	201	unsigned int reg, unsigned int index)
203	202	{
204	203	efx_reado(efx, value, reg + index * sizeof(efx_oword_t));
205		-}
206		-
207		-/* Write a 32-bit CSR forming part of a table, or 32-bit SRAM */
208		-static inline void efx_writed_table(struct efx_nic efx, efx_dword_t value,
209		- unsigned int reg, unsigned int index)
210		-{
211		- efx_writed(efx, value, reg + index * sizeof(efx_oword_t));
212		-}
213		-
214		-/* Read a 32-bit CSR forming part of a table, or 32-bit SRAM */
215		-static inline void efx_readd_table(struct efx_nic efx, efx_dword_t value,
216		- unsigned int reg, unsigned int index)
217		-{
218		- efx_readd(efx, value, reg + index * sizeof(efx_dword_t));
219	204	}
220	205
221	206	/* Page-mapped register block size */
...	...	@@ -765,8 +765,13 @@
765	765
766	766	EFX_POPULATE_DWORD_1(reg, FRF_AZ_EVQ_RPTR,
767	767	channel->eventq_read_ptr & channel->eventq_mask);
768		- efx_writed_table(efx, &reg, efx->type->evq_rptr_tbl_base,
769		- channel->channel);
	768	+
	769	+ /* For Falcon A1, EVQ_RPTR_KER is documented as having a step size
	770	+ * of 4 bytes, but it is really 16 bytes just like later revisions.
	771	+ */
	772	+ efx_writed(efx, &reg,
	773	+ efx->type->evq_rptr_tbl_base +
	774	+ FR_BZ_EVQ_RPTR_STEP * channel->channel);
770	775	}
771	776
772	777	/* Use HW to insert a SW defined event */
...	...	@@ -1564,7 +1569,9 @@
1564	1569	for (i = 0; i < FR_BZ_RX_INDIRECTION_TBL_ROWS; i++) {
1565	1570	EFX_POPULATE_DWORD_1(dword, FRF_BZ_IT_QUEUE,
1566	1571	efx->rx_indir_table[i]);
1567		- efx_writed_table(efx, &dword, FR_BZ_RX_INDIRECTION_TBL, i);
	1572	+ efx_writed(efx, &dword,
	1573	+ FR_BZ_RX_INDIRECTION_TBL +
	1574	+ FR_BZ_RX_INDIRECTION_TBL_STEP * i);
1568	1575	}
1569	1576	}
1570	1577
1571	1578
...	...	@@ -2028,15 +2035,15 @@
2028	2035
2029	2036	for (i = 0; i < table->rows; i++) {
2030	2037	switch (table->step) {
2031		- case 4: /* 32-bit register or SRAM */
2032		- efx_readd_table(efx, buf, table->offset, i);
	2038	+ case 4: /* 32-bit SRAM */
	2039	+ efx_readd(efx, buf, table->offset + 4 * i);
2033	2040	break;
2034	2041	case 8: /* 64-bit SRAM */
2035	2042	efx_sram_readq(efx,
2036	2043	efx->membase + table->offset,
2037	2044	buf, i);
2038	2045	break;
2039		- case 16: /* 128-bit register */
	2046	+ case 16: /* 128-bit-readable register */
2040	2047	efx_reado_table(efx, buf, table->offset, i);
2041	2048	break;
2042	2049	case 32: /* 128-bit register, interleaved */
...	...	@@ -993,7 +993,7 @@
993	993	FRF_AZ_EVQ_BUF_BASE_ID, buftbl);
994	994	efx_writeo_table(efx, &reg, FR_BZ_EVQ_PTR_TBL, abs_evq);
995	995	EFX_POPULATE_DWORD_1(ptr, FRF_AZ_EVQ_RPTR, 0);
996		- efx_writed_table(efx, &ptr, FR_BZ_EVQ_RPTR, abs_evq);
	996	+ efx_writed(efx, &ptr, FR_BZ_EVQ_RPTR + FR_BZ_EVQ_RPTR_STEP * abs_evq);
997	997
998	998	mutex_unlock(&vf->status_lock);
999	999	}