[PATCH 6/6] net: ks8851: Deduplicate register macros

[PATCH 6/6] net: ks8851: Deduplicate register macros

[Lukas Wunner]

The ks8851 chip is sold either with an SPI interface (KSZ8851SNL) or
with a so-called non-PCI interface (KSZ8851-16MLL).  When the driver
for the latter was introduced with commit a55c0a0ed415 ("drivers/net:
ks8851_mll ethernet network driver"), it duplicated the register macros
introduced by the driver for the former with commit 3ba81f3ece3c ("net:
Micrel KS8851 SPI network driver").

The chips are almost identical, so the duplication seems unwarranted.
There are a handful of bits which are in use on the KSZ8851-16MLL but
reserved on the KSZ8851SNL, and vice-versa, but there are no actual
collisions.

Thus, remove the duplicate definitions from the KSZ8851-16MLL driver.
Mark all bits which differ between the two chips.  Move the SPI frame
opcodes, which are specific to KSZ8851SNL, to its driver.

The KSZ8851-16MLL driver added a RXFCTR_THRESHOLD_MASK macro which is a
duplication of the RXFCTR_RXFCT_MASK macro, rename it where it's used.
Same for P1MBCR_FORCE_FDX, which duplicates the BMCR_FULLDPLX macro and
OBCR_ODS_16MA, which duplicates OBCR_ODS_16mA.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/ethernet/micrel/ks8851.c     |   6 +
 drivers/net/ethernet/micrel/ks8851.h     |  41 +--
 drivers/net/ethernet/micrel/ks8851_mll.c | 317 +----------------------
 3 files changed, 34 insertions(+), 330 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index b83b070a9eec..7849119d407a 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -142,6 +142,12 @@ struct ks8851_net {
 
 static int msg_enable;
 
+/* SPI frame opcodes */
+#define KS_SPIOP_RD	(0x00)
+#define KS_SPIOP_WR	(0x40)
+#define KS_SPIOP_RXFIFO	(0x80)
+#define KS_SPIOP_TXFIFO	(0xC0)
+
 /* shift for byte-enable data */
 #define BYTE_EN(_x)	((_x) << 2)
 
diff --git a/drivers/net/ethernet/micrel/ks8851.h b/drivers/net/ethernet/micrel/ks8851.h
index 1050fa48b73c..23da1e3ee429 100644
--- a/drivers/net/ethernet/micrel/ks8851.h
+++ b/drivers/net/ethernet/micrel/ks8851.h
@@ -11,9 +11,15 @@
 */
 
 #define KS_CCR					0x08
+#define CCR_LE					(1 << 10)   /* KSZ8851-16MLL */
 #define CCR_EEPROM				(1 << 9)
-#define CCR_SPI					(1 << 8)
-#define CCR_32PIN				(1 << 0)
+#define CCR_SPI					(1 << 8)    /* KSZ8851SNL    */
+#define CCR_8BIT				(1 << 7)    /* KSZ8851-16MLL */
+#define CCR_16BIT				(1 << 6)    /* KSZ8851-16MLL */
+#define CCR_32BIT				(1 << 5)    /* KSZ8851-16MLL */
+#define CCR_SHARED				(1 << 4)    /* KSZ8851-16MLL */
+#define CCR_48PIN				(1 << 1)    /* KSZ8851-16MLL */
+#define CCR_32PIN				(1 << 0)    /* KSZ8851SNL    */
 
 /* MAC address registers */
 #define KS_MAR(_m)				(0x15 - (_m))
@@ -112,13 +118,13 @@
 #define RXCR1_RXE				(1 << 0)
 
 #define KS_RXCR2				0x76
-#define RXCR2_SRDBL_MASK			(0x7 << 5)
-#define RXCR2_SRDBL_SHIFT			(5)
-#define RXCR2_SRDBL_4B				(0x0 << 5)
-#define RXCR2_SRDBL_8B				(0x1 << 5)
-#define RXCR2_SRDBL_16B				(0x2 << 5)
-#define RXCR2_SRDBL_32B				(0x3 << 5)
-#define RXCR2_SRDBL_FRAME			(0x4 << 5)
+#define RXCR2_SRDBL_MASK			(0x7 << 5)  /* KSZ8851SNL    */
+#define RXCR2_SRDBL_SHIFT			(5)	    /* KSZ8851SNL    */
+#define RXCR2_SRDBL_4B				(0x0 << 5)  /* KSZ8851SNL    */
+#define RXCR2_SRDBL_8B				(0x1 << 5)  /* KSZ8851SNL    */
+#define RXCR2_SRDBL_16B				(0x2 << 5)  /* KSZ8851SNL    */
+#define RXCR2_SRDBL_32B				(0x3 << 5)  /* KSZ8851SNL    */
+#define RXCR2_SRDBL_FRAME			(0x4 << 5)  /* KSZ8851SNL    */
 #define RXCR2_IUFFP				(1 << 4)
 #define RXCR2_RXIUFCEZ				(1 << 3)
 #define RXCR2_UDPLFE				(1 << 2)
@@ -143,8 +149,10 @@
 #define RXFSHR_RXCE				(1 << 0)
 
 #define KS_RXFHBCR				0x7E
+#define RXFHBCR_CNT_MASK			(0xfff << 0)
+
 #define KS_TXQCR				0x80
-#define TXQCR_AETFE				(1 << 2)
+#define TXQCR_AETFE				(1 << 2)    /* KSZ8851SNL    */
 #define TXQCR_TXQMAM				(1 << 1)
 #define TXQCR_METFE				(1 << 0)
 
@@ -167,6 +175,10 @@
 
 #define KS_RXFDPR				0x86
 #define RXFDPR_RXFPAI				(1 << 14)
+#define RXFDPR_WST				(1 << 12)   /* KSZ8851-16MLL */
+#define RXFDPR_EMS				(1 << 11)   /* KSZ8851-16MLL */
+#define RXFDPR_RXFP_MASK			(0x7ff << 0)
+#define RXFDPR_RXFP_SHIFT			(0)
 
 #define KS_RXDTTR				0x8C
 #define KS_RXDBCTR				0x8E
@@ -184,7 +196,7 @@
 #define IRQ_RXMPDI				(1 << 4)
 #define IRQ_LDI					(1 << 3)
 #define IRQ_EDI					(1 << 2)
-#define IRQ_SPIBEI				(1 << 1)
+#define IRQ_SPIBEI				(1 << 1)    /* KSZ8851SNL    */
 #define IRQ_DEDI				(1 << 0)
 
 #define KS_RXFCTR				0x9C
@@ -288,13 +300,6 @@
 #define P1SR_PNTR_10BT_HDX			(1 << 0)
 
 /* TX Frame control */
-
 #define TXFR_TXIC				(1 << 15)
 #define TXFR_TXFID_MASK				(0x3f << 0)
 #define TXFR_TXFID_SHIFT			(0)
-
-/* SPI frame opcodes */
-#define KS_SPIOP_RD				(0x00)
-#define KS_SPIOP_WR				(0x40)
-#define KS_SPIOP_RXFIFO				(0x80)
-#define KS_SPIOP_TXFIFO				(0xC0)
diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
index 35f8c9ef204d..c946841c0a06 100644
--- a/drivers/net/ethernet/micrel/ks8851_mll.c
+++ b/drivers/net/ethernet/micrel/ks8851_mll.c
@@ -40,6 +40,8 @@
 #include <linux/of_device.h>
 #include <linux/of_net.h>
 
+#include "ks8851.h"
+
 #define	DRV_NAME	"ks8851_mll"
 
 static u8 KS_DEFAULT_MAC_ADDRESS[] = { 0x00, 0x10, 0xA1, 0x86, 0x95, 0x11 };
@@ -48,319 +50,10 @@ static u8 KS_DEFAULT_MAC_ADDRESS[] = { 0x00, 0x10, 0xA1, 0x86, 0x95, 0x11 };
 #define TX_BUF_SIZE			2000
 #define RX_BUF_SIZE			2000
 
-#define KS_CCR				0x08
-#define CCR_EEPROM			(1 << 9)
-#define CCR_SPI				(1 << 8)
-#define CCR_8BIT			(1 << 7)
-#define CCR_16BIT			(1 << 6)
-#define CCR_32BIT			(1 << 5)
-#define CCR_SHARED			(1 << 4)
-#define CCR_32PIN			(1 << 0)
-
-/* MAC address registers */
-#define KS_MARL				0x10
-#define KS_MARM				0x12
-#define KS_MARH				0x14
-
-#define KS_OBCR				0x20
-#define OBCR_ODS_16MA			(1 << 6)
-
-#define KS_EEPCR			0x22
-#define EEPCR_EESA			(1 << 4)
-#define EEPCR_EESB			(1 << 3)
-#define EEPCR_EEDO			(1 << 2)
-#define EEPCR_EESCK			(1 << 1)
-#define EEPCR_EECS			(1 << 0)
-
-#define KS_MBIR				0x24
-#define MBIR_TXMBF			(1 << 12)
-#define MBIR_TXMBFA			(1 << 11)
-#define MBIR_RXMBF			(1 << 4)
-#define MBIR_RXMBFA			(1 << 3)
-
-#define KS_GRR				0x26
-#define GRR_QMU				(1 << 1)
-#define GRR_GSR				(1 << 0)
-
-#define KS_WFCR				0x2A
-#define WFCR_MPRXE			(1 << 7)
-#define WFCR_WF3E			(1 << 3)
-#define WFCR_WF2E			(1 << 2)
-#define WFCR_WF1E			(1 << 1)
-#define WFCR_WF0E			(1 << 0)
-
-#define KS_WF0CRC0			0x30
-#define KS_WF0CRC1			0x32
-#define KS_WF0BM0			0x34
-#define KS_WF0BM1			0x36
-#define KS_WF0BM2			0x38
-#define KS_WF0BM3			0x3A
-
-#define KS_WF1CRC0			0x40
-#define KS_WF1CRC1			0x42
-#define KS_WF1BM0			0x44
-#define KS_WF1BM1			0x46
-#define KS_WF1BM2			0x48
-#define KS_WF1BM3			0x4A
-
-#define KS_WF2CRC0			0x50
-#define KS_WF2CRC1			0x52
-#define KS_WF2BM0			0x54
-#define KS_WF2BM1			0x56
-#define KS_WF2BM2			0x58
-#define KS_WF2BM3			0x5A
-
-#define KS_WF3CRC0			0x60
-#define KS_WF3CRC1			0x62
-#define KS_WF3BM0			0x64
-#define KS_WF3BM1			0x66
-#define KS_WF3BM2			0x68
-#define KS_WF3BM3			0x6A
-
-#define KS_TXCR				0x70
-#define TXCR_TCGICMP			(1 << 8)
-#define TXCR_TCGUDP			(1 << 7)
-#define TXCR_TCGTCP			(1 << 6)
-#define TXCR_TCGIP			(1 << 5)
-#define TXCR_FTXQ			(1 << 4)
-#define TXCR_TXFCE			(1 << 3)
-#define TXCR_TXPE			(1 << 2)
-#define TXCR_TXCRC			(1 << 1)
-#define TXCR_TXE			(1 << 0)
-
-#define KS_TXSR				0x72
-#define TXSR_TXLC			(1 << 13)
-#define TXSR_TXMC			(1 << 12)
-#define TXSR_TXFID_MASK			(0x3f << 0)
-#define TXSR_TXFID_SHIFT		(0)
-#define TXSR_TXFID_GET(_v)		(((_v) >> 0) & 0x3f)
-
-
-#define KS_RXCR1			0x74
-#define RXCR1_FRXQ			(1 << 15)
-#define RXCR1_RXUDPFCC			(1 << 14)
-#define RXCR1_RXTCPFCC			(1 << 13)
-#define RXCR1_RXIPFCC			(1 << 12)
-#define RXCR1_RXPAFMA			(1 << 11)
-#define RXCR1_RXFCE			(1 << 10)
-#define RXCR1_RXEFE			(1 << 9)
-#define RXCR1_RXMAFMA			(1 << 8)
-#define RXCR1_RXBE			(1 << 7)
-#define RXCR1_RXME			(1 << 6)
-#define RXCR1_RXUE			(1 << 5)
-#define RXCR1_RXAE			(1 << 4)
-#define RXCR1_RXINVF			(1 << 1)
-#define RXCR1_RXE			(1 << 0)
 #define RXCR1_FILTER_MASK    		(RXCR1_RXINVF | RXCR1_RXAE | \
 					 RXCR1_RXMAFMA | RXCR1_RXPAFMA)
-
-#define KS_RXCR2			0x76
-#define RXCR2_SRDBL_MASK		(0x7 << 5)
-#define RXCR2_SRDBL_SHIFT		(5)
-#define RXCR2_SRDBL_4B			(0x0 << 5)
-#define RXCR2_SRDBL_8B			(0x1 << 5)
-#define RXCR2_SRDBL_16B			(0x2 << 5)
-#define RXCR2_SRDBL_32B			(0x3 << 5)
-/* #define RXCR2_SRDBL_FRAME		(0x4 << 5) */
-#define RXCR2_IUFFP			(1 << 4)
-#define RXCR2_RXIUFCEZ			(1 << 3)
-#define RXCR2_UDPLFE			(1 << 2)
-#define RXCR2_RXICMPFCC			(1 << 1)
-#define RXCR2_RXSAF			(1 << 0)
-
-#define KS_TXMIR			0x78
-
-#define KS_RXFHSR			0x7C
-#define RXFSHR_RXFV			(1 << 15)
-#define RXFSHR_RXICMPFCS		(1 << 13)
-#define RXFSHR_RXIPFCS			(1 << 12)
-#define RXFSHR_RXTCPFCS			(1 << 11)
-#define RXFSHR_RXUDPFCS			(1 << 10)
-#define RXFSHR_RXBF			(1 << 7)
-#define RXFSHR_RXMF			(1 << 6)
-#define RXFSHR_RXUF			(1 << 5)
-#define RXFSHR_RXMR			(1 << 4)
-#define RXFSHR_RXFT			(1 << 3)
-#define RXFSHR_RXFTL			(1 << 2)
-#define RXFSHR_RXRF			(1 << 1)
-#define RXFSHR_RXCE			(1 << 0)
-#define	RXFSHR_ERR			(RXFSHR_RXCE | RXFSHR_RXRF |\
-					RXFSHR_RXFTL | RXFSHR_RXMR |\
-					RXFSHR_RXICMPFCS | RXFSHR_RXIPFCS |\
-					RXFSHR_RXTCPFCS)
-#define KS_RXFHBCR			0x7E
-#define RXFHBCR_CNT_MASK		0x0FFF
-
-#define KS_TXQCR			0x80
-#define TXQCR_AETFE			(1 << 2)
-#define TXQCR_TXQMAM			(1 << 1)
-#define TXQCR_METFE			(1 << 0)
-
-#define KS_RXQCR			0x82
-#define RXQCR_RXDTTS			(1 << 12)
-#define RXQCR_RXDBCTS			(1 << 11)
-#define RXQCR_RXFCTS			(1 << 10)
-#define RXQCR_RXIPHTOE			(1 << 9)
-#define RXQCR_RXDTTE			(1 << 7)
-#define RXQCR_RXDBCTE			(1 << 6)
-#define RXQCR_RXFCTE			(1 << 5)
-#define RXQCR_ADRFE			(1 << 4)
-#define RXQCR_SDA			(1 << 3)
-#define RXQCR_RRXEF			(1 << 0)
 #define RXQCR_CMD_CNTL                	(RXQCR_RXFCTE|RXQCR_ADRFE)
 
-#define KS_TXFDPR			0x84
-#define TXFDPR_TXFPAI			(1 << 14)
-#define TXFDPR_TXFP_MASK		(0x7ff << 0)
-#define TXFDPR_TXFP_SHIFT		(0)
-
-#define KS_RXFDPR			0x86
-#define RXFDPR_RXFPAI			(1 << 14)
-
-#define KS_RXDTTR			0x8C
-#define KS_RXDBCTR			0x8E
-
-#define KS_IER				0x90
-#define KS_ISR				0x92
-#define IRQ_LCI				(1 << 15)
-#define IRQ_TXI				(1 << 14)
-#define IRQ_RXI				(1 << 13)
-#define IRQ_RXOI			(1 << 11)
-#define IRQ_TXPSI			(1 << 9)
-#define IRQ_RXPSI			(1 << 8)
-#define IRQ_TXSAI			(1 << 6)
-#define IRQ_RXWFDI			(1 << 5)
-#define IRQ_RXMPDI			(1 << 4)
-#define IRQ_LDI				(1 << 3)
-#define IRQ_EDI				(1 << 2)
-#define IRQ_SPIBEI			(1 << 1)
-#define IRQ_DEDI			(1 << 0)
-
-#define KS_RXFCTR			0x9C
-#define RXFCTR_THRESHOLD_MASK     	0x00FF
-
-#define KS_RXFC				0x9D
-#define RXFCTR_RXFC_MASK		(0xff << 8)
-#define RXFCTR_RXFC_SHIFT		(8)
-#define RXFCTR_RXFC_GET(_v)		(((_v) >> 8) & 0xff)
-#define RXFCTR_RXFCT_MASK		(0xff << 0)
-#define RXFCTR_RXFCT_SHIFT		(0)
-
-#define KS_TXNTFSR			0x9E
-
-#define KS_MAHTR0			0xA0
-#define KS_MAHTR1			0xA2
-#define KS_MAHTR2			0xA4
-#define KS_MAHTR3			0xA6
-
-#define KS_FCLWR			0xB0
-#define KS_FCHWR			0xB2
-#define KS_FCOWR			0xB4
-
-#define KS_CIDER			0xC0
-#define CIDER_ID			0x8870
-#define CIDER_REV_MASK			(0x7 << 1)
-#define CIDER_REV_SHIFT			(1)
-#define CIDER_REV_GET(_v)		(((_v) >> 1) & 0x7)
-
-#define KS_CGCR				0xC6
-#define KS_IACR				0xC8
-#define IACR_RDEN			(1 << 12)
-#define IACR_TSEL_MASK			(0x3 << 10)
-#define IACR_TSEL_SHIFT			(10)
-#define IACR_TSEL_MIB			(0x3 << 10)
-#define IACR_ADDR_MASK			(0x1f << 0)
-#define IACR_ADDR_SHIFT			(0)
-
-#define KS_IADLR			0xD0
-#define KS_IAHDR			0xD2
-
-#define KS_PMECR			0xD4
-#define PMECR_PME_DELAY			(1 << 14)
-#define PMECR_PME_POL			(1 << 12)
-#define PMECR_WOL_WAKEUP		(1 << 11)
-#define PMECR_WOL_MAGICPKT		(1 << 10)
-#define PMECR_WOL_LINKUP		(1 << 9)
-#define PMECR_WOL_ENERGY		(1 << 8)
-#define PMECR_AUTO_WAKE_EN		(1 << 7)
-#define PMECR_WAKEUP_NORMAL		(1 << 6)
-#define PMECR_WKEVT_MASK		(0xf << 2)
-#define PMECR_WKEVT_SHIFT		(2)
-#define PMECR_WKEVT_GET(_v)		(((_v) >> 2) & 0xf)
-#define PMECR_WKEVT_ENERGY		(0x1 << 2)
-#define PMECR_WKEVT_LINK		(0x2 << 2)
-#define PMECR_WKEVT_MAGICPKT		(0x4 << 2)
-#define PMECR_WKEVT_FRAME		(0x8 << 2)
-#define PMECR_PM_MASK			(0x3 << 0)
-#define PMECR_PM_SHIFT			(0)
-#define PMECR_PM_NORMAL			(0x0 << 0)
-#define PMECR_PM_ENERGY			(0x1 << 0)
-#define PMECR_PM_SOFTDOWN		(0x2 << 0)
-#define PMECR_PM_POWERSAVE		(0x3 << 0)
-
-/* Standard MII PHY data */
-#define KS_P1MBCR			0xE4
-#define P1MBCR_FORCE_FDX		(1 << 8)
-
-#define KS_P1MBSR			0xE6
-#define P1MBSR_AN_COMPLETE		(1 << 5)
-#define P1MBSR_AN_CAPABLE		(1 << 3)
-#define P1MBSR_LINK_UP			(1 << 2)
-
-#define KS_PHY1ILR			0xE8
-#define KS_PHY1IHR			0xEA
-#define KS_P1ANAR			0xEC
-#define KS_P1ANLPR			0xEE
-
-#define KS_P1SCLMD			0xF4
-#define P1SCLMD_LEDOFF			(1 << 15)
-#define P1SCLMD_TXIDS			(1 << 14)
-#define P1SCLMD_RESTARTAN		(1 << 13)
-#define P1SCLMD_DISAUTOMDIX		(1 << 10)
-#define P1SCLMD_FORCEMDIX		(1 << 9)
-#define P1SCLMD_AUTONEGEN		(1 << 7)
-#define P1SCLMD_FORCE100		(1 << 6)
-#define P1SCLMD_FORCEFDX		(1 << 5)
-#define P1SCLMD_ADV_FLOW		(1 << 4)
-#define P1SCLMD_ADV_100BT_FDX		(1 << 3)
-#define P1SCLMD_ADV_100BT_HDX		(1 << 2)
-#define P1SCLMD_ADV_10BT_FDX		(1 << 1)
-#define P1SCLMD_ADV_10BT_HDX		(1 << 0)
-
-#define KS_P1CR				0xF6
-#define P1CR_HP_MDIX			(1 << 15)
-#define P1CR_REV_POL			(1 << 13)
-#define P1CR_OP_100M			(1 << 10)
-#define P1CR_OP_FDX			(1 << 9)
-#define P1CR_OP_MDI			(1 << 7)
-#define P1CR_AN_DONE			(1 << 6)
-#define P1CR_LINK_GOOD			(1 << 5)
-#define P1CR_PNTR_FLOW			(1 << 4)
-#define P1CR_PNTR_100BT_FDX		(1 << 3)
-#define P1CR_PNTR_100BT_HDX		(1 << 2)
-#define P1CR_PNTR_10BT_FDX		(1 << 1)
-#define P1CR_PNTR_10BT_HDX		(1 << 0)
-
-/* TX Frame control */
-
-#define TXFR_TXIC			(1 << 15)
-#define TXFR_TXFID_MASK			(0x3f << 0)
-#define TXFR_TXFID_SHIFT		(0)
-
-#define KS_P1SR				0xF8
-#define P1SR_HP_MDIX			(1 << 15)
-#define P1SR_REV_POL			(1 << 13)
-#define P1SR_OP_100M			(1 << 10)
-#define P1SR_OP_FDX			(1 << 9)
-#define P1SR_OP_MDI			(1 << 7)
-#define P1SR_AN_DONE			(1 << 6)
-#define P1SR_LINK_GOOD			(1 << 5)
-#define P1SR_PNTR_FLOW			(1 << 4)
-#define P1SR_PNTR_100BT_FDX		(1 << 3)
-#define P1SR_PNTR_100BT_HDX		(1 << 2)
-#define P1SR_PNTR_10BT_FDX		(1 << 1)
-#define P1SR_PNTR_10BT_HDX		(1 << 0)
-
 #define	ENUM_BUS_NONE			0
 #define	ENUM_BUS_8BIT			1
 #define	ENUM_BUS_16BIT			2
@@ -1475,7 +1168,7 @@ static void ks_setup(struct ks_net *ks)
 	ks_wrreg16(ks, KS_RXFDPR, RXFDPR_RXFPAI);
 
 	/* Setup Receive Frame Threshold - 1 frame (RXFCTFC) */
-	ks_wrreg16(ks, KS_RXFCTR, 1 & RXFCTR_THRESHOLD_MASK);
+	ks_wrreg16(ks, KS_RXFCTR, 1 & RXFCTR_RXFCT_MASK);
 
 	/* Setup RxQ Command Control (RXQCR) */
 	ks->rc_rxqcr = RXQCR_CMD_CNTL;
@@ -1488,7 +1181,7 @@ static void ks_setup(struct ks_net *ks)
 	 */
 
 	w = ks_rdreg16(ks, KS_P1MBCR);
-	w &= ~P1MBCR_FORCE_FDX;
+	w &= ~BMCR_FULLDPLX;
 	ks_wrreg16(ks, KS_P1MBCR, w);
 
 	w = TXCR_TXFCE | TXCR_TXPE | TXCR_TXCRC | TXCR_TCGIP;
@@ -1629,7 +1322,7 @@ static int ks8851_probe(struct platform_device *pdev)
 	ks_setup_int(ks);
 
 	data = ks_rdreg16(ks, KS_OBCR);
-	ks_wrreg16(ks, KS_OBCR, data | OBCR_ODS_16MA);
+	ks_wrreg16(ks, KS_OBCR, data | OBCR_ODS_16mA);
 
 	/* overwriting the default MAC address */
 	if (pdev->dev.of_node) {
-- 
2.20.1

[PATCH 0/6] ks8851 fixes & cleanups

[Lukas Wunner]

Four fixes and two cleanups for the Microchip (formerly Micrel) KSZ8851
SPI Ethernet driver.

Some of the fixes might even pass as stable material, but I haven't marked
them as such for cautiousness: Doesn't hurt letting them bake in linux-next
for a few weeks to raise the confidence, even though we've tested them
extensively on our Revolution Pi open source PLCs.

Thanks,

Lukas

Lukas Wunner (6):
  net: ks8851: Dequeue RX packets explicitly
  net: ks8851: Reassert reset pin if chip ID check fails
  net: ks8851: Delay requesting IRQ until opened
  net: ks8851: Set initial carrier state to down
  net: ks8851: Fix register macro misnomers
  net: ks8851: Deduplicate register macros

 drivers/net/ethernet/micrel/ks8851.c     |  42 +--
 drivers/net/ethernet/micrel/ks8851.h     |  93 ++++---
 drivers/net/ethernet/micrel/ks8851_mll.c | 317 +----------------------
 3 files changed, 79 insertions(+), 373 deletions(-)

-- 
2.20.1

[PATCH 2/6] net: ks8851: Reassert reset pin if chip ID check fails

[Lukas Wunner]

Commit 73fdeb82e963 ("net: ks8851: Add optional vdd_io regulator and
reset gpio") amended the ks8851 driver to briefly assert the chip's
reset pin on probe. It also amended the probe routine's error path to
reassert the reset pin if a subsequent initialization step fails.

However the commit misplaced reassertion of the reset pin in the error
path such that it is not performed if the check of the Chip ID and
Enable Register (CIDER) fails. The error path is therefore slightly
asymmetrical to the probe routine's body. Fix it.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Nishanth Menon <nm@ti.com>
---
 drivers/net/ethernet/micrel/ks8851.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index a93f8e842c07..1633fa5c709c 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1554,9 +1554,9 @@ static int ks8851_probe(struct spi_device *spi)
 	free_irq(ndev->irq, ks);
 
 err_irq:
+err_id:
 	if (gpio_is_valid(gpio))
 		gpio_set_value(gpio, 0);
-err_id:
 	regulator_disable(ks->vdd_reg);
 err_reg:
 	regulator_disable(ks->vdd_io);
-- 
2.20.1

[PATCH 1/6] net: ks8851: Dequeue RX packets explicitly

[Lukas Wunner]

The ks8851 driver lets the chip auto-dequeue received packets once they
have been read in full. It achieves that by setting the ADRFE flag in
the RXQCR register ("Auto-Dequeue RXQ Frame Enable").

However if allocation of a packet's socket buffer or retrieval of the
packet over the SPI bus fails, the packet will not have been read in
full and is not auto-dequeued. Such partial retrieval of a packet
confuses the chip's RX queue management:  On the next RX interrupt,
the first packet read from the queue will be the one left there
previously and this one can be retrieved without issues. But for any
newly received packets, the frame header status and byte count registers
(RXFHSR and RXFHBCR) contain bogus values, preventing their retrieval.

The chip allows explicitly dequeueing a packet from the RX queue by
setting the RRXEF flag in the RXQCR register ("Release RX Error Frame").
This could be used to dequeue the packet in case of an error, but if
that error is a failed SPI transfer, it is unknown if the packet was
transferred in full and was auto-dequeued or if it was only transferred
in part and requires an explicit dequeue. The safest approach is thus
to always dequeue packets explicitly and forgo auto-dequeueing.

Without this change, I've witnessed packet retrieval break completely
when an SPI DMA transfer fails, requiring a chip reset. Explicit
dequeueing magically fixes this and makes packet retrieval absolutely
robust for me.

The chip's documentation suggests auto-dequeuing and uses the RRXEF
flag only to dequeue error frames which the driver doesn't want to
retrieve. But that seems to be a fair-weather approach.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/ethernet/micrel/ks8851.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index bd6e9014bc74..a93f8e842c07 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -535,9 +535,8 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 		/* set dma read address */
 		ks8851_wrreg16(ks, KS_RXFDPR, RXFDPR_RXFPAI | 0x00);
 
-		/* start the packet dma process, and set auto-dequeue rx */
-		ks8851_wrreg16(ks, KS_RXQCR,
-			       ks->rc_rxqcr | RXQCR_SDA | RXQCR_ADRFE);
+		/* start DMA access */
+		ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_SDA);
 
 		if (rxlen > 4) {
 			unsigned int rxalign;
@@ -568,7 +567,8 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
 			}
 		}
 
-		ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr);
+		/* end DMA access and dequeue packet */
+		ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_RRXEF);
 	}
 }
 
