[RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

Wireless Broadcom chips can have either their SPROM data stored
on either external SPROM or on-chip OTP memory. Both are accessed
through the same register space. This patch adds support for the
on-chip OTP memory.

Tested with:
BCM43224 OTP and SPROM
BCM4331 SPROM
BCM4313 OTP

This patch is in response so gmane article [1].

[1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426

Cc: Saul St. John <saul.stjohn@gmail.com>
Cc: Rafal Milecki <zajec5@gmail.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
Determining the offset for OTP sprom data turned out to be
easier as it boils down to reading a register. This change
collides with patch posted by Hauke:

bcma: add support for sprom not found on the device.

Now working on changes in brcmsmac to start using the sprom
data stored in struct bcma_bus. Feel free to comment this patch.

Gr. AvS
---
 drivers/bcma/sprom.c                        |  118 ++++++++++++++++++++++++---
 include/linux/bcma/bcma_driver_chipcommon.h |    9 ++
 2 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
index ca77525..4dd537c 100644
--- a/drivers/bcma/sprom.c
+++ b/drivers/bcma/sprom.c
@@ -246,22 +246,121 @@ static void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
 	     SSB_SROM8_FEM_ANTSWLUT_SHIFT);
 }
 
+/*
+ * Indicates the presence of external SPROM.
+ */
+static bool bcma_sprom_ext_available(struct bcma_bus *bus)
+{
+	u32 chip_status;
+	u32 srom_control;
+	u32 present_mask;
+
+	if (bus->drv_cc.core->id.rev >= 31) {
+		if (!(bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM))
+				return false;
+
+		srom_control = bcma_read32(bus->drv_cc.core,
+					   BCMA_CC_SROM_CONTROL);
+		return !!(srom_control & BCMA_CC_SROM_CONTROL_PRESENT);
+	}
+
+	/* older chipcommon revisions use chip status register */
+	chip_status = bcma_read32(bus->drv_cc.core, BCMA_CC_CHIPSTAT);
+	switch (bus->chipinfo.id) {
+	case 0x4313:
+		present_mask = BCMA_CC_CHIPST_4313_SPROM_PRESENT;
+		break;
+
+	case 0x4331:
+		present_mask = BCMA_CC_CHIPST_4331_SPROM_PRESENT;
+		break;
+
+	default:
+		return true;
+	}
+
+	return (chip_status & present_mask) == present_mask;
+}
+
+/*
+ * Indicates that on-chip OTP memory is present and enabled.
+ */
+static bool bcma_sprom_onchip_available(struct bcma_bus *bus)
+{
+	u32 chip_status;
+	u32 otpsize = 0;
+	bool present;
+
+	chip_status = bcma_read32(bus->drv_cc.core, BCMA_CC_CHIPSTAT);
+	switch (bus->chipinfo.id) {
+	case 0x4313:
+		present = chip_status & BCMA_CC_CHIPST_4313_OTP_PRESENT;
+		break;
+
+	case 0x4331:
+		present = chip_status & BCMA_CC_CHIPST_4331_OTP_PRESENT;
+		break;
+
+	case 43224:
+	case 43225:
+		/* for these chips OTP is always available */
+		present = true;
+		break;
+
+	default:
+		present = false;
+		break;
+	}
+
+	if (present) {
+		otpsize = bus->drv_cc.capabilities & BCMA_CC_CAP_OTPS;
+		otpsize >>= BCMA_CC_CAP_OTPS_SHIFT;
+	}
+
+	return otpsize != 0;
+}
+
+/*
+ * Verify OTP is filled and determine the byte
+ * offset where SPROM data is located.
+ *
+ * On error, returns 0; byte offset otherwise.
+ */
+static int bcma_sprom_onchip_offset(struct bcma_bus *bus)
+{
+	struct bcma_device *cc = bus->drv_cc.core;
+	u32 offset;
+
+	/* verify OTP status */
+	if ((bcma_read32(cc, BCMA_CC_OTPS) & BCMA_CC_OTPS_GU_PROG_HW) == 0)
+		return 0;
+
+	/* obtain bit offset from otplayout register */
+	offset = (bcma_read32(cc, BCMA_CC_OTPL) & BCMA_CC_OTPL_GURGN_OFFSET);
+	return BCMA_CC_SPROM + (offset >> 3);
+}
+
 int bcma_sprom_get(struct bcma_bus *bus)
 {
-	u16 offset;
+	u16 offset = BCMA_CC_SPROM;
 	u16 *sprom;
-	u32 sromctrl;
 	int err = 0;
 
 	if (!bus->drv_cc.core)
 		return -EOPNOTSUPP;
 
-	if (!(bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM))
-		return -ENOENT;
+	if (!bcma_sprom_ext_available(bus)) {
+		/*
+		 * External SPROM takes precedence so check
+		 * on-chip OTP only when no external SPROM
+		 * is present.
+		 */
+		if (!bcma_sprom_onchip_available(bus))
+			return -ENOENT;
 
-	if (bus->drv_cc.core->id.rev >= 32) {
-		sromctrl = bcma_read32(bus->drv_cc.core, BCMA_CC_SROM_CONTROL);
-		if (!(sromctrl & BCMA_CC_SROM_CONTROL_PRESENT))
+		/* determine offset */
+		offset = bcma_sprom_onchip_offset(bus);
+		if (!offset)
 			return -ENOENT;
 	}
 
@@ -273,11 +372,6 @@ int bcma_sprom_get(struct bcma_bus *bus)
 	if (bus->chipinfo.id == 0x4331)
 		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, false);
 
-	/* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
-	 * According to brcm80211 this applies to cards with PCIe rev >= 6
-	 * TODO: understand this condition and use it */
-	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
-		BCMA_CC_SPROM_PCIE6;
 	pr_debug("SPROM offset 0x%x\n", offset);
 	bcma_sprom_read(bus, offset, sprom);
 
diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
index e72938b..d963510 100644
--- a/include/linux/bcma/bcma_driver_chipcommon.h
+++ b/include/linux/bcma/bcma_driver_chipcommon.h
@@ -56,6 +56,9 @@
 #define	 BCMA_CC_OTPS_HW_PROTECT	0x00000001
 #define	 BCMA_CC_OTPS_SW_PROTECT	0x00000002
 #define	 BCMA_CC_OTPS_CID_PROTECT	0x00000004
+#define  BCMA_CC_OTPS_GU_PROG_IND	0x00000F00	/* General Use programmed indication */
+#define  BCMA_CC_OTPS_GU_PROG_IND_SHIFT	8
+#define  BCMA_CC_OTPS_GU_PROG_HW	0x00000100	/* HW region programmed */
 #define BCMA_CC_OTPC			0x0014		/* OTP control */
 #define	 BCMA_CC_OTPC_RECWAIT		0xFF000000
 #define	 BCMA_CC_OTPC_PROGWAIT		0x00FFFF00
@@ -72,6 +75,8 @@
 #define	 BCMA_CC_OTPP_READ		0x40000000
 #define	 BCMA_CC_OTPP_START		0x80000000
 #define	 BCMA_CC_OTPP_BUSY		0x80000000
+#define BCMA_CC_OTPL			0x001C		/* OTP layout */
+#define  BCMA_CC_OTPL_GURGN_OFFSET	0x00000FFF	/* offset of general use region */
 #define BCMA_CC_IRQSTAT			0x0020
 #define BCMA_CC_IRQMASK			0x0024
 #define	 BCMA_CC_IRQ_GPIO		0x00000001	/* gpio intr */
@@ -79,6 +84,10 @@
 #define	 BCMA_CC_IRQ_WDRESET		0x80000000	/* watchdog reset occurred */
 #define BCMA_CC_CHIPCTL			0x0028		/* Rev >= 11 only */
 #define BCMA_CC_CHIPSTAT		0x002C		/* Rev >= 11 only */
+#define  BCMA_CC_CHIPST_4313_SPROM_PRESENT	1
+#define  BCMA_CC_CHIPST_4313_OTP_PRESENT	2
+#define  BCMA_CC_CHIPST_4331_SPROM_PRESENT	2
+#define	 BCMA_CC_CHIPST_4331_OTP_PRESENT	4
 #define BCMA_CC_JCMD			0x0030		/* Rev >= 10 only */
 #define  BCMA_CC_JCMD_START		0x80000000
 #define  BCMA_CC_JCMD_BUSY		0x80000000
-- 
1.7.5.4

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Saul St. John]

