Re: [PATCH net-next 1/1] stmmac: intel: change all EHL/TGL to auto detect phy addr
On 06.11.20 10:43, Wong Vee Khee wrote: > From: Voon Weifeng > > Set all EHL/TGL phy_addr to -1 so that the driver will automatically > detect it at run-time by probing all the possible 32 addresses. > > Signed-off-by: Voon Weifeng > Signed-off-by: Wong Vee Khee > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > index b6e5e3e36b63..7c1353f37247 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c > @@ -236,6 +236,7 @@ static int intel_mgbe_common_data(struct pci_dev *pdev, > int ret; > int i; > > + plat->phy_addr = -1; > plat->clk_csr = 5; > plat->has_gmac = 0; > plat->has_gmac4 = 1; > @@ -345,7 +346,6 @@ static int ehl_sgmii_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > plat->bus_id = 1; > - plat->phy_addr = 0; > plat->phy_interface = PHY_INTERFACE_MODE_SGMII; > > plat->serdes_powerup = intel_serdes_powerup; > @@ -362,7 +362,6 @@ static int ehl_rgmii_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > plat->bus_id = 1; > - plat->phy_addr = 0; > plat->phy_interface = PHY_INTERFACE_MODE_RGMII; > > return ehl_common_data(pdev, plat); > @@ -376,7 +375,6 @@ static int ehl_pse0_common_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > plat->bus_id = 2; > - plat->phy_addr = 1; > return ehl_common_data(pdev, plat); > } > > @@ -408,7 +406,6 @@ static int ehl_pse1_common_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > plat->bus_id = 3; > - plat->phy_addr = 1; > return ehl_common_data(pdev, plat); > } > > @@ -450,7 +447,6 @@ static int tgl_sgmii_data(struct pci_dev *pdev, > struct plat_stmmacenet_data *plat) > { > plat->bus_id = 1; > - plat->phy_addr = 0; > plat->phy_interface = PHY_INTERFACE_MODE_SGMII; > plat->serdes_powerup = intel_serdes_powerup; > plat->serdes_powerdown = intel_serdes_powerdown; > This fixes PHY detection on one of our EHL-based boards. Can this also be applied to stable 5.10? Thanks, Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
Re: [PATCH net] e1000e: Remove Other from EIAC.
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
On 2017-06-22 19:40, David Miller wrote: > From: Jan Kiszka > 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
From: 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 Reviewed-by: Andy Shevchenko --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 36 +++- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index 0efe42659a37..d3d74e526e17 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -100,7 +100,9 @@ static void common_default_data(struct plat_stmmacenet_data *plat) plat->rx_queues_cfg[0].pkt_route = 0x0; } -static void stmmac_default_data(struct plat_stmmacenet_data *plat) +static int stmmac_default_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + const struct stmmac_pci_info *info) { /* Set common default data first */ common_default_data(plat); @@ -112,8 +114,14 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat) plat->dma_cfg->pbl = 32; plat->dma_cfg->pblx8 = true; /* TODO: AXI */ + + return 0; } +static const struct stmmac_pci_info stmmac_pci_info = { + .setup = stmmac_default_data, +}; + static int quark_default_data(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, const struct stmmac_pci_info *info) @@ -236,14 +244,9 @@ static int stmmac_pci_probe(struct pci_dev *pdev, pci_set_master(pdev); - if (info) { - if (info->setup) { - ret = info->setup(pdev, plat, info); - if (ret) - return ret; - } - } else - stmmac_default_data(plat); + ret = info->setup(pdev, plat, info); + if (ret) + return ret; pci_enable_msi(pdev); @@ -269,14 +272,21 @@ static void stmmac_pci_remove(struct pci_dev *pdev) static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume); -#define STMMAC_VENDOR_ID 0x700 +/* synthetic ID, no official vendor */ +#define PCI_VENDOR_ID_STMMAC 0x700 + #define STMMAC_QUARK_ID 0x0937 #define STMMAC_DEVICE_ID 0x1108 +#define STMMAC_DEVICE(vendor_id, dev_id, info) { \ + PCI_VDEVICE(vendor_id, dev_id), \ + .driver_data = (kernel_ulong_t)&info\ + } + static const struct pci_device_id stmmac_id_table[] = { - {PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)}, - {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)}, - {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info}, + STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info), + STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info), + STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info), {} }; -- 2.12.3
[PATCH v5 0/5] stmmac: pci: Refactor DMI probing
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
From: 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 Reviewed-by: Andy Shevchenko --- 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 = "
[PATCH v5 3/5] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
From: 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 Reviewed-by: Andy Shevchenko --- 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
From: 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 Reviewed-by: Andy Shevchenko --- 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
From: Jan Kiszka Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr. Signed-off-by: Jan Kiszka --- 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 *)&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
[PATCH v4 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr. Signed-off-by: Jan Kiszka --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 99 1 file changed, 66 insertions(+), 33 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index a6e10d3ced5c..2be15a8a9c40 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -30,36 +30,39 @@ * negative value of the address means that MAC controller is not connected * with PHY. */ -struct stmmac_pci_dmi_data { - const char *name; - const char *asset_tag; +struct stmmac_pci_func_data { unsigned int func; int phy_addr; }; +struct stmmac_pci_dmi_data { + const struct stmmac_pci_func_data *func; + size_t nfuncs; +}; + struct stmmac_pci_info { int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); }; static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, - struct stmmac_pci_dmi_data *dmi_data) + const struct dmi_system_id *dmi_list) { - const char *name = dmi_get_system_info(DMI_BOARD_NAME); - const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG); - unsigned int func = PCI_FUNC(pdev->devfn); - struct stmmac_pci_dmi_data *dmi; - - if (!name) + const struct stmmac_pci_func_data *func_data; + const struct stmmac_pci_dmi_data *dmi_data; + const struct dmi_system_id *dmi_id; + int func = PCI_FUNC(pdev->devfn); + size_t n; + + dmi_id = dmi_first_match(dmi_list); + if (!dmi_id) return -ENODEV; - for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) { - if (!strcmp(dmi->name, name) && dmi->func == func) { - /* If asset tag is provided, match on it as well. */ - if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag)) - continue; - return dmi->phy_addr; - } - } + dmi_data = dmi_id->driver_data; + func_data = dmi_data->func; + + for (n = 0; n < dmi_data->nfuncs; n++, func_data++) + if (func_data->func == func) + return func_data->phy_addr; return -ENODEV; } @@ -115,34 +118,64 @@ static const struct stmmac_pci_info stmmac_pci_info = { .setup = stmmac_default_data, }; -static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = { +static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = { { - .name = "Galileo", .func = 6, .phy_addr = 1, }, + { }, +}; + +static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data = { + .func = galileo_stmmac_func_data, + .nfuncs = ARRAY_SIZE(galileo_stmmac_func_data), +}; + +static const struct stmmac_pci_func_data iot2040_stmmac_func_data[] = { { - .name = "GalileoGen2", .func = 6, .phy_addr = 1, }, { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-0YA2", - .func = 6, + .func = 7, .phy_addr = 1, }, + { }, +}; + +static const struct stmmac_pci_dmi_data iot2040_stmmac_dmi_data = { + .func = iot2040_stmmac_func_data, + .nfuncs = ARRAY_SIZE(iot2040_stmmac_func_data), +}; + +static const struct dmi_system_id quark_pci_dmi[] = { { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-1YA2", - .func = 6, - .phy_addr = 1, + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Galileo"), + }, + .driver_data = (void *)&galileo_stmmac_dmi_data, }, { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-1YA2", - .func = 7, - .phy_addr = 1, + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"), + }, + .driver_data = (void *)&galileo_stmmac_dmi_data, + }, + { + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_NAME, "SIMATIC IOT2000"), + DMI_EXACT_MATCH(DMI_BOARD_ASSET_TAG, + "6ES7647-0AA00-0YA2"), + }, + .driver_data = (void *)&galileo_stmmac_dmi_data, + }, + { + .matches = { + DMI_EXACT_MATCH(DMI_BOARD_NAME
[PATCH v4 4/6] stmmac: pci: Select quark_pci_dmi_data from quark_default_data
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 Reviewed-by: Andy Shevchenko --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 83 +++- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index f44ae49eb11c..a6e10d3ced5c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -38,13 +38,11 @@ struct stmmac_pci_dmi_data { }; struct stmmac_pci_info { - int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, -const struct stmmac_pci_info *info); - struct stmmac_pci_dmi_data *dmi; + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); }; static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, - const struct stmmac_pci_info *info) + struct stmmac_pci_dmi_data *dmi_data) { const char *name = dmi_get_system_info(DMI_BOARD_NAME); const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG); @@ -54,7 +52,7 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, if (!name) return -ENODEV; - for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) { + for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) { if (!strcmp(dmi->name, name) && dmi->func == func) { /* If asset tag is provided, match on it as well. */ if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag)) @@ -97,8 +95,7 @@ static void common_default_data(struct plat_stmmacenet_data *plat) } static int stmmac_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat, - const struct stmmac_pci_info *info) + struct plat_stmmacenet_data *plat) { /* Set common default data first */ common_default_data(plat); @@ -118,9 +115,40 @@ static const struct stmmac_pci_info stmmac_pci_info = { .setup = stmmac_default_data, }; +static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = { + { + .name = "Galileo", + .func = 6, + .phy_addr = 1, + }, + { + .name = "GalileoGen2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-0YA2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-1YA2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-1YA2", + .func = 7, + .phy_addr = 1, + }, + {} +}; + static int quark_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat, - const struct stmmac_pci_info *info) + struct plat_stmmacenet_data *plat) { int ret; @@ -131,7 +159,7 @@ static int quark_default_data(struct pci_dev *pdev, * Refuse to load the driver and register net device if MAC controller * does not connect to any PHY interface. */ - ret = stmmac_pci_find_phy_addr(pdev, info); + ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data); if (ret < 0) { /* Return error to the caller on DMI enabled boards. */ if (dmi_get_system_info(DMI_BOARD_NAME)) @@ -157,41 +185,8 @@ static int quark_default_data(struct pci_dev *pdev, return 0; } -static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = { - { - .name = "Galileo", - .func = 6, - .phy_addr = 1, - }, - { - .name = "GalileoGen2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-0YA2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-1YA2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000&qu
[PATCH v4 0/6] stmmac: pci: Refactor DMI probing
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
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 Reviewed-by: Andy Shevchenko --- 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
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 Reviewed-by: Andy Shevchenko --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 36 +++- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index 0efe42659a37..d3d74e526e17 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -100,7 +100,9 @@ static void common_default_data(struct plat_stmmacenet_data *plat) plat->rx_queues_cfg[0].pkt_route = 0x0; } -static void stmmac_default_data(struct plat_stmmacenet_data *plat) +static int stmmac_default_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + const struct stmmac_pci_info *info) { /* Set common default data first */ common_default_data(plat); @@ -112,8 +114,14 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat) plat->dma_cfg->pbl = 32; plat->dma_cfg->pblx8 = true; /* TODO: AXI */ + + return 0; } +static const struct stmmac_pci_info stmmac_pci_info = { + .setup = stmmac_default_data, +}; + static int quark_default_data(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, const struct stmmac_pci_info *info) @@ -236,14 +244,9 @@ static int stmmac_pci_probe(struct pci_dev *pdev, pci_set_master(pdev); - if (info) { - if (info->setup) { - ret = info->setup(pdev, plat, info); - if (ret) - return ret; - } - } else - stmmac_default_data(plat); + ret = info->setup(pdev, plat, info); + if (ret) + return ret; pci_enable_msi(pdev); @@ -269,14 +272,21 @@ static void stmmac_pci_remove(struct pci_dev *pdev) static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume); -#define STMMAC_VENDOR_ID 0x700 +/* synthetic ID, no official vendor */ +#define PCI_VENDOR_ID_STMMAC 0x700 + #define STMMAC_QUARK_ID 0x0937 #define STMMAC_DEVICE_ID 0x1108 +#define STMMAC_DEVICE(vendor_id, dev_id, info) { \ + PCI_VDEVICE(vendor_id, dev_id), \ + .driver_data = (kernel_ulong_t)&info\ + } + static const struct pci_device_id stmmac_id_table[] = { - {PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)}, - {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)}, - {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info}, + STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info), + STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info), + STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info), {} }; -- 2.12.3
[PATCH v4 6/6] stmmac: pci: Remove setup handler indirection via stmmac_pci_info
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 --- 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)&info\ + .driver_data = (kernel_ulong_t)&setup \ } static const struct pci_device_id stmmac_id_table[] = { - STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info), - STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info), - STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info), + STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_default_setup), + STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_default_setup), + STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_default_setup), {} }; -- 2.12.3
[PATCH v4 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
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 Reviewed-by: Andy Shevchenko --- 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
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 --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 34 +--- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index a3909ab0da05..73b7b5d3a11c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -35,9 +35,7 @@ struct stmmac_pci_dmi_data { int phy_addr; }; -struct stmmac_pci_info { - int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); -}; +typedef int (*stmmac_setup)(struct pci_dev *, struct plat_stmmacenet_data *); static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, const struct dmi_system_id *dmi_list) @@ -87,8 +85,8 @@ static void common_default_data(struct plat_stmmacenet_data *plat) plat->rx_queues_cfg[0].pkt_route = 0x0; } -static int stmmac_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat) +static int stmmac_default_setup(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { /* Set common default data first */ common_default_data(plat); @@ -104,10 +102,6 @@ static int stmmac_default_data(struct pci_dev *pdev, return 0; } -static const struct stmmac_pci_info stmmac_pci_info = { - .setup = stmmac_default_data, -}; - static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = { { .func = 6, @@ -160,8 +154,8 @@ static const struct dmi_system_id quark_pci_dmi[] = { {} }; -static int quark_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat) +static int quark_default_setup(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { int ret; @@ -198,10 +192,6 @@ static int quark_default_data(struct pci_dev *pdev, return 0; } -static const struct stmmac_pci_info quark_pci_info = { - .setup = quark_default_data, -}; - /** * stmmac_pci_probe * @@ -217,7 +207,7 @@ static const struct stmmac_pci_info quark_pci_info = { static int stmmac_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data; + stmmac_setup setup = (stmmac_setup)id->driver_data; struct plat_stmmacenet_data *plat; struct stmmac_resources res; int i; @@ -258,7 +248,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev, pci_set_master(pdev); - ret = info->setup(pdev, plat); + ret = setup(pdev, plat); if (ret) return ret; @@ -292,15 +282,15 @@ static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume); #define STMMAC_QUARK_ID 0x0937 #define STMMAC_DEVICE_ID 0x1108 -#define STMMAC_DEVICE(vendor_id, dev_id, info) { \ +#define STMMAC_DEVICE(vendor_id, dev_id, setup){ \ PCI_VDEVICE(vendor_id, dev_id), \ - .driver_data = (kernel_ulong_t)&info\ + .driver_data = (kernel_ulong_t)&setup \ } static const struct pci_device_id stmmac_id_table[] = { - STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info), - STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info), - STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info), + STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_default_setup), + STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_default_setup), + STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_default_setup), {} }; -- 2.12.3
[PATCH v3 2/6] stmmac: pci: Use stmmac_pci_info for all devices
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 --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 36 +++- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index 0efe42659a37..d3d74e526e17 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -100,7 +100,9 @@ static void common_default_data(struct plat_stmmacenet_data *plat) plat->rx_queues_cfg[0].pkt_route = 0x0; } -static void stmmac_default_data(struct plat_stmmacenet_data *plat) +static int stmmac_default_data(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat, + const struct stmmac_pci_info *info) { /* Set common default data first */ common_default_data(plat); @@ -112,8 +114,14 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat) plat->dma_cfg->pbl = 32; plat->dma_cfg->pblx8 = true; /* TODO: AXI */ + + return 0; } +static const struct stmmac_pci_info stmmac_pci_info = { + .setup = stmmac_default_data, +}; + static int quark_default_data(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, const struct stmmac_pci_info *info) @@ -236,14 +244,9 @@ static int stmmac_pci_probe(struct pci_dev *pdev, pci_set_master(pdev); - if (info) { - if (info->setup) { - ret = info->setup(pdev, plat, info); - if (ret) - return ret; - } - } else - stmmac_default_data(plat); + ret = info->setup(pdev, plat, info); + if (ret) + return ret; pci_enable_msi(pdev); @@ -269,14 +272,21 @@ static void stmmac_pci_remove(struct pci_dev *pdev) static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume); -#define STMMAC_VENDOR_ID 0x700 +/* synthetic ID, no official vendor */ +#define PCI_VENDOR_ID_STMMAC 0x700 + #define STMMAC_QUARK_ID 0x0937 #define STMMAC_DEVICE_ID 0x1108 +#define STMMAC_DEVICE(vendor_id, dev_id, info) { \ + PCI_VDEVICE(vendor_id, dev_id), \ + .driver_data = (kernel_ulong_t)&info\ + } + static const struct pci_device_id stmmac_id_table[] = { - {PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)}, - {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)}, - {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info}, + STMMAC_DEVICE(STMMAC, STMMAC_DEVICE_ID, stmmac_pci_info), + STMMAC_DEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC, stmmac_pci_info), + STMMAC_DEVICE(INTEL, STMMAC_QUARK_ID, quark_pci_info), {} }; -- 2.12.3
[PATCH v3 1/6] stmmac: pci: Make stmmac_pci_info structure constant
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 --- 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 0/6] stmmac: pci: Refactor DMI probing
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 4/6] stmmac: pci: Select quark_pci_dmi_data from quark_default_data
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 --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 83 +++- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index f44ae49eb11c..a6e10d3ced5c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -38,13 +38,11 @@ struct stmmac_pci_dmi_data { }; struct stmmac_pci_info { - int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat, -const struct stmmac_pci_info *info); - struct stmmac_pci_dmi_data *dmi; + int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); }; static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, - const struct stmmac_pci_info *info) + struct stmmac_pci_dmi_data *dmi_data) { const char *name = dmi_get_system_info(DMI_BOARD_NAME); const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG); @@ -54,7 +52,7 @@ static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, if (!name) return -ENODEV; - for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) { + for (dmi = dmi_data; dmi->name && *dmi->name; dmi++) { if (!strcmp(dmi->name, name) && dmi->func == func) { /* If asset tag is provided, match on it as well. */ if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag)) @@ -97,8 +95,7 @@ static void common_default_data(struct plat_stmmacenet_data *plat) } static int stmmac_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat, - const struct stmmac_pci_info *info) + struct plat_stmmacenet_data *plat) { /* Set common default data first */ common_default_data(plat); @@ -118,9 +115,40 @@ static const struct stmmac_pci_info stmmac_pci_info = { .setup = stmmac_default_data, }; +static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = { + { + .name = "Galileo", + .func = 6, + .phy_addr = 1, + }, + { + .name = "GalileoGen2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-0YA2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-1YA2", + .func = 6, + .phy_addr = 1, + }, + { + .name = "SIMATIC IOT2000", + .asset_tag = "6ES7647-0AA00-1YA2", + .func = 7, + .phy_addr = 1, + }, + {} +}; + static int quark_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat, - const struct stmmac_pci_info *info) + struct plat_stmmacenet_data *plat) { int ret; @@ -131,7 +159,7 @@ static int quark_default_data(struct pci_dev *pdev, * Refuse to load the driver and register net device if MAC controller * does not connect to any PHY interface. */ - ret = stmmac_pci_find_phy_addr(pdev, info); + ret = stmmac_pci_find_phy_addr(pdev, quark_pci_dmi_data); if (ret < 0) { /* Return error to the caller on DMI enabled boards. */ if (dmi_get_system_info(DMI_BOARD_NAME)) @@ -157,41 +185,8 @@ static int quark_default_data(struct pci_dev *pdev, return 0; } -static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = { - { - .name = "Galileo", - .func = 6, - .phy_addr = 1, - }, - { - .name = "GalileoGen2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-0YA2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000", - .asset_tag = "6ES7647-0AA00-1YA2", - .func = 6, - .phy_addr = 1, - }, - { - .name = "SIMATIC IOT2000", - .asset_tag = &
[PATCH v3 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
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 --- 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 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr. Signed-off-by: Jan Kiszka --- 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
On 2017-05-27 15:38, Andy Shevchenko wrote: > On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka 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
On 2017-05-27 15:28, Andy Shevchenko wrote: > On Fri, May 26, 2017 at 7:07 PM, Jan Kiszka 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 = &galileo_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
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 --- drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 35 +--- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c index 061cb28f642d..485216369705 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c @@ -35,9 +35,7 @@ struct stmmac_pci_dmi_data { int phy_addr; }; -struct stmmac_pci_info { - int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat); -}; +typedef int (*stmmac_setup)(struct pci_dev *, struct plat_stmmacenet_data *); static int stmmac_pci_find_phy_addr(struct pci_dev *pdev, const struct dmi_system_id *dmi_list) @@ -87,8 +85,8 @@ static void common_default_data(struct plat_stmmacenet_data *plat) plat->rx_queues_cfg[0].pkt_route = 0x0; } -static int stmmac_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat) +static int stmmac_default_setup(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { /* Set common default data first */ common_default_data(plat); @@ -104,10 +102,6 @@ static int stmmac_default_data(struct pci_dev *pdev, return 0; } -static const struct stmmac_pci_info stmmac_pci_info = { - .setup = stmmac_default_data, -}; - static const struct stmmac_pci_dmi_data galileo_stmmac_dmi_data[] = { { .func = 6, @@ -160,8 +154,8 @@ static const struct dmi_system_id quark_pci_dmi[] = { {} }; -static int quark_default_data(struct pci_dev *pdev, - struct plat_stmmacenet_data *plat) +static int quark_default_setup(struct pci_dev *pdev, + struct plat_stmmacenet_data *plat) { int ret; @@ -197,10 +191,6 @@ static int quark_default_data(struct pci_dev *pdev, return 0; } -static const struct stmmac_pci_info quark_pci_info = { - .setup = quark_default_data, -}; - /** * stmmac_pci_probe * @@ -216,7 +206,7 @@ static const struct stmmac_pci_info quark_pci_info = { static int stmmac_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data; + stmmac_setup setup = (stmmac_setup)id->driver_data; struct plat_stmmacenet_data *plat; struct stmmac_resources res; int i; @@ -257,7 +247,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev, pci_set_master(pdev); - ret = info->setup(pdev, plat); + ret = setup(pdev, plat); if (ret) return ret; @@ -289,16 +279,17 @@ static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_suspend, stmmac_resume); #define STMMAC_QUARK_ID 0x0937 #define STMMAC_DEVICE_ID 0x1108 -#define STMMAC_DEVICE(vendor_id, dev_id, info) { \ +#define STMMAC_DEVICE(vendor_id, dev_id, setup){ \ PCI_DEVICE(vendor_id, dev_id), \ - .driver_data = (kernel_ulong_t)&info\ + .driver_data = (kernel_ulong_t)&setup \ } static const struct pci_device_id stmmac_id_table[] = { - STMMAC_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_pci_info), + STMMAC_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID, stmmac_default_setup), STMMAC_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC, - stmmac_pci_info), - STMMAC_DEVICE(PCI_VENDOR_ID_INTEL, STMMAC_QUARK_ID, quark_pci_info), + stmmac_default_setup), + STMMAC_DEVICE(PCI_VENDOR_ID_INTEL, STMMAC_QUARK_ID, + quark_default_setup), {} }; -- 2.12.0
[PATCH v2 3/6] stmmac: pci: Make stmmac_pci_find_phy_addr truly generic
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 --- 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
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 --- 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)&info\ + } + static const struct pci_device_id stmmac_id_table[] = { - {PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)}, - {PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)}, - {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info}, + STMMAC_DEVICE(STMMAC_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
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 --- 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
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 --- 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_tag = "6ES7647-0AA
[PATCH v2 5/6] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr. Signed-off-by: Jan Kiszka --- 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
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
On 2017-05-22 18:35, David Miller wrote: > From: Jan Kiszka > Date: Mon, 22 May 2017 13:12:06 +0200 > >> Some cleanups of the way we probe DMI platforms in the driver. Reduces >> a bit of open-coding and makes the logic easier reusable for any >> potential DMI platform != Quark. >> >> Tested on IOT2000 and Galileo Gen2. > > This doesn't compile: > > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:285:3: error: initializer > element is not computable at load time >(kernel_ulong_t)&stmmac_default_setup, >^ > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:285:3: note: (near > initialization for ‘stmmac_id_table[0].class’) > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:289:3: error: initializer > element is not computable at load time >(kernel_ulong_t)&stmmac_default_setup, >^ > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:289:3: note: (near > initialization for ‘stmmac_id_table[1].class’) > scripts/Makefile.build:302: recipe for target > 'drivers/net/ethernet/stmicro/stmmac/stmmac_pci.o' failed > make[5]: *** [drivers/net/ethernet/stmicro/stmmac/stmmac_pci.o] Error 1 > Hmm. Which arch is this? Jan
Re: [PATCH 3/3] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
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
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 --- 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, - }, - { - .name =
[PATCH 3/3] stmmac: pci: Use dmi_system_id table for retrieving PHY addresses
Avoids reimplementation of DMI matching in stmmac_pci_find_phy_addr. Signed-off-by: Jan Kiszka --- 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
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 --- 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
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
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 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
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&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=KhdAPUtP0twDkibE89cLYs8JjnxEvBgav5uf08WL_e8&e= >>> >>> >>> 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&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=rTxn0fwdudwq9XAquH60xNHN538KBQ6_n4wODdLoyA0&e= >>> >>> >>> 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
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 %d)\n", mode); >
[PATCH v2] stmmac: Add support for SIMATIC IOT2000 platform
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 Signed-off-by: Sascha Weisenberger --- 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
On 2017-04-25 13:42, Andy Shevchenko wrote: > On Tue, Apr 25, 2017 at 1:09 PM, Jan Kiszka 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 >>>> wrote: >>>>> On 2017-04-25 09:30, Andy Shevchenko wrote: >>>>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka >>>>>> wrote: >>>>>>> On 2017-04-24 23:27, Andy Shevchenko wrote: >>>>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka >>>>>>>> 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
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 wrote: >>> On 2017-04-25 09:30, Andy Shevchenko wrote: >>>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka wrote: >>>>> On 2017-04-24 23:27, Andy Shevchenko wrote: >>>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka >>>>>> 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
On 2017-04-25 11:46, Andy Shevchenko wrote: > On Tue, Apr 25, 2017 at 12:00 PM, Jan Kiszka wrote: >> On 2017-04-25 09:30, Andy Shevchenko wrote: >>> On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka wrote: >>>> On 2017-04-24 23:27, Andy Shevchenko wrote: >>>>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka >>>>> 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
On 2017-04-25 09:30, Andy Shevchenko wrote: > On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka wrote: >> On 2017-04-24 23:27, Andy Shevchenko wrote: >>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka 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
On 2017-04-24 23:27, Andy Shevchenko wrote: > On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka 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 >> Signed-off-by: Sascha Weisenberger > > 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
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 Signed-off-by: Sascha Weisenberger --- 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
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
[PATCH] d80211: fix default key symlink creation/cleanup
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
Re: [PATCH] d80211: don't symlink empty default keys
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
Re: [PATCH] d80211: don't symlink empty default keys
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
[PATCH] d80211: don't symlink empty default keys
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: d80211: How does TX flow control work?
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() >>>> ieee80211_master_start_xmit+0x105/0x430 [80211] >>>> __ip_ct_refresh_acct+0x4d/0x60 >>>> tcp_packet+0x941/0x970 qdisc_restart+0x92/0x100 >>>> dev_queue_xmit+0xbd/0x1a0 >>>> ieee80211_subif_start_xmit+0x468/0x480 [80211] >>>> skb_clone+0x3a/0x1a0 nf_hook_slow+0x4d/0xc0 >>>> dev_queue_xmit+0x115/0x1a0 ip_output+0x1c3/0x200 >>>> ip_finish_output+0x0/0x180 >>>> ip_queue_xmit+0x36b/0x3b0 >>>> dst_output+0x0/0x10 usb_hcd_giveback_urb+0x2d/0x60 >>>> [usbcore] >>>> tcp_v4_send_check+0x82/0xd0 >>>> tcp_v4_send_check+0x82/0xd0 >>>> tcp_transmit_skb+0x5e4/0x610 >>>> __tcp_push_pending_frames+0x676/0x740 >>>> __alloc_skb+0x51/0x100 tcp_sendmsg+0x897/0x980 >>>> core_sys_select+0x1b9/0x2b0 inet_sendmsg+0x3d/0x50 >>>> do_sock_write+0x8f/0xa0 sock_aio_write+0x5f/0x70 >>>> do_sync_write+0xc3/0x100 >>>> autoremove_wake_function+0x0/0x40 >>>> vfs_write+0xa1/0x140 sys_write+0x43/0x70 >>>> 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: d80211: How does TX flow control work?
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() >>> ieee80211_master_start_xmit+0x105/0x430 [80211] >>> __ip_ct_refresh_acct+0x4d/0x60 >>> tcp_packet+0x941/0x970 qdisc_restart+0x92/0x100 >>> dev_queue_xmit+0xbd/0x1a0 >>> ieee80211_subif_start_xmit+0x468/0x480 [80211] >>> skb_clone+0x3a/0x1a0 nf_hook_slow+0x4d/0xc0 >>> dev_queue_xmit+0x115/0x1a0 ip_output+0x1c3/0x200 >>> ip_finish_output+0x0/0x180 ip_queue_xmit+0x36b/0x3b0 >>> dst_output+0x0/0x10 usb_hcd_giveback_urb+0x2d/0x60 >>> [usbcore] >>> tcp_v4_send_check+0x82/0xd0 >>> tcp_v4_send_check+0x82/0xd0 >>> tcp_transmit_skb+0x5e4/0x610 >>> __tcp_push_pending_frames+0x676/0x740 >>> __alloc_skb+0x51/0x100 tcp_sendmsg+0x897/0x980 >>> core_sys_select+0x1b9/0x2b0 inet_sendmsg+0x3d/0x50 >>> do_sock_write+0x8f/0xa0 sock_aio_write+0x5f/0x70 >>> do_sync_write+0xc3/0x100 >>> autoremove_wake_function+0x0/0x40 >>> vfs_write+0xa1/0x140 sys_write+0x43/0x70 >>> 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: [PATCH] d80211: Fix inconsistent sta_lock usage
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: [PATCH] d80211: Fix inconsistent sta_lock usage
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: d80211: How does TX flow control work?
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() >> ieee80211_master_start_xmit+0x105/0x430 [80211] >> __ip_ct_refresh_acct+0x4d/0x60 >> tcp_packet+0x941/0x970 qdisc_restart+0x92/0x100 >> dev_queue_xmit+0xbd/0x1a0 >> ieee80211_subif_start_xmit+0x468/0x480 [80211] >> skb_clone+0x3a/0x1a0 nf_hook_slow+0x4d/0xc0 >> dev_queue_xmit+0x115/0x1a0 ip_output+0x1c3/0x200 >> ip_finish_output+0x0/0x180 ip_queue_xmit+0x36b/0x3b0 >> dst_output+0x0/0x10 usb_hcd_giveback_urb+0x2d/0x60 >> [usbcore] >> tcp_v4_send_check+0x82/0xd0 >> tcp_v4_send_check+0x82/0xd0 >> tcp_transmit_skb+0x5e4/0x610 >> __tcp_push_pending_frames+0x676/0x740 >> __alloc_skb+0x51/0x100 tcp_sendmsg+0x897/0x980 >> core_sys_select+0x1b9/0x2b0 inet_sendmsg+0x3d/0x50 >> do_sock_write+0x8f/0xa0 sock_aio_write+0x5f/0x70 >> do_sync_write+0xc3/0x100 >> autoremove_wake_function+0x0/0x40 >> vfs_write+0xa1/0x140 sys_write+0x43/0x70 >> 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? > Yes. Would it make a difference /wrt to that warning when I switch it off? Jan signature.asc Description: OpenPGP digital signature
Re: d80211: How does TX flow control work?
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() ieee80211_master_start_xmit+0x105/0x430 [80211] __ip_ct_refresh_acct+0x4d/0x60 tcp_packet+0x941/0x970 qdisc_restart+0x92/0x100 dev_queue_xmit+0xbd/0x1a0 ieee80211_subif_start_xmit+0x468/0x480 [80211] skb_clone+0x3a/0x1a0 nf_hook_slow+0x4d/0xc0 dev_queue_xmit+0x115/0x1a0 ip_output+0x1c3/0x200 ip_finish_output+0x0/0x180 ip_queue_xmit+0x36b/0x3b0 dst_output+0x0/0x10 usb_hcd_giveback_urb+0x2d/0x60 [usbcore] tcp_v4_send_check+0x82/0xd0 tcp_v4_send_check+0x82/0xd0 tcp_transmit_skb+0x5e4/0x610 __tcp_push_pending_frames+0x676/0x740 __alloc_skb+0x51/0x100 tcp_sendmsg+0x897/0x980 core_sys_select+0x1b9/0x2b0 inet_sendmsg+0x3d/0x50 do_sock_write+0x8f/0xa0 sock_aio_write+0x5f/0x70 do_sync_write+0xc3/0x100 autoremove_wake_function+0x0/0x40 vfs_write+0xa1/0x140 sys_write+0x43/0x70 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
Re: [2.6 patch] the scheduled eepro100 removal
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
d80211: How does TX flow control work?
Hi, can someone explain how TX flow control in d80211 is supposed to work? I failed to understand the full design so far. 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, and can cause that up to *one* packet per queue is stored in ieee80211_local::pending_packets[queue]. 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. 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... Jan signature.asc Description: OpenPGP digital signature
[PATCH] d80211: Fix inconsistent sta_lock usage
Hacking a bit on rt2x00 to make it work in master and ad-hoc mode, lockdep popped up on some hostapd ioctls, pointing out remaining inconsistencies related to sta_lock: 1. sta_lock holders must always be protected against softirq 2. bss_tim_set/clear must not be called with sta_lock held, rather an unprotected variant 3. ieee80211_ioctl_remove_sta is not already holding the lock when calling sta_info_free As I was not sure if sta_info_remove_aid_ptr needs lock protection or not, I played safe and moved it always under the lock. Please correct me if this is overkill. Signed-off-by: Jan Kiszka <[EMAIL PROTECTED]> [Sorry, patch is against rt2x00 CVS. I'm lacking time and bandwidth to pull the d80211 git repos and rebase.] --- ieee80211/ieee80211_i.h | 24 ++-- ieee80211/ieee80211_ioctl.c |4 +++- ieee80211/sta_info.c|2 +- 3 files changed, 22 insertions(+), 8 deletions(-) Index: rt2x00/ieee80211/ieee80211_ioctl.c === --- rt2x00.orig/ieee80211/ieee80211_ioctl.c +++ rt2x00/ieee80211/ieee80211_ioctl.c @@ -286,7 +286,9 @@ static int ieee80211_ioctl_add_sta(struc if (sta->dev != dev) { /* Binding STA to a new interface, so remove all references to * the old BSS. */ + spin_lock_bh(&local->sta_lock); sta_info_remove_aid_ptr(sta); + spin_unlock_bh(&local->sta_lock); } /* TODO @@ -360,7 +362,7 @@ static int ieee80211_ioctl_remove_sta(st sta = sta_info_get(local, param->sta_addr); if (sta) { sta_info_put(sta); - sta_info_free(sta, 1); + sta_info_free(sta, 0); } return sta ? 0 : -ENOENT; Index: rt2x00/ieee80211/ieee80211_i.h === --- rt2x00.orig/ieee80211/ieee80211_i.h +++ rt2x00/ieee80211/ieee80211_i.h @@ -565,20 +565,32 @@ struct sta_attribute { ssize_t (*store)(struct sta_info *, const char *buf, size_t count); }; +static inline void __bss_tim_set(struct ieee80211_local *local, +struct ieee80211_if_ap *bss, int aid) +{ + bss->tim[(aid)/8] |= 1<<((aid) % 8); +} + static inline void bss_tim_set(struct ieee80211_local *local, struct ieee80211_if_ap *bss, int aid) { - spin_lock(&local->sta_lock); - bss->tim[(aid)/8] |= 1<<((aid) % 8); - spin_unlock(&local->sta_lock); + spin_lock_bh(&local->sta_lock); + __bss_tim_set(local, bss, aid); + spin_unlock_bh(&local->sta_lock); +} + +static inline void __bss_tim_clear(struct ieee80211_local *local, + struct ieee80211_if_ap *bss, int aid) +{ + bss->tim[(aid)/8] &= !(1<<((aid) % 8)); } 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)); - spin_unlock(&local->sta_lock); + spin_lock_bh(&local->sta_lock); + __bss_tim_clear(local, bss, aid); + spin_unlock_bh(&local->sta_lock); } /* ieee80211.c */ Index: rt2x00/ieee80211/sta_info.c === --- rt2x00.orig/ieee80211/sta_info.c +++ rt2x00/ieee80211/sta_info.c @@ -439,7 +439,7 @@ void sta_info_remove_aid_ptr(struct sta_ sdata->local->ops->set_tim(local_to_hw(sdata->local), sta->aid, 0); if (sdata->bss) - bss_tim_clear(sdata->local, sdata->bss, sta->aid); + __bss_tim_clear(sdata->local, sdata->bss, sta->aid); } signature.asc Description: OpenPGP digital signature
[PATCH] d80211: Reinit keys on mode change
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
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/ieee8
Re: d80211: ieee80211_hw handlers in atomic context
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: schedule+0x43/0xa84 > extract_buf+0x97/0xc8 > Sep 13 12:27:34 wz4a kernel: wait_for_completion+0x6a/0x9f > default_wake_function+0x0/0xc > Sep 13 12:27:34 wz4a kernel: usb_start_wait_urb+0x98/0xdc > [usbcore] timeout_kill+0x0/0x5 [usbcore] > Sep 13 12:27:34 wz4a kernel: usb_control_msg+0xc3/0xde [usbcore] > rt2x00_vendor_request+0x7c/0xa6 [rt73usb] > Sep 13 12:27:34 wz4a kernel: rt73usb_reset_tsf+0x30/0x59 > [rt73usb] ieee80211_sta_join_ibss+0x3a/0x572 [80211] > Sep 13 12:27:34 wz4a kernel: printk+0x14/0x18 > ieee80211_rx_bss_add+0x88/0x90 [80211] > Sep 13 12:27:34 wz4a kernel: ieee80211_sta_find_ibss+0x30e/0x366 > [80211] ieee80211_sta_timer+0x0/0x18f [80211] > Sep 13 12:27:34 wz4a kernel: ieee80211_sta_timer+0x7a/0x18f > [80211] ieee80211_sta_timer+0x0/0x18f [80211] > Sep 13 12:27:34 wz4a kernel: run_timer_softirq+0x10b/0x153 > __do_softirq+0x58/0xc2 > Sep 13 12:27:34 wz4a kernel: do_softirq+0x2e/0x32 > do_IRQ+0x1e/0x24 > Sep 13 12:27:34 wz4a kernel: common_interrupt+0x1a/0x20 > acpi_processor_idle+0x18a/0x39e [processor] > Sep 13 12:27:34 wz4a kernel: cpu_idle+0x8f/0xa8 > 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
Re: d80211: ieee80211_hw handlers in atomic context
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
Re: d80211: ieee80211_hw handlers in atomic context
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
d80211: ieee80211_hw handlers in atomic context
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 show_trace+0x12/0x14 dump_stack+0x1c/0x1e schedule+0x5f/0x652 wait_for_completion+0xb8/0x134 usb_start_wait_urb+0x89/0xcb [usbcore] usb_control_msg+0xb2/0xcc [usbcore] rt2x00_vendor_request+0x85/0xbb [rt2500usb] rt2500usb_config+0x5e/0x3d7 [rt2500usb] ieee80211_hw_config+0x2c/0x93 [80211] ieee80211_ioctl_siwfreq+0x132/0x141 [80211] ieee80211_sta_join_ibss+0xcc/0x5af [80211] ieee80211_sta_find_ibss+0x32a/0x374 [80211] ieee80211_sta_timer+0x81/0x1b4 [80211] run_timer_softirq+0x171/0x205 __do_softirq+0x41/0x90 do_softirq+0x37/0x4a irq_exit+0x2d/0x45 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: e100 fails, eepro100 works
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
e100 fails, eepro100 works
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