[PATCH] Marvell CF8381

[PATCH] Marvell CF8381

[Marek Vasut]

[-- Attachment #1: Type: text/plain, Size: 66 bytes --]

Hi,
I've finally got this card working ...
See the patches below.

[-- Attachment #2: 0002-Add-support-for-CF8381-WiFi-card.patch --]
[-- Type: text/x-diff, Size: 2035 bytes --]

From 31724e385f974b334b3a77949ab8aca1cad97e6e Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Sat, 21 Mar 2009 20:53:25 +0100
Subject: [PATCH 2/2] Add support for CF8381 WiFi card

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..6d00d82 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,14 @@ static int if_cs_poll_while_fw_download(struct if_cs_card *card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID		0x0000001C
 #define IF_CS_CF8385_B1_REV		0x12
+#define	IF_CS_CF8381_B3_REV		0x04
 
+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define	CF8381_MANFID		0x02db
+#define	CF8381_CARDID		0x6064
 
 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -859,7 +866,9 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
 
 	/* Check if we have a current silicon */
-	if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
+	if (!(p_dev->manf_id == CF8381_MANFID &&
+		p_dev->card_id == CF8381_CARDID) &&
+		(if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV)) {
 		lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
 		ret = -ENODEV;
 		goto out2;
@@ -950,6 +959,7 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/
 
 static struct pcmcia_device_id if_cs_ids[] = {
+	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
 	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
 	PCMCIA_DEVICE_NULL,
 };
-- 
1.6.2


[-- Attachment #3: 0001-Correct-return-value-of-firmware-loading-functions-h.patch --]
[-- Type: text/x-diff, Size: 973 bytes --]

From f87515bca27571370323292f07526820823b0553 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Sat, 21 Mar 2009 19:20:30 +0100
Subject: [PATCH 1/2] Correct return value of firmware loading functions handling bug

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 842a08d..3f02e6a 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -867,9 +867,9 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 
 	/* Load the firmware early, before calling into libertas.ko */
 	ret = if_cs_prog_helper(card);
-	if (ret == 0)
+	if (ret >= 0)
 		ret = if_cs_prog_real(card);
-	if (ret)
+	if (ret < 0)
 		goto out2;
 
 	/* Make this card known to the libertas driver */
-- 
1.6.2

Re: [PATCH] Marvell CF8381

[Marek Vasut]

On Sunday 22 of March 2009 01:27:21 Marek Vasut wrote:
> Hi,
> I've finally got this card working ...
> See the patches below.


One more thing - I also had to apply the following patch in order to get 
region code detected properly.

le16_to_cpu(cmd.regioncode) = 0x3031 for me and Im definitelly not in spain, 
but 0x30 (eu) looks reasonable.

diff --git a/drivers/net/wireless/libertas/cmd.c 
b/drivers/net/wireless/libertas/cmd.c
index 639dd02..ce32bc9 100644
--- a/drivers/net/wireless/libertas/cmd.c
+++ b/drivers/net/wireless/libertas/cmd.c
@@ -123,7 +123,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
         * only ever be 8-bit, even though the field size is 16-bit.  Some 
firmware
         * returns non-zero high 8 bits here.
         */
-       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
+       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 0xFF00) >> 8;

        for (i = 0; i < MRVDRV_MAX_REGION_CODE; i++) {
                /* use the region code to search for the index */

Re: [PATCH] Marvell CF8381

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

On Sun, 2009-03-22 at 05:11 +0100, Marek Vasut wrote:

> One more thing - I also had to apply the following patch in order to get 
> region code detected properly.
> 
> le16_to_cpu(cmd.regioncode) = 0x3031 for me and Im definitelly not in spain, 
> but 0x30 (eu) looks reasonable.
> 
> diff --git a/drivers/net/wireless/libertas/cmd.c 
> b/drivers/net/wireless/libertas/cmd.c
> index 639dd02..ce32bc9 100644
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -123,7 +123,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
>          * only ever be 8-bit, even though the field size is 16-bit.  Some 
> firmware
>          * returns non-zero high 8 bits here.
>          */
> -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> +       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 0xFF00) >> 8;

I'd be more inclined to think that this was an endian bug? Does your
machine happen to be big endian?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Marvell CF8381

[Marek Vasut]

On Sunday 22 of March 2009 09:04:08 Johannes Berg wrote:
> On Sun, 2009-03-22 at 05:11 +0100, Marek Vasut wrote:
> > One more thing - I also had to apply the following patch in order to get
> > region code detected properly.
> >
> > le16_to_cpu(cmd.regioncode) = 0x3031 for me and Im definitelly not in
> > spain, but 0x30 (eu) looks reasonable.
> >
> > diff --git a/drivers/net/wireless/libertas/cmd.c
> > b/drivers/net/wireless/libertas/cmd.c
> > index 639dd02..ce32bc9 100644
> > --- a/drivers/net/wireless/libertas/cmd.c
> > +++ b/drivers/net/wireless/libertas/cmd.c
> > @@ -123,7 +123,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
> >          * only ever be 8-bit, even though the field size is 16-bit. 
> > Some firmware
> >          * returns non-zero high 8 bits here.
> >          */
> > -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> > +       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 0xFF00) >> 8;
>
> I'd be more inclined to think that this was an endian bug? Does your
> machine happen to be big endian?

intel pxa270 (little endian). Btw. that macro le16_to_cpu should handle the 
endianness, shouldn't it ?

>
> johannes

Re: [PATCH] Marvell CF8381

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

On Sun, 2009-03-22 at 14:01 +0100, Marek Vasut wrote:

> > > diff --git a/drivers/net/wireless/libertas/cmd.c
> > > b/drivers/net/wireless/libertas/cmd.c
> > > index 639dd02..ce32bc9 100644
> > > --- a/drivers/net/wireless/libertas/cmd.c
> > > +++ b/drivers/net/wireless/libertas/cmd.c
> > > @@ -123,7 +123,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
> > >          * only ever be 8-bit, even though the field size is 16-bit. 
> > > Some firmware
> > >          * returns non-zero high 8 bits here.
> > >          */
> > > -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> > > +       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 0xFF00) >> 8;
> >
> > I'd be more inclined to think that this was an endian bug? Does your
> > machine happen to be big endian?
> 
> intel pxa270 (little endian). Btw. that macro le16_to_cpu should handle the 
> endianness, shouldn't it ?

Yes, but maybe that part is _not_ a le16 but two u8s. :) But if you're
on LE too then I guess that's not it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Marvell CF8381

[Holger Schurig]

Hi Marek !

Great.


Some nitpicks:

1. you attached the patches, not inline
2. There's no signed-off
3. You didn't submit one mail per patch

All of this makes review harder. If you adhere a bit to
Documentation/SubmittingPatches you'd get the fame all by
yourself :-)