On Thu, Feb 23, 2012 at 10:52:57PM +0100, Arend van Spriel wrote:
> Wireless Broadcom chips can have either their SPROM data stored
> on either external SPROM or on-chip OTP memory. Both are accessed
> through the same register space. This patch adds support for the
> on-chip OTP memory.
> 
> Tested with:
> BCM43224 OTP and SPROM
> BCM4331 SPROM
> BCM4313 OTP
> 
> This patch is in response so gmane article [1].
> 
> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
> 
Tested-by: Saul St. John <saul.stjohn@gmail.com>
> Cc: Rafal Milecki <zajec5@gmail.com>
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
> Determining the offset for OTP sprom data turned out to be
> easier as it boils down to reading a register. This change
> collides with patch posted by Hauke:
> 
> bcma: add support for sprom not found on the device.
> 
> Now working on changes in brcmsmac to start using the sprom
> data stored in struct bcma_bus. Feel free to comment this patch.
> 
> Gr. AvS

This works for me. Do you still need to define BCMA_CC_SPROM_PCIE6?

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/24/2012 03:42 AM, Saul St. John wrote:
> On Thu, Feb 23, 2012 at 10:52:57PM +0100, Arend van Spriel wrote:
>> Wireless Broadcom chips can have either their SPROM data stored
>> on either external SPROM or on-chip OTP memory. Both are accessed
>> through the same register space. This patch adds support for the
>> on-chip OTP memory.
>>
>> Tested with:
>> BCM43224 OTP and SPROM
>> BCM4331 SPROM
>> BCM4313 OTP
>>
>> This patch is in response so gmane article [1].
>>
>> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
>>
> Tested-by: Saul St. John<saul.stjohn@gmail.com>
>> Cc: Rafal Milecki<zajec5@gmail.com>
>> Cc: Hauke Mehrtens<hauke@hauke-m.de>
>> Cc: Larry Finger<Larry.Finger@lwfinger.net>
>> Signed-off-by: Arend van Spriel<arend@broadcom.com>
>> ---
>> Determining the offset for OTP sprom data turned out to be
>> easier as it boils down to reading a register. This change
>> collides with patch posted by Hauke:
>>
>> bcma: add support for sprom not found on the device.
>>
>> Now working on changes in brcmsmac to start using the sprom
>> data stored in struct bcma_bus. Feel free to comment this patch.
>>
>> Gr. AvS
>
> This works for me. Do you still need to define BCMA_CC_SPROM_PCIE6?
>

Nope. You are right. I will remove that define in the final patch.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Rafał Miłecki]

2012/2/23 Arend van Spriel <arend@broadcom.com>:
> Wireless Broadcom chips can have either their SPROM data stored
> on either external SPROM or on-chip OTP memory. Both are accessed
> through the same register space. This patch adds support for the
> on-chip OTP memory.
>
> Tested with:
> BCM43224 OTP and SPROM
> BCM4331 SPROM
> BCM4313 OTP
>
> This patch is in response so gmane article [1].
>
> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426

Great, thanks a lot for your work! I'll give it a try with my cards.

May I ask how did you test this with BCM4331? What card (slot) /
machine did you use for your test?


> +       if (bus->drv_cc.core->id.rev >= 31) {
> +               if (!(bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM))
> +                               return false;

One less indent will be fine ;)


> +               srom_control = bcma_read32(bus->drv_cc.core,
> +                                          BCMA_CC_SROM_CONTROL);
> +               return !!(srom_control & BCMA_CC_SROM_CONTROL_PRESENT);

Does any compiler complain on returning sth like 0xF as a bool?


> +       return (chip_status & present_mask) == present_mask;

Same :)


> +       u16 offset = BCMA_CC_SPROM;

I guess we can drop second define offset now?


-- 
Rafał

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/24/2012 08:52 AM, Rafał Miłecki wrote:
> 2012/2/23 Arend van Spriel<arend@broadcom.com>:
>> Wireless Broadcom chips can have either their SPROM data stored
>> on either external SPROM or on-chip OTP memory. Both are accessed
>> through the same register space. This patch adds support for the
>> on-chip OTP memory.
>>
>> Tested with:
>> BCM43224 OTP and SPROM
>> BCM4331 SPROM
>> BCM4313 OTP
>>
>> This patch is in response so gmane article [1].
>>
>> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
>
> Great, thanks a lot for your work! I'll give it a try with my cards.
>
> May I ask how did you test this with BCM4331? What card (slot) /
> machine did you use for your test?

It is a half mini PCIe card used in my test laptop, ie. Dell Latitude E6410.

>> +       if (bus->drv_cc.core->id.rev>= 31) {
>> +               if (!(bus->drv_cc.capabilities&  BCMA_CC_CAP_SPROM))
>> +                               return false;
>
> One less indent will be fine ;)
>

Let's blame my editor :-p. Will fix it.

>> +               srom_control = bcma_read32(bus->drv_cc.core,
>> +                                          BCMA_CC_SROM_CONTROL);
>> +               return !!(srom_control&  BCMA_CC_SROM_CONTROL_PRESENT);
>
> Does any compiler complain on returning sth like 0xF as a bool?
>

Probably not. Just being overly correct, I guess.

>> +       return (chip_status&  present_mask) == present_mask;
>
> Same :)
>

Same.

>
>> +       u16 offset = BCMA_CC_SPROM;
>
> I guess we can drop second define offset now?
>

Yes. Will do that?

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/24/2012 08:52 AM, Rafał Miłecki wrote:
>> +               srom_control = bcma_read32(bus->drv_cc.core,
>> >  +                                          BCMA_CC_SROM_CONTROL);
>> >  +               return !!(srom_control&  BCMA_CC_SROM_CONTROL_PRESENT);
> Does any compiler complain on returning sth like 0xF as a bool?
>

The compiler probably will not complain, but the caller could have 
following:

if (bcma_sprom_ext_available(bus) == true) {
	return;
}
BUG();

I guess you would see the BUG show up in your log with the function 
returning 0xF.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Johannes Berg]

On Fri, 2012-02-24 at 11:39 +0100, Arend van Spriel wrote:
> On 02/24/2012 08:52 AM, Rafał Miłecki wrote:
> >> +               srom_control = bcma_read32(bus->drv_cc.core,
> >> >  +                                          BCMA_CC_SROM_CONTROL);
> >> >  +               return !!(srom_control&  BCMA_CC_SROM_CONTROL_PRESENT);
> > Does any compiler complain on returning sth like 0xF as a bool?
> >
> 
> The compiler probably will not complain, but the caller could have 
> following:
> 
> if (bcma_sprom_ext_available(bus) == true) {
> 	return;
> }
> BUG();
> 
> I guess you would see the BUG show up in your log with the function 
> returning 0xF.

If it's really

bool foo = a & b;

then the compiler has to compile that as if it was

u8 foo = !!(a & b);

since bool can only carry the values 0 and 1.

So the !! isn't necessary.

johannes

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/24/2012 11:58 AM, Johannes Berg wrote:
> On Fri, 2012-02-24 at 11:39 +0100, Arend van Spriel wrote:
>> On 02/24/2012 08:52 AM, Rafał Miłecki wrote:
>>>> +               srom_control = bcma_read32(bus->drv_cc.core,
>>>>>   +                                          BCMA_CC_SROM_CONTROL);
>>>>>   +               return !!(srom_control&   BCMA_CC_SROM_CONTROL_PRESENT);
>>> Does any compiler complain on returning sth like 0xF as a bool?
>>>
>>
>> The compiler probably will not complain, but the caller could have
>> following:
>>
>> if (bcma_sprom_ext_available(bus) == true) {
>> 	return;
>> }
>> BUG();
>>
>> I guess you would see the BUG show up in your log with the function
>> returning 0xF.
>
> If it's really
>
> bool foo = a&  b;
>
> then the compiler has to compile that as if it was
>
> u8 foo = !!(a&  b);
>
> since bool can only carry the values 0 and 1.
>
> So the !! isn't necessary.
>
> johannes
>
>

