2012-08-16 19:04:35

by Saul St. John

[permalink] [raw]
Subject: [PATCH 0/2] bcma: expose cc sprom for read/write in sysfs

The ssb driver exposes the device SPROM to sysfs, which allows users to
modify various device settings using ssb-sprom from the b43-tools package.

These patches implement similar functionality in bcma, so that ssb-sprom
can be used with newer Broadcom devices.

Saul St. John (2):
bcma: register cc core driver, device
bcma: expose cc sprom to sysfs

drivers/bcma/bcma_private.h | 5 +
drivers/bcma/driver_chipcommon.c | 212 ++++++++++++++++++++++++---
drivers/bcma/main.c | 79 +++++-----
drivers/bcma/sprom.c | 47 +++++-
include/linux/bcma/bcma_driver_chipcommon.h | 4 +-
5 files changed, 287 insertions(+), 60 deletions(-)

--
1.7.10.4



2012-08-23 19:44:31

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

2012/8/17 Arend van Spriel <[email protected]>:
> On 08/16/2012 09:06 PM, Saul St. John wrote:
>>
>> On BCMA devices with a ChipCommon core of revision 31 or higher, the
>> device
>> SPROM can be accessed through CC core registers. This patch exposes the
>> SPROM on such devices for read/write access as a sysfs attribute.
>>
>> Tested on a MacBookPro8,2 with BCM4331.
>>
>> Cc: Rafał Miłecki <[email protected]>
>> Cc: John W. Linville <[email protected]>
>> Signed-off-by: Saul St. John <[email protected]>
>
>
> Hi Saul,
>
> I was still planning to come back to your reply on August 14. Just wanted to
> reply to this patch as I still feel it is a bad thing to open up the sprom
> as a whole. I can see the use-cases you mentioned as useful, but maybe we
> can get a specific solution for that.

I agree with Arend's doubts, on the other hand it would be nice to
provide some workaround for that stupid HP wifi blacklisting.

Providing a way to overwrite just a vendor is really close to allowing
overwriting anything. In that case we probably should just allow
writing whole SPROM... Which again, is sth some want to avoid.


I wonder if we could write some user-space tool for writing SPROM.
Accessing ChipCommon registers is quite trivial, the thing I'm not
familiar with is accessing PCIE Wifi card registers. I know there are
tools for accessing GPU card regs. They work really well, I wonder if
we can use the same method for Wifi cards?
If so, we could write user-space app and keep this out of kernel.
Maybe we could even extend that tool to cover ssb cards and drop SPROM
on SSB writing support from kernel?

--
Rafał

2012-08-16 19:06:20

by Saul St. John

[permalink] [raw]
Subject: [PATCH 2/2] bcma: expose cc sprom to sysfs

On BCMA devices with a ChipCommon core of revision 31 or higher, the device
SPROM can be accessed through CC core registers. This patch exposes the
SPROM on such devices for read/write access as a sysfs attribute.

Tested on a MacBookPro8,2 with BCM4331.

Cc: Rafał Miłecki <[email protected]>
Cc: John W. Linville <[email protected]>
Signed-off-by: Saul St. John <[email protected]>
---
drivers/bcma/bcma_private.h | 4 +
drivers/bcma/driver_chipcommon.c | 163 ++++++++++++++++++++++++++-
drivers/bcma/sprom.c | 47 +++++++-
include/linux/bcma/bcma_driver_chipcommon.h | 2 +
4 files changed, 213 insertions(+), 3 deletions(-)

diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
index 19b97f9..de32bb9 100644
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -41,6 +41,10 @@ void bcma_init_bus(struct bcma_bus *bus);

/* sprom.c */
int bcma_sprom_get(struct bcma_bus *bus);
+int bcma_sprom_valid(const u16 *sprom);
+void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom);
+int bcma_sprom_fromhex(u16 *sprom, const char *dump, size_t len);
+int bcma_sprom_tohex(const u16 *sprom, char *buf, size_t buf_len);

/* driver_chipcommon.c */
#ifdef CONFIG_BCMA_DRIVER_MIPS
diff --git a/drivers/bcma/driver_chipcommon.c b/drivers/bcma/driver_chipcommon.c
index 8c2013c..4dbddb3 100644
--- a/drivers/bcma/driver_chipcommon.c
+++ b/drivers/bcma/driver_chipcommon.c
@@ -22,6 +22,149 @@ static inline u32 bcma_cc_write32_masked(struct bcma_drv_cc *cc, u16 offset,
return value;
}

