Re: [PATCH net] e1000e: Remove Other from EIAC.

[Jan Kiszka]

On 2018-01-31 08:26, Benjamin Poirier wrote:
> It was reported that emulated e1000e devices in vmware esxi 6.5 Build
> 7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
> overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
> e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
> on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
> icr=0x8004 (_INT_ASSERTED | _LSC) in the same situation.
> 
> Some experimentation showed that this flaw in vmware e1000e emulation can
> be worked around by not setting Other in EIAC. This is how it was before
> 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).
> 
> Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 9f18d39bdc8f..625a4c9a86a4 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -1918,6 +1918,8 @@ static irqreturn_t e1000_msix_other(int __always_unused 
> irq, void *data)
>   bool enable = true;
>  
>   icr = er32(ICR);
> + ew32(ICR, E1000_ICR_OTHER);
> +
>   if (icr & E1000_ICR_RXO) {
>   ew32(ICR, E1000_ICR_RXO);
>   enable = false;
> @@ -2040,7 +2042,6 @@ static void e1000_configure_msix(struct e1000_adapter 
> *adapter)
>  hw->hw_addr + E1000_EITR_82574(vector));
>   else
>   writel(1, hw->hw_addr + E1000_EITR_82574(vector));
> - adapter->eiac_mask |= E1000_IMS_OTHER;
>  
>   /* Cause Tx interrupts on every write back */
>   ivar |= BIT(31);
> @@ -2265,7 +2266,7 @@ static void e1000_irq_enable(struct e1000_adapter 
> *adapter)
>  
>   if (adapter->msix_entries) {
>   ew32(EIAC_82574, adapter->eiac_mask & E1000_EIAC_MASK_82574);
> - ew32(IMS, adapter->eiac_mask | E1000_IMS_LSC);
> + ew32(IMS, adapter->eiac_mask | E1000_IMS_OTHER | E1000_IMS_LSC);
>   } else if (hw->mac.type >= e1000_pch_lpt) {
>   ew32(IMS, IMS_ENABLE_MASK | E1000_IMS_ECCER);
>   } else {
> 

Shouldn't this be queued for stable as well? I'm missing it in 4.14 LTS
at least.

BTW, it seems QEMU's e1000e model is affected by the same issue. I've
proposed a fix for it [1].

Jan

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg525182.html



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 0/5] stmmac: pci: Refactor DMI probing

[Jan Kiszka]

On 2017-06-22 19:40, David Miller wrote:
> From: Jan Kiszka <jan.kis...@siemens.com>
> Date: Thu, 22 Jun 2017 08:17:56 +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.
>>
>> Changes in v5:
>>  - fixed a remaining issue in patch 5
>>  - dropped patch 6 for now
> 
> Series applied to net-next.
> 
> Any chance the DMI table can be marked const as well?
> 

Hmm, they are all const - or which one do you mean?

Jan

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

[PATCH v5 2/5] stmmac: pci: Use stmmac_pci_info for all devices

[Jan Kiszka]

From: Jan Kiszka <jan.kis...@siemens.com>

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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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)\
+   }
+
 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)_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 v5 0/5] 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 v5:
 - fixed a remaining issue in patch 5
 - dropped patch 6 for now

Jan

Jan Kiszka (5):
  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

 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 203 ++-
 1 file changed, 122 insertions(+), 81 deletions(-)

-- 
2.12.3

[PATCH v5 4/5] stmmac: pci: Select quark_pci_dmi_data from quark_default_data

[Jan Kiszka]

From: Jan Kiszka <jan.kis...@siemens.com>

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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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);
@@ -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",
-

[PATCH v5 3/5] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic

[Jan Kiszka]

From: Jan Kiszka <jan.kis...@siemens.com>

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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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.
 */
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 v5 1/5] stmmac: pci: Make stmmac_pci_info structure constant

[Jan Kiszka]

From: Jan Kiszka <jan.kis...@siemens.com>

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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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);
if (ret)
return ret;
}
-- 
2.12.3

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

[Jan Kiszka]

From: Jan Kiszka <jan.kis...@siemens.com>

Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.

Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 97 
 1 file changed, 64 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..8d375e51a526 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,62 @@ 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 *)_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 *)_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 *)_stmmac_dmi_data,
+   },
+   {
+   .matches = {
+   DMI_EXACT_MATCH(DMI_BO

[PATCH v4 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.kis...@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 *)_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 *)_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 *)_stmmac_dmi_data,
+   },
+   {
+   .matches = {
+   DMI_EXACT_MATCH(DMI_BOARD_NAME, &quo

[PATCH v4 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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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);
@@ -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,
-  

[PATCH v4 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 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

[PATCH v4 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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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);
if (ret)
return ret;
}
-- 
2.12.3

[PATCH v4 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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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)\
+   }
+
 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)_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 v4 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.kis...@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);
 #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)\
+   .driver_data = (kernel_ulong_t)   \
}
 
 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

[PATCH v4 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.kis...@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevche...@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.
 */
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 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.kis...@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)\
+   .driver_data = (kernel_ulong_t)   \
}
 
 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

[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.kis...@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)\
+   }
+
 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)_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 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.kis...@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 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.kis...@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 IOT2

[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.kis...@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 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 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.kis...@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 v2 6/6] stmmac: pci: Remove setup handler indirection via stmmac_pci_info

[Jan Kiszka]

On 2017-05-27 15:38, Andy Shevchenko wrote:
> On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kis...@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.

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

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

[Jan Kiszka]

On 2017-05-27 15:28, Andy Shevchenko wrote:
> On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka <jan.kis...@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 leave unsigned type here...
> 
>> -static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
>> +static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = {
> 
>> +   {-1, -1},
>> +};
> 
>> +static const struct stmmac_pci_dmi_data iot2040_stmmac_dmi_data[] = {
> 
>> +   {-1, -1},
>> +};
> 
> ...and avoid this not so standard terminators?

0 is a valid PCI function, thus can't be use as terminator. Therefore I
chose -1 as an obviously invalid value.

> 
>> +   .matches = {
>> +   DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
>> +   },
> 
>> +   .driver_data = (void *)galileo_stmmac_dmi_data,
> 
> Can't be slightly better
> 
>  .driver_data = _stmmac_dmi_data,
> 
> ?
> 

Interesting, that removes the "const" as well. OK.

Jan

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

[PATCH v2 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.kis...@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)\
+   .driver_data = (kernel_ulong_t)   \
}
 
 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

[PATCH v2 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.kis...@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 9aca14f8b55e..1a89fa9ee39d 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,17 @@ 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) {
+   /*
+* 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.
+*/
+   if (!dmi_get_system_info(DMI_BOARD_NAME))
+   ret = 1;
+   else
+   return ret;
+   }
 
plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
plat->phy_addr = ret;
-- 
2.12.0

[PATCH v2 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.kis...@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 33 +++-
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 0efe42659a37..9aca14f8b55e 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);
 
@@ -273,10 +276,16 @@ 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) {   \
+   PCI_DEVICE(vendor_id, dev_id),  \
+   .driver_data = (kernel_ulong_t)\
+   }
+
 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)_pci_info},
+   STMMAC_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_pci_info),
+   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),
{}
 };
 
-- 
2.12.0