I guess the compiler will convert the return value to _Bool (aka. bool) 
so indeed !! is not  necessary. I will remove it.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Hauke Mehrtens]

On 02/23/2012 10:52 PM, Arend van Spriel wrote:
> Wireless Broadcom chips can have either their SPROM data stored
> on either external SPROM or on-chip OTP memory. Both are accessed
> through the same register space. This patch adds support for the
> on-chip OTP memory.
> 
> Tested with:
> BCM43224 OTP and SPROM
> BCM4331 SPROM
> BCM4313 OTP

Does bcma now support the same features regarding sprom and otp as
brcmsamc expect it does not read out all the attributes brcmsmac reads
out or are there any features still missing? I am just asking because
the code used in brcmsmac is ~4 times longer.

> This patch is in response so gmane article [1].
> 
> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
> 
> Cc: Saul St. John <saul.stjohn@gmail.com>
> Cc: Rafal Milecki <zajec5@gmail.com>
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
> Determining the offset for OTP sprom data turned out to be
> easier as it boils down to reading a register. This change
> collides with patch posted by Hauke:

I will test you patch on my device soon and will report if something is
wrong. If you are sending a non RFC patch in the next days I would
rebase my patch onto yours. The code searching in the SoCs flash chip
will be added to run if bcma_sprom_onchip_available() returns false.

