[PATCH] pch_can: fix error passive level test

[PATCH] pch_can: fix error passive level test

[Xi Wang]

The test (((errc & PCH_REC) >> 8) > 127) would always be false because
the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
PCH_REC is defined as 0x7f00.  To test whether the receive error counter
has reached the error passive level, the RP bit (15) should be used.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 drivers/net/can/pch_can.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index d11fbb2..6edc25e 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -66,6 +66,7 @@
 #define PCH_IF_CREQ_BUSY	BIT(15)
 
 #define PCH_STATUS_INT		0x8000
+#define PCH_RP			0x00008000
 #define PCH_REC			0x00007f00
 #define PCH_TEC			0x000000ff
 
@@ -527,7 +528,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 		priv->can.can_stats.error_passive++;
 		state = CAN_STATE_ERROR_PASSIVE;
 		cf->can_id |= CAN_ERR_CRTL;
-		if (((errc & PCH_REC) >> 8) > 127)
+		if (errc & PCH_RP)
 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
 		if ((errc & PCH_TEC) > 127)
 			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
-- 
1.7.5.4

Re: [PATCH] pch_can: fix error passive level test

[Wolfgang Grandegger]

On 12/12/2011 08:16 AM, Xi Wang wrote:
> The test (((errc & PCH_REC) >> 8) > 127) would always be false because
> the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
> PCH_REC is defined as 0x7f00.  To test whether the receive error counter
> has reached the error passive level, the RP bit (15) should be used.
> 
> Signed-off-by: Xi Wang <xi.wang@gmail.com>

Acked-by: Wolfgang Grandegger <wg@grandegger.com>

The C_CAN driver, which supports the same CAN controller, does handle
the error passive state correctly. This reminds me to get rid of pch_can
in favor of C_CAN sooner than later.

Thanks,

Wolfgang.

Re: [PATCH] pch_can: fix error passive level test

[Marc Kleine-Budde]

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

On 12/12/2011 09:05 AM, Wolfgang Grandegger wrote:
> On 12/12/2011 08:16 AM, Xi Wang wrote:
>> The test (((errc & PCH_REC) >> 8) > 127) would always be false because
>> the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
>> PCH_REC is defined as 0x7f00.  To test whether the receive error counter
>> has reached the error passive level, the RP bit (15) should be used.
>>
>> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> 
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>

Is this patch a candidate for stable?

> The C_CAN driver, which supports the same CAN controller, does handle
> the error passive state correctly. This reminds me to get rid of pch_can
> in favor of C_CAN sooner than later.

+1

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] pch_can: fix error passive level test

[Wolfgang Grandegger]

On 12/12/2011 10:17 AM, Marc Kleine-Budde wrote:
> On 12/12/2011 09:05 AM, Wolfgang Grandegger wrote:
>> On 12/12/2011 08:16 AM, Xi Wang wrote:
>>> The test (((errc & PCH_REC) >> 8) > 127) would always be false because
>>> the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
>>> PCH_REC is defined as 0x7f00.  To test whether the receive error counter
>>> has reached the error passive level, the RP bit (15) should be used.
>>>
>>> Signed-off-by: Xi Wang <xi.wang@gmail.com>
>>
>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
> 
> Is this patch a candidate for stable?

You mean for the "net" branch? Yes, I think so.

Wolfgang.

Re: [PATCH] pch_can: fix error passive level test

[Marc Kleine-Budde]

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

On 12/12/2011 10:31 AM, Wolfgang Grandegger wrote:
> On 12/12/2011 10:17 AM, Marc Kleine-Budde wrote:
>> On 12/12/2011 09:05 AM, Wolfgang Grandegger wrote:
>>> On 12/12/2011 08:16 AM, Xi Wang wrote:
>>>> The test (((errc & PCH_REC) >> 8) > 127) would always be false because
>>>> the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
>>>> PCH_REC is defined as 0x7f00.  To test whether the receive error counter
>>>> has reached the error passive level, the RP bit (15) should be used.
>>>>
>>>> Signed-off-by: Xi Wang <xi.wang@gmail.com>
>>>
>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
>>
>> Is this patch a candidate for stable?
> 
> You mean for the "net" branch? Yes, I think so.

Even for all trees which contain this driver (in a working version),
which is v2.6.38 and newer.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] pch_can: fix error passive level test

[Wolfgang Grandegger]

On 12/12/2011 10:39 AM, Marc Kleine-Budde wrote:
> On 12/12/2011 10:31 AM, Wolfgang Grandegger wrote:
>> On 12/12/2011 10:17 AM, Marc Kleine-Budde wrote:
>>> On 12/12/2011 09:05 AM, Wolfgang Grandegger wrote:
>>>> On 12/12/2011 08:16 AM, Xi Wang wrote:
>>>>> The test (((errc & PCH_REC) >> 8) > 127) would always be false because
>>>>> the receive error counter ((errc & PCH_REC) >> 8) is at most 127, where
>>>>> PCH_REC is defined as 0x7f00.  To test whether the receive error counter
>>>>> has reached the error passive level, the RP bit (15) should be used.
>>>>>
>>>>> Signed-off-by: Xi Wang <xi.wang@gmail.com>
>>>>
>>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
>>>
>>> Is this patch a candidate for stable?
>>
>> You mean for the "net" branch? Yes, I think so.
> 
> Even for all trees which contain this driver (in a working version),
> which is v2.6.38 and newer.

OK, Well, it's not a serious fix, at least.

Wolfgang.

Re: [PATCH] pch_can: fix error passive level test

[Xi Wang]

On Dec 12, 2011, at 4:50 AM, Wolfgang Grandegger wrote:
> On 12/12/2011 10:39 AM, Marc Kleine-Budde wrote:
>> 
>> Even for all trees which contain this driver (in a working version),
>> which is v2.6.38 and newer.
> 
> OK, Well, it's not a serious fix, at least.

Should I resend the patch and cc to stable?  Thanks.

- xi