[PATCH v2 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.kis...@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

[PATCH v2 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.kis...@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 1a89fa9ee39d..07af42531fd4 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) {
/*
 * Galileo boards with old firmware don't support DMI. We always
@@ -156,41 +184,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_t

[PATCH v2 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.kis...@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 07af42531fd4..061cb28f642d 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) {
/*
 * Galileo boards with old firmware don't support DMI. We always
-- 
2.12.0

[PATCH v2 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 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

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

[Jan Kiszka]

On 2017-05-22 18:35, David Miller wrote:
> From: Jan Kiszka <jan.kis...@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)_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)_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
> 

Hmm. Which arch is this?

Jan

Re: [PATCH 3/3] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses

[Jan Kiszka]

On 2017-05-22 13:33, Joe Perches wrote:
> On Mon, 2017-05-22 at 13:12 +0200, Jan Kiszka wrote:
>> Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr.
> []
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> []
>> @@ -31,65 +31,78 @@
> []
>> +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,
> 
> Why change this from 6 to 7?
> 

The diff is confusing here: If you look at the outcome, we now have
galileo_stmmac_dmi_data with function 6 only (also used for the
IOT2020), and iot2040_stmmac_dmi_data with both function 6 and 7 (both
MACs are wired up).

Jan

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

[PATCH 1/3] stmmac: pci: Overcome stmmac_pci_info structure

[Jan Kiszka]

First, pass the PCI device reference as function parameter. Then the
setup function knows which stmmac_pci_dmi_data structure to use.
Finally, we are left with a setup function in stmmac_pci_info and can
convert the structure into a function pointer. By converting
stmmac_default_data to that type, we can make a setup function
mandatory, and probing becomes more regular.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 22f910795be4..990a61acd70e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -37,18 +37,46 @@ struct stmmac_pci_dmi_data {
int phy_addr;
 };
 
-struct stmmac_pci_info {
-   struct pci_dev *pdev;
-   int (*setup)(struct plat_stmmacenet_data *plat,
-struct stmmac_pci_info *info);
-   struct stmmac_pci_dmi_data *dmi;
+typedef int (*stmmac_setup)(struct pci_dev *, struct plat_stmmacenet_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 stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
+static int stmmac_pci_find_phy_addr(struct pci_dev *pdev,
+   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);
-   unsigned int func = PCI_FUNC(info->pdev->devfn);
+   unsigned int func = PCI_FUNC(pdev->devfn);
struct stmmac_pci_dmi_data *dmi;
 
/*
@@ -58,7 +86,7 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info 
*info)
if (!name)
return 1;
 
-   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))
@@ -100,7 +128,8 @@ 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_setup(struct pci_dev *pdev,
+   struct plat_stmmacenet_data *plat)
 {
/* Set common default data first */
common_default_data(plat);
@@ -112,12 +141,13 @@ 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 int quark_default_data(struct plat_stmmacenet_data *plat,
- struct stmmac_pci_info *info)
+static int quark_default_setup(struct pci_dev *pdev,
+  struct plat_stmmacenet_data *plat)
 {
-   struct pci_dev *pdev = info->pdev;
int ret;
 
/* Set common default data first */
@@ -127,7 +157,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, quark_pci_dmi_data);
if (ret < 0)
return ret;
 
@@ -143,43 +173,6 @@ static int quark_default_data(struct plat_stmmacenet_data 
*plat,
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,
-   },
-   {
-   .n

[PATCH 3/3] 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.kis...@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 ffa59b76e884..23ef235c6c0d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -31,65 +31,78 @@
  * with PHY.
  */
 struct stmmac_pci_dmi_data {
-   const char *name;
-   const char *asset_tag;
-   unsigned int func;
+   int func;
int phy_addr;
 };
 
 typedef int (*stmmac_setup)(struct pci_dev *, struct plat_stmmacenet_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,
},
{}
 };
 
 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;
 }
@@ -153,7 +166,7 @@ static int quark_default_setup(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) {
/*
 * Galileo boards with old firmware don't support DMI. We always
-- 
2.12.0

[PATCH 2/3] 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.kis...@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 990a61acd70e..ffa59b76e884 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -79,12 +79,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 = dmi_data; dmi->name && *dmi->name; dmi++) {
if (!strcmp(dmi->name, name) && dmi->func == func) {
@@ -158,8 +154,17 @@ static int quark_default_setup(struct pci_dev *pdev,
 * does not connect to any PHY interface.
 */
ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data);
-   if (ret < 0)
-   return ret;
+   if (ret < 0) {
+   /*
+* 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.
+*/
+   if (!dmi_get_system_info(DMI_BOARD_NAME))
+   ret = 1;
+   else
+   return ret;
+   }
 
plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
plat->phy_addr = ret;
-- 
2.12.0

[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

Re: [PATCH v1 0/4] stmmac: pci: Fix crash on Intel Galileo Gen2

[Jan Kiszka]

On 2017-05-08 16:14, Andy Shevchenko wrote:
> Due to misconfiguration of PCI driver for Intel Quark the user will get
> a kernel crash:
> 
> # udhcpc -i eth0
> udhcpc: started, v1.26.2
> stmmaceth :00:14.6 eth0: device MAC address 98:4f:ee:05:ac:47
> Generic PHY stmmac-a6:01: attached PHY driver [Generic PHY] 
> (mii_bus:phy_addr=stmmac-a6:01, irq=-1)
> stmmaceth :00:14.6 eth0: IEEE 1588-2008 Advanced Timestamp supported
> stmmaceth :00:14.6 eth0: registered PTP clock
> IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> 
> udhcpc: sending discover
> 
> stmmaceth :00:14.6 eth0: Link is Up - 100Mbps/Full - flow control off
> IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: stmmac_xmit+0xf1/0x1080
> 
> Fix this by adding necessary settings.
> 
> P.S. I split fix to three patches according to what each of them adds.
> 
> Andy Shevchenko (4):
>   stmmac: pci: set default number of rx and tx queues
>   stmmac: pci: TX and RX queue priority configuration
>   stmmac: pci: RX queue routing configuration
>   stmmac: pci: split out common_default_data() helper
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 41 
> +++++++-
>  1 file changed, 18 insertions(+), 23 deletions(-)
> 

Tested-by: Jan Kiszka <jan.kis...@siemens.com>

All fine again, thanks!

Jan

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

Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues

[Jan Kiszka]

On 2017-05-08 14:02, Joao Pinto wrote:
> Às 12:56 PM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 2:40 PM, Joao Pinto  wrote:
>>> Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
 On Mon, May 8, 2017 at 1:42 PM, Joao Pinto  wrote:
> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto  
>> wrote:
>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
>>

 [   44.374161] stmmac_dvr_probe <<< 0 0

>>>
>>> Ok, so this is the cause of the problem. The driver is geting 0 for real RX 
>>> and
>>> TX queues.
>>>
>>> Your setup uses standard DT parsing from stmmac_platform or a custom one?
>>>
>>> If you are using stmmac_probe_config_dt():
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n363=DwIFaQ=DPL6_X_6JkXFx7AXWqB0tg=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc=KhdAPUtP0twDkibE89cLYs8JjnxEvBgav5uf08WL_e8=
>>>  
>>>
>>> You will find a function named stmmac_mtl_setup() being called:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n492=DwIFaQ=DPL6_X_6JkXFx7AXWqB0tg=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc=rTxn0fwdudwq9XAquH60xNHN538KBQ6_n4wODdLoyA0=
>>>  
>>>
>>> In this function, the number of RX and TX queues is being set to 1 by 
>>> default.
>>
>> Ah-ha, now I know how it's happened.
>> You forget to update all setup() hooks in PCI bus driver :-)
>>
>> I will prepare a fix.
>> Just tell me should I put Fixes tag or not? And if yes, what commit
>> should I refer to?
>>
> 
> Great, you can use this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c?id=26d6851fd24ed5d88580d66b4c8384947d5ca29b
> 
> Thanks!
> 
> Joao
> 

Perfect, looking forward to try out a fix. Thanks, folks!

Jan

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

Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues

[Jan Kiszka]

On 2017-03-15 12:04, Joao Pinto wrote:
> This patch prepares DMA Operation Mode configuration for multiple queues.
> The work consisted on breaking the DMA operation Mode configuration function
> into RX and TX scope and adapting its mechanism in stmmac_main.
> 
> Signed-off-by: Joao Pinto 
> ---
> changes v1->v3:
> - Just to keep up the patch-set version
> 
>  drivers/net/ethernet/stmicro/stmmac/common.h  |   3 +
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 118 
> +++---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  82 +++
>  3 files changed, 124 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
> b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 9f0d26d..13bd3d4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -424,6 +424,9 @@ struct stmmac_dma_ops {
>* An invalid value enables the store-and-forward mode */
>   void (*dma_mode)(void __iomem *ioaddr, int txmode, int rxmode,
>int rxfifosz);
> + void (*dma_rx_mode)(void __iomem *ioaddr, int mode, u32 channel,
> + int fifosz);
> + void (*dma_tx_mode)(void __iomem *ioaddr, int mode, u32 channel);
>   /* To track extra statistic (if supported) */
>   void (*dma_diagnostic_fr) (void *data, struct stmmac_extra_stats *x,
>  void __iomem *ioaddr);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 6ac6b26..6285e8a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -182,70 +182,26 @@ static void dwmac4_rx_watchdog(void __iomem *ioaddr, 
> u32 riwt)
>   writel(riwt, ioaddr + DMA_CHAN_RX_WATCHDOG(i));
>  }
>  
> -static void dwmac4_dma_chan_op_mode(void __iomem *ioaddr, int txmode,
> - int rxmode, u32 channel, int rxfifosz)
> +static void dwmac4_dma_rx_chan_op_mode(void __iomem *ioaddr, int mode,
> +u32 channel, int fifosz)
>  {
> - unsigned int rqs = rxfifosz / 256 - 1;
> - u32 mtl_tx_op, mtl_rx_op, mtl_rx_int;
> -
> - /* Following code only done for channel 0, other channels not yet
> -  * supported.
> -  */
> - mtl_tx_op = readl(ioaddr + MTL_CHAN_TX_OP_MODE(channel));
> -
> - if (txmode == SF_DMA_MODE) {
> - pr_debug("GMAC: enable TX store and forward mode\n");
> - /* Transmit COE type 2 cannot be done in cut-through mode. */
> - mtl_tx_op |= MTL_OP_MODE_TSF;
> - } else {
> - pr_debug("GMAC: disabling TX SF (threshold %d)\n", txmode);
> - mtl_tx_op &= ~MTL_OP_MODE_TSF;
> - mtl_tx_op &= MTL_OP_MODE_TTC_MASK;
> - /* Set the transmit threshold */
> - if (txmode <= 32)
> - mtl_tx_op |= MTL_OP_MODE_TTC_32;
> - else if (txmode <= 64)
> - mtl_tx_op |= MTL_OP_MODE_TTC_64;
> - else if (txmode <= 96)
> - mtl_tx_op |= MTL_OP_MODE_TTC_96;
> - else if (txmode <= 128)
> - mtl_tx_op |= MTL_OP_MODE_TTC_128;
> - else if (txmode <= 192)
> - mtl_tx_op |= MTL_OP_MODE_TTC_192;
> - else if (txmode <= 256)
> - mtl_tx_op |= MTL_OP_MODE_TTC_256;
> - else if (txmode <= 384)
> - mtl_tx_op |= MTL_OP_MODE_TTC_384;
> - else
> - mtl_tx_op |= MTL_OP_MODE_TTC_512;
> - }
> - /* For an IP with DWC_EQOS_NUM_TXQ == 1, the fields TXQEN and TQS are RO
> -  * with reset values: TXQEN on, TQS == DWC_EQOS_TXFIFO_SIZE.
> -  * For an IP with DWC_EQOS_NUM_TXQ > 1, the fields TXQEN and TQS are R/W
> -  * with reset values: TXQEN off, TQS 256 bytes.
> -  *
> -  * Write the bits in both cases, since it will have no effect when RO.
> -  * For DWC_EQOS_NUM_TXQ > 1, the top bits in MTL_OP_MODE_TQS_MASK might
> -  * be RO, however, writing the whole TQS field will result in a value
> -  * equal to DWC_EQOS_TXFIFO_SIZE, just like for DWC_EQOS_NUM_TXQ == 1.
> -  */
> - mtl_tx_op |= MTL_OP_MODE_TXQEN | MTL_OP_MODE_TQS_MASK;
> - writel(mtl_tx_op, ioaddr +  MTL_CHAN_TX_OP_MODE(channel));
> + unsigned int rqs = fifosz / 256 - 1;
> + u32 mtl_rx_op, mtl_rx_int;
>  
>   mtl_rx_op = readl(ioaddr + MTL_CHAN_RX_OP_MODE(channel));
>  
> - if (rxmode == SF_DMA_MODE) {
> + if (mode == SF_DMA_MODE) {
>   pr_debug("GMAC: enable RX store and forward mode\n");
>   mtl_rx_op |= MTL_OP_MODE_RSF;
>   } else {
> - pr_debug("GMAC: disable RX SF mode (threshold %d)\n", rxmode);
> + pr_debug("GMAC: disable RX SF mode (threshold 

[PATCH v2] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

The IOT2000 is industrial controller platform, derived from the Intel
Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
IOT2040 has two of them. They can be told apart based on the board asset
tag in the DMI table.

Based on patch by Sascha Weisenberger.

Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
Signed-off-by: Sascha Weisenberger <sascha.weisenber...@siemens.com>
---

Changes in v2:
- reformatted match conditions [Andy]

 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 26 +++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5c9e462276b9..11d2229e536b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -32,6 +32,7 @@
  */
 struct stmmac_pci_dmi_data {
const char *name;
+   const char *asset_tag;
unsigned int func;
int phy_addr;
 };
@@ -46,6 +47,7 @@ struct stmmac_pci_info {
 static int stmmac_pci_find_phy_addr(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);
struct stmmac_pci_dmi_data *dmi;
 
@@ -57,8 +59,12 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info 
*info)
return 1;
 
for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
-   if (!strcmp(dmi->name, name) && dmi->func == func)
+   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;
+   }
}
 
return -ENODEV;
@@ -142,6 +148,24 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
.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,
+   },
{}
 };
 

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 13:42, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 1:09 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>> On 2017-04-25 12:07, Jan Kiszka wrote:
>>> On 2017-04-25 11:46, Andy Shevchenko wrote:
>>>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kis...@siemens.com> 
>>>> wrote:
>>>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kis...@siemens.com> 
>>>>>> wrote:
>>>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kis...@siemens.com> 
>>>>>>>> wrote:
> 
>>>>{
>>>>.name = "SIMATIC IOT2000",
>>>>.func = 6,
>>>>.phy_addr = 1,
>>>>},
>>>>{
>>>>.name = "SIMATIC IOT2000",
>>>>.func = 7,
>>>>.phy_addr = 1,
>>>>},
>>>>
>>>> That's all what you need.
>>>
>>> Nope. Again: the asset tag is the way to tell both apart AND to ensure
>>> that we do not match on future devices.
> 
>> To be more verbose: your version (which is our old one) would even
>> enable the second, not connected port on the IOT2020. Incorrectly.
> 
> So, name has 2000 for 2020 device? It's clear bug in DMI table you have. :-(
> 
> What else do you have in DMI which can be used to distinguish those devices?

Andy, there are devices out there in the field, if we as engineers like
it or not, that are all called "IOT2000" although they are sightly
different inside. This patch accounts for that. And it does that even
without adding "platform_data" hacks like in other patches of mine. ;)

