| 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
| ||
|
Message-ID: <ZwilfpoFFHgcVr4K@lizhi-Precision-Tower-5810>
Date: Fri, 11 Oct 2024 00:11:42 -0400
From: Frank Li <Frank.li@....com>
To: Wei Fang <wei.fang@....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 Fri, Oct 11, 2024 at 02:02:03AM +0000, Wei Fang wrote:
> > -----Original Message-----
> > From: Frank Li <frank.li@....com>
> > Sent: 2024年10月10日 23:22
> > To: Wei Fang <wei.fang@....com>
> > Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> > pabeni@...hat.com; robh@...nel.org; krzk+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; linux@...linux.org.uk; bhelgaas@...gle.com;
> > imx@...ts.linux.dev; netdev@...r.kernel.org; devicetree@...r.kernel.org;
> > linux-kernel@...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 Thu, Oct 10, 2024 at 04:59:45AM +0000, Wei Fang wrote:
> > > > 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.
> >
> > Suggest use flags, such as, IS_SUPPORT_ETHTOOL.
> >
> > otherwise, your check may become complex in future.
> >
> > If use flags, you just change id table in future.
>
> enetc_ethtool just is an example, I meant that the ENETCv4 and ENETCv1
> use some common drivers, like enect_pf_common, enetc-core, so different
> hardware versions have different logic, that's all.
My means is that avoid use v1\v2 to distingiush it and use supported
features in difference version for example:
ENETC_FEATURE_1, ENETC_FEATURE_2, ENETC_FEATURE_3, ENETC_FEATURE_4.
{ PCI_DEVICE(PCI_VENDOR_ID_NXP2, PCI_DEVICE_ID_NXP2_ENETC_PF)
.driver_data = ENETC_FEATURE_1 | ENETC_FEATURE_2 | ENETC_FEATURE_4
PCI_DEVICE(....)
.driver_data = ENETC_FEATURE_1 | ENETC_FEATURE_3,
PCI_DEVICE(...)
.driver_data = ENETC_FEATURE_4,
)
It will be easy to know the difference between difference version. Your if
check logic will be simple.
if (driver_data & ENETC_FEATURE_1)
....
otherwise
if (vers == 1 || vers == 2 || ver == 5), which distribute to difference
places in whole code.
It is real hard to know hardware differences between version in future.
You can ref drivers/misc/pci_endpoint_test.c
Frank
>
> >
> > { PCI_DEVICE(PCI_VENDOR_ID_NXP2, PCI_DEVICE_ID_NXP2_ENETC_PF),
> > .driver_data = IS_SUPPORT_ETHTOOL | .... },
> >
> > Frank
> > >
> > > > > 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 ().
Powered by blists - more mailing lists