2010-04-01 21:14:37

by Luis Correia

Subject: [PATCH V3] rt2x00: remove MCU requests for SoC platforms

The ralink SoC platforms do not have an MCU.

Signed-off-by: Luis Correia <[email protected]>
---

--- a/drivers/net/wireless/rt2x00/rt2800lib.c 2010-03-26
18:25:50.000000000 +0000
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c 2010-04-01
13:05:18.249747122 +0100
@@ -221,9 +221,9 @@
u32 reg;

/*
- * SOC devices don't support MCU requests.
+ * some devices don't support MCU requests.
*/
- if (rt2x00_is_soc(rt2x00dev))
+ if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
return;

mutex_lock(&rt2x00dev->csr_mutex);
--- a/drivers/net/wireless/rt2x00/rt2800pci.c 2010-03-26
18:25:50.000000000 +0000
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c 2010-04-01
13:04:42.453621607 +0100
@@ -59,6 +59,12 @@
{
unsigned int i;
u32 reg;
+
+ /*
+ * some devices don't support MCU requests.
+ */
+ if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
+ return;

for (i = 0; i < 200; i++) {
rt2800_register_read(rt2x00dev, H2M_MAILBOX_CID, &reg);
@@ -1098,10 +1104,12 @@
__set_bit(DRIVER_SUPPORT_CONTROL_FILTER_PSPOLL, &rt2x00dev->flags);

/*
- * This device requires firmware.
+ * This device requires firmware and MCU access.
*/
- if (!rt2x00_is_soc(rt2x00dev))
+ if (!rt2x00_is_soc(rt2x00dev)) {
__set_bit(DRIVER_REQUIRE_FIRMWARE, &rt2x00dev->flags);
+ __set_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags);
+ }
__set_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags);
__set_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags);
if (!modparam_nohwcrypt)
--- a/drivers/net/wireless/rt2x00/rt2x00.h 2010-03-26
18:25:50.000000000 +0000
+++ b/drivers/net/wireless/rt2x00/rt2x00.h 2010-04-01
13:01:26.812694036 +0100
@@ -631,6 +631,7 @@
* Driver requirements
*/
DRIVER_REQUIRE_FIRMWARE,
+ DRIVER_REQUIRE_MCU,
DRIVER_REQUIRE_BEACON_GUARD,
DRIVER_REQUIRE_ATIM_QUEUE,
DRIVER_REQUIRE_DMA,

2010-04-01 21:52:35

by Luis Correia

Subject: Re: [PATCH V3] rt2x00: remove MCU requests for SoC platforms

Hi Gertjan,

On Thu, Apr 1, 2010 at 22:44, Gertjan van Wingerde <[email protected]> wrote:
> Luis,
>
> On 04/01/10 23:14, Luis Correia wrote:
>> The ralink SoC platforms do not have an MCU.
>>
>> Signed-off-by: Luis Correia <[email protected]>
>
> I know Ivo already acked the v2 version of the patch, but isn't the
> addition of a driver flag a bit overkill?
>
> We have the test on whether the platform is SOC w.r.t. MCU requests
> in 2 places, and both of them are in rt2800 code. I do not really see
> a need to clutter the global rt2x00 space with a rt2800 specific flag,
> which is only used in rt2800 code.
>


Well, I confess that it was really his suggestion.

For me, if a chipset needs firmware then it implicitly says there is
an MCU involved.
And I had a previously unpublished patch that just tested for SoC and
prevented MCU stuff.

But the patch itself is mostly harmless, since it doesn't add any
functionality nor makes the driver misbehave more then it already is.

I can revise it if needed.


