[PATCH v1 1/1] spi_ks8995: use regmap to access chip registers.

[Sven Van Asbroeck]

The register map layouts used in this driver are well suited to
being accessed through a regmap. This makes the driver simpler
and shorter, by eliminating some spi boilerplate code.

Testing:
- tested on a ksz8785.
- not tested on the other supported chips (ks8995, ksz8864)
  because I don't have access to them.
  However, I instrumented the spi layer to verify that the
  correct spi transactions are generated to read the ID
  registers on those chips.

Signed-off-by: Sven Van Asbroeck <svendev@xxxxxxxx>
---
 drivers/net/phy/spi_ks8995.c | 163 +++++++++++++------------------------------
 1 file changed, 50 insertions(+), 113 deletions(-)

diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index 1e2d4f1..34223ff 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -21,6 +21,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/regmap.h>
 
 #include <linux/spi/spi.h>
 
@@ -106,11 +107,27 @@ enum ks8995_chip_variant {
 
 struct ks8995_chip_params {
 	char *name;
+	const struct regmap_config *regmap_cfg;
 	int family_id;
 	int chip_id;
 	int regs_size;
-	int addr_width;
-	int addr_shift;
+};
+
+static const struct regmap_config ksz8795_regmap_cfg = {
+	.reg_bits = 15,
+	.pad_bits = 1,
+	.val_bits = 8,
+	.write_flag_mask = KS8995_CMD_WRITE << 5,
+	.read_flag_mask = KS8995_CMD_READ << 5,
+	/* max_register filled in at runtime */
+};
+
+static const struct regmap_config ks8995_regmap_cfg = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.write_flag_mask = KS8995_CMD_WRITE,
+	.read_flag_mask = KS8995_CMD_READ,
+	/* max_register filled in at runtime */
 };
 
 static const struct ks8995_chip_params ks8995_chip[] = {
@@ -119,24 +136,21 @@ struct ks8995_chip_params {
 		.family_id = FAMILY_KS8995,
 		.chip_id = KS8995_CHIP_ID,
 		.regs_size = KS8995_REGS_SIZE,
-		.addr_width = 8,
-		.addr_shift = 0,
+		.regmap_cfg = &ks8995_regmap_cfg,
 	},
 	[ksz8864] = {
 		.name = "KSZ8864RMN",
 		.family_id = FAMILY_KS8995,
 		.chip_id = KSZ8864_CHIP_ID,
 		.regs_size = KSZ8864_REGS_SIZE,
-		.addr_width = 8,
-		.addr_shift = 0,
+		.regmap_cfg = &ks8995_regmap_cfg,
 	},
 	[ksz8795] = {
 		.name = "KSZ8795CLX",
 		.family_id = FAMILY_KSZ8795,
 		.chip_id = KSZ8795_CHIP_ID,
 		.regs_size = KSZ8795_REGS_SIZE,
-		.addr_width = 12,
-		.addr_shift = 1,
+		.regmap_cfg = &ksz8795_regmap_cfg,
 	},
 };
 
@@ -152,6 +166,7 @@ struct ks8995_switch {
 	struct bin_attribute	regs_attr;
 	const struct ks8995_chip_params	*chip;
 	int			revision_id;
+	struct regmap *regmap;
 };
 
 static const struct spi_device_id ks8995_id[] = {
@@ -162,118 +177,24 @@ struct ks8995_switch {
 };
 MODULE_DEVICE_TABLE(spi, ks8995_id);
 
-static inline u8 get_chip_id(u8 val)
+static inline u8 get_chip_id(u32 val)
 {
 	return (val >> ID1_CHIPID_S) & ID1_CHIPID_M;
 }
 
-static inline u8 get_chip_rev(u8 val)
+static inline u8 get_chip_rev(u32 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)
-{
-	__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].len = sizeof(cmd);
-	spi_message_add_tail(&t[0], &m);
-
-	t[1].rx_buf = buf;
-	t[1].len = count;
-	spi_message_add_tail(&t[1], &m);
-
-	mutex_lock(&ks->lock);
-	err = spi_sync(ks->spi, &m);
-	mutex_unlock(&ks->lock);
-
-	return err ? err : count;
-}
-
-static int ks8995_write(struct ks8995_switch *ks, char *buf,
-		 unsigned offset, size_t count)
-{
-	__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].len = sizeof(cmd);
-	spi_message_add_tail(&t[0], &m);
-
-	t[1].tx_buf = buf;
-	t[1].len = count;
-	spi_message_add_tail(&t[1], &m);
-
-	mutex_lock(&ks->lock);
-	err = spi_sync(ks->spi, &m);
-	mutex_unlock(&ks->lock);
-
-	return err ? err : count;
-}
-
-static inline int ks8995_read_reg(struct ks8995_switch *ks, u8 addr, u8 *buf)
-{
-	return ks8995_read(ks, buf, addr, 1) != 1;
-}
-
-static inline int ks8995_write_reg(struct ks8995_switch *ks, u8 addr, u8 val)
-{
-	char buf = val;
-
-	return ks8995_write(ks, &buf, addr, 1) != 1;
-}
-
-/* ------------------------------------------------------------------------ */
-
 static int ks8995_stop(struct ks8995_switch *ks)
 {
-	return ks8995_write_reg(ks, KS8995_REG_ID1, 0);
+	return regmap_write(ks->regmap, KS8995_REG_ID1, 0);
 }
 
 static int ks8995_start(struct ks8995_switch *ks)
 {
-	return ks8995_write_reg(ks, KS8995_REG_ID1, 1);
+	return regmap_write(ks->regmap, KS8995_REG_ID1, 1);
 }
 
 static int ks8995_reset(struct ks8995_switch *ks)
