From: Andrew Lunn <andrew@lunn•ch>
To: Marek Vasut <marex@denx•de>
Cc: netdev@vger•kernel.org, "David S . Miller" <davem@davemloft•net>,
Lukas Wunner <lukas@wunner•de>, Petr Stetiar <ynezz@true•cz>,
YueHaibing <yuehaibing@huawei•com>
Subject: Re: [PATCH V5 18/19] net: ks8851: Implement Parallel bus operations
Date: Thu, 14 May 2020 03:57:53 +0200 [thread overview]
Message-ID: <20200514015753.GL527401@lunn.ch> (raw)
In-Reply-To: <20200514000747.159320-19-marex@denx.de>
> 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@simtec•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
next prev parent reply other threads:[~2020-05-14 1:58 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-14 0:07 [PATCH V5 00/19] net: ks8851: Unify KS8851 SPI and MLL drivers Marek Vasut
2020-05-14 0:07 ` [PATCH V5 01/19] net: ks8851: Factor out spi->dev in probe()/remove() Marek Vasut
2020-05-14 0:49 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 02/19] net: ks8851: Rename ndev to netdev in probe Marek Vasut
2020-05-14 0:50 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 03/19] net: ks8851: Replace dev_err() with netdev_err() in IRQ handler Marek Vasut
2020-05-14 0:07 ` [PATCH V5 04/19] net: ks8851: Pass device node into ks8851_init_mac() Marek Vasut
2020-05-14 0:51 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 05/19] net: ks8851: Use devm_alloc_etherdev() Marek Vasut
2020-05-14 0:07 ` [PATCH V5 06/19] net: ks8851: Use dev_{get,set}_drvdata() Marek Vasut
2020-05-14 0:54 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 07/19] net: ks8851: Remove ks8851_rdreg32() Marek Vasut
2020-05-14 0:07 ` [PATCH V5 08/19] net: ks8851: Use 16-bit writes to program MAC address Marek Vasut
2020-05-14 0:55 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 09/19] net: ks8851: Use 16-bit read of RXFC register Marek Vasut
2020-05-14 0:07 ` [PATCH V5 10/19] net: ks8851: Factor out bus lock handling Marek Vasut
2020-05-14 1:19 ` Andrew Lunn
2020-05-14 1:37 ` Marek Vasut
2020-05-14 2:07 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 11/19] net: ks8851: Factor out SKB receive function Marek Vasut
2020-05-14 0:07 ` [PATCH V5 12/19] net: ks8851: Split out SPI specific entries in struct ks8851_net Marek Vasut
2020-05-14 1:25 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 13/19] net: ks8851: Split out SPI specific code from probe() and remove() Marek Vasut
2020-05-14 1:31 ` Andrew Lunn
2020-05-14 1:34 ` Marek Vasut
2020-05-14 0:07 ` [PATCH V5 14/19] net: ks8851: Factor out TX work flush function Marek Vasut
2020-05-14 1:33 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 15/19] net: ks8851: Permit overridding interrupt enable register Marek Vasut
2020-05-14 1:35 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 16/19] net: ks8851: Implement register, FIFO, lock accessor callbacks Marek Vasut
2020-05-14 1:38 ` Andrew Lunn
2020-05-14 0:07 ` [PATCH V5 17/19] net: ks8851: Separate SPI operations into separate file Marek Vasut
2020-05-14 0:07 ` [PATCH V5 18/19] net: ks8851: Implement Parallel bus operations Marek Vasut
2020-05-14 1:57 ` Andrew Lunn [this message]
2020-05-14 2:26 ` Marek Vasut
2020-05-14 13:15 ` Andrew Lunn
2020-05-14 14:00 ` Marek Vasut
2020-05-14 14:07 ` Andrew Lunn
2020-05-14 14:14 ` Marek Vasut
2020-05-14 14:22 ` Andrew Lunn
2020-05-14 14:33 ` Marek Vasut
2020-05-14 0:07 ` [PATCH V5 19/19] net: ks8851: Remove ks8851_mll.c Marek Vasut
2020-05-14 1:48 ` [PATCH V5 00/19] net: ks8851: Unify KS8851 SPI and MLL drivers Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200514015753.GL527401@lunn.ch \
--to=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=lukas@wunner$(echo .)de \
--cc=marex@denx$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ynezz@true$(echo .)cz \
--cc=yuehaibing@huawei$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox