From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756583AbdEKV65 (ORCPT ); Thu, 11 May 2017 17:58:57 -0400 Received: from mout.gmx.net ([212.227.15.18]:57907 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752901AbdEKV6y (ORCPT ); Thu, 11 May 2017 17:58:54 -0400 Subject: Re: [PATCH v5 17/17] net: qualcomm: add QCA7000 UART driver To: Stefan Wahren , Rob Herring , "David S. Miller" References: <1494406408-31760-1-git-send-email-stefan.wahren@i2se.com> <1494406408-31760-18-git-send-email-stefan.wahren@i2se.com> Cc: Mark Rutland , Greg Kroah-Hartman , Jiri Slaby , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org From: Lino Sanfilippo Message-ID: Date: Thu, 11 May 2017 23:58:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1494406408-31760-18-git-send-email-stefan.wahren@i2se.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:bzeznRxZpkAgO8BIYaHeRKtUnL2fd5kCAi21a+GKfq1ckFEWeir c8MvINkq4HepIJana/qj6eRsWMYFhhonXjN09PtDHOVCy3CQSYF7N6dEZrbSLZx9CMej6vs XJB7b7Tfuuy4CmOqXXwTyHRTTWD1C5bfrp5x7V44Ix5v6jsTuxUCuHmksYtZvkaqoSWm4Wf MccC+IjOsjlSllkIeqk9A== X-UI-Out-Filterresults: notjunk:1;V01:K0:CVGjnHczz18=:UUKAf+yWfUo2WxmAkDoLd8 CCiE6HZt37GEoDtkhLMwqcX1PqPVwhLoL2C6Dmaosf4yjf71VCMqTPMCKeggEPcLPmyRgel0V vPonwaxCm7Ulj2BWDMcII166CHwnBDgzJQeCFzcIZCm5gFYp7wT/2ljUU7Mxb+4QbaHfK1zvW uTbVn1hoMMP4VPPqq98X45MamYhtPc4ADkpJBQ5UalNS6TlxR3qzGKoWp2iP3cEiFllFiAzqu wWP1JJN4ldzcouflwf4HXJH+Ewuo268BmWrglB29Svj3/gyOigjLvVUZ4sVi0a6ifSCX4mlRC kMpkEfNNgVZJoNhBdPVrpE3ioZbl2udBr/NpJ535juasCmGpHk/Ylnfhd8B/9jxbG8sC/Dx36 +GT/ivq7hxJpu06BgS7acftg3h+g84/FC146BAjLUQNLKJfSB3a67A1g+loTPg50bCI8Qq/em fGWczOQC3veF1YG99lrqhAgpd4b8iNw+GB6BYhF1CXLcraxjB4N1wlJdnu257YsXRkivZ06MM wIgdSQGgzFQED6UEeAy+wEPmd/HjU2T6pgG954kIzyxp9xA10jtpT16EJtVr4up+zjskLtaEU MIZsfYzKziOF0MzT6DRecgm6l7gn+txXE5EZqHm/9RY885eOAldJv728FPRXR2fdHqj1Gnh22 +eKGejFTK9kD0yiWWtr/TEqyeUBb1JsdQpgF5k8ceJJVpdGF3DN1hTbwS4nkeQyApMUY7Weur GbNmy6WWiYzopF/ZOpL6U4h5VmaYxJphY7qRpQBEiSXVpSwPMwHqwzV9cmf7LEZCAm1MTzs3t 81yjXya Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 10.05.2017 10:53, Stefan Wahren wrote: > +static int qcauart_netdev_init(struct net_device *dev) > +{ > + struct qcauart *qca = netdev_priv(dev); > + size_t len; > + > + /* Finish setting up the device info. */ > + dev->mtu = QCAFRM_MAX_MTU; > + dev->type = ARPHRD_ETHER; > + > + qca->rx_skb = netdev_alloc_skb_ip_align(qca->net_dev, > + qca->net_dev->mtu + > + VLAN_ETH_HLEN); > + if (!qca->rx_skb) > + return -ENOBUFS; > + > + len = QCAFRM_HEADER_LEN + QCAFRM_MAX_LEN + QCAFRM_FOOTER_LEN; > + qca->tx_buffer = kmalloc(len, GFP_KERNEL); > + if (!qca->tx_buffer) > + return -ENOBUFS; Freeing the rx_skb is missing here. > + > +static void qcauart_netdev_setup(struct net_device *dev) > +{ > + struct qcauart *qca; > + > + dev->netdev_ops = &qcauart_netdev_ops; > + dev->watchdog_timeo = QCAUART_TX_TIMEOUT; > + dev->priv_flags &= ~IFF_TX_SKB_SHARING; > + dev->tx_queue_len = 100; > + > + /* MTU range: 46 - 1500 */ > + dev->min_mtu = QCAFRM_MIN_MTU; > + dev->max_mtu = QCAFRM_MAX_MTU; > + > + qca = netdev_priv(dev); > + memset(qca, 0, sizeof(struct qcauart)); Zeroing the private data is not necessary since it is already done at device allocation. > + > +static int qca_uart_probe(struct serdev_device *serdev) > +{ > + struct net_device *qcauart_dev = alloc_etherdev(sizeof(struct qcauart)); > + struct qcauart *qca; > + const char *mac; > + u32 speed = 115200; > + int ret; > + > + if (!qcauart_dev) > + return -ENOMEM; > + > + qcauart_netdev_setup(qcauart_dev); > + > + qca = netdev_priv(qcauart_dev); > + if (!qca) { > + pr_err("qca_uart: Fail to retrieve private structure\n"); > + ret = -ENOMEM; > + goto free; > + } > + qca->net_dev = qcauart_dev; > + qca->serdev = serdev; > + qcafrm_fsm_init_uart(&qca->frm_handle); > + > + spin_lock_init(&qca->lock); > + INIT_WORK(&qca->tx_work, qcauart_transmit); > + > + of_property_read_u32(serdev->dev.of_node, "current-speed", &speed); > + > + mac = of_get_mac_address(serdev->dev.of_node); > + > + if (mac) > + ether_addr_copy(qca->net_dev->dev_addr, mac); > + > + if (!is_valid_ether_addr(qca->net_dev->dev_addr)) { > + eth_hw_addr_random(qca->net_dev); > + dev_info(&serdev->dev, "Using random MAC address: %pM\n", > + qca->net_dev->dev_addr); > + } > + > + netif_carrier_on(qca->net_dev); > + serdev_device_set_drvdata(serdev, qca); > + serdev_device_set_client_ops(serdev, &qca_serdev_ops); > + > + ret = serdev_device_open(serdev); > + if (ret) { > + dev_err(&serdev->dev, "Unable to open device %s\n", > + qcauart_dev->name); > + goto free; > + } > + > + speed = serdev_device_set_baudrate(serdev, speed); > + dev_info(&serdev->dev, "Using baudrate: %u\n", speed); > + > + serdev_device_set_flow_control(serdev, false); > + > + ret = register_netdev(qcauart_dev); > + if (ret) { > + dev_err(&serdev->dev, "Unable to register net device %s\n", > + qcauart_dev->name); serdev_device_close() ? > + goto free; > + } > + > + return 0; > + > +free: > + free_netdev(qcauart_dev); > + return ret; > +} > + > +static void qca_uart_remove(struct serdev_device *serdev) > +{ > + struct qcauart *qca = serdev_device_get_drvdata(serdev); Here you should make sure that the tx_work is not running/pending any more (e.g by calling cancel_delayed_work_sync()). > + /* Flush any pending characters in the driver. */ > + serdev_device_close(serdev); > + > + netif_carrier_off(qca->net_dev); > + unregister_netdev(qca->net_dev); > + free_netdev(qca->net_dev); > +} Regards, Lino