[PATCH v3 0/6] stmmac: pci: Refactor DMI probing

[PATCH v3 0/6] stmmac: pci: Refactor DMI probing

[Jan Kiszka]

Some cleanups of the way we probe DMI platforms in the driver. Reduces
a bit of open-coding and makes the logic easier reusable for any
potential DMI platform != Quark.

Tested on IOT2000 and Galileo Gen2.

Changes in v3:
 - Rename STMAC vendor ID define and use PCI_VDEVICE
 - rearrange stmmac_pci_find_phy_addr according to review feedback

Jan

Jan Kiszka (6):
  stmmac: pci: Make stmmac_pci_info structure constant
  stmmac: pci: Use stmmac_pci_info for all devices
  stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
  stmmac: pci: Select quark_pci_dmi_data from quark_default_data
  stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
  stmmac: pci: Remove setup handler indirection via stmmac_pci_info

 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 187 ++++++++++++-----------
 1 file changed, 100 insertions(+), 87 deletions(-)

-- 
2.12.3

[PATCH v3 1/6] stmmac: pci: Make stmmac_pci_info structure constant

[Jan Kiszka]

By removing the PCI device reference from the structure and passing it
as parameters to the interested functions, we can make quark_pci_info
const.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 22f910795be4..0efe42659a37 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -38,17 +38,17 @@ struct stmmac_pci_dmi_data {
 };
 
 struct stmmac_pci_info {
-	struct pci_dev *pdev;
-	int (*setup)(struct plat_stmmacenet_data *plat,
-		     struct stmmac_pci_info *info);
+	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat,
+		     const struct stmmac_pci_info *info);
 	struct stmmac_pci_dmi_data *dmi;
 };
 
-static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
+static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
+				    const struct stmmac_pci_info *info)
 {
 	const char *name = dmi_get_system_info(DMI_BOARD_NAME);
 	const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
-	unsigned int func = PCI_FUNC(info->pdev->devfn);
+	unsigned int func = PCI_FUNC(pdev->devfn);
 	struct stmmac_pci_dmi_data *dmi;
 
 	/*
@@ -114,10 +114,10 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 	/* TODO: AXI */
 }
 
-static int quark_default_data(struct plat_stmmacenet_data *plat,
-			      struct stmmac_pci_info *info)
+static int quark_default_data(struct pci_dev *pdev,
+			      struct plat_stmmacenet_data *plat,
+			      const struct stmmac_pci_info *info)
 {
-	struct pci_dev *pdev = info->pdev;
 	int ret;
 
 	/* Set common default data first */
@@ -127,7 +127,7 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	 * Refuse to load the driver and register net device if MAC controller
 	 * does not connect to any PHY interface.
 	 */
-	ret = stmmac_pci_find_phy_addr(info);
+	ret = stmmac_pci_find_phy_addr(pdev, info);
 	if (ret < 0)
 		return ret;
 
@@ -175,7 +175,7 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
 	{}
 };
 
-static struct stmmac_pci_info quark_pci_info = {
+static const struct stmmac_pci_info quark_pci_info = {
 	.setup = quark_default_data,
 	.dmi = quark_pci_dmi_data,
 };
@@ -237,9 +237,8 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	if (info) {
-		info->pdev = pdev;
 		if (info->setup) {
-			ret = info->setup(plat, info);
+			ret = info->setup(pdev, plat, info);
 			if (ret)
 				return ret;
 		}
-- 
2.12.3

[PATCH v3 2/6] stmmac: pci: Use stmmac_pci_info for all devices

[Jan Kiszka]

Make stmmac_default_data compatible with stmmac_pci_info.setup and use
an info structure for all devices. This allows to make the probing more
regular.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 36 +++++++++++++++---------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 0efe42659a37..d3d74e526e17 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -100,7 +100,9 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
 }
 
-static void stmmac_default_data(struct plat_stmmacenet_data *plat)
+static int stmmac_default_data(struct pci_dev *pdev,
+			       struct plat_stmmacenet_data *plat,
+			       const struct stmmac_pci_info *info)
 {
 	/* Set common default data first */
 	common_default_data(plat);
@@ -112,8 +114,14 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
 	/* TODO: AXI */
+
+	return 0;
 }
 
+static const struct stmmac_pci_info stmmac_pci_info = {
+	.setup = stmmac_default_data,
+};
+
 static int quark_default_data(struct pci_dev *pdev,
 			      struct plat_stmmacenet_data *plat,
 			      const struct stmmac_pci_info *info)
@@ -236,14 +244,9 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	if (info) {
-		if (info->setup) {
-			ret = info->setup(pdev, plat, info);
-			if (ret)
-				return ret;
-		}
-	} else
-		stmmac_default_data(plat);
+	ret = info->setup(pdev, plat, info);
+	if (ret)
+		return ret;
 
 	pci_enable_msi(pdev);
 
@@ -269,14 +272,21 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
 
 static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume);
 