-- 
2.20.1

[PATCH 3/6] net: ks8851: Delay requesting IRQ until opened

[Lukas Wunner]

The ks8851 driver currently requests the IRQ before registering the
net_device.  Because the net_device name is used as IRQ name and is
still "eth%d" when the IRQ is requested, it's impossibe to tell IRQs
apart if multiple ks8851 chips are present.  Most other drivers delay
requesting the IRQ until the net_device is opened.  Do the same.

The driver doesn't enable interrupts on the chip before opening the
net_device and disables them when closing it, so there doesn't seem to
be a need to request the IRQ already on probe.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/ethernet/micrel/ks8851.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index 1633fa5c709c..c9faec4c5b25 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -785,6 +785,15 @@ static void ks8851_tx_work(struct work_struct *work)
 static int ks8851_net_open(struct net_device *dev)
 {
 	struct ks8851_net *ks = netdev_priv(dev);
+	int ret;
+
+	ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
+				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				   dev->name, ks);
+	if (ret < 0) {
+		netdev_err(dev, "failed to get irq\n");
+		return ret;
+	}
 
 	/* lock the card, even if we may not actually be doing anything
 	 * else at the moment */
@@ -899,6 +908,8 @@ static int ks8851_net_stop(struct net_device *dev)
 		dev_kfree_skb(txb);
 	}
 
