| Message ID | 1323674180-16916-1-git-send-email-xi.wang@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)