lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <1539361567-3602-4-git-send-email-ben.whitten@lairdtech.com>
Date:   Fri, 12 Oct 2018 17:26:04 +0100
From:   Ben Whitten <ben.whitten@...il.com>
To:     afaerber@...e.de
Cc:     starnight@...cu.edu.tw, hasnain.virk@....com,
        netdev@...r.kernel.org, liuxuenetmail@...il.com, shess@...sware.de,
        Ben Whitten <ben.whitten@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org
Subject: [PATCH v3 lora-next 3/5] net: lora: sx1301: convert to using regmap fields for bit ops

From: Ben Whitten <ben.whitten@...il.com>

We convert to using regmap fields to allow bit access to the registers
where regmap handles the read update write.

Signed-off-by: Ben Whitten <ben.whitten@...il.com>
---
 drivers/net/lora/sx1301.c | 234 +++++++++++++---------------------------------
 drivers/net/lora/sx1301.h |  46 +++++++++
 2 files changed, 110 insertions(+), 170 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 9c85fe7..339f8d9 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -24,27 +24,6 @@
 
 #include "sx1301.h"
 
-#define REG_PAGE_RESET_SOFT_RESET	BIT(7)
-
-#define REG_16_GLOBAL_EN		BIT(3)
-
-#define REG_17_CLK32M_EN		BIT(0)
-
-#define REG_0_105_FORCE_HOST_RADIO_CTRL		BIT(1)
-#define REG_0_105_FORCE_HOST_FE_CTRL		BIT(2)
-#define REG_0_105_FORCE_DEC_FILTER_GAIN		BIT(3)
-
-#define REG_0_MCU_RST_0			BIT(0)
-#define REG_0_MCU_RST_1			BIT(1)
-#define REG_0_MCU_SELECT_MUX_0		BIT(2)
-#define REG_0_MCU_SELECT_MUX_1		BIT(3)
-
-#define REG_2_43_RADIO_A_EN		BIT(0)
-#define REG_2_43_RADIO_B_EN		BIT(1)
-#define REG_2_43_RADIO_RST		BIT(2)
-
-#define REG_EMERGENCY_FORCE_HOST_CTRL	BIT(0)
-
 static const struct regmap_range_cfg sx1301_regmap_ranges[] = {
 	{
 		.name = "Pages",
@@ -74,9 +53,10 @@ static struct regmap_config sx1301_regmap_config = {
 	.max_register = SX1301_MAX_REGISTER,
 };
 
-static int sx1301_soft_reset(struct sx1301_priv *priv)
+static int sx1301_field_write(struct sx1301_priv *priv,
+		enum sx1301_fields field_id, u8 val)
 {
-	return regmap_write(priv->regmap, SX1301_PAGE, REG_PAGE_RESET_SOFT_RESET);
+	return regmap_field_write(priv->regmap_fields[field_id], val);
 }
 
 static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int *val)
@@ -120,7 +100,7 @@ static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int *
 static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct firmware *fw)
 {
 	u8 *buf;
-	u8 rst, select_mux;
+	enum sx1301_fields rst, select_mux;
 	unsigned int val;
 	int ret;
 
@@ -131,29 +111,26 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 
 	switch (mcu) {
 	case 0:
-		rst = REG_0_MCU_RST_0;
-		select_mux = REG_0_MCU_SELECT_MUX_0;
+		rst = F_MCU_RST_0;
+		select_mux = F_MCU_SELECT_MUX_0;
 		break;
 	case 1:
-		rst = REG_0_MCU_RST_1;
-		select_mux = REG_0_MCU_SELECT_MUX_1;
+		rst = F_MCU_RST_1;
+		select_mux = F_MCU_SELECT_MUX_1;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
+	ret = sx1301_field_write(priv, rst, 1);
 	if (ret) {
-		dev_err(priv->dev, "MCU read failed\n");
+		dev_err(priv->dev, "MCU reset failed\n");
 		return ret;
 	}
 
-	val |= rst;
-	val &= ~select_mux;
-
-	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
+	ret = sx1301_field_write(priv, select_mux, 0);
 	if (ret) {
-		dev_err(priv->dev, "MCU reset / select mux write failed\n");
+		dev_err(priv->dev, "MCU RAM select mux failed\n");
 		return ret;
 	}
 
@@ -194,17 +171,9 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 
 	kfree(buf);
 
-	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "MCU read (1) failed\n");
-		return ret;
-	}
-
-	val |= select_mux;
-
-	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
+	ret = sx1301_field_write(priv, select_mux, 1);
 	if (ret) {
-		dev_err(priv->dev, "MCU reset / select mux write (1) failed\n");
+		dev_err(priv->dev, "MCU RAM release mux failed\n");
 		return ret;
 	}
 