+	free_irq(dev->irq, ks);
+
 	return 0;
 }
 
@@ -1529,14 +1540,6 @@ static int ks8851_probe(struct spi_device *spi)
 	ks8851_read_selftest(ks);
 	ks8851_init_mac(ks);
 
-	ret = request_threaded_irq(spi->irq, NULL, ks8851_irq,
-				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-				   ndev->name, ks);
-	if (ret < 0) {
-		dev_err(&spi->dev, "failed to get irq\n");
-		goto err_irq;
-	}
-
 	ret = register_netdev(ndev);
 	if (ret) {
 		dev_err(&spi->dev, "failed to register network device\n");
@@ -1549,11 +1552,7 @@ static int ks8851_probe(struct spi_device *spi)
 
 	return 0;
 
-
 err_netdev:
-	free_irq(ndev->irq, ks);
-
-err_irq:
 err_id:
 	if (gpio_is_valid(gpio))
 		gpio_set_value(gpio, 0);
@@ -1574,7 +1573,6 @@ static int ks8851_remove(struct spi_device *spi)
 		dev_info(&spi->dev, "remove\n");
 
 	unregister_netdev(priv->netdev);
-	free_irq(spi->irq, priv);
 	if (gpio_is_valid(priv->gpio))
 		gpio_set_value(priv->gpio, 0);
 	regulator_disable(priv->vdd_reg);
-- 
2.20.1

[PATCH 4/6] net: ks8851: Set initial carrier state to down

[Lukas Wunner]

The ks8851 chip's initial carrier state is down. A Link Change Interrupt
is signaled once interrupts are enabled if the carrier is up.

The ks8851 driver has it backwards by assuming that the initial carrier
state is up. The state is therefore misrepresented if the interface is
opened with no cable attached. Fix it.

The Link Change interrupt is sometimes not signaled unless the P1MBSR
register (which contains the Link Status bit) is read on ->ndo_open().
This might be a hardware erratum. Read the register by calling
mii_check_link(), which has the desirable side effect of setting the
carrier state to down if the cable was detached while the interface was
closed.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/ethernet/micrel/ks8851.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index c9faec4c5b25..b83b070a9eec 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -858,6 +858,7 @@ static int ks8851_net_open(struct net_device *dev)
 	netif_dbg(ks, ifup, ks->netdev, "network device up\n");
 
 	mutex_unlock(&ks->lock);
+	mii_check_link(&ks->mii);
 	return 0;
 }
 
