diff mbox series

[V5,18/19] net: ks8851: Implement Parallel bus operations

Message ID 20200514000747.159320-19-marex@denx.de
State Changes Requested
Delegated to: David Miller
Headers show
Series net: ks8851: Unify KS8851 SPI and MLL drivers | expand

Commit Message

Marek Vasut May 14, 2020, 12:07 a.m. UTC
Implement accessors for KS8851-16MLL/MLLI/MLLU parallel bus variant of
the KS8851. This is based off the ks8851_mll.c , which is a driver for
exactly the same hardware, however the ks8851.c code is much higher
quality. Hence, this patch pulls out the relevant information from the
ks8851_mll.c on how to access the bus, but uses the common ks8851.c
code. To make this patch reviewable, instead of rewriting ks8851_mll.c,
ks8851_mll.c is removed in a separate subsequent patch.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Petr Stetiar <ynezz@true.cz>
Cc: YueHaibing <yuehaibing@huawei.com>
---
V2: - Drop 'must be first' comment on the embedded struct ks8851_net
    - Add missing Kconfig 'select's to match the KS8851 SPI driver
    - Get rid of the pointer indirection
    - Repair the endian handling and add endian check
V3: - Adjust Makefile to permit build as a module
    - Enable only RX and LCx interrupts
V4: No change
V5: - Move msg_enable module param to ks8851_par.c
---
 drivers/net/ethernet/micrel/Kconfig      |   2 +
 drivers/net/ethernet/micrel/Makefile     |   1 +
 drivers/net/ethernet/micrel/ks8851_par.c | 348 +++++++++++++++++++++++
 3 files changed, 351 insertions(+)
 create mode 100644 drivers/net/ethernet/micrel/ks8851_par.c

Comments

Andrew Lunn May 14, 2020, 1:57 a.m. UTC | #1
> 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
Marek Vasut May 14, 2020, 2:26 a.m. UTC | #2
On 5/14/20 3:57 AM, Andrew Lunn wrote:
>> 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.

This actually was in the original ks8851.c since forever, so I wonder.
Maybe a separate patch would be better ?

[...]

>> +	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.

OK

>> +		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.

This was in the original ks8851.c , so I need to retain it , no ?
Andrew Lunn May 14, 2020, 1:15 p.m. UTC | #3
On Thu, May 14, 2020 at 04:26:30AM +0200, Marek Vasut wrote:
> On 5/14/20 3:57 AM, Andrew Lunn wrote:
> >> 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.
> 
> This actually was in the original ks8851.c since forever, so I wonder.
> Maybe a separate patch would be better ?

Yes, please add another patch.

> >> +		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.
> 
> This was in the original ks8851.c , so I need to retain it , no ?

Ah. Err.

This patch looks like a new driver. It has probe, remove
module_platform_driver(), etc. So as a new driver, it should not have
module parameters.

But then your next patch removes the mll driver. Your intention is
that this driver replaces the mll driver. So for backwards
compatibility, yes you do need the module parameter.

	Andrew
Marek Vasut May 14, 2020, 2 p.m. UTC | #4
On 5/14/20 3:15 PM, Andrew Lunn wrote:
> On Thu, May 14, 2020 at 04:26:30AM +0200, Marek Vasut wrote:
>> On 5/14/20 3:57 AM, Andrew Lunn wrote:
>>>> 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.
>>
>> This actually was in the original ks8851.c since forever, so I wonder.
>> Maybe a separate patch would be better ?
> 
> Yes, please add another patch.

OK

>>>> +		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.
>>
>> This was in the original ks8851.c , so I need to retain it , no ?
> 
> Ah. Err.
> 
> This patch looks like a new driver. It has probe, remove
> module_platform_driver(), etc. So as a new driver, it should not have
> module parameters.
> 
> But then your next patch removes the mll driver. Your intention is
> that this driver replaces the mll driver. So for backwards
> compatibility, yes you do need the module parameter.

All right

btw is jiffies-based timeout OK? Like this:

diff --git a/drivers/net/ethernet/micrel/ks8851_par.c
b/drivers/net/ethernet/micrel/ks8851_par.c
index 5163d10971f4..1d9e7a33ffcf 100644
--- a/drivers/net/ethernet/micrel/ks8851_par.c
+++ b/drivers/net/ethernet/micrel/ks8851_par.c
@@ -238,6 +238,7 @@ static netdev_tx_t ks8851_start_xmit_par(struct
sk_buff *skb,
                                         struct net_device *dev)
 {
        struct ks8851_net *ks = netdev_priv(dev);
+       unsigned long txpoll_start_time;
        netdev_tx_t ret = NETDEV_TX_OK;
        unsigned long flags;
        u16 txmir;
@@ -254,8 +255,20 @@ static netdev_tx_t ks8851_start_xmit_par(struct
sk_buff *skb,
                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)
-                       ;
+
+               txpoll_start_time = jiffies;
+               while (true) {
+                       if (!(ks8851_rdreg16_par(ks, KS_TXQCR) &
TXQCR_METFE))
+                               break;
+
+                       if (time_after(jiffies, txpoll_start_time + HZ)) {
+                               netdev_warn(dev, "%s: did not complete.\n",
+                                           __func__);
+                               ret = NETDEV_TX_BUSY;
+                               break;
+                       }
+               }
+
                ks8851_done_tx(ks, skb);
        } else {
                ret = NETDEV_TX_BUSY;
Andrew Lunn May 14, 2020, 2:07 p.m. UTC | #5
> All right
> 
> btw is jiffies-based timeout OK? Like this:

If you can, make use of include/linux/iopoll.h

   Andrew
Marek Vasut May 14, 2020, 2:14 p.m. UTC | #6
On 5/14/20 4:07 PM, Andrew Lunn wrote:
>> All right
>>
>> btw is jiffies-based timeout OK? Like this:
> 
> If you can, make use of include/linux/iopoll.h

I can't, because I need those weird custom accessors, see
ks8851_wrreg16_par(), unless I'm missing something there?
Andrew Lunn May 14, 2020, 2:22 p.m. UTC | #7
On Thu, May 14, 2020 at 04:14:13PM +0200, Marek Vasut wrote:
> On 5/14/20 4:07 PM, Andrew Lunn wrote:
> >> All right
> >>
> >> btw is jiffies-based timeout OK? Like this:
> > 
> > If you can, make use of include/linux/iopoll.h
> 
> I can't, because I need those weird custom accessors, see
> ks8851_wrreg16_par(), unless I'm missing something there?

static int ks8851_rdreg16_par_txqcr(struct foo ks) {
       return ks8851_rdreg16_par(ks, KS_TXQCR)
}

int txqcr;

err = readx_poll_timeout(ks8851_rdreg16_par_txqcr, txqcr,
                         txqcr & TXQCR_METFE, 10, 10, ks)

	 Andrew
Marek Vasut May 14, 2020, 2:33 p.m. UTC | #8
On 5/14/20 4:22 PM, Andrew Lunn wrote:
> On Thu, May 14, 2020 at 04:14:13PM +0200, Marek Vasut wrote:
>> On 5/14/20 4:07 PM, Andrew Lunn wrote:
>>>> All right
>>>>
>>>> btw is jiffies-based timeout OK? Like this:
>>>
>>> If you can, make use of include/linux/iopoll.h
>>
>> I can't, because I need those weird custom accessors, see
>> ks8851_wrreg16_par(), unless I'm missing something there?
> 
> static int ks8851_rdreg16_par_txqcr(struct foo ks) {
>        return ks8851_rdreg16_par(ks, KS_TXQCR)
> }
> 
> int txqcr;
> 
> err = readx_poll_timeout(ks8851_rdreg16_par_txqcr, txqcr,
>                          txqcr & TXQCR_METFE, 10, 10, ks)
> 

OK
diff mbox series

Patch

diff --git a/drivers/net/ethernet/micrel/Kconfig b/drivers/net/ethernet/micrel/Kconfig
index b9c4d48e28e4..09f35209d43d 100644
--- a/drivers/net/ethernet/micrel/Kconfig
+++ b/drivers/net/ethernet/micrel/Kconfig
@@ -38,6 +38,8 @@  config KS8851_MLL
 	tristate "Micrel KS8851 MLL"
 	depends on HAS_IOMEM
 	select MII
+	select CRC32
+	select EEPROM_93CX6
 	---help---
 	  This platform driver is for Micrel KS8851 Address/data bus
 	  multiplexed network chip.
diff --git a/drivers/net/ethernet/micrel/Makefile b/drivers/net/ethernet/micrel/Makefile
index c7a4725c2e95..5cc00d22c708 100644
--- a/drivers/net/ethernet/micrel/Makefile
+++ b/drivers/net/ethernet/micrel/Makefile
@@ -7,4 +7,5 @@  obj-$(CONFIG_KS8842) += ks8842.o
 obj-$(CONFIG_KS8851) += ks8851.o
 ks8851-objs = ks8851_common.o ks8851_spi.o
 obj-$(CONFIG_KS8851_MLL) += ks8851_mll.o
+ks8851_mll-objs = ks8851_common.o ks8851_par.o
 obj-$(CONFIG_KSZ884X_PCI) += ksz884x.o
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
+
+#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>
+
+#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
+ * the DMA has been de-asserted.
+ */
+struct ks8851_net_par {
+	struct ks8851_net	ks8851;
+	spinlock_t		lock;
+	void __iomem		*hw_addr;
+	void __iomem		*hw_addr_cmd;
+	u16			cmd_reg_cache;
+};
+
+#define to_ks8851_par(ks) container_of((ks), struct ks8851_net_par, ks8851)
+
+/**
+ * ks8851_lock_par - register access lock
+ * @ks: The chip state
+ * @flags: Spinlock flags
+ *
+ * Claim chip register access lock
+ */
+static void ks8851_lock_par(struct ks8851_net *ks, unsigned long *flags)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+
+	spin_lock_irqsave(&ksp->lock, *flags);
+}
+
+/**
+ * ks8851_unlock_par - register access unlock
+ * @ks: The chip state
+ * @flags: Spinlock flags
+ *
+ * Release chip register access lock
+ */
+static void ks8851_unlock_par(struct ks8851_net *ks, unsigned long *flags)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+
+	spin_unlock_irqrestore(&ksp->lock, *flags);
+}
+
+/**
+ * ks_check_endian - Check whether endianness of the bus is correct
+ * @ks	  : The chip information
+ *
+ * The KS8851-16MLL EESK pin allows selecting the endianness of the 16bit
+ * bus. To maintain optimum performance, the bus endianness should be set
+ * such that it matches the endianness of the CPU.
+ */
+static int ks_check_endian(struct ks8851_net *ks)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+	u16 cider;
+
+	/*
+	 * Read CIDER register first, however read it the "wrong" way around.
+	 * If the endian strap on the KS8851-16MLL in incorrect and the chip
+	 * is operating in different endianness than the CPU, then the meaning
+	 * of BE[3:0] byte-enable bits is also swapped such that:
+	 *    BE[3,2,1,0] becomes BE[1,0,3,2]
+	 *
+	 * Luckily for us, the byte-enable bits are the top four MSbits of
+	 * the address register and the CIDER register is at offset 0xc0.
+	 * Hence, by reading address 0xc0c0, which is not impacted by endian
+	 * swapping, we assert either BE[3:2] or BE[1:0] while reading the
+	 * CIDER register.
+	 *
+	 * If the bus configuration is correct, reading 0xc0c0 asserts
+	 * BE[3:2] and this read returns 0x0000, because to read register
+	 * with bottom two LSbits of address set to 0, BE[1:0] must be
+	 * asserted.
+	 *
+	 * If the bus configuration is NOT correct, reading 0xc0c0 asserts
+	 * BE[1:0] and this read returns non-zero 0x8872 value.
+	 */
+	iowrite16(BE3 | BE2 | KS_CIDER, ksp->hw_addr_cmd);
+	cider = ioread16(ksp->hw_addr);
+	if (!cider)
+		return 0;
+
+	netdev_err(ks->netdev, "incorrect EESK endian strap setting\n");
+
+	return -EINVAL;
+}
+
+/**
+ * ks8851_wrreg16_par - write 16bit register value to chip
+ * @ks: The chip state
+ * @reg: The register address
+ * @val: The value to write
+ *
+ * Issue a write to put the value @val into the register specified in @reg.
+ */
+static void ks8851_wrreg16_par(struct ks8851_net *ks, unsigned int reg,
+			       unsigned int val)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+
+	ksp->cmd_reg_cache = (u16)reg | ((BE1 | BE0) << (reg & 0x02));
+	iowrite16(ksp->cmd_reg_cache, ksp->hw_addr_cmd);
+	iowrite16(val, ksp->hw_addr);
+}
+
+/**
+ * ks8851_rdreg16_par - read 16 bit register from chip
+ * @ks: The chip information
+ * @reg: The register address
+ *
+ * Read a 16bit register from the chip, returning the result
+ */
+static unsigned int ks8851_rdreg16_par(struct ks8851_net *ks, unsigned int reg)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+
+	ksp->cmd_reg_cache = (u16)reg | ((BE1 | BE0) << (reg & 0x02));
+	iowrite16(ksp->cmd_reg_cache, ksp->hw_addr_cmd);
+	return ioread16(ksp->hw_addr);
+}
+
+/**
+ * ks8851_rdfifo_par - read data from the receive fifo
+ * @ks: The device state.
+ * @buff: The buffer address
+ * @len: The length of the data to read
+ *
+ * Issue an RXQ FIFO read command and read the @len amount of data from
+ * the FIFO into the buffer specified by @buff.
+ */
+static void ks8851_rdfifo_par(struct ks8851_net *ks, u8 *buff, unsigned int len)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+
+	netif_dbg(ks, rx_status, ks->netdev,
+		  "%s: %d@%p\n", __func__, len, buff);
+
+	ioread16_rep(ksp->hw_addr, (u16 *)buff + 1, len / 2);
+}
+
+/**
+ * ks8851_wrfifo_par - write packet to TX FIFO
+ * @ks: The device state.
+ * @txp: The sk_buff to transmit.
+ * @irq: IRQ on completion of the packet.
+ *
+ * Send the @txp to the chip. This means creating the relevant packet header
+ * specifying the length of the packet and the other information the chip
+ * needs, such as IRQ on completion. Send the header and the packet data to
+ * the device.
+ */
+static void ks8851_wrfifo_par(struct ks8851_net *ks, struct sk_buff *txp,
+			      bool irq)
+{
+	struct ks8851_net_par *ksp = to_ks8851_par(ks);
+	unsigned int len = ALIGN(txp->len, 4);
+	unsigned int fid = 0;
+
+	netif_dbg(ks, tx_queued, ks->netdev, "%s: skb %p, %d@%p, irq %d\n",
+		  __func__, txp, txp->len, txp->data, irq);
+
+	fid = ks->fid++;
+	fid &= TXFR_TXFID_MASK;
+
+	if (irq)
+		fid |= TXFR_TXIC;	/* irq on completion */
+
+	iowrite16(fid, ksp->hw_addr);
+	iowrite16(txp->len, ksp->hw_addr);
+
+	iowrite16_rep(ksp->hw_addr, txp->data, len / 2);
+}
+
+/**
+ * ks8851_rx_skb_par - receive skbuff
+ * @skb: The skbuff
+ */
+static void ks8851_rx_skb_par(struct ks8851_net *ks, struct sk_buff *skb)
+{
+	netif_rx(skb);
+}
+
+/**
+ * ks8851_start_xmit_par - transmit packet
+ * @skb: The buffer to transmit
+ * @dev: The device used to transmit the packet.
+ *
+ * Called by the network layer to transmit the @skb. Queue the packet for
+ * the device and schedule the necessary work to transmit the packet when
+ * it is free.
+ *
+ * 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)
+			;
+		ks8851_done_tx(ks, skb);
+	} else {
+		ret = NETDEV_TX_BUSY;
+	}
+
+	ks8851_unlock_par(ks, &flags);
+
+	return ret;
+}
+
+static int ks8851_probe_par(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ks8851_net_par *ksp;
+	struct net_device *netdev;
+	struct ks8851_net *ks;
+	int ret;
+
+	netdev = devm_alloc_etherdev(dev, sizeof(struct ks8851_net_par));
+	if (!netdev)
+		return -ENOMEM;
+
+	ks = netdev_priv(netdev);
+
+	ks->lock = ks8851_lock_par;
+	ks->unlock = ks8851_unlock_par;
+	ks->rdreg16 = ks8851_rdreg16_par;
+	ks->wrreg16 = ks8851_wrreg16_par;
+	ks->rdfifo = ks8851_rdfifo_par;
+	ks->wrfifo = ks8851_wrfifo_par;
+	ks->start_xmit = ks8851_start_xmit_par;
+	ks->rx_skb = ks8851_rx_skb_par;
+
+#define STD_IRQ (IRQ_LCI |	/* Link Change */	\
+		 IRQ_RXI |	/* RX done */		\
+		 IRQ_RXPSI)	/* RX process stop */
+	ks->rc_ier = STD_IRQ;
+
+	ksp = to_ks8851_par(ks);
+	spin_lock_init(&ksp->lock);
+
+	ksp->hw_addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ksp->hw_addr))
+		return PTR_ERR(ksp->hw_addr);
+
+	ksp->hw_addr_cmd = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(ksp->hw_addr_cmd))
+		return PTR_ERR(ksp->hw_addr_cmd);
+
+	ret = ks_check_endian(ks);
+	if (ret)
+		return ret;
+
+	netdev->irq = platform_get_irq(pdev, 0);
+
+	return ks8851_probe_common(netdev, dev, msg_enable);
+}
+
+static int ks8851_remove_par(struct platform_device *pdev)
+{
+	return ks8851_remove_common(&pdev->dev);
+}
+
+static const struct of_device_id ks8851_match_table[] = {
+	{ .compatible = "micrel,ks8851-mll" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ks8851_match_table);
+
+static struct platform_driver ks8851_driver = {
+	.driver = {
+		.name = "ks8851",
+		.of_match_table = ks8851_match_table,
+		.pm = &ks8851_pm_ops,
+	},
+	.probe = ks8851_probe_par,
+	.remove = ks8851_remove_par,
+};
+module_platform_driver(ks8851_driver);
+
+MODULE_DESCRIPTION("KS8851 Network driver");
+MODULE_AUTHOR("Ben Dooks <ben@simtec.co.uk>");
+MODULE_LICENSE("GPL");
+
+module_param_named(message, msg_enable, int, 0);
+MODULE_PARM_DESC(message, "Message verbosity level (0=none, 31=all)");