-#define STMMAC_VENDOR_ID 0x700
+/* synthetic ID, no official vendor */
+#define PCI_VENDOR_ID_STMMAC 0x700
+
 #define STMMAC_QUARK_ID  0x0937
 #define STMMAC_DEVICE_ID 0x1108
 
+#define STMMAC_DEVICE(vendor_id, dev_id, info)	{	\
+	PCI_VDEVICE(vendor_id, dev_id),			\
+	.driver_data = (kernel_ulong_t)&info		\
+	}
+
 static const struct pci_device_id stmmac_id_table[] = {
-	{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)},
-	{PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)},
-	{PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info},
+	STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info),
+	STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info),
+	STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info),
 	{}
 };
 
-- 
2.12.3

[PATCH v3 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic

[Jan Kiszka]

Move the special case for the early Galileo firmware into
quark_default_setup. This allows to use stmmac_pci_find_phy_addr for
non-quark cases.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index d3d74e526e17..f44ae49eb11c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -51,12 +51,8 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
 	unsigned int func = PCI_FUNC(pdev->devfn);
 	struct stmmac_pci_dmi_data *dmi;
 
-	/*
-	 * Galileo boards with old firmware don't support DMI. We always return
-	 * 1 here, so at least first found MAC controller would be probed.
-	 */
 	if (!name)
-		return 1;
+		return -ENODEV;
 
 	for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
 		if (!strcmp(dmi->name, name) && dmi->func == func) {
@@ -136,8 +132,18 @@ static int quark_default_data(struct pci_dev *pdev,
 	 * does not connect to any PHY interface.
 	 */
 	ret = stmmac_pci_find_phy_addr(pdev, info);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		/* Return error to the caller on DMI enabled boards. */
+		if (dmi_get_system_info(DMI_BOARD_NAME))
+			return ret;
+
+		/*
+		 * Galileo boards with old firmware don't support DMI. We always
+		 * use 1 here as PHY address, so at least the first found MAC
+		 * controller would be probed.
+		 */
+		ret = 1;
+	}
 
 	plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
 	plat->phy_addr = ret;
-- 
2.12.3

[PATCH v3 4/6] stmmac: pci: Select quark_pci_dmi_data from quark_default_data

[Jan Kiszka]

No need to carry this reference in stmmac_pci_info - the Quark-specific
setup handler knows that it needs to use the Quark-specific DMI table.
This also allows to drop the stmmac_pci_info reference from the setup
handler parameter list.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 83 +++++++++++-------------
 1 file changed, 39 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index f44ae49eb11c..a6e10d3ced5c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -38,13 +38,11 @@ struct stmmac_pci_dmi_data {
 };
 
 struct stmmac_pci_info {
-	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat,
-		     const struct stmmac_pci_info *info);
-	struct stmmac_pci_dmi_data *dmi;
+	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
 };
 
 static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
-				    const struct stmmac_pci_info *info)
+				    struct stmmac_pci_dmi_data *dmi_data)
 {
 	const char *name = dmi_get_system_info(DMI_BOARD_NAME);
 	const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
@@ -54,7 +52,7 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
 	if (!name)
 		return -ENODEV;
 
-	for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
+	for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) {
 		if (!strcmp(dmi->name, name) && dmi->func == func) {
 			/* If asset tag is provided, match on it as well. */
 			if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
@@ -97,8 +95,7 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
 }
 
 static int stmmac_default_data(struct pci_dev *pdev,
-			       struct plat_stmmacenet_data *plat,
-			       const struct stmmac_pci_info *info)
+			       struct plat_stmmacenet_data *plat)
 {
 	/* Set common default data first */
 	common_default_data(plat);
@@ -118,9 +115,40 @@ static const struct stmmac_pci_info stmmac_pci_info = {
 	.setup = stmmac_default_data,
 };
 
+static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
+	{
+		.name = "Galileo",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "GalileoGen2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-0YA2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-1YA2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "SIMATIC IOT2000",
+		.asset_tag = "6ES7647-0AA00-1YA2",
+		.func = 7,
+		.phy_addr = 1,
+	},
+	{}
+};
+
 static int quark_default_data(struct pci_dev *pdev,
-			      struct plat_stmmacenet_data *plat,
-			      const struct stmmac_pci_info *info)
+			      struct plat_stmmacenet_data *plat)
 {
 	int ret;
 
@@ -131,7 +159,7 @@ static int quark_default_data(struct pci_dev *pdev,
 	 * Refuse to load the driver and register net device if MAC controller
 	 * does not connect to any PHY interface.
 	 */
-	ret = stmmac_pci_find_phy_addr(pdev, info);
+	ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data);
 	if (ret < 0) {
 		/* Return error to the caller on DMI enabled boards. */
 		if (dmi_get_system_info(DMI_BOARD_NAME))
@@ -157,41 +185,8 @@ static int quark_default_data(struct pci_dev *pdev,
 	return 0;
 }
 
-static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
-	{
-		.name = "Galileo",
-		.func = 6,
-		.phy_addr = 1,
-	},
-	{
-		.name = "GalileoGen2",
-		.func = 6,
-		.phy_addr = 1,
-	},
-	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-0YA2",
-		.func = 6,
-		.phy_addr = 1,
-	},
-	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-1YA2",
-		.func = 6,
-		.phy_addr = 1,
-	},
-	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-1YA2",
-		.func = 7,
-		.phy_addr = 1,
-	},
-	{}
-};
-
 static const struct stmmac_pci_info quark_pci_info = {
 	.setup = quark_default_data,
-	.dmi = quark_pci_dmi_data,
 };
 
 /**
@@ -250,7 +245,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	ret = info->setup(pdev, plat, info);
+	ret = info->setup(pdev, plat);
 	if (ret)
 		return ret;
 
-- 
2.12.3

[PATCH v3 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses

[Jan Kiszka]

Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 77 ++++++++++++++----------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a6e10d3ced5c..a3909ab0da05 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -31,9 +31,7 @@
  * with PHY.
  */
 struct stmmac_pci_dmi_data {
-	const char *name;
-	const char *asset_tag;
-	unsigned int func;
+	int func;
 	int phy_addr;
 };
 