@@ -1519,6 +1520,7 @@ static int ks8851_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, ks);
 
+	netif_carrier_off(ks->netdev);
 	ndev->if_port = IF_PORT_100BASET;
 	ndev->netdev_ops = &ks8851_netdev_ops;
 	ndev->irq = spi->irq;
-- 
2.20.1

[PATCH 5/6] net: ks8851: Fix register macro misnomers

[Lukas Wunner]

In the header file accompanying the ks8851 driver, the P1SCLMD register
macros are misnamed, they actually pertain to the P1CR register.

The P1CR macros in turn pertain to the P1SR register, see pages 65 to 68
of the spec:
http://www.hqchip.com/uploads/pdf/201703/47c98946d6c97a4766e14db3f24955f2.pdf

The misnomers have no negative consequences so far because the macros
aren't used by ks8851.c, but that's about to change.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/ethernet/micrel/ks8851.h | 52 +++++++++++++++-------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.h b/drivers/net/ethernet/micrel/ks8851.h
index 852256ef1f22..1050fa48b73c 100644
--- a/drivers/net/ethernet/micrel/ks8851.h
+++ b/drivers/net/ethernet/micrel/ks8851.h
@@ -257,33 +257,35 @@
 #define KS_P1ANLPR				0xEE
 
 #define KS_P1SCLMD				0xF4