> bcma: add support for sprom not found on the device.
> 
> Now working on changes in brcmsmac to start using the sprom
> data stored in struct bcma_bus. Feel free to comment this patch.
> 
> Gr. AvS
> ---
>  drivers/bcma/sprom.c                        |  118 ++++++++++++++++++++++++---
>  include/linux/bcma/bcma_driver_chipcommon.h |    9 ++
>  2 files changed, 115 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
> index ca77525..4dd537c 100644
> --- a/drivers/bcma/sprom.c
> +++ b/drivers/bcma/sprom.c
> @@ -246,22 +246,121 @@ static void bcma_sprom_extract_r8(struct bcma_bus *bus, const u16 *sprom)
>  	     SSB_SROM8_FEM_ANTSWLUT_SHIFT);
>  }
>  
> +/*
> + * Indicates the presence of external SPROM.
> + */
> +static bool bcma_sprom_ext_available(struct bcma_bus *bus)
> +{
> +	u32 chip_status;
> +	u32 srom_control;
> +	u32 present_mask;
> +
> +	if (bus->drv_cc.core->id.rev >= 31) {
> +		if (!(bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM))
> +				return false;
> +
> +		srom_control = bcma_read32(bus->drv_cc.core,
> +					   BCMA_CC_SROM_CONTROL);
> +		return !!(srom_control & BCMA_CC_SROM_CONTROL_PRESENT);
> +	}
> +
> +	/* older chipcommon revisions use chip status register */
> +	chip_status = bcma_read32(bus->drv_cc.core, BCMA_CC_CHIPSTAT);
> +	switch (bus->chipinfo.id) {
> +	case 0x4313:
> +		present_mask = BCMA_CC_CHIPST_4313_SPROM_PRESENT;
> +		break;
> +
> +	case 0x4331:
> +		present_mask = BCMA_CC_CHIPST_4331_SPROM_PRESENT;
> +		break;
> +
> +	default:
> +		return true;
> +	}
> +
> +	return (chip_status & present_mask) == present_mask;
> +}
> +
> +/*
> + * Indicates that on-chip OTP memory is present and enabled.
> + */
> +static bool bcma_sprom_onchip_available(struct bcma_bus *bus)
> +{
> +	u32 chip_status;
> +	u32 otpsize = 0;
> +	bool present;
> +
> +	chip_status = bcma_read32(bus->drv_cc.core, BCMA_CC_CHIPSTAT);
> +	switch (bus->chipinfo.id) {
> +	case 0x4313:
> +		present = chip_status & BCMA_CC_CHIPST_4313_OTP_PRESENT;
> +		break;
> +
> +	case 0x4331:
> +		present = chip_status & BCMA_CC_CHIPST_4331_OTP_PRESENT;
> +		break;
> +
> +	case 43224:
> +	case 43225:
> +		/* for these chips OTP is always available */
> +		present = true;
> +		break;
> +
> +	default:
> +		present = false;
> +		break;
> +	}
> +
> +	if (present) {
> +		otpsize = bus->drv_cc.capabilities & BCMA_CC_CAP_OTPS;
> +		otpsize >>= BCMA_CC_CAP_OTPS_SHIFT;
> +	}
> +
> +	return otpsize != 0;
> +}
> +
> +/*
> + * Verify OTP is filled and determine the byte
> + * offset where SPROM data is located.
> + *
> + * On error, returns 0; byte offset otherwise.
> + */
> +static int bcma_sprom_onchip_offset(struct bcma_bus *bus)
> +{
> +	struct bcma_device *cc = bus->drv_cc.core;
> +	u32 offset;
> +
> +	/* verify OTP status */
> +	if ((bcma_read32(cc, BCMA_CC_OTPS) & BCMA_CC_OTPS_GU_PROG_HW) == 0)
> +		return 0;
> +
> +	/* obtain bit offset from otplayout register */
> +	offset = (bcma_read32(cc, BCMA_CC_OTPL) & BCMA_CC_OTPL_GURGN_OFFSET);
> +	return BCMA_CC_SPROM + (offset >> 3);
> +}
> +
>  int bcma_sprom_get(struct bcma_bus *bus)
>  {
> -	u16 offset;
> +	u16 offset = BCMA_CC_SPROM;
>  	u16 *sprom;
> -	u32 sromctrl;
>  	int err = 0;
>  
>  	if (!bus->drv_cc.core)
>  		return -EOPNOTSUPP;
>  
> -	if (!(bus->drv_cc.capabilities & BCMA_CC_CAP_SPROM))
> -		return -ENOENT;
> +	if (!bcma_sprom_ext_available(bus)) {
> +		/*
> +		 * External SPROM takes precedence so check
> +		 * on-chip OTP only when no external SPROM
> +		 * is present.
> +		 */
> +		if (!bcma_sprom_onchip_available(bus))
> +			return -ENOENT;
>  
> -	if (bus->drv_cc.core->id.rev >= 32) {
> -		sromctrl = bcma_read32(bus->drv_cc.core, BCMA_CC_SROM_CONTROL);
> -		if (!(sromctrl & BCMA_CC_SROM_CONTROL_PRESENT))
> +		/* determine offset */
> +		offset = bcma_sprom_onchip_offset(bus);
> +		if (!offset)
>  			return -ENOENT;
>  	}
>  
> @@ -273,11 +372,6 @@ int bcma_sprom_get(struct bcma_bus *bus)
>  	if (bus->chipinfo.id == 0x4331)
>  		bcma_chipco_bcm4331_ext_pa_lines_ctl(&bus->drv_cc, false);
>  
> -	/* Most cards have SPROM moved by additional offset 0x30 (48 dwords).
> -	 * According to brcm80211 this applies to cards with PCIe rev >= 6
> -	 * TODO: understand this condition and use it */
> -	offset = (bus->chipinfo.id == 0x4331) ? BCMA_CC_SPROM :
> -		BCMA_CC_SPROM_PCIE6;
>  	pr_debug("SPROM offset 0x%x\n", offset);
>  	bcma_sprom_read(bus, offset, sprom);
>  
> diff --git a/include/linux/bcma/bcma_driver_chipcommon.h b/include/linux/bcma/bcma_driver_chipcommon.h
> index e72938b..d963510 100644
> --- a/include/linux/bcma/bcma_driver_chipcommon.h
> +++ b/include/linux/bcma/bcma_driver_chipcommon.h
> @@ -56,6 +56,9 @@
>  #define	 BCMA_CC_OTPS_HW_PROTECT	0x00000001
>  #define	 BCMA_CC_OTPS_SW_PROTECT	0x00000002
>  #define	 BCMA_CC_OTPS_CID_PROTECT	0x00000004
> +#define  BCMA_CC_OTPS_GU_PROG_IND	0x00000F00	/* General Use programmed indication */
> +#define  BCMA_CC_OTPS_GU_PROG_IND_SHIFT	8
> +#define  BCMA_CC_OTPS_GU_PROG_HW	0x00000100	/* HW region programmed */
>  #define BCMA_CC_OTPC			0x0014		/* OTP control */
>  #define	 BCMA_CC_OTPC_RECWAIT		0xFF000000
>  #define	 BCMA_CC_OTPC_PROGWAIT		0x00FFFF00
> @@ -72,6 +75,8 @@
>  #define	 BCMA_CC_OTPP_READ		0x40000000
>  #define	 BCMA_CC_OTPP_START		0x80000000
>  #define	 BCMA_CC_OTPP_BUSY		0x80000000
> +#define BCMA_CC_OTPL			0x001C		/* OTP layout */
> +#define  BCMA_CC_OTPL_GURGN_OFFSET	0x00000FFF	/* offset of general use region */
>  #define BCMA_CC_IRQSTAT			0x0020
>  #define BCMA_CC_IRQMASK			0x0024
>  #define	 BCMA_CC_IRQ_GPIO		0x00000001	/* gpio intr */
> @@ -79,6 +84,10 @@
>  #define	 BCMA_CC_IRQ_WDRESET		0x80000000	/* watchdog reset occurred */
>  #define BCMA_CC_CHIPCTL			0x0028		/* Rev >= 11 only */
>  #define BCMA_CC_CHIPSTAT		0x002C		/* Rev >= 11 only */
> +#define  BCMA_CC_CHIPST_4313_SPROM_PRESENT	1
> +#define  BCMA_CC_CHIPST_4313_OTP_PRESENT	2
> +#define  BCMA_CC_CHIPST_4331_SPROM_PRESENT	2
> +#define	 BCMA_CC_CHIPST_4331_OTP_PRESENT	4
>  #define BCMA_CC_JCMD			0x0030		/* Rev >= 10 only */
>  #define  BCMA_CC_JCMD_START		0x80000000
>  #define  BCMA_CC_JCMD_BUSY		0x80000000

What is the correct way to format this file? BCMA_CC_JCMD_BUSY uses two
spaces after the define and BCMA_CC_OTPS_CID_PROTECT uses a tabulator
and a space, what is the correct or intended way to format this? This
does not have directly something to do with this patches as both ways
are currently coded in this file.

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Rafał Miłecki]

2012/2/25 Hauke Mehrtens <hauke@hauke-m.de>:
> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>> +#define BCMA_CC_OTPL                 0x001C          /* OTP layout */
>> +#define  BCMA_CC_OTPL_GURGN_OFFSET   0x00000FFF      /* offset of general use region */
>>  #define BCMA_CC_IRQSTAT                      0x0020
>>  #define BCMA_CC_IRQMASK                      0x0024
>>  #define       BCMA_CC_IRQ_GPIO               0x00000001      /* gpio intr */
>> @@ -79,6 +84,10 @@
>>  #define       BCMA_CC_IRQ_WDRESET            0x80000000      /* watchdog reset occurred */
>>  #define BCMA_CC_CHIPCTL                      0x0028          /* Rev >= 11 only */
>>  #define BCMA_CC_CHIPSTAT             0x002C          /* Rev >= 11 only */
>> +#define  BCMA_CC_CHIPST_4313_SPROM_PRESENT   1
>> +#define  BCMA_CC_CHIPST_4313_OTP_PRESENT     2
>> +#define  BCMA_CC_CHIPST_4331_SPROM_PRESENT   2
>> +#define       BCMA_CC_CHIPST_4331_OTP_PRESENT        4
>>  #define BCMA_CC_JCMD                 0x0030          /* Rev >= 10 only */
>>  #define  BCMA_CC_JCMD_START          0x80000000
>>  #define  BCMA_CC_JCMD_BUSY           0x80000000
>
> What is the correct way to format this file? BCMA_CC_JCMD_BUSY uses two
> spaces after the define and BCMA_CC_OTPS_CID_PROTECT uses a tabulator
> and a space, what is the correct or intended way to format this? This
> does not have directly something to do with this patches as both ways
> are currently coded in this file.

I tried to use two spaces, I didn't mean to add tabs after "define".
If I did, it was done by a mistake.

I also used to use hexadecimal numbers for values (like
BCMA_CC_CHIPST_4331_OTP_PRESENT):
0x00000004 rather than 4

I don't really have any arguments for that choice, I've just copied
ssb (and b43?) coding style. However keeping one styles sounds
reasonable.

-- 
Rafał

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>> Wireless Broadcom chips can have either their SPROM data stored
>> on either external SPROM or on-chip OTP memory. Both are accessed
>> through the same register space. This patch adds support for the
>> on-chip OTP memory.
>>
>> Tested with:
>> BCM43224 OTP and SPROM
>> BCM4331 SPROM
>> BCM4313 OTP
>
> Does bcma now support the same features regarding sprom and otp as
> brcmsamc expect it does not read out all the attributes brcmsmac reads
> out or are there any features still missing? I am just asking because
> the code used in brcmsmac is ~4 times longer.

I started on using bcma sprom content in brcmsmac and indeed there are 
some entries missing. The change in this patch only provides read-access 
to the srom data. As the chip comes up for read-access there is not much 
programming need to accomplish that. The only feature that is not there 
is that on some chips OTP can be powered down for power-saving. The 
current chips supported by BCMA don't have that.

>> This patch is in response so gmane article [1].
>>
>> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
>>
>> Cc: Saul St. John<saul.stjohn@gmail.com>
>> Cc: Rafal Milecki<zajec5@gmail.com>
>> Cc: Hauke Mehrtens<hauke@hauke-m.de>
>> Cc: Larry Finger<Larry.Finger@lwfinger.net>
>> Signed-off-by: Arend van Spriel<arend@broadcom.com>
>> ---
>> Determining the offset for OTP sprom data turned out to be
>> easier as it boils down to reading a register. This change
>> collides with patch posted by Hauke:
>
> I will test you patch on my device soon and will report if something is
> wrong. If you are sending a non RFC patch in the next days I would
> rebase my patch onto yours. The code searching in the SoCs flash chip
> will be added to run if bcma_sprom_onchip_available() returns false.

Appreciate any testing on SoCs. I think I will need some time to modify 
brcmsmac so let your patch go first.

>> bcma: add support for sprom not found on the device.
>>
>> Now working on changes in brcmsmac to start using the sprom
>> data stored in struct bcma_bus. Feel free to comment this patch.
>>
>> Gr. AvS
>> ---
>>   drivers/bcma/sprom.c                        |  118 ++++++++++++++++++++++++---
>>   include/linux/bcma/bcma_driver_chipcommon.h |    9 ++
>>   2 files changed, 115 insertions(+), 12 deletions(-)
>>
>> +#define BCMA_CC_OTPL			0x001C		/* OTP layout */
>> +#define  BCMA_CC_OTPL_GURGN_OFFSET	0x00000FFF	/* offset of general use region */
>>   #define BCMA_CC_IRQSTAT			0x0020
>>   #define BCMA_CC_IRQMASK			0x0024
>>   #define	 BCMA_CC_IRQ_GPIO		0x00000001	/* gpio intr */
>> @@ -79,6 +84,10 @@
>>   #define	 BCMA_CC_IRQ_WDRESET		0x80000000	/* watchdog reset occurred */
>>   #define BCMA_CC_CHIPCTL			0x0028		/* Rev>= 11 only */
>>   #define BCMA_CC_CHIPSTAT		0x002C		/* Rev>= 11 only */
>> +#define  BCMA_CC_CHIPST_4313_SPROM_PRESENT	1
>> +#define  BCMA_CC_CHIPST_4313_OTP_PRESENT	2
>> +#define  BCMA_CC_CHIPST_4331_SPROM_PRESENT	2
>> +#define	 BCMA_CC_CHIPST_4331_OTP_PRESENT	4
>>   #define BCMA_CC_JCMD			0x0030		/* Rev>= 10 only */
>>   #define  BCMA_CC_JCMD_START		0x80000000
>>   #define  BCMA_CC_JCMD_BUSY		0x80000000
>
> What is the correct way to format this file? BCMA_CC_JCMD_BUSY uses two
> spaces after the define and BCMA_CC_OTPS_CID_PROTECT uses a tabulator
> and a space, what is the correct or intended way to format this? This
> does not have directly something to do with this patches as both ways
> are currently coded in this file.

I assumed the convention was to use two spaces and I corrected 
BCMA_CC_CHIPST_4331_OTP_PRESENT accordingly after reading back my RFC patch.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Hauke Mehrtens]

