From: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public•gmane.org>
To: Sebastian Haas <haas-zsNKPWJ8Pib6hrUXjxyGrA@public•gmane.org>
Cc: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public•gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
oliver-GvhC2dPhHPQdnm+yROfE0A@public•gmane.org,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org,
oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public•gmane.org,
greg-U8xfFu+wG4EAvxtiuMwx3w@public•gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public•gmane.org
Subject: Re: [PATCH V2 2/2] ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface
Date: Wed, 16 Sep 2009 11:26:27 +0200 [thread overview]
Message-ID: <4AB0AF43.6090605@grandegger.com> (raw)
In-Reply-To: <20090916090418.12370.16510.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Sebastian Haas wrote:
> This patch adds support for one channel CAN/USB interace CPC-USB/ARM7 from
> EMS Dr. Thomas Wuensche (http://www.ems-wuensche.com).
>
> Signed-off-by: Sebastian Haas <haas-zsNKPWJ8Pib6hrUXjxyGrA@public•gmane.org>
The cleanup in the probe function is still not OK :-(.
> ---
>
> drivers/net/can/Kconfig | 7
> drivers/net/can/Makefile | 2
> drivers/net/can/usb/Makefile | 5
> drivers/net/can/usb/ems_usb.c | 1151 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1165 insertions(+), 0 deletions(-)
> create mode 100644 drivers/net/can/usb/Makefile
> create mode 100644 drivers/net/can/usb/ems_usb.c
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 0900743..6ac5aa5 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -75,6 +75,13 @@ config CAN_EMS_PCI
> CPC-PCIe and CPC-104P cards from EMS Dr. Thomas Wuensche
> (http://www.ems-wuensche.de).
>
> +config CAN_EMS_USB
> + tristate "EMS CPC-USB/ARM7 CAN/USB interface"
> + depends on USB && CAN_DEV
> + ---help---
> + This driver is for the one channel CPC-USB/ARM7 CAN/USB interface
> + from from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
> +
> config CAN_KVASER_PCI
> tristate "Kvaser PCIcanx and Kvaser PCIcan PCI Cards"
> depends on PCI && CAN_SJA1000
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 523a941..9b035ec 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -7,6 +7,8 @@ obj-$(CONFIG_CAN_VCAN) += vcan.o
> obj-$(CONFIG_CAN_DEV) += can-dev.o
> can-dev-y := dev.o
>
> +obj-y += usb/
> +
> obj-$(CONFIG_CAN_SJA1000) += sja1000/
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> new file mode 100644
> index 0000000..c3f75ba
> --- /dev/null
> +++ b/drivers/net/can/usb/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the Linux Controller Area Network USB drivers.
> +#
> +
> +obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
[snip]
> +/*
> + * probe function for new CPC-USB devices
> + */
> +static int ems_usb_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct net_device *netdev;
> + struct ems_usb *dev;
> + int i, err;
> + int retval = -ENOMEM;
Why do you need an extra variable here.
int err = -ENOMEM;
Should work fine!?
> +
> + netdev = alloc_candev(sizeof(struct ems_usb));
> + if (!netdev) {
> + dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + dev = netdev_priv(netdev);
> +
> + dev->udev = interface_to_usbdev(intf);
> + dev->netdev = netdev;
> +
> + dev->can.state = CAN_STATE_STOPPED;
> + dev->can.clock.freq = EMS_USB_ARM7_CLOCK;
> + dev->can.bittiming_const = &ems_usb_bittiming_const;
> + dev->can.do_set_bittiming = ems_usb_set_bittiming;
> + dev->can.do_set_mode = ems_usb_set_mode;
> +
> + netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> + /*
> + * The device actually uses a 16MHz clock to generate the CAN clock
> + * but it expects SJA1000 bit settings based on 8MHz (is internally
> + * converted).
> + */
> +
> + netdev->netdev_ops = &ems_usb_netdev_ops;
> +
> + netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> + init_usb_anchor(&dev->rx_submitted);
> +
> + init_usb_anchor(&dev->tx_submitted);
> + atomic_set(&dev->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + dev->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + dev->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!dev->intr_urb) {
> + dev_err(netdev->dev.parent, "Couldn't alloc intr URB\n");
> + goto cleanup_candev;
> + }
> +
> + dev->intr_in_buffer = kzalloc(INTR_IN_BUFFER_SIZE, GFP_KERNEL);
> + if (!dev->intr_in_buffer) {
> + dev_err(netdev->dev.parent, "Couldn't alloc Intr buffer\n");
> + goto cleanup_intr_urb;
> + }
> +
> + dev->tx_msg_buffer = kzalloc(CPC_HEADER_SIZE +
> + sizeof(struct ems_cpc_msg), GFP_KERNEL);
> + if (!dev->tx_msg_buffer) {
> + dev_err(netdev->dev.parent, "Couldn't alloc Tx buffer\n");
> + goto cleanup_intr_in_buffer;
> + }
> +
> + usb_set_intfdata(intf, dev);
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + init_params_sja1000(&dev->active_params);
> +
> + err = ems_usb_command_msg(dev, &dev->active_params);
> + if (err) {
> + dev_err(netdev->dev.parent,
> + "couldn't initialize controller: %d\n", err);
> + retval = err;
Could be removed then.
> + goto cleanup_tx_msg_buffer;
> + }
> +
> + return register_candev(netdev);
Cleanup is missing if register_candev() fails!
> +
> +cleanup_tx_msg_buffer:
> + kfree(dev->tx_msg_buffer);
> +
> +cleanup_intr_in_buffer:
> + kfree(dev->intr_in_buffer);
> +
> +cleanup_intr_urb:
> + usb_free_urb(dev->intr_urb);
> +
> +cleanup_candev:
> + free_candev(netdev);
> +
> + return retval;
> +}
Wolfgang.
prev parent reply other threads:[~2009-09-16 9:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-16 9:04 [PATCH V2 0/2 -next] ems_usb: Added support for EMS CPC-USB/ARM7 CAN interface Sebastian Haas
[not found] ` <20090916090408.12370.94680.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-09-16 9:04 ` [PATCH V2 1/2] cpc-usb: Removed driver from staging tree Sebastian Haas
2009-09-16 9:04 ` [PATCH V2 2/2] ems_usb: Added support for EMS CPC-USB/ARM7 CAN/USB interface Sebastian Haas
[not found] ` <20090916090418.12370.16510.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-09-16 9:26 ` Wolfgang Grandegger [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AB0AF43.6090605@grandegger.com \
--to=wg-5yr1bzd7o62+xt7jha+gda@public$(echo .)gmane.org \
--cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public$(echo .)gmane.org \
--cc=eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public$(echo .)gmane.org \
--cc=greg-U8xfFu+wG4EAvxtiuMwx3w@public$(echo .)gmane.org \
--cc=haas-zsNKPWJ8Pib6hrUXjxyGrA@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=oliver-GvhC2dPhHPQdnm+yROfE0A@public$(echo .)gmane.org \
--cc=oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public$(echo .)gmane.org \
--cc=socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox