Re: Driver for CAN in at91

Re: Driver for CAN in at91

[Marc Kleine-Budde]

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

On 02/06/2014 03:13 PM, Yoann DI RUZZA wrote:
> I'll send you a patch to add the listen only mode in AT91 CAN driver.
> I don't know if it is the good way to send a patch.

Thanks for your contribution, please add the linux-can mailinglist on
Cc, too.

> I'm working and testing this patch with the 3.6.9 kernel version on a
> at91sam9x35-ek.
> cordially,
> 
> Yoann DI RUZZA

Can you add your Signed-off-by to the next patch. See
http://elinux.org/Developer_Certificate_Of_Origin for more information.

> 
> can_listenonly.patch
> 
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index 1cf6104..d4c3213 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -422,7 +422,11 @@ static void at91_chip_start(struct net_device *dev)
>  	at91_transceiver_switch(priv, 1);
>  
>  	/* enable chip */
> -	at91_write(priv, AT91_MR, AT91_MR_CANEN);
> +	if(priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> +		reg_mr = reg_mr | AT91_MR_CANEN | AT91_MR_ABM;
> +	else
> +		reg_mr = reg_mr | AT91_MR_CANEN;

The documentation says it's "autobaud/liste mode". Is this another name
for listen only?

Your patch does change what's written to the CAN core, the original code
writes only AT91_MR_CANEN, you preserve the original contents. Who will
clear AT91_MR_ABM if you switch from listen only to normal mode?

> +	at91_write(priv, AT91_MR, reg_mr);
>  
>  	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  
> @@ -1345,7 +1349,7 @@ static int __devinit at91_can_probe(struct platform_device *pdev)
>  	priv->can.bittiming_const = &at91_bittiming_const;
>  	priv->can.do_set_mode = at91_set_mode;
>  	priv->can.do_get_berr_counter = at91_get_berr_counter;
> -	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY;
>  	priv->dev = dev;
>  	priv->reg_base = addr;
>  	priv->devtype_data = *devtype_data;
> 

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: 242 bytes --]

Re: Driver for CAN in at91

[Marc Kleine-Budde]

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

On 02/11/2014 09:46 AM, Yoann DI RUZZA wrote:
>     > diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>     > index 1cf6104..d4c3213 100644
>     > --- a/drivers/net/can/at91_can.c
>     > +++ b/drivers/net/can/at91_can.c
>     > @@ -422,7 +422,11 @@ static void at91_chip_start(struct net_device
>     *dev)
>     >       at91_transceiver_switch(priv, 1);
>     >
>     >       /* enable chip */
>     > -     at91_write(priv, AT91_MR, AT91_MR_CANEN);
>     > +     if(priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>     > +             reg_mr = reg_mr | AT91_MR_CANEN | AT91_MR_ABM;
>     > +     else
>     > +             reg_mr = reg_mr | AT91_MR_CANEN;
> 
>     The documentation says it's "autobaud/liste mode". Is this another name
>     for listen only?
> 
> Yes, in this mode you can make autobauding because you don't have bus
> error even if you don't have the right baudrate.  
> LISTEN_ONLY is the word used on the other net can driver (mcp251x for
> example).

Thanks for clarifying this.

> 
>     Your patch does change what's written to the CAN core, the original code
>     writes only AT91_MR_CANEN, you preserve the original contents. Who will
>     clear AT91_MR_ABM if you switch from listen only to normal mode?
> 
>  
> I modify the patch to correct this.
> thanks

Thanks for the patch, please send it via "git send-email" next time, as
you mailer has converted tabs into spaces. I've applied the patch manually.

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: 242 bytes --]