Some more rants:


--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,14 @@ static int if_cs_poll_while_fw_download(struct if_cs_card *card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID               0x0000001C
 #define IF_CS_CF8385_B1_REV            0x12
+#define        IF_CS_CF8381_B3_REV             0x04

Here's a tab, not a space. Maybe you run scripts/checkpatch.pl on it as well?




-       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
+       if (!(p_dev->manf_id == CF8381_MANFID &&
+               p_dev->card_id == CF8381_CARDID) &&
+               (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV)) {

I think change then breaks the existing logic for 8385 cards.
code broke then the 8385 case. Therefore a NAK for now.




Kindly repost both patches again, with signed-off and fixes, thanks!

Re: [PATCH] Marvell CF8381

[Holger Schurig]

> -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> +       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 
0xFF00) >> 8;

Hmm, the change that this breaks 8385 is quite high, maybe 100
percent.

It could simply be the case that your firmware returns the value
in a different order than mine. So we either need to test for the
firmware version or for the hardware before getting the value via
one or the other method.

Maybe it's worthwhile to create some hw_is_8381() function and 
then do the things that need to be differently based on it's 
return value?



The documentation for CMG_GET_HW_SPEC that I have says

RegionCode   UINT16    Set to 0

so I see no indicator that we have two 8 bit values here. But
we've seen bugs in docs before :-)

Re: [PATCH] Marvell CF8381

[Marek Vasut]

On Monday 23 of March 2009 13:13:10 Holger Schurig wrote:
> Hi Marek !
>
> Great.
>
>
> Some nitpicks:
>
> 1. you attached the patches, not inline
Yes, I know, I just needed some comments if they are OK
> 2. There's no signed-off
cat ../0001-Correct-return-value-of-firmware-loading-functions-h.patch | 
head -n 6
~~~~~~~
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

cat ../0002-Add-support-for-CF8381-WiFi-card.patch | head -n 6
~~~~~~~
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

So as you see, there is.
> 3. You didn't submit one mail per patch
Yes, I know, see 1.
>
> All of this makes review harder. If you adhere a bit to
> Documentation/SubmittingPatches you'd get the fame all by
> yourself :-)
>
>
> Some more rants:
>
>
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -273,7 +273,14 @@ static int if_cs_poll_while_fw_download(struct
> if_cs_card *card, uint addr, u8 r */
>  #define IF_CS_PRODUCT_ID               0x0000001C
>  #define IF_CS_CF8385_B1_REV            0x12
> +#define        IF_CS_CF8381_B3_REV             0x04
>
> Here's a tab, not a space. Maybe you run scripts/checkpatch.pl on it as
> well?
# ./scripts/checkpatch.pl ../0001-Correct-return-value-of-firmware-loading-functions-h.patch
total: 0 errors, 0 warnings, 11 lines checked

../0001-Correct-return-value-of-firmware-loading-functions-h.patch has no 
obvious style problems and is ready for submission.
# ./scripts/checkpatch.pl ../0002-Add-support-for-CF8381-WiFi-card.patch
total: 0 errors, 0 warnings, 31 lines checked

../0002-Add-support-for-CF8381-WiFi-card.patch has no obvious style problems 
and is ready for submission.

As you can see, I did. Otherwise I wont send them.
>
>
>
>
> -       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
> +       if (!(p_dev->manf_id == CF8381_MANFID &&
> +               p_dev->card_id == CF8381_CARDID) &&
> +               (if_cs_read8(card, IF_CS_PRODUCT_ID) <
> IF_CS_CF8385_B1_REV)) {
>
> I think change then breaks the existing logic for 8385 cards.
> code broke then the 8385 case. Therefore a NAK for now.
This might need review ... hang on, I'll recheck
>
>
>
>
> Kindly repost both patches again, with signed-off and fixes, thanks!

Re: [PATCH1/2] Fix return value handling

[Marek Vasut]

[-- Attachment #1: Type: text/plain, Size: 42 bytes --]

Hi,
here is a resend. One patch per mail.

[-- Attachment #2: 0001-Correct-return-value-of-firmware-loading-functions-h.patch --]
[-- Type: text/x-diff, Size: 1040 bytes --]

From fd2e610a87a8372cbc513e336fa71e3438742c9d Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 23 Mar 2009 15:57:11 +0100
Subject: [PATCH 1/2] Firmware loading functions can return possitive values
 This is not a bug, but take it into consideration and handle it properly.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 842a08d..3f02e6a 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -867,9 +867,9 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 
 	/* Load the firmware early, before calling into libertas.ko */
 	ret = if_cs_prog_helper(card);
-	if (ret == 0)
+	if (ret >= 0)
 		ret = if_cs_prog_real(card);
-	if (ret)
+	if (ret < 0)
 		goto out2;
 
 	/* Make this card known to the libertas driver */
-- 
1.6.2

Re: [PATCH] Marvell CF8381 and CF8305

[Marek Vasut]

[-- Attachment #1: Type: text/plain, Size: 97 bytes --]

Here's the other patch.
I also added cf8305 support (if_cs.c patched this way works for it too).

[-- Attachment #2: 0002-Add-support-for-CF8381-and-CF8305-WiFi-card.patch --]
[-- Type: text/x-diff, Size: 3291 bytes --]

From 4c7fcf79de8ba900c4a9a56e68c9da89491dff27 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 23 Mar 2009 16:50:30 +0100
Subject: [PATCH 2/2] Add support for CF8381 and CF8305 WiFi card.
 Two detection functions were added for identifying CF8381 and CF8305.
 Also, CF8305 uses only one-stage firmware, so handle this as well.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   35 ++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..e115c8f 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card *card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID		0x0000001C
 #define IF_CS_CF8385_B1_REV		0x12
+#define IF_CS_CF8381_B3_REV		0x04
 
+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define CF8305_MANFID		0x02db
+#define CF8305_CARDID		0x8103
+#define CF8381_MANFID		0x02db
+#define CF8381_CARDID		0x6064
+
+static inline int if_cs_hw_is_cf8305(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8305_MANFID &&
+		p_dev->card_id == CF8305_CARDID);
+}
+
+static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8381_MANFID &&
+		p_dev->card_id == CF8381_CARDID);
+}
 
 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
 static int if_cs_probe(struct pcmcia_device *p_dev)
 {
 	int ret = -ENOMEM;
+	unsigned int prod_id;
 	struct lbs_private *priv;
 	struct if_cs_card *card;
 	/* CIS parsing */
@@ -859,15 +881,20 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
 
 	/* Check if we have a current silicon */
-	if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
-		lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
+	prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
+	if (!(if_cs_hw_is_cf8305(p_dev) ||
+		(if_cs_hw_is_cf8381(p_dev) &&
+		prod_id >= IF_CS_CF8381_B3_REV)) &&
+		(prod_id < IF_CS_CF8385_B1_REV)) {
+		lbs_pr_err("old chips like 8385 rev B1 or "
+			"8381 rev B3 aren't supported\n");
 		ret = -ENODEV;
 		goto out2;
 	}
 
 	/* Load the firmware early, before calling into libertas.ko */
 	ret = if_cs_prog_helper(card);
-	if (ret >= 0)
+	if (ret >= 0 && !if_cs_hw_is_cf8305(p_dev))
 		ret = if_cs_prog_real(card);
 	if (ret < 0)
 		goto out2;
@@ -950,6 +977,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/
 
 static struct pcmcia_device_id if_cs_ids[] = {
+	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
+	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
 	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
 	PCMCIA_DEVICE_NULL,
 };
-- 
1.6.2

Re: [PATCH1/2] Fix return value handling

[Marek Vasut]

On Monday 23 of March 2009 16:57:24 Marek Vasut wrote:
> Hi,
> here is a resend. One patch per mail.
sorry, inlining

>From fd2e610a87a8372cbc513e336fa71e3438742c9d Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 23 Mar 2009 15:57:11 +0100
Subject: [PATCH 1/2] Firmware loading functions can return possitive values
 This is not a bug, but take it into consideration and handle it properly.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c 
b/drivers/net/wireless/libertas/if_cs.c
index 842a08d..3f02e6a 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -867,9 +867,9 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 
 	/* Load the firmware early, before calling into libertas.ko */
 	ret = if_cs_prog_helper(card);
-	if (ret == 0)
+	if (ret >= 0)
 		ret = if_cs_prog_real(card);
-	if (ret)
+	if (ret < 0)
 		goto out2;
 
 	/* Make this card known to the libertas driver */
-- 
1.6.2

Re: [PATCH] Marvell CF8381 and CF8305

[Marek Vasut]

On Monday 23 of March 2009 16:59:05 Marek Vasut wrote:
> Here's the other patch.
> I also added cf8305 support (if_cs.c patched this way works for it too).

here as well...inlining

>From 4c7fcf79de8ba900c4a9a56e68c9da89491dff27 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 23 Mar 2009 16:50:30 +0100
Subject: [PATCH 2/2] Add support for CF8381 and CF8305 WiFi card.
 Two detection functions were added for identifying CF8381 and CF8305.
 Also, CF8305 uses only one-stage firmware, so handle this as well.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   35 ++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c 
b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..e115c8f 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
*card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID		0x0000001C
 #define IF_CS_CF8385_B1_REV		0x12
+#define IF_CS_CF8381_B3_REV		0x04
 
+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define CF8305_MANFID		0x02db
+#define CF8305_CARDID		0x8103
+#define CF8381_MANFID		0x02db
+#define CF8381_CARDID		0x6064
+
+static inline int if_cs_hw_is_cf8305(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8305_MANFID &&
+		p_dev->card_id == CF8305_CARDID);
+}
+
+static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8381_MANFID &&
+		p_dev->card_id == CF8381_CARDID);
+}
 
 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
 static int if_cs_probe(struct pcmcia_device *p_dev)
 {
 	int ret = -ENOMEM;
+	unsigned int prod_id;
 	struct lbs_private *priv;
 	struct if_cs_card *card;
 	/* CIS parsing */
@@ -859,15 +881,20 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
 
 	/* Check if we have a current silicon */
-	if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
-		lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
+	prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
+	if (!(if_cs_hw_is_cf8305(p_dev) ||
+		(if_cs_hw_is_cf8381(p_dev) &&
+		prod_id >= IF_CS_CF8381_B3_REV)) &&
+		(prod_id < IF_CS_CF8385_B1_REV)) {
+		lbs_pr_err("old chips like 8385 rev B1 or "
+			"8381 rev B3 aren't supported\n");
 		ret = -ENODEV;
 		goto out2;
 	}
 
 	/* Load the firmware early, before calling into libertas.ko */
 	ret = if_cs_prog_helper(card);
-	if (ret >= 0)
+	if (ret >= 0 && !if_cs_hw_is_cf8305(p_dev))
 		ret = if_cs_prog_real(card);
 	if (ret < 0)
 		goto out2;
@@ -950,6 +977,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/
 
 static struct pcmcia_device_id if_cs_ids[] = {
+	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
+	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
 	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
 	PCMCIA_DEVICE_NULL,
 };
-- 
1.6.2

Re: [PATCH] Marvell CF8381 and CF8305

[Holger Schurig]

> +#define CF8305_MANFID		0x02db
> +#define CF8305_CARDID		0x8103

It's 8385, not 8305.

>  	/* Check if we have a current silicon */
> -	if (if_cs_read8(card, IF_CS_PRODUCT_ID) <
> IF_CS_CF8385_B1_REV) { -		lbs_pr_err("old chips like 8385 rev
> B1 aren't supported\n"); +	prod_id = if_cs_read8(card,
> IF_CS_PRODUCT_ID);
> +	if (!(if_cs_hw_is_cf8305(p_dev) ||
> +		(if_cs_hw_is_cf8381(p_dev) &&
> +		prod_id >= IF_CS_CF8381_B3_REV)) &&
> +		(prod_id < IF_CS_CF8385_B1_REV)) {
> +		lbs_pr_err("old chips like 8385 rev B1 or "
> +			"8381 rev B3 aren't supported\n");

I still find this if hard to read. Why not something like this:

if ((if_cs_is_8385() && prod_id < IF_CS_CF8385_B1_REV) ||
    (if_cs_Is_8381() && prod_id < IF_CS_CF8381_B3_REV)) {
      ....
}

>  static struct pcmcia_device_id if_cs_ids[] = {
> +	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
> +	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
>  	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
>  	PCMCIA_DEVICE_NULL,
>  };

Now we end with two entries of 0x02df, 0x8103 :-/

Re: [PATCH1/2] Fix return value handling

[Dan Williams]

On Mon, 2009-03-23 at 16:59 +0100, Marek Vasut wrote:
> On Monday 23 of March 2009 16:57:24 Marek Vasut wrote:
> > Hi,
> > here is a resend. One patch per mail.
> sorry, inlining
> 
> From fd2e610a87a8372cbc513e336fa71e3438742c9d Mon Sep 17 00:00:00 2001
> From: Marek Vasut <marek.vasut@gmail.com>
> Date: Mon, 23 Mar 2009 15:57:11 +0100
> Subject: [PATCH 1/2] Firmware loading functions can return possitive values
>  This is not a bug, but take it into consideration and handle it properly.

This patch should no longer be required, since the patch "libertas: fix
CF firmware loading for some cards" was applied.  The firmware helpers
should only return an error (< 0) or 0 (success).

Dan

> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/net/wireless/libertas/if_cs.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/if_cs.c 
> b/drivers/net/wireless/libertas/if_cs.c
> index 842a08d..3f02e6a 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -867,9 +867,9 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  
>  	/* Load the firmware early, before calling into libertas.ko */
>  	ret = if_cs_prog_helper(card);
> -	if (ret == 0)
> +	if (ret >= 0)
>  		ret = if_cs_prog_real(card);
> -	if (ret)
> +	if (ret < 0)
>  		goto out2;
>  
>  	/* Make this card known to the libertas driver */

Re: [PATCH] Marvell CF8381

[Dan Williams]

On Mon, 2009-03-23 at 13:27 +0100, Holger Schurig wrote:
> > -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> > +       priv->regioncode = (le16_to_cpu(cmd.regioncode) & 
> 0xFF00) >> 8;
> 
> Hmm, the change that this breaks 8385 is quite high, maybe 100
> percent.

The cf8385-5.0.16.p0-26306 (gumstix) driver has:

	Adapter->RegionCode = wlan_le16_to_cpu(hwspec->RegionCode) >> 8;

So it appears there's precedent for this even on 8385 parts.

Dan

> It could simply be the case that your firmware returns the value
> in a different order than mine. So we either need to test for the
> firmware version or for the hardware before getting the value via
> one or the other method.
> 
> Maybe it's worthwhile to create some hw_is_8381() function and 
> then do the things that need to be differently based on it's 
> return value?
> 
> 
> 
> The documentation for CMG_GET_HW_SPEC that I have says
> 
> RegionCode   UINT16    Set to 0
> 
> so I see no indicator that we have two 8 bit values here. But
> we've seen bugs in docs before :-)
> --
> 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

Re: [PATCH] Marvell CF8381 and CF8305

[Dan Williams]

On Mon, 2009-03-23 at 17:00 +0100, Marek Vasut wrote:
> On Monday 23 of March 2009 16:59:05 Marek Vasut wrote:
> > Here's the other patch.
> > I also added cf8305 support (if_cs.c patched this way works for it too).
> 
> here as well...inlining
> 
> From 4c7fcf79de8ba900c4a9a56e68c9da89491dff27 Mon Sep 17 00:00:00 2001
> From: Marek Vasut <marek.vasut@gmail.com>
> Date: Mon, 23 Mar 2009 16:50:30 +0100
> Subject: [PATCH 2/2] Add support for CF8381 and CF8305 WiFi card.
>  Two detection functions were added for identifying CF8381 and CF8305.
>  Also, CF8305 uses only one-stage firmware, so handle this as well.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/net/wireless/libertas/if_cs.c |   35 ++++++++++++++++++++++++++++++--
>  1 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/if_cs.c 
> b/drivers/net/wireless/libertas/if_cs.c
> index 3f02e6a..e115c8f 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
> *card, uint addr, u8 r
>   */
>  #define IF_CS_PRODUCT_ID		0x0000001C
>  #define IF_CS_CF8385_B1_REV		0x12
> +#define IF_CS_CF8381_B3_REV		0x04
>  
> +/*
> + * Used to detect other cards than CF8385 since their revisions of silicon
> + * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
> + */
> +#define CF8305_MANFID		0x02db
> +#define CF8305_CARDID		0x8103
> +#define CF8381_MANFID		0x02db
> +#define CF8381_CARDID		0x6064
> +
> +static inline int if_cs_hw_is_cf8305(struct pcmcia_device *p_dev)
> +{
> +	return (p_dev->manf_id == CF8305_MANFID &&
> +		p_dev->card_id == CF8305_CARDID);
> +}
> +
> +static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
> +{
> +	return (p_dev->manf_id == CF8381_MANFID &&
> +		p_dev->card_id == CF8381_CARDID);
> +}
>  
>  /********************************************************************/
>  /* I/O and interrupt handling                                       */
> @@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
>  static int if_cs_probe(struct pcmcia_device *p_dev)
>  {
>  	int ret = -ENOMEM;
> +	unsigned int prod_id;
>  	struct lbs_private *priv;
>  	struct if_cs_card *card;
>  	/* CIS parsing */
> @@ -859,15 +881,20 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
>  
>  	/* Check if we have a current silicon */
> -	if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
> -		lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
> +	prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
> +	if (!(if_cs_hw_is_cf8305(p_dev) ||
> +		(if_cs_hw_is_cf8381(p_dev) &&
> +		prod_id >= IF_CS_CF8381_B3_REV)) &&
> +		(prod_id < IF_CS_CF8385_B1_REV)) {
> +		lbs_pr_err("old chips like 8385 rev B1 or "
> +			"8381 rev B3 aren't supported\n");

This gets ugly fast; couldn't we instead do something like:

if (if_cs_hw_is_cf8381(p_dev) && (prod_id < IF_CS_CF8381_B3_REV)) {
	lbs_pr_err("Only 88w8381 hardware revisions B3 and later"
	           " are supported.\n"
	ret = -ENODEV;
	goto out2;
}

if (if_cs_hw_is_cf8385(p_dev) && (prod_id <= IF_CS_CF8385_B1_REV)) {
	lbs_pr_err("Only 88w8385 hardware revisions B2 and later"
	           " are supported.\n"
	ret = -ENODEV;
	goto out2;
}

instead?

>  		ret = -ENODEV;
>  		goto out2;
>  	}
>  
>  	/* Load the firmware early, before calling into libertas.ko */
>  	ret = if_cs_prog_helper(card);
> -	if (ret >= 0)
> +	if (ret >= 0 && !if_cs_hw_is_cf8305(p_dev))
>  		ret = if_cs_prog_real(card);

Again, not required with current wireless-testing.

Dan

>  	if (ret < 0)
>  		goto out2;
> @@ -950,6 +977,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
>  /********************************************************************/
>  
>  static struct pcmcia_device_id if_cs_ids[] = {
> +	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
> +	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
>  	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
>  	PCMCIA_DEVICE_NULL,
>  };

Re: [PATCH] Marvell CF8381 and CF8305

[Marek Vasut]

On Monday 23 of March 2009 17:06:16 Holger Schurig wrote:
> > +#define CF8305_MANFID		0x02db
> > +#define CF8305_CARDID		0x8103
>
> It's 8385, not 8305.
it's 8305 ... that's even older card ;)
>
> >  	/* Check if we have a current silicon */
> > -	if (if_cs_read8(card, IF_CS_PRODUCT_ID) <
> > IF_CS_CF8385_B1_REV) { -		lbs_pr_err("old chips like 8385 rev
> > B1 aren't supported\n"); +	prod_id = if_cs_read8(card,
> > IF_CS_PRODUCT_ID);
> > +	if (!(if_cs_hw_is_cf8305(p_dev) ||
> > +		(if_cs_hw_is_cf8381(p_dev) &&
> > +		prod_id >= IF_CS_CF8381_B3_REV)) &&
> > +		(prod_id < IF_CS_CF8385_B1_REV)) {
> > +		lbs_pr_err("old chips like 8385 rev B1 or "
> > +			"8381 rev B3 aren't supported\n");
>
> I still find this if hard to read. Why not something like this:
>
> if ((if_cs_is_8385() && prod_id < IF_CS_CF8385_B1_REV) ||
>     (if_cs_Is_8381() && prod_id < IF_CS_CF8381_B3_REV)) {
>       ....
> }
>
> >  static struct pcmcia_device_id if_cs_ids[] = {
> > +	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
> > +	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
> >  	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
> >  	PCMCIA_DEVICE_NULL,
> >  };
>
> Now we end with two entries of 0x02df, 0x8103 :-/

Re: [PATCH] Marvell CF8381 and CF8305

[Dan Williams]

On Mon, 2009-03-23 at 18:09 +0100, Marek Vasut wrote:
> On Monday 23 of March 2009 17:06:16 Holger Schurig wrote:
> > > +#define CF8305_MANFID		0x02db
> > > +#define CF8305_CARDID		0x8103
> >
> > It's 8385, not 8305.
> it's 8305 ... that's even older card ;)

v4 firmware or v5?

Dan

> >
> > >  	/* Check if we have a current silicon */
> > > -	if (if_cs_read8(card, IF_CS_PRODUCT_ID) <
> > > IF_CS_CF8385_B1_REV) { -		lbs_pr_err("old chips like 8385 rev
> > > B1 aren't supported\n"); +	prod_id = if_cs_read8(card,
> > > IF_CS_PRODUCT_ID);
> > > +	if (!(if_cs_hw_is_cf8305(p_dev) ||
> > > +		(if_cs_hw_is_cf8381(p_dev) &&
> > > +		prod_id >= IF_CS_CF8381_B3_REV)) &&
> > > +		(prod_id < IF_CS_CF8385_B1_REV)) {
> > > +		lbs_pr_err("old chips like 8385 rev B1 or "
> > > +			"8381 rev B3 aren't supported\n");
> >
> > I still find this if hard to read. Why not something like this:
> >
> > if ((if_cs_is_8385() && prod_id < IF_CS_CF8385_B1_REV) ||
> >     (if_cs_Is_8381() && prod_id < IF_CS_CF8381_B3_REV)) {
> >       ....
> > }
> >
> > >  static struct pcmcia_device_id if_cs_ids[] = {
> > > +	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
> > > +	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
> > >  	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
> > >  	PCMCIA_DEVICE_NULL,
> > >  };
> >
> > Now we end with two entries of 0x02df, 0x8103 :-/
> 
> 
> 
> _______________________________________________
> libertas-dev mailing list
> libertas-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev

Re: [PATCH] Marvell CF8381 and CF8305

[Marek Vasut]

On Monday 23 of March 2009 18:19:55 Dan Williams wrote:
> On Mon, 2009-03-23 at 18:09 +0100, Marek Vasut wrote:
> > On Monday 23 of March 2009 17:06:16 Holger Schurig wrote:
> > > > +#define CF8305_MANFID		0x02db
> > > > +#define CF8305_CARDID		0x8103
> > >
> > > It's 8385, not 8305.
> >
> > it's 8305 ... that's even older card ;)
>
> v4 firmware or v5?

8381 is v4, 8305 is v3

>
> Dan
>
> > > >  	/* Check if we have a current silicon */
> > > > -	if (if_cs_read8(card, IF_CS_PRODUCT_ID) <
> > > > IF_CS_CF8385_B1_REV) { -		lbs_pr_err("old chips like 8385 rev
> > > > B1 aren't supported\n"); +	prod_id = if_cs_read8(card,
> > > > IF_CS_PRODUCT_ID);
> > > > +	if (!(if_cs_hw_is_cf8305(p_dev) ||
> > > > +		(if_cs_hw_is_cf8381(p_dev) &&
> > > > +		prod_id >= IF_CS_CF8381_B3_REV)) &&
> > > > +		(prod_id < IF_CS_CF8385_B1_REV)) {
> > > > +		lbs_pr_err("old chips like 8385 rev B1 or "
> > > > +			"8381 rev B3 aren't supported\n");
> > >
> > > I still find this if hard to read. Why not something like this:
> > >
> > > if ((if_cs_is_8385() && prod_id < IF_CS_CF8385_B1_REV) ||
> > >     (if_cs_Is_8381() && prod_id < IF_CS_CF8381_B3_REV)) {
> > >       ....
> > > }
> > >
> > > >  static struct pcmcia_device_id if_cs_ids[] = {
> > > > +	PCMCIA_DEVICE_MANF_CARD(CF8305_MANFID, CF8305_CARDID),
> > > > +	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
> > > >  	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
> > > >  	PCMCIA_DEVICE_NULL,
> > > >  };
> > >
> > > Now we end with two entries of 0x02df, 0x8103 :-/
> >
> > _______________________________________________
> > libertas-dev mailing list
> > libertas-dev@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/libertas-dev

Re: [PATCH] Marvell CF8381

[Marek Vasut]

On Monday 23 of March 2009 17:15:27 Dan Williams wrote:
> On Mon, 2009-03-23 at 13:27 +0100, Holger Schurig wrote:
> > > -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> > > +       priv->regioncode = (le16_to_cpu(cmd.regioncode) &
> >
> > 0xFF00) >> 8;
> >
> > Hmm, the change that this breaks 8385 is quite high, maybe 100
> > percent.
>
> The cf8385-5.0.16.p0-26306 (gumstix) driver has:
>
> 	Adapter->RegionCode = wlan_le16_to_cpu(hwspec->RegionCode) >> 8;
>
> So it appears there's precedent for this even on 8385 parts.
>
> Dan
>
> > It could simply be the case that your firmware returns the value
> > in a different order than mine. So we either need to test for the
> > firmware version or for the hardware before getting the value via
> > one or the other method.
> >
> > Maybe it's worthwhile to create some hw_is_8381() function and
> > then do the things that need to be differently based on it's
> > return value?
> >
> >
> >
> > The documentation for CMG_GET_HW_SPEC that I have says
> >
> > RegionCode   UINT16    Set to 0
> >
> > so I see no indicator that we have two 8 bit values here. But
> > we've seen bugs in docs before :-)
> > --
> > 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

Ok, so this should be it. I dropped the 8305 support since I found I have some 
issues with it still.

>From 3ac8f34db9c4d96197fbdc2776ebbcd571e3fbbe Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Mon, 23 Mar 2009 22:55:51 +0100
Subject: [PATCH] Add support for CF8381 WiFi card.
 A detection function was added for identifying CF8381.

Signed-off-by: <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c 
b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..56b6475 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,20 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
*card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID               0x0000001C
 #define IF_CS_CF8385_B1_REV            0x12
+#define IF_CS_CF8381_B3_REV            0x04

+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define CF8381_MANFID          0x02db
+#define CF8381_CARDID          0x6064
+
+static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
+{
+       return (p_dev->manf_id == CF8381_MANFID &&
+               p_dev->card_id == CF8381_CARDID);
+}

 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -757,6 +770,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
 static int if_cs_probe(struct pcmcia_device *p_dev)
 {
        int ret = -ENOMEM;
+       unsigned int prod_id;
        struct lbs_private *priv;
        struct if_cs_card *card;
        /* CIS parsing */
@@ -859,7 +873,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
               p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);

        /* Check if we have a current silicon */
-       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
+       prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
+       if (if_cs_hw_is_cf8381(p_dev) && prod_id < IF_CS_CF8381_B3_REV) {
+               lbs_pr_err("old chips like 8381 rev B3 aren't supported\n");
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       if (prod_id < IF_CS_CF8385_B1_REV) {
                lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
                ret = -ENODEV;
                goto out2;
@@ -950,6 +971,7 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/

 static struct pcmcia_device_id if_cs_ids[] = {
+       PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
        PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
        PCMCIA_DEVICE_NULL,
 };
--
1.6.2

Re: [PATCH] Marvell CF8381

[Dan Williams]

On Mon, 2009-03-23 at 22:58 +0100, Marek Vasut wrote:
> On Monday 23 of March 2009 17:15:27 Dan Williams wrote:
> > On Mon, 2009-03-23 at 13:27 +0100, Holger Schurig wrote:
> > > > -       priv->regioncode = le16_to_cpu(cmd.regioncode) & 0xFF;
> > > > +       priv->regioncode = (le16_to_cpu(cmd.regioncode) &
> > >
> > > 0xFF00) >> 8;
> > >
> > > Hmm, the change that this breaks 8385 is quite high, maybe 100
> > > percent.
> >
> > The cf8385-5.0.16.p0-26306 (gumstix) driver has:
> >
> > 	Adapter->RegionCode = wlan_le16_to_cpu(hwspec->RegionCode) >> 8;
> >
> > So it appears there's precedent for this even on 8385 parts.
> >
> > Dan
> >
> > > It could simply be the case that your firmware returns the value
> > > in a different order than mine. So we either need to test for the
> > > firmware version or for the hardware before getting the value via
> > > one or the other method.
> > >
> > > Maybe it's worthwhile to create some hw_is_8381() function and
> > > then do the things that need to be differently based on it's
> > > return value?
> > >
> > >
> > >
> > > The documentation for CMG_GET_HW_SPEC that I have says
> > >
> > > RegionCode   UINT16    Set to 0
> > >
> > > so I see no indicator that we have two 8 bit values here. But
> > > we've seen bugs in docs before :-)
> > > --
> > > 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
> 
> Ok, so this should be it. I dropped the 8305 support since I found I have some 
> issues with it still.
> 
> From 3ac8f34db9c4d96197fbdc2776ebbcd571e3fbbe Mon Sep 17 00:00:00 2001
> From: Marek Vasut <marek.vasut@gmail.com>
> Date: Mon, 23 Mar 2009 22:55:51 +0100
> Subject: [PATCH] Add support for CF8381 WiFi card.
>  A detection function was added for identifying CF8381.
> 
> Signed-off-by: <marek.vasut@gmail.com>
> ---
>  drivers/net/wireless/libertas/if_cs.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/if_cs.c 
> b/drivers/net/wireless/libertas/if_cs.c
> index 3f02e6a..56b6475 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -273,7 +273,20 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
> *card, uint addr, u8 r
>   */
>  #define IF_CS_PRODUCT_ID               0x0000001C
>  #define IF_CS_CF8385_B1_REV            0x12
> +#define IF_CS_CF8381_B3_REV            0x04
> 
> +/*
> + * Used to detect other cards than CF8385 since their revisions of silicon
> + * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
> + */
> +#define CF8381_MANFID          0x02db
> +#define CF8381_CARDID          0x6064
> +
> +static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
> +{
> +       return (p_dev->manf_id == CF8381_MANFID &&
> +               p_dev->card_id == CF8381_CARDID);
> +}
> 
>  /********************************************************************/
>  /* I/O and interrupt handling                                       */
> @@ -757,6 +770,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
>  static int if_cs_probe(struct pcmcia_device *p_dev)
>  {
>         int ret = -ENOMEM;
> +       unsigned int prod_id;
>         struct lbs_private *priv;
>         struct if_cs_card *card;
>         /* CIS parsing */
> @@ -859,7 +873,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>                p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
> 
>         /* Check if we have a current silicon */
> -       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
> +       prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
> +       if (if_cs_hw_is_cf8381(p_dev) && prod_id < IF_CS_CF8381_B3_REV) {
> +               lbs_pr_err("old chips like 8381 rev B3 aren't supported\n");
> +               ret = -ENODEV;
> +               goto out2;
> +       }
> +
> +       if (prod_id < IF_CS_CF8385_B1_REV) {

You'll still want to implement a if_cs_hw_is_cf8385() here though;
otherwise, wouldn't 8381 get caught by this 8385 check and get rejected?

Dan

>                 lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
>                 ret = -ENODEV;
>                 goto out2;
> @@ -950,6 +971,7 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
>  /********************************************************************/
> 
>  static struct pcmcia_device_id if_cs_ids[] = {
> +       PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
>         PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
>         PCMCIA_DEVICE_NULL,
>  };
> --
> 1.6.2
> 
> --
> 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

Re: [PATCH] Marvell CF8381

[Marek Vasut]

Ok, here is another version, hopefully last one.

>From a7522df9d610702cfc3066861a2e315552e29086 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Tue, 24 Mar 2009 00:40:44 +0100
Subject: [PATCH] Add support for CF8381 WiFi card.
 A detection function was added for identifying CF8381.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   34 +++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c 
b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..1da23e5 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
*card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID               0x0000001C
 #define IF_CS_CF8385_B1_REV            0x12
+#define IF_CS_CF8381_B3_REV            0x04

+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define CF8381_MANFID          0x02db
+#define CF8381_CARDID          0x6064
+#define CF8385_MANFID          0x02df
+#define CF8385_CARDID          0x8103
+
+static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
+{
+       return (p_dev->manf_id == CF8381_MANFID &&
+               p_dev->card_id == CF8381_CARDID);
+}
+
+static inline int if_cs_hw_is_cf8385(struct pcmcia_device *p_dev)
+{
+       return (p_dev->manf_id == CF8385_MANFID &&
+               p_dev->card_id == CF8385_CARDID);
+}

 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
 static int if_cs_probe(struct pcmcia_device *p_dev)
 {
        int ret = -ENOMEM;
+       unsigned int prod_id;
        struct lbs_private *priv;
        struct if_cs_card *card;
        /* CIS parsing */
@@ -859,7 +881,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
               p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);

        /* Check if we have a current silicon */
-       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
+       prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
+       if (if_cs_hw_is_cf8381(p_dev) && prod_id < IF_CS_CF8381_B3_REV) {
+               lbs_pr_err("old chips like 8381 rev B3 aren't supported\n");
+               ret = -ENODEV;
+               goto out2;
+       }
+
+       if (if_cs_hw_is_cf8385(p_dev) && prod_id < IF_CS_CF8385_B1_REV) {
                lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
                ret = -ENODEV;
                goto out2;
@@ -950,7 +979,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/

 static struct pcmcia_device_id if_cs_ids[] = {
-       PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
+       PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
+       PCMCIA_DEVICE_MANF_CARD(CF8385_MANFID, CF8385_CARDID),
        PCMCIA_DEVICE_NULL,
 };
 MODULE_DEVICE_TABLE(pcmcia, if_cs_ids);
--
1.6.2

Re: [PATCH] Marvell CF8381

[Dan Williams]

On Tue, 2009-03-24 at 00:42 +0100, Marek Vasut wrote:
> Ok, here is another version, hopefully last one.
> 
> From a7522df9d610702cfc3066861a2e315552e29086 Mon Sep 17 00:00:00 2001
> From: Marek Vasut <marek.vasut@gmail.com>
> Date: Tue, 24 Mar 2009 00:40:44 +0100
> Subject: [PATCH] Add support for CF8381 WiFi card.
>  A detection function was added for identifying CF8381.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Acked-by: Dan Williams <dcbw@redhat.com>

> ---
>  drivers/net/wireless/libertas/if_cs.c |   34 +++++++++++++++++++++++++++++++-
>  1 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/if_cs.c 
> b/drivers/net/wireless/libertas/if_cs.c
> index 3f02e6a..1da23e5 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card 
> *card, uint addr, u8 r
>   */
>  #define IF_CS_PRODUCT_ID               0x0000001C
>  #define IF_CS_CF8385_B1_REV            0x12
> +#define IF_CS_CF8381_B3_REV            0x04
> 
> +/*
> + * Used to detect other cards than CF8385 since their revisions of silicon
> + * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
> + */
> +#define CF8381_MANFID          0x02db
> +#define CF8381_CARDID          0x6064
> +#define CF8385_MANFID          0x02df
> +#define CF8385_CARDID          0x8103
> +
> +static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
> +{
> +       return (p_dev->manf_id == CF8381_MANFID &&
> +               p_dev->card_id == CF8381_CARDID);
> +}
> +
> +static inline int if_cs_hw_is_cf8385(struct pcmcia_device *p_dev)
> +{
> +       return (p_dev->manf_id == CF8385_MANFID &&
> +               p_dev->card_id == CF8385_CARDID);
> +}
> 
>  /********************************************************************/
>  /* I/O and interrupt handling                                       */
> @@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
>  static int if_cs_probe(struct pcmcia_device *p_dev)
>  {
>         int ret = -ENOMEM;
> +       unsigned int prod_id;
>         struct lbs_private *priv;
>         struct if_cs_card *card;
>         /* CIS parsing */
> @@ -859,7 +881,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>                p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
> 
>         /* Check if we have a current silicon */
> -       if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
> +       prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
> +       if (if_cs_hw_is_cf8381(p_dev) && prod_id < IF_CS_CF8381_B3_REV) {
> +               lbs_pr_err("old chips like 8381 rev B3 aren't supported\n");
> +               ret = -ENODEV;
> +               goto out2;
> +       }
> +
> +       if (if_cs_hw_is_cf8385(p_dev) && prod_id < IF_CS_CF8385_B1_REV) {
>                 lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
>                 ret = -ENODEV;
>                 goto out2;
> @@ -950,7 +979,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
>  /********************************************************************/
> 
>  static struct pcmcia_device_id if_cs_ids[] = {
> -       PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
> +       PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
> +       PCMCIA_DEVICE_MANF_CARD(CF8385_MANFID, CF8385_CARDID),
>         PCMCIA_DEVICE_NULL,
>  };
>  MODULE_DEVICE_TABLE(pcmcia, if_cs_ids);
> --
> 1.6.2
> 

Re: [PATCH] Marvell CF8381

[John W. Linville]

On Tue, Mar 24, 2009 at 06:26:27AM -0400, Dan Williams wrote:
> On Tue, 2009-03-24 at 00:42 +0100, Marek Vasut wrote:
> > Ok, here is another version, hopefully last one.
> > 
> > From a7522df9d610702cfc3066861a2e315552e29086 Mon Sep 17 00:00:00 2001
> > From: Marek Vasut <marek.vasut@gmail.com>
> > Date: Tue, 24 Mar 2009 00:40:44 +0100
> > Subject: [PATCH] Add support for CF8381 WiFi card.
> >  A detection function was added for identifying CF8381.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> Acked-by: Dan Williams <dcbw@redhat.com>

The patch is whitespace-damaged.  Please resubmit w/o whitespace
damage in a separate email.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] Marvell CF8381

[Marek Vasut]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On Tuesday 24 of March 2009 21:01:18 John W. Linville wrote:
> On Tue, Mar 24, 2009 at 06:26:27AM -0400, Dan Williams wrote:
> > On Tue, 2009-03-24 at 00:42 +0100, Marek Vasut wrote:
> > > Ok, here is another version, hopefully last one.
> > >
> > > From a7522df9d610702cfc3066861a2e315552e29086 Mon Sep 17 00:00:00 2001
> > > From: Marek Vasut <marek.vasut@gmail.com>
> > > Date: Tue, 24 Mar 2009 00:40:44 +0100
> > > Subject: [PATCH] Add support for CF8381 WiFi card.
> > >  A detection function was added for identifying CF8381.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >
> > Acked-by: Dan Williams <dcbw@redhat.com>
>
> The patch is whitespace-damaged.  Please resubmit w/o whitespace
> damage in a separate email.
Sending as an attachment so it wont get broken.
>
> Thanks,
>
> John



[-- Attachment #2: 0001-Add-support-for-CF8381-WiFi-card.patch --]
[-- Type: text/x-diff, Size: 3003 bytes --]

From a7522df9d610702cfc3066861a2e315552e29086 Mon Sep 17 00:00:00 2001
From: Marek Vasut <marek.vasut@gmail.com>
Date: Tue, 24 Mar 2009 00:40:44 +0100
Subject: [PATCH] Add support for CF8381 WiFi card.
 A detection function was added for identifying CF8381.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/net/wireless/libertas/if_cs.c |   34 +++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 3f02e6a..1da23e5 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -273,7 +273,28 @@ static int if_cs_poll_while_fw_download(struct if_cs_card *card, uint addr, u8 r
  */
 #define IF_CS_PRODUCT_ID		0x0000001C
 #define IF_CS_CF8385_B1_REV		0x12
+#define IF_CS_CF8381_B3_REV		0x04
 
+/*
+ * Used to detect other cards than CF8385 since their revisions of silicon
+ * doesn't match those from CF8385, eg. CF8381 B3 works with this driver.
+ */
+#define CF8381_MANFID		0x02db
+#define CF8381_CARDID		0x6064
+#define CF8385_MANFID		0x02df
+#define CF8385_CARDID		0x8103
+
+static inline int if_cs_hw_is_cf8381(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8381_MANFID &&
+		p_dev->card_id == CF8381_CARDID);
+}
+
+static inline int if_cs_hw_is_cf8385(struct pcmcia_device *p_dev)
+{
+	return (p_dev->manf_id == CF8385_MANFID &&
+		p_dev->card_id == CF8385_CARDID);
+}
 
 /********************************************************************/
 /* I/O and interrupt handling                                       */
@@ -757,6 +778,7 @@ static void if_cs_release(struct pcmcia_device *p_dev)
 static int if_cs_probe(struct pcmcia_device *p_dev)
 {
 	int ret = -ENOMEM;
+	unsigned int prod_id;
 	struct lbs_private *priv;
 	struct if_cs_card *card;
 	/* CIS parsing */
@@ -859,7 +881,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
 	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
 
 	/* Check if we have a current silicon */
-	if (if_cs_read8(card, IF_CS_PRODUCT_ID) < IF_CS_CF8385_B1_REV) {
+	prod_id = if_cs_read8(card, IF_CS_PRODUCT_ID);
+	if (if_cs_hw_is_cf8381(p_dev) && prod_id < IF_CS_CF8381_B3_REV) {
+		lbs_pr_err("old chips like 8381 rev B3 aren't supported\n");
+		ret = -ENODEV;
+		goto out2;
+	}
+
+	if (if_cs_hw_is_cf8385(p_dev) && prod_id < IF_CS_CF8385_B1_REV) {
 		lbs_pr_err("old chips like 8385 rev B1 aren't supported\n");
 		ret = -ENODEV;
 		goto out2;
@@ -950,7 +979,8 @@ static void if_cs_detach(struct pcmcia_device *p_dev)
 /********************************************************************/
 
 static struct pcmcia_device_id if_cs_ids[] = {
-	PCMCIA_DEVICE_MANF_CARD(0x02df, 0x8103),
+	PCMCIA_DEVICE_MANF_CARD(CF8381_MANFID, CF8381_CARDID),
+	PCMCIA_DEVICE_MANF_CARD(CF8385_MANFID, CF8385_CARDID),
 	PCMCIA_DEVICE_NULL,
 };
 MODULE_DEVICE_TABLE(pcmcia, if_cs_ids);
-- 
1.6.2