lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <8494e02c-6c04-46c9-af86-a414f27fcf23@lunn.ch> Date: Wed, 29 Mar 2023 00:45:37 +0200 From: Andrew Lunn <andrew@...n.ch> To: Daniel Golle <daniel@...rotopia.org> Cc: netdev@...r.kernel.org, linux-mediatek@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>, Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Matthias Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Sean Wang <sean.wang@...iatek.com>, Landen Chao <Landen.Chao@...iatek.com>, DENG Qingfang <dqfext@...il.com>, Philipp Zabel <p.zabel@...gutronix.de>, Sam Shih <Sam.Shih@...iatek.com>, Lorenzo Bianconi <lorenzo@...nel.org>, John Crispin <john@...ozen.org>, Felix Fietkau <nbd@....name> Subject: Re: [RFC PATCH net-next v2 2/2] net: dsa: mt7530: introduce MMIO driver for MT7988 SoC > @@ -146,11 +149,13 @@ core_write(struct mt7530_priv *priv, u32 reg, u32 val) > { > struct mii_bus *bus = priv->bus; > > - mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > + if (bus) > + mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > core_write_mmd_indirect(priv, reg, MDIO_MMD_VEND2, val); > > - mutex_unlock(&bus->mdio_lock); > + if (bus) > + mutex_unlock(&bus->mdio_lock); > } All this if (bus) is pretty ugly. First off, what is this mutex actually protecting? And why is the same protection not required for MMIO? If you have a convincing argument the mutex is not needed, please add two helpers: mt7530_mutex_lock(struct mt7530_priv *priv) mt7530_mutex_unlock(struct mt7530_priv *priv) and hide the if inside that. As part of the commit message, explain why the mutex is not needed for MDIO. Adding these helpers and changing all calls can be a preparation patch. It is the sort of patch which should be obviously correct. > @@ -2588,6 +2664,8 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode, > case PHY_INTERFACE_MODE_NA: > case PHY_INTERFACE_MODE_1000BASEX: > case PHY_INTERFACE_MODE_2500BASEX: > + case PHY_INTERFACE_MODE_USXGMII: > + case PHY_INTERFACE_MODE_10GKR: > /* handled in SGMII PCS driver */ > return 0; > default: > @@ -2712,7 +2790,9 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port, > * variants. > */ > if (interface == PHY_INTERFACE_MODE_TRGMII || > - (phy_interface_mode_is_8023z(interface))) { > + interface == PHY_INTERFACE_MODE_USXGMII || > + interface == PHY_INTERFACE_MODE_10GKR || > + phy_interface_mode_is_8023z(interface)) { > speed = SPEED_1000; > duplex = DUPLEX_FULL; > } This looks like something which should be in a separate patch. > +static int mt7988_setup(struct dsa_switch *ds) > +{ > + struct mt7530_priv *priv = ds->priv; > + u32 unused_pm = 0; > + int i; > + > + /* Reset the switch */ > + reset_control_assert(priv->rstc); > + udelay(20); > + reset_control_deassert(priv->rstc); > + udelay(20); > + > + /* Reset the switch PHYs */ > + mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST); > + > + /* BPDU to CPU port */ > + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK, > + BIT(MT7988_CPU_PORT)); > + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK, > + MT753X_BPDU_CPU_ONLY); > + > + /* Enable and reset MIB counters */ > + mt7530_mib_reset(ds); > + > + for (i = 0; i < MT7530_NUM_PORTS; i++) { > + /* Disable forwarding by default on all ports */ > + mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK, > + PCR_MATRIX_CLR); > + > + mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR); > + > + if (dsa_is_unused_port(ds, i)) > + unused_pm |= BIT(i); > + else if (dsa_is_cpu_port(ds, i)) > + mt753x_cpu_port_enable(ds, i); > + else > + mt7530_port_disable(ds, i); > + > + /* Enable consistent egress tag */ > + mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK, > + PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT)); > + } > + > + ds->configure_vlan_while_not_filtering = true; > + > + /* Flush the FDB table */ > + return mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL); Is this really specific to the mt7988? Andrew
Powered by blists - more mailing lists