Re: [PATCH RFC] Add Microchip KSZ8895 DSA driver

2017-10-22 Thread Pavel Machek 
Hi!

> diff --git a/drivers/net/dsa/microchip/Kconfig 
> b/drivers/net/dsa/microchip/Kconfig
> index cb95d3d..b854c4b 100644
> --- a/drivers/net/dsa/microchip/Kconfig
> +++ b/drivers/net/dsa/microchip/Kconfig
> @@ -27,3 +27,20 @@ config MICROCHIP_KSZ8795_SPI_DRIVER
>  
> It is required to use the KSZ8795 switch driver as the only access
> is through SPI.
> +
> +menuconfig MICROCHIP_KSZ8895
> + tristate "Microchip KSZ8895 series switch support"
> + depends on NET_DSA
> + select NET_DSA_TAG_KSZ8795

Aha. So this depends not only on "preparation" series, but also on
8795 driver. Surprise!

If you had a git tree with series applied, that would be nice.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH RFC] Add Microchip KSZ8895 DSA driver

2017-10-11 Thread Pavel Machek 
Hi!

> +static void ksz8895_set_prio_queue(struct ksz_device *dev, int port, int 
> queue)
> +{
> + u8 hi;
> + u8 lo;
> +
> + /* Number of queues can only be 1, 2, or 4. */
> + switch (queue) {
> + case 4:
> + case 3:
> + queue = PORT_QUEUE_SPLIT_4;
> + break;
> + case 2:
> + queue = PORT_QUEUE_SPLIT_2;
> + break;
> + default:
> + queue = PORT_QUEUE_SPLIT_1;
> + }
> + ksz_pread8(dev, port, REG_PORT_CTRL_0, );
> + ksz_pread8(dev, port, P_DROP_TAG_CTRL, );
> + lo &= ~PORT_QUEUE_SPLIT_L;
> + if (queue & PORT_QUEUE_SPLIT_2)
> + lo |= PORT_QUEUE_SPLIT_L;
> + hi &= ~PORT_QUEUE_SPLIT_H;
> + if (queue & PORT_QUEUE_SPLIT_4)
> + hi |= PORT_QUEUE_SPLIT_H;
> + ksz_pwrite8(dev, port, REG_PORT_CTRL_0, lo);
> + ksz_pwrite8(dev, port, P_DROP_TAG_CTRL, hi);
> +
> + /* Default is port based for egress rate limit. */
> + if (queue != PORT_QUEUE_SPLIT_1)
> + ksz_cfg(dev, REG_SW_CTRL_19, SW_OUT_RATE_LIMIT_QUEUE_BASED,
> + true);
> +}

This is same as the other driver, right? Same comments apply here, and
please find a way to make it shared.


> +static void ksz8895_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
> +   u64 *cnt)
> +{
> + u32 data;
> + u16 ctrl_addr;
> + u8 check;
> + int loop;
> +
> + ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> + ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> +
> + mutex_lock(>alu_mutex);
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> +
> + /* It is almost guaranteed to always read the valid bit because of
> +  * slow SPI speed.
> +  */
> + for (loop = 2; loop > 0; loop--) {
> + ksz_read8(dev, REG_IND_MIB_CHECK, );
> +
> + if (check & MIB_COUNTER_VALID) {
> + ksz_read32(dev, REG_IND_DATA_LO, );
> + if (check & MIB_COUNTER_OVERFLOW)
> + *cnt += MIB_COUNTER_VALUE + 1;
> + *cnt += data & MIB_COUNTER_VALUE;
> + break;
> + }
> + }
> + mutex_unlock(>alu_mutex);

Again, same function, same review comments.

> +static void ksz8895_r_table(struct ksz_device *dev, int table, u16 addr,
> + u64 *data)
> +{
> + u16 ctrl_addr;
> +
> + ctrl_addr = IND_ACC_TABLE(table | TABLE_READ) | addr;
> +
> + mutex_lock(>alu_mutex);
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> + ksz_get(dev, REG_IND_DATA_HI, data, sizeof(u64));
> + mutex_unlock(>alu_mutex);
> + *data = be64_to_cpu(*data);
> +}

I've seen this before; this is duplicated code, it does not make sense
to review.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH RFC] Add Microchip KSZ8895 DSA driver