@@ -294,11 +215,13 @@ static ssize_t ks8995_registers_read(struct file *filp, struct kobject *kobj,
 {
 	struct device *dev;
 	struct ks8995_switch *ks8995;
+	int err;
 
 	dev = container_of(kobj, struct device, kobj);
 	ks8995 = dev_get_drvdata(dev);
 
-	return ks8995_read(ks8995, buf, off, count);
+	err = regmap_bulk_read(ks8995->regmap, off, buf, count);
+	return err ? : count;
 }
 
 static ssize_t ks8995_registers_write(struct file *filp, struct kobject *kobj,
@@ -306,11 +229,13 @@ static ssize_t ks8995_registers_write(struct file *filp, struct kobject *kobj,
 {
 	struct device *dev;
 	struct ks8995_switch *ks8995;
+	int err;
 
 	dev = container_of(kobj, struct device, kobj);
 	ks8995 = dev_get_drvdata(dev);
 
-	return ks8995_write(ks8995, buf, off, count);
+	err = regmap_bulk_write(ks8995->regmap, off, buf, count);
+	return err ? : count;
 }
 
 /* ks8995_get_revision - get chip revision
@@ -321,10 +246,10 @@ static ssize_t ks8995_registers_write(struct file *filp, struct kobject *kobj,
 static int ks8995_get_revision(struct ks8995_switch *ks)
 {
 	int err;
-	u8 id0, id1, ksz8864_id;
+	u32 id0, id1, ksz8864_id;
 
 	/* read family id */
-	err = ks8995_read_reg(ks, KS8995_REG_ID0, &id0);
+	err = regmap_read(ks->regmap, KS8995_REG_ID0, &id0);
 	if (err) {
 		err = -EIO;
 		goto err_out;
@@ -341,7 +266,7 @@ static int ks8995_get_revision(struct ks8995_switch *ks)
 	switch (ks->chip->family_id) {
 	case FAMILY_KS8995:
 		/* try reading chip id at CHIP ID1 */
-		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
+		err = regmap_read(ks->regmap, KS8995_REG_ID1, &id1);
 		if (err) {
 			err = -EIO;
 			goto err_out;
@@ -354,7 +279,8 @@ static int ks8995_get_revision(struct ks8995_switch *ks)
 			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);
+			err = regmap_read(ks->regmap, KS8995_REG_ID1,
+					  &ksz8864_id);
 			if (err) {
 				err = -EIO;
 				goto err_out;
@@ -373,7 +299,7 @@ static int ks8995_get_revision(struct ks8995_switch *ks)
 		break;
 	case FAMILY_KSZ8795:
 		/* try reading chip id at CHIP ID1 */
-		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
+		err = regmap_read(ks->regmap, KS8995_REG_ID1, &id1);
 		if (err) {
 			err = -EIO;
 			goto err_out;
@@ -429,6 +355,7 @@ static int ks8995_probe(struct spi_device *spi)
 {
 	struct ks8995_switch *ks;
 	int err;
+	struct regmap_config regmap_cfg;
 	int variant = spi_get_device_id(spi)->driver_data;
 
 	if (variant >= max_variant) {
@@ -487,6 +414,16 @@ static int ks8995_probe(struct spi_device *spi)
 		return err;
 	}
 
+	memcpy(&regmap_cfg, ks->chip->regmap_cfg, sizeof(regmap_cfg));
+	regmap_cfg.max_register = ks->chip->regs_size - 1;
+	ks->regmap = devm_regmap_init_spi(spi, &regmap_cfg);
+	if (IS_ERR(ks->regmap)) {
+		err = PTR_ERR(ks->regmap);
+		dev_err(&spi->dev, "Failed to allocate register map: %d\n",
+			err);
+		return err;
+	}
+
 	err = ks8995_get_revision(ks);
 	if (err)
 		return err;
-- 
1.9.1