linux-kernel - RE: [PATCH v3 net-next 12/13] net: enetc: add preliminary support for i.MX95 ENETC PF

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:
 <PAXPR04MB8510E7AD1EEAFD9332DAE29788402@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Fri, 18 Oct 2024 02:04:06 +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>
Subject: RE: [PATCH v3 net-next 12/13] net: enetc: add preliminary support for
 i.MX95 ENETC PF

[...]
> > @@ -1721,14 +1724,25 @@ void enetc_get_si_caps(struct enetc_si *si)
> >  	struct enetc_hw *hw = &si->hw;
> >  	u32 val;
> >
> > +	if (is_enetc_rev1(si))
> > +		si->clk_freq = ENETC_CLK;
> > +	else
> > +		si->clk_freq = ENETC_CLK_333M;
> 
> can you use clk_gate_rate() to get frequency instead of hardcode here.

clk_gate_rate() is not possible to be used here, enetc_get_si_caps() is shared
by PF and VFs, but VF does not have DT node. Second, LS1028A and S32
platform do not use the clocks property.

> Or you should use standard PCIe version information.
> 

What do you mean standard PCIe version? is_enetc_rev1() gets the revision
from struct pci_dev:: revision, my understanding is that this is the revision
provided by PCIe.

[...]
> > +
> > @@ -593,6 +620,9 @@ static int enetc_get_rxnfc(struct net_device *ndev,
> struct ethtool_rxnfc *rxnfc,
> >  	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> >  	int i, j;
> >
> > +	if (is_enetc_rev4(priv->si))
> > +		return -EOPNOTSUPP;
> > +
> >  	switch (rxnfc->cmd) {
> >  	case ETHTOOL_GRXRINGS:
> >  		rxnfc->data = priv->num_rx_rings;
> > @@ -643,6 +673,9 @@ static int enetc_set_rxnfc(struct net_device *ndev,
> struct ethtool_rxnfc *rxnfc)
> >  	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> >  	int err;
> >
> > +	if (is_enetc_rev4(priv->si))
> > +		return -EOPNOTSUPP;
> > +
> >  	switch (rxnfc->cmd) {
> >  	case ETHTOOL_SRXCLSRLINS:
> >  		if (rxnfc->fs.location >= priv->si->num_fs_entries) @@ -678,6
> > +711,9 @@ static u32 enetc_get_rxfh_key_size(struct net_device *ndev)
> > {
> >  	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> >
> > @@ -843,8 +890,12 @@ static int enetc_set_coalesce(struct net_device
> > *ndev,  static int enetc_get_ts_info(struct net_device *ndev,
> >  			     struct kernel_ethtool_ts_info *info)  {
> > +	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> >  	int *phc_idx;
> >
> > +	if (is_enetc_rev4(priv->si))
> > +		return -EOPNOTSUPP;
> > +
> 
> Can you just not set enetc_pf_ethtool_ops if it is imx95 instead of check each
> ethtools function? or use difference enetc_pf_ethtool_ops for imx95?
> 

For the first question, in the current patch, i.MX95 already supports some
ethtool interfaces, so there is no need to remove them.

For the second question, for LS1028A and i.MX95, the logic of these ethtool
interfaces is basically the same, the difference is the hardware operation part.
It's just that support for i.MX95 has not yet been added. Both the current
approach and the approach you suggested will eventually merge into using the
same enetc_pf_ethtool_ops, so I don't think there is much practical point in
switching to the approach you mentioned.