2017-10-06 Thread Tristram.Ha 
From: Tristram Ha 

Add Microchip KSZ8895 DSA driver.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Kconfig   |   17 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8895.c | 1288 +++
 drivers/net/dsa/microchip/ksz8895_reg.h |  824 
 drivers/net/dsa/microchip/ksz8895_spi.c |  157 
 drivers/net/dsa/microchip/ksz_priv.h|1 +
 6 files changed, 2289 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8895.c
 create mode 100644 drivers/net/dsa/microchip/ksz8895_reg.h
 create mode 100644 drivers/net/dsa/microchip/ksz8895_spi.c

diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index cb95d3d..b854c4b 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -27,3 +27,20 @@ config MICROCHIP_KSZ8795_SPI_DRIVER
 
  It is required to use the KSZ8795 switch driver as the only access
  is through SPI.
+
+menuconfig MICROCHIP_KSZ8895
+   tristate "Microchip KSZ8895 series switch support"
+   depends on NET_DSA
+   select NET_DSA_TAG_KSZ8795
+   help
+ This driver adds support for Microchip KSZ8895 switch chips.
+
+config MICROCHIP_KSZ8895_SPI_DRIVER
+   tristate "KSZ8895 series SPI connected switch driver"
+   depends on MICROCHIP_KSZ8895 && SPI
+   default y
+   help
+ This driver accesses KSZ8895 chip through SPI.
+
+ It is required to use the KSZ8895 switch driver as the only access
+ is through SPI.
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index 99a283e..8dd6312 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -2,3 +2,5 @@ obj-$(CONFIG_MICROCHIP_KSZ9477) += ksz9477.o 
ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ9477_SPI_DRIVER) += ksz9477_spi.o
 obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
+obj-$(CONFIG_MICROCHIP_KSZ8895)+= ksz8895.o ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ8895_SPI_DRIVER) += ksz8895_spi.o
diff --git a/drivers/net/dsa/microchip/ksz8895.c 
b/drivers/net/dsa/microchip/ksz8895.c
new file mode 100644
index 000..947638b
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8895.c
@@ -0,0 +1,1288 @@
+/*
+ * Microchip KSZ8895 switch driver
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz_common.h"
+#include "ksz8895_reg.h"
+
+static const struct {
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { "rx" },
+   { "rx_hi" },
+   { "rx_undersize" },
+   { "rx_fragments" },
+   { "rx_oversize" },
+   { "rx_jabbers" },
+   { "rx_symbol_err" },
+   { "rx_crc_err" },
+   { "rx_align_err" },
+   { "rx_mac_ctrl" },
+   { "rx_pause" },
+   { "rx_bcast" },
+   { "rx_mcast" },
+   { "rx_ucast" },
+   { "rx_64_or_less" },
+   { "rx_65_127" },
+   { "rx_128_255" },
+   { "rx_256_511" },
+   { "rx_512_1023" },
+   { "rx_1024_1522" },
+   { "tx" },
+   { "tx_hi" },
+   { "tx_late_col" },
+   { "tx_pause" },
+   { "tx_bcast" },
+   { "tx_mcast" },
+   { "tx_ucast" },
+   { "tx_deferred" },
+   { "tx_total_col" },
+   { "tx_exc_col" },
+   { "tx_single_col" },
+   { "tx_mult_col" },
+   { "rx_discards" },
+   { "tx_discards" },
+};
+
+static int ksz8895_reset_switch(struct ksz_device *dev)
+{
+   /* reset switch */
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
+  SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
+
+   return 0;
+}
+
+static void ksz8895_set_prio_queue(struct ksz_device *dev, int port, int queue)
+{
+   u8 hi;
+   u8 lo;
+
+   /* Number of queues can only be 1, 2, or 4. */
+   switch (queue) {
+   case 4:
+   case 3:
+   queue =