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

Reply via email to