[PATCH 0/7] Add support for MICREL KSZ8795CLX 5-port switch

[PATCH 0/7] Add support for MICREL KSZ8795CLX 5-port switch

[Helmut Buchsbaum]

This patch series refactors the spi-ks8995 driver to finally add support
for the MICREL KSZ8795CLX. Additionally support for setting initial
register values as well as controlling a GPIO line for resetting the
switch is added.

Helmut Buchsbaum (7):
  net: phy: spi_ks8995: introduce spi_device_id table
  net: phy: spi_ks8995: add chip verification and determine chip
    revision
  net: phy: spi_ks8995: add ability to configure register initialization
  net: phy: spi_ks8995: add support for resetting switch using GPIO
  net: phy: spi_ks8995: generalize creation of SPI commands
  net: phy: spi_ks8995: add support for MICREL KSZ8795CLX 5-Port managed
    switch
  dt-bindings: net: ks8995: add documentation for MICREL ks8995 driver

 .../devicetree/bindings/net/micrel-ks8995.txt      |  27 ++
 drivers/net/phy/spi_ks8995.c                       | 407 +++++++++++++++++----
 2 files changed, 371 insertions(+), 63 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ks8995.txt

-- 
2.1.4

[PATCH 1/7] net: phy: spi_ks8995: introduce spi_device_id table

[Helmut Buchsbaum]

Refactor to use spi_device_id table to facilitate easy
extendability.

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index f091d69..e848ad9 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -89,6 +89,28 @@
 
 #define KS8995_RESET_DELAY	10 /* usec */
 
+enum ks8995_chip_variant {
+	ks8995,
+	ksz8864,
+	max_variant
+};
+
+struct ks8995_chip_params {
+	char *name;
+	int regs_size;
+};
+
+static const struct ks8995_chip_params ks8995_chip[] = {
+	[ks8995] = {
+		.name = "KS8995MA",
+		.regs_size = KS8995_REGS_SIZE,
+	},
+	[ksz8864] = {
+		.name = "KSZ8864RMN",
+		.regs_size = KSZ8864_REGS_SIZE,
+	},
+};
+
 struct ks8995_pdata {
 	/* not yet implemented */
 };
@@ -98,8 +120,16 @@ struct ks8995_switch {
 	struct mutex		lock;
 	struct ks8995_pdata	*pdata;
 	struct bin_attribute	regs_attr;
+	const struct ks8995_chip_params	*chip;
 };
 
+static const struct spi_device_id ks8995_id[] = {
+	{"ks8995", ks8995},
+	{"ksz8864", ksz8864},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ks8995_id);
+
 static inline u8 get_chip_id(u8 val)
 {
 	return (val >> ID1_CHIPID_S) & ID1_CHIPID_M;
@@ -244,17 +274,22 @@ static const struct bin_attribute ks8995_registers_attr = {
 };
 
 /* ------------------------------------------------------------------------ */
-
 static int ks8995_probe(struct spi_device *spi)
 {
 	struct ks8995_switch    *ks;
 	struct ks8995_pdata     *pdata;
 	u8      ids[2];
 	int     err;
+	int variant = spi_get_device_id(spi)->driver_data;
 
 	/* Chip description */
 	pdata = spi->dev.platform_data;
 
+	if (variant >= max_variant) {
+		dev_err(&spi->dev, "bad chip variant %d\n", variant);
+		return -ENODEV;
+	}
+
 	ks = devm_kzalloc(&spi->dev, sizeof(*ks), GFP_KERNEL);
 	if (!ks)
 		return -ENOMEM;
@@ -262,6 +297,8 @@ static int ks8995_probe(struct spi_device *spi)
 	mutex_init(&ks->lock);
 	ks->pdata = pdata;
 	ks->spi = spi_dev_get(spi);
+	ks->chip = &ks8995_chip[variant];
+
 	spi_set_drvdata(spi, ks);
 
 	spi->mode = SPI_MODE_0;
@@ -287,6 +324,7 @@ static int ks8995_probe(struct spi_device *spi)
 		return -ENODEV;
 	}
 
+	ks->regs_attr.size = ks->chip->regs_size;
 	memcpy(&ks->regs_attr, &ks8995_registers_attr, sizeof(ks->regs_attr));
 	if (get_chip_id(ids[1]) != CHIPID_M) {
 		u8 val;
@@ -303,7 +341,6 @@ static int ks8995_probe(struct spi_device *spi)
 			dev_err(&spi->dev, "unknown chip:%02x,0\n", ids[1]);
 			return err;
 		}
-		ks->regs_attr.size = KSZ8864_REGS_SIZE;
 	}
 
 	err = ks8995_reset(ks);