@@ -230,17 +199,9 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_FORCE_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "0|105 read failed\n");
-		return ret;
-	}
-
-	val &= ~REG_0_105_FORCE_HOST_RADIO_CTRL;
-
-	ret = regmap_write(priv->regmap, SX1301_FORCE_CTRL, val);
+	ret = sx1301_field_write(priv, F_FORCE_HOST_RADIO_CTRL, 0);
 	if (ret) {
-		dev_err(priv->dev, "0|105 write failed\n");
+		dev_err(priv->dev, "force host control failed\n");
 		return ret;
 	}
 
@@ -254,17 +215,9 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "MCU read (0) failed\n");
-		return ret;
-	}
-
-	val &= ~REG_0_MCU_RST_1;
-
-	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
+	ret = sx1301_field_write(priv, F_MCU_RST_1, 0);
 	if (ret) {
-		dev_err(priv->dev, "MCU write (0) failed\n");
+		dev_err(priv->dev, "MCU 1 reset failed\n");
 		return ret;
 	}
 
@@ -282,34 +235,18 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return -ENXIO;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "emergency force read failed\n");
-		return ret;
-	}
-
-	val &= ~REG_EMERGENCY_FORCE_HOST_CTRL;
-
-	ret = regmap_write(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, val);
+	ret = sx1301_field_write(priv, F_EMERGENCY_FORCE_HOST_CTRL, 0);
 	if (ret) {
-		dev_err(priv->dev, "emergency force write failed\n");
+		dev_err(priv->dev, "emergency force failed\n");
 		return ret;
 	}
 
 	dev_err(priv->dev, "starting calibration...\n");
 	msleep(2300);
 
-	ret = regmap_read(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "emergency force read (1) failed\n");
-		return ret;
-	}
-
-	val |= REG_EMERGENCY_FORCE_HOST_CTRL;
-
-	ret = regmap_write(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, val);
+	ret = sx1301_field_write(priv, F_EMERGENCY_FORCE_HOST_CTRL, 1);
 	if (ret) {
-		dev_err(priv->dev, "emergency force write (1) failed\n");
+		dev_err(priv->dev, "emergency force release failed\n");
 		return ret;
 	}
 
@@ -356,19 +293,15 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 	if (ret)
 		return ret;
 