@@ -42,24 +40,19 @@ struct stmmac_pci_info {
 };
 
 static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
-				    struct stmmac_pci_dmi_data *dmi_data)
+				    const struct dmi_system_id *dmi_list)
 {
-	const char *name = dmi_get_system_info(DMI_BOARD_NAME);
-	const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
-	unsigned int func = PCI_FUNC(pdev->devfn);
-	struct stmmac_pci_dmi_data *dmi;
+	const struct stmmac_pci_dmi_data *dmi_data;
+	const struct dmi_system_id *dmi_id;
+	int func = PCI_FUNC(pdev->devfn);
 
-	if (!name)
+	dmi_id = dmi_first_match(dmi_list);
+	if (!dmi_id)
 		return -ENODEV;
 
-	for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) {
-		if (!strcmp(dmi->name, name) && dmi->func == func) {
-			/* If asset tag is provided, match on it as well. */
-			if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
-				continue;
-			return dmi->phy_addr;
-		}
-	}
+	for (dmi_data = dmi_id->driver_data; dmi_data->func >= 0; dmi_data++)
+		if (dmi_data->func == func)
+			return dmi_data->phy_addr;
 
 	return -ENODEV;
 }
@@ -115,34 +108,54 @@ static const struct stmmac_pci_info stmmac_pci_info = {
 	.setup = stmmac_default_data,
 };
 
