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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 14 May 2020 03:57:53 +0200 From: Andrew Lunn <andrew@...n.ch> To: Marek Vasut <marex@...x.de> Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>, Lukas Wunner <lukas@...ner.de>, Petr Stetiar <ynezz@...e.cz>, YueHaibing <yuehaibing@...wei.com> Subject: Re: [PATCH V5 18/19] net: ks8851: Implement Parallel bus operations > diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c > new file mode 100644 > index 000000000000..90fffacb1695 > --- /dev/null > +++ b/drivers/net/ethernet/micrel/ks8851_par.c > @@ -0,0 +1,348 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* drivers/net/ethernet/micrel/ks8851.c > + * > + * Copyright 2009 Simtec Electronics > + * http://www.simtec.co.uk/ > + * Ben Dooks <ben@...tec.co.uk> > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#define DEBUG I don't think you wanted that left in. > +#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/crc32.h> > +#include <linux/mii.h> > +#include <linux/regulator/consumer.h> > + > +#include <linux/platform_device.h> > +#include <linux/gpio.h> > +#include <linux/of_gpio.h> > +#include <linux/of_net.h> I think some of these includes can be removed. There is no regular, or gpio code in this file, etc. > + > +#include "ks8851.h" > + > +static int msg_enable; > + > +#define BE3 0x8000 /* Byte Enable 3 */ > +#define BE2 0x4000 /* Byte Enable 2 */ > +#define BE1 0x2000 /* Byte Enable 1 */ > +#define BE0 0x1000 /* Byte Enable 0 */ > + > +/** > + * struct ks8851_net_par - KS8851 Parallel driver private data > + * @ks8851: KS8851 driver common private data > + * @lock: Lock to ensure that the device is not accessed when busy. > + * @hw_addr : start address of data register. > + * @hw_addr_cmd : start address of command register. > + * @cmd_reg_cache : command register cached. > + * > + * The @lock ensures that the chip is protected when certain operations are > + * in progress. When the read or write packet transfer is in progress, most > + * of the chip registers are not ccessible until the transfer is finished and accessible > + * We do this to firstly avoid sleeping with the network device locked, > + * and secondly so we can round up more than one packet to transmit which > + * means we can try and avoid generating too many transmit done interrupts. > + */ > +static netdev_tx_t ks8851_start_xmit_par(struct sk_buff *skb, > + struct net_device *dev) > +{ > + struct ks8851_net *ks = netdev_priv(dev); > + netdev_tx_t ret = NETDEV_TX_OK; > + unsigned long flags; > + u16 txmir; > + > + netif_dbg(ks, tx_queued, ks->netdev, > + "%s: skb %p, %d@%p\n", __func__, skb, skb->len, skb->data); > + > + ks8851_lock_par(ks, &flags); > + > + txmir = ks8851_rdreg16_par(ks, KS_TXMIR) & 0x1fff; > + > + if (likely(txmir >= skb->len + 12)) { > + ks8851_wrreg16_par(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_SDA); > + ks8851_wrfifo_par(ks, skb, false); > + ks8851_wrreg16_par(ks, KS_RXQCR, ks->rc_rxqcr); > + ks8851_wrreg16_par(ks, KS_TXQCR, TXQCR_METFE); > + while (ks8851_rdreg16_par(ks, KS_TXQCR) & TXQCR_METFE) > + ; You should have a timeout/retry limit here, just in case. > + ks8851_done_tx(ks, skb); > + } else { > + ret = NETDEV_TX_BUSY; > + } > + > + ks8851_unlock_par(ks, &flags); > + > + return ret; > +} > +module_param_named(message, msg_enable, int, 0); > +MODULE_PARM_DESC(message, "Message verbosity level (0=none, 31=all)"); Module parameters are bad. A new driver should not have one, if possible. Please implement the ethtool .get_msglevel and .set_msglevel instead. Andrew
Powered by blists - more mailing lists