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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <PAXPR04MB851033EAFED9B2AB2A4CDBBB884D2@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Wed, 23 Oct 2024 01:57:10 +0000
From: Wei Fang <wei.fang@....com>
To: Frank Li <frank.li@....com>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, Vladimir Oltean <vladimir.oltean@....com>, Claudiu
 Manoil <claudiu.manoil@....com>, Clark Wang <xiaoning.wang@....com>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
	"linux@...linux.org.uk" <linux@...linux.org.uk>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "horms@...nel.org" <horms@...nel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "alexander.stein@...tq-group.com"
	<alexander.stein@...tq-group.com>
Subject: RE: [PATCH v4 net-next 12/13] net: enetc: add preliminary support for
 i.MX95 ENETC PF

> > @@ -1726,9 +1728,15 @@ void enetc_get_si_caps(struct enetc_si *si)
> >  	si->num_rx_rings = (val >> 16) & 0xff;
> >  	si->num_tx_rings = val & 0xff;
> >
> > -	val = enetc_rd(hw, ENETC_SIRFSCAPR);
> > -	si->num_fs_entries = ENETC_SIRFSCAPR_GET_NUM_RFS(val);
> > -	si->num_fs_entries = min(si->num_fs_entries, ENETC_MAX_RFS_SIZE);
> > +	val = enetc_rd(hw, ENETC_SIPCAPR0);
> > +	if (val & ENETC_SIPCAPR0_RFS) {
> > +		val = enetc_rd(hw, ENETC_SIRFSCAPR);
> > +		si->num_fs_entries = ENETC_SIRFSCAPR_GET_NUM_RFS(val);
> > +		si->num_fs_entries = min(si->num_fs_entries, ENETC_MAX_RFS_SIZE);
> > +	} else {
> > +		/* ENETC which not supports RFS */
> > +		si->num_fs_entries = 0;
> > +	}
> >
> >  	si->num_rss = 0;
> >  	val = enetc_rd(hw, ENETC_SIPCAPR0);
> > @@ -1742,8 +1750,11 @@ void enetc_get_si_caps(struct enetc_si *si)
> >  	if (val & ENETC_SIPCAPR0_QBV)
> >  		si->hw_features |= ENETC_SI_F_QBV;
> >
> > -	if (val & ENETC_SIPCAPR0_QBU)
> > +	if (val & ENETC_SIPCAPR0_QBU) {
> >  		si->hw_features |= ENETC_SI_F_QBU;
> > +		si->pmac_offset = is_enetc_rev1(si) ? ENETC_PMAC_OFFSET :
> > +						      ENETC4_PMAC_OFFSET;
> 
> pmac_offset should not relate with ENETC_SIPCAPR0_QBU.
> such data should be in drvdata generally.
> 
> pmac_offset always be ENETC_PMAC_OFFSET at ver1 and
> ENETC4_PMAC_OFFSET at rev4 regardless if support ENETC_SIPCAPR0_QBU.
> 


> > +	}
> >
> >  	if (val & ENETC_SIPCAPR0_PSFP)
> >  		si->hw_features |= ENETC_SI_F_PSFP; @@ -2056,7 +2067,7 @@ int
> > enetc_configure_si(struct enetc_ndev_priv *priv)
> >  	/* enable SI */
> >  	enetc_wr(hw, ENETC_SIMR, ENETC_SIMR_EN);
> >
> > -	if (si->num_rss) {
> > +	if (si->num_rss && is_enetc_rev1(si)) {
> >  		err = enetc_setup_default_rss_table(si, priv->num_rx_rings);
> >  		if (err)
> >  			return err;
> > @@ -2080,9 +2091,9 @@ void enetc_init_si_rings_params(struct
> enetc_ndev_priv *priv)
> >  	 */
> >  	priv->num_rx_rings = min_t(int, cpus, si->num_rx_rings);
> >  	priv->num_tx_rings = si->num_tx_rings;
> > -	priv->bdr_int_num = cpus;
> > +	priv->bdr_int_num = priv->num_rx_rings;
> >  	priv->ic_mode = ENETC_IC_RX_ADAPTIVE | ENETC_IC_TX_MANUAL;
> > -	priv->tx_ictt = ENETC_TXIC_TIMETHR;
> > +	priv->tx_ictt = enetc_usecs_to_cycles(600, priv->sysclk_freq);
> >  }
> >  EXPORT_SYMBOL_GPL(enetc_init_si_rings_params);
> >
> > @@ -2475,10 +2486,14 @@ int enetc_open(struct net_device *ndev)
> >
> >  	extended = !!(priv->active_offloads & ENETC_F_RX_TSTAMP);
> >
> > -	err = enetc_setup_irqs(priv);
> > +	err = clk_prepare_enable(priv->ref_clk);
> >  	if (err)
> >  		return err;
> >
> > +	err = enetc_setup_irqs(priv);
> > +	if (err)
> > +		goto err_setup_irqs;
> > +
> >  	err = enetc_phylink_connect(ndev);
> >  	if (err)
> >  		goto err_phy_connect;
> > @@ -2510,6 +2525,8 @@ int enetc_open(struct net_device *ndev)
> >  		phylink_disconnect_phy(priv->phylink);
> >  err_phy_connect:
> >  	enetc_free_irqs(priv);
> > +err_setup_irqs:
> > +	clk_disable_unprepare(priv->ref_clk);
> >
> >  	return err;
> >  }
> > @@ -2559,6 +2576,7 @@ int enetc_close(struct net_device *ndev)
> >  	enetc_assign_tx_resources(priv, NULL);
> >
> >  	enetc_free_irqs(priv);
> > +	clk_disable_unprepare(priv->ref_clk);
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> > b/drivers/net/ethernet/freescale/enetc/enetc.h
> > index 97524dfa234c..fe4bc082b3cf 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> > @@ -14,6 +14,7 @@
> >  #include <net/xdp.h>
> >
> >  #include "enetc_hw.h"
> > +#include "enetc4_hw.h"
> >
> >  #define ENETC_MAC_MAXFRM_SIZE	9600
> >  #define ENETC_MAX_MTU		(ENETC_MAC_MAXFRM_SIZE - \
> > @@ -247,10 +248,16 @@ struct enetc_si {
> >  	int num_rss; /* number of RSS buckets */
> >  	unsigned short pad;
> >  	int hw_features;
> > +	int pmac_offset; /* Only valid for PSI which supports 802.1Qbu */
> >  };
> >
> >  #define ENETC_SI_ALIGN	32
> >
> > +static inline bool is_enetc_rev1(struct enetc_si *si) {
> > +	return si->pdev->revision == ENETC_REV1; }
> > +
> >  static inline void *enetc_si_priv(const struct enetc_si *si)  {
> >  	return (char *)si + ALIGN(sizeof(struct enetc_si), ENETC_SI_ALIGN);
> > @@ -302,7 +309,7 @@ struct enetc_cls_rule {
> >  	int used;
> >  };
> >
> > -#define ENETC_MAX_BDR_INT	2 /* fixed to max # of available cpus */
> > +#define ENETC_MAX_BDR_INT	6 /* fixed to max # of available cpus */
> >  struct psfp_cap {
> >  	u32 max_streamid;
> >  	u32 max_psfp_filter;
> > @@ -340,7 +347,6 @@ enum enetc_ic_mode {
> >
> >  #define ENETC_RXIC_PKTTHR	min_t(u32, 256,
> ENETC_RX_RING_DEFAULT_SIZE / 2)
> >  #define ENETC_TXIC_PKTTHR	min_t(u32, 128,
> ENETC_TX_RING_DEFAULT_SIZE / 2)
> > -#define ENETC_TXIC_TIMETHR	enetc_usecs_to_cycles(600)
> >
> >  struct enetc_ndev_priv {
> >  	struct net_device *ndev;
> > @@ -388,6 +394,9 @@ struct enetc_ndev_priv {
> >  	 * and link state updates
> >  	 */
> >  	struct mutex		mm_lock;
> > +
> > +	struct clk *ref_clk; /* RGMII/RMII reference clock */
> > +	u64 sysclk_freq; /* NETC system clock frequency */
> 
> You can get ref_clk from dt, why not get sysclk from dt also and fetch frequency
> from clock provider instead of hardcode in driver.
> 

I explained it in the v3 patch, maybe you missed the reply.

There are two reasons are as follows.
The first reason is that ENETC VF does not have a DT node, so VF cannot get the system
clock from DT.

The second reason is that S32 platform also not use the clocks property in DT, so this
solution is not suitable for S32 platform. In addition, for i.MX platforms, there is a 1/2
divider inside the NETCMIX, the clock rate we get from clk_get_rate() is 666MHz, and
we need to divide by 2 to get the correct system clock rate. But S32 does not have a
NETCMIX so there may not have a 1/2 divider or may have other dividers, even if it
supports the clocks property, the calculation of getting the system clock rate is different.
Therefore, the hardcode based on the IP revision is simpler and clearer, and can be
shared by both PF and VFs.

> >  };
> > +static void enetc4_pf_reset_tc_msdu(struct enetc_hw *hw) {
> > +	u32 val = ENETC_MAC_MAXFRM_SIZE;
> > +	int tc;
> > +
> > +	val = u32_replace_bits(val, SDU_TYPE_MPDU, PTCTMSDUR_SDU_TYPE);
> > +
> > +	for (tc = 0; tc < 8; tc++)
> 
> hard code 8? can you macro for it?
> 

Okay, I will add a macro for the TC number.

> > +		enetc_port_wr(hw, ENETC4_PTCTMSDUR(tc), val); }
> > +
> > a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > index dfcaac302e24..2295742b7090 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > @@ -133,6 +133,8 @@ static void enetc_vf_netdev_setup(struct enetc_si *si,
> struct net_device *ndev,
> >  	ndev->watchdog_timeo = 5 * HZ;
> >  	ndev->max_mtu = ENETC_MAX_MTU;
> >
> > +	priv->sysclk_freq = ENETC_CLK_400M;
> > +
> 
> why vf is fixed at 400M?
> 

Because I have not added the VF support for i.MX95 ENETC, so the current VF
driver is still only available for LS1028A ENETC. I can add a comment here, so
as not to confuse others.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