+static u16 bcma_cc_srom_cmd(struct bcma_device *cc, u32 cmd,
+ uint wordoff, u16 data)
+{
+ u16 res = 0xffff;
+ uint wait_cnt = 1000;
+
+ if ((cmd == BCMA_CC_SROM_CONTROL_OP_READ) ||
+ (cmd == BCMA_CC_SROM_CONTROL_OP_WRITE)) {
+ bcma_write32(cc, BCMA_CC_SROM_ADDRESS, wordoff * 2);
+ if (cmd == BCMA_CC_SROM_CONTROL_OP_WRITE)
+ bcma_write32(cc, BCMA_CC_SROM_DATA, data);
+ }
+
+ bcma_write32(cc, BCMA_CC_SROM_CONTROL,
+ BCMA_CC_SROM_CONTROL_START | cmd);
+
+ while (wait_cnt--) {
+ uint tmp = bcma_read32(cc, BCMA_CC_SROM_CONTROL);
+ if ((tmp & BCMA_CC_SROM_CONTROL_BUSY) == 0)
+ break;
+ }
+
+ if (!wait_cnt)
+ bcma_warn(cc->bus, "timed out waiting for busy to clear.\n");
+ else if (cmd == BCMA_CC_SROM_CONTROL_OP_READ)
+ res = (u16)bcma_read32(cc, BCMA_CC_SROM_DATA);
+
+ return res;
+}
+
+static ssize_t bcma_core_cc_attr_sprom_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u16 *sprom;
+ int i;
+ ssize_t res = -ERESTARTSYS;
+
+ struct bcma_device *cc = container_of(dev, struct bcma_device, dev);
+
+ sprom = kcalloc(SSB_SPROMSIZE_WORDS_R4, sizeof(u16),
+ GFP_KERNEL);
+ if (!sprom)
+ return -ENOMEM;
+
+ if (mutex_lock_interruptible(&cc->dev.mutex))
+ goto out_kfree;
+
+ if (cc->bus->chipinfo.id == BCMA_CHIP_ID_BCM4331 ||
+ cc->bus->chipinfo.id == BCMA_CHIP_ID_BCM43431)
+ bcma_chipco_bcm4331_ext_pa_lines_ctl(&cc->bus->drv_cc,
+ false);
+
+ for (i = 0; i < SSB_SPROMSIZE_WORDS_R4; i++)
+ sprom[i] = bcma_cc_srom_cmd(cc, BCMA_CC_SROM_CONTROL_OP_READ,
+ i, 0);
+
+ if (cc->bus->chipinfo.id == BCMA_CHIP_ID_BCM4331 ||
+ cc->bus->chipinfo.id == BCMA_CHIP_ID_BCM43431)
+ bcma_chipco_bcm4331_ext_pa_lines_ctl(&cc->bus->drv_cc,
+ true);
+
+ mutex_unlock(&cc->dev.mutex);
+
+ res = bcma_sprom_tohex(sprom, buf, PAGE_SIZE);
+
+out_kfree:
+ kfree(sprom);
+
+ return res;
+}
+
+static ssize_t bcma_core_cc_attr_sprom_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u16 *sprom;
+ int err, i;
+
+ struct bcma_device *core = container_of(dev, struct bcma_device, dev);
+
+ sprom = kcalloc(SSB_SPROMSIZE_WORDS_R4, sizeof(u16), GFP_KERNEL);
+ if (!sprom)
+ return -ENOMEM;
+
+ err = bcma_sprom_fromhex(sprom, buf, count);
+ if (!err)
+ err = bcma_sprom_valid(sprom);
+ if (err)
+ goto out_kfree;
+
+ if (mutex_lock_interruptible(&core->dev.mutex)) {
+ err = -ERESTARTSYS;
+ goto out_kfree;
+ }
+
+ if (core->bus->chipinfo.id == BCMA_CHIP_ID_BCM4331 ||
+ core->bus->chipinfo.id == BCMA_CHIP_ID_BCM43431)
+ bcma_chipco_bcm4331_ext_pa_lines_ctl(&core->bus->drv_cc,
+ false);
+
+ bcma_warn(core->bus, "Writing SPROM. Do NOT turn off the power!\n");
+
+ bcma_cc_srom_cmd(core, BCMA_CC_SROM_CONTROL_OP_WREN, 0, 0);
+
+ msleep(500);
+
+ for (i = 0; i < SSB_SPROMSIZE_WORDS_R4; i++) {
+ if (i == SSB_SPROMSIZE_WORDS_R4 / 4)
+ bcma_warn(core->bus, "SPROM write 25%% complete.\n");
+ else if (i == SSB_SPROMSIZE_WORDS_R4 / 2)
+ bcma_warn(core->bus, "SPROM write 50%% complete.\n");
+ else if (i == (SSB_SPROMSIZE_WORDS_R4 * 3) / 4)
+ bcma_warn(core->bus, "SPROM write 75%% complete.\n");
+ bcma_cc_srom_cmd(core, BCMA_CC_SROM_CONTROL_OP_WRITE,
+ i, sprom[i]);
+ msleep(20);
+ }
+
+ bcma_cc_srom_cmd(core, BCMA_CC_SROM_CONTROL_OP_WRDIS, 0, 0);
+
+ msleep(500);
+
+ bcma_warn(core->bus, "SPROM wrte complete.\n");
+
+ if (core->bus->chipinfo.id == BCMA_CHIP_ID_BCM4331 ||
+ core->bus->chipinfo.id == BCMA_CHIP_ID_BCM43431)
+ bcma_chipco_bcm4331_ext_pa_lines_ctl(&core->bus->drv_cc,
+ true);
+
+ mutex_unlock(&core->dev.mutex);
+
+ bcma_sprom_extract_r8(core->bus, sprom);
+
+out_kfree:
+ kfree(sprom);
+
+ return err ? err : count;
+}
+
+static DEVICE_ATTR(sprom, 0600, bcma_core_cc_attr_sprom_show,
+ bcma_core_cc_attr_sprom_store);
+
static int bcma_core_cc_initialize(struct bcma_device *core)
{
u32 leddc_on = 10;
@@ -60,6 +203,23 @@ static int bcma_core_cc_initialize(struct bcma_device *core)
return 0;
}