-static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
+static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = {
 	{
-		.name = "Galileo",
 		.func = 6,
 		.phy_addr = 1,
 	},
+	{-1, -1},
+};
+
+static const struct stmmac_pci_dmi_data iot2040_stmmac_dmi_data[] = {
 	{
-		.name = "GalileoGen2",
 		.func = 6,
 		.phy_addr = 1,
 	},
 	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-0YA2",
-		.func = 6,
+		.func = 7,
 		.phy_addr = 1,
 	},
+	{-1, -1},
+};
+
+static const struct dmi_system_id quark_pci_dmi[] = {
 	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-1YA2",
-		.func = 6,
-		.phy_addr = 1,
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "Galileo"),
+		},
+		.driver_data = (void *)galileo_stmmac_dmi_data,
 	},
 	{
-		.name = "SIMATIC IOT2000",
-		.asset_tag = "6ES7647-0AA00-1YA2",
-		.func = 7,
-		.phy_addr = 1,
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
+		},
+		.driver_data = (void *)galileo_stmmac_dmi_data,
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
+			DMI_EXACT_MATCH(DMI_BOARD_ASSET_TAG,
+					"6ES7647-0AA00-0YA2"),
+		},
+		.driver_data = (void *)galileo_stmmac_dmi_data,
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"),
+			DMI_EXACT_MATCH(DMI_BOARD_ASSET_TAG,
+					"6ES7647-0AA00-1YA2"),
+		},
+		.driver_data = (void *)iot2040_stmmac_dmi_data,
 	},
 	{}
 };
@@ -159,7 +172,7 @@ static int quark_default_data(struct pci_dev *pdev,
 	 * Refuse to load the driver and register net device if MAC controller
 	 * does not connect to any PHY interface.
 	 */
-	ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data);
+	ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi);
 	if (ret < 0) {
 		/* Return error to the caller on DMI enabled boards. */
 		if (dmi_get_system_info(DMI_BOARD_NAME))
-- 
2.12.3

Re: [PATCH v3 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses

[Andy Shevchenko]

On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

>  struct stmmac_pci_dmi_data {
> -       const char *name;
> -       const char *asset_tag;
> -       unsigned int func;
> +       int func;
>         int phy_addr;
>  };

Can we use the following instead:

struct stmmac_pci_dmi_data {
       unsigned int *func;
       int *phy_addr;
       size_t nfuncs;
};

or something like

struct stmmac_pci_func_data {
       unsigned int func;
       int phy_addr;
};

struct stmmac_pci_dmi_data {
    struct stmmac_pci_func_data *func;
    size_t nfuncs;
};

(Latter would be better since it allows to use ARRAY_SIZE() and less
error prone for possible asymmetrical amount of values in the former)

?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 6/6] stmmac: pci: Remove setup handler indirection via stmmac_pci_info

[Jan Kiszka]

By now, stmmac_pci_info only contains a single entry. Register this
directly with the PCI device table, removing one indirection.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 34 +++++++++---------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a3909ab0da05..73b7b5d3a11c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -35,9 +35,7 @@ struct stmmac_pci_dmi_data {
 	int phy_addr;
 };
 
-struct stmmac_pci_info {
-	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
-};
+typedef int (*stmmac_setup)(struct pci_dev *, struct plat_stmmacenet_data *);
 
 static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
 				    const struct dmi_system_id *dmi_list)
@@ -87,8 +85,8 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
 }
 