Jan

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

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 12:07, Jan Kiszka wrote:
> On 2017-04-25 11:46, Andy Shevchenko wrote:
>> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kis...@siemens.com> 
>>>>>> wrote:
>>>>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>>>>> tag in the DMI table.
>>>>
>>>>>>> +   const char *asset_tag;
>>>>>>
>>>>>> I guess this is redundant. See below.
>>>>>>
>>>>>>> +   {
>>>>>>> +   .name = "SIMATIC IOT2000",
>>>>>>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>>>>>>> +   .func = 6,
>>>>>>> +   .phy_addr = 1,
>>>>>>> +   },
>>>>>>
>>>>>> The below has same definition disregard on asset_tag.
>>>>>>
>>>>>
>>>>> There is a small difference in the asset tag, just not at the last digit
>>>>> where one may expect it, look:
>>>>>
>>>>> ...-0YA2 -> IOT2020
>>>>> ...-1YA2 -> IOT2040
>>>>
>>>> Yes. And how does it change my statement? You may use one record here
>>>> instead of two.
>>>
>>> How? Please be more verbose in your comments.
>>
>>{
>>.name = "SIMATIC IOT2000",
>>.func = 6,
>>.phy_addr = 1,
>>},
>>{
>>.name = "SIMATIC IOT2000",
>>.func = 7,
>>.phy_addr = 1,
>>},
>>
>> That's all what you need.
> 
> Nope. Again: the asset tag is the way to tell both apart AND to ensure
> that we do not match on future devices.

To be more verbose: your version (which is our old one) would even
enable the second, not connected port on the IOT2020. Incorrectly. Plus
the risk to match different future devices.