@@ -347,6 +384,7 @@ static struct spi_driver ks8995_driver = {
 	},
 	.probe	  = ks8995_probe,
 	.remove	  = ks8995_remove,
+	.id_table = ks8995_id,
 };
 
 module_spi_driver(ks8995_driver);
-- 
2.1.4

[PATCH 2/7] net: phy: spi_ks8995: verify chip and determine revision

[Helmut Buchsbaum]

Since the chip variant is now determined by spi_device_id, verify
family and chip id and determine the revision id.

Conflicts:
	drivers/net/phy/spi_ks8995.c

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 118 +++++++++++++++++++++++++++++--------------
 1 file changed, 80 insertions(+), 38 deletions(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index e848ad9..2803c8e 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -83,6 +83,8 @@
 
 #define FAMILY_KS8995		0x95
 #define CHIPID_M		0
+#define KS8995_CHIP_ID		0x00
+#define KSZ8864_CHIP_ID		0x01
 
 #define KS8995_CMD_WRITE	0x02U
 #define KS8995_CMD_READ		0x03U
@@ -97,16 +99,22 @@ enum ks8995_chip_variant {
 
 struct ks8995_chip_params {
 	char *name;
+	int family_id;
+	int chip_id;
 	int regs_size;
 };
 
 static const struct ks8995_chip_params ks8995_chip[] = {
 	[ks8995] = {
 		.name = "KS8995MA",
+		.family_id = FAMILY_KS8995,
+		.chip_id = KS8995_CHIP_ID,
 		.regs_size = KS8995_REGS_SIZE,
 	},
 	[ksz8864] = {
 		.name = "KSZ8864RMN",
+		.family_id = FAMILY_KS8995,
+		.chip_id = KSZ8864_CHIP_ID,
 		.regs_size = KSZ8864_REGS_SIZE,
 	},
 };
@@ -121,6 +129,7 @@ struct ks8995_switch {
 	struct ks8995_pdata	*pdata;
 	struct bin_attribute	regs_attr;
 	const struct ks8995_chip_params	*chip;
+	int			revision_id;
 };
 
 static const struct spi_device_id ks8995_id[] = {
@@ -263,6 +272,73 @@ static ssize_t ks8995_registers_write(struct file *filp, struct kobject *kobj,
 	return ks8995_write(ks8995, buf, off, count);
 }
 
+/* ks8995_get_revision - get chip revision
+ * @ks: pointer to switch instance
+ *
+ * Verify chip family and id and get chip revision.
+ */
+static int ks8995_get_revision(struct ks8995_switch *ks)
+{
+	int err;
+	u8 id0, id1, ksz8864_id;
+
+	/* read family id */
+	err = ks8995_read_reg(ks, KS8995_REG_ID0, &id0);
+	if (err) {
+		err = -EIO;
+		goto err_out;
+	}
+
+	/* verify family id */
+	if (id0 != ks->chip->family_id) {
+		dev_err(&ks->spi->dev, "chip family id mismatch: expected 0x%02x but 0x%02x read\n",
+			ks->chip->family_id, id0);
+		err = -ENODEV;
+		goto err_out;
+	}
+
+	switch (ks->chip->family_id) {
+	case FAMILY_KS8995:
+		/* try reading chip id at CHIP ID1 */
+		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
+		if (err) {
+			err = -EIO;
+			goto err_out;
+		}
+
+		/* verify chip id */
+		if ((get_chip_id(id1) == CHIPID_M) &&
+		    (get_chip_id(id1) == ks->chip->chip_id)) {
+			/* KS8995MA */
+			ks->revision_id = get_chip_rev(id1);
+		} else if (get_chip_id(id1) != CHIPID_M) {
+			/* KSZ8864RMN */
+			err = ks8995_read_reg(ks, KS8995_REG_ID1, &ksz8864_id);
+			if (err) {
+				err = -EIO;
+				goto err_out;
+			}
+
+			if ((ksz8864_id & 0x80) &&
+			    (ks->chip->chip_id == KSZ8864_CHIP_ID)) {
+				ks->revision_id = get_chip_rev(id1);
+			}
+
+		} else {
+			dev_err(&ks->spi->dev, "unsupported chip id for KS8995 family: 0x%02x\n",
+				id1);
+			err = -ENODEV;
+		}
+		break;
+	default:
+		dev_err(&ks->spi->dev, "unsupported family id: 0x%02x\n", id0);
+		err = -ENODEV;
+		break;
+	}
+err_out:
+	return err;
+}
+
 static const struct bin_attribute ks8995_registers_attr = {
 	.attr = {
 		.name   = "registers",
@@ -278,7 +354,6 @@ static int ks8995_probe(struct spi_device *spi)
 {
 	struct ks8995_switch    *ks;
 	struct ks8995_pdata     *pdata;
-	u8      ids[2];
 	int     err;
 	int variant = spi_get_device_id(spi)->driver_data;
 
@@ -309,39 +384,12 @@ static int ks8995_probe(struct spi_device *spi)
 		return err;
 	}
 
-	err = ks8995_read(ks, ids, KS8995_REG_ID0, sizeof(ids));
-	if (err < 0) {
-		dev_err(&spi->dev, "unable to read id registers, err=%d\n",
-				err);
+	err = ks8995_get_revision(ks);
+	if (err)
 		return err;
-	}
-
-	switch (ids[0]) {
-	case FAMILY_KS8995:
-		break;
-	default:
-		dev_err(&spi->dev, "unknown family id:%02x\n", ids[0]);
-		return -ENODEV;
-	}
 
 	ks->regs_attr.size = ks->chip->regs_size;
 	memcpy(&ks->regs_attr, &ks8995_registers_attr, sizeof(ks->regs_attr));
-	if (get_chip_id(ids[1]) != CHIPID_M) {
-		u8 val;
-
-		/* Check if this is a KSZ8864RMN */
-		err = ks8995_read(ks, &val, KSZ8864_REG_ID1, sizeof(val));
-		if (err < 0) {
-			dev_err(&spi->dev,
-				"unable to read chip id register, err=%d\n",
-				err);
-			return err;
-		}
-		if ((val & 0x80) == 0) {
-			dev_err(&spi->dev, "unknown chip:%02x,0\n", ids[1]);
-			return err;
-		}
-	}
 
 	err = ks8995_reset(ks);
 	if (err)
@@ -354,14 +402,8 @@ static int ks8995_probe(struct spi_device *spi)
 		return err;
 	}
 
-	if (get_chip_id(ids[1]) == CHIPID_M) {
-		dev_info(&spi->dev,
-			 "KS8995 device found, Chip ID:%x, Revision:%x\n",
-			 get_chip_id(ids[1]), get_chip_rev(ids[1]));
-	} else {
-		dev_info(&spi->dev, "KSZ8864 device found, Revision:%x\n",
-			 get_chip_rev(ids[1]));
-	}
+	dev_info(&spi->dev, "%s device found, Chip ID:%x, Revision:%x\n",
+		 ks->chip->name, ks->chip->chip_id, ks->revision_id);
 
 	return 0;
 }
-- 
2.1.4

[PATCH 3/7] net: phy: spi_ks8995: add register initialization

[Helmut Buchsbaum]

Since several use cases need to setup at least some basic control
registers add the ability to configure an array containing such
register initialization values within the platform data of the switch.
Furthermore expose this capabilty to the devicetree.

Platform data now contains a pointer to an array and the array length
where each member contains the register to be initialized, the
initialization value and a register mask, since in many use cases there
is only the need to init some bits of a register, e.g. disabling unused
ports.

The devicetree notation add the property 'settings' to the SPI node of the
ks8985 driver, which is a list of triple values (register, value, mask),
e.g.:

settings = <0x4D 0x08 0x08
            0x5D 0x08 0x08>;

to power down port 3 and 4 of a KSZ8864RMN.

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 136 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 128 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index 2803c8e..d50f091 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/of.h>
 
 #include <linux/spi/spi.h>
 
@@ -120,7 +121,12 @@ static const struct ks8995_chip_params ks8995_chip[] = {
 };
 
 struct ks8995_pdata {
-	/* not yet implemented */
+	struct reg_init {
+		int reg;
+		int val;
+		int mask;
+	} *settings;
+	int nsettings;
 };
 
 struct ks8995_switch {
@@ -235,6 +241,47 @@ static int ks8995_start(struct ks8995_switch *ks)
 	return ks8995_write_reg(ks, KS8995_REG_ID1, 1);
 }
 
+/* ks8995_register_init - initialize registers
+ * @ks: pointer to switch instance
+ *
+ * Initialize registers from platform data if available.
+ */
+static int ks8995_register_init(struct ks8995_switch *ks)
+{
+	int i;
+	int err = 0;
+	struct ks8995_pdata *pdata = ks->pdata;
+
+	if (!pdata || !pdata->settings)
+		return 0;
+
+	for (i = 0; i < pdata->nsettings; ++i) {
+		u8 oldval, newval;
+		struct reg_init *s = &pdata->settings[i];
+
+		err = ks8995_read_reg(ks, s->reg, &oldval);
+		if (err) {
+			dev_err(&ks->spi->dev,
+				"Reading register 0x%02x failed\n",
+				s->reg);
+			err = -EIO;
+			break;
+		}
+
+		oldval &= ~s->mask;
+		newval = oldval | (s->val & s->mask);
+
+		err = ks8995_write_reg(ks, s->reg, newval);
+		if (err) {
+			dev_err(&ks->spi->dev,
+				"Writing register 0x%02x failed\n",
+				s->reg);
+			err = -EIO;
+			break;
+		}
+	}
+	return err;
+}
 static int ks8995_reset(struct ks8995_switch *ks)
 {
 	int err;
@@ -245,6 +292,10 @@ static int ks8995_reset(struct ks8995_switch *ks)
 
 	udelay(KS8995_RESET_DELAY);
 
+	err = ks8995_register_init(ks);
+	if (err)
+		return err;
+
 	return ks8995_start(ks);
 }
 
@@ -339,6 +390,64 @@ err_out:
 	return err;
 }
 
+/* ks8995_parse_dt - setup platform data from devicetree
+ * @ks: pointer to switch instance
+ *
+ * Parses supported DT properties and sets up platform data
+ * accordingly.
+ */
+static int ks8995_parse_dt(struct ks8995_switch *ks)
+{
+	const __be32 *settings;
+	int size, nsettings, i;
+	struct device_node *np = ks->spi->dev.of_node;
+	struct ks8995_pdata *pdata = ks->pdata;
+
+	if (!np)
+		return 0;
+
+	/* we have something like:
+	 * settings = <0x22 0x80 0xF0>;
+	 *               ^   ^    ^
+	 *               |   |    |
+	 *               |   |    + register bit mask
+	 *               |   + register value
+	 *               + register number
+	 *
+	 * for multiple registers it is
+	 *
+	 * settings = <0x22 0x80 0xF0 0x23 0x01 0xFF>;
+	 */
+	settings = of_get_property(np, "settings", &size);
+	if (!settings)
+		return 0;
+
+	if (size < sizeof(*settings) * 2) {
+		dev_err(&ks->spi->dev, "bad data for settings\n");
+		return -EINVAL;
+	}
+
+	size /= sizeof(*settings);	/* Number of elements in DT array */
+	nsettings = size / 3;		/* Number of register settings */
+
+	pdata->settings = devm_kzalloc(&ks->spi->dev,
+		sizeof(*pdata->settings) * nsettings, GFP_KERNEL);
+
+	if (!pdata->settings)
+		return -ENOMEM;
+
+	for (i = 0; i < nsettings; ++i) {
+		struct reg_init *s = &pdata->settings[i];
+
+		s->reg = be32_to_cpup(settings + 3 * i);
+		s->val = be32_to_cpup(settings + 3 * i + 1);
+		s->mask = be32_to_cpup(settings + 3 * i + 2);
+	}
+	pdata->nsettings = nsettings;
+
+	return 0;
+}
+
 static const struct bin_attribute ks8995_registers_attr = {
 	.attr = {
 		.name   = "registers",
@@ -352,14 +461,10 @@ static const struct bin_attribute ks8995_registers_attr = {
 /* ------------------------------------------------------------------------ */
 static int ks8995_probe(struct spi_device *spi)
 {
-	struct ks8995_switch    *ks;
-	struct ks8995_pdata     *pdata;
-	int     err;
+	struct ks8995_switch *ks;
+	int err;
 	int variant = spi_get_device_id(spi)->driver_data;
 
-	/* Chip description */
-	pdata = spi->dev.platform_data;
-
 	if (variant >= max_variant) {
 		dev_err(&spi->dev, "bad chip variant %d\n", variant);
 		return -ENODEV;
@@ -370,10 +475,25 @@ static int ks8995_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	mutex_init(&ks->lock);
-	ks->pdata = pdata;
 	ks->spi = spi_dev_get(spi);
 	ks->chip = &ks8995_chip[variant];
 
+	if (ks->spi->dev.of_node) {
+		ks->pdata = devm_kzalloc(&spi->dev, sizeof(*ks->pdata),
+					 GFP_KERNEL);
+		if (!ks->pdata)
+			return -ENOMEM;
+
+		err = ks8995_parse_dt(ks);
+		if (err) {
+			dev_err(&ks->spi->dev, "bad data DT data\n");
+			return err;
+		}
+	}
+
+	if (!ks->pdata)
+		ks->pdata = spi->dev.platform_data;
+
 	spi_set_drvdata(spi, ks);
 
 	spi->mode = SPI_MODE_0;
-- 
2.1.4

[PATCH 4/7] net: phy: spi_ks8995: add support for resetting switch using GPIO

[Helmut Buchsbaum]

When using device tree it is no more possible to reset the PHY at board
level. Furthermore, doing in the driver allows to power down the switch
when the it is not used any more.

The patch introduces a new optional property "reset-gpios" denoting an
appropriate GPIO handle, e.g.:

reset-gpios = <&gpio0 46 1>

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index d50f091..60479c4 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -19,6 +19,8 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/of.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
 
 #include <linux/spi/spi.h>
 
@@ -127,6 +129,7 @@ struct ks8995_pdata {
 		int mask;
 	} *settings;
 	int nsettings;
+	int reset_gpio;
 };
 
 struct ks8995_switch {
@@ -406,6 +409,8 @@ static int ks8995_parse_dt(struct ks8995_switch *ks)
 	if (!np)
 		return 0;
 
+	pdata->reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);
+
 	/* we have something like:
 	 * settings = <0x22 0x80 0xF0>;
 	 *               ^   ^    ^
@@ -484,6 +489,8 @@ static int ks8995_probe(struct spi_device *spi)
 		if (!ks->pdata)
 			return -ENOMEM;
 
+		ks->pdata->reset_gpio = -1;
+
 		err = ks8995_parse_dt(ks);
 		if (err) {
 			dev_err(&ks->spi->dev, "bad data DT data\n");
@@ -494,6 +501,18 @@ static int ks8995_probe(struct spi_device *spi)
 	if (!ks->pdata)
 		ks->pdata = spi->dev.platform_data;
 
+	if (ks->pdata && gpio_is_valid(ks->pdata->reset_gpio)) {
+		err = devm_gpio_request_one(&spi->dev,
+					    ks->pdata->reset_gpio,
+					    GPIOF_OUT_INIT_HIGH,
+					    "switch-reset");
+		if (err) {
+			dev_err(&spi->dev,
+				"failed to get reset-gpios: %d\n", err);
+			return -EIO;
+		}
+	}
+
 	spi_set_drvdata(spi, ks);
 
 	spi->mode = SPI_MODE_0;
@@ -534,11 +553,13 @@ static int ks8995_remove(struct spi_device *spi)
 
 	sysfs_remove_bin_file(&spi->dev.kobj, &ks->regs_attr);
 
+	if (ks->pdata && gpio_is_valid(ks->pdata->reset_gpio))
+		gpio_set_value(ks->pdata->reset_gpio, 0);
+
 	return 0;
 }
 
 /* ------------------------------------------------------------------------ */
-
 static struct spi_driver ks8995_driver = {
 	.driver = {
 		.name	    = "spi-ks8995",
-- 
2.1.4

[PATCH 5/7] net: phy: spi_ks8995: generalize creation of SPI commands

[Helmut Buchsbaum]

Prepare creating SPI reads and writes for other switch families.
The KS8995 family uses the straight forward
	<8bit CMD><8bit ADDR>
sequence.
To be able to support KSZ8795 family, which uses
	<3bit CMD><12bit ADDR><1 bit TR>
make the SPI command creation chip variant dependent.

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 46 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index 60479c4..6d6f984 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -105,6 +105,8 @@ struct ks8995_chip_params {
 	int family_id;
 	int chip_id;
 	int regs_size;
+	int addr_width;
+	int addr_shift;
 };
 
 static const struct ks8995_chip_params ks8995_chip[] = {
@@ -113,12 +115,16 @@ static const struct ks8995_chip_params ks8995_chip[] = {
 		.family_id = FAMILY_KS8995,
 		.chip_id = KS8995_CHIP_ID,
 		.regs_size = KS8995_REGS_SIZE,
+		.addr_width = 8,
+		.addr_shift = 0,
 	},
 	[ksz8864] = {
 		.name = "KSZ8864RMN",
 		.family_id = FAMILY_KS8995,
 		.chip_id = KSZ8864_CHIP_ID,
 		.regs_size = KSZ8864_REGS_SIZE,
+		.addr_width = 8,
+		.addr_shift = 0,
 	},
 };
 
@@ -158,20 +164,44 @@ static inline u8 get_chip_rev(u8 val)
 	return (val >> ID1_REVISION_S) & ID1_REVISION_M;
 }
 
+/* create_spi_cmd - create a chip specific SPI command header
+ * @ks: pointer to switch instance
+ * @cmd: SPI command for switch
+ * @address: register address for command
+ *
+ * Different chip families use different bit pattern to address the switches
+ * registers:
+ *
+ * KS8995: 8bit command + 8bit address
+ * KSZ8795: 3bit command + 12bit address + 1bit TR (?)
+ */
+static inline __be16 create_spi_cmd(struct ks8995_switch *ks, int cmd,
+				    unsigned address)
+{
+	u16 result = cmd;
+
+	/* make room for address (incl. address shift) */
+	result <<= ks->chip->addr_width + ks->chip->addr_shift;
+	/* add address */
+	result |= address << ks->chip->addr_shift;
+	/* SPI protocol needs big endian */
+	return cpu_to_be16(result);
+}
 /* ------------------------------------------------------------------------ */
 static int ks8995_read(struct ks8995_switch *ks, char *buf,
 		 unsigned offset, size_t count)
 {
-	u8 cmd[2];
+	__be16 cmd;
 	struct spi_transfer t[2];
 	struct spi_message m;
 	int err;
 
+	cmd = create_spi_cmd(ks, KS8995_CMD_READ, offset);
 	spi_message_init(&m);
 
 	memset(&t, 0, sizeof(t));
 
-	t[0].tx_buf = cmd;
+	t[0].tx_buf = &cmd;
 	t[0].len = sizeof(cmd);
 	spi_message_add_tail(&t[0], &m);
 
@@ -179,9 +209,6 @@ static int ks8995_read(struct ks8995_switch *ks, char *buf,
 	t[1].len = count;
 	spi_message_add_tail(&t[1], &m);
 
-	cmd[0] = KS8995_CMD_READ;
-	cmd[1] = offset;
-
 	mutex_lock(&ks->lock);
 	err = spi_sync(ks->spi, &m);
 	mutex_unlock(&ks->lock);
@@ -189,20 +216,20 @@ static int ks8995_read(struct ks8995_switch *ks, char *buf,
 	return err ? err : count;
 }
 
-
 static int ks8995_write(struct ks8995_switch *ks, char *buf,
 		 unsigned offset, size_t count)
 {
-	u8 cmd[2];
+	__be16 cmd;
 	struct spi_transfer t[2];
 	struct spi_message m;
 	int err;
 
+	cmd = create_spi_cmd(ks, KS8995_CMD_WRITE, offset);
 	spi_message_init(&m);
 
 	memset(&t, 0, sizeof(t));
 
-	t[0].tx_buf = cmd;
+	t[0].tx_buf = &cmd;
 	t[0].len = sizeof(cmd);
 	spi_message_add_tail(&t[0], &m);
 
@@ -210,9 +237,6 @@ static int ks8995_write(struct ks8995_switch *ks, char *buf,
 	t[1].len = count;
 	spi_message_add_tail(&t[1], &m);
 
-	cmd[0] = KS8995_CMD_WRITE;
-	cmd[1] = offset;
-
 	mutex_lock(&ks->lock);
 	err = spi_sync(ks->spi, &m);
 	mutex_unlock(&ks->lock);
-- 
2.1.4

[PATCH 6/7] net: phy: spi_ks8995: add support for MICREL KSZ8795CLX

[Helmut Buchsbaum]

Add support for MICREL KSZ8795CLX Integrated 5-Port, 10-/100-Managed
Ethernet Switch with Gigabit GMII/RGMII and MII/RMII interfaces.

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 drivers/net/phy/spi_ks8995.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index 6d6f984..5a3f75d 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -77,6 +77,7 @@
 
 #define KS8995_REGS_SIZE	0x80
 #define KSZ8864_REGS_SIZE	0x100
+#define KSZ8795_REGS_SIZE	0x100
 
 #define ID1_CHIPID_M		0xf
 #define ID1_CHIPID_S		4
@@ -85,9 +86,11 @@
 #define ID1_START_SW		1	/* start the switch */
 
 #define FAMILY_KS8995		0x95
+#define FAMILY_KSZ8795		0x87
 #define CHIPID_M		0
 #define KS8995_CHIP_ID		0x00
 #define KSZ8864_CHIP_ID		0x01
+#define KSZ8795_CHIP_ID		0x09
 
 #define KS8995_CMD_WRITE	0x02U
 #define KS8995_CMD_READ		0x03U
@@ -97,6 +100,7 @@
 enum ks8995_chip_variant {
 	ks8995,
 	ksz8864,
+	ksz8795,
 	max_variant
 };
 
@@ -126,6 +130,14 @@ static const struct ks8995_chip_params ks8995_chip[] = {
 		.addr_width = 8,
 		.addr_shift = 0,
 	},
+	[ksz8795] = {
+		.name = "KSZ8795CLX",
+		.family_id = FAMILY_KSZ8795,
+		.chip_id = KSZ8795_CHIP_ID,
+		.regs_size = KSZ8795_REGS_SIZE,
+		.addr_width = 12,
+		.addr_shift = 1,
+	},
 };
 
 struct ks8995_pdata {
@@ -150,6 +162,7 @@ struct ks8995_switch {
 static const struct spi_device_id ks8995_id[] = {
 	{"ks8995", ks8995},
 	{"ksz8864", ksz8864},
+	{"ksz8795", ksz8795},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ks8995_id);
@@ -408,6 +421,22 @@ static int ks8995_get_revision(struct ks8995_switch *ks)
 			err = -ENODEV;
 		}
 		break;
+	case FAMILY_KSZ8795:
+		/* try reading chip id at CHIP ID1 */
+		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
+		if (err) {
+			err = -EIO;
+			goto err_out;
+		}
+
+		if (get_chip_id(id1) == ks->chip->chip_id) {
+			ks->revision_id = get_chip_rev(id1);
+		} else {
+			dev_err(&ks->spi->dev, "unsupported chip id for KSZ8795 family: 0x%02x\n",
+				id1);
+			err = -ENODEV;
+		}
+		break;
 	default:
 		dev_err(&ks->spi->dev, "unsupported family id: 0x%02x\n", id0);
 		err = -ENODEV;
-- 
2.1.4

[PATCH 7/7] dt-bindings: net: ks8995: add bindings documentation for ks8995

[Helmut Buchsbaum]

Signed-off-by: Helmut Buchsbaum <helmut.buchsbaum@gmail.com>
---
 .../devicetree/bindings/net/micrel-ks8995.txt      | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/micrel-ks8995.txt

diff --git a/Documentation/devicetree/bindings/net/micrel-ks8995.txt b/Documentation/devicetree/bindings/net/micrel-ks8995.txt
new file mode 100644
index 0000000..d178153
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/micrel-ks8995.txt
@@ -0,0 +1,27 @@
+Micrel KS8995 SPI controlled Ethernet Switch families
+
+Required properties (according to spi-bus.txt):
+- compatible: either "micrel,ks8995", "micrel,ksz8864" or "micrel,ksz8795"
+
+Optional properties:
+- settings: list of register initialization triples containing
+            register offset, register value and register mask.
+- reset-gpios : phandle of gpio that will be used to reset chip during probe
+
+Example:
+
+spi-master {
+	...
+	ksz8795 {
+		compatible = "micrel,ksz8795";
+
+		reg = <0>;
+		spi-max-frequency = <50000000>;
+		settings = <
+			0x56 0x10 0x10 /* Enable Ingress RGMII-ID Mode */
+			0x3D 0x08 0x08 /* power down port 3 */
+			0x4D 0x08 0x08 /* power down port 4 */
+		>;
+		reset-gpios = <&gpio0 46 1>;
+	};
+};
-- 
2.1.4

Re: [PATCH 3/7] net: phy: spi_ks8995: add register initialization

[Florian Fainelli]

On 07/02/2016 14:39, Helmut Buchsbaum wrote:
> Since several use cases need to setup at least some basic control
> registers add the ability to configure an array containing such
> register initialization values within the platform data of the switch.
> Furthermore expose this capabilty to the devicetree.
> 
> Platform data now contains a pointer to an array and the array length
> where each member contains the register to be initialized, the
> initialization value and a register mask, since in many use cases there
> is only the need to init some bits of a register, e.g. disabling unused
> ports.
> 
> The devicetree notation add the property 'settings' to the SPI node of the
> ks8985 driver, which is a list of triple values (register, value, mask),
> e.g.:
> 
> settings = <0x4D 0x08 0x08
>             0x5D 0x08 0x08>;

You encode way too much in the Device Tree that should be knowledge to
the driver on how to configure the switch. This is very tempting,
because you do not dictate any use case and let people define it based
on their Device Tree source, but at the same time, this is very error
prone and does not provide what a proper device driver needs to be doing
by defining a standard and predictable behavior.

Right now this driver is a PHY driver, but it should be moved to a DSA
driver eventually such that each port is exposed as a network interface,
and you have hooks to power on/off ports based on whether a
corresponding network interface is up/down.

Re: [PATCH 3/7] net: phy: spi_ks8995: add register initialization

[Helmut Buchsbaum]

On 02/08/2016 05:38 AM, Florian Fainelli wrote:
> On 07/02/2016 14:39, Helmut Buchsbaum wrote:
>> Since several use cases need to setup at least some basic control
>> registers add the ability to configure an array containing such
>> register initialization values within the platform data of the switch.
>> Furthermore expose this capabilty to the devicetree.
>>
>> Platform data now contains a pointer to an array and the array length
>> where each member contains the register to be initialized, the
>> initialization value and a register mask, since in many use cases there
>> is only the need to init some bits of a register, e.g. disabling unused
>> ports.
>>
>> The devicetree notation add the property 'settings' to the SPI node of the
>> ks8985 driver, which is a list of triple values (register, value, mask),
>> e.g.:
>>
>> settings = <0x4D 0x08 0x08
>>              0x5D 0x08 0x08>;
>
> You encode way too much in the Device Tree that should be knowledge to
> the driver on how to configure the switch. This is very tempting,
> because you do not dictate any use case and let people define it based
> on their Device Tree source, but at the same time, this is very error
> prone and does not provide what a proper device driver needs to be doing
> by defining a standard and predictable behavior.
>
> Right now this driver is a PHY driver, but it should be moved to a DSA
> driver eventually such that each port is exposed as a network interface,
> and you have hooks to power on/off ports based on whether a
> corresponding network interface is up/down.
> --
> Florian
>
The way I built these initialization settings was inspired by the way it 
is done in the pinctrl subsystem: there you also configure the pin 
functions in a very hardware specific way (dependent on the underlying 
pinctrl hardware). Thus this was just extending a principle we can find 
in other subsystems of the kernel to this driver. Furthermore the 
register interface is already exposed to the user space via sysfs, 
which, in my opinion, is even more error prone then setting up the 
Device Tree carefully.

Nevertheless, I can perfectly understand your point of view. This is 
just what thought when I saw all registers are accessible from user space!

At the moment I use this driver with a KSZ8795CLX, port 5 directly 
connected to a MACB/GEM of a Zynq SOC, with the need to enable the RGMII 
internal clock delay (register 0x56,  bit 4), otherwise the the Zynq 
cannot talk to the switch on its RGMII interface (being able to switch 
off unused ports is just a nice add-on I use). Using the sysfs 
capabilities of this driver might be an alternative, but contradicts our 
requirement to set up the network interfaces as fast as possible. 
Furthermore stuff like IP_PNP or nfs root won't work. But maybe I should 
try to move this kind of basic setup to bootloader - I'll investigate 
this idea!

Since I'm not at all (yet) familiar with the DSA subsystem I wonder how 
I could manage setting the clock delay bit with DSA. Would this be a 
driver specific setting or can it be fulfilled within the subsystem?

Since I still want to share my work for the PHY only driver, is it ok if 
I'll resend the patch series just without part 3 to get support for the 
KSZ8795? Let's talk about the part 3 functionality and moving the driver 
to DSA separately!

BTW, are there any additional links about DSA complementing the kernel
documentation?

Thanks for your comments,
Helmut

Re: [PATCH 3/7] net: phy: spi_ks8995: add register initialization

[Andrew Lunn]

> At the moment I use this driver with a KSZ8795CLX, port 5 directly
> connected to a MACB/GEM of a Zynq SOC, with the need to enable the
> RGMII internal clock delay (register 0x56,  bit 4), otherwise the
> the Zynq cannot talk to the switch on its RGMII interface

Hi Helmut

This is possible with DSA.
Documentation/devicetree/bindings/net/dsa/dsa.txt says you can include
a phy-mode setting. phy-mode is defined in
Documentation/devicetree/bindings/net/ethernet.txt and includes
"rgmii-id", "rgmii-rxid", "rgmii-txid" which control these delays.

    Andrew

Re: [PATCH 4/7] net: phy: spi_ks8995: add support for resetting switch using GPIO

[Andrew Lunn]

On Sun, Feb 07, 2016 at 11:39:10PM +0100, Helmut Buchsbaum wrote:
> When using device tree it is no more possible to reset the PHY at board
> level. Furthermore, doing in the driver allows to power down the switch
> when the it is not used any more.
> 
> The patch introduces a new optional property "reset-gpios" denoting an
> appropriate GPIO handle, e.g.:
> 
> reset-gpios = <&gpio0 46 1>

The 1 here means active low flag.

>  
> +	pdata->reset_gpio = of_get_named_gpio(np, "reset-gpios", 0);
> +

Here you don't take any notice of the flags.

>  	/* we have something like:
>  	 * settings = <0x22 0x80 0xF0>;
>  	 *               ^   ^    ^
> @@ -484,6 +489,8 @@ static int ks8995_probe(struct spi_device *spi)
>  		if (!ks->pdata)
>  			return -ENOMEM;
>  
> +		ks->pdata->reset_gpio = -1;
> +
>  		err = ks8995_parse_dt(ks);
>  		if (err) {
>  			dev_err(&ks->spi->dev, "bad data DT data\n");
> @@ -494,6 +501,18 @@ static int ks8995_probe(struct spi_device *spi)
>  	if (!ks->pdata)
>  		ks->pdata = spi->dev.platform_data;
>  
> +	if (ks->pdata && gpio_is_valid(ks->pdata->reset_gpio)) {
> +		err = devm_gpio_request_one(&spi->dev,
> +					    ks->pdata->reset_gpio,
> +					    GPIOF_OUT_INIT_HIGH,

Hard coded HIGH. You should determine this from the flag....

DSA has the same functionality and does support the flag. You can copy
it from there.

     Andrew