* [PATCH 2/6] net: ks8851: Reassert reset pin if chip ID check fails
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 14:02 ` [PATCH 6/6] net: ks8851: Deduplicate register macros Lukas Wunner
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/6] net: ks8851: Deduplicate register macros
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
2019-03-20 14:02 ` [PATCH 2/6] net: ks8851: Reassert reset pin if chip ID check fails Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 14:02 ` [PATCH 4/6] net: ks8851: Set initial carrier state to down Lukas Wunner
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] net: ks8851: Set initial carrier state to down
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
2019-03-20 14:02 ` [PATCH 2/6] net: ks8851: Reassert reset pin if chip ID check fails Lukas Wunner
2019-03-20 14:02 ` [PATCH 6/6] net: ks8851: Deduplicate register macros Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 14:02 ` [PATCH 1/6] net: ks8851: Dequeue RX packets explicitly Lukas Wunner
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 1/6] net: ks8851: Dequeue RX packets explicitly
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
` (2 preceding siblings ...)
2019-03-20 14:02 ` [PATCH 4/6] net: ks8851: Set initial carrier state to down Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 14:02 ` [PATCH 5/6] net: ks8851: Fix register macro misnomers Lukas Wunner
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/6] net: ks8851: Fix register macro misnomers
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
` (3 preceding siblings ...)
2019-03-20 14:02 ` [PATCH 1/6] net: ks8851: Dequeue RX packets explicitly Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 14:02 ` [PATCH 3/6] net: ks8851: Delay requesting IRQ until opened Lukas Wunner
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/6] net: ks8851: Delay requesting IRQ until opened
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
` (4 preceding siblings ...)
2019-03-20 14:02 ` [PATCH 5/6] net: ks8851: Fix register macro misnomers Lukas Wunner
@ 2019-03-20 14:02 ` Lukas Wunner
2019-03-20 17:26 ` [PATCH 0/6] ks8851 fixes & cleanups Ben Dooks
2019-03-20 19:44 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Lukas Wunner @ 2019-03-20 14:02 UTC (permalink / raw)
To: David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Ben Dooks, Nishanth Menon,
Stephen Boyd, Sergey Shcherbakov,
Jean-Christophe PLAGNIOL-VILLARD
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] ks8851 fixes & cleanups
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
` (5 preceding siblings ...)
2019-03-20 14:02 ` [PATCH 3/6] net: ks8851: Delay requesting IRQ until opened Lukas Wunner
@ 2019-03-20 17:26 ` Ben Dooks
2019-03-20 19:44 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Ben Dooks @ 2019-03-20 17:26 UTC (permalink / raw)
To: Lukas Wunner, David S. Miller, Tristram Ha, netdev
Cc: Frank Pavlic, Eduard Mainhardt, Nishanth Menon, Stephen Boyd,
Sergey Shcherbakov, Jean-Christophe PLAGNIOL-VILLARD,
Felipe Balbi
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] ks8851 fixes & cleanups
2019-03-20 14:02 [PATCH 0/6] ks8851 fixes & cleanups Lukas Wunner
` (6 preceding siblings ...)
2019-03-20 17:26 ` [PATCH 0/6] ks8851 fixes & cleanups Ben Dooks
@ 2019-03-20 19:44 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-03-20 19:44 UTC (permalink / raw)
To: lukas
Cc: Tristram.Ha, netdev, f.pavlic, e.mainhardt, ben.dooks, nm, sboyd,
shchers, plagnioj, balbi
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.
^ permalink raw reply [flat|nested] 9+ messages in thread