net: bcmgenet: add support for GENETv5
Subsystems (6)
Doug Berger
maintainercover net: bcmgenet: add support for GENETv5
This collection of patches contains changes related to adding support for the BCM7260, BCM7268, and BCM7271 devices that contain a new version of the GENET MAC IP block (v5) and a new fast ethernet (10/100BASE-T) internal PHY.
These patches were originally developed on top of the bug fixes of the "[PATCH v2 net 0/8] net: bcmgenet: minor bug fixes" patch set previously accepted into the net repository, but this submission is designed to be applied to the current net-next that does not yet include them. As a result there will be some merge conflicts that I would be happy to help resolve if desired.
Specifically, conflicts should occur with these patches from the minor bug fixes set:
[PATCH v2 net 3/8] net: bcmgenet: reserved phy revisions must be checked first [PATCH v2 net 5/8] net: bcmgenet: synchronize irq0 status between the isr and task [PATCH v2 net 8/8] net: bcmgenet: decouple flow control from bcmgenet_tx_reclaim
Doug Berger (12):
net: phy: bcm-phylib: replace obsolete EEE macro references
net: phy: bcm7xxx: add support for 28nm EPHY
net: bcmgenet: simplify circular pointer arithmetic
net: bcmgenet: remove meaningless lines
net: bcmgenet: manage dma interrupts in napi code
net: bcmgenet: remove handling of wol interrupts from isr0
net: bcmgenet: clear status to reduce spurious interrupts
net: bcmgenet: correct return value of __bcmgenet_tx_reclaim
net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands
dt-bindings: net: document bcmgenet WoL interrupt
dt-bindings: net: update bcmgenet binding for GENETv5
net: bcmgenet: add support for the GENETv5 hardware
.../devicetree/bindings/net/brcm,bcmgenet.txt | 19 +- .../devicetree/bindings/net/brcm,unimac-mdio.txt | 5 +- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 214 +++++++++++--------- drivers/net/ethernet/broadcom/genet/bcmgenet.h | 10 +- drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 13 -- drivers/net/ethernet/broadcom/genet/bcmmii.c | 62 +++--- drivers/net/phy/bcm-phy-lib.c | 6 +- drivers/net/phy/bcm7xxx.c | 215 ++++++++++++++++++++- include/linux/brcmphy.h | 3 + 9 files changed, 403 insertions(+), 144 deletions(-)
Signature · 6 lines
2.11.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 1/12 net: phy: bcm-phylib: replace obsolete EEE macro references
The macros MDIO_AN_EEE_ADV_100TX and MDIO_AN_EEE_ADV_1000T are now considered obsolete and are replaced in the kernel with the generic macros MDIO_EEE_100TX and MDIO_EEE_1000T respectively.
--- drivers/net/phy/bcm-phy-lib.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/phy/bcm-phy-lib.c +3 -3
@@ -1,5 +1,5 @@/* -* Copyright (C) 2015 Broadcom Corporation +* Copyright (C) 2015-2017 Broadcom * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as
@@ -221,9 +221,9 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)return val;if (enable) - val |= (MDIO_AN_EEE_ADV_100TX |MDIO_AN_EEE_ADV_1000T ); + val |= (MDIO_EEE_100TX |MDIO_EEE_1000T );else - val &= ~(MDIO_AN_EEE_ADV_100TX |MDIO_AN_EEE_ADV_1000T ); + val &= ~(MDIO_EEE_100TX |MDIO_EEE_1000T );phy_write_mmd_indirect (phydev,BCM_CL45VEN_EEE_ADV ,MDIO_MMD_AN , (u32 )val);
Signature · 1 line
2.11.1
Re: [PATCH net-next 01/12] net: phy: bcm-phylib: replace obsolete EEE macro references
Quoted 5 lines · Doug Berger
The macros
MDIO_AN_EEE_ADV_100TXandMDIO_AN_EEE_ADV_1000Tare now considered obsolete and are replaced in the kernel with the generic macrosMDIO_EEE_100TXandMDIO_EEE_1000Trespectively.Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 2/12 net: phy: bcm7xxx: add support for 28nm EPHY
In reply to Doug BergerThis commit adds support for the internal fast ethernet 10/100 PHY found in the BCM7260, BCM7268, and BCM7271 devices.
--- drivers/net/phy/bcm7xxx.c | 215 +++++++++++++++++++++++++++++++++++++++++++++- include/linux/brcmphy.h | 3 + 2 files changed, 216 insertions(+), 2 deletions(-)
drivers/net/phy/bcm7xxx.c +213 -2
@@ -1,7 +1,7 @@/* * Broadcom BCM7xxx internal transceivers support. * -* Copyright (C) 2014, Broadcom Corporation +* Copyright (C) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License
@@ -19,7 +19,7 @@/* Broadcom BCM7xxx internal PHY registers */ -/* 40nm only register definitions */ +/* EPHY only register definitions */ #define MII_BCM7XXX_100TX_AUX_CTL 0x10#define MII_BCM7XXX_100TX_FALSE_CAR 0x13#define MII_BCM7XXX_100TX_DISC 0x14
@@ -27,6 +27,19 @@#define MII_BCM7XXX_64CLK_MDIO BIT(12)#define MII_BCM7XXX_TEST 0x1f#define MII_BCM7XXX_SHD_MODE_2 BIT(2) +#define MII_BCM7XXX_SHD_2_ADDR_CTRL 0xe +#define MII_BCM7XXX_SHD_2_CTRL_STAT 0xf +#define MII_BCM7XXX_SHD_2_BIAS_TRIM 0x1a +#define MII_BCM7XXX_SHD_3_AN_EEE_ADV 0x3 +#define MII_BCM7XXX_SHD_3_PCS_CTRL_2 0x6 +#define MII_BCM7XXX_PCS_CTRL_2_DEF 0x4400 +#define MII_BCM7XXX_SHD_3_AN_STAT 0xb +#define MII_BCM7XXX_AN_NULL_MSG_EN BIT(0) +#define MII_BCM7XXX_AN_EEE_EN BIT(1) +#define MII_BCM7XXX_SHD_3_EEE_THRESH 0xe +#define MII_BCM7XXX_EEE_THRESH_DEF 0x50 +#define MII_BCM7XXX_SHD_3_TL4 0x23 +#define MII_BCM7XXX_TL4_RST_MSK (BIT(2) | BIT(1))/* 28nm only register definitions */ #define MISC_ADDR (base, channel) base, channel
@@ -286,6 +299,181 @@ static int phy_set_clr_bits(struct phy_device *dev, int location,return v; } +static int bcm7xxx_28nm_ephy_01_afe_config_init (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; + +/* Set current trim values INT_trim = -1, Ext_trim =0 */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_BIAS_TRIM ,0x3BE0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_TL4_RST_MSK ,0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset disable */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +0 ,MII_BCM7XXX_TL4_RST_MSK ); +if (ret <0 ) + goto reset_shadow_mode; + +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret; + +return 0 ; +} + +/* The 28nm EPHY does not support Clause 45 (MMD) used by bcm-phy-lib */ +static int bcm7xxx_28nm_ephy_apd_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST , +MII_BRCM_FET_BT_SRE ,0 ); +if (ret <0 ) +return ret; + +/* Enable auto-power down */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_SHDW_AUXSTAT2 , +MII_BRCM_FET_SHDW_AS2_APDE ,0 ); +if (ret <0 ) +return ret; + +/* reset shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST ,0 , +MII_BRCM_FET_BT_SRE ); +if (ret <0 ) +return ret; + +return 0 ; +} + +static int bcm7xxx_28nm_ephy_eee_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; + +/* Advertise supported modes */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_EEE_ADV ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MDIO_EEE_100TX ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Restore Defaults */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_PCS_CTRL_2 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_PCS_CTRL_2_DEF ); +if (ret <0 ) + goto reset_shadow_mode; + + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_EEE_THRESH ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_EEE_THRESH_DEF ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Enable EEE autonegotiation */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_STAT ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , + (MII_BCM7XXX_AN_NULL_MSG_EN |MII_BCM7XXX_AN_EEE_EN )); +if (ret <0 ) + goto reset_shadow_mode; + +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret; + +/* Restart autoneg */ +phy_write (phydev,MII_BMCR , + (BMCR_SPEED100 |BMCR_ANENABLE |BMCR_ANRESTART )); + +return 0 ; +} + +static int bcm7xxx_28nm_ephy_config_init (struct phy_device *phydev) +{ +u8 rev = phydev->phy_id & ~phydev->drv->phy_id_mask; +int ret =0 ; + +pr_info_once ("%s: %s PHY revision: 0x%02x\n" , +phydev_name (phydev), phydev->drv->name, rev); + +/* Dummy read to a register to workaround a possible issue upon reset +* where the internal inverter may not allow the first MDIO transaction +* to pass the MDIO management controller and make us return 0xffff for +* such reads. +*/ +phy_read (phydev,MII_BMSR ); + +/* Apply AFE software work-around if necessary */ +if (rev ==0x01 ) { + ret =bcm7xxx_28nm_ephy_01_afe_config_init (phydev); +if (ret) +return ret; + } + + ret =bcm7xxx_28nm_ephy_eee_enable (phydev); +if (ret) +return ret; + +return bcm7xxx_28nm_ephy_apd_enable (phydev); +} + +static int bcm7xxx_28nm_ephy_resume (struct phy_device *phydev) +{ +int ret; + +/* Re-apply workarounds coming out suspend/resume */ + ret =bcm7xxx_28nm_ephy_config_init (phydev); +if (ret) +return ret; + +return genphy_config_aneg (phydev); +} +static int bcm7xxx_config_init (struct phy_device *phydev) {int ret;
@@ -434,6 +622,23 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev) .probe = bcm7xxx_28nm_probe, \ } +#define BCM7XXX_28NM_EPHY (_oui, _name) \ +{ \ + .phy_id = (_oui), \ + .phy_id_mask = 0xfffffff0, \ + .name = _name, \ + .features = PHY_BASIC_FEATURES, \ + .flags = PHY_IS_INTERNAL, \ + .config_init = bcm7xxx_28nm_ephy_config_init, \ + .config_aneg = genphy_config_aneg, \ + .read_status = genphy_read_status, \ + .resume = bcm7xxx_28nm_ephy_resume, \ + .get_sset_count = bcm_phy_get_sset_count, \ + .get_strings = bcm_phy_get_strings, \ + .get_stats = bcm7xxx_28nm_get_phy_stats, \ + .probe = bcm7xxx_28nm_probe, \ +} +#define BCM7XXX_40NM_EPHY (_oui, _name) \ { \ .phy_id = (_oui), \
@@ -450,6 +655,9 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)static struct phy_driver bcm7xxx_driver[] = {BCM7XXX_28NM_GPHY (PHY_ID_BCM7250 ,"Broadcom BCM7250" ), +BCM7XXX_28NM_EPHY (PHY_ID_BCM7260 ,"Broadcom BCM7260" ), +BCM7XXX_28NM_EPHY (PHY_ID_BCM7268 ,"Broadcom BCM7268" ), +BCM7XXX_28NM_EPHY (PHY_ID_BCM7271 ,"Broadcom BCM7271" ),BCM7XXX_28NM_GPHY (PHY_ID_BCM7278 ,"Broadcom BCM7278" ),BCM7XXX_28NM_GPHY (PHY_ID_BCM7364 ,"Broadcom BCM7364" ),BCM7XXX_28NM_GPHY (PHY_ID_BCM7366 ,"Broadcom BCM7366" ),
@@ -466,6 +674,9 @@ static struct phy_driver bcm7xxx_driver[] = {static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = { {PHY_ID_BCM7250 ,0xfffffff0 , }, + {PHY_ID_BCM7260 ,0xfffffff0 , }, + {PHY_ID_BCM7268 ,0xfffffff0 , }, + {PHY_ID_BCM7271 ,0xfffffff0 , }, {PHY_ID_BCM7278 ,0xfffffff0 , }, {PHY_ID_BCM7364 ,0xfffffff0 , }, {PHY_ID_BCM7366 ,0xfffffff0 , },
include/linux/brcmphy.h +3 -0
@@ -25,6 +25,9 @@#define PHY_ID_BCM57780 0x03625d90#define PHY_ID_BCM7250 0xae025280 +#define PHY_ID_BCM7260 0xae025190 +#define PHY_ID_BCM7268 0xae025090 +#define PHY_ID_BCM7271 0xae0253b0#define PHY_ID_BCM7278 0xae0251a0#define PHY_ID_BCM7364 0xae025260#define PHY_ID_BCM7366 0x600d8490
Signature · 1 line
2.11.1
Re: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY
Quoted 4 lines · Doug Berger
This commit adds support for the internal fast ethernet 10/100 PHY found in the BCM7260, BCM7268, and BCM7271 devices.
Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrew Lunn
maintainerRe: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY
In reply to Doug BergerQuoted 10 lines · Doug Berger
drivers/net/phy/bcm7xxx.c +10
-0+static int bcm7xxx_28nm_ephy_01_afe_config_init (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; +
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 321
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+/* Set current trim values INT_trim = -1, Ext_trim =0 */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_BIAS_TRIM ,0x3BE0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;
Hi Doug
It would be nice to have a few blank lines here and there...
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 331
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_TL4_RST_MSK ,0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset disable */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;
... just to break things up into readable chunks.
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 344
Quoted 13 lines
drivers/net/phy/bcm7xxx.c +13
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +0 ,MII_BCM7XXX_TL4_RST_MSK ); +if (ret <0 ) + goto reset_shadow_mode; + +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret; + +return 0 ;
How about:
return phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, MII_BCM7XXX_SHD_MODE_2);
Quoted 14 lines
drivers/net/phy/bcm7xxx.c +14
-0+/* The 28nm EPHY does not support Clause 45 (MMD) used by bcm-phy-lib */ +static int bcm7xxx_28nm_ephy_apd_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST , +MII_BRCM_FET_BT_SRE ,0 ); +if (ret <0 ) +return ret; + +/* Enable auto-power down */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_SHDW_AUXSTAT2 , +MII_BRCM_FET_SHDW_AS2_APDE ,0 );
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 370
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+if (ret <0 ) +return ret; + +/* reset shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST ,0 , +MII_BRCM_FET_BT_SRE ); +if (ret <0 ) +return ret; + +return 0 ;
How about just
return phy_set_clr_bits(phydev, MII_BRCM_FET_BRCMTEST, 0,
MII_BRCM_FET_BT_SRE);
Quoted 7 lines
drivers/net/phy/bcm7xxx.c +7
-0+} + +static int bcm7xxx_28nm_ephy_eee_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 387
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; + +/* Advertise supported modes */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_EEE_ADV ); +if (ret <0 ) + goto reset_shadow_mode;
blank...
Quoted 36 lines
drivers/net/phy/bcm7xxx.c +36
-0+ ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MDIO_EEE_100TX ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Restore Defaults */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_PCS_CTRL_2 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_PCS_CTRL_2_DEF ); +if (ret <0 ) + goto reset_shadow_mode; + + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_EEE_THRESH ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_EEE_THRESH_DEF ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Enable EEE autonegotiation */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_STAT ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , + (MII_BCM7XXX_AN_NULL_MSG_EN |MII_BCM7XXX_AN_EEE_EN )); +if (ret <0 ) + goto reset_shadow_mode; + +reset_shadow_mode: +/* reset shadow mode 2 */
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 433
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret; + +/* Restart autoneg */ +phy_write (phydev,MII_BMCR , + (BMCR_SPEED100 |BMCR_ANENABLE |BMCR_ANRESTART )); + +return 0 ;
return phy_write(.....); ?
Quoted 1 line
+}
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerRe: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY
Quoted 24 lines · Andrew Lunn
Quoted 10 lines · Doug Berger
drivers/net/phy/bcm7xxx.c +10
-0+static int bcm7xxx_28nm_ephy_01_afe_config_init (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; +drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 321
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+/* Set current trim values INT_trim = -1, Ext_trim =0 */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_BIAS_TRIM ,0x3BE0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;Hi Doug
Quoted 2 lines
It would be nice to have a few blank lines here and there...
Thanks for taking the time to review this.
In general I try to keep lines of related functionality together and use the blank lines to help identify boundaries. In this particular case, I believe it is clearer to keep the code that may return an error code together with the code that tests for the error.
Perhaps this would be a better alternative:
+ /* Set current trim values INT_trim = -1, Ext_trim =0 */
+ if (phy_write(phydev, MII_BCM7XXX_SHD_2_BIAS_TRIM, 0x3BE0) < 0)
+ goto reset_shadow_mode;
+
+ /* Cal reset */
+ if (phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL,
+ MII_BCM7XXX_SHD_3_TL4) < 0)
+ goto reset_shadow_mode;
Quoted 4 lines
Quoted 4 lines
drivers/net/phy/bcm7xxx.c +4
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_TL4_RST_MSK ,0 ); +if (ret <0 ) + goto reset_shadow_mode;
Quoted 9 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 331
Quoted 6 lines
drivers/net/phy/bcm7xxx.c +6
-0+ +/* Cal reset disable */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;... just to break things up into readable chunks.
Maybe:
+ if (phy_set_clr_bits(phydev, MII_BCM7XXX_SHD_2_CTRL_STAT,
+ MII_BCM7XXX_TL4_RST_MSK, 0) < 0)
+ goto reset_shadow_mode;
+
+ /* Cal reset disable */
+ if (phy_write(phydev, MII_BCM7XXX_SHD_2_ADDR_CTRL,
+ MII_BCM7XXX_SHD_3_TL4) < 0)
+ goto reset_shadow_mode;
Quoted 16 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 344
Quoted 13 lines
drivers/net/phy/bcm7xxx.c +13
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +0 ,MII_BCM7XXX_TL4_RST_MSK ); +if (ret <0 ) + goto reset_shadow_mode; + +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret; + +return 0 ;How about:
Quoted 3 lines
return phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, MII_BCM7XXX_SHD_MODE_2);
The trouble here is that currently the phy_set_clr_bits() function returns the value written or a negative error and the function bcm7xxx_28nm_ephy_01_afe_config_init() is supposed to return 0 on success and non-zero on failure so this would not have the same functionality. I suppose we could change the phy_set_clr_bits() API to return 0 on success to make this work, but I wasn't trying to change preexisting functionality in this file.
Quoted 27 lines
Quoted 14 lines
drivers/net/phy/bcm7xxx.c +14
-0+/* The 28nm EPHY does not support Clause 45 (MMD) used by bcm-phy-lib */ +static int bcm7xxx_28nm_ephy_apd_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST , +MII_BRCM_FET_BT_SRE ,0 ); +if (ret <0 ) +return ret; + +/* Enable auto-power down */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_SHDW_AUXSTAT2 , +MII_BRCM_FET_SHDW_AS2_APDE ,0 );drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 370
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+if (ret <0 ) +return ret; + +/* reset shadow mode 1 */ + ret =phy_set_clr_bits (phydev,MII_BRCM_FET_BRCMTEST ,0 , +MII_BRCM_FET_BT_SRE ); +if (ret <0 ) +return ret; + +return 0 ;How about just
Quoted 3 lines
return phy_set_clr_bits(phydev,
MII_BRCM_FET_BRCMTEST, 0,
MII_BRCM_FET_BT_SRE);
Same remark as above.
Quoted 20 lines
Quoted 7 lines
drivers/net/phy/bcm7xxx.c +7
-0+} + +static int bcm7xxx_28nm_ephy_eee_enable (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 387
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; + +/* Advertise supported modes */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_EEE_ADV ); +if (ret <0 ) + goto reset_shadow_mode;blank...
Quoted 4 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 391
Quoted 4 lines
drivers/net/phy/bcm7xxx.c +4
-0+ ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MDIO_EEE_100TX ); +if (ret <0 ) + goto reset_shadow_mode;
Here the two phy_write() calls are required to "/* Advertise supported modes */" (one sets an address and the other specifies the data to write to that address) so I kept them together to imply an association with the preceding comment. If new code in the future came between them it would be "bad" so closing the space helps indicate that it is not a safe coding boundary for inserting code.
Quoted 10 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 401
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ +/* Restore Defaults */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_PCS_CTRL_2 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_PCS_CTRL_2_DEF ); +if (ret <0 ) + goto reset_shadow_mode;
Same here.
Quoted 9 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 410
Quoted 9 lines
drivers/net/phy/bcm7xxx.c +9
-0+ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_EEE_THRESH ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_EEE_THRESH_DEF ); +if (ret <0 ) + goto reset_shadow_mode;
Here...
Quoted 10 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 420
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ +/* Enable EEE autonegotiation */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_STAT ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , + (MII_BCM7XXX_AN_NULL_MSG_EN |MII_BCM7XXX_AN_EEE_EN )); +if (ret <0 ) + goto reset_shadow_mode;
and here.
Quoted 7 lines
Quoted 7 lines
drivers/net/phy/bcm7xxx.c +7
-0+ +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret;
Quoted 9 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 433
Quoted 6 lines
drivers/net/phy/bcm7xxx.c +6
-0+ +/* Restart autoneg */ +phy_write (phydev,MII_BMCR , + (BMCR_SPEED100 |BMCR_ANENABLE |BMCR_ANRESTART )); + +return 0 ;return
phy_write(.....); ?
I would feel more comfortable with this if the return value of the struct mii_bus write member function was more clearly defined. In our case, we return 0 on success so I would consider this change, but I would prefer a consensus that all mii_bus write functions return 0 on success before doing so.
Quoted 4 lines
Quoted 1 line
+}
Andrew
Thanks again,
Doug
Andrew Lunn
maintainerRe: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY
Quoted 25 lines · Doug Berger
Quoted 24 lines · Andrew Lunn
Quoted 10 lines · Doug Berger
drivers/net/phy/bcm7xxx.c +10
-0+static int bcm7xxx_28nm_ephy_01_afe_config_init (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; +drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 321
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+/* Set current trim values INT_trim = -1, Ext_trim =0 */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_BIAS_TRIM ,0x3BE0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;Hi Doug
Quoted 8 lines
Quoted 2 lines
It would be nice to have a few blank lines here and there...
Thanks for taking the time to review this.
In general I try to keep lines of related functionality together and use the blank lines to help identify boundaries. In this particular case, I believe it is clearer to keep the code that may return an error code together with the code that tests for the error.
Hi Doug
I agree with that. Which is why i placed the comment between the goto and the next block of code. This is where i think there should be a blank line, to separate it from setting the trim values.
Quoted 8 lines
Quoted 3 lines
return phy_set_clr_bits(phydev, MII_BCM7XXX_TEST, 0, MII_BCM7XXX_SHD_MODE_2);The trouble here is that currently the
phy_set_clr_bits()function returns the value written or a negative error and the functionbcm7xxx_28nm_ephy_01_afe_config_init()is supposed to return 0 on success and non-zero on failure so this would not have the same functionality.
Ah, O.K. No problem.
Quoted 8 lines
Quoted 8 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 387
Quoted 5 lines
drivers/net/phy/bcm7xxx.c +5
-0+/* Advertise supported modes */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_EEE_ADV ); +if (ret <0 ) + goto reset_shadow_mode;blank...
Quoted 8 lines
Quoted 4 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 391
Quoted 4 lines
drivers/net/phy/bcm7xxx.c +4
-0+ ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MDIO_EEE_100TX ); +if (ret <0 ) + goto reset_shadow_mode;Here the two
phy_write()calls are required to "/* Advertise supported modes */" (one sets an address and the other specifies the data to write to that address) so I kept them together to imply an association with the preceding comment.
O.K, i probably would if written a little helper function. And you seem to have this repeated a few times, so the helper would be used a few times.
Quoted 51 lines
Quoted 10 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 401
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ +/* Restore Defaults */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_PCS_CTRL_2 ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_PCS_CTRL_2_DEF ); +if (ret <0 ) + goto reset_shadow_mode;Same here.
Quoted 9 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 410
Quoted 9 lines
drivers/net/phy/bcm7xxx.c +9
-0+ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_EEE_THRESH ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , +MII_BCM7XXX_EEE_THRESH_DEF ); +if (ret <0 ) + goto reset_shadow_mode;Here...
Quoted 10 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 420
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+ +/* Enable EEE autonegotiation */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_AN_STAT ); +if (ret <0 ) + goto reset_shadow_mode; + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_CTRL_STAT , + (MII_BCM7XXX_AN_NULL_MSG_EN |MII_BCM7XXX_AN_EEE_EN )); +if (ret <0 ) + goto reset_shadow_mode;and here.
Quoted 7 lines
Quoted 7 lines
drivers/net/phy/bcm7xxx.c +7
-0+ +reset_shadow_mode: +/* reset shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST ,0 , +MII_BCM7XXX_SHD_MODE_2 ); +if (ret <0 ) +return ret;Quoted 9 lines
drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 433
Quoted 6 lines
drivers/net/phy/bcm7xxx.c +6
-0+ +/* Restart autoneg */ +phy_write (phydev,MII_BMCR , + (BMCR_SPEED100 |BMCR_ANENABLE |BMCR_ANRESTART )); + +return 0 ;return
phy_write(.....); ?
Quoted 5 lines
I would feel more comfortable with this if the return value of the struct
mii_buswrite member function was more clearly defined. In our case, we return 0 on success so I would consider this change, but I would prefer a consensus that allmii_buswrite functions return 0 on success before doing so.
You are right in that this is not clearly defined. But i just looked through all the mdio drivers in drivers/net/phy and they all do return 0 for their write operation.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerRe: [PATCH net-next 02/12] net: phy: bcm7xxx: add support for 28nm EPHY
Quoted 35 lines · Andrew Lunn
Quoted 25 lines · Doug Berger
Quoted 24 lines · Andrew Lunn
Quoted 10 lines · Doug Berger
drivers/net/phy/bcm7xxx.c +10
-0+static int bcm7xxx_28nm_ephy_01_afe_config_init (struct phy_device *phydev) +{ +int ret; + +/* set shadow mode 2 */ + ret =phy_set_clr_bits (phydev,MII_BCM7XXX_TEST , +MII_BCM7XXX_SHD_MODE_2 ,0 ); +if (ret <0 ) +return ret; +drivers/net/phy/bcm7xxx.c static int phy_set_clr_bits(struct phy_device *dev, int location, · line 321
Quoted 10 lines
drivers/net/phy/bcm7xxx.c +10
-0+/* Set current trim values INT_trim = -1, Ext_trim =0 */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_BIAS_TRIM ,0x3BE0 ); +if (ret <0 ) + goto reset_shadow_mode; + +/* Cal reset */ + ret =phy_write (phydev,MII_BCM7XXX_SHD_2_ADDR_CTRL , +MII_BCM7XXX_SHD_3_TL4 ); +if (ret <0 ) + goto reset_shadow_mode;Hi Doug
Quoted 8 lines
Quoted 2 lines
It would be nice to have a few blank lines here and there...
Thanks for taking the time to review this.
In general I try to keep lines of related functionality together and use the blank lines to help identify boundaries. In this particular case, I believe it is clearer to keep the code that may return an error code together with the code that tests for the error.
Quoted 6 lines
Hi Doug
I agree with that. Which is why i placed the comment between the goto and the next block of code. This is where i think there should be a blank line, to separate it from setting the trim values.
OK, I see. I thought you were referring to the code blocks above the comment. In that case, as described earlier, the code below the comment is tightly coupled with the code above the comment since the pair of transactions are how we "/* Cal reset */". The idea of introducing a subroutine/helper function for these paired (addr/data) transactions might help readability so I will consider it for a future patch.
Thanks again for the feedback,
Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 3/12 net: bcmgenet: simplify circular pointer arithmetic
In reply to Doug BergerA 2's complement subtraction will always do a borrow, so masking off the sign bits is the same as conditionally adding (mask+1).
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +5 -14
@@ -1,7 +1,7 @@/* * Broadcom GENET (Gigabit Ethernet) controller driver * -* Copyright (c) 2014 Broadcom Corporation +* Copyright (c) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as
@@ -1175,13 +1175,9 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,unsigned int txbds_processed =0 ;/* Compute how many buffers are transmitted since last xmit call */ - c_index =bcmgenet_tdma_ring_readl (priv, ring->index,TDMA_CONS_INDEX ); - c_index &=DMA_C_INDEX_MASK ; - -if (likely (c_index >= ring->c_index)) - txbds_ready = c_index - ring->c_index; -else - txbds_ready = (DMA_C_INDEX_MASK +1 ) - ring->c_index + c_index; + c_index =bcmgenet_tdma_ring_readl (priv, ring->index,TDMA_CONS_INDEX ) + &DMA_C_INDEX_MASK ; + txbds_ready = (c_index - ring->c_index) &DMA_C_INDEX_MASK ; netif_dbg(priv, tx_done, dev, "%s ring=%d old_c_index=%u c_index=%u txbds_ready=%u\n",
@@ -1611,12 +1607,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring, } p_index &=DMA_P_INDEX_MASK ; - -if (likely (p_index >= ring->c_index)) - rxpkttoprocess = p_index - ring->c_index; -else - rxpkttoprocess = (DMA_C_INDEX_MASK +1 ) - ring->c_index + - p_index; + rxpkttoprocess = (p_index - ring->c_index) &DMA_C_INDEX_MASK ;netif_dbg (priv, rx_status, dev,"RDMA: rxpkttoprocess=%d\n" , rxpkttoprocess);
Signature · 1 line
2.11.1
Re: [PATCH net-next 03/12] net: bcmgenet: simplify circular pointer arithmetic
Quoted 4 lines · Doug Berger
A 2's complement subtraction will always do a borrow, so masking off the sign bits is the same as conditionally adding (mask+1).
Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 4/12 net: bcmgenet: remove meaningless lines
In reply to Doug BergerAn assortment of non-functional lines are removed to reduce confusion and some typos in comments are corrected.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +3 -10
@@ -605,7 +605,7 @@ static int bcmgenet_set_coalesce(struct net_device *dev,/* GENET TDMA hardware does not support a configurable timeout, but will * always generate an interrupt either after MBDONE packets have been -* transmitted, or when the ring is emtpy. +* transmitted, or when the ring is empty. */ if (ec->tx_coalesce_usecs || ec->tx_coalesce_usecs_high || ec->tx_coalesce_usecs_irq || ec->tx_coalesce_usecs_low)
@@ -1834,10 +1834,8 @@ static void bcmgenet_intr_disable(struct bcmgenet_priv *priv)/* Mask all interrupts.*/ bcmgenet_intrl2_0_writel (priv,0xFFFFFFFF ,INTRL2_CPU_MASK_SET );bcmgenet_intrl2_0_writel (priv,0xFFFFFFFF ,INTRL2_CPU_CLEAR ); -bcmgenet_intrl2_0_writel (priv,0 ,INTRL2_CPU_MASK_CLEAR );bcmgenet_intrl2_1_writel (priv,0xFFFFFFFF ,INTRL2_CPU_MASK_SET );bcmgenet_intrl2_1_writel (priv,0xFFFFFFFF ,INTRL2_CPU_CLEAR ); -bcmgenet_intrl2_1_writel (priv,0 ,INTRL2_CPU_MASK_CLEAR ); }static void bcmgenet_link_intr_enable (struct bcmgenet_priv *priv)
@@ -1926,7 +1924,6 @@ static int init_umac(struct bcmgenet_priv *priv)bcmgenet_intrl2_0_writel (priv, int0_enable,INTRL2_CPU_MASK_CLEAR );bcmgenet_intrl2_1_writel (priv, int1_enable,INTRL2_CPU_MASK_CLEAR ); -/* Enable rx/tx engine.*/ dev_dbg (kdev,"done init umac\n" );return 0 ;
@@ -2836,7 +2833,7 @@ static int bcmgenet_close(struct net_device *dev)if (ret)return ret; -/* Disable MAC transmit. TX DMA disabled have to done before this */ +/* Disable MAC transmit. TX DMA disabled must be done before this */ umac_enable_set (priv,CMD_TX_EN , false);/* tx reclaim */
@@ -3115,22 +3112,18 @@ static void bcmgenet_set_hw_params(struct bcmgenet_priv *priv) bcmgenet_dma_regs = bcmgenet_dma_regs_v3plus; genet_dma_ring_regs = genet_dma_ring_regs_v4; priv->dma_rx_chk_bit =DMA_RX_CHK_V3PLUS ; - priv->version =GENET_V4 ; }else if (GENET_IS_V3 (priv)) { bcmgenet_dma_regs = bcmgenet_dma_regs_v3plus; genet_dma_ring_regs = genet_dma_ring_regs_v123; priv->dma_rx_chk_bit =DMA_RX_CHK_V3PLUS ; - priv->version =GENET_V3 ; }else if (GENET_IS_V2 (priv)) { bcmgenet_dma_regs = bcmgenet_dma_regs_v2; genet_dma_ring_regs = genet_dma_ring_regs_v123; priv->dma_rx_chk_bit =DMA_RX_CHK_V12 ; - priv->version =GENET_V2 ; }else if (GENET_IS_V1 (priv)) { bcmgenet_dma_regs = bcmgenet_dma_regs_v1; genet_dma_ring_regs = genet_dma_ring_regs_v123; priv->dma_rx_chk_bit =DMA_RX_CHK_V12 ; - priv->version =GENET_V1 ; }/* enum genet_version starts at 1 */
@@ -3397,7 +3390,7 @@ static int bcmgenet_suspend(struct device *d)if (ret)return ret; -/* Disable MAC transmit. TX DMA disabled have to done before this */ +/* Disable MAC transmit. TX DMA disabled must be done before this */ umac_enable_set (priv,CMD_TX_EN , false);/* tx reclaim */
Signature · 1 line
2.11.1
Re: [PATCH net-next 04/12] net: bcmgenet: remove meaningless lines
Quoted 4 lines · Doug Berger
An assortment of non-functional lines are removed to reduce confusion and some typos in comments are corrected.
Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 5/12 net: bcmgenet: manage dma interrupts in napi code
In reply to Doug BergerThis commit moves DMA interrupt enabling out of init_umac() and adds the masking of these interrupts to the napi enable and disable code.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 39 +++++++++++++++----------- 1 file changed, 22 insertions(+), 17 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +22 -17
@@ -1862,8 +1862,6 @@ static int init_umac(struct bcmgenet_priv *priv)int ret;u32 reg;u32 int0_enable =0 ; -u32 int1_enable =0 ; -int i;dev_dbg (&priv->pdev->dev,"bcmgenet: init_umac\n" );
@@ -1890,12 +1888,6 @@ static int init_umac(struct bcmgenet_priv *priv)bcmgenet_intr_disable (priv); -/* Enable Rx default queue 16 interrupts */ - int0_enable |=UMAC_IRQ_RXDMA_DONE ; - -/* Enable Tx default queue 16 interrupts */ - int0_enable |=UMAC_IRQ_TXDMA_DONE ; -/* Configure backpressure vectors for MoCA */ if (priv->phy_interface ==PHY_INTERFACE_MODE_MOCA ) { reg =bcmgenet_bp_mc_get (priv);
@@ -1913,16 +1905,7 @@ static int init_umac(struct bcmgenet_priv *priv)if (priv->hw_params->flags &GENET_HAS_MDIO_INTR ) int0_enable |= (UMAC_IRQ_MDIO_DONE |UMAC_IRQ_MDIO_ERROR ); -/* Enable Rx priority queue interrupts */ -for (i =0 ; i < priv->hw_params->rx_queues; ++i) - int1_enable |= (1 << (UMAC_IRQ1_RX_INTR_SHIFT + i)); - -/* Enable Tx priority queue interrupts */ -for (i =0 ; i < priv->hw_params->tx_queues; ++i) - int1_enable |= (1 << i); -bcmgenet_intrl2_0_writel (priv, int0_enable,INTRL2_CPU_MASK_CLEAR ); -bcmgenet_intrl2_1_writel (priv, int1_enable,INTRL2_CPU_MASK_CLEAR );dev_dbg (kdev,"done init umac\n" );
@@ -2055,22 +2038,33 @@ static void bcmgenet_init_tx_napi(struct bcmgenet_priv *priv)static void bcmgenet_enable_tx_napi (struct bcmgenet_priv *priv) {unsigned int i; +u32 int0_enable =UMAC_IRQ_TXDMA_DONE ; +u32 int1_enable =0 ;struct bcmgenet_tx_ring *ring;for (i =0 ; i < priv->hw_params->tx_queues; ++i) { ring = &priv->tx_rings[i];napi_enable (&ring->napi); + int1_enable |= (1 << i); } ring = &priv->tx_rings[DESC_INDEX ];napi_enable (&ring->napi); + +bcmgenet_intrl2_0_writel (priv, int0_enable,INTRL2_CPU_MASK_CLEAR ); +bcmgenet_intrl2_1_writel (priv, int1_enable,INTRL2_CPU_MASK_CLEAR ); }static void bcmgenet_disable_tx_napi (struct bcmgenet_priv *priv) {unsigned int i; +u32 int0_disable =UMAC_IRQ_TXDMA_DONE ; +u32 int1_disable =0xffff ;struct bcmgenet_tx_ring *ring; +bcmgenet_intrl2_0_writel (priv, int0_disable,INTRL2_CPU_MASK_SET ); +bcmgenet_intrl2_1_writel (priv, int1_disable,INTRL2_CPU_MASK_SET ); +for (i =0 ; i < priv->hw_params->tx_queues; ++i) { ring = &priv->tx_rings[i];napi_disable (&ring->napi);
@@ -2183,22 +2177,33 @@ static void bcmgenet_init_rx_napi(struct bcmgenet_priv *priv)static void bcmgenet_enable_rx_napi (struct bcmgenet_priv *priv) {unsigned int i; +u32 int0_enable =UMAC_IRQ_RXDMA_DONE ; +u32 int1_enable =0 ;struct bcmgenet_rx_ring *ring;for (i =0 ; i < priv->hw_params->rx_queues; ++i) { ring = &priv->rx_rings[i];napi_enable (&ring->napi); + int1_enable |= (1 << (UMAC_IRQ1_RX_INTR_SHIFT + i)); } ring = &priv->rx_rings[DESC_INDEX ];napi_enable (&ring->napi); + +bcmgenet_intrl2_0_writel (priv, int0_enable,INTRL2_CPU_MASK_CLEAR ); +bcmgenet_intrl2_1_writel (priv, int1_enable,INTRL2_CPU_MASK_CLEAR ); }static void bcmgenet_disable_rx_napi (struct bcmgenet_priv *priv) {unsigned int i; +u32 int0_disable =UMAC_IRQ_RXDMA_DONE ; +u32 int1_disable =0xffff <<UMAC_IRQ1_RX_INTR_SHIFT ;struct bcmgenet_rx_ring *ring; +bcmgenet_intrl2_0_writel (priv, int0_disable,INTRL2_CPU_MASK_SET ); +bcmgenet_intrl2_1_writel (priv, int1_disable,INTRL2_CPU_MASK_SET ); +for (i =0 ; i < priv->hw_params->rx_queues; ++i) { ring = &priv->rx_rings[i];napi_disable (&ring->napi);
Signature · 1 line
2.11.1
Re: [PATCH net-next 05/12] net: bcmgenet: manage dma interrupts in napi code
Quoted 4 lines · Doug Berger
This commit moves DMA interrupt enabling out of
init_umac()and adds the masking of these interrupts to the napi enable and disable code.Signed-off-by Doug Berger <[email protected]>
Signature · 1 line
Florian
Doug Berger
maintainerPATCH 6/12 net: bcmgenet: remove handling of wol interrupts from isr0
In reply to Doug BergerThe bcmgenet_wol_isr() handler performs the necessary processing for waking from a GENET event. There is no necessary functionality behind servicing the UMAC_IRQ_MPD_R event in the handling of isr0. Therefore the code that unmasks and masks this interrupt and that gets invoked in response to it is removed by this commit.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 +--------- drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 15 +-------------- 2 files changed, 2 insertions(+), 23 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +1 -9
@@ -2455,13 +2455,6 @@ static void bcmgenet_irq_task(struct work_struct *work)netif_dbg (priv, intr, priv->dev,"%s\n" , __func__); -if (priv->irq0_stat &UMAC_IRQ_MPD_R ) { - priv->irq0_stat &= ~UMAC_IRQ_MPD_R ; -netif_dbg (priv, wol, priv->dev, -"magic packet detected, waking up\n" ); -bcmgenet_power_up (priv,GENET_POWER_WOL_MAGIC ); - } -/* Link UP/DOWN event */ if (priv->irq0_stat &UMAC_IRQ_LINK_EVENT ) { phy_mac_interrupt(priv->phydev,
@@ -2558,8 +2551,7 @@ static irqreturn_t bcmgenet_isr0(int irq, void *dev_id) UMAC_IRQ_PHY_DET_F | UMAC_IRQ_LINK_EVENT | UMAC_IRQ_HFB_SM | - UMAC_IRQ_HFB_MM | - UMAC_IRQ_MPD_R)) { + UMAC_IRQ_HFB_MM)) {/* all other interested interrupts handled in bottom half */ schedule_work (&priv->bcmgenet_irq_work); }
drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c +1 -14
@@ -1,7 +1,7 @@/* * Broadcom GENET (Gigabit Ethernet) Wake-on-LAN support * -* Copyright (c) 2014 Broadcom Corporation +* Copyright (c) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as
@@ -127,7 +127,6 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv, enum bcmgenet_power_mode mode) {struct net_device *dev = priv->dev; -u32 cpu_mask_clear;int retries =0 ;u32 reg;
@@ -173,18 +172,12 @@ int bcmgenet_wol_power_down_cfg(struct bcmgenet_priv *priv,bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT ); } -/* Enable the MPD interrupt */ - cpu_mask_clear =UMAC_IRQ_MPD_R ; - -bcmgenet_intrl2_0_writel (priv, cpu_mask_clear,INTRL2_CPU_MASK_CLEAR ); -return 0 ; }void bcmgenet_wol_power_up_cfg (struct bcmgenet_priv *priv,enum bcmgenet_power_mode mode) { -u32 cpu_mask_set;u32 reg;if (mode !=GENET_POWER_WOL_MAGIC ) {
@@ -201,10 +194,4 @@ void bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv, reg &= ~CMD_CRC_FWD ;bcmgenet_umac_writel (priv, reg,UMAC_CMD ); priv->crc_fwd_en =0 ; - -/* Stop monitoring magic packet IRQ */ - cpu_mask_set =UMAC_IRQ_MPD_R ; - -/* Stop monitoring magic packet IRQ */ -bcmgenet_intrl2_0_writel (priv, cpu_mask_set,INTRL2_CPU_MASK_SET ); }
Signature · 6 lines
2.11.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 06/12] net: bcmgenet: remove handling of wol interrupts from isr0
Quoted 7 lines · Doug Berger
The
bcmgenet_wol_isr()handler performs the necessary processing for waking from a GENET event. There is no necessary functionality behind servicing theUMAC_IRQ_MPD_Revent in the handling of isr0. Therefore the code that unmasks and masks this interrupt and that gets invoked in response to it is removed by this commit.Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 7/12 net: bcmgenet: clear status to reduce spurious interrupts
In reply to Doug BergerSince the DMA interrupt status is latched and the DMA servicing can be polled, it is a good idea to clear the latched status of a DMA interrupt before performing the service that would be invoked by the interrupt.
This prevents old status from causing spurious interrupts when the interrupt is unmasked at a later time.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +20 -1
@@ -1174,6 +1174,14 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,unsigned int txbds_ready;unsigned int txbds_processed =0 ; +/* Clear status before servicing to reduce spurious interrupts */ +if (ring->index ==DESC_INDEX ) +bcmgenet_intrl2_0_writel (priv,UMAC_IRQ_TXDMA_DONE , +INTRL2_CPU_CLEAR ); +else +bcmgenet_intrl2_1_writel (priv, (1 << ring->index), +INTRL2_CPU_CLEAR ); +/* Compute how many buffers are transmitted since last xmit call */ c_index =bcmgenet_tdma_ring_readl (priv, ring->index,TDMA_CONS_INDEX ) &DMA_C_INDEX_MASK ;
@@ -1584,10 +1592,21 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,unsigned long dma_flag;int len;unsigned int rxpktprocessed =0 , rxpkttoprocess; -unsigned int p_index; +unsigned int p_index, mask;unsigned int discards;unsigned int chksum_ok =0 ; +/* Clear status before servicing to reduce spurious interrupts */ +if (ring->index ==DESC_INDEX ) { +bcmgenet_intrl2_0_writel (priv,UMAC_IRQ_RXDMA_DONE , +INTRL2_CPU_CLEAR ); + }else { + mask =1 << (UMAC_IRQ1_RX_INTR_SHIFT + ring->index); +bcmgenet_intrl2_1_writel (priv, + mask, +INTRL2_CPU_CLEAR ); + } + p_index =bcmgenet_rdma_ring_readl (priv, ring->index,RDMA_PROD_INDEX ); discards = (p_index >> DMA_P_INDEX_DISCARD_CNT_SHIFT) &
Signature · 1 line
2.11.1
Re: [PATCH net-next 07/12] net: bcmgenet: clear status to reduce spurious interrupts
Quoted 8 lines · Doug Berger
Since the DMA interrupt status is latched and the DMA servicing can be polled, it is a good idea to clear the latched status of a DMA interrupt before performing the service that would be invoked by the interrupt.
This prevents old status from causing spurious interrupts when the interrupt is unmasked at a later time.
Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 8/12 net: bcmgenet: correct return value of __bcmgenet_tx_reclaim
In reply to Doug BergerThe reclaim function should return the number of buffer descriptors reclaimed, not just the number corresponding to skb packets.
Also, remove the unnecessary computation when updating the consumer index.
While this is not a functional problem it could degrade performance of napi in a fragmented transmit stream.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +2 -2
@@ -1218,7 +1218,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev, } ring->free_bds += txbds_processed; - ring->c_index = (ring->c_index + txbds_processed) &DMA_C_INDEX_MASK ; + ring->c_index = c_index; dev->stats.tx_packets += pkts_compl; dev->stats.tx_bytes += bytes_compl;
@@ -1231,7 +1231,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,netif_tx_wake_queue (txq); } -return pkts_compl; +return txbds_processed; } static unsigned int bcmgenet_tx_reclaim(struct net_device *dev,
Signature · 1 line
2.11.1
Re: [PATCH net-next 08/12] net: bcmgenet: correct return value of __bcmgenet_tx_reclaim
Quoted 10 lines · Doug Berger
The reclaim function should return the number of buffer descriptors reclaimed, not just the number corresponding to skb packets.
Also, remove the unnecessary computation when updating the consumer index.
While this is not a functional problem it could degrade performance of napi in a fragmented transmit stream.
Signed-off-by Doug Berger <[email protected]>
Signature · 1 line
Florian
Doug Berger
maintainerPATCH 9/12 net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands
In reply to Doug BergerThis commit changes the ioctl handling behavior to return the EOPNOTSUPP error code instead of the EINVAL error code when an unknown ioctl command value is detected.
It also removes some redundant parsing of the ioctl command value and allows the SIOCSHWTSTAMP value to be handled.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +3 -16
@@ -1062,27 +1062,14 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,static int bcmgenet_ioctl (struct net_device *dev,struct ifreq *rq,int cmd) {struct bcmgenet_priv *priv =netdev_priv (dev); -int val =0 ;if (!netif_running (dev))return -EINVAL ; -switch (cmd) { -case SIOCGMIIPHY : -case SIOCGMIIREG : -case SIOCSMIIREG : -if (!priv->phydev) - val = -ENODEV ; -else - val =phy_mii_ioctl (priv->phydev, rq, cmd); -break ; - -default : - val = -EINVAL ; -break ; - } +if (!priv->phydev) +return -ENODEV ; -return val; +return phy_mii_ioctl (priv->phydev, rq, cmd); } static struct enet_cb *bcmgenet_get_txcb(struct bcmgenet_priv *priv,
Signature · 1 line
2.11.1
Re: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands
Quoted 8 lines · Doug Berger
This commit changes the ioctl handling behavior to return the EOPNOTSUPP error code instead of the EINVAL error code when an unknown ioctl command value is detected.
It also removes some redundant parsing of the ioctl command value and allows the SIOCSHWTSTAMP value to be handled.
Signed-off-by Doug Berger <[email protected]>
Signature · 1 line
Florian
RE: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands
In reply to Doug BergerFrom: Doug Berger
Quoted 7 lines
Sent: 14 March 2017 00:42
This commit changes the ioctl handling behavior to return the EOPNOTSUPP error code instead of the EINVAL error code when an unknown ioctl command value is detected.It also removes some redundant parsing of the ioctl command value and allows the SIOCSHWTSTAMP value to be handled.
A better description would seem to be:
Remove checks on ioctl command and just forward all ioctl requests to phy_mii_ioctl().
I also thought the 'generic' response to an unknown ioctl command was ENOTTY.
David
Doug Berger
maintainerRe: [PATCH net-next 09/12] net: bcmgenet: return EOPNOTSUPP for unknown ioctl commands
Quoted 6 lines · David Laight
From: Doug Berger
Quoted 5 lines
Sent: 14 March 2017 00:42
This commit changes the ioctl handling behavior to return the EOPNOTSUPP error code instead of the EINVAL error code when an unknown ioctl command value is detected.
Quoted 6 lines
Quoted 2 lines
It also removes some redundant parsing of the ioctl command value and allows the SIOCSHWTSTAMP value to be handled.
A better description would seem to be:
Remove checks on ioctl command and just forward all ioctl requests tophy_mii_ioctl().
That is a good description of the code change, but I felt that was clearly conveyed by the patch content. I thought it would be a better use of the comment to describe the more subtle functional change that might be less clear.
Quoted 3 lines
I also thought the 'generic' response to an unknown ioctl command was ENOTTY.
and I think it probably helped solicit this feedback :). I would have thought that error makes more sense if there is no ioctl handler, but I will definitely look into it.
Thanks for the feedback,
Doug
Doug Berger
maintainerPATCH 10/12 dt-bindings: net: document bcmgenet WoL interrupt
In reply to Doug BergerA third interrupt cell can be provided to optionally specify the interrupt used for handling Wake on LAN events.
Typically the wake up handling uses a separate interrupt controller, so the interrupts-extended property is used to accommodate this.
--- Documentation/devicetree/bindings/net/brcm,bcmgenet.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +6 -3
@@ -4,9 +4,12 @@ Required properties: - compatible: should contain one of "brcm,genet-v1", "brcm,genet-v2", "brcm,genet-v3", "brcm,genet-v4". - reg: address and length of the register set for the device -- interrupts: must be two cells, the first cell is the general purpose - interrupt line, while the second cell is the interrupt for the ring - RX and TX queues operating in ring mode +- interrupts and/or interrupts-extended: must be two cells, the first cell + is the general purpose interrupt line, while the second cell is the + interrupt for the ring RX and TX queues operating in ring mode. An + optional third interrupt cell for Wake-on-LAN can be specified. + See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt + for information on the property specifics. - phy-mode: see ethernet.txt file in the same directory - #address-cells: should be 1 - #size-cells: should be 1
Signature · 1 line
2.11.1
Re: [PATCH net-next 10/12] dt-bindings: net: document bcmgenet WoL interrupt
Quoted 8 lines · Doug Berger
A third interrupt cell can be provided to optionally specify the interrupt used for handling Wake on LAN events.
Typically the wake up handling uses a separate interrupt controller, so the interrupts-extended property is used to accommodate this.
Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob Herring
maintainerRe: [PATCH net-next 10/12] dt-bindings: net: document bcmgenet WoL interrupt
In reply to Doug BergerQuoted 6 lines · Doug Berger
A third interrupt cell can be provided to optionally specify the interrupt used for handling Wake on LAN events.
Typically the wake up handling uses a separate interrupt controller, so the interrupts-extended property is used to accommodate this.
Using interrupts vs. interrupts-extended is beyond the scope of the binding doc. IOW, documenting interrupts is enough and using interrupts-extended is allowed.
Quoted 11 lines
Signed-off-by Doug Berger <[email protected]>--- Documentation/devicetree/bindings/net/brcm,bcmgenet.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +0
-0
Documentation/devicetree/bindings/net/brcm,bcmgenet.txt Required properties: · line 10
Quoted 10 lines
Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +4
-3@@ -4,9 +4,12 @@ Required properties: - compatible: should contain one of "brcm,genet-v1", "brcm,genet-v2", "brcm,genet-v3", "brcm,genet-v4". - reg: address and length of the register set for the device -- interrupts: must be two cells, the first cell is the general purpose - interrupt line, while the second cell is the interrupt for the ring - RX and TX queues operating in ring mode +- interrupts and/or interrupts-extended: must be two cells, the first cell + is the general purpose interrupt line, while the second cell is the + interrupt for the ring RX and TX queues operating in ring mode. An + optional third interrupt cell for Wake-on-LAN can be specified.
Does the generic wakeup source work for this?
Quoted 8 lines
Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +2
-0+ See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt + for information on the property specifics. - phy-mode: see ethernet.txt file in the same directory - #address-cells: should be 1 - #size-cells: should be 1Signature · 1 line
2.11.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Berger
maintainerPATCH 11/12 dt-bindings: net: update bcmgenet binding for GENETv5
In reply to Doug BergerThe device tree documentation must be updated to reflect the new compatible strings "brcm,genet-v5" and "brcm,genet-mdio-v5" used by the GENETv5 driver.
--- Documentation/devicetree/bindings/net/brcm,bcmgenet.txt | 10 +++++----- Documentation/devicetree/bindings/net/brcm,unimac-mdio.txt | 5 +++-- 2 files changed, 8 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/net/brcm,bcmgenet.txt +5 -5
@@ -2,7 +2,7 @@ Required properties: - compatible: should contain one of "brcm,genet-v1", "brcm,genet-v2", - "brcm,genet-v3", "brcm,genet-v4". + "brcm,genet-v3", "brcm,genet-v4", "brcm,genet-v5". - reg: address and length of the register set for the device - interrupts and/or interrupts-extended: must be two cells, the first cell is the general purpose interrupt line, while the second cell is the
@@ -32,15 +32,15 @@ Optional properties: Required child nodes: -- mdio bus node: this node should always be present regarless of the PHY +- mdio bus node: this node should always be present regardless of the PHY configuration of the GENET instance MDIO bus node required properties: - compatible: should contain one of "brcm,genet-mdio-v1", "brcm,genet-mdio-v2" - "brcm,genet-mdio-v3", "brcm,genet-mdio-v4", the version has to match the - parent node compatible property (e.g: brcm,genet-v4 pairs with - brcm,genet-mdio-v4) + "brcm,genet-mdio-v3", "brcm,genet-mdio-v4", "brcm,genet-mdio-v5", the version + has to match the parent node compatible property (e.g: brcm,genet-v4 pairs + with brcm,genet-mdio-v4) - reg: address and length relative to the parent node base register address - #address-cells: address cell for MDIO bus addressing, should be 1 - #size-cells: size of the cells for MDIO bus addressing, should be 0
Documentation/devicetree/bindings/net/brcm,unimac-mdio.txt +3 -2
@@ -2,8 +2,9 @@ Required properties: - compatible: should one from "brcm,genet-mdio-v1", "brcm,genet-mdio-v2", - "brcm,genet-mdio-v3", "brcm,genet-mdio-v4" or "brcm,unimac-mdio" -- reg: address and length of the regsiter set for the device, first one is the + "brcm,genet-mdio-v3", "brcm,genet-mdio-v4", "brcm,genet-mdio-v5" or + "brcm,unimac-mdio" +- reg: address and length of the register set for the device, first one is the base register, and the second one is optional and for indirect accesses to larger than 16-bits MDIO transactions - reg-names: name(s) of the register must be "mdio" and optional "mdio_indir_rw"
Signature · 1 line
2.11.1
Re: [PATCH net-next 11/12] dt-bindings: net: update bcmgenet binding for GENETv5
Quoted 4 lines · Doug Berger
The device tree documentation must be updated to reflect the new compatible strings "brcm,genet-v5" and "brcm,genet-mdio-v5" used by the GENETv5 driver.
Signed-off-by Doug Berger <[email protected]>
Signature · 1 line
Florian
Doug Berger
maintainerPATCH 12/12 net: bcmgenet: add support for the GENETv5 hardware
In reply to Doug BergerThis commit adds support for the GENETv5 implementation.
The GENETv5 reports a major version of 6 instead of 5 so compensate for this when verifying the configuration of the driver. Also the EPHY revision is now contained in the MDIO registers of the PHY so the EPHY revision of 0 in GENET_VER_FMT is expected for GENETv5.
--- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 91 ++++++++++++++++++++------ drivers/net/ethernet/broadcom/genet/bcmgenet.h | 12 +++- drivers/net/ethernet/broadcom/genet/bcmmii.c | 62 ++++++++++-------- drivers/net/phy/mdio-bcm-unimac.c | 3 +- 4 files changed, 118 insertions(+), 50 deletions(-)
drivers/net/ethernet/broadcom/genet/bcmgenet.c +72 -19
@@ -1011,8 +1011,17 @@ static int bcmgenet_power_down(struct bcmgenet_priv *priv,/* Power down LED */ if (priv->hw_params->flags &GENET_HAS_EXT ) { reg =bcmgenet_ext_readl (priv,EXT_EXT_PWR_MGMT ); - reg |= (EXT_PWR_DOWN_PHY | -EXT_PWR_DOWN_DLL |EXT_PWR_DOWN_BIAS ); +if (GENET_IS_V5 (priv)) + reg |=EXT_PWR_DOWN_PHY_EN | +EXT_PWR_DOWN_PHY_RD | +EXT_PWR_DOWN_PHY_SD | +EXT_PWR_DOWN_PHY_RX | +EXT_PWR_DOWN_PHY_TX | +EXT_IDDQ_GLBL_PWR ; +else + reg |=EXT_PWR_DOWN_PHY ; + + reg |= (EXT_PWR_DOWN_DLL |EXT_PWR_DOWN_BIAS );bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT );bcmgenet_phy_power_set (priv->dev, false);
@@ -1037,12 +1046,34 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,switch (mode) {case GENET_POWER_PASSIVE : - reg &= ~(EXT_PWR_DOWN_DLL |EXT_PWR_DOWN_PHY | -EXT_PWR_DOWN_BIAS ); -/* fallthrough */ + reg &= ~(EXT_PWR_DOWN_DLL |EXT_PWR_DOWN_BIAS ); +if (GENET_IS_V5 (priv)) { + reg &= ~(EXT_PWR_DOWN_PHY_EN | +EXT_PWR_DOWN_PHY_RD | +EXT_PWR_DOWN_PHY_SD | +EXT_PWR_DOWN_PHY_RX | +EXT_PWR_DOWN_PHY_TX | +EXT_IDDQ_GLBL_PWR ); + reg |=EXT_PHY_RESET ; +bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT ); +mdelay (1 ); + + reg &= ~EXT_PHY_RESET ; + }else { + reg &= ~EXT_PWR_DOWN_PHY ; + reg |=EXT_PWR_DN_EN_LD ; + } +bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT ); +bcmgenet_phy_power_set (priv->dev, true); +bcmgenet_mii_reset (priv->dev); +break ; +case GENET_POWER_CABLE_SENSE :/* enable APD */ - reg |=EXT_PWR_DN_EN_LD ; +if (!GENET_IS_V5 (priv)) { + reg |=EXT_PWR_DN_EN_LD ; +bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT ); + }break ;case GENET_POWER_WOL_MAGIC :bcmgenet_wol_power_up_cfg (priv, mode);
@@ -1050,12 +1081,6 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,default :break ; } - -bcmgenet_ext_writel (priv, reg,EXT_EXT_PWR_MGMT ); -if (mode ==GENET_POWER_PASSIVE ) { -bcmgenet_phy_power_set (priv->dev, true); -bcmgenet_mii_reset (priv->dev); - } }/* ioctl handle special commands that are not present in ethtool. */
@@ -3101,6 +3126,25 @@ static struct bcmgenet_hw_params bcmgenet_hw_params[] = { .flags = GENET_HAS_40BITS | GENET_HAS_EXT | GENET_HAS_MDIO_INTR | GENET_HAS_MOCA_LINK_DET, }, + [GENET_V5] = { + .tx_queues = 4, + .tx_bds_per_q = 32, + .rx_queues = 0, + .rx_bds_per_q = 0, + .bp_in_en_shift = 17, + .bp_in_mask = 0x1ffff, + .hfb_filter_cnt = 48, + .hfb_filter_size = 128, + .qtag_mask = 0x3F, + .tbuf_offset = 0x0600, + .hfb_offset = 0x8000, + .hfb_reg_offset = 0xfc00, + .rdma_offset = 0x2000, + .tdma_offset = 0x4000, + .words_per_bd = 3, + .flags =GENET_HAS_40BITS |GENET_HAS_EXT | + GENET_HAS_MDIO_INTR | GENET_HAS_MOCA_LINK_DET, + }, };/* Infer hardware parameters from the detected GENET version */
@@ -3111,7 +3155,7 @@ static void bcmgenet_set_hw_params(struct bcmgenet_priv *priv)u8 major;u16 gphy_rev; -if (GENET_IS_V4 (priv)) { +if (GENET_IS_V5 (priv) ||GENET_IS_V4 (priv)) { bcmgenet_dma_regs = bcmgenet_dma_regs_v3plus; genet_dma_ring_regs = genet_dma_ring_regs_v4; priv->dma_rx_chk_bit =DMA_RX_CHK_V3PLUS ;
@@ -3136,7 +3180,9 @@ static void bcmgenet_set_hw_params(struct bcmgenet_priv *priv)/* Read GENET HW version */ reg =bcmgenet_sys_readl (priv,SYS_REV_CTRL ); major = (reg >>24 &0x0f ); -if (major ==5 ) +if (major ==6 ) + major =5 ; +else if (major ==5 ) major =4 ;else if (major ==0 ) major =1 ;
@@ -3164,16 +3210,22 @@ static void bcmgenet_set_hw_params(struct bcmgenet_priv *priv)*/ gphy_rev = reg &0xffff ; +if (GENET_IS_V5 (priv)) { +/* The EPHY revision should come from the MDIO registers of +* the PHY not from GENET. +*/ +if (gphy_rev !=0 ) { +pr_warn ("GENET is reporting EPHY revision: 0x%04x\n" , + gphy_rev); + }/* This is the good old scheme, just GPHY major, no minor nor patch */ -if ((gphy_rev &0xf0 ) !=0 ) + }else if ((gphy_rev &0xf0 ) !=0 ) { priv->gphy_rev = gphy_rev <<8 ; -/* This is the new scheme, GPHY major rolls over with 0x10 = rev G0 */ -else if ((gphy_rev &0xff00 ) !=0 ) + }else if ((gphy_rev &0xff00 ) !=0 ) { priv->gphy_rev = gphy_rev; -/* This is reserved so should require special treatment */ -else if (gphy_rev ==0 || gphy_rev ==0x01ff ) { + }else if (gphy_rev ==0 || gphy_rev ==0x01ff ) {pr_warn ("Invalid GPHY revision detected: 0x%04x\n" , gphy_rev);return ; }
@@ -3206,6 +3258,7 @@ static const struct of_device_id bcmgenet_match[] = { { .compatible = "brcm,genet-v2", .data = (void *)GENET_V2 }, { .compatible = "brcm,genet-v3", .data = (void *)GENET_V3 }, { .compatible = "brcm,genet-v4", .data = (void *)GENET_V4 }, + { .compatible = "brcm,genet-v5", .data = (void *)GENET_V5 }, { }, };MODULE_DEVICE_TABLE (of, bcmgenet_match);
drivers/net/ethernet/broadcom/genet/bcmgenet.h +10 -2
@@ -1,5 +1,5 @@/* -* Copyright (c) 2014 Broadcom Corporation +* Copyright (c) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as
@@ -351,8 +351,14 @@ struct bcmgenet_mib_counters {#define EXT_PWR_DN_EN_LD (1 << 3)#define EXT_ENERGY_DET (1 << 4)#define EXT_IDDQ_FROM_PHY (1 << 5) +#define EXT_IDDQ_GLBL_PWR (1 << 7)#define EXT_PHY_RESET (1 << 8)#define EXT_ENERGY_DET_MASK (1 << 12) +#define EXT_PWR_DOWN_PHY_TX (1 << 16) +#define EXT_PWR_DOWN_PHY_RX (1 << 17) +#define EXT_PWR_DOWN_PHY_SD (1 << 18) +#define EXT_PWR_DOWN_PHY_RD (1 << 19) +#define EXT_PWR_DOWN_PHY_EN (1 << 20)#define EXT_RGMII_OOB_CTRL 0x0C#define RGMII_LINK (1 << 4)
@@ -495,13 +501,15 @@ enum bcmgenet_version {GENET_V1 =1 ,GENET_V2 ,GENET_V3 , - GENET_V4 +GENET_V4 , + GENET_V5 };#define GENET_IS_V1 (p) ((p)->version == GENET_V1)#define GENET_IS_V2 (p) ((p)->version == GENET_V2)#define GENET_IS_V3 (p) ((p)->version == GENET_V3)#define GENET_IS_V4 (p) ((p)->version == GENET_V4) +#define GENET_IS_V5 (p) ((p)->version == GENET_V5)/* Hardware flags */ #define GENET_HAS_40BITS (1 << 0)
drivers/net/ethernet/broadcom/genet/bcmmii.c +34 -28
90 lines not on this page · raw message
drivers/net/phy/mdio-bcm-unimac.c +2 -1
@@ -1,7 +1,7 @@/* * Broadcom UniMAC MDIO bus controller driver * -* Copyright (C) 2014, Broadcom Corporation +* Copyright (C) 2014-2017 Broadcom * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
@@ -228,6 +228,7 @@ static int unimac_mdio_remove(struct platform_device *pdev) }static const struct of_device_id unimac_mdio_ids[] = { + { .compatible ="brcm,genet-mdio-v5" , }, { .compatible ="brcm,genet-mdio-v4" , }, { .compatible ="brcm,genet-mdio-v3" , }, { .compatible ="brcm,genet-mdio-v2" , },
Signature · 1 line
2.11.1
Re: [PATCH net-next 12/12] net: bcmgenet: add support for the GENETv5 hardware
Quoted 8 lines · Doug Berger
This commit adds support for the GENETv5 implementation.
The GENETv5 reports a major version of 6 instead of 5 so compensate for this when verifying the configuration of the driver. Also the EPHY revision is now contained in the MDIO registers of the PHY so the EPHY revision of 0 in
GENET_VER_FMTis expected for GENETv5.Signed-off-by Doug Berger <[email protected]>
Signature · 5 lines
Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
maintainerFrom: Doug Berger <[email protected]>
Date: Mon, 13 Mar 2017 17:41:30 -0700
Quoted 12 lines
This collection of patches contains changes related to adding support for the BCM7260, BCM7268, and BCM7271 devices that contain a new version of the GENET MAC IP block (v5) and a new fast ethernet (10/100BASE-T) internal PHY.
These patches were originally developed on top of the bug fixes of the "[PATCH v2 net 0/8] net: bcmgenet: minor bug fixes" patch set previously accepted into the net repository, but this submission is designed to be applied to the current net-next that does not yet include them. As a result there will be some merge conflicts that I would be happy to help resolve if desired.
Quoted 5 lines
Specifically, conflicts should occur with these patches from the minor bug fixes set:
[PATCH v2 net 3/8] net: bcmgenet: reserved phy revisions must be checked first [PATCH v2 net 5/8] net: bcmgenet: synchronize irq0 status between the isr and task [PATCH v2 net 8/8] net: bcmgenet: decouple flow control frombcmgenet_tx_reclaim
Series applied, thanks Doug.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html