| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YbIKW9R5iyK7sQqg@unreal>
Date: Thu, 9 Dec 2021 15:53:31 +0200
From: Leon Romanovsky <leon@...nel.org>
To: JosephCHANG <josright123@...il.com>
Cc: "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Rob Herring <robh+dt@...nel.org>, joseph_chang@...icom.com.tw,
netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2, 2/2] net: Add DM9051 driver
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.
>
> Signed-off-by: JosephCHANG <josright123@...il.com>
> [Submit v1 has Reported-by: kernel test robot <lkp@...el.com>]
> ---
> 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.h
>
> diff --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_DAVICOM
> diff --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.o
> diff --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 <joseph_chang@...icom.com.tw>
> + * 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.
<...>
> +++ 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
Powered by blists - more mailing lists