RFC: [PATCH] can: c_can: disable one shot mode until driver is fixed

RFC: [PATCH] can: c_can: disable one shot mode until driver is fixed

[Marc Kleine-Budde]

This patch disables the one shot mode, until the driver has been fixed and
tested to support it.

> I'm quite sure I've seen a situation where msg_obj 17 "seemed" to be
> pending, while msg_obj 18 and 19 already have been transmitted. But
> in that case, I enabled ONESHOT for the can interface, which enables
> the DA mode (automatic retransmission is disabled).

Reported-by: Jan Altenberg <jan-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
---
 drivers/net/can/c_can/c_can.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 110eda0..2040959 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -1112,8 +1112,7 @@ struct net_device *alloc_c_can_dev(void)
 	priv->can.bittiming_const = &c_can_bittiming_const;
 	priv->can.do_set_mode = c_can_set_mode;
 	priv->can.do_get_berr_counter = c_can_get_berr_counter;
-	priv->can.ctrlmode_supported = CAN_CTRLMODE_ONE_SHOT |
-					CAN_CTRLMODE_LOOPBACK |
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
 					CAN_CTRLMODE_LISTENONLY |
 					CAN_CTRLMODE_BERR_REPORTING;
 
-- 
1.7.2.3

Re: RFC: [PATCH] can: c_can: disable one shot mode until driver is fixed

[Kurt Van Dijck]

On Thu, Mar 24, 2011 at 11:12:29AM +0100, Marc Kleine-Budde wrote:
> This patch disables the one shot mode, until the driver has been fixed and
> tested to support it.
> 
isn't this part necessary now (temporarily) to avoid dead code?

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 1405078..2c3d090 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -588,14 +588,9 @@ static void c_can_chip_config(struct net_device *dev)
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
-		/* disable automatic retransmission */
-		priv->write_reg(priv, &priv->regs->control,
-				CONTROL_DISABLE_AR);
-	else
-		/* enable automatic retransmission */
-		priv->write_reg(priv, &priv->regs->control,
-				CONTROL_ENABLE_AR);
+	/* enable automatic retransmission */
+	priv->write_reg(priv, &priv->regs->control,
+			CONTROL_ENABLE_AR);
 
 	if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
 					CAN_CTRLMODE_LOOPBACK)) {

Re: RFC: [PATCH] can: c_can: disable one shot mode until driver is fixed

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1823 bytes --]

On 03/24/2011 11:28 AM, Kurt Van Dijck wrote:
> On Thu, Mar 24, 2011 at 11:12:29AM +0100, Marc Kleine-Budde wrote:
>> This patch disables the one shot mode, until the driver has been fixed and
>> tested to support it.
>>
> isn't this part necessary now (temporarily) to avoid dead code?

It's not necessary, the driver should still work. But it's dead code. If
someone fixes the driver she/he can easily revert the patch.

Can I fold your patch and add your S-o-b?

Marc
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 1405078..2c3d090 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -588,14 +588,9 @@ static void c_can_chip_config(struct net_device *dev)
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> -		/* disable automatic retransmission */
> -		priv->write_reg(priv, &priv->regs->control,
> -				CONTROL_DISABLE_AR);
> -	else
> -		/* enable automatic retransmission */
> -		priv->write_reg(priv, &priv->regs->control,
> -				CONTROL_ENABLE_AR);
> +	/* enable automatic retransmission */
> +	priv->write_reg(priv, &priv->regs->control,
> +			CONTROL_ENABLE_AR);
>  
>  	if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
>  					CAN_CTRLMODE_LOOPBACK)) {
> 
> _______________________________________________
> Socketcan-core mailing list
> Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> https://lists.berlios.de/mailman/listinfo/socketcan-core


-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

Re: RFC: [PATCH] can: c_can: disable one shot mode until driver is fixed

[Kurt Van Dijck]

On Thu, Mar 24, 2011 at 11:44:46AM +0100, Marc Kleine-Budde wrote:
> On 03/24/2011 11:28 AM, Kurt Van Dijck wrote:
> > On Thu, Mar 24, 2011 at 11:12:29AM +0100, Marc Kleine-Budde wrote:
> >> This patch disables the one shot mode, until the driver has been fixed and
> >> tested to support it.
> >>
> > isn't this part necessary now (temporarily) to avoid dead code?
> 
> It's not necessary, the driver should still work.
Yep, I saw that.
> But it's dead code. If
> someone fixes the driver she/he can easily revert the patch.
> 
> Can I fold your patch and add your S-o-b?
Yes.
> 
Kurt