diff mbox series

[11/15] soc: octeontx2: Add Marvell OcteonTX2 CGX driver

Message ID 1535453838-12154-12-git-send-email-sunil.kovvuri@gmail.com (mailing list archive)
State New, archived
Headers show
Series soc: octeontx2: Add RVU admin function driver | expand

Commit Message

Sunil Kovvuri Aug. 28, 2018, 10:57 a.m. UTC
From: Sunil Goutham <sgoutham@marvell.com>

This patch adds basic template for Marvell OcteonTX2's
CGX ethernet interface driver. Just the probe.
RVU AF driver will use APIs exported by this driver
for various things like PF to physical interface mapping,
loopback mode, interface stats etc.

Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
 drivers/soc/marvell/Kconfig            |  10 +++
 drivers/soc/marvell/octeontx2/Makefile |   2 +
 drivers/soc/marvell/octeontx2/cgx.c    | 117 +++++++++++++++++++++++++++++++++
 drivers/soc/marvell/octeontx2/cgx.h    |  20 ++++++
 4 files changed, 149 insertions(+)
 create mode 100644 drivers/soc/marvell/octeontx2/cgx.c
 create mode 100644 drivers/soc/marvell/octeontx2/cgx.h

Comments

Arnd Bergmann Aug. 28, 2018, 12:10 p.m. UTC | #1
On Tue, Aug 28, 2018 at 12:58 PM <sunil.kovvuri@gmail.com> wrote:
>
> From: Sunil Goutham <sgoutham@marvell.com>
>
> This patch adds basic template for Marvell OcteonTX2's
> CGX ethernet interface driver. Just the probe.
> RVU AF driver will use APIs exported by this driver
> for various things like PF to physical interface mapping,
> loopback mode, interface stats etc.
>
> Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> ---
>  drivers/soc/marvell/Kconfig            |  10 +++
>  drivers/soc/marvell/octeontx2/Makefile |   2 +
>  drivers/soc/marvell/octeontx2/cgx.c    | 117 +++++++++++++++++++++++++++++++++
>  drivers/soc/marvell/octeontx2/cgx.h    |  20 ++++++

If this is a regular PCI ethernet driver, why do you put it into driver/soc
rather than drivers/net/ethernet/ ?

      Arnd
Sunil Kovvuri Aug. 28, 2018, 12:30 p.m. UTC | #2
On Tue, Aug 28, 2018 at 5:40 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 28, 2018 at 12:58 PM <sunil.kovvuri@gmail.com> wrote:
> >
> > From: Sunil Goutham <sgoutham@marvell.com>
> >
> > This patch adds basic template for Marvell OcteonTX2's
> > CGX ethernet interface driver. Just the probe.
> > RVU AF driver will use APIs exported by this driver
> > for various things like PF to physical interface mapping,
> > loopback mode, interface stats etc.
> >
> > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > ---
> >  drivers/soc/marvell/Kconfig            |  10 +++
> >  drivers/soc/marvell/octeontx2/Makefile |   2 +
> >  drivers/soc/marvell/octeontx2/cgx.c    | 117 +++++++++++++++++++++++++++++++++
> >  drivers/soc/marvell/octeontx2/cgx.h    |  20 ++++++
>
> If this is a regular PCI ethernet driver, why do you put it into driver/soc
> rather than drivers/net/ethernet/ ?

No, this is not a ethernet driver, as mentioned in the cover letter
this driver and AF driver doesn't
handle any IO. There will be a separate ethernet driver (will submit
that as well in future) which will
communicate with these drivers for configuring hardware.

The driver in question here is for a serdes controller which handles
physical ethernet interfaces.
Admin function driver gathers info w.r.t current state of physical
ethernet interfaces from this driver
and notifies actual ethernet driver about changes, if any.

Thanks,
Sunil.