-	ret = regmap_read(priv->regmap, SX1301_FORCE_CTRL, &val);
-	if (ret) {
-		dev_err(priv->dev, "0|105 read failed\n");
+	ret = sx1301_field_write(priv, F_FORCE_HOST_RADIO_CTRL, 0);
+	if (ret)
 		return ret;
-	}
-
-	val &= ~(REG_0_105_FORCE_HOST_RADIO_CTRL | REG_0_105_FORCE_HOST_FE_CTRL | REG_0_105_FORCE_DEC_FILTER_GAIN);
-
-	ret = regmap_write(priv->regmap, SX1301_FORCE_CTRL, val);
-	if (ret) {
-		dev_err(priv->dev, "0|105 write failed\n");
+	ret = sx1301_field_write(priv, F_FORCE_HOST_FE_CTRL, 0);
+	if (ret)
+		return ret;
+	ret = sx1301_field_write(priv, F_FORCE_DEC_FILTER_GAIN, 0);
+	if (ret)
 		return ret;
-	}
 
 	ret = regmap_write(priv->regmap, SX1301_CHRS, 0);
 	if (ret) {
@@ -376,17 +309,15 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
+	ret = sx1301_field_write(priv, F_MCU_RST_0, 0);
 	if (ret) {
-		dev_err(priv->dev, "MCU read (0) failed\n");
+		dev_err(priv->dev, "MCU 0 release failed\n");
 		return ret;
 	}
 
-	val &= ~(REG_0_MCU_RST_1 | REG_0_MCU_RST_0);
-
-	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
+	ret = sx1301_field_write(priv, F_MCU_RST_1, 0);
 	if (ret) {
-		dev_err(priv->dev, "MCU write (0) failed\n");
+		dev_err(priv->dev, "MCU 1 release failed\n");
 		return ret;
 	}
 
@@ -438,7 +369,6 @@ static netdev_tx_t sx130x_loradev_start_xmit(struct sk_buff *skb, struct net_dev
 static int sx130x_loradev_open(struct net_device *netdev)
 {
 	struct sx1301_priv *priv = netdev_priv(netdev);
-	unsigned int val;
 	int ret;
 
 	netdev_dbg(netdev, "%s", __func__);
@@ -448,31 +378,15 @@ static int sx130x_loradev_open(struct net_device *netdev)
 		return -ENXIO;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_GEN, &val);
-	if (ret) {
-		netdev_err(netdev, "16 read (1) failed\n");
-		return ret;
-	}
-
-	val |= REG_16_GLOBAL_EN;
-
-	ret = regmap_write(priv->regmap, SX1301_GEN, val);
-	if (ret) {
-		netdev_err(netdev, "16 write (1) failed\n");
-		return ret;
-	}
-
-	ret = regmap_read(priv->regmap, SX1301_CKEN, &val);
+	ret = sx1301_field_write(priv, F_GLOBAL_EN, 1);
 	if (ret) {
-		netdev_err(netdev, "17 read (1) failed\n");
+		dev_err(priv->dev, "enable global clocks failed\n");
 		return ret;
 	}
 
-	val |= REG_17_CLK32M_EN;
-
-	ret = regmap_write(priv->regmap, SX1301_CKEN, val);
+	ret = sx1301_field_write(priv, F_CLK32M_EN, 1);
 	if (ret) {
-		netdev_err(netdev, "17 write (1) failed\n");
+		dev_err(priv->dev, "enable 32M clock failed\n");
 		return ret;
 	}
 
@@ -519,6 +433,7 @@ static int sx1301_probe(struct spi_device *spi)
 	struct sx1301_priv *priv;
 	struct gpio_desc *rst;
 	int ret;
+	int i;
 	unsigned int ver;
 	unsigned int val;
 
@@ -557,6 +472,19 @@ static int sx1301_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(sx1301_regmap_fields); i++) {
+		const struct reg_field *reg_fields = sx1301_regmap_fields;
+
+		priv->regmap_fields[i] = devm_regmap_field_alloc(&spi->dev,
+				priv->regmap,
+				reg_fields[i]);
+		if (IS_ERR(priv->regmap_fields[i])) {
+			ret = PTR_ERR(priv->regmap_fields[i]);
+			dev_err(&spi->dev, "Cannot allocate regmap field: %d\n", ret);
+			return ret;
+		}
+	}
+
 	ret = regmap_read(priv->regmap, SX1301_VER, &ver);
 	if (ret) {
 		dev_err(&spi->dev, "version read failed\n");
@@ -574,83 +502,49 @@ static int sx1301_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = sx1301_soft_reset(priv);
+	ret = sx1301_field_write(priv, F_SOFT_RESET, 1);
 	if (ret) {
 		dev_err(&spi->dev, "soft reset failed\n");
 		return ret;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_GEN, &val);
+	ret = sx1301_field_write(priv, F_GLOBAL_EN, 0);
 	if (ret) {
-		dev_err(&spi->dev, "16 read failed\n");
+		dev_err(&spi->dev, "gate global clocks failed\n");
 		return ret;
 	}
 
-	val &= ~REG_16_GLOBAL_EN;
-
-	ret = regmap_write(priv->regmap, SX1301_GEN, val);
+	ret = sx1301_field_write(priv, F_CLK32M_EN, 0);
 	if (ret) {
-		dev_err(&spi->dev, "16 write failed\n");
+		dev_err(&spi->dev, "gate 32M clock failed\n");
 		return ret;
 	}
 
-	ret = regmap_read(priv->regmap, SX1301_CKEN, &val);
+	ret = sx1301_field_write(priv, F_RADIO_A_EN, 1);
 	if (ret) {
-		dev_err(&spi->dev, "17 read failed\n");
+		dev_err(&spi->dev, "radio a enable failed\n");
 		return ret;
 	}
 
-	val &= ~REG_17_CLK32M_EN;
-
-	ret = regmap_write(priv->regmap, SX1301_CKEN, val);
+	ret = sx1301_field_write(priv, F_RADIO_B_EN, 1);
 	if (ret) {
-		dev_err(&spi->dev, "17 write failed\n");
-		return ret;
-	}
-
-	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
-	if (ret) {
-		dev_err(&spi->dev, "2|43 read failed\n");
-		return ret;
-	}
-
-	val |= REG_2_43_RADIO_B_EN | REG_2_43_RADIO_A_EN;
-
-	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
-	if (ret) {
-		dev_err(&spi->dev, "2|43 write failed\n");
+		dev_err(&spi->dev, "radio b enable failed\n");
 		return ret;
 	}
 
 	msleep(500);
 
-	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
+	ret = sx1301_field_write(priv, F_RADIO_RST, 1);
 	if (ret) {
-		dev_err(&spi->dev, "2|43 read failed\n");
-		return ret;
-	}
-
-	val |= REG_2_43_RADIO_RST;
-
-	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
-	if (ret) {
-		dev_err(&spi->dev, "2|43 write failed\n");
+		dev_err(&spi->dev, "radio asert reset failed\n");
 		return ret;
 	}
 
 	msleep(5);
 
-	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
-	if (ret) {
-		dev_err(&spi->dev, "2|43 read failed\n");
-		return ret;
-	}
-
-	val &= ~REG_2_43_RADIO_RST;
-
-	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
+	ret = sx1301_field_write(priv, F_RADIO_RST, 0);
 	if (ret) {
-		dev_err(&spi->dev, "2|43 write failed\n");
+		dev_err(&spi->dev, "radio deasert reset failed\n");
 		return ret;
 	}
 
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index e6400f8..0bbd948 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -60,11 +60,57 @@
 
 #define SX1301_MAX_REGISTER         (SX1301_PAGE_BASE(3) + 0x7F)
 
+enum sx1301_fields {
+	F_SOFT_RESET,
+	F_GLOBAL_EN,
+	F_CLK32M_EN,
+	F_RADIO_A_EN,
+	F_RADIO_B_EN,
+	F_RADIO_RST,
+
+	F_MCU_RST_0,
+	F_MCU_RST_1,
+	F_MCU_SELECT_MUX_0,
+	F_MCU_SELECT_MUX_1,
+
+	F_FORCE_HOST_RADIO_CTRL,
+	F_FORCE_HOST_FE_CTRL,
+	F_FORCE_DEC_FILTER_GAIN,
+
+	F_EMERGENCY_FORCE_HOST_CTRL,
+};
+
+static const struct reg_field sx1301_regmap_fields[] = {
+	/* PAGE */
+	[F_SOFT_RESET]          = REG_FIELD(SX1301_PAGE, 7, 7),
+	/* GEN */
+	[F_GLOBAL_EN]           = REG_FIELD(SX1301_GEN,  3, 3),
+	/* CKEN */
+	[F_CLK32M_EN]           = REG_FIELD(SX1301_CKEN, 0, 0),
+	/* RADIO_CFG */
+	[F_RADIO_A_EN]          = REG_FIELD(SX1301_RADIO_CFG, 0, 0),
+	[F_RADIO_B_EN]          = REG_FIELD(SX1301_RADIO_CFG, 1, 1),
+	[F_RADIO_RST]           = REG_FIELD(SX1301_RADIO_CFG, 2, 2),
+	/* MCU_CTRL */
+	[F_MCU_RST_0]           = REG_FIELD(SX1301_MCU_CTRL, 0, 0),
+	[F_MCU_RST_1]           = REG_FIELD(SX1301_MCU_CTRL, 1, 1),
+	[F_MCU_SELECT_MUX_0]    = REG_FIELD(SX1301_MCU_CTRL, 2, 2),
+	[F_MCU_SELECT_MUX_1]    = REG_FIELD(SX1301_MCU_CTRL, 3, 3),
+	/* FORCE_CTRL */
+	[F_FORCE_HOST_RADIO_CTRL] = REG_FIELD(SX1301_FORCE_CTRL, 1, 1),
+	[F_FORCE_HOST_FE_CTRL]    = REG_FIELD(SX1301_FORCE_CTRL, 2, 2),
+	[F_FORCE_DEC_FILTER_GAIN] = REG_FIELD(SX1301_FORCE_CTRL, 3, 3),
+	/* EMERGENCY_FORCE_HOST_CTRL */
+	[F_EMERGENCY_FORCE_HOST_CTRL] =
+		REG_FIELD(SX1301_EMERGENCY_FORCE_HOST_CTRL, 0, 0),
+};
+
 struct sx1301_priv {
 	struct lora_dev_priv lora;
 	struct device		*dev;
 	struct gpio_desc *rst_gpio;
 	struct regmap		*regmap;
+	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx1301_regmap_fields)];
 };
 
 int __init sx130x_radio_init(void);
-- 
2.7.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