Jan

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

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 11:46, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>> On 2017-04-25 09:30, Andy Shevchenko wrote:
>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>>>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kis...@siemens.com> 
>>>>> wrote:
>>>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>>>> tag in the DMI table.
>>>
>>>>>> +   const char *asset_tag;
>>>>>
>>>>> I guess this is redundant. See below.
>>>>>
>>>>>> +   {
>>>>>> +   .name = "SIMATIC IOT2000",
>>>>>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>>>>>> +   .func = 6,
>>>>>> +   .phy_addr = 1,
>>>>>> +   },
>>>>>
>>>>> The below has same definition disregard on asset_tag.
>>>>>
>>>>
>>>> There is a small difference in the asset tag, just not at the last digit
>>>> where one may expect it, look:
>>>>
>>>> ...-0YA2 -> IOT2020
>>>> ...-1YA2 -> IOT2040
>>>
>>> Yes. And how does it change my statement? You may use one record here
>>> instead of two.
>>
>> How? Please be more verbose in your comments.
> 
>{
>.name = "SIMATIC IOT2000",
>.func = 6,
>.phy_addr = 1,
>},
>{
>.name = "SIMATIC IOT2000",
>.func = 7,
>.phy_addr = 1,
>},
> 
> That's all what you need.

Nope. Again: the asset tag is the way to tell both apart AND to ensure
that we do not match on future devices.

Jan

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

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-25 09:30, Andy Shevchenko wrote:
> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>> On 2017-04-24 23:27, Andy Shevchenko wrote:
>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>>>> The IOT2000 is industrial controller platform, derived from the Intel
>>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>>> IOT2040 has two of them. They can be told apart based on the board asset
>>>> tag in the DMI table.
> 
>>>> +   const char *asset_tag;
>>>
>>> I guess this is redundant. See below.
>>>
>>>> +   {
>>>> +   .name = "SIMATIC IOT2000",
>>>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>>>> +   .func = 6,
>>>> +   .phy_addr = 1,
>>>> +   },
>>>
>>> The below has same definition disregard on asset_tag.
>>>
>>
>> There is a small difference in the asset tag, just not at the last digit
>> where one may expect it, look:
>>
>> ...-0YA2 -> IOT2020
>> ...-1YA2 -> IOT2040
> 
> Yes. And how does it change my statement? You may use one record here
> instead of two.

How? Please be more verbose in your comments.

> 
>>
>>>> +   {
>>>> +   .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,
>>>> +   },
>>>
>>> How this supposed to work if phy_addr is the same?
>> That address space is MAC-local, and we have two different MACs here.
> 
> Got it, though asset_tag here is redundant as well.
> 

It's not as it is the only differentiating criteria to tell the
two-ports variant apart from the one-port (and to avoid confusing it
with any potential future variant). We could leave out the name, but I
kept it for documentation purposes.

Jan

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

Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

On 2017-04-24 23:27, Andy Shevchenko wrote:
> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
>> The IOT2000 is industrial controller platform, derived from the Intel
>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>> IOT2040 has two of them. They can be told apart based on the board asset
>> tag in the DMI table.
>>
>> Based on patch by Sascha Weisenberger.
>>
> 
>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
>> Signed-off-by: Sascha Weisenberger <sascha.weisenber...@siemens.com>
> 
> Shoudn't be ordered other way around?

Nope. My changes invalidated Sascha's signed-off on the original patch,
but he signed off again on the final version.

> 
>> +   const char *asset_tag;
> 
> I guess this is redundant. See below.
> 
>> +   {
>> +   .name = "SIMATIC IOT2000",
>> +   .asset_tag = "6ES7647-0AA00-0YA2",
>> +   .func = 6,
>> +   .phy_addr = 1,
>> +   },
> 
> The below has same definition disregard on asset_tag.
> 

There is a small difference in the asset tag, just not at the last digit
where one may expect it, look:

...-0YA2 -> IOT2020
...-1YA2 -> IOT2040

>> +   {
>> +   .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,
>> +   },
> 
> How this supposed to work if phy_addr is the same?
> 

That address space is MAC-local, and we have two different MACs here.

Jan

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

[PATCH] stmmac: Add support for SIMATIC IOT2000 platform

[Jan Kiszka]

The IOT2000 is industrial controller platform, derived from the Intel
Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
IOT2040 has two of them. They can be told apart based on the board asset
tag in the DMI table.

Based on patch by Sascha Weisenberger.

Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
Signed-off-by: Sascha Weisenberger <sascha.weisenber...@siemens.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5c9e462276b9..de87c329fb5c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -32,6 +32,7 @@
  */
 struct stmmac_pci_dmi_data {
const char *name;
+   const char *asset_tag;
unsigned int func;
int phy_addr;
 };
@@ -46,6 +47,7 @@ struct stmmac_pci_info {
 static int stmmac_pci_find_phy_addr(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);
struct stmmac_pci_dmi_data *dmi;
 
@@ -57,7 +59,8 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info 
*info)
return 1;
 
for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
-   if (!strcmp(dmi->name, name) && dmi->func == func)
+   if (dmi->func == func && !strcmp(dmi->name, name) &&
+   (!dmi->asset_tag || !strcmp(dmi->asset_tag, asset_tag)))
return dmi->phy_addr;
}
 
@@ -142,6 +145,24 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
.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,
+   },
{}
 };
 

Re: [PATCH 4/6] d80211: don't display name of invisible network device

[Jan Kiszka]

2007/1/30, Johannes Berg [EMAIL PROTECTED]:

On Mon, 2007-01-29 at 18:48 +0100, Jiri Benc wrote:
 Invisible master interface does not have meaningful name. Display the wiphy
 identifier in kernel messages instead.

 Also, remove the allocation of master interface name as it is purposeless
 now.

