linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/6] bcma: Find names of non BCM cores
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  6:16   ` Arend van Spriel
  2012-05-05  4:56 ` [PATCH v3 2/6] bcma: Move initialization of SPROM to prevent overwrite Nathan Hintz
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

bcma_device_name only provides names for Broadcom cores.  Modify logic to
provide names for MIPS and ARM cores as well.

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 drivers/bcma/scan.c |   54 +++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
index f94cccc..e19e987 100644
--- a/drivers/bcma/scan.c
+++ b/drivers/bcma/scan.c
@@ -19,7 +19,14 @@ struct bcma_device_id_name {
 	u16 id;
 	const char *name;
 };
-struct bcma_device_id_name bcma_device_names[] = {
+
+static const struct bcma_device_id_name bcma_arm_device_names[] = {
+	{ BCMA_CORE_ARM_1176, "ARM 1176" },
+	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
+	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
+};
+
+static const struct bcma_device_id_name bcma_bcm_device_names[] = {
 	{ BCMA_CORE_OOB_ROUTER, "OOB Router" },
 	{ BCMA_CORE_INVALID, "Invalid" },
 	{ BCMA_CORE_CHIPCOMMON, "ChipCommon" },
@@ -27,7 +34,6 @@ struct bcma_device_id_name bcma_device_names[] = {
 	{ BCMA_CORE_SRAM, "SRAM" },
 	{ BCMA_CORE_SDRAM, "SDRAM" },
 	{ BCMA_CORE_PCI, "PCI" },
-	{ BCMA_CORE_MIPS, "MIPS" },
 	{ BCMA_CORE_ETHERNET, "Fast Ethernet" },
 	{ BCMA_CORE_V90, "V90" },
 	{ BCMA_CORE_USB11_HOSTDEV, "USB 1.1 Hostdev" },
@@ -44,7 +50,6 @@ struct bcma_device_id_name bcma_device_names[] = {
 	{ BCMA_CORE_PHY_A, "PHY A" },
 	{ BCMA_CORE_PHY_B, "PHY B" },
 	{ BCMA_CORE_PHY_G, "PHY G" },
-	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
 	{ BCMA_CORE_USB11_HOST, "USB 1.1 Host" },
 	{ BCMA_CORE_USB11_DEV, "USB 1.1 Device" },
 	{ BCMA_CORE_USB20_HOST, "USB 2.0 Host" },
@@ -58,15 +63,11 @@ struct bcma_device_id_name bcma_device_names[] = {
 	{ BCMA_CORE_PHY_N, "PHY N" },
 	{ BCMA_CORE_SRAM_CTL, "SRAM Controller" },
 	{ BCMA_CORE_MINI_MACPHY, "Mini MACPHY" },
-	{ BCMA_CORE_ARM_1176, "ARM 1176" },
-	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
 	{ BCMA_CORE_PHY_LP, "PHY LP" },
 	{ BCMA_CORE_PMU, "PMU" },
 	{ BCMA_CORE_PHY_SSN, "PHY SSN" },
 	{ BCMA_CORE_SDIO_DEV, "SDIO Device" },
-	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
 	{ BCMA_CORE_PHY_HT, "PHY HT" },
-	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
 	{ BCMA_CORE_MAC_GBIT, "GBit MAC" },
 	{ BCMA_CORE_DDR12_MEM_CTL, "DDR1/DDR2 Memory Controller" },
 	{ BCMA_CORE_PCIE_RC, "PCIe Root Complex" },
@@ -79,16 +80,41 @@ struct bcma_device_id_name bcma_device_names[] = {
 	{ BCMA_CORE_SHIM, "SHIM" },
 	{ BCMA_CORE_DEFAULT, "Default" },
 };
-const char *bcma_device_name(struct bcma_device_id *id)
+
+static const struct bcma_device_id_name bcma_mips_device_names[] = {
+	{ BCMA_CORE_MIPS, "MIPS" },
+	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
+	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
+};
+
+static const char *bcma_device_name(const struct bcma_device_id *id)
 {
-	int i;
+	const struct bcma_device_id_name *names;
+	int size, i;
+
+	/* search manufacturer specific names */
+	switch (id->manuf) {
+	case BCMA_MANUF_ARM:
+		names = bcma_arm_device_names;
+		size = ARRAY_SIZE(bcma_arm_device_names);
+		break;
+	case BCMA_MANUF_BCM:
+		names = bcma_bcm_device_names;
+		size = ARRAY_SIZE(bcma_bcm_device_names);
+		break;
+	case BCMA_MANUF_MIPS:
+		names = bcma_mips_device_names;
+		size = ARRAY_SIZE(bcma_mips_device_names);
+		break;
+	default:
+		return "UNKNOWN";
+	}
 
-	if (id->manuf == BCMA_MANUF_BCM) {
-		for (i = 0; i < ARRAY_SIZE(bcma_device_names); i++) {
-			if (bcma_device_names[i].id == id->id)
-				return bcma_device_names[i].name;
-		}
+	for (i = 0; i < size; i++) {
+		if (names[i].id == id->id)
+			return names[i].name;
 	}
+
 	return "UNKNOWN";
 }
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/6] bcma: Move initialization of SPROM to prevent overwrite
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
  2012-05-05  4:56 ` [PATCH v3 1/6] bcma: Find names of non BCM cores Nathan Hintz
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 3/6] bcma: Account for variable PCI memory base/size Nathan Hintz
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

The first thing bcm47xx_fill_sprom does is initialize (zero fill) the SPROM.  For
BCMA SOC, this wipes out any values previously read by bcm47xx_fill_sprom_ethernet
(see arch/mips/bcm47xx/setup.c - bcm47xx_get_sprom_bcma).  Move the initialization
of SPROM so it is called prior to filling in any values.

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 arch/mips/bcm47xx/setup.c |    4 ++++
 arch/mips/bcm47xx/sprom.c |    2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
index 19780aa..67d3d52 100644
--- a/arch/mips/bcm47xx/setup.c
+++ b/arch/mips/bcm47xx/setup.c
@@ -90,6 +90,7 @@ static int bcm47xx_get_sprom_ssb(struct ssb_bus *bus, struct ssb_sprom *out)
 	char prefix[10];
 
 	if (bus->bustype == SSB_BUSTYPE_PCI) {
+		memset(out, 0, sizeof(struct ssb_sprom));
 		snprintf(prefix, sizeof(prefix), "pci/%u/%u/",
 			 bus->host_pci->bus->number + 1,
 			 PCI_SLOT(bus->host_pci->devfn));
@@ -118,6 +119,7 @@ static int bcm47xx_get_invariants(struct ssb_bus *bus,
 	if (nvram_getenv("boardrev", buf, sizeof(buf)) >= 0)
 		iv->boardinfo.rev = (u16)simple_strtoul(buf, NULL, 0);
 
+	memset(&iv->sprom, 0, sizeof(struct ssb_sprom));
 	bcm47xx_fill_sprom(&iv->sprom, NULL);
 
 	if (nvram_getenv("cardbus", buf, sizeof(buf)) >= 0)
@@ -166,12 +168,14 @@ static int bcm47xx_get_sprom_bcma(struct bcma_bus *bus, struct ssb_sprom *out)
 
 	switch (bus->hosttype) {
 	case BCMA_HOSTTYPE_PCI:
+		memset(out, 0, sizeof(struct ssb_sprom));
 		snprintf(prefix, sizeof(prefix), "pci/%u/%u/",
 			 bus->host_pci->bus->number + 1,
 			 PCI_SLOT(bus->host_pci->devfn));
 		bcm47xx_fill_sprom(out, prefix);
 		return 0;
 	case BCMA_HOSTTYPE_SOC:
+		memset(out, 0, sizeof(struct ssb_sprom));
 		bcm47xx_fill_sprom_ethernet(out, NULL);
 		core = bcma_find_core(bus, BCMA_CORE_80211);
 		if (core) {
diff --git a/arch/mips/bcm47xx/sprom.c b/arch/mips/bcm47xx/sprom.c
index 5c8dcd2..bc19234 100644
--- a/arch/mips/bcm47xx/sprom.c
+++ b/arch/mips/bcm47xx/sprom.c
@@ -555,8 +555,6 @@ void bcm47xx_fill_sprom_ethernet(struct ssb_sprom *sprom, const char *prefix)
 
 void bcm47xx_fill_sprom(struct ssb_sprom *sprom, const char *prefix)
 {
-	memset(sprom, 0, sizeof(struct ssb_sprom));
-
 	bcm47xx_fill_sprom_ethernet(sprom, prefix);
 
 	nvram_read_u8(prefix, NULL, "sromrev", &sprom->revision, 0);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/6] bcma: Account for variable PCI memory base/size
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
  2012-05-05  4:56 ` [PATCH v3 1/6] bcma: Find names of non BCM cores Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 2/6] bcma: Move initialization of SPROM to prevent overwrite Nathan Hintz
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 4/6] bcma: reads/writes are always 4 bytes, so always map 4 bytes Nathan Hintz
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

PCI Memory Resource start address and size are variable, dependent on
the H/W configuration.  Modify the computation of io_map_base to use the
computed values.

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 drivers/bcma/driver_pci_host.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bcma/driver_pci_host.c b/drivers/bcma/driver_pci_host.c
index d2097a1..8407b25 100644
--- a/drivers/bcma/driver_pci_host.c
+++ b/drivers/bcma/driver_pci_host.c
@@ -491,8 +491,8 @@ void __devinit bcma_core_pci_hostmode_init(struct bcma_drv_pci *pc)
 	/* Ok, ready to run, register it to the system.
 	 * The following needs change, if we want to port hostmode
 	 * to non-MIPS platform. */
-	io_map_base = (unsigned long)ioremap_nocache(BCMA_SOC_PCI_MEM,
-						     0x04000000);
+	io_map_base = (unsigned long)ioremap_nocache(pc_host->mem_resource.start,
+						     resource_size(&pc_host->mem_resource));
 	pc_host->pci_controller.io_map_base = io_map_base;
 	set_io_port_base(pc_host->pci_controller.io_map_base);
 	/* Give some time to the PCI controller to configure itself with the new
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/6] bcma: reads/writes are always 4 bytes, so always map 4 bytes
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
                   ` (2 preceding siblings ...)
  2012-05-05  4:56 ` [PATCH v3 3/6] bcma: Account for variable PCI memory base/size Nathan Hintz
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 5/6] bcma: Add __devexit to bcma_host_pci_remove Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write Nathan Hintz
  5 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

Modify ioremap_nocache calls to reflect the number of bytes read/written.

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 drivers/bcma/driver_pci_host.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bcma/driver_pci_host.c b/drivers/bcma/driver_pci_host.c
index 8407b25..b9a86ed 100644
--- a/drivers/bcma/driver_pci_host.c
+++ b/drivers/bcma/driver_pci_host.c
@@ -119,7 +119,7 @@ static int bcma_extpci_read_config(struct bcma_drv_pci *pc, unsigned int dev,
 		if (unlikely(!addr))
 			goto out;
 		err = -ENOMEM;
-		mmio = ioremap_nocache(addr, len);
+		mmio = ioremap_nocache(addr, sizeof(val));
 		if (!mmio)
 			goto out;
 
@@ -171,7 +171,7 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev,
 			addr = pc->core->addr + BCMA_CORE_PCI_PCICFG0;
 			addr |= (func << 8);
 			addr |= (off & 0xfc);
-			mmio = ioremap_nocache(addr, len);
+			mmio = ioremap_nocache(addr, sizeof(val));
 			if (!mmio)
 				goto out;
 		}
@@ -180,7 +180,7 @@ static int bcma_extpci_write_config(struct bcma_drv_pci *pc, unsigned int dev,
 		if (unlikely(!addr))
 			goto out;
 		err = -ENOMEM;
-		mmio = ioremap_nocache(addr, len);
+		mmio = ioremap_nocache(addr, sizeof(val));
 		if (!mmio)
 			goto out;
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 5/6] bcma: Add __devexit to bcma_host_pci_remove
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
                   ` (3 preceding siblings ...)
  2012-05-05  4:56 ` [PATCH v3 4/6] bcma: reads/writes are always 4 bytes, so always map 4 bytes Nathan Hintz
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  4:56 ` [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write Nathan Hintz
  5 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

Add missing __devexit attribute to bcma_host_pci_remove.

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 drivers/bcma/host_pci.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bcma/host_pci.c b/drivers/bcma/host_pci.c
index e3928d6..915ab2f 100644
--- a/drivers/bcma/host_pci.c
+++ b/drivers/bcma/host_pci.c
@@ -222,7 +222,7 @@ err_kfree_bus:
 	return err;
 }
 
-static void bcma_host_pci_remove(struct pci_dev *dev)
+static void __devexit bcma_host_pci_remove(struct pci_dev *dev)
 {
 	struct bcma_bus *bus = pci_get_drvdata(dev);
 
@@ -277,7 +277,7 @@ static struct pci_driver bcma_pci_bridge_driver = {
 	.name = "bcma-pci-bridge",
 	.id_table = bcma_pci_bridge_tbl,
 	.probe = bcma_host_pci_probe,
-	.remove = bcma_host_pci_remove,
+	.remove = __devexit_p(bcma_host_pci_remove),
 	.driver.pm = BCMA_PM_OPS,
 };
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
       [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
                   ` (4 preceding siblings ...)
  2012-05-05  4:56 ` [PATCH v3 5/6] bcma: Add __devexit to bcma_host_pci_remove Nathan Hintz
@ 2012-05-05  4:56 ` Nathan Hintz
  2012-05-05  6:03   ` Arend van Spriel
  5 siblings, 1 reply; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  4:56 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, hauke, Nathan Hintz

Adds a missing read to flush the previous write (per the Broadcom SDK).

Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
---
 drivers/bcma/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
index 893f6e0..c4e6deb 100644
--- a/drivers/bcma/core.c
+++ b/drivers/bcma/core.c
@@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
 	udelay(10);
 
 	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
+	bcma_aread32(core, BCMA_RESET_CTL);
 	udelay(1);
 }
 EXPORT_SYMBOL_GPL(bcma_core_disable);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-05  4:56 ` [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write Nathan Hintz
@ 2012-05-05  6:03   ` Arend van Spriel
  2012-05-05  7:50     ` Nathan Hintz
  0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2012-05-05  6:03 UTC (permalink / raw)
  To: Nathan Hintz; +Cc: linville, linux-wireless, hauke

On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> Adds a missing read to flush the previous write (per the Broadcom SDK).
> 
> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> ---
>  drivers/bcma/core.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
> index 893f6e0..c4e6deb 100644
> --- a/drivers/bcma/core.c
> +++ b/drivers/bcma/core.c
> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
>  	udelay(10);
>  
>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
> +	bcma_aread32(core, BCMA_RESET_CTL);
>  	udelay(1);
>  }
>  EXPORT_SYMBOL_GPL(bcma_core_disable);

Hi Nathan,

The read after write is only needed on (certain) SoCs. As bcma is not
being used just for these SoCs I suggest to make the flush conditional.
In brcmsmac we introduced "flushed write" function, which does the read
only for those SoCs.

Gr. AvS


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/6] bcma: Find names of non BCM cores
  2012-05-05  4:56 ` [PATCH v3 1/6] bcma: Find names of non BCM cores Nathan Hintz
@ 2012-05-05  6:16   ` Arend van Spriel
  2012-05-05  7:23     ` Nathan Hintz
  0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2012-05-05  6:16 UTC (permalink / raw)
  To: Nathan Hintz; +Cc: linville, linux-wireless, hauke

On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> bcma_device_name only provides names for Broadcom cores.  Modify logic to
> provide names for MIPS and ARM cores as well.

The description does not seem accurate. In my opinion the aim is to make
the device name lookup a little bit more efficient by moving from one
lookup table to a lookup table per vendor.

> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> ---
>  drivers/bcma/scan.c |   54 +++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
> index f94cccc..e19e987 100644
> --- a/drivers/bcma/scan.c
> +++ b/drivers/bcma/scan.c
> @@ -19,7 +19,14 @@ struct bcma_device_id_name {
>  	u16 id;
>  	const char *name;
>  };
> -struct bcma_device_id_name bcma_device_names[] = {
> +
> +static const struct bcma_device_id_name bcma_arm_device_names[] = {
> +	{ BCMA_CORE_ARM_1176, "ARM 1176" },
> +	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
> +	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
> +};
> +
> +static const struct bcma_device_id_name bcma_bcm_device_names[] = {
>  	{ BCMA_CORE_OOB_ROUTER, "OOB Router" },
>  	{ BCMA_CORE_INVALID, "Invalid" },
>  	{ BCMA_CORE_CHIPCOMMON, "ChipCommon" },
> @@ -27,7 +34,6 @@ struct bcma_device_id_name bcma_device_names[] = {
>  	{ BCMA_CORE_SRAM, "SRAM" },
>  	{ BCMA_CORE_SDRAM, "SDRAM" },
>  	{ BCMA_CORE_PCI, "PCI" },
> -	{ BCMA_CORE_MIPS, "MIPS" },
>  	{ BCMA_CORE_ETHERNET, "Fast Ethernet" },
>  	{ BCMA_CORE_V90, "V90" },
>  	{ BCMA_CORE_USB11_HOSTDEV, "USB 1.1 Hostdev" },
> @@ -44,7 +50,6 @@ struct bcma_device_id_name bcma_device_names[] = {
>  	{ BCMA_CORE_PHY_A, "PHY A" },
>  	{ BCMA_CORE_PHY_B, "PHY B" },
>  	{ BCMA_CORE_PHY_G, "PHY G" },
> -	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
>  	{ BCMA_CORE_USB11_HOST, "USB 1.1 Host" },
>  	{ BCMA_CORE_USB11_DEV, "USB 1.1 Device" },
>  	{ BCMA_CORE_USB20_HOST, "USB 2.0 Host" },
> @@ -58,15 +63,11 @@ struct bcma_device_id_name bcma_device_names[] = {
>  	{ BCMA_CORE_PHY_N, "PHY N" },
>  	{ BCMA_CORE_SRAM_CTL, "SRAM Controller" },
>  	{ BCMA_CORE_MINI_MACPHY, "Mini MACPHY" },
> -	{ BCMA_CORE_ARM_1176, "ARM 1176" },
> -	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
>  	{ BCMA_CORE_PHY_LP, "PHY LP" },
>  	{ BCMA_CORE_PMU, "PMU" },
>  	{ BCMA_CORE_PHY_SSN, "PHY SSN" },
>  	{ BCMA_CORE_SDIO_DEV, "SDIO Device" },
> -	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
>  	{ BCMA_CORE_PHY_HT, "PHY HT" },
> -	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
>  	{ BCMA_CORE_MAC_GBIT, "GBit MAC" },
>  	{ BCMA_CORE_DDR12_MEM_CTL, "DDR1/DDR2 Memory Controller" },
>  	{ BCMA_CORE_PCIE_RC, "PCIe Root Complex" },
> @@ -79,16 +80,41 @@ struct bcma_device_id_name bcma_device_names[] = {
>  	{ BCMA_CORE_SHIM, "SHIM" },
>  	{ BCMA_CORE_DEFAULT, "Default" },
>  };
> -const char *bcma_device_name(struct bcma_device_id *id)
> +
> +static const struct bcma_device_id_name bcma_mips_device_names[] = {
> +	{ BCMA_CORE_MIPS, "MIPS" },
> +	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
> +	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
> +};
> +
> +static const char *bcma_device_name(const struct bcma_device_id *id)
>  {
> -	int i;
> +	const struct bcma_device_id_name *names;
> +	int size, i;
> +
> +	/* search manufacturer specific names */
> +	switch (id->manuf) {
> +	case BCMA_MANUF_ARM:
> +		names = bcma_arm_device_names;
> +		size = ARRAY_SIZE(bcma_arm_device_names);
> +		break;
> +	case BCMA_MANUF_BCM:
> +		names = bcma_bcm_device_names;
> +		size = ARRAY_SIZE(bcma_bcm_device_names);
> +		break;
> +	case BCMA_MANUF_MIPS:
> +		names = bcma_mips_device_names;
> +		size = ARRAY_SIZE(bcma_mips_device_names);
> +		break;
> +	default:
> +		return "UNKNOWN";
> +	}
>  
> -	if (id->manuf == BCMA_MANUF_BCM) {
> -		for (i = 0; i < ARRAY_SIZE(bcma_device_names); i++) {
> -			if (bcma_device_names[i].id == id->id)
> -				return bcma_device_names[i].name;
> -		}
> +	for (i = 0; i < size; i++) {
> +		if (names[i].id == id->id)
> +			return names[i].name;
>  	}
> +
>  	return "UNKNOWN";
>  }
>  



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/6] bcma: Find names of non BCM cores
  2012-05-05  6:16   ` Arend van Spriel
@ 2012-05-05  7:23     ` Nathan Hintz
  0 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  7:23 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linville, linux-wireless, hauke

On Sat, 2012-05-05 at 08:16 +0200, Arend van Spriel wrote:
> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> > bcma_device_name only provides names for Broadcom cores.  Modify logic to
> > provide names for MIPS and ARM cores as well.
> 
> The description does not seem accurate. In my opinion the aim is to make
> the device name lookup a little bit more efficient by moving from one
> lookup table to a lookup table per vendor.
> 
> > Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> > ---
> >  drivers/bcma/scan.c |   54 +++++++++++++++++++++++++++++++++++++-------------
> >  1 files changed, 40 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
> > index f94cccc..e19e987 100644
> > --- a/drivers/bcma/scan.c
> > +++ b/drivers/bcma/scan.c
> > @@ -19,7 +19,14 @@ struct bcma_device_id_name {
> >  	u16 id;
> >  	const char *name;
> >  };
> > -struct bcma_device_id_name bcma_device_names[] = {
> > +
> > +static const struct bcma_device_id_name bcma_arm_device_names[] = {
> > +	{ BCMA_CORE_ARM_1176, "ARM 1176" },
> > +	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
> > +	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
> > +};
> > +
> > +static const struct bcma_device_id_name bcma_bcm_device_names[] = {
> >  	{ BCMA_CORE_OOB_ROUTER, "OOB Router" },
> >  	{ BCMA_CORE_INVALID, "Invalid" },
> >  	{ BCMA_CORE_CHIPCOMMON, "ChipCommon" },
> > @@ -27,7 +34,6 @@ struct bcma_device_id_name bcma_device_names[] = {
> >  	{ BCMA_CORE_SRAM, "SRAM" },
> >  	{ BCMA_CORE_SDRAM, "SDRAM" },
> >  	{ BCMA_CORE_PCI, "PCI" },
> > -	{ BCMA_CORE_MIPS, "MIPS" },
> >  	{ BCMA_CORE_ETHERNET, "Fast Ethernet" },
> >  	{ BCMA_CORE_V90, "V90" },
> >  	{ BCMA_CORE_USB11_HOSTDEV, "USB 1.1 Hostdev" },
> > @@ -44,7 +50,6 @@ struct bcma_device_id_name bcma_device_names[] = {
> >  	{ BCMA_CORE_PHY_A, "PHY A" },
> >  	{ BCMA_CORE_PHY_B, "PHY B" },
> >  	{ BCMA_CORE_PHY_G, "PHY G" },
> > -	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
> >  	{ BCMA_CORE_USB11_HOST, "USB 1.1 Host" },
> >  	{ BCMA_CORE_USB11_DEV, "USB 1.1 Device" },
> >  	{ BCMA_CORE_USB20_HOST, "USB 2.0 Host" },
> > @@ -58,15 +63,11 @@ struct bcma_device_id_name bcma_device_names[] = {
> >  	{ BCMA_CORE_PHY_N, "PHY N" },
> >  	{ BCMA_CORE_SRAM_CTL, "SRAM Controller" },
> >  	{ BCMA_CORE_MINI_MACPHY, "Mini MACPHY" },
> > -	{ BCMA_CORE_ARM_1176, "ARM 1176" },
> > -	{ BCMA_CORE_ARM_7TDMI, "ARM 7TDMI" },
> >  	{ BCMA_CORE_PHY_LP, "PHY LP" },
> >  	{ BCMA_CORE_PMU, "PMU" },
> >  	{ BCMA_CORE_PHY_SSN, "PHY SSN" },
> >  	{ BCMA_CORE_SDIO_DEV, "SDIO Device" },
> > -	{ BCMA_CORE_ARM_CM3, "ARM CM3" },
> >  	{ BCMA_CORE_PHY_HT, "PHY HT" },
> > -	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
> >  	{ BCMA_CORE_MAC_GBIT, "GBit MAC" },
> >  	{ BCMA_CORE_DDR12_MEM_CTL, "DDR1/DDR2 Memory Controller" },
> >  	{ BCMA_CORE_PCIE_RC, "PCIe Root Complex" },
> > @@ -79,16 +80,41 @@ struct bcma_device_id_name bcma_device_names[] = {
> >  	{ BCMA_CORE_SHIM, "SHIM" },
> >  	{ BCMA_CORE_DEFAULT, "Default" },
> >  };
> > -const char *bcma_device_name(struct bcma_device_id *id)
> > +
> > +static const struct bcma_device_id_name bcma_mips_device_names[] = {
> > +	{ BCMA_CORE_MIPS, "MIPS" },
> > +	{ BCMA_CORE_MIPS_3302, "MIPS 3302" },
> > +	{ BCMA_CORE_MIPS_74K, "MIPS 74K" },
> > +};
> > +
> > +static const char *bcma_device_name(const struct bcma_device_id *id)
> >  {
> > -	int i;
> > +	const struct bcma_device_id_name *names;
> > +	int size, i;
> > +
> > +	/* search manufacturer specific names */
> > +	switch (id->manuf) {
> > +	case BCMA_MANUF_ARM:
> > +		names = bcma_arm_device_names;
> > +		size = ARRAY_SIZE(bcma_arm_device_names);
> > +		break;
> > +	case BCMA_MANUF_BCM:
> > +		names = bcma_bcm_device_names;
> > +		size = ARRAY_SIZE(bcma_bcm_device_names);
> > +		break;
> > +	case BCMA_MANUF_MIPS:
> > +		names = bcma_mips_device_names;
> > +		size = ARRAY_SIZE(bcma_mips_device_names);
> > +		break;
> > +	default:
> > +		return "UNKNOWN";
> > +	}
> >  
> > -	if (id->manuf == BCMA_MANUF_BCM) {
> > -		for (i = 0; i < ARRAY_SIZE(bcma_device_names); i++) {
> > -			if (bcma_device_names[i].id == id->id)
> > -				return bcma_device_names[i].name;
> > -		}
> > +	for (i = 0; i < size; i++) {
> > +		if (names[i].id == id->id)
> > +			return names[i].name;
> >  	}
> > +
> >  	return "UNKNOWN";
> >  }
> >  
> 
> 
> 
While it is true the change was structured with efficiency in mind; the
goal was to enable printing for non-BCM cores.  In the old code, the
statement "if (id->manuf == BCMA_MANUF_BCM) {" prevented cores from
other vendors from ever being recognized.

Nathan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-05  6:03   ` Arend van Spriel
@ 2012-05-05  7:50     ` Nathan Hintz
  2012-05-05 12:41       ` Hauke Mehrtens
  0 siblings, 1 reply; 15+ messages in thread
From: Nathan Hintz @ 2012-05-05  7:50 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linville, linux-wireless, hauke

On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> > Adds a missing read to flush the previous write (per the Broadcom SDK).
> > 
> > Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> > ---
> >  drivers/bcma/core.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
> > index 893f6e0..c4e6deb 100644
> > --- a/drivers/bcma/core.c
> > +++ b/drivers/bcma/core.c
> > @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
> >  	udelay(10);
> >  
> >  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
> > +	bcma_aread32(core, BCMA_RESET_CTL);
> >  	udelay(1);
> >  }
> >  EXPORT_SYMBOL_GPL(bcma_core_disable);
> 
> Hi Nathan,
> 
> The read after write is only needed on (certain) SoCs. As bcma is not
> being used just for these SoCs I suggest to make the flush conditional.
> In brcmsmac we introduced "flushed write" function, which does the read
> only for those SoCs.
> 
> Gr. AvS
> 
> 
There are several other occurrences in core.c that are the same
implementation as what was added in this patch.  Is there a reason
why the existing code isn't the way you have suggested already?
Would it be better to submit your suggestion as a separate patch, or
just roll it all into this one?

Nathan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-05  7:50     ` Nathan Hintz
@ 2012-05-05 12:41       ` Hauke Mehrtens
  2012-05-05 20:40         ` Arend van Spriel
  0 siblings, 1 reply; 15+ messages in thread
From: Hauke Mehrtens @ 2012-05-05 12:41 UTC (permalink / raw)
  To: Nathan Hintz; +Cc: Arend van Spriel, linville, linux-wireless

On 05/05/2012 09:50 AM, Nathan Hintz wrote:
> On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
>> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
>>> Adds a missing read to flush the previous write (per the Broadcom SDK).
>>>
>>> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
>>> ---
>>>  drivers/bcma/core.c |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
>>> index 893f6e0..c4e6deb 100644
>>> --- a/drivers/bcma/core.c
>>> +++ b/drivers/bcma/core.c
>>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
>>>  	udelay(10);
>>>  
>>>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
>>> +	bcma_aread32(core, BCMA_RESET_CTL);
>>>  	udelay(1);
>>>  }
>>>  EXPORT_SYMBOL_GPL(bcma_core_disable);
>>
>> Hi Nathan,
>>
>> The read after write is only needed on (certain) SoCs. As bcma is not
>> being used just for these SoCs I suggest to make the flush conditional.
>> In brcmsmac we introduced "flushed write" function, which does the read
>> only for those SoCs.
>>
>> Gr. AvS
>>
>>
> There are several other occurrences in core.c that are the same
> implementation as what was added in this patch.  Is there a reason
> why the existing code isn't the way you have suggested already?
> Would it be better to submit your suggestion as a separate patch, or
> just roll it all into this one?
> 
> Nathan

In some older versions of the Broadcom SDK (e.g. version with wl
5.10.147.0 and the version brcmsmac seams to be based on) this read is
not included in ai_core_disable(), but in more recent versions (e.g.
version including wl 5.100.138.9) this read is included unconditionally.

@Arend For what SoCs should we read after this write?
When you are speaking of (certain) SoCs are you talking about this code
and comment from brcmsmac?

#ifdef CONFIG_BCM47XX
/*
 * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder
 * transactions. As a fix, a read after write is performed on certain places
 * in the code. Older chips and the newer 5357 family don't require this
fix.
 */
#define bcma_wflush16(c, o, v) \
	({ bcma_write16(c, o, v); (void)bcma_read16(c, o); })


Hauke

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-05 12:41       ` Hauke Mehrtens
@ 2012-05-05 20:40         ` Arend van Spriel
  2012-05-06 18:07           ` Nathan Hintz
  0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2012-05-05 20:40 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: Nathan Hintz, linville, linux-wireless

On 05/05/2012 02:41 PM, Hauke Mehrtens wrote:
> On 05/05/2012 09:50 AM, Nathan Hintz wrote:
>> On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
>>> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
>>>> Adds a missing read to flush the previous write (per the Broadcom SDK).
>>>>
>>>> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
>>>> ---
>>>>  drivers/bcma/core.c |    1 +
>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
>>>> index 893f6e0..c4e6deb 100644
>>>> --- a/drivers/bcma/core.c
>>>> +++ b/drivers/bcma/core.c
>>>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
>>>>  	udelay(10);
>>>>  
>>>>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
>>>> +	bcma_aread32(core, BCMA_RESET_CTL);
>>>>  	udelay(1);
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(bcma_core_disable);
>>>
>>> Hi Nathan,
>>>
>>> The read after write is only needed on (certain) SoCs. As bcma is not
>>> being used just for these SoCs I suggest to make the flush conditional.
>>> In brcmsmac we introduced "flushed write" function, which does the read
>>> only for those SoCs.
>>>
>>> Gr. AvS
>>>
>>>
>> There are several other occurrences in core.c that are the same
>> implementation as what was added in this patch.  Is there a reason
>> why the existing code isn't the way you have suggested already?
>> Would it be better to submit your suggestion as a separate patch, or
>> just roll it all into this one?
>>
>> Nathan
> 
> In some older versions of the Broadcom SDK (e.g. version with wl
> 5.10.147.0 and the version brcmsmac seams to be based on) this read is
> not included in ai_core_disable(), but in more recent versions (e.g.
> version including wl 5.100.138.9) this read is included unconditionally.
> 
> @Arend For what SoCs should we read after this write?
> When you are speaking of (certain) SoCs are you talking about this code
> and comment from brcmsmac?
> 
> #ifdef CONFIG_BCM47XX
> /*
>  * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder
>  * transactions. As a fix, a read after write is performed on certain places
>  * in the code. Older chips and the newer 5357 family don't require this
> fix.
>  */
> #define bcma_wflush16(c, o, v) \
> 	({ bcma_write16(c, o, v); (void)bcma_read16(c, o); })
> 
> 
> Hauke
> 

Yes, Those are indeed the SoCs I meant. I was being to lazy to look it
up, so thanks for making the effort ;-)

Gr. AvS


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-05 20:40         ` Arend van Spriel
@ 2012-05-06 18:07           ` Nathan Hintz
  2012-05-06 20:48             ` Arend van Spriel
  0 siblings, 1 reply; 15+ messages in thread
From: Nathan Hintz @ 2012-05-06 18:07 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: Hauke Mehrtens, linville, linux-wireless

On Sat, 2012-05-05 at 22:40 +0200, Arend van Spriel wrote:
> On 05/05/2012 02:41 PM, Hauke Mehrtens wrote:
> > On 05/05/2012 09:50 AM, Nathan Hintz wrote:
> >> On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
> >>> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> >>>> Adds a missing read to flush the previous write (per the Broadcom SDK).
> >>>>
> >>>> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> >>>> ---
> >>>>  drivers/bcma/core.c |    1 +
> >>>>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
> >>>> index 893f6e0..c4e6deb 100644
> >>>> --- a/drivers/bcma/core.c
> >>>> +++ b/drivers/bcma/core.c
> >>>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
> >>>>  	udelay(10);
> >>>>  
> >>>>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
> >>>> +	bcma_aread32(core, BCMA_RESET_CTL);
> >>>>  	udelay(1);
> >>>>  }
> >>>>  EXPORT_SYMBOL_GPL(bcma_core_disable);
> >>>
> >>> Hi Nathan,
> >>>
> >>> The read after write is only needed on (certain) SoCs. As bcma is not
> >>> being used just for these SoCs I suggest to make the flush conditional.
> >>> In brcmsmac we introduced "flushed write" function, which does the read
> >>> only for those SoCs.
> >>>
> >>> Gr. AvS
> >>>
> >>>
> >> There are several other occurrences in core.c that are the same
> >> implementation as what was added in this patch.  Is there a reason
> >> why the existing code isn't the way you have suggested already?
> >> Would it be better to submit your suggestion as a separate patch, or
> >> just roll it all into this one?
> >>
> >> Nathan
> > 
> > In some older versions of the Broadcom SDK (e.g. version with wl
> > 5.10.147.0 and the version brcmsmac seams to be based on) this read is
> > not included in ai_core_disable(), but in more recent versions (e.g.
> > version including wl 5.100.138.9) this read is included unconditionally.
> > 
> > @Arend For what SoCs should we read after this write?
> > When you are speaking of (certain) SoCs are you talking about this code
> > and comment from brcmsmac?
> > 
> > #ifdef CONFIG_BCM47XX
> > /*
> >  * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder
> >  * transactions. As a fix, a read after write is performed on certain places
> >  * in the code. Older chips and the newer 5357 family don't require this
> > fix.
> >  */
> > #define bcma_wflush16(c, o, v) \
> > 	({ bcma_write16(c, o, v); (void)bcma_read16(c, o); })
> > 
> > 
> > Hauke
> > 
> 
> Yes, Those are indeed the SoCs I meant. I was being to lazy to look it
> up, so thanks for making the effort ;-)
> 
> Gr. AvS
> 
> 
Is there a reason why a similar 'flush write' function was not applied
to all of the writes to 'objaddr' in 'brcmsmac/main.c', or are these
flushes not '4716/4717/4718/4706' specific?  For example:

static void brcms_b_set_cwmin(struct brcms_hardware *wlc_hw, u16 newmin)
{
	wlc_hw->band->CWmin = newmin;

	bcma_write32(wlc_hw->d11core, D11REGOFFS(objaddr),
		     OBJADDR_SCR_SEL | S_DOT11_CWMIN);
	(void)bcma_read32(wlc_hw->d11core, D11REGOFFS(objaddr));
	bcma_write32(wlc_hw->d11core, D11REGOFFS(objdata), newmin);
}

Nathan


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-06 18:07           ` Nathan Hintz
@ 2012-05-06 20:48             ` Arend van Spriel
  2012-12-11  7:44               ` Nathan Hintz
  0 siblings, 1 reply; 15+ messages in thread
From: Arend van Spriel @ 2012-05-06 20:48 UTC (permalink / raw)
  To: Nathan Hintz; +Cc: Hauke Mehrtens, linville, linux-wireless

On 05/06/2012 08:07 PM, Nathan Hintz wrote:
> On Sat, 2012-05-05 at 22:40 +0200, Arend van Spriel wrote:
>> On 05/05/2012 02:41 PM, Hauke Mehrtens wrote:
>>> On 05/05/2012 09:50 AM, Nathan Hintz wrote:
>>>> On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
>>>>> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
>>>>>> Adds a missing read to flush the previous write (per the Broadcom SDK).
>>>>>>
>>>>>> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
>>>>>> ---
>>>>>>  drivers/bcma/core.c |    1 +
>>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
>>>>>> index 893f6e0..c4e6deb 100644
>>>>>> --- a/drivers/bcma/core.c
>>>>>> +++ b/drivers/bcma/core.c
>>>>>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
>>>>>>  	udelay(10);
>>>>>>  
>>>>>>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
>>>>>> +	bcma_aread32(core, BCMA_RESET_CTL);
>>>>>>  	udelay(1);
>>>>>>  }
>>>>>>  EXPORT_SYMBOL_GPL(bcma_core_disable);
>>>>>
>>>>> Hi Nathan,
>>>>>
>>>>> The read after write is only needed on (certain) SoCs. As bcma is not
>>>>> being used just for these SoCs I suggest to make the flush conditional.
>>>>> In brcmsmac we introduced "flushed write" function, which does the read
>>>>> only for those SoCs.
>>>>>
>>>>> Gr. AvS
>>>>>
>>>>>
>>>> There are several other occurrences in core.c that are the same
>>>> implementation as what was added in this patch.  Is there a reason
>>>> why the existing code isn't the way you have suggested already?
>>>> Would it be better to submit your suggestion as a separate patch, or
>>>> just roll it all into this one?
>>>>
>>>> Nathan
>>>
>>> In some older versions of the Broadcom SDK (e.g. version with wl
>>> 5.10.147.0 and the version brcmsmac seams to be based on) this read is
>>> not included in ai_core_disable(), but in more recent versions (e.g.
>>> version including wl 5.100.138.9) this read is included unconditionally.
>>>
>>> @Arend For what SoCs should we read after this write?
>>> When you are speaking of (certain) SoCs are you talking about this code
>>> and comment from brcmsmac?
>>>
>>> #ifdef CONFIG_BCM47XX
>>> /*
>>>  * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder
>>>  * transactions. As a fix, a read after write is performed on certain places
>>>  * in the code. Older chips and the newer 5357 family don't require this
>>> fix.
>>>  */
>>> #define bcma_wflush16(c, o, v) \
>>> 	({ bcma_write16(c, o, v); (void)bcma_read16(c, o); })
>>>
>>>
>>> Hauke
>>>
>>
>> Yes, Those are indeed the SoCs I meant. I was being to lazy to look it
>> up, so thanks for making the effort ;-)
>>
>> Gr. AvS
>>
>>
> Is there a reason why a similar 'flush write' function was not applied
> to all of the writes to 'objaddr' in 'brcmsmac/main.c', or are these
> flushes not '4716/4717/4718/4706' specific?  For example:
> 
> static void brcms_b_set_cwmin(struct brcms_hardware *wlc_hw, u16 newmin)
> {
> 	wlc_hw->band->CWmin = newmin;
> 
> 	bcma_write32(wlc_hw->d11core, D11REGOFFS(objaddr),
> 		     OBJADDR_SCR_SEL | S_DOT11_CWMIN);
> 	(void)bcma_read32(wlc_hw->d11core, D11REGOFFS(objaddr));
> 	bcma_write32(wlc_hw->d11core, D11REGOFFS(objdata), newmin);
> }
> 
> Nathan
> 

These are indeed needed for all devices.

Gr. AvS


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write
  2012-05-06 20:48             ` Arend van Spriel
@ 2012-12-11  7:44               ` Nathan Hintz
  0 siblings, 0 replies; 15+ messages in thread
From: Nathan Hintz @ 2012-12-11  7:44 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: Hauke Mehrtens, linville, linux-wireless

On Sun, 2012-05-06 at 22:48 +0200, Arend van Spriel wrote:
> On 05/06/2012 08:07 PM, Nathan Hintz wrote:
> > On Sat, 2012-05-05 at 22:40 +0200, Arend van Spriel wrote:
> >> On 05/05/2012 02:41 PM, Hauke Mehrtens wrote:
> >>> On 05/05/2012 09:50 AM, Nathan Hintz wrote:
> >>>> On Sat, 2012-05-05 at 08:03 +0200, Arend van Spriel wrote:
> >>>>> On 05/05/2012 06:56 AM, Nathan Hintz wrote:
> >>>>>> Adds a missing read to flush the previous write (per the Broadcom SDK).
> >>>>>>
> >>>>>> Signed-off-by: Nathan Hintz <nlhintz@hotmail.com>
> >>>>>> ---
> >>>>>>  drivers/bcma/core.c |    1 +
> >>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
> >>>>>> index 893f6e0..c4e6deb 100644
> >>>>>> --- a/drivers/bcma/core.c
> >>>>>> +++ b/drivers/bcma/core.c
> >>>>>> @@ -30,6 +30,7 @@ void bcma_core_disable(struct bcma_device *core, u32 flags)
> >>>>>>  	udelay(10);
> >>>>>>  
> >>>>>>  	bcma_awrite32(core, BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
> >>>>>> +	bcma_aread32(core, BCMA_RESET_CTL);
> >>>>>>  	udelay(1);
> >>>>>>  }
> >>>>>>  EXPORT_SYMBOL_GPL(bcma_core_disable);
> >>>>>
> >>>>> Hi Nathan,
> >>>>>
> >>>>> The read after write is only needed on (certain) SoCs. As bcma is not
> >>>>> being used just for these SoCs I suggest to make the flush conditional.
> >>>>> In brcmsmac we introduced "flushed write" function, which does the read
> >>>>> only for those SoCs.
> >>>>>
> >>>>> Gr. AvS
> >>>>>
> >>>>>
> >>>> There are several other occurrences in core.c that are the same
> >>>> implementation as what was added in this patch.  Is there a reason
> >>>> why the existing code isn't the way you have suggested already?
> >>>> Would it be better to submit your suggestion as a separate patch, or
> >>>> just roll it all into this one?
> >>>>
> >>>> Nathan
> >>>
> >>> In some older versions of the Broadcom SDK (e.g. version with wl
> >>> 5.10.147.0 and the version brcmsmac seams to be based on) this read is
> >>> not included in ai_core_disable(), but in more recent versions (e.g.
> >>> version including wl 5.100.138.9) this read is included unconditionally.
> >>>
> >>> @Arend For what SoCs should we read after this write?
> >>> When you are speaking of (certain) SoCs are you talking about this code
> >>> and comment from brcmsmac?
> >>>
> >>> #ifdef CONFIG_BCM47XX
> >>> /*
> >>>  * bcm4716 (which includes 4717 & 4718), plus 4706 on PCIe can reorder
> >>>  * transactions. As a fix, a read after write is performed on certain places
> >>>  * in the code. Older chips and the newer 5357 family don't require this
> >>> fix.
> >>>  */
> >>> #define bcma_wflush16(c, o, v) \
> >>> 	({ bcma_write16(c, o, v); (void)bcma_read16(c, o); })
> >>>
> >>>
> >>> Hauke
> >>>
> >>
> >> Yes, Those are indeed the SoCs I meant. I was being to lazy to look it
> >> up, so thanks for making the effort ;-)
> >>
> >> Gr. AvS
> >>
> >>
> > Is there a reason why a similar 'flush write' function was not applied
> > to all of the writes to 'objaddr' in 'brcmsmac/main.c', or are these
> > flushes not '4716/4717/4718/4706' specific?  For example:
> > 
> > static void brcms_b_set_cwmin(struct brcms_hardware *wlc_hw, u16 newmin)
> > {
> > 	wlc_hw->band->CWmin = newmin;
> > 
> > 	bcma_write32(wlc_hw->d11core, D11REGOFFS(objaddr),
> > 		     OBJADDR_SCR_SEL | S_DOT11_CWMIN);
> > 	(void)bcma_read32(wlc_hw->d11core, D11REGOFFS(objaddr));
> > 	bcma_write32(wlc_hw->d11core, D11REGOFFS(objdata), newmin);
> > }
> > 
> > Nathan
> > 
> 
> These are indeed needed for all devices.
> 
> Gr. AvS
> 
> 
Should there be a similar 'flush write' used at the end of 
brcms_b_core_ioctl (in brcmsmac/main.c), based on how writes to 
the IOCTL register are treated in drivers/bcma/core.c and in the 
Broadcom SDK.


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-12-11  7:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1336193796-32599-1-git-send-email-nlhintz@hotmail.com>
2012-05-05  4:56 ` [PATCH v3 1/6] bcma: Find names of non BCM cores Nathan Hintz
2012-05-05  6:16   ` Arend van Spriel
2012-05-05  7:23     ` Nathan Hintz
2012-05-05  4:56 ` [PATCH v3 2/6] bcma: Move initialization of SPROM to prevent overwrite Nathan Hintz
2012-05-05  4:56 ` [PATCH v3 3/6] bcma: Account for variable PCI memory base/size Nathan Hintz
2012-05-05  4:56 ` [PATCH v3 4/6] bcma: reads/writes are always 4 bytes, so always map 4 bytes Nathan Hintz
2012-05-05  4:56 ` [PATCH v3 5/6] bcma: Add __devexit to bcma_host_pci_remove Nathan Hintz
2012-05-05  4:56 ` [PATCH v3 6/6] bcma: Add flush for BCMA_RESET_CTL write Nathan Hintz
2012-05-05  6:03   ` Arend van Spriel
2012-05-05  7:50     ` Nathan Hintz
2012-05-05 12:41       ` Hauke Mehrtens
2012-05-05 20:40         ` Arend van Spriel
2012-05-06 18:07           ` Nathan Hintz
2012-05-06 20:48             ` Arend van Spriel
2012-12-11  7:44               ` Nathan Hintz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).