Re: [PATCH v2 2/2] can: usb: esd_usb2: Add support for CAN-USB/Micro

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

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

On 10/29/2012 10:54 AM, matthias.fuchs@esd.eu wrote:
> From: Matthias Fuchs <matthias.fuchs@esd.eu>
> 
> This patch extends the esd_usb2 driver to support the
> tiny CAN-USB/Micro CAN/USB interface.
> 
> Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
> ---
>  drivers/net/can/usb/esd_usb2.c |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index f4317c0..124e0dd 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -1,7 +1,7 @@
>  /*
> - * CAN driver for esd CAN-USB/2
> + * CAN driver for esd CAN-USB/2 and CAN-USB/Micro
>   *
> - * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
> + * Copyright (C) 2010-2012 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms of the GNU General Public License as published
> @@ -28,14 +28,16 @@
>  #include <linux/can/error.h>
>  
>  MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs@esd.eu>");
> -MODULE_DESCRIPTION("CAN driver for esd CAN-USB/2 interfaces");
> +MODULE_DESCRIPTION("CAN driver for esd CAN-USB/2 and CAN-USB/Micro interfaces");
>  MODULE_LICENSE("GPL v2");
>  
>  /* Define these values to match your devices */
>  #define USB_ESDGMBH_VENDOR_ID	0x0ab4
>  #define USB_CANUSB2_PRODUCT_ID	0x0010
> +#define USB_CANUSBM_PRODUCT_ID	0x0011
>  
>  #define ESD_USB2_CAN_CLOCK	60000000
> +#define ESD_USBM_CAN_CLOCK	36000000
>  #define ESD_USB2_MAX_NETS	2
>  
>  /* USB2 commands */
> @@ -69,6 +71,7 @@ MODULE_LICENSE("GPL v2");
>  #define ESD_USB2_TSEG2_SHIFT	20
>  #define ESD_USB2_SJW_MAX	4
>  #define ESD_USB2_SJW_SHIFT	14
> +#define ESD_USBM_SJW_SHIFT	24
>  #define ESD_USB2_BRP_MIN	1
>  #define ESD_USB2_BRP_MAX	1024
>  #define ESD_USB2_BRP_INC	1
> @@ -183,6 +186,7 @@ struct __attribute__ ((packed)) esd_usb2_msg {
>  
>  static struct usb_device_id esd_usb2_table[] = {
>  	{USB_DEVICE(USB_ESDGMBH_VENDOR_ID, USB_CANUSB2_PRODUCT_ID)},
> +	{USB_DEVICE(USB_ESDGMBH_VENDOR_ID, USB_CANUSBM_PRODUCT_ID)},
>  	{}

I just noticed that you can add a driver_info pointer to the struct
usb_device_id [1].

You can define a struct describing your hardware:

struct esd_driver_info {
	int sjw_shift;
	int clock_freq;	
};

struct esd_driver_info esd_usb2 = {
	.sjw_shift = 23,
	.clock_freq = 42,
};

static struct usb_device_id esd_usb2_table[] = {
  	{USB_DEVICE(USB_ESDGMBH_VENDOR_ID, USB_CANUSB2_PRODUCT_ID),
		.driver_info = (kernel_ulong_t)&esd_usb2, },
};

And then access the driver_info pointer in the probe function (via
id->driver_info)....

Bit I think your solution is easier. If you add more device a conversion
might become handy.

>  };
>  MODULE_DEVICE_TABLE(usb, esd_usb2_table);
> @@ -889,14 +893,22 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
>  	struct can_bittiming *bt = &priv->can.bittiming;
>  	struct esd_usb2_msg msg;
>  	u32 canbtr;
> +	int sjw_shift;
>  
>  	canbtr = ESD_USB2_UBR;
>  	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>  		canbtr |= ESD_USB2_LOM;
>  
>  	canbtr |= (bt->brp - 1) & (ESD_USB2_BRP_MAX - 1);
> +
> +	if (le16_to_cpu(priv->usb2->udev->descriptor.idProduct) ==
> +	    USB_CANUSBM_PRODUCT_ID)
> +		sjw_shift = ESD_USBM_SJW_SHIFT;
> +	else
> +		sjw_shift = ESD_USB2_SJW_SHIFT;
> +
>  	canbtr |= ((bt->sjw - 1) & (ESD_USB2_SJW_MAX - 1))
> -		<< ESD_USB2_SJW_SHIFT;
> +		<< sjw_shift;
>  	canbtr |= ((bt->prop_seg + bt->phase_seg1 - 1)
>  		   & (ESD_USB2_TSEG1_MAX - 1))
>  		<< ESD_USB2_TSEG1_SHIFT;
> @@ -975,12 +987,19 @@ static int esd_usb2_probe_one_net(struct usb_interface *intf, int index)
>  
>  	priv->can.state = CAN_STATE_STOPPED;
>  	priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY;
> -	priv->can.clock.freq = ESD_USB2_CAN_CLOCK;
> +
> +	if (le16_to_cpu(dev->udev->descriptor.idProduct) ==
> +	    USB_CANUSBM_PRODUCT_ID)
> +		priv->can.clock.freq = ESD_USBM_CAN_CLOCK;
> +	else {
> +		priv->can.clock.freq = ESD_USB2_CAN_CLOCK;
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> +	}
> +
>  	priv->can.bittiming_const = &esd_usb2_bittiming_const;
>  	priv->can.do_set_bittiming = esd_usb2_set_bittiming;
>  	priv->can.do_set_mode = esd_usb2_set_mode;
>  	priv->can.do_get_berr_counter = esd_usb2_get_berr_counter;
> -	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
>  
>  	netdev->flags |= IFF_ECHO; /* we support local echo */
>  
> 

Marc

[1]
http://lxr.free-electrons.com/source/include/linux/mod_devicetable.h#L101

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