On 02/27/2012 11:12 AM, Arend van Spriel wrote:
> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>> Wireless Broadcom chips can have either their SPROM data stored
>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>> through the same register space. This patch adds support for the
>>> on-chip OTP memory.
>>>
>>> Tested with:
>>> BCM43224 OTP and SPROM
>>> BCM4331 SPROM
>>> BCM4313 OTP
>>
>> Does bcma now support the same features regarding sprom and otp as
>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>> out or are there any features still missing? I am just asking because
>> the code used in brcmsmac is ~4 times longer.
> 
> I started on using bcma sprom content in brcmsmac and indeed there are
> some entries missing. The change in this patch only provides read-access
> to the srom data. As the chip comes up for read-access there is not much
> programming need to accomplish that. The only feature that is not there
> is that on some chips OTP can be powered down for power-saving. The
> current chips supported by BCMA don't have that.
> 
>>> This patch is in response so gmane article [1].
>>>
>>> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
>>>
>>> Cc: Saul St. John<saul.stjohn@gmail.com>
>>> Cc: Rafal Milecki<zajec5@gmail.com>
>>> Cc: Hauke Mehrtens<hauke@hauke-m.de>
>>> Cc: Larry Finger<Larry.Finger@lwfinger.net>
>>> Signed-off-by: Arend van Spriel<arend@broadcom.com>
>>> ---
>>> Determining the offset for OTP sprom data turned out to be
>>> easier as it boils down to reading a register. This change
>>> collides with patch posted by Hauke:
>>
>> I will test you patch on my device soon and will report if something is
>> wrong. If you are sending a non RFC patch in the next days I would
>> rebase my patch onto yours. The code searching in the SoCs flash chip
>> will be added to run if bcma_sprom_onchip_available() returns false.
> 
> Appreciate any testing on SoCs. I think I will need some time to modify
> brcmsmac so let your patch go first.

The sprom part of my SoC is working with this patch on top of my sprom
patches, but it uses the sprom from flash/nvram for both wifi devices
(one integrated in the bCM4716 and the other a BCM43224 connected to the
PCIe host controller of the BCM4716).
For my BCM4716 bcma_sprom_ext_available() and
bcma_sprom_onchip_available() are returning false and for the BCM43224
bcma_sprom_ext_available() is returning false and
bcma_sprom_onchip_offset() 0.
> 
>>> bcma: add support for sprom not found on the device.
>>>
>>> Now working on changes in brcmsmac to start using the sprom
>>> data stored in struct bcma_bus. Feel free to comment this patch.
>>>
>>> Gr. AvS
>>> ---
>>>   drivers/bcma/sprom.c                        |  118
>>> ++++++++++++++++++++++++---
>>>   include/linux/bcma/bcma_driver_chipcommon.h |    9 ++
>>>   2 files changed, 115 insertions(+), 12 deletions(-)
>>>
>>> +#define BCMA_CC_OTPL            0x001C        /* OTP layout */
>>> +#define  BCMA_CC_OTPL_GURGN_OFFSET    0x00000FFF    /* offset of
>>> general use region */
>>>   #define BCMA_CC_IRQSTAT            0x0020
>>>   #define BCMA_CC_IRQMASK            0x0024
>>>   #define     BCMA_CC_IRQ_GPIO        0x00000001    /* gpio intr */
>>> @@ -79,6 +84,10 @@
>>>   #define     BCMA_CC_IRQ_WDRESET        0x80000000    /* watchdog
>>> reset occurred */
>>>   #define BCMA_CC_CHIPCTL            0x0028        /* Rev>= 11 only */
>>>   #define BCMA_CC_CHIPSTAT        0x002C        /* Rev>= 11 only */
>>> +#define  BCMA_CC_CHIPST_4313_SPROM_PRESENT    1
>>> +#define  BCMA_CC_CHIPST_4313_OTP_PRESENT    2
>>> +#define  BCMA_CC_CHIPST_4331_SPROM_PRESENT    2
>>> +#define     BCMA_CC_CHIPST_4331_OTP_PRESENT    4
>>>   #define BCMA_CC_JCMD            0x0030        /* Rev>= 10 only */
>>>   #define  BCMA_CC_JCMD_START        0x80000000
>>>   #define  BCMA_CC_JCMD_BUSY        0x80000000
>>
>> What is the correct way to format this file? BCMA_CC_JCMD_BUSY uses two
>> spaces after the define and BCMA_CC_OTPS_CID_PROTECT uses a tabulator
>> and a space, what is the correct or intended way to format this? This
>> does not have directly something to do with this patches as both ways
>> are currently coded in this file.
> 
> I assumed the convention was to use two spaces and I corrected
> BCMA_CC_CHIPST_4331_OTP_PRESENT accordingly after reading back my RFC
> patch.
> 
> Gr. AvS
> 

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/28/2012 09:11 PM, Hauke Mehrtens wrote:
> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>> I will test you patch on my device soon and will report if something is
>>> wrong. If you are sending a non RFC patch in the next days I would
>>> rebase my patch onto yours. The code searching in the SoCs flash chip
>>> will be added to run if bcma_sprom_onchip_available() returns false.
>>
>> Appreciate any testing on SoCs. I think I will need some time to modify
>> brcmsmac so let your patch go first.
>
> The sprom part of my SoC is working with this patch on top of my sprom
> patches, but it uses the sprom from flash/nvram for both wifi devices
> (one integrated in the bCM4716 and the other a BCM43224 connected to the
> PCIe host controller of the BCM4716).
> For my BCM4716 bcma_sprom_ext_available() and
> bcma_sprom_onchip_available() are returning false and for the BCM43224
> bcma_sprom_ext_available() is returning false and
> bcma_sprom_onchip_offset() 0.

Thanks, Hauke

Did the BCM43224 come with the router. If so I am assuming the onchip is 
not used and the system relies on flash/nvram completely. I will inform 
whether my assumption is correct.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Hauke Mehrtens]

On 03/01/2012 03:12 PM, Arend van Spriel wrote:
> On 02/28/2012 09:11 PM, Hauke Mehrtens wrote:
>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>>> I will test you patch on my device soon and will report if something is
>>>> wrong. If you are sending a non RFC patch in the next days I would
>>>> rebase my patch onto yours. The code searching in the SoCs flash chip
>>>> will be added to run if bcma_sprom_onchip_available() returns false.
>>>
>>> Appreciate any testing on SoCs. I think I will need some time to modify
>>> brcmsmac so let your patch go first.
>>
>> The sprom part of my SoC is working with this patch on top of my sprom
>> patches, but it uses the sprom from flash/nvram for both wifi devices
>> (one integrated in the bCM4716 and the other a BCM43224 connected to the
>> PCIe host controller of the BCM4716).
>> For my BCM4716 bcma_sprom_ext_available() and
>> bcma_sprom_onchip_available() are returning false and for the BCM43224
>> bcma_sprom_ext_available() is returning false and
>> bcma_sprom_onchip_offset() 0.
> 
> Thanks, Hauke
> 
> Did the BCM43224 come with the router. If so I am assuming the onchip is
> not used and the system relies on flash/nvram completely. I will inform
> whether my assumption is correct.
Yes the BCM43224 is direly soldered onto the board where also the
BCM4716 is on, there is no physical PCIe port or something like this.
The nvram on the flash chip contains the sprom for this devices, one set
of variables in the nvram has the prefix pci/1/1/ (for the BCM43224 with
the options for 5GHz only) and the other has sb/1/ (for the BCM4716 with
the options for 2.4 GHz only).

It is this device: http://infodepot.wikia.com/wiki/Netgear_WNDR3400

Hauke

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/01/2012 03:35 PM, Hauke Mehrtens wrote:
> On 03/01/2012 03:12 PM, Arend van Spriel wrote:
>> On 02/28/2012 09:11 PM, Hauke Mehrtens wrote:
>>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>>>> I will test you patch on my device soon and will report if something is
>>>>> wrong. If you are sending a non RFC patch in the next days I would
>>>>> rebase my patch onto yours. The code searching in the SoCs flash chip
>>>>> will be added to run if bcma_sprom_onchip_available() returns false.
>>>>
>>>> Appreciate any testing on SoCs. I think I will need some time to modify
>>>> brcmsmac so let your patch go first.
>>>
>>> The sprom part of my SoC is working with this patch on top of my sprom
>>> patches, but it uses the sprom from flash/nvram for both wifi devices
>>> (one integrated in the bCM4716 and the other a BCM43224 connected to the
>>> PCIe host controller of the BCM4716).
>>> For my BCM4716 bcma_sprom_ext_available() and
>>> bcma_sprom_onchip_available() are returning false and for the BCM43224
>>> bcma_sprom_ext_available() is returning false and
>>> bcma_sprom_onchip_offset() 0.
>>
>> Thanks, Hauke
>>
>> Did the BCM43224 come with the router. If so I am assuming the onchip is
>> not used and the system relies on flash/nvram completely. I will inform
>> whether my assumption is correct.
> Yes the BCM43224 is direly soldered onto the board where also the
> BCM4716 is on, there is no physical PCIe port or something like this.
> The nvram on the flash chip contains the sprom for this devices, one set
> of variables in the nvram has the prefix pci/1/1/ (for the BCM43224 with
> the options for 5GHz only) and the other has sb/1/ (for the BCM4716 with
> the options for 2.4 GHz only).
>
> It is this device: http://infodepot.wikia.com/wiki/Netgear_WNDR3400
>
> Hauke
>

You made me curious so I followed the link. It says BCM4718.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Hauke Mehrtens]

On 03/01/2012 04:16 PM, Arend van Spriel wrote:
> On 03/01/2012 03:35 PM, Hauke Mehrtens wrote:
>> On 03/01/2012 03:12 PM, Arend van Spriel wrote:
>>> On 02/28/2012 09:11 PM, Hauke Mehrtens wrote:
>>>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>>>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>>>>> I will test you patch on my device soon and will report if
>>>>>> something is
>>>>>> wrong. If you are sending a non RFC patch in the next days I would
>>>>>> rebase my patch onto yours. The code searching in the SoCs flash chip
>>>>>> will be added to run if bcma_sprom_onchip_available() returns false.
>>>>>
>>>>> Appreciate any testing on SoCs. I think I will need some time to
>>>>> modify
>>>>> brcmsmac so let your patch go first.
>>>>
>>>> The sprom part of my SoC is working with this patch on top of my sprom
>>>> patches, but it uses the sprom from flash/nvram for both wifi devices
>>>> (one integrated in the bCM4716 and the other a BCM43224 connected to
>>>> the
>>>> PCIe host controller of the BCM4716).
>>>> For my BCM4716 bcma_sprom_ext_available() and
>>>> bcma_sprom_onchip_available() are returning false and for the BCM43224
>>>> bcma_sprom_ext_available() is returning false and
>>>> bcma_sprom_onchip_offset() 0.
>>>
>>> Thanks, Hauke
>>>
>>> Did the BCM43224 come with the router. If so I am assuming the onchip is
>>> not used and the system relies on flash/nvram completely. I will inform
>>> whether my assumption is correct.
>> Yes the BCM43224 is direly soldered onto the board where also the
>> BCM4716 is on, there is no physical PCIe port or something like this.
>> The nvram on the flash chip contains the sprom for this devices, one set
>> of variables in the nvram has the prefix pci/1/1/ (for the BCM43224 with
>> the options for 5GHz only) and the other has sb/1/ (for the BCM4716 with
>> the options for 2.4 GHz only).
>>
>> It is this device: http://infodepot.wikia.com/wiki/Netgear_WNDR3400
>>
>> Hauke
>>
> 
> You made me curious so I followed the link. It says BCM4718.

The BCM4716, BCM4717 and BCM4718 are the same chips from a driver
developer point of view. ;-) They share the same device id (0x4716),
just the package id is different. The BCM4717 and BCM4718 have a PCIe
host controller, the BCM4716 does not have a PCIe host controller. The
BCM4718 also has a higher CPU clock than the others. The PCIe host
controller is automatically detected and the CPU clock speed is read out
of some register. They are sharing the same workarounds in the driver.
So if I speak of the BCM4716 mostly I mean all of these three or one of
these three. ;-)

Hauke

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Rafał Miłecki]

2012/2/28 Hauke Mehrtens <hauke@hauke-m.de>:
> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>>> Wireless Broadcom chips can have either their SPROM data stored
>>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>>> through the same register space. This patch adds support for the
>>>> on-chip OTP memory.
>>>>
>>>> Tested with:
>>>> BCM43224 OTP and SPROM
>>>> BCM4331 SPROM
>>>> BCM4313 OTP
>>>
>>> Does bcma now support the same features regarding sprom and otp as
>>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>>> out or are there any features still missing? I am just asking because
>>> the code used in brcmsmac is ~4 times longer.
>>
>> I started on using bcma sprom content in brcmsmac and indeed there are
>> some entries missing. The change in this patch only provides read-access
>> to the srom data. As the chip comes up for read-access there is not much
>> programming need to accomplish that. The only feature that is not there
>> is that on some chips OTP can be powered down for power-saving. The
>> current chips supported by BCMA don't have that.
>>
>>>> This patch is in response so gmane article [1].
>>>>
>>>> [1] http://article.gmane.org/gmane.linux.kernel.wireless.general/85426
>>>>
>>>> Cc: Saul St. John<saul.stjohn@gmail.com>
>>>> Cc: Rafal Milecki<zajec5@gmail.com>
>>>> Cc: Hauke Mehrtens<hauke@hauke-m.de>
>>>> Cc: Larry Finger<Larry.Finger@lwfinger.net>
>>>> Signed-off-by: Arend van Spriel<arend@broadcom.com>
>>>> ---
>>>> Determining the offset for OTP sprom data turned out to be
>>>> easier as it boils down to reading a register. This change
>>>> collides with patch posted by Hauke:
>>>
>>> I will test you patch on my device soon and will report if something is
>>> wrong. If you are sending a non RFC patch in the next days I would
>>> rebase my patch onto yours. The code searching in the SoCs flash chip
>>> will be added to run if bcma_sprom_onchip_available() returns false.
>>
>> Appreciate any testing on SoCs. I think I will need some time to modify
>> brcmsmac so let your patch go first.
>
> The sprom part of my SoC is working with this patch on top of my sprom
> patches, but it uses the sprom from flash/nvram for both wifi devices
> (one integrated in the bCM4716 and the other a BCM43224 connected to the
> PCIe host controller of the BCM4716).
> For my BCM4716 bcma_sprom_ext_available() and
> bcma_sprom_onchip_available() are returning false and for the BCM43224
> bcma_sprom_ext_available() is returning false and
> bcma_sprom_onchip_offset() 0.

I guess that's wrong...? So is there something wrong with the Arend's
patch causing this regression? Or was this wrong even earlier?

I'm not sure if I should test this patch against my cards or should I
wait for V2.

-- 
Rafał

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/03/2012 11:44 PM, Rafał Miłecki wrote:
> 2012/2/28 Hauke Mehrtens<hauke@hauke-m.de>:
>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>>
>>> Appreciate any testing on SoCs. I think I will need some time to modify
>>> brcmsmac so let your patch go first.
>>
>> The sprom part of my SoC is working with this patch on top of my sprom
>> patches, but it uses the sprom from flash/nvram for both wifi devices
>> (one integrated in the bCM4716 and the other a BCM43224 connected to the
>> PCIe host controller of the BCM4716).
>> For my BCM4716 bcma_sprom_ext_available() and
>> bcma_sprom_onchip_available() are returning false and for the BCM43224
>> bcma_sprom_ext_available() is returning false and
>> bcma_sprom_onchip_offset() 0.
>
> I guess that's wrong...? So is there something wrong with the Arend's
> patch causing this regression? Or was this wrong even earlier?
>
> I'm not sure if I should test this patch against my cards or should I
> wait for V2.
>

