[PATCH 0/3] 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.

Jan

Jan Kiszka (3):
stmmac: pci: Overcome stmmac_pci_info structure
stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
stmmac: pci: Use dmi_system_id table for retrieving PHY addresses

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 184 ++++++++++++-----------
1 file changed, 99 insertions(+), 85 deletions(-)

--
2.12.0

[David Miller]

From: Jan Kiszka <jan.k...@siemens.com>
Date: Mon, 22 May 2017 13:12:06 +0200

> 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.

This doesn't compile:

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:285:3: error: initializer element is not computable at load time
(kernel_ulong_t)&stmmac_default_setup,
^
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:285:3: note: (near initialization for ‘stmmac_id_table[0].class’)
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:289:3: error: initializer element is not computable at load time
(kernel_ulong_t)&stmmac_default_setup,
^
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:289:3: note: (near initialization for ‘stmmac_id_table[1].class’)
scripts/Makefile.build:302: recipe for target 'drivers/net/ethernet/stmicro/stmmac/stmmac_pci.o' failed
make[5]: *** [drivers/net/ethernet/stmicro/stmmac/stmmac_pci.o] Error 1

[Jan Kiszka]

Hmm. Which arch is this?

Jan

[David Miller]

From: Jan Kiszka <jan.k...@siemens.com>
Date: Mon, 22 May 2017 19:06:07 +0200

x86_64, allmodconfig

[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 v2:
- fixed bug that broke x86-64 build (and likely more)
- refactored series to do smaller steps

All this remains cosmetic from a certain distance, but the result looks
more appealing, at least to me.

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 | 184 ++++++++++++-----------
1 file changed, 98 insertions(+), 86 deletions(-)

--
2.12.0

[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.k...@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.0

[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.k...@siemens.com>
---

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 35 +++++++++---------------
1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 061cb28f642d..485216369705 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;

@@ -197,10 +191,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
*
@@ -216,7 +206,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;
@@ -257,7 +247,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;

@@ -289,16 +279,17 @@ 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_DEVICE(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_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_pci_info),
+ STMMAC_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_default_setup),
STMMAC_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC,
- stmmac_pci_info),
- STMMAC_DEVICE(PCI_VENDOR_ID_INTEL, STMMAC_QUARK_ID, quark_pci_info),
+ stmmac_default_setup),
+ STMMAC_DEVICE(PCI_VENDOR_ID_INTEL, STMMAC_QUARK_ID,
+ quark_default_setup),
{}
};

--
2.12.0

[Andy Shevchenko]

On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.k...@siemens.com> wrote:
> By now, stmmac_pci_info only contains a single entry.

_For now_.

> Register this
> directly with the PCI device table, removing one indirection.

I am not sure this patch is needed.

Next time something comes up we would need to extend this and
effectively revert this change.
So, my vote is to leave it as is for now.

--
With Best Regards,
Andy Shevchenko

[Jan Kiszka]

Therefore moved this to the end: may the maintainer pick it or not.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

[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.k...@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

/* 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

[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.k...@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

[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

[Jan Kiszka]

Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

Signed-off-by: Jan Kiszka <jan.k...@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

[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.k...@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);

[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.k...@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

[Andy Shevchenko]

On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.k...@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.sh...@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

[David Miller]

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

> On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.k...@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.sh...@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.

[Andy Shevchenko]

On Tue, May 30, 2017 at 10:33 AM, Jan Kiszka <jan.k...@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)

?

[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.k...@siemens.com>

Reviewed-by: Andy Shevchenko <andy.sh...@gmail.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);

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 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);

[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.k...@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 2be15a8a9c40..393710815e4b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -40,9 +40,7 @@ struct stmmac_pci_dmi_data {
size_t nfuncs;

};

-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)
@@ -97,8 +95,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);

@@ -114,10 +112,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_func_data galileo_stmmac_func_data[] = {
{
.func = 6,
@@ -180,8 +174,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;

@@ -218,10 +212,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
*

@@ -237,7 +227,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;

@@ -278,7 +268,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;

@@ -312,15 +302,15 @@ static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume);

[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.k...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.sh...@gmail.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);

[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 v4:
- Refactor patch 5 according to 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 | 205 +++++++++++++----------
1 file changed, 119 insertions(+), 86 deletions(-)

--
2.12.3

[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.k...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.sh...@gmail.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.
*/

[Jan Kiszka]

Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 99 ++++++++++++++++--------
1 file changed, 66 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a6e10d3ced5c..2be15a8a9c40 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -30,36 +30,39 @@
* negative value of the address means that MAC controller is not connected
* with PHY.
*/
-struct stmmac_pci_dmi_data {

- const char *name;
- const char *asset_tag;

+struct stmmac_pci_func_data {

unsigned int func;
int phy_addr;
};

+struct stmmac_pci_dmi_data {
+ const struct stmmac_pci_func_data *func;
+ size_t nfuncs;
+};
+
struct stmmac_pci_info {
int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
};

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;
-
- if (!name)
+ const struct stmmac_pci_func_data *func_data;

+ const struct stmmac_pci_dmi_data *dmi_data;
+ const struct dmi_system_id *dmi_id;

+ int func = PCI_FUNC(pdev->devfn);
+ size_t n;
+

+ 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;
- }
- }

+ dmi_data = dmi_id->driver_data;
+ func_data = dmi_data->func;
+
+ for (n = 0; n < dmi_data->nfuncs; n++, func_data++)
+ if (func_data->func == func)
+ return func_data->phy_addr;

return -ENODEV;
}
@@ -115,34 +118,64 @@ 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_func_data galileo_stmmac_func_data[] = {

{
- .name = "Galileo",

.func = 6,
.phy_addr = 1,
},

+ { },
+};
+
+static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data = {
+ .func = galileo_stmmac_func_data,
+ .nfuncs = ARRAY_SIZE(galileo_stmmac_func_data),
+};
+
+static const struct stmmac_pci_func_data iot2040_stmmac_func_data[] = {

{
- .name = "GalileoGen2",

.func = 6,
.phy_addr = 1,
},

{
- .name = "SIMATIC IOT2000",
- .asset_tag = "6ES7647-0AA00-0YA2",
- .func = 6,

+ .func = 7,
.phy_addr = 1,
},

+ { },

+};
+
+static const struct stmmac_pci_dmi_data iot2040_stmmac_dmi_data = {

+ .func = iot2040_stmmac_func_data,
+ .nfuncs = ARRAY_SIZE(iot2040_stmmac_func_data),
+};
+
+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 +192,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

[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.k...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.sh...@gmail.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

[Andy Shevchenko]

On Fri, Jun 2, 2017 at 10:34 AM, Jan Kiszka <jan.k...@siemens.com> wrote:
> Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

> +static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = {
> {

> .func = 6,
> .phy_addr = 1,
> },

> + { },

Now this is redundant.

> +};

> +static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data = {
> + .func = galileo_stmmac_func_data,
> + .nfuncs = ARRAY_SIZE(galileo_stmmac_func_data),

It will be 2, when is supposed to be 1, see above.

> +};

> +static const struct stmmac_pci_func_data iot2040_stmmac_func_data[] = {

> + { },
> +};

> + .nfuncs = ARRAY_SIZE(iot2040_stmmac_func_data),

Ditto.

[Andy Shevchenko]

On Fri, Jun 2, 2017 at 10:34 AM, Jan Kiszka <jan.k...@siemens.com> wrote:

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

Please, drop this patch from the next version.

We can discuss it after the first 5 will be in.