+static int bcma_core_cc_probe(struct bcma_device *core)
+{
+ bcma_core_cc_initialize(core);
+ if (core->id.rev >= 31 &&
+ core->bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM)
+ device_create_file(&core->dev, &dev_attr_sprom);
+
+ return 0;
+}
+
+static void bcma_core_cc_remove(struct bcma_device *core)
+{
+ if (core->id.rev >= 31 &&
+ core->bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM)
+ device_remove_file(&core->dev, &dev_attr_sprom);
+}
+
static struct bcma_device_id bcma_core_cc_id_table[] = {
BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_CHIPCOMMON,
BCMA_ANY_REV, BCMA_ANY_CLASS),
@@ -70,8 +230,9 @@ static struct bcma_device_id bcma_core_cc_id_table[] = {

struct bcma_driver bcma_core_cc_driver = {
.name = "bcma-cc-core",
- .probe = bcma_core_cc_initialize,
+ .probe = bcma_core_cc_probe,
.id_table = bcma_core_cc_id_table,
+ .remove = bcma_core_cc_remove,
#ifdef CONFIG_PM
.resume = bcma_core_cc_initialize,
#endif
diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
index 9ea4627..eff6104 100644
--- a/drivers/bcma/sprom.c
+++ b/drivers/bcma/sprom.c
@@ -15,6 +15,7 @@
#include <linux/io.h>
#include <linux/dma-mapping.h>
#include <linux/slab.h>
+#include <linux/ctype.h>

static int(*get_fallback_sprom)(struct bcma_bus *dev, struct ssb_sprom *out);

@@ -154,7 +155,7 @@ static int bcma_sprom_check_crc(const u16 *sprom)
return 0;
}

-static int bcma_sprom_valid(const u16 *sprom)
+int bcma_sprom_valid(const u16 *sprom)
{
u16 revision;
int err;
@@ -197,7 +198,7 @@ static int bcma_sprom_valid(const u16 *sprom)
SPEX(_field[7], _offset + 14, _mask, _shift); \
} while (0)

-static void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
+void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
{
u16 v, o;
int i;
@@ -602,3 +603,45 @@ out:
kfree(sprom);
return err;
}
+
+int bcma_sprom_tohex(const u16 *sprom, char *buf, size_t buf_len)
+{
+ int i, pos = 0;
+
+ for (i = 0; i < SSB_SPROMSIZE_WORDS_R4; i++)
+ pos += snprintf(buf + pos, buf_len - pos - 1,
+ "%04X", swab16(sprom[i]) & 0xFFFF);
+ pos += snprintf(buf + pos, buf_len - pos - 1, "\n");
+
+ return pos + 1;
+}
+
+int bcma_sprom_fromhex(u16 *sprom, const char *dump, size_t len)
+{
+ char c, tmp[5] = { 0 };
+ int err, cnt = 0;
+ unsigned long parsed;
+
+ /* Strip whitespace at the end. */
+ while (len) {
+ c = dump[len - 1];
+ if (!isspace(c) && c != '\0')
+ break;
+ len--;
+ }
+
+ /* Length must match exactly. */
+ if (len != SSB_SPROMSIZE_WORDS_R4 * 4)
+ return -EINVAL;
+
+ while (cnt < SSB_SPROMSIZE_WORDS_R4) {
+ memcpy(tmp, dump, 4);
+ dump += 4;
+ err = kstrtoul(tmp, 16, &parsed);
+ if (err)
+ return err;
+ sprom[cnt++] = swab16((u16)parsed);
+ }
+
+ return 0;
+}
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index f6e5858..57893a8 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -251,6 +251,8 @@
#define BCMA_CC_FLASH_CFG_DS 0x0010 /* Data size, 0=8bit, 1=16bit */
#define BCMA_CC_FLASH_WAITCNT 0x012C
#define BCMA_CC_SROM_CONTROL 0x0190
+#define BCMA_CC_SROM_ADDRESS 0x0194
+#define BCMA_CC_SROM_DATA 0x0198
#define BCMA_CC_SROM_CONTROL_START 0x80000000
#define BCMA_CC_SROM_CONTROL_BUSY 0x80000000
#define BCMA_CC_SROM_CONTROL_OPCODE 0x60000000
--
1.7.10.4


2012-08-17 22:54:12

by Saul St. John

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

On Fri, Aug 17, 2012 at 03:47:28PM +0200, Arend van Spriel wrote:
[snip]
>
> Hi Saul,
>
> I was still planning to come back to your reply on August 14. Just
> wanted to reply to this patch as I still feel it is a bad thing to
> open up the sprom as a whole. I can see the use-cases you mentioned
> as useful, but maybe we can get a specific solution for that.
>
> Gr. AvS
>

Hi Arend!

Apologies for forking the discussion. I just wasn't sure that any more replies
would be forthcoming.

I'm curious why you feel it's a "bad" thing to open up the sprom as a whole. I
see a number of distinct advantages to it; most prominently, that it allows
for the use of existing tools and processes. I can find several examples of
end-users attempting to modify the sprom on bcma devices using the
instructions from b43-tools ([1][2][3]), and suggest that it would be
confusing to those users were the procedures for sprom-modification through
bcma substantially different than the procedures through ssb.

That said, I do recognize that this functionality is potentially dangerous in
some circumstances. I'd considered hiding it behind a non-default module
parameter-- would that assuage your concerns? Alternatively, if you have
specific solutions in mind for the use-cases previously discussed, I'm very
interested in knowing them.

-saul

[1] http://ubuntuforums.org/showthread.php?t=1830739
[2] http://www.tonymacx86.com/network/25669-rebranding-bcm94322mc-based-prasys-guide-what-device-id-8.html#post291328
[3] http://www.tonymacx86.com/network/26904-help-rebrand-broadcom-bcm94322-5.html

2012-08-16 19:05:45

by Saul St. John

[permalink] [raw]
Subject: [PATCH 1/2] bcma: register cc core driver, device

This patch adds a device driver for the ChipCommon core ("bcma-cc-core"),
registers that driver during module initialization, and registers the CC
device if present with device_register().

Tested on a MacBookPro8,2 with BCM4331.

Cc: Rafał Miłecki <[email protected]>
Cc: John W. Linville <[email protected]>
Signed-off-by: Saul St. John <[email protected]>
---
drivers/bcma/bcma_private.h | 1 +
drivers/bcma/driver_chipcommon.c | 51 +++++++++++------
drivers/bcma/main.c | 79 ++++++++++++++-------------
include/linux/bcma/bcma_driver_chipcommon.h | 2 -
4 files changed, 75 insertions(+), 58 deletions(-)

diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
index 3cf9cc9..19b97f9 100644
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -46,6 +46,7 @@ int bcma_sprom_get(struct bcma_bus *bus);
#ifdef CONFIG_BCMA_DRIVER_MIPS
void bcma_chipco_serial_init(struct bcma_drv_cc *cc);
#endif /* CONFIG_BCMA_DRIVER_MIPS */
+extern struct bcma_driver bcma_core_cc_driver;

/* driver_chipcommon_pmu.c */
u32 bcma_pmu_alp_clock(struct bcma_drv_cc *cc);
diff --git a/drivers/bcma/driver_chipcommon.c b/drivers/bcma/driver_chipcommon.c
index a4c3ebc..8c2013c 100644
--- a/drivers/bcma/driver_chipcommon.c
+++ b/drivers/bcma/driver_chipcommon.c
@@ -22,35 +22,34 @@ static inline u32 bcma_cc_write32_masked(struct bcma_drv_cc *cc, u16 offset,
return value;
}

-void bcma_core_chipcommon_init(struct bcma_drv_cc *cc)
+static int bcma_core_cc_initialize(struct bcma_device *core)
{
u32 leddc_on = 10;
u32 leddc_off = 90;

- if (cc->setup_done)
- return;
+ struct bcma_drv_cc *cc = &core->bus->drv_cc;

- if (cc->core->id.rev >= 11)
- cc->status = bcma_cc_read32(cc, BCMA_CC_CHIPSTAT);
- cc->capabilities = bcma_cc_read32(cc, BCMA_CC_CAP);
- if (cc->core->id.rev >= 35)
- cc->capabilities_ext = bcma_cc_read32(cc, BCMA_CC_CAP_EXT);
+ if (core->id.rev >= 11)
+ cc->status = bcma_read32(core, BCMA_CC_CHIPSTAT);
+ cc->capabilities = bcma_read32(core, BCMA_CC_CAP);
+ if (core->id.rev >= 35)
+ cc->capabilities_ext = bcma_read32(core, BCMA_CC_CAP_EXT);

- if (cc->core->id.rev >= 20) {
- bcma_cc_write32(cc, BCMA_CC_GPIOPULLUP, 0);
- bcma_cc_write32(cc, BCMA_CC_GPIOPULLDOWN, 0);
+ if (core->id.rev >= 20) {
+ bcma_write32(core, BCMA_CC_GPIOPULLUP, 0);
+ bcma_write32(core, BCMA_CC_GPIOPULLDOWN, 0);
}

if (cc->capabilities & BCMA_CC_CAP_PMU)
bcma_pmu_init(cc);
if (cc->capabilities & BCMA_CC_CAP_PCTL)
- bcma_err(cc->core->bus, "Power control not implemented!\n");
+ bcma_err(core->bus, "Power control not implemented!\n");

- if (cc->core->id.rev >= 16) {
- if (cc->core->bus->sprom.leddc_on_time &&
- cc->core->bus->sprom.leddc_off_time) {
- leddc_on = cc->core->bus->sprom.leddc_on_time;
- leddc_off = cc->core->bus->sprom.leddc_off_time;
+ if (core->id.rev >= 16) {
+ if (core->bus->sprom.leddc_on_time &&
+ core->bus->sprom.leddc_off_time) {
+ leddc_on = core->bus->sprom.leddc_on_time;
+ leddc_off = core->bus->sprom.leddc_off_time;
}
bcma_cc_write32(cc, BCMA_CC_GPIOTIMER,
((leddc_on << BCMA_CC_GPIOTIMER_ONTIME_SHIFT) |
@@ -58,8 +57,26 @@ void bcma_core_chipcommon_init(struct bcma_drv_cc *cc)
}

cc->setup_done = true;
+ return 0;
}

+static struct bcma_device_id bcma_core_cc_id_table[] = {
+ BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_CHIPCOMMON,
+ BCMA_ANY_REV, BCMA_ANY_CLASS),
+ BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_CHIPCOMMON,
+ BCMA_ANY_REV, BCMA_ANY_CLASS),
+ BCMA_CORETABLE_END
+};
+
+struct bcma_driver bcma_core_cc_driver = {
+ .name = "bcma-cc-core",
+ .probe = bcma_core_cc_initialize,
+ .id_table = bcma_core_cc_id_table,
+#ifdef CONFIG_PM
+ .resume = bcma_core_cc_initialize,
+#endif
+};
+
/* Set chip watchdog reset timer to fire in 'ticks' backplane cycles */
void bcma_chipco_watchdog_timer_set(struct bcma_drv_cc *cc, u32 ticks)
{
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 758af9c..498733b 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -90,13 +90,44 @@ static void bcma_release_core_dev(struct device *dev)
kfree(core);
}

+static void bcma_register_core(struct bcma_device *core)
+{
+ int err;
+
+ core->dev.release = bcma_release_core_dev;
+ core->dev.bus = &bcma_bus_type;
+ dev_set_name(&core->dev, "bcma%d:%d",
+ core->bus->num, core->core_index);
+
+ switch (core->bus->hosttype) {
+ case BCMA_HOSTTYPE_PCI:
+ core->dev.parent = &core->bus->host_pci->dev;
+ core->dma_dev = &core->bus->host_pci->dev;
+ core->irq = core->bus->host_pci->irq;
+ break;
+ case BCMA_HOSTTYPE_SOC:
+ core->dev.dma_mask = &core->dev.coherent_dma_mask;
+ core->dma_dev = &core->dev;
+ break;
+ case BCMA_HOSTTYPE_SDIO:
+ break;
+ }
+
+ err = device_register(&core->dev);
+ if (err) {
+ bcma_err(core->bus,
+ "Could not register dev for core 0x%03X\n",
+ core->id.id);
+ }
+ core->dev_registered = true;
+}
+
static int bcma_register_cores(struct bcma_bus *bus)
{
struct bcma_device *core;
- int err, dev_id = 0;

list_for_each_entry(core, &bus->cores, list) {
- /* We support that cores ourself */
+ /* We support these cores ourself */
switch (core->id.id) {
case BCMA_CORE_4706_CHIPCOMMON:
case BCMA_CORE_CHIPCOMMON:
@@ -106,34 +137,7 @@ static int bcma_register_cores(struct bcma_bus *bus)
case BCMA_CORE_4706_MAC_GBIT_COMMON:
continue;
}
-
- core->dev.release = bcma_release_core_dev;
- core->dev.bus = &bcma_bus_type;
- dev_set_name(&core->dev, "bcma%d:%d", bus->num, dev_id);
-
- switch (bus->hosttype) {
- case BCMA_HOSTTYPE_PCI:
- core->dev.parent = &bus->host_pci->dev;
- core->dma_dev = &bus->host_pci->dev;
- core->irq = bus->host_pci->irq;
- break;
- case BCMA_HOSTTYPE_SOC:
- core->dev.dma_mask = &core->dev.coherent_dma_mask;
- core->dma_dev = &core->dev;
- break;
- case BCMA_HOSTTYPE_SDIO:
- break;
- }
-
- err = device_register(&core->dev);
- if (err) {
- bcma_err(bus,
- "Could not register dev for core 0x%03X\n",
- core->id.id);
- continue;
- }
- core->dev_registered = true;
- dev_id++;
+ bcma_register_core(core);
}

return 0;
@@ -143,7 +147,7 @@ static void bcma_unregister_cores(struct bcma_bus *bus)
{
struct bcma_device *core;

- list_for_each_entry(core, &bus->cores, list) {
+ list_for_each_entry_reverse(core, &bus->cores, list) {
if (core->dev_registered)
device_unregister(&core->dev);
}
@@ -169,7 +173,7 @@ int __devinit bcma_bus_register(struct bcma_bus *bus)
core = bcma_find_core(bus, bcma_cc_core_id(bus));
if (core) {
bus->drv_cc.core = core;
- bcma_core_chipcommon_init(&bus->drv_cc);
+ bcma_register_core(core);
}

/* Init MIPS core */
@@ -251,7 +255,7 @@ int __init bcma_bus_early_register(struct bcma_bus *bus,
core = bcma_find_core(bus, bcma_cc_core_id(bus));
if (core) {
bus->drv_cc.core = core;
- bcma_core_chipcommon_init(&bus->drv_cc);
+ bcma_register_core(core);
}

/* Init MIPS core */
@@ -286,12 +290,6 @@ int bcma_bus_resume(struct bcma_bus *bus)
{
struct bcma_device *core;

- /* Init CC core */
- if (bus->drv_cc.core) {
- bus->drv_cc.setup_done = false;
- bcma_core_chipcommon_init(&bus->drv_cc);
- }
-
list_for_each_entry(core, &bus->cores, list) {
struct device_driver *drv = core->dev.driver;
if (drv) {
@@ -381,6 +379,8 @@ static int __init bcma_modinit(void)
if (err)
return err;

+ bcma_driver_register(&bcma_core_cc_driver);
+
#ifdef CONFIG_BCMA_HOST_PCI
err = bcma_host_pci_init();
if (err) {
@@ -398,6 +398,7 @@ static void __exit bcma_modexit(void)
#ifdef CONFIG_BCMA_HOST_PCI
bcma_host_pci_exit();
#endif
+ bcma_driver_unregister(&bcma_core_cc_driver);
bus_unregister(&bcma_bus_type);
}
module_exit(bcma_modexit)
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index d323a4b..f6e5858 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -469,8 +469,6 @@ struct bcma_drv_cc {
#define bcma_cc_maskset32(cc, offset, mask, set) \
bcma_cc_write32(cc, offset, (bcma_cc_read32(cc, offset) & (mask)) | (set))

-extern void bcma_core_chipcommon_init(struct bcma_drv_cc *cc);
-
extern void bcma_chipco_suspend(struct bcma_drv_cc *cc);
extern void bcma_chipco_resume(struct bcma_drv_cc *cc);

--
1.7.10.4


2012-08-24 08:37:35

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

On 08/23/2012 10:53 PM, Larry Finger wrote:
> On 08/23/2012 02:44 PM, Rafał Miłecki wrote:
>> 2012/8/17 Arend van Spriel <[email protected]>:
>>> On 08/16/2012 09:06 PM, Saul St. John wrote:
>>>>
>>>> On BCMA devices with a ChipCommon core of revision 31 or higher, the
>>>> device
>>>> SPROM can be accessed through CC core registers. This patch exposes the
>>>> SPROM on such devices for read/write access as a sysfs attribute.
>>>>
>>>> Tested on a MacBookPro8,2 with BCM4331.
>>>>
>>>> Cc: Rafał Miłecki <[email protected]>
>>>> Cc: John W. Linville <[email protected]>
>>>> Signed-off-by: Saul St. John <[email protected]>
>>>
>>>
>>> Hi Saul,
>>>
>>> I was still planning to come back to your reply on August 14. Just
>>> wanted to
>>> reply to this patch as I still feel it is a bad thing to open up the
>>> sprom
>>> as a whole. I can see the use-cases you mentioned as useful, but
>>> maybe we
>>> can get a specific solution for that.
>>
>> I agree with Arend's doubts, on the other hand it would be nice to
>> provide some workaround for that stupid HP wifi blacklisting.
>>
>> Providing a way to overwrite just a vendor is really close to allowing
>> overwriting anything. In that case we probably should just allow
>> writing whole SPROM... Which again, is sth some want to avoid.
>>
>>
>> I wonder if we could write some user-space tool for writing SPROM.
>> Accessing ChipCommon registers is quite trivial, the thing I'm not
>> familiar with is accessing PCIE Wifi card registers. I know there are
>> tools for accessing GPU card regs. They work really well, I wonder if
>> we can use the same method for Wifi cards?
>> If so, we could write user-space app and keep this out of kernel.
>> Maybe we could even extend that tool to cover ssb cards and drop SPROM
>> on SSB writing support from kernel?
>
> This idea sounds good to me. The only valid use of the ssb SPROM writing
> was when we found some G-PHY cards that had the BT compatibility setting
> wrong. Now there is a set of quirks that eliminate that need for
> rewriting the SPROM.
>
> With a separate utility, we can control what parameters can be changed.
> The vendor codes are one possibility. What else would be useful? I have
> seen changing the MAC address be mentioned, but I would argue against
> that. There are too many possibilities for misuse.

That was indeed my concern. I know people have their legitimate reasons
for it, but it is more for convenience as it is a necessity. Given that
MAC spoofing also allows misuse.

The same is true for the country code. When left to the user they can
select a country code with a less restrictive regulatory parameters for
which the device might not be certified. With the utility we could also
control how the parameters are changed. However, the country code change
on system level is already covered and properly restricted by the
regulatory framework.

Gr. AvS


2012-08-23 20:53:35

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

On 08/23/2012 02:44 PM, Rafał Miłecki wrote:
> 2012/8/17 Arend van Spriel <[email protected]>:
>> On 08/16/2012 09:06 PM, Saul St. John wrote:
>>>
>>> On BCMA devices with a ChipCommon core of revision 31 or higher, the
>>> device
>>> SPROM can be accessed through CC core registers. This patch exposes the
>>> SPROM on such devices for read/write access as a sysfs attribute.
>>>
>>> Tested on a MacBookPro8,2 with BCM4331.
>>>
>>> Cc: Rafał Miłecki <[email protected]>
>>> Cc: John W. Linville <[email protected]>
>>> Signed-off-by: Saul St. John <[email protected]>
>>
>>
>> Hi Saul,
>>
>> I was still planning to come back to your reply on August 14. Just wanted to
>> reply to this patch as I still feel it is a bad thing to open up the sprom
>> as a whole. I can see the use-cases you mentioned as useful, but maybe we
>> can get a specific solution for that.
>
> I agree with Arend's doubts, on the other hand it would be nice to
> provide some workaround for that stupid HP wifi blacklisting.
>
> Providing a way to overwrite just a vendor is really close to allowing
> overwriting anything. In that case we probably should just allow
> writing whole SPROM... Which again, is sth some want to avoid.
>
>
> I wonder if we could write some user-space tool for writing SPROM.
> Accessing ChipCommon registers is quite trivial, the thing I'm not
> familiar with is accessing PCIE Wifi card registers. I know there are
> tools for accessing GPU card regs. They work really well, I wonder if
> we can use the same method for Wifi cards?
> If so, we could write user-space app and keep this out of kernel.
> Maybe we could even extend that tool to cover ssb cards and drop SPROM
> on SSB writing support from kernel?

This idea sounds good to me. The only valid use of the ssb SPROM writing was
when we found some G-PHY cards that had the BT compatibility setting wrong. Now
there is a set of quirks that eliminate that need for rewriting the SPROM.

With a separate utility, we can control what parameters can be changed. The
vendor codes are one possibility. What else would be useful? I have seen
changing the MAC address be mentioned, but I would argue against that. There are
too many possibilities for misuse.

Larry


2012-08-17 23:05:56

by Saul St. John

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

On Fri, Aug 17, 2012 at 05:54:06PM -0500, Saul St. John wrote:
> [2] http://www.tonymacx86.com/network/25669-rebranding-bcm94322mc-based-prasys-guide-what-device-id-8.html#post291328
> [3] http://www.tonymacx86.com/network/26904-help-rebrand-broadcom-bcm94322-5.html

Sorry, these two references are not actually on point. Please ignore them, my
grep-fu is off today.

2012-08-17 13:47:40

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 2/2] bcma: expose cc sprom to sysfs

On 08/16/2012 09:06 PM, Saul St. John wrote:
> On BCMA devices with a ChipCommon core of revision 31 or higher, the device
> SPROM can be accessed through CC core registers. This patch exposes the
> SPROM on such devices for read/write access as a sysfs attribute.
>
> Tested on a MacBookPro8,2 with BCM4331.
>
> Cc: Rafał Miłecki <[email protected]>
> Cc: John W. Linville <[email protected]>
> Signed-off-by: Saul St. John <[email protected]>

Hi Saul,

I was still planning to come back to your reply on August 14. Just
wanted to reply to this patch as I still feel it is a bad thing to open
up the sprom as a whole. I can see the use-cases you mentioned as
useful, but maybe we can get a specific solution for that.

Gr. AvS