>
>       Arnd
Arnd Bergmann Aug. 28, 2018, 12:48 p.m. UTC | #3
On Tue, Aug 28, 2018 at 2:30 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 5:40 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Aug 28, 2018 at 12:58 PM <sunil.kovvuri@gmail.com> wrote:
> > >
> > > From: Sunil Goutham <sgoutham@marvell.com>
> > >
> > > This patch adds basic template for Marvell OcteonTX2's
> > > CGX ethernet interface driver. Just the probe.
> > > RVU AF driver will use APIs exported by this driver
> > > for various things like PF to physical interface mapping,
> > > loopback mode, interface stats etc.
> > >
> > > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > > ---
> > >  drivers/soc/marvell/Kconfig            |  10 +++
> > >  drivers/soc/marvell/octeontx2/Makefile |   2 +
> > >  drivers/soc/marvell/octeontx2/cgx.c    | 117 +++++++++++++++++++++++++++++++++
> > >  drivers/soc/marvell/octeontx2/cgx.h    |  20 ++++++
> >
> > If this is a regular PCI ethernet driver, why do you put it into driver/soc
> > rather than drivers/net/ethernet/ ?
>
> No, this is not a ethernet driver, as mentioned in the cover letter
> this driver and AF driver doesn't
> handle any IO. There will be a separate ethernet driver (will submit
> that as well in future) which will
> communicate with these drivers for configuring hardware.
>
> The driver in question here is for a serdes controller which handles
> physical ethernet interfaces.
> Admin function driver gathers info w.r.t current state of physical
> ethernet interfaces from this driver
> and notifies actual ethernet driver about changes, if any.

Ok. Can you describe the structure that the PCI devices appear
in? It might help to be make the connection between the differnet
patches to understand how things fit together. In the final
picture, how many different pci_driver instances do you have,
and what part are they for?

Is the idea that an ethernet device driver always attaches to a
virtual function that gets created by the main driver, and that
the two drivers share no interfaces on the kernel side, or do
you have multiple drivers linking to each other?

      Arnd
Sunil Kovvuri Aug. 28, 2018, 1:09 p.m. UTC | #4
> > > If this is a regular PCI ethernet driver, why do you put it into driver/soc
> > > rather than drivers/net/ethernet/ ?
> >
> > No, this is not a ethernet driver, as mentioned in the cover letter
> > this driver and AF driver doesn't
> > handle any IO. There will be a separate ethernet driver (will submit
> > that as well in future) which will
> > communicate with these drivers for configuring hardware.
> >
> > The driver in question here is for a serdes controller which handles
> > physical ethernet interfaces.
> > Admin function driver gathers info w.r.t current state of physical
> > ethernet interfaces from this driver
> > and notifies actual ethernet driver about changes, if any.
>
> Ok. Can you describe the structure that the PCI devices appear
> in? It might help to be make the connection between the differnet
> patches to understand how things fit together. In the final
> picture, how many different pci_driver instances do you have,
> and what part are they for?

List of PCI devices are CGX, RVU PF0-PFn SRIOV physical functions
and RVU VF0-VFn SRIOV virtual functions. No of VFs per PF is configurable
and done by low level firmware.

List of PCI driver instances would be CGX driver, RVU PF0 (i.e admin
function) driver,
PF1-PFn either netdev driver or crypto driver, VF0-VFn functionality would be
same as their PF.

The current plan is to have CGX driver, Admin function driver, PF
netdev driver,
VF netdev driver and PF/VF crypto drivers.

>
> Is the idea that an ethernet device driver always attaches to a
> virtual function that gets created by the main driver, and that
> the two drivers share no interfaces on the kernel side, or do
> you have multiple drivers linking to each other?

Ethernet device driver can attach to both physical function and virtual function
whose HW resources are provisioned by admin function driver.

Yes the PF/VF ethernet drivers and these drivers won't share any
kernel interfaces.
Physical ethernet interface is owned by ethernet driver only, this driver just
configures which ethernet driver instance uses which physcial interface.