Hi Rafał,

It is not wrong. I asked Hauke a question about his router regarding 
this. The story for routers is that sprom data for both cards resides in 
nvram/flash, ie. the fallback sprom source in Hauke's patch. Not sure 
why that bcma patch has not yet made it into wireless-next. Your cards 
are probably pci(e) cards so those should have sprom data on board.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Rafał Miłecki]

W dniu 5 marca 2012 10:16 użytkownik Arend van Spriel
<arend@broadcom.com> napisał:
> On 03/03/2012 11:44 PM, Rafał Miłecki wrote:
>>
>> 2012/2/28 Hauke Mehrtens<hauke@hauke-m.de>:
>>>
>>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>>>
>>>>
>>>> Appreciate any testing on SoCs. I think I will need some time to modify
>>>> brcmsmac so let your patch go first.
>>>
>>>
>>> The sprom part of my SoC is working with this patch on top of my sprom
>>> patches, but it uses the sprom from flash/nvram for both wifi devices
>>> (one integrated in the bCM4716 and the other a BCM43224 connected to the
>>> PCIe host controller of the BCM4716).
>>> For my BCM4716 bcma_sprom_ext_available() and
>>> bcma_sprom_onchip_available() are returning false and for the BCM43224
>>> bcma_sprom_ext_available() is returning false and
>>> bcma_sprom_onchip_offset() 0.
>>
>>
>> I guess that's wrong...? So is there something wrong with the Arend's
>> patch causing this regression? Or was this wrong even earlier?
>>
>> I'm not sure if I should test this patch against my cards or should I
>> wait for V2.
>>
>
> Hi Rafał,
>
> It is not wrong. I asked Hauke a question about his router regarding this.
> The story for routers is that sprom data for both cards resides in
> nvram/flash, ie. the fallback sprom source in Hauke's patch.

OK, I gave it a try, it works with my BCM43224, nice work.


> Not sure why
> that bcma patch has not yet made it into wireless-next.

Have you sent it in final form and without RFC? I don't think I've received it.

-- 
Rafał

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/06/2012 09:52 AM, Rafał Miłecki wrote:
>
> Have you sent it in final form and without RFC? I don't think I've received it.
>

Hi Rafał,

I did not send it. I have another related RFC that I am working on to 
enable brcmsmac to use the bcma sprom data. Also I was waiting until the 
patches that Hauke sent on bcma were taken by John. I just noticed that 
he did last night so I will send it.

Gr. AvS

[RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/06/2012 09:52 AM, Rafa? Mi?ecki wrote:
>
> Have you sent it in final form and without RFC? I don't think I've received it.
>

Hi Rafa?,

I did not send it. I have another related RFC that I am working on to 
enable brcmsmac to use the bcma sprom data. Also I was waiting until the 
patches that Hauke sent on bcma were taken by John. I just noticed that 
he did last night so I will send it.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 02/27/2012 11:12 AM, Arend van Spriel wrote:
> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>> Wireless Broadcom chips can have either their SPROM data stored
>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>> through the same register space. This patch adds support for the
>>> on-chip OTP memory.
>>>
>>> Tested with:
>>> BCM43224 OTP and SPROM
>>> BCM4331 SPROM
>>> BCM4313 OTP
>>
>> Does bcma now support the same features regarding sprom and otp as
>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>> out or are there any features still missing? I am just asking because
>> the code used in brcmsmac is ~4 times longer.
>
> I started on using bcma sprom content in brcmsmac and indeed there are
> some entries missing. The change in this patch only provides read-access
> to the srom data. As the chip comes up for read-access there is not much
> programming need to accomplish that. The only feature that is not there
> is that on some chips OTP can be powered down for power-saving. The
> current chips supported by BCMA don't have that.

With BCMA retrieving the SPROM data correctly, regardless how/where it 
is stored I went to change brcmsmac to make use of it. However, it 
turned out several attributes were missing.

Now I could go and extend the ssb_sprom structure, but there I got 
confused. The structure is filled by BCMA in the function 
bcma_sprom_extract_r8(). The name suggest it deals with SROM revision 8 
attributes, but:

	for (i = 0; i < 3; i++) {
		v = sprom[SPOFF(SSB_SPROM8_IL0MAC) + i];
		*(((__be16 *)bus->sprom.il0mac) + i) = cpu_to_be16(v);
	}

	SPEX(board_rev, SSB_SPROM8_BOARDREV, ~0, 0);

	SPEX(txpid2g[0], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G0,
	     SSB_SPROM4_TXPID2G0_SHIFT);
	SPEX(txpid2g[1], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G1,
	     SSB_SPROM4_TXPID2G1_SHIFT);

The attributes txpid2g that are filled here are deprecated for srom 
revision 8. Is b43 using these fields for boards with srom rev 8?

I propose to move to BCMA having its own sprom structure definition. 
Srom revision 10 is upcoming and it does only apply to BCMA-based cards.

Gr. AvS

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Larry Finger]

On 03/01/2012 03:26 PM, Arend van Spriel wrote:
>
> SPEX(board_rev, SSB_SPROM8_BOARDREV, ~0, 0);
>
> SPEX(txpid2g[0], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G0,
> SSB_SPROM4_TXPID2G0_SHIFT);
> SPEX(txpid2g[1], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G1,
> SSB_SPROM4_TXPID2G1_SHIFT);
>
> The attributes txpid2g that are filled here are deprecated for srom revision 8.
> Is b43 using these fields for boards with srom rev 8?
>
> I propose to move to BCMA having its own sprom structure definition. Srom
> revision 10 is upcoming and it does only apply to BCMA-based cards.

If there will be SPROM revisions that are not appropriate for SSB, then BCMA 
should likely have its own definition.

Those attributes that are deprecated for Rev 8 are there because we had no way 
to know that they were being eliminated. That was not apparent when doing the 
reverse engineering.

Larry

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Hauke Mehrtens]

On 03/01/2012 10:26 PM, Arend van Spriel wrote:
> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>>> Wireless Broadcom chips can have either their SPROM data stored
>>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>>> through the same register space. This patch adds support for the
>>>> on-chip OTP memory.
>>>>
>>>> Tested with:
>>>> BCM43224 OTP and SPROM
>>>> BCM4331 SPROM
>>>> BCM4313 OTP
>>>
>>> Does bcma now support the same features regarding sprom and otp as
>>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>>> out or are there any features still missing? I am just asking because
>>> the code used in brcmsmac is ~4 times longer.
>>
>> I started on using bcma sprom content in brcmsmac and indeed there are
>> some entries missing. The change in this patch only provides read-access
>> to the srom data. As the chip comes up for read-access there is not much
>> programming need to accomplish that. The only feature that is not there
>> is that on some chips OTP can be powered down for power-saving. The
>> current chips supported by BCMA don't have that.
> 
> With BCMA retrieving the SPROM data correctly, regardless how/where it
> is stored I went to change brcmsmac to make use of it. However, it
> turned out several attributes were missing.
> 
> Now I could go and extend the ssb_sprom structure, but there I got
> confused. The structure is filled by BCMA in the function
> bcma_sprom_extract_r8(). The name suggest it deals with SROM revision 8
> attributes, but:
> 
>     for (i = 0; i < 3; i++) {
>         v = sprom[SPOFF(SSB_SPROM8_IL0MAC) + i];
>         *(((__be16 *)bus->sprom.il0mac) + i) = cpu_to_be16(v);
>     }
> 
>     SPEX(board_rev, SSB_SPROM8_BOARDREV, ~0, 0);
> 
>     SPEX(txpid2g[0], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G0,
>          SSB_SPROM4_TXPID2G0_SHIFT);
>     SPEX(txpid2g[1], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G1,
>          SSB_SPROM4_TXPID2G1_SHIFT);
> 
> The attributes txpid2g that are filled here are deprecated for srom
> revision 8. Is b43 using these fields for boards with srom rev 8?
The code filling txpid2g is probably wrong or dated and should be
removed. It could be that the spec b43 is written on was based on an old
version of the Broadcom wl driver and they used txpid2g in n-phy code.
txpid2g is used by b43 (b43_nphy_tx_power_fix()), but brcmsmac
(wlc_phy_txpwr_fixpower_nphy()) uses 0 where b43 used the sprom values.

