| Message ID | 20230411011354.2619359-3-pavan.kumar.linga@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce Intel IDPF driver | expand |
On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: > From: Phani Burra <phani.r.burra@intel.com> > > Add the required support to register IDPF PCI driver, as well as > probe and remove call backs. Enable the PCI device and request > the kernel to reserve the memory resources that will be used by the > driver. Finally map the BAR0 address space. > > PCI IDs table is intentionally left blank to prevent the kernel from > probing the device with the incomplete driver. It will be added > in the last patch of the series. > > Signed-off-by: Phani Burra <phani.r.burra@intel.com> > Co-developed-by: Alan Brady <alan.brady@intel.com> > Signed-off-by: Alan Brady <alan.brady@intel.com> > Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> > Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> > Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > Reviewed-by: Willem de Bruijn <willemb@google.com> > --- > drivers/net/ethernet/intel/Kconfig | 11 +++ > drivers/net/ethernet/intel/Makefile | 1 + > drivers/net/ethernet/intel/idpf/Makefile | 10 ++ > drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ > .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ > drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ > drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ > 7 files changed, 229 insertions(+) > create mode 100644 drivers/net/ethernet/intel/idpf/Makefile > create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c <...> > +/** > + * idpf_remove_common - Device removal routine > + * @pdev: PCI device information struct > + */ > +void idpf_remove_common(struct pci_dev *pdev) > +{ > + struct idpf_adapter *adapter = pci_get_drvdata(pdev); > + > + if (!adapter) How is it possible to have adapter be NULL here? > + return; > + > + pci_disable_pcie_error_reporting(pdev); > +} > diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c > new file mode 100644 > index 000000000000..617df9b924fa > --- /dev/null > +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (C) 2023 Intel Corporation */ > + > +#include "idpf.h" > + > +#define DRV_SUMMARY "Infrastructure Data Path Function Linux Driver" > + > +MODULE_DESCRIPTION(DRV_SUMMARY); > +MODULE_LICENSE("GPL"); > + > +/** > + * idpf_remove - Device removal routine > + * @pdev: PCI device information struct > + */ > +static void idpf_remove(struct pci_dev *pdev) > +{ > + struct idpf_adapter *adapter = pci_get_drvdata(pdev); > + > + if (!adapter) Ditto > + return; > + > + idpf_remove_common(pdev); > + pci_set_drvdata(pdev, NULL); > +} > + > +/** > + * idpf_shutdown - PCI callback for shutting down device > + * @pdev: PCI device information struct > + */ > +static void idpf_shutdown(struct pci_dev *pdev) > +{ > + idpf_remove(pdev); > + > + if (system_state == SYSTEM_POWER_OFF) > + pci_set_power_state(pdev, PCI_D3hot); > +} > + > +/** > + * idpf_probe - Device initialization routine > + * @pdev: PCI device information struct > + * @ent: entry in idpf_pci_tbl > + * > + * Returns 0 on success, negative on failure > + */ > +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > +{ > + struct idpf_adapter *adapter; > + > + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL); Why devm_kzalloc() and not kzalloc? > + if (!adapter) > + return -ENOMEM; > + > + return idpf_probe_common(pdev, adapter); There is no need in idpf_probe_common/idpf_remove_common functions and they better be embedded here. They called only once and just obfuscate the code. > +} > + > +/* idpf_pci_tbl - PCI Dev idpf ID Table > + */ > +static const struct pci_device_id idpf_pci_tbl[] = { > + { /* Sentinel */ } What does it mean empty pci_device_id table? > +}; > +MODULE_DEVICE_TABLE(pci, idpf_pci_tbl); > + > +static struct pci_driver idpf_driver = { > + .name = KBUILD_MODNAME, > + .id_table = idpf_pci_tbl, > + .probe = idpf_probe, > + .remove = idpf_remove, > + .shutdown = idpf_shutdown, > +}; > +module_pci_driver(idpf_driver); > -- > 2.37.3 >
On 4/11/2023 5:36 AM, Leon Romanovsky wrote: > On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: >> From: Phani Burra <phani.r.burra@intel.com> >> >> Add the required support to register IDPF PCI driver, as well as >> probe and remove call backs. Enable the PCI device and request >> the kernel to reserve the memory resources that will be used by the >> driver. Finally map the BAR0 address space. >> >> PCI IDs table is intentionally left blank to prevent the kernel from >> probing the device with the incomplete driver. It will be added >> in the last patch of the series. >> >> Signed-off-by: Phani Burra <phani.r.burra@intel.com> >> Co-developed-by: Alan Brady <alan.brady@intel.com> >> Signed-off-by: Alan Brady <alan.brady@intel.com> >> Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> >> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> >> Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >> Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >> Reviewed-by: Willem de Bruijn <willemb@google.com> >> --- >> drivers/net/ethernet/intel/Kconfig | 11 +++ >> drivers/net/ethernet/intel/Makefile | 1 + >> drivers/net/ethernet/intel/idpf/Makefile | 10 ++ >> drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ >> .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ >> drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ >> drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ >> 7 files changed, 229 insertions(+) >> create mode 100644 drivers/net/ethernet/intel/idpf/Makefile >> create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h >> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h >> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c >> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c > > <...> > >> +/** >> + * idpf_remove_common - Device removal routine >> + * @pdev: PCI device information struct >> + */ >> +void idpf_remove_common(struct pci_dev *pdev) >> +{ >> + struct idpf_adapter *adapter = pci_get_drvdata(pdev); >> + >> + if (!adapter) > > How is it possible to have adapter be NULL here? I think it is just defensive check since remove can be called on error code path, but in general you're right and we likely don't need it. Will remove in v3. > >> + return; >> + >> + pci_disable_pcie_error_reporting(pdev); >> +} >> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c >> new file mode 100644 >> index 000000000000..617df9b924fa >> --- /dev/null >> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c >> @@ -0,0 +1,70 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* Copyright (C) 2023 Intel Corporation */ >> + >> +#include "idpf.h" >> + >> +#define DRV_SUMMARY "Infrastructure Data Path Function Linux Driver" >> + >> +MODULE_DESCRIPTION(DRV_SUMMARY); >> +MODULE_LICENSE("GPL"); >> + >> +/** >> + * idpf_remove - Device removal routine >> + * @pdev: PCI device information struct >> + */ >> +static void idpf_remove(struct pci_dev *pdev) >> +{ >> + struct idpf_adapter *adapter = pci_get_drvdata(pdev); >> + >> + if (!adapter) > > Ditto Will remove in v3. >> + return; >> + >> + idpf_remove_common(pdev); >> + pci_set_drvdata(pdev, NULL); >> +} >> + >> +/** >> + * idpf_shutdown - PCI callback for shutting down device >> + * @pdev: PCI device information struct >> + */ >> +static void idpf_shutdown(struct pci_dev *pdev) >> +{ >> + idpf_remove(pdev); >> + >> + if (system_state == SYSTEM_POWER_OFF) >> + pci_set_power_state(pdev, PCI_D3hot); >> +} >> + >> +/** >> + * idpf_probe - Device initialization routine >> + * @pdev: PCI device information struct >> + * @ent: entry in idpf_pci_tbl >> + * >> + * Returns 0 on success, negative on failure >> + */ >> +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> +{ >> + struct idpf_adapter *adapter; >> + >> + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL); > > Why devm_kzalloc() and not kzalloc? It provides managed memory allocation on probe, which seems to be the preferred method in that case. >> + if (!adapter) >> + return -ENOMEM; >> + >> + return idpf_probe_common(pdev, adapter); > > There is no need in idpf_probe_common/idpf_remove_common functions and > they better be embedded here. They called only once and just obfuscate > the code. Will remove in v3. >> +} >> + >> +/* idpf_pci_tbl - PCI Dev idpf ID Table >> + */ >> +static const struct pci_device_id idpf_pci_tbl[] = { >> + { /* Sentinel */ } > > What does it mean empty pci_device_id table? Device ID's are added later, but it does make sense to be in this patch instead. Will fix in v3. >> +}; >> +MODULE_DEVICE_TABLE(pci, idpf_pci_tbl); >> + >> +static struct pci_driver idpf_driver = { >> + .name = KBUILD_MODNAME, >> + .id_table = idpf_pci_tbl, >> + .probe = idpf_probe, >> + .remove = idpf_remove, >> + .shutdown = idpf_shutdown, >> +}; >> +module_pci_driver(idpf_driver); >> -- >> 2.37.3 >>
On Wed, Apr 12, 2023 at 04:10:18PM -0700, Tantilov, Emil S wrote: > > > On 4/11/2023 5:36 AM, Leon Romanovsky wrote: > > On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: > > > From: Phani Burra <phani.r.burra@intel.com> > > > > > > Add the required support to register IDPF PCI driver, as well as > > > probe and remove call backs. Enable the PCI device and request > > > the kernel to reserve the memory resources that will be used by the > > > driver. Finally map the BAR0 address space. > > > > > > PCI IDs table is intentionally left blank to prevent the kernel from > > > probing the device with the incomplete driver. It will be added > > > in the last patch of the series. > > > > > > Signed-off-by: Phani Burra <phani.r.burra@intel.com> > > > Co-developed-by: Alan Brady <alan.brady@intel.com> > > > Signed-off-by: Alan Brady <alan.brady@intel.com> > > > Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> > > > Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> > > > Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > > > Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > > > Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > > > Reviewed-by: Willem de Bruijn <willemb@google.com> > > > --- > > > drivers/net/ethernet/intel/Kconfig | 11 +++ > > > drivers/net/ethernet/intel/Makefile | 1 + > > > drivers/net/ethernet/intel/idpf/Makefile | 10 ++ > > > drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ > > > .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ > > > drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ > > > drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ > > > 7 files changed, 229 insertions(+) > > > create mode 100644 drivers/net/ethernet/intel/idpf/Makefile > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c > > > > <...> <...> > > > +/** > > > + * idpf_probe - Device initialization routine > > > + * @pdev: PCI device information struct > > > + * @ent: entry in idpf_pci_tbl > > > + * > > > + * Returns 0 on success, negative on failure > > > + */ > > > +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > +{ > > > + struct idpf_adapter *adapter; > > > + > > > + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL); > > > > Why devm_kzalloc() and not kzalloc? > It provides managed memory allocation on probe, which seems to be the > preferred method in that case. I don't think so, as PCI probe/remove has very clear lifetime model and doesn't need garbage collection memory logic. In general, it is better to avoid devm_*() APIs as they hide error unwind flows. Thanks
On 4/12/2023 11:03 PM, Leon Romanovsky wrote: > On Wed, Apr 12, 2023 at 04:10:18PM -0700, Tantilov, Emil S wrote: >> >> >> On 4/11/2023 5:36 AM, Leon Romanovsky wrote: >>> On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: >>>> From: Phani Burra <phani.r.burra@intel.com> >>>> >>>> Add the required support to register IDPF PCI driver, as well as >>>> probe and remove call backs. Enable the PCI device and request >>>> the kernel to reserve the memory resources that will be used by the >>>> driver. Finally map the BAR0 address space. >>>> >>>> PCI IDs table is intentionally left blank to prevent the kernel from >>>> probing the device with the incomplete driver. It will be added >>>> in the last patch of the series. >>>> >>>> Signed-off-by: Phani Burra <phani.r.burra@intel.com> >>>> Co-developed-by: Alan Brady <alan.brady@intel.com> >>>> Signed-off-by: Alan Brady <alan.brady@intel.com> >>>> Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> >>>> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> >>>> Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >>>> Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >>>> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >>>> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >>>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >>>> Reviewed-by: Willem de Bruijn <willemb@google.com> >>>> --- >>>> drivers/net/ethernet/intel/Kconfig | 11 +++ >>>> drivers/net/ethernet/intel/Makefile | 1 + >>>> drivers/net/ethernet/intel/idpf/Makefile | 10 ++ >>>> drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ >>>> .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ >>>> drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ >>>> drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ >>>> 7 files changed, 229 insertions(+) >>>> create mode 100644 drivers/net/ethernet/intel/idpf/Makefile >>>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h >>>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h >>>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c >>>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c >>> >>> <...> > > <...> > >>>> +/** >>>> + * idpf_probe - Device initialization routine >>>> + * @pdev: PCI device information struct >>>> + * @ent: entry in idpf_pci_tbl >>>> + * >>>> + * Returns 0 on success, negative on failure >>>> + */ >>>> +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>>> +{ >>>> + struct idpf_adapter *adapter; >>>> + >>>> + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL); >>> >>> Why devm_kzalloc() and not kzalloc? >> It provides managed memory allocation on probe, which seems to be the >> preferred method in that case. > > I don't think so, as PCI probe/remove has very clear lifetime model and > doesn't need garbage collection memory logic. In general, it is better > to avoid devm_*() APIs as they hide error unwind flows. We'll remove it in v3. Thanks, Emil > Thanks
On 4/12/2023 4:10 PM, Tantilov, Emil S wrote: > > > On 4/11/2023 5:36 AM, Leon Romanovsky wrote: >> On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: >>> From: Phani Burra <phani.r.burra@intel.com> >>> >>> Add the required support to register IDPF PCI driver, as well as >>> probe and remove call backs. Enable the PCI device and request >>> the kernel to reserve the memory resources that will be used by the >>> driver. Finally map the BAR0 address space. >>> >>> PCI IDs table is intentionally left blank to prevent the kernel from >>> probing the device with the incomplete driver. It will be added >>> in the last patch of the series. >>> >>> Signed-off-by: Phani Burra <phani.r.burra@intel.com> >>> Co-developed-by: Alan Brady <alan.brady@intel.com> >>> Signed-off-by: Alan Brady <alan.brady@intel.com> >>> Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> >>> Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> >>> Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >>> Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> >>> Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >>> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> >>> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >>> Reviewed-by: Willem de Bruijn <willemb@google.com> >>> --- >>> drivers/net/ethernet/intel/Kconfig | 11 +++ >>> drivers/net/ethernet/intel/Makefile | 1 + >>> drivers/net/ethernet/intel/idpf/Makefile | 10 ++ >>> drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ >>> .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ >>> drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ >>> drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ >>> 7 files changed, 229 insertions(+) >>> create mode 100644 drivers/net/ethernet/intel/idpf/Makefile >>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h >>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h >>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c >>> create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c >> >> <...> >>> +} >>> + >>> +/* idpf_pci_tbl - PCI Dev idpf ID Table >>> + */ >>> +static const struct pci_device_id idpf_pci_tbl[] = { >>> + { /* Sentinel */ } >> >> What does it mean empty pci_device_id table? > > Device ID's are added later, but it does make sense to be in this patch > instead. Will fix in v3. On second look, the reason PCI ids are added in the last patch is because none of the modules from the previous patches would result in a functional driver. Thanks, Emil
On Thu, Apr 20, 2023 at 11:13:09AM -0700, Tantilov, Emil S wrote: > > > On 4/12/2023 4:10 PM, Tantilov, Emil S wrote: > > > > > > On 4/11/2023 5:36 AM, Leon Romanovsky wrote: > > > On Mon, Apr 10, 2023 at 06:13:41PM -0700, Pavan Kumar Linga wrote: > > > > From: Phani Burra <phani.r.burra@intel.com> > > > > > > > > Add the required support to register IDPF PCI driver, as well as > > > > probe and remove call backs. Enable the PCI device and request > > > > the kernel to reserve the memory resources that will be used by the > > > > driver. Finally map the BAR0 address space. > > > > > > > > PCI IDs table is intentionally left blank to prevent the kernel from > > > > probing the device with the incomplete driver. It will be added > > > > in the last patch of the series. > > > > > > > > Signed-off-by: Phani Burra <phani.r.burra@intel.com> > > > > Co-developed-by: Alan Brady <alan.brady@intel.com> > > > > Signed-off-by: Alan Brady <alan.brady@intel.com> > > > > Co-developed-by: Madhu Chittim <madhu.chittim@intel.com> > > > > Signed-off-by: Madhu Chittim <madhu.chittim@intel.com> > > > > Co-developed-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > > > > Signed-off-by: Shailendra Bhatnagar <shailendra.bhatnagar@intel.com> > > > > Co-developed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > > > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com> > > > > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > > > > Reviewed-by: Willem de Bruijn <willemb@google.com> > > > > --- > > > > drivers/net/ethernet/intel/Kconfig | 11 +++ > > > > drivers/net/ethernet/intel/Makefile | 1 + > > > > drivers/net/ethernet/intel/idpf/Makefile | 10 ++ > > > > drivers/net/ethernet/intel/idpf/idpf.h | 27 ++++++ > > > > .../net/ethernet/intel/idpf/idpf_controlq.h | 14 +++ > > > > drivers/net/ethernet/intel/idpf/idpf_lib.c | 96 +++++++++++++++++++ > > > > drivers/net/ethernet/intel/idpf/idpf_main.c | 70 ++++++++++++++ > > > > 7 files changed, 229 insertions(+) > > > > create mode 100644 drivers/net/ethernet/intel/idpf/Makefile > > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf.h > > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h > > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_lib.c > > > > create mode 100644 drivers/net/ethernet/intel/idpf/idpf_main.c > > > > > > <...> > > > > +} > > > > + > > > > +/* idpf_pci_tbl - PCI Dev idpf ID Table > > > > + */ > > > > +static const struct pci_device_id idpf_pci_tbl[] = { > > > > + { /* Sentinel */ } > > > > > > What does it mean empty pci_device_id table? > > > > Device ID's are added later, but it does make sense to be in this patch > > instead. Will fix in v3. > > On second look, the reason PCI ids are added in the last patch is because > none of the modules from the previous patches would result in a functional > driver. And yet patches should be split to meaningful and logical chunks. If you add pci_device_id, please add relevant device at the same patch. Thanks > > Thanks, > Emil
diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig index c18c3b373846..8c250326985f 100644 --- a/drivers/net/ethernet/intel/Kconfig +++ b/drivers/net/ethernet/intel/Kconfig @@ -373,4 +373,15 @@ config IGC To compile this driver as a module, choose M here. The module will be called igc. +config IDPF + tristate "Intel(R) Infrastructure Data Path Function Support" + depends on PCI_MSI + select DIMLIB + help + This driver supports Intel(R) Infrastructure Processing Unit (IPU) + devices. + + To compile this driver as a module, choose M here. The module + will be called idpf. + endif # NET_VENDOR_INTEL diff --git a/drivers/net/ethernet/intel/Makefile b/drivers/net/ethernet/intel/Makefile index 3075290063f6..70cc2ae1cb97 100644 --- a/drivers/net/ethernet/intel/Makefile +++ b/drivers/net/ethernet/intel/Makefile @@ -16,3 +16,4 @@ obj-$(CONFIG_IXGB) += ixgb/ obj-$(CONFIG_IAVF) += iavf/ obj-$(CONFIG_FM10K) += fm10k/ obj-$(CONFIG_ICE) += ice/ +obj-$(CONFIG_IDPF) += idpf/ diff --git a/drivers/net/ethernet/intel/idpf/Makefile b/drivers/net/ethernet/intel/idpf/Makefile new file mode 100644 index 000000000000..d26521c987e1 --- /dev/null +++ b/drivers/net/ethernet/intel/idpf/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-only +# Copyright (C) 2023 Intel Corporation + +# Makefile for Infrastructure Data Path Function Linux Driver + +obj-$(CONFIG_IDPF) += idpf.o + +idpf-y := \ + idpf_lib.o \ + idpf_main.o diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h new file mode 100644 index 000000000000..7494f9d29970 --- /dev/null +++ b/drivers/net/ethernet/intel/idpf/idpf.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright (C) 2023 Intel Corporation */ + +#ifndef _IDPF_H_ +#define _IDPF_H_ + +#include <linux/aer.h> +#include <linux/etherdevice.h> +#include <linux/pci.h> + +#include "idpf_controlq.h" + +/* available message levels */ +#define IDPF_AVAIL_NETIF_M (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK) + +struct idpf_adapter { + struct pci_dev *pdev; + u32 msg_enable; + struct idpf_hw hw; +}; + +int idpf_pf_probe(struct pci_dev *pdev, struct idpf_adapter *adapter); +int idpf_vf_probe(struct pci_dev *pdev, struct idpf_adapter *adapter); +int idpf_probe_common(struct pci_dev *pdev, struct idpf_adapter *adapter); +void idpf_remove_common(struct pci_dev *pdev); + +#endif /* !_IDPF_H_ */ diff --git a/drivers/net/ethernet/intel/idpf/idpf_controlq.h b/drivers/net/ethernet/intel/idpf/idpf_controlq.h new file mode 100644 index 000000000000..383089c91675 --- /dev/null +++ b/drivers/net/ethernet/intel/idpf/idpf_controlq.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright (C) 2023 Intel Corporation */ + +#ifndef _IDPF_CONTROLQ_H_ +#define _IDPF_CONTROLQ_H_ + +struct idpf_hw { + void __iomem *hw_addr; + resource_size_t hw_addr_len; + + void *back; +}; + +#endif /* _IDPF_CONTROLQ_H_ */ diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c new file mode 100644 index 000000000000..bf25574c75e4 --- /dev/null +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (C) 2023 Intel Corporation */ + +#include "idpf.h" + +/** + * idpf_cfg_hw - Initialize HW struct + * @adapter: adapter to setup hw struct for + * + * Returns 0 on success, negative on failure + */ +static int idpf_cfg_hw(struct idpf_adapter *adapter) +{ + struct pci_dev *pdev = adapter->pdev; + struct idpf_hw *hw = &adapter->hw; + + hw->hw_addr = pcim_iomap_table(pdev)[0]; + if (!hw->hw_addr) { + pci_err(pdev, "failed to allocate PCI iomap table\n"); + + return -ENOMEM; + } + + hw->back = adapter; + + return 0; +} + +/** + * idpf_probe_common - Device initialization routine + * @pdev: PCI device information struct + * @adapter: driver specific private structure + * + * Returns 0 on success, negative on failure + */ +int idpf_probe_common(struct pci_dev *pdev, struct idpf_adapter *adapter) +{ + struct device *dev = &pdev->dev; + int err; + + adapter->pdev = pdev; + + err = pcim_enable_device(pdev); + if (err) + return err; + + err = pcim_iomap_regions(pdev, BIT(0), pci_name(pdev)); + if (err) { + pci_err(pdev, "pcim_iomap_regions failed %pe\n", ERR_PTR(err)); + + return err; + } + + /* set up for high or low dma */ + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); + if (err) { + pci_err(pdev, "DMA configuration failed: %pe\n", ERR_PTR(err)); + + return err; + } + + pci_enable_pcie_error_reporting(pdev); + pci_set_master(pdev); + pci_set_drvdata(pdev, adapter); + + /* setup msglvl */ + adapter->msg_enable = netif_msg_init(-1, IDPF_AVAIL_NETIF_M); + + err = idpf_cfg_hw(adapter); + if (err) { + dev_err(dev, "Failed to configure HW structure for adapter: %d\n", + err); + goto err_cfg_hw; + } + + return 0; + +err_cfg_hw: + pci_disable_pcie_error_reporting(pdev); + + return err; +} + +/** + * idpf_remove_common - Device removal routine + * @pdev: PCI device information struct + */ +void idpf_remove_common(struct pci_dev *pdev) +{ + struct idpf_adapter *adapter = pci_get_drvdata(pdev); + + if (!adapter) + return; + + pci_disable_pcie_error_reporting(pdev); +} diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c new file mode 100644 index 000000000000..617df9b924fa --- /dev/null +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (C) 2023 Intel Corporation */ + +#include "idpf.h" + +#define DRV_SUMMARY "Infrastructure Data Path Function Linux Driver" + +MODULE_DESCRIPTION(DRV_SUMMARY); +MODULE_LICENSE("GPL"); + +/** + * idpf_remove - Device removal routine + * @pdev: PCI device information struct + */ +static void idpf_remove(struct pci_dev *pdev) +{ + struct idpf_adapter *adapter = pci_get_drvdata(pdev); + + if (!adapter) + return; + + idpf_remove_common(pdev); + pci_set_drvdata(pdev, NULL); +} + +/** + * idpf_shutdown - PCI callback for shutting down device + * @pdev: PCI device information struct + */ +static void idpf_shutdown(struct pci_dev *pdev) +{ + idpf_remove(pdev); + + if (system_state == SYSTEM_POWER_OFF) + pci_set_power_state(pdev, PCI_D3hot); +} + +/** + * idpf_probe - Device initialization routine + * @pdev: PCI device information struct + * @ent: entry in idpf_pci_tbl + * + * Returns 0 on success, negative on failure + */ +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +{ + struct idpf_adapter *adapter; + + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL); + if (!adapter) + return -ENOMEM; + + return idpf_probe_common(pdev, adapter); +} + +/* idpf_pci_tbl - PCI Dev idpf ID Table + */ +static const struct pci_device_id idpf_pci_tbl[] = { + { /* Sentinel */ } +}; +MODULE_DEVICE_TABLE(pci, idpf_pci_tbl); + +static struct pci_driver idpf_driver = { + .name = KBUILD_MODNAME, + .id_table = idpf_pci_tbl, + .probe = idpf_probe, + .remove = idpf_remove, + .shutdown = idpf_shutdown, +}; +module_pci_driver(idpf_driver);