If the master netdev no longer has a name, how can you still use `tc' on
it?


I hope you can't, because that was recently proven to be able to
subtly break the stack:

http://www.mail-archive.com/netdev@vger.kernel.org/msg29219.html

Jan
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: don't symlink empty default keys

[Jan Kiszka]

Jiri Benc wrote:
 On Tue, 09 Jan 2007 23:33:34 +0100, Jan Kiszka wrote:
 This gets rid of annoying

 wlan0: cannot create symlink to default key

 in my syslog with latest rt2x00. The patch takes care that in case of
 (key/old_key == NULL  set_tx_key) the existing default key symlink is
 removed correctly. Moreover, it tests for key!=NULL before trying to register
 a new default link.

 Signed-off-by: Jan Kiszka [EMAIL PROTECTED]

 ---
  ieee80211/ieee80211_ioctl.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 Index: rt2x00/ieee80211/ieee80211_ioctl.c
 ===
 --- rt2x00.orig/ieee80211/ieee80211_ioctl.c
 +++ rt2x00/ieee80211/ieee80211_ioctl.c
 @@ -629,7 +629,7 @@ static int ieee80211_set_encryption(stru
  }
  kfree(keyconf);
  
 -if (key  sdata-default_key == key) {
 +if (set_tx_key || (key  sdata-default_key == key)) {
  ieee80211_key_sysfs_remove_default(sdata);
 
 This is not correct when set_tx_key is set and sdata-default_key is
 NULL.

Hmm, is this required? Will sysfs_remove_link panic on non-existent
nodes? If yes or if it's considered better style, are you OK with
catching NULL in ieee80211_key_sysfs_remove_default and refactoring the
existing tests along this way?

Jan



signature.asc
Description: OpenPGP digital signature

[PATCH] d80211: fix default key symlink creation/cleanup

[Jan Kiszka]

Hi Jiri,

here is the revised version of 'don't symlink empty default keys'. Run-tested
and diff'ed against your repos.

Jan


--

This gets rid of annoying

wlan0: cannot create symlink to default key

in my syslog with latest rt2x00. The patch takes care to always delete an
existing symlink to the default key before trying to register a new one.
Moreover, it avoids to call ieee80211_key_sysfs_add_default for a NULL key.

Signed-off-by: Jan Kiszka [EMAIL PROTECTED]

---
 net/d80211/ieee80211_ioctl.c |9 -
 net/d80211/ieee80211_sysfs_sta.c |3 ++-
 2 files changed, 6 insertions(+), 6 deletions(-)

Index: linux-dscape/net/d80211/ieee80211_ioctl.c
===
--- linux-dscape.orig/net/d80211/ieee80211_ioctl.c
+++ linux-dscape/net/d80211/ieee80211_ioctl.c
@@ -627,7 +627,7 @@ static int ieee80211_set_encryption(stru
}
kfree(keyconf);
 
-   if (key  sdata-default_key == key) {
+   if (set_tx_key || sdata-default_key == key) {
ieee80211_key_sysfs_remove_default(sdata);
sdata-default_key = NULL;
}
@@ -671,7 +671,7 @@ static int ieee80211_set_encryption(stru
}
}
 
-   if (old_key  sdata-default_key == old_key) {
+   if (set_tx_key || sdata-default_key == old_key) {
ieee80211_key_sysfs_remove_default(sdata);
sdata-default_key = NULL;
}
@@ -698,7 +698,7 @@ static int ieee80211_set_encryption(stru
 
if (set_tx_key || (!sta  !sdata-default_key  key)) {
sdata-default_key = key;
-   if (ieee80211_key_sysfs_add_default(sdata))
+   if (key  ieee80211_key_sysfs_add_default(sdata))
printk(KERN_WARNING %s: cannot create symlink to 
   default key\n, dev-name);
if (local-ops-set_key_idx 
@@ -2892,8 +2892,7 @@ static int ieee80211_ioctl_siwencode(str
else if (erq-length == 0) {
/* No key data - just set the default TX key index */
if (sdata-default_key != sdata-keys[idx]) {
-   if (sdata-default_key)
-   ieee80211_key_sysfs_remove_default(sdata);
+   ieee80211_key_sysfs_remove_default(sdata);
sdata-default_key = sdata-keys[idx];
if (sdata-default_key)
ieee80211_key_sysfs_add_default(sdata);
Index: linux-dscape/net/d80211/ieee80211_sysfs_sta.c
===
--- linux-dscape.orig/net/d80211/ieee80211_sysfs_sta.c
+++ linux-dscape/net/d80211/ieee80211_sysfs_sta.c
@@ -433,5 +433,6 @@ int ieee80211_key_sysfs_add_default(stru
 
 void ieee80211_key_sysfs_remove_default(struct ieee80211_sub_if_data *sdata)
 {
-   sysfs_remove_link(sdata-key_kset.kobj, default);
+   if (sdata-default_key)
+   sysfs_remove_link(sdata-key_kset.kobj, default);
 }





signature.asc
Description: OpenPGP digital signature

[PATCH] d80211: don't symlink empty default keys

[Jan Kiszka]

This gets rid of annoying

wlan0: cannot create symlink to default key

in my syslog with latest rt2x00. The patch takes care that in case of
(key/old_key == NULL  set_tx_key) the existing default key symlink is
removed correctly. Moreover, it tests for key!=NULL before trying to register
a new default link.

Signed-off-by: Jan Kiszka [EMAIL PROTECTED]

---
 ieee80211/ieee80211_ioctl.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: rt2x00/ieee80211/ieee80211_ioctl.c
===
--- rt2x00.orig/ieee80211/ieee80211_ioctl.c
+++ rt2x00/ieee80211/ieee80211_ioctl.c
@@ -629,7 +629,7 @@ static int ieee80211_set_encryption(stru
}
kfree(keyconf);
 
-   if (key  sdata-default_key == key) {
+   if (set_tx_key || (key  sdata-default_key == key)) {
ieee80211_key_sysfs_remove_default(sdata);
sdata-default_key = NULL;
}
@@ -673,7 +673,7 @@ static int ieee80211_set_encryption(stru
}
}
 
-   if (old_key  sdata-default_key == old_key) {
+   if (set_tx_key || (old_key  sdata-default_key == old_key)) {
ieee80211_key_sysfs_remove_default(sdata);
sdata-default_key = NULL;
}
@@ -700,7 +700,7 @@ static int ieee80211_set_encryption(stru
 
if (set_tx_key || (!sta  !sdata-default_key  key)) {
sdata-default_key = key;
-   if (ieee80211_key_sysfs_add_default(sdata))
+   if (key  ieee80211_key_sysfs_add_default(sdata))
printk(KERN_WARNING %s: cannot create symlink to 
   default key\n, dev-name);
if (local-ops-set_key_idx 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] d80211: don't symlink empty default keys

[Jan Kiszka]

Jan Kiszka wrote:
 This gets rid of annoying
 
 wlan0: cannot create symlink to default key
 
 in my syslog with latest rt2x00. The patch takes care that in case of
 (key/old_key == NULL  set_tx_key) the existing default key symlink is
 removed correctly. Moreover, it tests for key!=NULL before trying to register
 a new default link.
 

Grr, just noticed that the subject was still only reflecting one part of
the patch. Let's call it fix default key symlink creation/cleanup.

Jan



signature.asc
Description: OpenPGP digital signature

Re: d80211: How does TX flow control work?

[Jan Kiszka]

Jan Kiszka wrote:
 Jan Kiszka wrote:
 Jiri Benc wrote:
 On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
 BUG: warning at 
 /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
  cfa02245 ieee80211_master_start_xmit+0x105/0x430 [80211]  c024e35d 
 __ip_ct_refresh_acct+0x4d/0x60
  c024fd11 tcp_packet+0x941/0x970  c0217442 qdisc_restart+0x92/0x100
  c020d43d dev_queue_xmit+0xbd/0x1a0  cfa050d8 
 ieee80211_subif_start_xmit+0x468/0x480 [80211]
  c0207dca skb_clone+0x3a/0x1a0  c021d16d nf_hook_slow+0x4d/0xc0
  c020d495 dev_queue_xmit+0x115/0x1a0  c0226a63 ip_output+0x1c3/0x200
  c0225740 ip_finish_output+0x0/0x180  c022628b 
 ip_queue_xmit+0x36b/0x3b0
  c0224130 dst_output+0x0/0x10  ce9bae7d usb_hcd_giveback_urb+0x2d/0x60 
 [usbcore]
  c0237da2 tcp_v4_send_check+0x82/0xd0  c0237da2 
 tcp_v4_send_check+0x82/0xd0
  c0233244 tcp_transmit_skb+0x5e4/0x610  c0234b36 
 __tcp_push_pending_frames+0x676/0x740
  c0207f81 __alloc_skb+0x51/0x100  c022b817 tcp_sendmsg+0x897/0x980
  c0153fa9 core_sys_select+0x1b9/0x2b0  c0241f1d inet_sendmsg+0x3d/0x50
  c0202a8f do_sock_write+0x8f/0xa0  c020301f sock_aio_write+0x5f/0x70
  c01443d3 do_sync_write+0xc3/0x100  c01247f0 
 autoremove_wake_function+0x0/0x40
  c0144ca1 vfs_write+0xa1/0x140  c01451d3 sys_write+0x43/0x70
  c0102ae7 syscall_call+0x7/0xb

 Does it tell you anything already? Is there something I may instrument? 
 What
 could the driver do wrong to trigger such bug?
 Do you have CONFIG_NET_SCHED enabled?

 
 Sorry, this was most probably false alarm for the official stack. The
 problem now appears to be related to a patch against d80211 that is only
 present in the rt2x00 CVS.

Well, I said most probably...

The actual problem was meanwhile identified: shorewall happened to
overwrite the queueing discipline of wmaster0 with pfifo_fast. I found
the magic knob to tell shorewall to no longer do this (at least until I
want to manage traffic control that way...), but I still wonder if it is
an acceptable situation. Currently, the user can intentionally or
accidentally screw up the stack this way.

Jan


PS: Tests performed on a 2.6.17 kernel, but I don't see a reason why
newer kernels should be immune.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] d80211: Fix inconsistent sta_lock usage

[Jan Kiszka]

Ivo van Doorn wrote:
 +#define __bss_tim_set(__bss, __aid)  __set_bit((__aid), (__bss)-tim)
 +

__set/clear_bit demands unsigned long, tim is u8. That causes quite some
warnings here.

...
  static inline void bss_tim_clear(struct ieee80211_local *local,
struct ieee80211_if_ap *bss, int aid)
  {
   spin_lock(local-sta_lock);
 - bss-tim[(aid)/8] = !(1((aid) % 8));
 + __bss_tim_clear(bss, aid);
   spin_unlock(local-sta_lock);

Probably forgotten: we need _bh here as well.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] d80211: Fix inconsistent sta_lock usage

[Jan Kiszka]

Johannes Berg wrote:
 On Fri, 2007-01-05 at 21:08 +0100, Ivo van Doorn wrote:
 
 This patch uses the __set_bit and __clear_but as suggested by Christoph.
 It also removes the local argument since it was unused.
 
 NACK. This breaks spec compliance for drivers that use the TIM in their
 beacon frames.

Bit ordering, I see. Then go for my original patch + comments why this
is open-coded in __bss_tim_set/clear + removed unused arguments from
those functions, OK?

Jan



signature.asc
Description: OpenPGP digital signature

Re: d80211: How does TX flow control work?

[Jan Kiszka]

Jan Kiszka wrote:
 Jiri Benc wrote:
 On Wed, 03 Jan 2007 19:10:01 +0100, Jan Kiszka wrote:
 BUG: warning at 
 /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
  cfa02245 ieee80211_master_start_xmit+0x105/0x430 [80211]  c024e35d 
 __ip_ct_refresh_acct+0x4d/0x60
  c024fd11 tcp_packet+0x941/0x970  c0217442 qdisc_restart+0x92/0x100
  c020d43d dev_queue_xmit+0xbd/0x1a0  cfa050d8 
 ieee80211_subif_start_xmit+0x468/0x480 [80211]
  c0207dca skb_clone+0x3a/0x1a0  c021d16d nf_hook_slow+0x4d/0xc0
  c020d495 dev_queue_xmit+0x115/0x1a0  c0226a63 ip_output+0x1c3/0x200
  c0225740 ip_finish_output+0x0/0x180  c022628b ip_queue_xmit+0x36b/0x3b0
  c0224130 dst_output+0x0/0x10  ce9bae7d usb_hcd_giveback_urb+0x2d/0x60 
 [usbcore]
  c0237da2 tcp_v4_send_check+0x82/0xd0  c0237da2 
 tcp_v4_send_check+0x82/0xd0
  c0233244 tcp_transmit_skb+0x5e4/0x610  c0234b36 
 __tcp_push_pending_frames+0x676/0x740
  c0207f81 __alloc_skb+0x51/0x100  c022b817 tcp_sendmsg+0x897/0x980
  c0153fa9 core_sys_select+0x1b9/0x2b0  c0241f1d inet_sendmsg+0x3d/0x50
  c0202a8f do_sock_write+0x8f/0xa0  c020301f sock_aio_write+0x5f/0x70
  c01443d3 do_sync_write+0xc3/0x100  c01247f0 
 autoremove_wake_function+0x0/0x40
  c0144ca1 vfs_write+0xa1/0x140  c01451d3 sys_write+0x43/0x70
  c0102ae7 syscall_call+0x7/0xb

 Does it tell you anything already? Is there something I may instrument? What
 could the driver do wrong to trigger such bug?
 Do you have CONFIG_NET_SCHED enabled?


Sorry, this was most probably false alarm for the official stack. The
problem now appears to be related to a patch against d80211 that is only
present in the rt2x00 CVS.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [2.6 patch] the scheduled eepro100 removal

[Jan Kiszka]

Adrian Bunk wrote:
 This patch contains the scheduled removal of the eepro100 driver.
 

I'm sorry to disturb the schedule, but I'm not sure right now if this
pending issue of the e100 was meanwhile solved or declared a non-issue:

http://lkml.org/lkml/2006/9/8/105

Auke, can you confirm that it makes sense to re-test? IIRC, our private
thread ended without resolution after I discovered that the chip
revision makes the difference for me. Looked like it is either handled
incorrectly by e100 or screwed up on that board.

Jan
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: d80211: How does TX flow control work?

[Jan Kiszka]

Jiri Benc wrote:

 On Tue, 02 Jan 2007 14:08:21 +0100, Jan Kiszka wrote:
   
 What I (think to) understand is that a low-level drivers call
 ieee80211_stop_queue() if they run out of buffers. That flips a
 per-queue bit (IEEE80211_LINK_STATE_XOFF), prevents that any further
 frame is passed to the low-level TX routine,
 

 Correct.

   
 and can cause that up to
 *one* packet per queue is stored in
 ieee80211_local::pending_packets[queue].
 

 This is needed due to fragmented frames. After resume, passing of
 fragments to the driver has to continue where it was stopped. Returning
 the half-sent fragmented frame to the 802.11 qdisc wasn't possible
 until recently (I think the conversion of master interface to native
 802.11 type could allow that now - but it's probably not worth the
 effort).

   
 But it looks to me like nothing
 prevents ieee80211_tx() being invoked even in case that there is already
 some stuff in that single-packet storage.
 

 The 802.11 qdisc (see wme_qdiscop_dequeue) takes care of that.

   
Ahh, that is an interesting new piece in the puzzle.


 That in turn triggers WARN_ONs in ieee80211_tx() under high load for me
 (with rt2500usb). And it should also cause orphaned skbs because the
 storage is overwritten in that case. Either I'm blind or something is
 fishy...
 

 You are most likely hitting some bug. Could you post more information
 please?

   
Test scenario is rt2500usb from the rt2x00 CVS (+my currently half-pending
series), an ASUS WL167g USB stick, and hostapd driving that stick in master
mode. As soon as I trigger the AP to send out some longer TCP stream, I get
these warnings:

BUG: warning at /usr/src/rt2x00/rt2x00/ieee80211/ieee80211.c:1256/ieee80211_tx()
 cfa02245 ieee80211_master_start_xmit+0x105/0x430 [80211]  c024e35d 
__ip_ct_refresh_acct+0x4d/0x60
 c024fd11 tcp_packet+0x941/0x970  c0217442 qdisc_restart+0x92/0x100
 c020d43d dev_queue_xmit+0xbd/0x1a0  cfa050d8 
ieee80211_subif_start_xmit+0x468/0x480 [80211]
 c0207dca skb_clone+0x3a/0x1a0  c021d16d nf_hook_slow+0x4d/0xc0
 c020d495 dev_queue_xmit+0x115/0x1a0  c0226a63 ip_output+0x1c3/0x200
 c0225740 ip_finish_output+0x0/0x180  c022628b ip_queue_xmit+0x36b/0x3b0
 c0224130 dst_output+0x0/0x10  ce9bae7d usb_hcd_giveback_urb+0x2d/0x60 
[usbcore]
 c0237da2 tcp_v4_send_check+0x82/0xd0  c0237da2 tcp_v4_send_check+0x82/0xd0
 c0233244 tcp_transmit_skb+0x5e4/0x610  c0234b36 
__tcp_push_pending_frames+0x676/0x740
 c0207f81 __alloc_skb+0x51/0x100  c022b817 tcp_sendmsg+0x897/0x980
 c0153fa9 core_sys_select+0x1b9/0x2b0  c0241f1d inet_sendmsg+0x3d/0x50
 c0202a8f do_sock_write+0x8f/0xa0  c020301f sock_aio_write+0x5f/0x70
 c01443d3 do_sync_write+0xc3/0x100  c01247f0 
autoremove_wake_function+0x0/0x40
 c0144ca1 vfs_write+0xa1/0x140  c01451d3 sys_write+0x43/0x70
 c0102ae7 syscall_call+0x7/0xb

Does it tell you anything already? Is there something I may instrument? What
could the driver do wrong to trigger such bug?

Jan




signature.asc
Description: OpenPGP digital signature

[PATCH] d80211: Reinit keys on mode change

[Jan Kiszka]

Switching the interface mode with some encryption keys set and then later
touching any key, triggers an oops because ieee80211_if_reinit fails to
NULL'ify the related pointers after free'ing the key on mode change. Long
explanation, simple fix below.

Signed-off-by: Jan Kiszka [EMAIL PROTECTED]

[Sorry, yet another rt2x00 CVS patch...]

---
 ieee80211/ieee80211_iface.c |1 +
 1 file changed, 1 insertion(+)

Index: rt2x00/ieee80211/ieee80211_iface.c
===
--- rt2x00.orig/ieee80211/ieee80211_iface.c
+++ rt2x00/ieee80211/ieee80211_iface.c
@@ -231,6 +231,7 @@ void ieee80211_if_reinit(struct net_devi
local-keys[i], 0);
 #endif
ieee80211_key_free(sdata-keys[i]);
+   sdata-keys[i] = NULL;
}
 
/* Shouldn't be necessary but won't hurt */




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] d80211: ieee80211_hw handlers should be allowed to sleep

[Jan Kiszka]

Ivo van Doorn wrote:
 On Wednesday 18 October 2006 15:06, Jiri Benc wrote:
 On Sat, 7 Oct 2006 11:23:15 +0200, Ivo van Doorn wrote:
 --- a/net/d80211/ieee80211.c
 +++ b/net/d80211/ieee80211.c
 @@ -2075,15 +2075,15 @@ void ieee80211_if_shutdown(struct net_de
 case IEEE80211_IF_TYPE_STA:
 case IEEE80211_IF_TYPE_IBSS:
 sdata-u.sta.state = IEEE80211_DISABLED;
 -   del_timer_sync(sdata-u.sta.timer);
 +   cancel_delayed_work(sdata-u.sta.work);
 if (local-scan_work.data == sdata-dev) {
 local-sta_scanning = 0;
 cancel_delayed_work(local-scan_work);
 -   flush_scheduled_work();
 /* see comment in ieee80211_unregister_hw to
  * understand why this works */
 local-scan_work.data = NULL;
 }
 +   flush_scheduled_work();
 This is racy. local-scan_work.data can be set to NULL only after
 flush_scheduled_work().
 
 Would something like the patch below be better?
 It keeps the flush_scheduled_work() at the same location, but a second
 is added in case local-scan_work.data != sdata-dev
 
 Jan, was there any particular reason to move flush_cheduled_work() outside of 
 the if-statement?

It is needed unconditionally now, so I moved it out without knowing
about this side effect. Your approach looks good to me.

 
 Signed-off-by Ivo van Doorn [EMAIL PROTECTED]
 
 ---
 
 diff --git a/net/d80211/ieee80211.c b/net/d80211/ieee80211.c
 index 32a1ba7..cb1180c 100644
 --- a/net/d80211/ieee80211.c
 +++ b/net/d80211/ieee80211.c
 @@ -2075,7 +2075,7 @@ void ieee80211_if_shutdown(struct net_de
   case IEEE80211_IF_TYPE_STA:
   case IEEE80211_IF_TYPE_IBSS:
   sdata-u.sta.state = IEEE80211_DISABLED;
 - del_timer_sync(sdata-u.sta.timer);
 + cancel_delayed_work(sdata-u.sta.work);
   if (local-scan_work.data == sdata-dev) {
   local-sta_scanning = 0;
   cancel_delayed_work(local-scan_work);
 @@ -2083,7 +2083,8 @@ void ieee80211_if_shutdown(struct net_de
   /* see comment in ieee80211_unregister_hw to
* understand why this works */
   local-scan_work.data = NULL;
 - }
 + } else
 + flush_scheduled_work();
   break;
   }
  }
 @@ -4605,8 +4606,8 @@ void ieee80211_unregister_hw(struct net_
   flush_scheduled_work();
   /* The scan_work is guaranteed not to be called at this
* point. It is not scheduled and not running now. It can be
 -  * scheduled again only by some sta_timer (all of them are
 -  * stopped by now) or under rtnl lock. */
 +  * scheduled again only by sta_work (stopped by now) or under
 +  * rtnl lock. */
   }
  
   ieee80211_rx_bss_list_deinit(dev);
 diff --git a/net/d80211/ieee80211_i.h b/net/d80211/ieee80211_i.h
 index 89666ec..5b48ce2 100644
 --- a/net/d80211/ieee80211_i.h
 +++ b/net/d80211/ieee80211_i.h
 @@ -240,7 +240,7 @@ struct ieee80211_if_sta {
   IEEE80211_ASSOCIATE, IEEE80211_ASSOCIATED,
   IEEE80211_IBSS_SEARCH, IEEE80211_IBSS_JOINED
   } state;
 - struct timer_list timer;
 + struct work_struct work;
   u8 bssid[ETH_ALEN], prev_bssid[ETH_ALEN];
   u8 ssid[IEEE80211_MAX_SSID_LEN];
   size_t ssid_len;
 @@ -621,7 +621,7 @@ int ieee80211_set_compression(struct iee
 struct net_device *dev, struct sta_info *sta);
  int ieee80211_init_client(struct net_device *dev);
  /* ieee80211_sta.c */
 -void ieee80211_sta_timer(unsigned long ptr);
 +void ieee80211_sta_work(void *ptr);
  void ieee80211_sta_rx_mgmt(struct net_device *dev, struct sk_buff *skb,
  struct ieee80211_rx_status *rx_status);
  int ieee80211_sta_set_ssid(struct net_device *dev, char *ssid, size_t len);
 diff --git a/net/d80211/ieee80211_iface.c b/net/d80211/ieee80211_iface.c
 index 9a187af..4dd480f 100644
 --- a/net/d80211/ieee80211_iface.c
 +++ b/net/d80211/ieee80211_iface.c
 @@ -194,9 +194,7 @@ void ieee80211_if_set_type(struct net_de
   struct ieee80211_if_sta *ifsta;
  
   ifsta = sdata-u.sta;
 - init_timer(ifsta-timer);
 - ifsta-timer.data = (unsigned long) dev;
 - ifsta-timer.function = ieee80211_sta_timer;
 + INIT_WORK(ifsta-work, ieee80211_sta_work, dev);
  
   ifsta-capab = WLAN_CAPABILITY_ESS;
   ifsta-auth_algs = IEEE80211_AUTH_ALG_OPEN |
 diff --git a/net/d80211/ieee80211_sta.c b/net/d80211/ieee80211_sta.c
 index cc336bd..bf74b6b 100644
 --- a/net/d80211/ieee80211_sta.c
 +++ b/net/d80211/ieee80211_sta.c
 @@ -457,7 +457,7 @@ static void ieee80211_authenticate(struc
  
   ieee80211_send_auth(dev, ifsta, 1, NULL, 0, 0);
  
 - mod_timer(ifsta-timer, jiffies + 

Re: d80211: ieee80211_hw handlers in atomic context

[Jan Kiszka]

Ivo van Doorn wrote:
 On Thursday 05 October 2006 13:37, Jiri Benc wrote:
 On Wed, 4 Oct 2006 19:22:38 +0200, Ivo van Doorn wrote:
 Well another point of concern for me is the TSF handling, those handlers 
 are called
 from interrupt context as well, and also deliver problems for the USB 
 drivers in case
 of adhoc mode.
 Where is a problem with tsf handlers? get_tsf is not called at all
 (unless CONFIG_D80211_IBSS_DEBUG is set; well, that raises a question
 why the function exists in the first place), reset_tsf returns void.
 
 Basically it comes down to this:
 
 Sep 13 12:27:34 wz4a kernel: wlan0: Creating new IBSS network, BSSID 
 7a:b9:60:8a:84:39
 Sep 13 12:27:34 wz4a kernel: BUG: scheduling while atomic: 
 swapper/0x0100/0
 Sep 13 12:27:34 wz4a kernel:  b02701e7 schedule+0x43/0xa84  b01ef8b8 
 extract_buf+0x97/0xc8
 Sep 13 12:27:34 wz4a kernel:  b0270ca8 wait_for_completion+0x6a/0x9f  
 b0116f2d default_wake_function+0x0/0xc
 Sep 13 12:27:34 wz4a kernel:  c08ac59f usb_start_wait_urb+0x98/0xdc 
 [usbcore]  c08ac418 timeout_kill+0x0/0x5 [usbcore]
 Sep 13 12:27:34 wz4a kernel:  c08ac7d5 usb_control_msg+0xc3/0xde [usbcore]  
 c0b6c0f1 rt2x00_vendor_request+0x7c/0xa6 [rt73usb]
 Sep 13 12:27:34 wz4a kernel:  c0b6fa68 rt73usb_reset_tsf+0x30/0x59 
 [rt73usb]  c0bdb3e8 ieee80211_sta_join_ibss+0x3a/0x572 [80211]
 Sep 13 12:27:34 wz4a kernel:  b011cea9 printk+0x14/0x18  c0bdaa4a 
 ieee80211_rx_bss_add+0x88/0x90 [80211]
 Sep 13 12:27:34 wz4a kernel:  c0bdbc2e ieee80211_sta_find_ibss+0x30e/0x366 
 [80211]  c0bdda17 ieee80211_sta_timer+0x0/0x18f [80211]
 Sep 13 12:27:34 wz4a kernel:  c0bdda91 ieee80211_sta_timer+0x7a/0x18f 
 [80211]  c0bdda17 ieee80211_sta_timer+0x0/0x18f [80211]
 Sep 13 12:27:34 wz4a kernel:  b01241b1 run_timer_softirq+0x10b/0x153  
 b0120a52 __do_softirq+0x58/0xc2
 Sep 13 12:27:34 wz4a kernel:  b0120aea do_softirq+0x2e/0x32  b0104fe6 
 do_IRQ+0x1e/0x24
 Sep 13 12:27:34 wz4a kernel:  b0103592 common_interrupt+0x1a/0x20  
 c0827484 acpi_processor_idle+0x18a/0x39e [processor]
 Sep 13 12:27:34 wz4a kernel:  b0101e77 cpu_idle+0x8f/0xa8  b03026d2 
 start_kernel+0x355/0x35c
 
 With the compilation of d80211 the CONFIG_D80211_DEBUG is set by default,
 so no CONFIG_D80211_IBSS_DEBUG.
 
 This does not happen in rt2500usb driver, since no TSF handling is possible
 due to a lack of TSF registers in the device.

This path would be fixed by my conversion patch of sta.timer into
sta.work that I sent you yesterday privately. Unfortunately, I don't
have a copy at hand ATM.

What about the other timers? Can they trigger any sleeping service of
rt2x00 drivers? Ok, waiting for a BUG is always possible... ;)

Jan



signature.asc
Description: OpenPGP digital signature

d80211: ieee80211_hw handlers in atomic context

[Jan Kiszka]

Hello Jiri,

Ivo suggested to bring this issue to a broader audience, specifically to
the stack maintainer.

Trying to run my Asus WL167G with rt2500usb I faced the following:

BUG: scheduling while atomic: swapper/0x0102/0
 c0103055 show_trace+0x12/0x14
 c01035e0 dump_stack+0x1c/0x1e
 c025fad1 schedule+0x5f/0x652
 c0260324 wait_for_completion+0xb8/0x134
 d0988fa1 usb_start_wait_urb+0x89/0xcb [usbcore]
 d0989192 usb_control_msg+0xb2/0xcc [usbcore]
 d089d127 rt2x00_vendor_request+0x85/0xbb [rt2500usb]
 d08a1350 rt2500usb_config+0x5e/0x3d7 [rt2500usb]
 d0823496 ieee80211_hw_config+0x2c/0x93 [80211]
 d0829950 ieee80211_ioctl_siwfreq+0x132/0x141 [80211]
 d082ee8b ieee80211_sta_join_ibss+0xcc/0x5af [80211]
 d082f698 ieee80211_sta_find_ibss+0x32a/0x374 [80211]
 d08317f8 ieee80211_sta_timer+0x81/0x1b4 [80211]
 c011ac50 run_timer_softirq+0x171/0x205
 c0117536 __do_softirq+0x41/0x90
 c01175bc do_softirq+0x37/0x4a
 c01176b7 irq_exit+0x2d/0x45
 c0104316 do_IRQ+0x53/0x5f

The reason is the invocation of rt2500usb's config handler in atomic
context (timer handler). But this service requires schedulable context
to submit and wait for some URBs.

That raises the question how to resolve the conflict best, at stack
level by pushing such work into thread context (workqueues?) or at
driver level by deferring these requests (if feasible at all without
breaking the stack's timing)? What other callback handlers in
ieee80211_hw can currently be called in atomic context? Given that all
USB WLAN adapters will have to cope with this issue in some way, it may
be wise to find a common solution.

Ivo told me about a patch for d80211 that moved certain timers to thread
context, effectively avoiding to call config from timer handlers, but I
didn't find any trace yet. Is there some modification in this direction
already scheduled? I'm not necessarily looking for work, at best I would
just enjoy to use it. ;)

Jan
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: d80211: ieee80211_hw handlers in atomic context

[Jan Kiszka]

Ivo van Doorn wrote:
 Hi,
 
 Ivo told me about a patch for d80211 that moved certain timers to thread
 context, effectively avoiding to call config from timer handlers, but I
 didn't find any trace yet. Is there some modification in this direction
 already scheduled? I'm not necessarily looking for work, at best I would
 just enjoy to use it. ;)
 
 I have found the actual patch:
 [PATCH 1/5] d80211: make sleeping in hw-config possible
 And was send on august 1st by Jiri to the netdev list.
 It was based on a patch by Michael Buesch.

Ah, looks like I didn't dug thoroughly enough.

Anyway, this means my BUG proved the patch's claim wrong :o), at least
one atomic gremlin is left:

ieee80211_sta_timer -
ieee80211_sta_find_ibss -
ieee80211_sta_join_ibss -
ieee80211_ioctl_siwfreq -
ieee80211_hw_config

Anyone already an idea how to fix it?

Jan
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: d80211: ieee80211_hw handlers in atomic context

[Jan Kiszka]

Jiri Benc wrote:
 On Wed, 4 Oct 2006 18:34:57 +0200, Ivo van Doorn wrote:
 You could replace the timer with a workqueue, the original patch
 also did that, so I think it would be good enough this time as well. :)
 
 Yes, the timing isn't required to be precise here.

Ok, I'm not promising success and I'm going to duck immediately if
someone else feels like working on it, but I could try to patch in this
direction.

Now there just remains my precautious question if there are other
services in the ieee_80211_hw interface that may conflict with sleeping
USB drivers. What about specifying the possible contexts in
include/net/d80211.h?

Jan
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

e100 fails, eepro100 works

[Jan Kiszka]

Hi,

we have a couple of industrial PCs here with Intel PRO/100 controllers
on board. Most of them work fine with the e100, but today I stumbled
over one box that doesn't: Reception works (RX counter increases, ARP
cache gets filled up), but transmission fails (TX counter is also zero).
In contrast, the eepro100 is fine, also Etherboot's driver.

I'm currently on 2.6.17.8, but I don't see any changes up to latest git
that may have positive influence. This is what lspci -v tells about this
piece of hardware:

00:12.0 Ethernet controller: Intel Corporation 8255xER/82551IT Fast
Ethernet Controller (rev 08)
Subsystem: Intel Corporation: Unknown device 1229
Flags: bus master, medium devsel, latency 66, IRQ 10
Memory at fc02 (32-bit, non-prefetchable) [size=4K]
I/O ports at 1080 [size=64]
Memory at fc00 (32-bit, non-prefetchable) [size=128K]
Capabilities: [dc] Power Management version 2

And here is the kernel log of e100 with highest debug level when sending
out a few pings while other packets arrive on the network:

e100: Intel(R) PRO/100 Network Driver, 3.5.10-k2-NAPI
e100: Copyright(c) 1999-2005 Intel Corporation
PCI: Found IRQ 10 for device :00:12.0
e100: eth0: e100_probe: addr 0xfc02, irq 10, MAC addr 00:30:59:01:07:A7
e100: eth0: e100_watchdog: right now = 35470
e100: eth0: e100_watchdog: link up, 100Mbps, full-duplex
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 35970
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 36470
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 36970
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 37470
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 37970
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_watchdog: right now = 38470
e100: eth0: e100_intr: stat_ack = 0x04
e100: eth0: e100_intr: stat_ack = 0x40
e100: eth0: e100_watchdog: right now = 38970
e100: eth0: e100_intr: stat_ack = 0x04

I may find the time one day to debug this at lower levels, but you could
accelerate this process with any pointer where to dig deeper precisely.

Jan



signature.asc
Description: OpenPGP digital signature

Re: e100 fails, eepro100 works

[Jan Kiszka]

Auke Kok wrote:
 Can you include a full `dmesg` and `lcpci -vv -s 00:12.0` ?
 
 Also you're using 3.5.10-k2, can you try the current git tree version
 instead? I can send you the e100.c if wanted.

Yes, please, to make sure that we'll really discuss the same version.
Will then try to collect the additional information on Monday.

Jan



signature.asc
Description: OpenPGP digital signature