>
>       Arnd
Arnd Bergmann Aug. 30, 2018, 2:07 p.m. UTC | #5
On Tue, Aug 28, 2018 at 3:10 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> > > > If this is a regular PCI ethernet driver, why do you put it into driver/soc
> > > > rather than drivers/net/ethernet/ ?
> > >
> > > No, this is not a ethernet driver, as mentioned in the cover letter
> > > this driver and AF driver doesn't
> > > handle any IO. There will be a separate ethernet driver (will submit
> > > that as well in future) which will
> > > communicate with these drivers for configuring hardware.
> > >
> > > The driver in question here is for a serdes controller which handles
> > > physical ethernet interfaces.
> > > Admin function driver gathers info w.r.t current state of physical
> > > ethernet interfaces from this driver
> > > and notifies actual ethernet driver about changes, if any.
> >
> > Ok. Can you describe the structure that the PCI devices appear
> > in? It might help to be make the connection between the differnet
> > patches to understand how things fit together. In the final
> > picture, how many different pci_driver instances do you have,
> > and what part are they for?
>
> List of PCI devices are CGX, RVU PF0-PFn SRIOV physical functions
> and RVU VF0-VFn SRIOV virtual functions. No of VFs per PF is configurable
> and done by low level firmware.
>
> List of PCI driver instances would be CGX driver, RVU PF0 (i.e admin
> function) driver,
> PF1-PFn either netdev driver or crypto driver, VF0-VFn functionality would be
> same as their PF.
>
> The current plan is to have CGX driver, Admin function driver, PF
> netdev driver,
> VF netdev driver and PF/VF crypto drivers.

Ok, I think I understand the PF/VF distinction now. One (to me)
surprising aspect here is that you not just have one physical function
that you can use to assign resources to multiple virtual functions,
but also a second level of virtualization that is used to assign
resources to "physical functions" that are less physical than the
name suggests.

The part that I have not grasped yet is what the split between
the CGX and the AF is for, how they relate to one another, and
what the software abstraction for the two is going to be.

> > Is the idea that an ethernet device driver always attaches to a
> > virtual function that gets created by the main driver, and that
> > the two drivers share no interfaces on the kernel side, or do
> > you have multiple drivers linking to each other?
>
> Ethernet device driver can attach to both physical function and virtual function
> whose HW resources are provisioned by admin function driver.
>
> Yes the PF/VF ethernet drivers and these drivers won't share any
> kernel interfaces.
> Physical ethernet interface is owned by ethernet driver only, this driver just
> configures which ethernet driver instance uses which physcial interface.

Ok.

         Arnd
Sunil Kovvuri Aug. 30, 2018, 5:55 p.m. UTC | #6
On Thu, Aug 30, 2018 at 7:37 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 28, 2018 at 3:10 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > > > > If this is a regular PCI ethernet driver, why do you put it into driver/soc
> > > > > rather than drivers/net/ethernet/ ?
> > > >
> > > > No, this is not a ethernet driver, as mentioned in the cover letter
> > > > this driver and AF driver doesn't
> > > > handle any IO. There will be a separate ethernet driver (will submit
> > > > that as well in future) which will
> > > > communicate with these drivers for configuring hardware.
> > > >
> > > > The driver in question here is for a serdes controller which handles
> > > > physical ethernet interfaces.
> > > > Admin function driver gathers info w.r.t current state of physical
> > > > ethernet interfaces from this driver
> > > > and notifies actual ethernet driver about changes, if any.
> > >
> > > Ok. Can you describe the structure that the PCI devices appear
> > > in? It might help to be make the connection between the differnet
> > > patches to understand how things fit together. In the final
> > > picture, how many different pci_driver instances do you have,
> > > and what part are they for?
> >
> > List of PCI devices are CGX, RVU PF0-PFn SRIOV physical functions
> > and RVU VF0-VFn SRIOV virtual functions. No of VFs per PF is configurable
> > and done by low level firmware.
> >
> > List of PCI driver instances would be CGX driver, RVU PF0 (i.e admin
> > function) driver,
> > PF1-PFn either netdev driver or crypto driver, VF0-VFn functionality would be
> > same as their PF.
> >
> > The current plan is to have CGX driver, Admin function driver, PF
> > netdev driver, VF netdev driver and PF/VF crypto drivers.
>
> Ok, I think I understand the PF/VF distinction now. One (to me)
> surprising aspect here is that you not just have one physical function
> that you can use to assign resources to multiple virtual functions,
> but also a second level of virtualization that is used to assign
> resources to "physical functions" that are less physical than the
> name suggests.

Yes, PF is just for name sake, on-boot there is no difference between
PFs/VFs as such.
PF0 has privilege access to assign resources to all PFs and their VFs.
This admin function driver loads for PF0.

>
> The part that I have not grasped yet is what the split between
> the CGX and the AF is for, how they relate to one another, and
> what the software abstraction for the two is going to be.

In HW, CGX is a separate PCI device which handles the serdes and
physical ethernet interface.
Ethernet driver in drivers/net/ethernet can only communicate to
admin function driver since they share a mailbox memory.
So we had to bind both CGX and admin function drivers to almost work as one,
inorder to provide relavent info to ethernet drivers. That's why we
have many functions
from CGX driver which AF uses.

eg: Firmware gets to know about a physical interface status change,
which CGX driver gets
to know and it uses AF's mailbox communication to inform ethernet
driver about the event.

>
> > > Is the idea that an ethernet device driver always attaches to a
> > > virtual function that gets created by the main driver, and that
> > > the two drivers share no interfaces on the kernel side, or do
> > > you have multiple drivers linking to each other?
> >
> > Ethernet device driver can attach to both physical function and virtual function
> > whose HW resources are provisioned by admin function driver.
> >
> > Yes the PF/VF ethernet drivers and these drivers won't share any
> > kernel interfaces.
> > Physical ethernet interface is owned by ethernet driver only, this driver just
> > configures which ethernet driver instance uses which physcial interface.
>
> Ok.
>
>          Arnd
Arnd Bergmann Aug. 31, 2018, 2:20 p.m. UTC | #7
On Thu, Aug 30, 2018 at 7:55 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Thu, Aug 30, 2018 at 7:37 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Aug 28, 2018 at 3:10 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > Ok, I think I understand the PF/VF distinction now. One (to me)
> > surprising aspect here is that you not just have one physical function
> > that you can use to assign resources to multiple virtual functions,
> > but also a second level of virtualization that is used to assign
> > resources to "physical functions" that are less physical than the
> > name suggests.
>
> Yes, PF is just for name sake, on-boot there is no difference between
> PFs/VFs as such.
> PF0 has privilege access to assign resources to all PFs and their VFs.
> This admin function driver loads for PF0.

ok

> > The part that I have not grasped yet is what the split between
> > the CGX and the AF is for, how they relate to one another, and
> > what the software abstraction for the two is going to be.
>
> In HW, CGX is a separate PCI device which handles the serdes and
> physical ethernet interface.
> Ethernet driver in drivers/net/ethernet can only communicate to
> admin function driver since they share a mailbox memory.
> So we had to bind both CGX and admin function drivers to almost work as one,
> inorder to provide relavent info to ethernet drivers. That's why we
> have many functions
> from CGX driver which AF uses.
>
> eg: Firmware gets to know about a physical interface status change,
> which CGX driver gets
> to know and it uses AF's mailbox communication to inform ethernet
> driver about the event.

Would it make sense then to combine the CGX driver and the AF
driver into a single module? It sounds like you can never really
use one without the other anyway, and that would make it easier
to have a sensible abstraction to user space.

      Arnd
Sunil Kovvuri Aug. 31, 2018, 4 p.m. UTC | #8
On Fri, Aug 31, 2018 at 7:50 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Aug 30, 2018 at 7:55 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > On Thu, Aug 30, 2018 at 7:37 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tue, Aug 28, 2018 at 3:10 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > > Ok, I think I understand the PF/VF distinction now. One (to me)
> > > surprising aspect here is that you not just have one physical function
> > > that you can use to assign resources to multiple virtual functions,
> > > but also a second level of virtualization that is used to assign
> > > resources to "physical functions" that are less physical than the
> > > name suggests.
> >
> > Yes, PF is just for name sake, on-boot there is no difference between
> > PFs/VFs as such.
> > PF0 has privilege access to assign resources to all PFs and their VFs.
> > This admin function driver loads for PF0.
>
> ok
>
> > > The part that I have not grasped yet is what the split between
> > > the CGX and the AF is for, how they relate to one another, and
> > > what the software abstraction for the two is going to be.
> >
> > In HW, CGX is a separate PCI device which handles the serdes and
> > physical ethernet interface.
> > Ethernet driver in drivers/net/ethernet can only communicate to
> > admin function driver since they share a mailbox memory.
> > So we had to bind both CGX and admin function drivers to almost work as one,
> > inorder to provide relavent info to ethernet drivers. That's why we
> > have many functions
> > from CGX driver which AF uses.
> >
> > eg: Firmware gets to know about a physical interface status change,
> > which CGX driver gets
> > to know and it uses AF's mailbox communication to inform ethernet
> > driver about the event.
>
> Would it make sense then to combine the CGX driver and the AF
> driver into a single module? It sounds like you can never really
> use one without the other anyway, and that would make it easier
> to have a sensible abstraction to user space.
>
>       Arnd

