Re: [PATCH v2, 2/2] net: Add DM9051 driver
From: Leon Romanovsky <leon@kernel.org>
Date: 2021-12-09 13:53:42
Also in:
lkml, netdev
On Thu, Dec 09, 2021 at 06:07:02PM +0800, JosephCHANG wrote:
Add davicom dm9051 SPI ethernet driver. The driver work with dts
for:
- spi bus number
- spi chip select
- spi clock frequency
- interrupt gpio pin
- interrupt polarity fixed as low
Test OK with Rpi 2 and Rpi 4. Max spi speed is 31200000.Please don't do this type of formatting, where you added extra indentations without reason.
quoted hunk ↗ jump to hunk
Signed-off-by: JosephCHANG <redacted> [Submit v1 has Reported-by: kernel test robot [off-list ref]] --- drivers/net/ethernet/davicom/Kconfig | 30 + drivers/net/ethernet/davicom/Makefile | 1 + drivers/net/ethernet/davicom/dm9051.c | 967 ++++++++++++++++++++++++++ drivers/net/ethernet/davicom/dm9051.h | 248 +++++++ 4 files changed, 1246 insertions(+) create mode 100644 drivers/net/ethernet/davicom/dm9051.c create mode 100644 drivers/net/ethernet/davicom/dm9051.hdiff --git a/drivers/net/ethernet/davicom/Kconfig b/drivers/net/ethernet/davicom/Kconfig index 7af86b6d4150..9c00328f6e05 100644 --- a/drivers/net/ethernet/davicom/Kconfig +++ b/drivers/net/ethernet/davicom/Kconfig@@ -3,6 +3,20 @@ # Davicom device configuration # +config NET_VENDOR_DAVICOM + bool "Davicom devices" + depends on ARM || MIPS || COLDFIRE || NIOS2 || COMPILE_TEST || SPI + default y + help + If you have a network (Ethernet) card belonging to this class, say Y. + + Note that the answer to this question doesn't directly affect the + kernel: saying N will just cause the configurator to skip all + the questions about Davicom devices. If you say Y, you will be asked + for your specific card in the following selections. + +if NET_VENDOR_DAVICOM + config DM9000 tristate "DM9000 support" depends on ARM || MIPS || COLDFIRE || NIOS2 || COMPILE_TEST@@ -22,3 +36,19 @@ config DM9000_FORCE_SIMPLE_PHY_POLL bit to determine if the link is up or down instead of the more costly MII PHY reads. Note, this will not work if the chip is operating with an external PHY. + +config DM9051 + tristate "DM9051 SPI support" + depends on SPI + select CRC32 + select MII + help + Support for DM9051 SPI chipset. + + To compile this driver as a module, choose M here. The module + will be called dm9051. + + The SPI mode for the host's SPI master to access DM9051 is mode + 0 on the SPI bus. + +endif # NET_VENDOR_DAVICOMdiff --git a/drivers/net/ethernet/davicom/Makefile b/drivers/net/ethernet/davicom/Makefile index 173c87d21076..225f85bc1f53 100644 --- a/drivers/net/ethernet/davicom/Makefile +++ b/drivers/net/ethernet/davicom/Makefile@@ -4,3 +4,4 @@ # obj-$(CONFIG_DM9000) += dm9000.o +obj-$(CONFIG_DM9051) += dm9051.odiff --git a/drivers/net/ethernet/davicom/dm9051.c b/drivers/net/ethernet/davicom/dm9051.c new file mode 100644 index 000000000000..cdcf9d37ed7f --- /dev/null +++ b/drivers/net/ethernet/davicom/dm9051.c@@ -0,0 +1,967 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Ethernet driver for the Davicom DM9051 chip. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details.
Please don't add license text. The SPDX line is more than enough.
+ * + * Copyright 2021 Davicom Semiconductor,Inc. + * http://www.davicom.com.tw/ + * Joseph CHANG [off-list ref] + * 20211110b, Total 933 lines + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/netdevice.h> +#include <linux/etherdevice.h> +#include <linux/interrupt.h> +#include <linux/skbuff.h> +#include <linux/spinlock.h> +#include <linux/cache.h> +#include <linux/crc32.h> +#include <linux/mii.h> +#include <linux/ethtool.h> +#include <linux/delay.h> +#include <linux/irq.h> +#include <linux/slab.h> +#include <linux/gpio.h> +#include <linux/iopoll.h> +#include <linux/of_gpio.h> +#include <linux/spi/spi.h> + +#include "dm9051.h" + +#define DRV_PRODUCT_NAME "dm9051" +#define DRV_VERSION_CODE DM_VERSION(5, 0, 5) //(VER5.0.0= 0x050000) +#define DRV_VERSION_DATE "20211209" //(update)"
Two points. 1. Try to avoid vertical alignment. It adds churn when backporting. 2. Don't add DRV_VERSION_CODE and DRV_VERSION_DATE. It is useless in upstream kernel.
+
+/* spi-spi_sync, low level code */
+static int burst_xfer(struct board_info *db, u8 cmdphase, u8 *txb, u8 *rxb, unsigned int len)
+{
+ struct device *dev = &db->spidev->dev;
+ int ret = 0;
+
+ db->cmd[0] = cmdphase;
+ db->spi_xfer2[0].tx_buf = &db->cmd[0];
+ db->spi_xfer2[0].rx_buf = NULL;
+ db->spi_xfer2[0].len = 1;
+ if (!rxb) { //write"//"is C++ coding style, which is also in C99 standard. Is it ok to use such comment style in netdev? <...>
+ //rcr_reg_start(db, db->rcr_all); //rx enable later
I spotted many commented functions. They shouldn't be part of submission. <...>
+static void dm_msg_open(struct net_device *ndev)
+{
+ struct board_info *db = netdev_priv(ndev);
+ struct device *dev = &db->spidev->dev;
+
+ snprintf(db->DRV_VERSION, sizeof(db->DRV_VERSION), "%s_V%d.%d.%d_date_%s",
+ DRV_PRODUCT_NAME, (DRV_VERSION_CODE >> 16 & 0xff),
+ (DRV_VERSION_CODE >> 8 & 0xff),
+ (DRV_VERSION_CODE & 0xff),
+ DRV_VERSION_DATE);
+ dev_info(dev, "version: %s\n", db->DRV_VERSION);
+}This should go. <...>
quoted hunk ↗ jump to hunk
+++ b/drivers/net/ethernet/davicom/dm9051.h@@ -0,0 +1,248 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright 2021 Davicom Semiconductor,Inc. + * http://www.davicom.com.tw + * 2014/03/11 Joseph CHANG v1.0 Create + * 2021/10/26 Joseph CHANG v5.0.1 Update + * 2021/12/09 Joseph CHANG v5.0.5 Update
It is new file, there is no value in off-tree history.
+ * + * DM9051 register definitions + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation.
No need. <...>
+#define DM9051_PIDL 0x2A +#define DM9051_PIDH 0x2B +#define DM9051_SMCR 0x2F +#define DM9051_ATCR 0x30 +#define DM9051_SPIBCR 0x38
Please be consistent. Thanks