diff mbox

[2/2] bcma: expose cc sprom to sysfs

Message ID 20120816190616.GE6726@eris.garyseven.net (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Saul St. John Aug. 16, 2012, 7:06 p.m. UTC
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 <zajec5@gmail.com> 
Cc: John W. Linville <linville@tuxdriver.com>
Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
---
 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(-)

Comments

Arend van Spriel Aug. 17, 2012, 1:47 p.m. UTC | #1
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 <zajec5@gmail.com>
> Cc: John W. Linville <linville@tuxdriver.com>
> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Saul St. John Aug. 17, 2012, 10:54 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Saul St. John Aug. 17, 2012, 11:05 p.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafał Miłecki Aug. 23, 2012, 7:44 p.m. UTC | #4
2012/8/17 Arend van Spriel <arend@broadcom.com>:
> 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 <zajec5@gmail.com>
>> Cc: John W. Linville <linville@tuxdriver.com>
>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>
>
> 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?
Larry Finger Aug. 23, 2012, 8:53 p.m. UTC | #5
On 08/23/2012 02:44 PM, Rafa? Mi?ecki wrote:
> 2012/8/17 Arend van Spriel <arend@broadcom.com>:
>> 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 <zajec5@gmail.com>
>>> Cc: John W. Linville <linville@tuxdriver.com>
>>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>>
>>
>> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arend van Spriel Aug. 24, 2012, 8:37 a.m. UTC | #6
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 <arend@broadcom.com>:
>>> 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 <zajec5@gmail.com>
>>>> Cc: John W. Linville <linville@tuxdriver.com>
>>>> Signed-off-by: Saul St. John <saul.stjohn@gmail.com>
>>>
>>>
>>> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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