Re: [PATCH 15/17] can: ems_usb: In CAN error handling routine checking which CAN controller type is issuing the error

[Marc Kleine-Budde <mkl@pengutronix.de>]

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

On 11/6/20 6:02 PM, Gerhard Uttenthaler wrote:
> Signed-off-by: Gerhard Uttenthaler <uttenthaler@ems-wuensche.com>
> ---
>  drivers/net/can/usb/ems_usb.c | 77 +++++++++++++++++++----------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index c36f02eeec85..4a67c57c4760 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -103,6 +103,9 @@ MODULE_LICENSE("GPL v2");
>  
>  #define SJA1000_DEFAULT_OUTPUT_CONTROL 0xDA
>  
> +#define SJA1000   2 // NXP basic CAN controller
> +#define LPC546XX  5 // NXP CAN FD controller

please add a proper prefix to these definees

> +
>  /* CPC-USB/ARM7 actually uses a 16MHz clock to generate the CAN clock
>   * but it expects SJA1000 bit settings based on 8MHz (is internally
>   * converted).
> @@ -441,6 +444,9 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
>  	if (!skb)
>  		return;
>  
> +	/* The CPC_MSG_TYPE_CAN_STATE works for both
> +	 * CPC-USB/ARM7 and CPC-USB/FD
> +	 */
>  	if (msg->type == CPC_MSG_TYPE_CAN_STATE) {
>  		u8 state = msg->msg.can_state;
>  
> @@ -458,40 +464,43 @@ static void ems_usb_rx_err(struct ems_usb *dev, struct ems_cpc_msg *msg)
>  			dev->can.can_stats.error_passive++;
>  		}
>  	} else if (msg->type == CPC_MSG_TYPE_CAN_FRAME_ERROR) {
> -		u8 ecc = msg->msg.error.cc.regs.sja1000.ecc;
> -		u8 txerr = msg->msg.error.cc.regs.sja1000.txerr;
> -		u8 rxerr = msg->msg.error.cc.regs.sja1000.rxerr;
> -
> -		/* bus error interrupt */
> -		dev->can.can_stats.bus_error++;
> -		stats->rx_errors++;
> -
> -		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> -
> -		switch (ecc & SJA1000_ECC_MASK) {
> -		case SJA1000_ECC_BIT:
> -			cf->data[2] |= CAN_ERR_PROT_BIT;
> -			break;
> -		case SJA1000_ECC_FORM:
> -			cf->data[2] |= CAN_ERR_PROT_FORM;
> -			break;
> -		case SJA1000_ECC_STUFF:
> -			cf->data[2] |= CAN_ERR_PROT_STUFF;
> -			break;
> -		default:
> -			cf->data[3] = ecc & SJA1000_ECC_SEG;
> -			break;
> -		}
> -
> -		/* Error occurred during transmission? */
> -		if ((ecc & SJA1000_ECC_DIR) == 0)
> -			cf->data[2] |= CAN_ERR_PROT_TX;
> -
> -		if (dev->can.state == CAN_STATE_ERROR_WARNING ||
> -		    dev->can.state == CAN_STATE_ERROR_PASSIVE) {
> -			cf->can_id |= CAN_ERR_CRTL;
> -			cf->data[1] = (txerr > rxerr) ?
> -			    CAN_ERR_CRTL_TX_PASSIVE : CAN_ERR_CRTL_RX_PASSIVE;
> +		// CPC-USB/ARM7

no c++ comments

> +		if (msg->msg.error.cc.cc_type == SJA1000) {
> +			u8 ecc = msg->msg.error.cc.regs.sja1000.ecc;
> +			u8 txerr = msg->msg.error.cc.regs.sja1000.txerr;
> +			u8 rxerr = msg->msg.error.cc.regs.sja1000.rxerr;
> +
> +			/* bus error interrupt */
> +			dev->can.can_stats.bus_error++;
> +			stats->rx_errors++;
> +
> +			cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> +			switch (ecc & SJA1000_ECC_MASK) {
> +			case SJA1000_ECC_BIT:
> +				cf->data[2] |= CAN_ERR_PROT_BIT;
> +				break;
> +			case SJA1000_ECC_FORM:
> +				cf->data[2] |= CAN_ERR_PROT_FORM;
> +				break;
> +			case SJA1000_ECC_STUFF:
> +				cf->data[2] |= CAN_ERR_PROT_STUFF;
> +				break;
> +			default:
> +				cf->data[3] = ecc & SJA1000_ECC_SEG;
> +				break;
> +			}
> +
> +			/* Error occurred during transmission? */
> +			if ((ecc & SJA1000_ECC_DIR) == 0)
> +				cf->data[2] |= CAN_ERR_PROT_TX;
> +
> +			if (dev->can.state == CAN_STATE_ERROR_WARNING ||
> +			    dev->can.state == CAN_STATE_ERROR_PASSIVE) {
> +				cf->can_id |= CAN_ERR_CRTL;
> +				cf->data[1] = (txerr > rxerr) ?
> +				    CAN_ERR_CRTL_TX_PASSIVE : CAN_ERR_CRTL_RX_PASSIVE;
> +			}
>  		}
>  	} else if (msg->type == CPC_MSG_TYPE_OVERRUN) {
>  		cf->can_id |= CAN_ERR_CRTL;
> 

Marc

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


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