> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> +#include <linux/cache.h>
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +
> +#include <linux/spi/spi.h>
> +#include <linux/of_net.h>
> +
> +#define MSG_DEFAULT (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | \
> + NETIF_MSG_TIMER)
> +
> +#define DRV_NAME "mse102x"
> +
> +#define DET_CMD 0x0001
> +#define DET_SOF 0x0002
> +#define DET_DFT 0x55AA
> +
> +#define CMD_SHIFT 12
> +#define CMD_RTS (0x1 << CMD_SHIFT)
> +#define CMD_CTR (0x2 << CMD_SHIFT)
> +
> +#define CMD_MASK GENMASK(15, CMD_SHIFT)
> +#define LEN_MASK GENMASK(CMD_SHIFT - 1, 0)
> +
> +#define DET_CMD_LEN 4
> +#define DET_SOF_LEN 2
> +#define DET_DFT_LEN 2
Looks like these tabs should be spaces?
> +static int msg_enable;
> +module_param_named(message, msg_enable, int, 0);
> +MODULE_PARM_DESC(message, "Message verbosity level (0=none, 31=all)");
I know a lot of drivers do this, but module parameters are not
liked. There is a well used ethtool setting for this, msglvl, which
should be used instead. Which in fact, you have support for.
> +static void mse102x_init_mac(struct mse102x_net *mse, struct device_node *np)
> +{
> + struct net_device *ndev = mse->ndev;
> + int ret = of_get_mac_address(np, ndev->dev_addr);
> +
> + if (ret) {
> + eth_hw_addr_random(ndev);
> + netdev_err(ndev, "Using random MAC address: %pM\n",
> + ndev->dev_addr);
> + }
> +}
No need to tell the hardware? Does it work in promiscuous mode by
default?
> +static int mse102x_net_stop(struct net_device *ndev)
> +{
> + struct mse102x_net *mse = netdev_priv(ndev);
> + struct mse102x_net_spi *mses = to_mse102x_spi(mse);
> +
> + netif_info(mse, ifdown, ndev, "shutting down\n");
> +
> + netif_stop_queue(ndev);
> +
> + /* stop any outstanding work */
> + flush_work(&mses->tx_work);
> +
> + /* ensure any queued tx buffers are dumped */
> + while (!skb_queue_empty(&mse->txq)) {
> + struct sk_buff *txb = skb_dequeue(&mse->txq);
> +
> + netif_dbg(mse, ifdown, ndev,
> + "%s: freeing txb %p\n", __func__, txb);
> +
> + dev_kfree_skb(txb);
> + }
> +
> + free_irq(ndev->irq, mse);
> +
> + return 0;
Maybe a netif_carrier_off() in there, to be symmetric with open?
> +/* ethtool support */
> +
> +static void mse102x_get_drvinfo(struct net_device *ndev,
> + struct ethtool_drvinfo *di)
> +{
> + strscpy(di->driver, DRV_NAME, sizeof(di->driver));
> + strscpy(di->version, "1.00", sizeof(di->version));
> + strscpy(di->bus_info, dev_name(ndev->dev.parent), sizeof(di->bus_info));
> +}
Version is pretty pointless. We suggest you don't use it. The ethtool
core will then fill it with the kernel version,
> +static int mse102x_probe_spi(struct spi_device *spi)
> +{
...
> + netif_carrier_off(mse->ndev);
> + ndev->if_port = IF_PORT_10BASET;
That is not correct. Maybe you should add a IF_PORT_HOMEPLUG ?
> + ndev->netdev_ops = &mse102x_netdev_ops;
> + ndev->ethtool_ops = &mse102x_ethtool_ops;
> +
> + mse102x_init_mac(mse, dev->of_node);
> +
> + ret = register_netdev(ndev);
> + if (ret) {
> + dev_err(dev, "failed to register network device: %d\n", ret);
> + return ret;
> + }
> +
> + mse102x_init_device_debugfs(mses);
> +
> + return 0;
> +}
> +static const struct of_device_id mse102x_match_table[] = {
> + { .compatible = "vertexcom,mse1021" },
> + { .compatible = "vertexcom,mse1022" },
Is there an ID register you can read to determine what device you
actually have? If so, i suggest you verify the correct compatible is
used.
Andrew