-#define P1SCLMD_LEDOFF				(1 << 15)
-#define P1SCLMD_TXIDS				(1 << 14)
-#define P1SCLMD_RESTARTAN			(1 << 13)
-#define P1SCLMD_DISAUTOMDIX			(1 << 10)
-#define P1SCLMD_FORCEMDIX			(1 << 9)
-#define P1SCLMD_AUTONEGEN			(1 << 7)
-#define P1SCLMD_FORCE100			(1 << 6)
-#define P1SCLMD_FORCEFDX			(1 << 5)
-#define P1SCLMD_ADV_FLOW			(1 << 4)
-#define P1SCLMD_ADV_100BT_FDX			(1 << 3)
-#define P1SCLMD_ADV_100BT_HDX			(1 << 2)
-#define P1SCLMD_ADV_10BT_FDX			(1 << 1)
-#define P1SCLMD_ADV_10BT_HDX			(1 << 0)
 
 #define KS_P1CR					0xF6
-#define P1CR_HP_MDIX				(1 << 15)
-#define P1CR_REV_POL				(1 << 13)
-#define P1CR_OP_100M				(1 << 10)
-#define P1CR_OP_FDX				(1 << 9)
-#define P1CR_OP_MDI				(1 << 7)
-#define P1CR_AN_DONE				(1 << 6)
-#define P1CR_LINK_GOOD				(1 << 5)
-#define P1CR_PNTR_FLOW				(1 << 4)
-#define P1CR_PNTR_100BT_FDX			(1 << 3)
-#define P1CR_PNTR_100BT_HDX			(1 << 2)
-#define P1CR_PNTR_10BT_FDX			(1 << 1)
-#define P1CR_PNTR_10BT_HDX			(1 << 0)
+#define P1CR_LEDOFF				(1 << 15)
+#define P1CR_TXIDS				(1 << 14)
+#define P1CR_RESTARTAN				(1 << 13)
+#define P1CR_DISAUTOMDIX			(1 << 10)
+#define P1CR_FORCEMDIX				(1 << 9)
+#define P1CR_AUTONEGEN				(1 << 7)
+#define P1CR_FORCE100				(1 << 6)
+#define P1CR_FORCEFDX				(1 << 5)
+#define P1CR_ADV_FLOW				(1 << 4)
+#define P1CR_ADV_100BT_FDX			(1 << 3)
+#define P1CR_ADV_100BT_HDX			(1 << 2)
+#define P1CR_ADV_10BT_FDX			(1 << 1)
+#define P1CR_ADV_10BT_HDX			(1 << 0)
+
+#define KS_P1SR					0xF8
+#define P1SR_HP_MDIX				(1 << 15)
+#define P1SR_REV_POL				(1 << 13)
+#define P1SR_OP_100M				(1 << 10)
+#define P1SR_OP_FDX				(1 << 9)
+#define P1SR_OP_MDI				(1 << 7)
+#define P1SR_AN_DONE				(1 << 6)
+#define P1SR_LINK_GOOD				(1 << 5)
+#define P1SR_PNTR_FLOW				(1 << 4)
+#define P1SR_PNTR_100BT_FDX			(1 << 3)
+#define P1SR_PNTR_100BT_HDX			(1 << 2)
+#define P1SR_PNTR_10BT_FDX			(1 << 1)
+#define P1SR_PNTR_10BT_HDX			(1 << 0)
 
 /* TX Frame control */
 
-- 
2.20.1

Re: [PATCH 0/6] ks8851 fixes & cleanups

[Ben Dooks]

On 20/03/2019 14:02, Lukas Wunner wrote:
> Four fixes and two cleanups for the Microchip (formerly Micrel) KSZ8851
> SPI Ethernet driver.
> 
> Some of the fixes might even pass as stable material, but I haven't marked
> them as such for cautiousness: Doesn't hurt letting them bake in linux-next
> for a few weeks to raise the confidence, even though we've tested them
> extensively on our Revolution Pi open source PLCs.

I have done a visual review, but have not had the hardware to do any
testing on this in a long time.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [PATCH 0/6] ks8851 fixes & cleanups

[David Miller]

From: Lukas Wunner <lukas@wunner.de>
Date: Wed, 20 Mar 2019 15:02:00 +0100

> Four fixes and two cleanups for the Microchip (formerly Micrel) KSZ8851
> SPI Ethernet driver.
> 
> Some of the fixes might even pass as stable material, but I haven't marked
> them as such for cautiousness: Doesn't hurt letting them bake in linux-next
> for a few weeks to raise the confidence, even though we've tested them
> extensively on our Revolution Pi open source PLCs.

Series applied, thanks Lukas.