>> ---
>>
>> --- a/drivers/net/wireless/rt2x00/rt2800lib.c ? 2010-03-26
>> 18:25:50.000000000 +0000
>> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c ? 2010-04-01
>> 13:05:18.249747122 +0100
>> @@ -221,9 +221,9 @@
>> ? ? ? ?u32 reg;
>>
>> ? ? ? ?/*
>> - ? ? ? ?* SOC devices don't support MCU requests.
>> + ? ? ? ?* some devices don't support MCU requests.
>> ? ? ? ? */
>> - ? ? ? if (rt2x00_is_soc(rt2x00dev))
>> + ? ? ? if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
>> ? ? ? ? ? ? ? ?return;
>>
>> ? ? ? ?mutex_lock(&rt2x00dev->csr_mutex);
>> --- a/drivers/net/wireless/rt2x00/rt2800pci.c ? 2010-03-26
>> 18:25:50.000000000 +0000
>> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c ? 2010-04-01
>> 13:04:42.453621607 +0100
>> @@ -59,6 +59,12 @@
>> ?{
>> ? ? ? ?unsigned int i;
>> ? ? ? ?u32 reg;
>> +
>> + ? ? ? /*
>> + ? ? ? ?* some devices don't support MCU requests.
>> + ? ? ? ?*/
>> + ? ? ? if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
>> + ? ? ? ? ? ? ? return;
>>
>> ? ? ? ?for (i = 0; i < 200; i++) {
>> ? ? ? ? ? ? ? ?rt2800_register_read(rt2x00dev, H2M_MAILBOX_CID, &reg);
>
> So, the minimal patch would be simply this change to rt2800pci, and to have it
> test for SoC (via rt2x00_is_soc).
>
>> @@ -1098,10 +1104,12 @@
>> ? ? ? ?__set_bit(DRIVER_SUPPORT_CONTROL_FILTER_PSPOLL, &rt2x00dev->flags);
>>
>> ? ? ? ?/*
>> - ? ? ? ?* This device requires firmware.
>> + ? ? ? ?* This device requires firmware and MCU access.
>> ? ? ? ? */
>> - ? ? ? if (!rt2x00_is_soc(rt2x00dev))
>> + ? ? ? if (!rt2x00_is_soc(rt2x00dev)) {
>> ? ? ? ? ? ? ? ?__set_bit(DRIVER_REQUIRE_FIRMWARE, &rt2x00dev->flags);
>> + ? ? ? ? ? ? ? __set_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags);
>> + ? ? ? }
>> ? ? ? ?__set_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags);
>> ? ? ? ?__set_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags);
>> ? ? ? ?if (!modparam_nohwcrypt)
>> --- a/drivers/net/wireless/rt2x00/rt2x00.h ? ? ?2010-03-26
>> 18:25:50.000000000 +0000
>> +++ b/drivers/net/wireless/rt2x00/rt2x00.h ? ? ?2010-04-01
>> 13:01:26.812694036 +0100
>> @@ -631,6 +631,7 @@
>> ? ? ? ? * Driver requirements
>> ? ? ? ? */
>> ? ? ? ?DRIVER_REQUIRE_FIRMWARE,
>> + ? ? ? DRIVER_REQUIRE_MCU,
>> ? ? ? ?DRIVER_REQUIRE_BEACON_GUARD,
>> ? ? ? ?DRIVER_REQUIRE_ATIM_QUEUE,
>> ? ? ? ?DRIVER_REQUIRE_DMA,
>
> If we choose to have the flag anyways:
> From a naming point of view, this name is aligned with the other flags.
> However, from a usage point of view it would be better to have a flag
> DRIVER_NO_MCU, so we don't have to have the negative tests above, and
> have no tests at all that determine if MCU is allowed.

Good point, lets wait on the decision above to make this change.

>
> ---
> Gertjan.

As you all know, i'm not really a programmer ;)

Luis Correia
rt2x00 project admin

2010-04-01 21:44:12

by Gertjan van Wingerde

Subject: Re: [PATCH V3] rt2x00: remove MCU requests for SoC platforms

Luis,

On 04/01/10 23:14, Luis Correia wrote:
> The ralink SoC platforms do not have an MCU.
>
> Signed-off-by: Luis Correia <[email protected]>

I know Ivo already acked the v2 version of the patch, but isn't the
addition of a driver flag a bit overkill?

