netdev - RE: [PATCH v5 net-next 02/14] net: enetc: add command BD ring support for i.MX95 ENETC

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:
 <PAXPR04MB85105C88656FA179985C8F0D88BF2@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Fri, 18 Apr 2025 13:49:25 +0000
From: Wei Fang <wei.fang@....com>
To: Vladimir Oltean <olteanv@...il.com>
CC: Claudiu Manoil <claudiu.manoil@....com>, Vladimir Oltean
	<vladimir.oltean@....com>, Clark Wang <xiaoning.wang@....com>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "linuxppc-dev@...ts.ozlabs.org"
	<linuxppc-dev@...ts.ozlabs.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v5 net-next 02/14] net: enetc: add command BD ring support
 for i.MX95 ENETC

> -----Original Message-----
> From: Vladimir Oltean <olteanv@...il.com>
> Sent: 2025年4月18日 21:25
> To: Wei Fang <wei.fang@....com>
> Cc: Claudiu Manoil <claudiu.manoil@....com>; Vladimir Oltean
> <vladimir.oltean@....com>; Clark Wang <xiaoning.wang@....com>;
> andrew+netdev@...n.ch; davem@...emloft.net; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; christophe.leroy@...roup.eu;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; imx@...ts.linux.dev;
> linuxppc-dev@...ts.ozlabs.org; linux-arm-kernel@...ts.infradead.org
> Subject: Re: [PATCH v5 net-next 02/14] net: enetc: add command BD ring
> support for i.MX95 ENETC
> 
> On Fri, Apr 11, 2025 at 05:57:40PM +0800, Wei Fang wrote:
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
> b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
> > index 20bfdf7fb4b4..ecb571e5ea50 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
> > @@ -60,6 +60,45 @@ void enetc_teardown_cbdr(struct enetc_cbdr *cbdr)
> >  }
> >  EXPORT_SYMBOL_GPL(enetc_teardown_cbdr);
> >
> > +int enetc4_setup_cbdr(struct enetc_si *si)
> > +{
> > +	struct ntmp_user *user = &si->ntmp_user;
> > +	struct device *dev = &si->pdev->dev;
> > +	struct enetc_hw *hw = &si->hw;
> > +	struct netc_cbdr_regs regs;
> > +
> > +	user->cbdr_num = 1;
> > +	user->cbdr_size = NETC_CBDR_BD_NUM;
> > +	user->dev = dev;
> > +	user->ring = devm_kcalloc(dev, user->cbdr_num,
> > +				  sizeof(struct netc_cbdr), GFP_KERNEL);
> > +	if (!user->ring)
> > +		return -ENOMEM;
> > +
> > +	/* set CBDR cache attributes */
> > +	enetc_wr(hw, ENETC_SICAR2,
> > +		 ENETC_SICAR_RD_COHERENT | ENETC_SICAR_WR_COHERENT);
> > +
> > +	regs.pir = hw->reg + ENETC_SICBDRPIR;
> > +	regs.cir = hw->reg + ENETC_SICBDRCIR;
> > +	regs.mr = hw->reg + ENETC_SICBDRMR;
> > +	regs.bar0 = hw->reg + ENETC_SICBDRBAR0;
> > +	regs.bar1 = hw->reg + ENETC_SICBDRBAR1;
> > +	regs.lenr = hw->reg + ENETC_SICBDRLENR;
> > +
> > +	return netc_setup_cbdr(dev, user->cbdr_size, &regs, user->ring);
> > +}
> > +EXPORT_SYMBOL_GPL(enetc4_setup_cbdr);
> > +
> > +void enetc4_teardown_cbdr(struct enetc_si *si)
> > +{
> > +	struct ntmp_user *user = &si->ntmp_user;
> > +
> > +	netc_teardown_cbdr(user->dev, user->ring);
> > +	user->dev = NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(enetc4_teardown_cbdr);
> 
> I wanted to ask why isn't netc_setup_cbdr() merged into enetc4_setup_cbdr()
> (and likewise for teardown_cbdr), because they sound very similar, and
> they operate on the same data - one is literally a continuation of the
> other. Then I looked downstream where the netc_switch is another API
> user of netc_setup_cbdr() and netc_teardown_cbdr().
> 
> Do you think you could rename netc_setup_cbdr() into something like below:

Sure, I will rename them.

> 
> struct ntmp_user *ntmp_user_create(struct device *dev, size_t num_cbdr,
> 				   const struct netc_cbdr_regs *regs);
> void ntmp_user_destroy(struct ntmp_user *user);
> 
> From a data encapsulation perspective, it would be great if the outside
> world only worked with an opaque struct ntmp_user * pointer.
> 
> Hide NETC_CBDR_BD_NUM from include/linux/fsl/ntmp.h if API users don't
> need to customize it, and let ntmp_user_create() set it.
> 

Do we need to retain cbdr_size in struct ntmp_user? Or just remove it in
next version?