> I propose to move to BCMA having its own sprom structure definition.
> Srom revision 10 is upcoming and it does only apply to BCMA-based cards.
Doesn't this also apply for srom rev 9, or are there any ssb based
devices with srom rev 9 out there?
> 
> Gr. AvS
> 

Re: [RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/01/2012 10:56 PM, Hauke Mehrtens wrote:
> On 03/01/2012 10:26 PM, Arend van Spriel wrote:
>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>>>> Wireless Broadcom chips can have either their SPROM data stored
>>>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>>>> through the same register space. This patch adds support for the
>>>>> on-chip OTP memory.
>>>>>
>>>>> Tested with:
>>>>> BCM43224 OTP and SPROM
>>>>> BCM4331 SPROM
>>>>> BCM4313 OTP
>>>>
>>>> Does bcma now support the same features regarding sprom and otp as
>>>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>>>> out or are there any features still missing? I am just asking because
>>>> the code used in brcmsmac is ~4 times longer.
>>>
>>> I started on using bcma sprom content in brcmsmac and indeed there are
>>> some entries missing. The change in this patch only provides read-access
>>> to the srom data. As the chip comes up for read-access there is not much
>>> programming need to accomplish that. The only feature that is not there
>>> is that on some chips OTP can be powered down for power-saving. The
>>> current chips supported by BCMA don't have that.
>>
>> With BCMA retrieving the SPROM data correctly, regardless how/where it
>> is stored I went to change brcmsmac to make use of it. However, it
>> turned out several attributes were missing.
>>
>> Now I could go and extend the ssb_sprom structure, but there I got
>> confused. The structure is filled by BCMA in the function
>> bcma_sprom_extract_r8(). The name suggest it deals with SROM revision 8
>> attributes, but:
>>
>>      for (i = 0; i<  3; i++) {
>>          v = sprom[SPOFF(SSB_SPROM8_IL0MAC) + i];
>>          *(((__be16 *)bus->sprom.il0mac) + i) = cpu_to_be16(v);
>>      }
>>
>>      SPEX(board_rev, SSB_SPROM8_BOARDREV, ~0, 0);
>>
>>      SPEX(txpid2g[0], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G0,
>>           SSB_SPROM4_TXPID2G0_SHIFT);
>>      SPEX(txpid2g[1], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G1,
>>           SSB_SPROM4_TXPID2G1_SHIFT);
>>
>> The attributes txpid2g that are filled here are deprecated for srom
>> revision 8. Is b43 using these fields for boards with srom rev 8?
> The code filling txpid2g is probably wrong or dated and should be
> removed. It could be that the spec b43 is written on was based on an old
> version of the Broadcom wl driver and they used txpid2g in n-phy code.
> txpid2g is used by b43 (b43_nphy_tx_power_fix()), but brcmsmac
> (wlc_phy_txpwr_fixpower_nphy()) uses 0 where b43 used the sprom values.

Take your word for it. Only looked at brcmsmac code and at a glance for 
n-phy it seems hardcoded and determined by the phy revision.

>> I propose to move to BCMA having its own sprom structure definition.
>> Srom revision 10 is upcoming and it does only apply to BCMA-based cards.
> Doesn't this also apply for srom rev 9, or are there any ssb based
> devices with srom rev 9 out there?

You are right. The overlap is for revision 8, which is found in both ssb 
and bcma based cards. For bcm4331 it is more subtle as there are cards 
with srom rev 8 and srom rev 9. Not sure if and how b43 is dealing with 
that.

I will prepare a RFC patch with a new bcma_sprom structure, but I would 
prefer someone from b43 to provide the patch for the b43 driver to use 
the new structure as I am not familiar with that driver.

Gr. AvS

[RFC] bcma: add support for on-chip OTP memory used for SPROM storage

[Arend van Spriel]

On 03/01/2012 10:56 PM, Hauke Mehrtens wrote:
> On 03/01/2012 10:26 PM, Arend van Spriel wrote:
>> On 02/27/2012 11:12 AM, Arend van Spriel wrote:
>>> On 02/25/2012 01:52 PM, Hauke Mehrtens wrote:
>>>> On 02/23/2012 10:52 PM, Arend van Spriel wrote:
>>>>> Wireless Broadcom chips can have either their SPROM data stored
>>>>> on either external SPROM or on-chip OTP memory. Both are accessed
>>>>> through the same register space. This patch adds support for the
>>>>> on-chip OTP memory.
>>>>>
>>>>> Tested with:
>>>>> BCM43224 OTP and SPROM
>>>>> BCM4331 SPROM
>>>>> BCM4313 OTP
>>>>
>>>> Does bcma now support the same features regarding sprom and otp as
>>>> brcmsamc expect it does not read out all the attributes brcmsmac reads
>>>> out or are there any features still missing? I am just asking because
>>>> the code used in brcmsmac is ~4 times longer.
>>>
>>> I started on using bcma sprom content in brcmsmac and indeed there are
>>> some entries missing. The change in this patch only provides read-access
>>> to the srom data. As the chip comes up for read-access there is not much
>>> programming need to accomplish that. The only feature that is not there
>>> is that on some chips OTP can be powered down for power-saving. The
>>> current chips supported by BCMA don't have that.
>>
>> With BCMA retrieving the SPROM data correctly, regardless how/where it
>> is stored I went to change brcmsmac to make use of it. However, it
>> turned out several attributes were missing.
>>
>> Now I could go and extend the ssb_sprom structure, but there I got
>> confused. The structure is filled by BCMA in the function
>> bcma_sprom_extract_r8(). The name suggest it deals with SROM revision 8
>> attributes, but:
>>
>>      for (i = 0; i<  3; i++) {
>>          v = sprom[SPOFF(SSB_SPROM8_IL0MAC) + i];
>>          *(((__be16 *)bus->sprom.il0mac) + i) = cpu_to_be16(v);
>>      }
>>
>>      SPEX(board_rev, SSB_SPROM8_BOARDREV, ~0, 0);
>>
>>      SPEX(txpid2g[0], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G0,
>>           SSB_SPROM4_TXPID2G0_SHIFT);
>>      SPEX(txpid2g[1], SSB_SPROM4_TXPID2G01, SSB_SPROM4_TXPID2G1,
>>           SSB_SPROM4_TXPID2G1_SHIFT);
>>
>> The attributes txpid2g that are filled here are deprecated for srom
>> revision 8. Is b43 using these fields for boards with srom rev 8?
> The code filling txpid2g is probably wrong or dated and should be
> removed. It could be that the spec b43 is written on was based on an old
> version of the Broadcom wl driver and they used txpid2g in n-phy code.
> txpid2g is used by b43 (b43_nphy_tx_power_fix()), but brcmsmac
> (wlc_phy_txpwr_fixpower_nphy()) uses 0 where b43 used the sprom values.

Take your word for it. Only looked at brcmsmac code and at a glance for 
n-phy it seems hardcoded and determined by the phy revision.

>> I propose to move to BCMA having its own sprom structure definition.
>> Srom revision 10 is upcoming and it does only apply to BCMA-based cards.
> Doesn't this also apply for srom rev 9, or are there any ssb based
> devices with srom rev 9 out there?

You are right. The overlap is for revision 8, which is found in both ssb 
and bcma based cards. For bcm4331 it is more subtle as there are cards 
with srom rev 8 and srom rev 9. Not sure if and how b43 is dealing with 
that.

I will prepare a RFC patch with a new bcma_sprom structure, but I would 
prefer someone from b43 to provide the patch for the b43 driver to use 
the new structure as I am not familiar with that driver.

Gr. AvS