We have the test on whether the platform is SOC w.r.t. MCU requests
in 2 places, and both of them are in rt2800 code. I do not really see
a need to clutter the global rt2x00 space with a rt2800 specific flag,
which is only used in rt2800 code.

> ---
>
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c 2010-03-26
> 18:25:50.000000000 +0000
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c 2010-04-01
> 13:05:18.249747122 +0100
> @@ -221,9 +221,9 @@
> u32 reg;
>
> /*
> - * SOC devices don't support MCU requests.
> + * some devices don't support MCU requests.
> */
> - if (rt2x00_is_soc(rt2x00dev))
> + if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
> return;
>
> mutex_lock(&rt2x00dev->csr_mutex);
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c 2010-03-26
> 18:25:50.000000000 +0000
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c 2010-04-01
> 13:04:42.453621607 +0100
> @@ -59,6 +59,12 @@
> {
> unsigned int i;
> u32 reg;
> +
> + /*
> + * some devices don't support MCU requests.
> + */
> + if (!test_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags))
> + return;
>
> for (i = 0; i < 200; i++) {
> rt2800_register_read(rt2x00dev, H2M_MAILBOX_CID, &reg);

So, the minimal patch would be simply this change to rt2800pci, and to have it
test for SoC (via rt2x00_is_soc).

> @@ -1098,10 +1104,12 @@
> __set_bit(DRIVER_SUPPORT_CONTROL_FILTER_PSPOLL, &rt2x00dev->flags);
>
> /*
> - * This device requires firmware.
> + * This device requires firmware and MCU access.
> */
> - if (!rt2x00_is_soc(rt2x00dev))
> + if (!rt2x00_is_soc(rt2x00dev)) {
> __set_bit(DRIVER_REQUIRE_FIRMWARE, &rt2x00dev->flags);
> + __set_bit(DRIVER_REQUIRE_MCU, &rt2x00dev->flags);
> + }
> __set_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags);
> __set_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags);
> if (!modparam_nohwcrypt)
> --- a/drivers/net/wireless/rt2x00/rt2x00.h 2010-03-26
> 18:25:50.000000000 +0000
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h 2010-04-01
> 13:01:26.812694036 +0100
> @@ -631,6 +631,7 @@
> * Driver requirements
> */
> DRIVER_REQUIRE_FIRMWARE,
> + DRIVER_REQUIRE_MCU,
> DRIVER_REQUIRE_BEACON_GUARD,
> DRIVER_REQUIRE_ATIM_QUEUE,
> DRIVER_REQUIRE_DMA,

If we choose to have the flag anyways:
>From a naming point of view, this name is aligned with the other flags.
However, from a usage point of view it would be better to have a flag
DRIVER_NO_MCU, so we don't have to have the negative tests above, and
have no tests at all that determine if MCU is allowed.

---
Gertjan.

2010-04-02 06:57:48

by Ivo Van Doorn

Subject: Re: [PATCH V3] rt2x00: remove MCU requests for SoC platforms

Hi,

On Thu, Apr 1, 2010 at 11:44 PM, Gertjan van Wingerde
<[email protected]> wrote:
> Luis,
>
> On 04/01/10 23:14, Luis Correia wrote:
>> The ralink SoC platforms do not have an MCU.
>>
>> Signed-off-by: Luis Correia <[email protected]>
>
> I know Ivo already acked the v2 version of the patch, but isn't the
> addition of a driver flag a bit overkill?
>
> We have the test on whether the platform is SOC w.r.t. MCU requests
> in 2 places, and both of them are in rt2800 code. I do not really see
> a need to clutter the global rt2x00 space with a rt2800 specific flag,
> which is only used in rt2800 code.

I suggested the new flag to Luis. I wonder if a case exists where the driver
has no firmware but does need MCU access. I already considered it strange
when Ralink released a chip without firmware while other revisions did need it.
If this combination would be extremely unlikely, then we can indeed remove
the flag.

Ivo