netdev - RE: [PATCH net-next 10/11] 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:
 <PAXPR04MB85100EB2E98527FCC4BAF89B88782@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Thu, 10 Oct 2024 04:59:45 +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>, "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 net-next 10/11] net: enetc: add preliminary support for
 i.MX95 ENETC PF

> On Wed, Oct 09, 2024 at 05:51:15PM +0800, Wei Fang wrote:
> > The i.MX95 ENETC has been upgraded to revision 4.1, which is very
> > different from the LS1028A ENETC (revision 1.0) except for the SI
> > part. Therefore, the fsl-enetc driver is incompatible with i.MX95
> > ENETC PF. So we developed the nxp-enetc4 driver for i.MX95 ENETC
>             So add new nxp-enetc4 driver for i.MX95 ENETC PF with
> major revision 4.
> 
> > PF, and this driver will be used to support the ENETC PF with major
> > revision 4 in the future.
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> b/drivers/net/ethernet/freescale/enetc/enetc.h
> > index 97524dfa234c..7f1ea11c33a0 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_SI_ALIGN	32
> >
> > +static inline bool is_enetc_rev1(struct enetc_si *si)
> > +{
> > +	return si->pdev->revision == ENETC_REV1;
> > +}
> > +
> > +static inline bool is_enetc_rev4(struct enetc_si *si)
> > +{
> > +	return si->pdev->revision == ENETC_REV4;
> > +}
> > +
> 
> Actually, I suggest you check features, instead of check version number.
> 
This is mainly used to distinguish between ENETC v1 and ENETC v4 in the
general interfaces. See enetc_ethtool.c.

> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > new file mode 100644
> > index 000000000000..e38ade76260b
> > --- /dev/null
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > @@ -0,0 +1,761 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> > +/* Copyright 2024 NXP */
> > +#include <linux/unaligned.h>
> > +#include <linux/module.h>
> > +#include <linux/of_net.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/clk.h>
> > +#include <linux/pinctrl/consumer.h>
> > +#include <linux/fsl/netc_global.h>
> 
> sort headers.
> 

Sure

> > +static int enetc4_pf_probe(struct pci_dev *pdev,
> > +			   const struct pci_device_id *ent)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct enetc_si *si;
> > +	struct enetc_pf *pf;
> > +	int err;
> > +
> > +	err = enetc_pci_probe(pdev, KBUILD_MODNAME, sizeof(*pf));
> > +	if (err) {
> > +		dev_err(dev, "PCIe probing failed\n");
> > +		return err;
> 
> use dev_err_probe()
> 

Okay

> > +	}
> > +
> > +	/* si is the private data. */
> > +	si = pci_get_drvdata(pdev);
> > +	if (!si->hw.port || !si->hw.global) {
> > +		err = -ENODEV;
> > +		dev_err(dev, "Couldn't map PF only space!\n");
> > +		goto err_enetc_pci_probe;
> > +	}
> > +
> > +	err = enetc4_pf_struct_init(si);
> > +	if (err)
> > +		goto err_pf_struct_init;
> > +
> > +	pf = enetc_si_priv(si);
> > +	err = enetc4_pf_init(pf);
> > +	if (err)
> > +		goto err_pf_init;
> > +
> > +	pinctrl_pm_select_default_state(dev);
> > +	enetc_get_si_caps(si);
> > +	err = enetc4_pf_netdev_create(si);
> > +	if (err)
> > +		goto err_netdev_create;
> > +
> > +	return 0;
> > +
> > +err_netdev_create:
> > +err_pf_init:
> > +err_pf_struct_init:
> > +err_enetc_pci_probe:
> > +	enetc_pci_remove(pdev);
> 
> you can use devm_add_action_or_reset() to remove these goto labels.
> 
Subsequent patches will have corresponding processing for these labels,
so I don't want to add too many devm_add_action_or_reset ().