Messages in this thread | | | Date | Sat, 12 Aug 2023 20:01:54 +0200 | From | Simon Horman <> | Subject | Re: [RFC net-next v1 3/5] net: phy: nxp-c45-tja11xx add MACsec support |
| |
On Fri, Aug 11, 2023 at 06:32:47PM +0300, Radu Pirea (NXP OSS) wrote: > Add MACsec support. > > Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
Hi Radu,
thanks for your patch-set. Some minor feedback from my side.
...
> diff --git a/drivers/net/phy/nxp-c45-tja11xx-macsec.c b/drivers/net/phy/nxp-c45-tja11xx-macsec.c
...
> +static int nxp_c45_tx_sc_set_flt(struct macsec_context *ctx, int secy_id) > +{ > + u32 tx_flt_base = TX_FLT_BASE(secy_id); > + const u8 *dev_addr = ctx->secy->netdev->dev_addr; > + u32 mac_sa;
nit: Please use reverse xmas tree - longest line to shortest ordering for local variable declarations in Networking code.
https://github.com/ecree-solarflare/xmastree is your friend here.
> + > + mac_sa = dev_addr[0] << 8 | dev_addr[1]; > + nxp_c45_macsec_write(ctx->phydev, > + TX_SC_FLT_MAC_DA_SA(tx_flt_base), > + mac_sa); > + mac_sa = dev_addr[5] | dev_addr[4] << 8 | > + dev_addr[3] << 16 | dev_addr[2] << 24; > + > + nxp_c45_macsec_write(ctx->phydev, > + TX_SC_FLT_MAC_SA(tx_flt_base), > + mac_sa); > + nxp_c45_macsec_write(ctx->phydev, > + TX_SC_FLT_MAC_CFG(tx_flt_base), > + TX_SC_FLT_BY_SA | TX_SC_FLT_EN | > + secy_id); > + > + return 0; > +} > + > +static bool nxp_c45_port_valid(struct nxp_c45_secy *phy_secy, u16 port) > +{ > + if (phy_secy->secy->tx_sc.end_station && __be16_to_cpu(port) != 1)
The type of port is host byte order, but it is assumed to be big endian by passing it to __be16_to_cpu(). Also, it's probably more efficient to convert the constant (1).
There are a number of other Sparse warnings introduced by this patch. Please take a look over them (and ideally fix them).
> + return false; > + > + return true; > +}
...
> +static int nxp_c45_mdo_upd_secy(struct macsec_context *ctx) > +{ > + struct phy_device *phydev = ctx->phydev; > + struct nxp_c45_phy *priv = phydev->priv; > + struct nxp_c45_tx_sa *new_tx_sa, *old_tx_sa;
nit: reverse xmas tree
...
| |