-static int stmmac_default_data(struct pci_dev *pdev,
-			       struct plat_stmmacenet_data *plat)
+static int stmmac_default_setup(struct pci_dev *pdev,
+				struct plat_stmmacenet_data *plat)
 {
 	/* Set common default data first */
 	common_default_data(plat);
@@ -104,10 +102,6 @@ static int stmmac_default_data(struct pci_dev *pdev,
 	return 0;
 }
 
-static const struct stmmac_pci_info stmmac_pci_info = {
-	.setup = stmmac_default_data,
-};
-
 static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = {
 	{
 		.func = 6,
@@ -160,8 +154,8 @@ static const struct dmi_system_id quark_pci_dmi[] = {
 	{}
 };
 
-static int quark_default_data(struct pci_dev *pdev,
-			      struct plat_stmmacenet_data *plat)
+static int quark_default_setup(struct pci_dev *pdev,
+			       struct plat_stmmacenet_data *plat)
 {
 	int ret;
 
@@ -198,10 +192,6 @@ static int quark_default_data(struct pci_dev *pdev,
 	return 0;
 }
 
-static const struct stmmac_pci_info quark_pci_info = {
-	.setup = quark_default_data,
-};
-
 /**
  * stmmac_pci_probe
  *
@@ -217,7 +207,7 @@ static const struct stmmac_pci_info quark_pci_info = {
 static int stmmac_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
-	struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data;
+	stmmac_setup setup = (stmmac_setup)id->driver_data;
 	struct plat_stmmacenet_data *plat;
 	struct stmmac_resources res;
 	int i;
@@ -258,7 +248,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	ret = info->setup(pdev, plat);
+	ret = setup(pdev, plat);
 	if (ret)
 		return ret;
 
@@ -292,15 +282,15 @@ static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume);
 #define STMMAC_QUARK_ID  0x0937
 #define STMMAC_DEVICE_ID 0x1108
 
-#define STMMAC_DEVICE(vendor_id, dev_id, info)	{	\
+#define STMMAC_DEVICE(vendor_id, dev_id, setup)	{	\
 	PCI_VDEVICE(vendor_id, dev_id),			\
-	.driver_data = (kernel_ulong_t)&info		\
+	.driver_data = (kernel_ulong_t)&setup		\
 	}
 
 static const struct pci_device_id stmmac_id_table[] = {
-	STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info),
-	STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info),
-	STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info),
+	STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_default_setup),
+	STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_default_setup),
+	STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_default_setup),
 	{}
 };
 
-- 
2.12.3

Re: [PATCH v3 0/6] stmmac: pci: Refactor DMI probing

[Andy Shevchenko]

On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Some cleanups of the way we probe DMI platforms in the driver. Reduces
> a bit of open-coding and makes the logic easier reusable for any
> potential DMI platform != Quark.
>
> Tested on IOT2000 and Galileo Gen2.

So, for patches 1-4,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

I'm not convinced that patch 6 is needed, so, my vote is not to include it.

Patch 5 I would like to review later. Main problem to me is {-1, -1}.
It looks non-standard and feels not good. I'll try to think on the
solution.

> Jan Kiszka (6):
>   stmmac: pci: Make stmmac_pci_info structure constant
>   stmmac: pci: Use stmmac_pci_info for all devices
>   stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
>   stmmac: pci: Select quark_pci_dmi_data from quark_default_data
>   stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
>   stmmac: pci: Remove setup handler indirection via stmmac_pci_info

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/6] stmmac: pci: Refactor DMI probing

[David Miller]

From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Tue, 30 May 2017 21:48:05 +0300

> On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Some cleanups of the way we probe DMI platforms in the driver. Reduces
>> a bit of open-coding and makes the logic easier reusable for any
>> potential DMI platform != Quark.
>>
>> Tested on IOT2000 and Galileo Gen2.
> 
> So, for patches 1-4,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> I'm not convinced that patch 6 is needed, so, my vote is not to include it.
> 
> Patch 5 I would like to review later. Main problem to me is {-1, -1}.
> It looks non-standard and feels not good. I'll try to think on the
> solution.

Ok, so at a minimum I'm expecting another respin of this.