Thanks for the suggestion, that does makes sense.
Actually i did thought about it, but i was skeptical if it would be
acceptable to make
a single module out of drivers registering for two different PCI devices.

Will wait for few more days for more feedback from anyone and port v2 series.

Sunil.
Arnd Bergmann Aug. 31, 2018, 6:29 p.m. UTC | #9
On Fri, Aug 31, 2018 at 6:01 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Aug 31, 2018 at 7:50 PM Arnd Bergmann <arnd@arndb.de> wrote:

>
> Thanks for the suggestion, that does makes sense.
> Actually i did thought about it, but i was skeptical if it would be
> acceptable to make
> a single module out of drivers registering for two different PCI devices.

I don't think it matters much whether there is one module or two
(others might have a strong opinion one way or the other).

What is important though are these two points:

- How to represent the two PCI devices to user space: You should only
  have one interface to user space I think, and this should be similar
  to how other drivers manage similar cases (I don't actually know what they
  do, but I assume you've done some research here)

- How you connect find the pair of devices: Generally speaking while
  the SoC might only have one of each, you shouldn't make that assumption
  in the code, but instead have a reliable way of having one driver wait
  for the other driver to finish probing so you can match the pair.

> Will wait for few more days for more feedback from anyone and port v2 series.

Ok.

      Arnd
Sunil Kovvuri Aug. 31, 2018, 6:35 p.m. UTC | #10
On Fri, Aug 31, 2018 at 11:59 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Aug 31, 2018 at 6:01 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > On Fri, Aug 31, 2018 at 7:50 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> >
> > Thanks for the suggestion, that does makes sense.
> > Actually i did thought about it, but i was skeptical if it would be
> > acceptable to make
> > a single module out of drivers registering for two different PCI devices.
>
> I don't think it matters much whether there is one module or two
> (others might have a strong opinion one way or the other).
>
> What is important though are these two points:
>
> - How to represent the two PCI devices to user space: You should only
>   have one interface to user space I think, and this should be similar
>   to how other drivers manage similar cases (I don't actually know what they
>   do, but I assume you've done some research here)
>
> - How you connect find the pair of devices: Generally speaking while
>   the SoC might only have one of each, you shouldn't make that assumption
>   in the code, but instead have a reliable way of having one driver wait
>   for the other driver to finish probing so you can match the pair.
>

Agreed.
I do have a patch in works for that, i.e to defer AF driver probe till
CGX driver is loaded.
https://github.com/sunilkovvuri/rvu_drivers/commit/27b449087c16224a9e93dd91d6d9f734b5210bed

> > Will wait for few more days for more feedback from anyone and port v2 series.
>
> Ok.
>
>       Arnd
diff mbox series

Patch

diff --git a/drivers/soc/marvell/Kconfig b/drivers/soc/marvell/Kconfig
index 4499caf..73c8f8d 100644
--- a/drivers/soc/marvell/Kconfig
+++ b/drivers/soc/marvell/Kconfig
@@ -7,7 +7,17 @@  menu "Marvell SoC drivers"
 config OCTEONTX2_AF
 	tristate "OcteonTX2 RVU Admin Function driver"
 	depends on ARM64 && PCI
+	select OCTEONTX2_CGX
 	help
 	  This driver supports Marvell's OcteonTX2 Resource Virtualization
 	  Unit's admin function manager which manages all RVU HW resources.
+
+config OCTEONTX2_CGX
+	tristate "OcteonTX2 MAC interface (CGX) driver"
+	depends on ARM64 && PCI
+	select PHYLIB
+	help
+	  This driver supports programming and controlling of MAC
+	  interfaces from RVU Admin Function driver.
+
 endmenu
diff --git a/drivers/soc/marvell/octeontx2/Makefile b/drivers/soc/marvell/octeontx2/Makefile
index 8737ec3..50b8f74 100644
--- a/drivers/soc/marvell/octeontx2/Makefile
+++ b/drivers/soc/marvell/octeontx2/Makefile
@@ -3,6 +3,8 @@ 
 # Makefile for Marvell's OcteonTX2 RVU Admin Function driver
 #
 
+obj-$(CONFIG_OCTEONTX2_CGX) += octeontx2_cgx.o
 obj-$(CONFIG_OCTEONTX2_AF) += octeontx2_af.o
 
+octeontx2_cgx-y := cgx.o
 octeontx2_af-y := rvu.o mbox.o
diff --git a/drivers/soc/marvell/octeontx2/cgx.c b/drivers/soc/marvell/octeontx2/cgx.c
new file mode 100644
index 0000000..6f0b6f4
--- /dev/null
+++ b/drivers/soc/marvell/octeontx2/cgx.c
@@ -0,0 +1,117 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell OcteonTx2 CGX driver
+ *
+ * Copyright (C) 2018 Marvell International Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/phy.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+
+#include "cgx.h"
+
+#define DRV_NAME	"octeontx2-cgx"
+#define DRV_STRING      "Marvell OcteonTX2 CGX/MAC Driver"
+#define DRV_VERSION	"1.0"
+
+struct cgx {
+	void __iomem		*reg_base;
+	struct pci_dev		*pdev;
+	u8			cgx_id;
+};
+
+/* Supported devices */
+static const struct pci_device_id cgx_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_OCTEONTX2_CGX) },
+	{ 0, }  /* end of table */
+};
+
+MODULE_AUTHOR("Marvell International Ltd.");
+MODULE_DESCRIPTION(DRV_STRING);
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION(DRV_VERSION);
+MODULE_DEVICE_TABLE(pci, cgx_id_table);
+
+static int cgx_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int err;
+	struct device *dev = &pdev->dev;
+	struct cgx *cgx;
+
+	cgx = devm_kzalloc(dev, sizeof(*cgx), GFP_KERNEL);
+	if (!cgx)
+		return -ENOMEM;
+	cgx->pdev = pdev;
+
+	pci_set_drvdata(pdev, cgx);
+
+	err = pci_enable_device(pdev);
+	if (err) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		pci_set_drvdata(pdev, NULL);
+		return err;
+	}
+
+	err = pci_request_regions(pdev, DRV_NAME);
+	if (err) {
+		dev_err(dev, "PCI request regions failed 0x%x\n", err);
+		goto err_disable_device;
+	}
+
+	/* MAP configuration registers */
+	cgx->reg_base = pcim_iomap(pdev, PCI_CFG_REG_BAR_NUM, 0);
+	if (!cgx->reg_base) {
+		dev_err(dev, "CGX: Cannot map CSR memory space, aborting\n");
+		err = -ENOMEM;
+		goto err_release_regions;
+	}
+
+	return 0;
+
+err_release_regions:
+	pci_release_regions(pdev);
+err_disable_device:
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+	return err;
+}
+
+static void cgx_remove(struct pci_dev *pdev)
+{
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+}
+
+static struct pci_driver cgx_driver = {
+	.name = DRV_NAME,
+	.id_table = cgx_id_table,
+	.probe = cgx_probe,
+	.remove = cgx_remove,
+};
+
+static int __init cgx_init_module(void)
+{
+	pr_info("%s: %s\n", DRV_NAME, DRV_STRING);
+
+	return pci_register_driver(&cgx_driver);
+}
+
+static void __exit cgx_cleanup_module(void)
+{
+	pci_unregister_driver(&cgx_driver);
+}
+
+module_init(cgx_init_module);
+module_exit(cgx_cleanup_module);
diff --git a/drivers/soc/marvell/octeontx2/cgx.h b/drivers/soc/marvell/octeontx2/cgx.h
new file mode 100644
index 0000000..8056264
--- /dev/null
+++ b/drivers/soc/marvell/octeontx2/cgx.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: GPL-2.0
+ * Marvell OcteonTx2 CGX driver
+ *
+ * Copyright (C) 2018 Marvell International Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef CGX_H
+#define CGX_H
+
+ /* PCI device IDs */
+#define	PCI_DEVID_OCTEONTX2_CGX			0xA059
+
+/* PCI BAR nos */
+#define PCI_CFG_REG_BAR_NUM			0
+
+#endif /* CGX_H */