linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API
       [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
@ 2018-10-12 16:26 ` Ben Whitten
  2018-10-18 16:59   ` Andreas Färber
  2018-10-12 16:26 ` [PATCH v3 lora-next 2/5] net: lora: sx1301: replace burst spi functions with regmap_noinc Ben Whitten
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Ben Whitten @ 2018-10-12 16:26 UTC (permalink / raw)
  To: afaerber
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki,
	linux-kernel

The regmap API had a noinc_read function added for instances where devices
supported returning data from an internal FIFO in a single read.

This commit adds the noinc_write variant to allow writing to a non
incrementing register, this is used in devices such as the sx1301 for
loading firmware.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/base/regmap/internal.h |  3 ++
 drivers/base/regmap/regmap.c   | 77 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h         | 19 +++++++++++
 3 files changed, 99 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index a6bf34d63..404f123 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -94,11 +94,13 @@ struct regmap {
 	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
+	bool (*writeable_noinc_reg)(struct device *dev, unsigned int reg);
 	bool (*readable_noinc_reg)(struct device *dev, unsigned int reg);
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
 	const struct regmap_access_table *volatile_table;
 	const struct regmap_access_table *precious_table;
+	const struct regmap_access_table *wr_noinc_table;
 	const struct regmap_access_table *rd_noinc_table;
 
 	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
@@ -183,6 +185,7 @@ bool regmap_writeable(struct regmap *map, unsigned int reg);
 bool regmap_readable(struct regmap *map, unsigned int reg);
 bool regmap_volatile(struct regmap *map, unsigned int reg);
 bool regmap_precious(struct regmap *map, unsigned int reg);
+bool regmap_writeable_noinc(struct regmap *map, unsigned int reg);
 bool regmap_readable_noinc(struct regmap *map, unsigned int reg);
 
 int _regmap_write(struct regmap *map, unsigned int reg,
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 0360a90..d4f1fc6 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -168,6 +168,17 @@ bool regmap_precious(struct regmap *map, unsigned int reg)
 	return false;
 }
 
+bool regmap_writeable_noinc(struct regmap *map, unsigned int reg)
+{
+	if (map->writeable_noinc_reg)
+		return map->writeable_noinc_reg(map->dev, reg);
+
+	if (map->wr_noinc_table)
+		return regmap_check_range_table(map, reg, map->wr_noinc_table);
+
+	return true;
+}
+
 bool regmap_readable_noinc(struct regmap *map, unsigned int reg)
 {
 	if (map->readable_noinc_reg)
@@ -777,11 +788,13 @@ struct regmap *__regmap_init(struct device *dev,
 	map->rd_table = config->rd_table;
 	map->volatile_table = config->volatile_table;
 	map->precious_table = config->precious_table;
+	map->wr_noinc_table = config->wr_noinc_table;
 	map->rd_noinc_table = config->rd_noinc_table;
 	map->writeable_reg = config->writeable_reg;
 	map->readable_reg = config->readable_reg;
 	map->volatile_reg = config->volatile_reg;
 	map->precious_reg = config->precious_reg;
+	map->writeable_noinc_reg = config->writeable_noinc_reg;
 	map->readable_noinc_reg = config->readable_noinc_reg;
 	map->cache_type = config->cache_type;
 
@@ -1298,6 +1311,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
 	map->readable_reg = config->readable_reg;
 	map->volatile_reg = config->volatile_reg;
 	map->precious_reg = config->precious_reg;
+	map->writeable_noinc_reg = config->writeable_noinc_reg;
 	map->readable_noinc_reg = config->readable_noinc_reg;
 	map->cache_type = config->cache_type;
 
@@ -1898,6 +1912,69 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 EXPORT_SYMBOL_GPL(regmap_raw_write);
 
 /**
+ * regmap_noinc_write(): Write data from a register without incrementing the
+ *			register number
+ *
+ * @map: Register map to write to
+ * @reg: Register to write to
+ * @val: Pointer to data buffer
+ * @val_len: Length of output buffer in bytes.
+ *
+ * The regmap API usually assumes that bulk bus write operations will write a
+ * range of registers. Some devices have certain registers for which a write
+ * operation can write to an internal FIFO.
+ *
+ * The target register must be volatile but registers after it can be
+ * completely unrelated cacheable registers.
+ *
+ * This will attempt multiple writes as required to write val_len bytes.
+ *
+ * A value of zero will be returned on success, a negative errno will be
+ * returned in error cases.
+ */
+int regmap_noinc_write(struct regmap *map, unsigned int reg,
+		      const void *val, size_t val_len)
+{
+	size_t write_len;
+	int ret;
+
+	if (!map->bus)
+		return -EINVAL;
+	if (!map->bus->write)
+		return -ENOTSUPP;
+	if (val_len % map->format.val_bytes)
+		return -EINVAL;
+	if (!IS_ALIGNED(reg, map->reg_stride))
+		return -EINVAL;
+	if (val_len == 0)
+		return -EINVAL;
+
+	map->lock(map->lock_arg);
+
+	if (!regmap_volatile(map, reg) || !regmap_writeable_noinc(map, reg)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	while (val_len) {
+		if (map->max_raw_write && map->max_raw_write < val_len)
+			write_len = map->max_raw_write;
+		else
+			write_len = val_len;
+		ret = _regmap_raw_write(map, reg, val, write_len);
+		if (ret)
+			goto out_unlock;
+		val = ((u8 *)val) + write_len;
+		val_len -= write_len;
+	}
+
+out_unlock:
+	map->unlock(map->lock_arg);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_noinc_write);
+
+/**
  * regmap_field_update_bits_base() - Perform a read/modify/write cycle a
  *                                   register field.
  *
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 379505a..de04dc4 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -268,6 +268,13 @@ typedef void (*regmap_unlock)(void *);
  *                field is NULL but precious_table (see below) is not, the
  *                check is performed on such table (a register is precious if
  *                it belongs to one of the ranges specified by precious_table).
+ * @writeable_noinc_reg: Optional callback returning true if the register
+ *			supports multiple write operations without incrementing
+ *			the register number. If this field is NULL but
+ *			wr_noinc_table (see below) is not, the check is
+ *			performed on such table (a register is no increment
+ *			writeable if it belongs to one of the ranges specified
+ *			by wr_noinc_table).
  * @readable_noinc_reg: Optional callback returning true if the register
  *			supports multiple read operations without incrementing
  *			the register number. If this field is NULL but
@@ -302,6 +309,7 @@ typedef void (*regmap_unlock)(void *);
  * @rd_table:     As above, for read access.
  * @volatile_table: As above, for volatile registers.
  * @precious_table: As above, for precious registers.
+ * @wr_noinc_table: As above, for no increment writeable registers.
  * @rd_noinc_table: As above, for no increment readable registers.
  * @reg_defaults: Power on reset values for registers (for use with
  *                register cache support).
@@ -352,6 +360,7 @@ struct regmap_config {
 	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
+	bool (*writeable_noinc_reg)(struct device *dev, unsigned int reg);
 	bool (*readable_noinc_reg)(struct device *dev, unsigned int reg);
 
 	bool disable_locking;
@@ -369,6 +378,7 @@ struct regmap_config {
 	const struct regmap_access_table *rd_table;
 	const struct regmap_access_table *volatile_table;
 	const struct regmap_access_table *precious_table;
+	const struct regmap_access_table *wr_noinc_table;
 	const struct regmap_access_table *rd_noinc_table;
 	const struct reg_default *reg_defaults;
 	unsigned int num_reg_defaults;
@@ -979,6 +989,8 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
 int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val);
 int regmap_raw_write(struct regmap *map, unsigned int reg,
 		     const void *val, size_t val_len);
+int regmap_noinc_write(struct regmap *map, unsigned int reg,
+		     const void *val, size_t val_len);
 int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			size_t val_count);
 int regmap_multi_reg_write(struct regmap *map, const struct reg_sequence *regs,
@@ -1222,6 +1234,13 @@ static inline int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 	return -EINVAL;
 }
 
+static inline int regmap_noinc_write(struct regmap *map, unsigned int reg,
+				    const void *val, size_t val_len)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_bulk_write(struct regmap *map, unsigned int reg,
 				    const void *val, size_t val_count)
 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 lora-next 2/5] net: lora: sx1301: replace burst spi functions with regmap_noinc
       [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
  2018-10-12 16:26 ` [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API Ben Whitten
@ 2018-10-12 16:26 ` Ben Whitten
  2018-10-12 16:26 ` [PATCH v3 lora-next 3/5] net: lora: sx1301: convert to using regmap fields for bit ops Ben Whitten
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Ben Whitten @ 2018-10-12 16:26 UTC (permalink / raw)
  To: afaerber
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, David S. Miller, linux-kernel

We can now use to regmap_noinc API to allow reading and writing to
the internal FIFO register which controls processor memory.
We also remove the now defunct spi element from the structure as this
completes the move to regmap.

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

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index fd29258..9c85fe7 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -74,23 +74,6 @@ static struct regmap_config sx1301_regmap_config = {
 	.max_register = SX1301_MAX_REGISTER,
 };
 
-static int sx1301_read_burst(struct sx1301_priv *priv, u8 reg, u8 *val, size_t len)
-{
-	u8 addr = reg & 0x7f;
-	return spi_write_then_read(priv->spi, &addr, 1, val, len);
-}
-
-static int sx1301_write_burst(struct sx1301_priv *priv, u8 reg, const u8 *val, size_t len)
-{
-	u8 addr = reg | BIT(7);
-	struct spi_transfer xfr[2] = {
-		{ .tx_buf = &addr, .len = 1 },
-		{ .tx_buf = val, .len = len },
-	};
-
-	return spi_sync_transfer(priv->spi, xfr, 2);
-}
-
 static int sx1301_soft_reset(struct sx1301_priv *priv)
 {
 	return regmap_write(priv->regmap, SX1301_PAGE, REG_PAGE_RESET_SOFT_RESET);
@@ -180,7 +163,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 		return ret;
 	}
 
-	ret = sx1301_write_burst(priv, SX1301_MPD, fw->data, fw->size);
+	ret = regmap_noinc_write(priv->regmap, SX1301_MPD, fw->data, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data write failed\n");
 		return ret;
@@ -196,7 +179,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 	if (!buf)
 		return -ENOMEM;
 
-	ret = sx1301_read_burst(priv, SX1301_MPD, buf, fw->size);
+	ret = regmap_noinc_read(priv->regmap, SX1301_MPD, buf, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data read failed\n");
 		kfree(buf);
@@ -566,7 +549,6 @@ static int sx1301_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, netdev);
 	priv->dev = &spi->dev;
-	priv->spi = spi;
 
 	priv->regmap = devm_regmap_init_spi(spi, &sx1301_regmap_config);
 	if (IS_ERR(priv->regmap)) {
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index e939c02..e6400f8 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -12,7 +12,6 @@
 #include <linux/regmap.h>
 #include <linux/gpio/consumer.h>
 #include <linux/lora/dev.h>
-#include <linux/spi/spi.h>
 
 #define SX1301_CHIP_VERSION 103
 
@@ -64,7 +63,6 @@
 struct sx1301_priv {
 	struct lora_dev_priv lora;
 	struct device		*dev;
-	struct spi_device	*spi;
 	struct gpio_desc *rst_gpio;
 	struct regmap		*regmap;
 };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 lora-next 3/5] net: lora: sx1301: convert to using regmap fields for bit ops
       [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
  2018-10-12 16:26 ` [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API Ben Whitten
  2018-10-12 16:26 ` [PATCH v3 lora-next 2/5] net: lora: sx1301: replace burst spi functions with regmap_noinc Ben Whitten
@ 2018-10-12 16:26 ` Ben Whitten
  2018-10-12 16:26 ` [PATCH v3 lora-next 4/5] net: lora: sx125x: convert to regmap fields Ben Whitten
  2018-10-12 16:26 ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Ben Whitten
  4 siblings, 0 replies; 18+ messages in thread
From: Ben Whitten @ 2018-10-12 16:26 UTC (permalink / raw)
  To: afaerber
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, David S. Miller, linux-kernel

From: Ben Whitten <ben.whitten@gmail.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@gmail.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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 lora-next 4/5] net: lora: sx125x: convert to regmap fields
       [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
                   ` (2 preceding siblings ...)
  2018-10-12 16:26 ` [PATCH v3 lora-next 3/5] net: lora: sx1301: convert to using regmap fields for bit ops Ben Whitten
@ 2018-10-12 16:26 ` Ben Whitten
  2018-10-12 16:26 ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Ben Whitten
  4 siblings, 0 replies; 18+ messages in thread
From: Ben Whitten @ 2018-10-12 16:26 UTC (permalink / raw)
  To: afaerber
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, David S. Miller, linux-kernel

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

We convert to using regmap fields to allow regmap to take care of read
modify writes and bit shifting for ofset fields.

Signed-off-by: Ben Whitten <ben.whitten@gmail.com>
---
 drivers/net/lora/sx125x.c | 59 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index dc13d1a..36b61b1 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -25,11 +25,25 @@
 
 #include "sx125x.h"
 
-#define REG_CLK_SELECT_TX_DAC_CLK_SELECT_CLK_IN	BIT(0)
-#define REG_CLK_SELECT_CLK_OUT			BIT(1)
+enum sx125x_fields {
+	F_CLK_OUT,
+	F_TX_DAC_CLK_SEL,
+	F_SX1257_XOSC_GM_STARTUP,
+	F_SX1257_XOSC_DISABLE_CORE,
+};
+
+static const struct reg_field sx125x_regmap_fields[] = {
+	/* CLK_SELECT */
+	[F_CLK_OUT]        = REG_FIELD(SX125X_CLK_SELECT, 1, 1),
+	[F_TX_DAC_CLK_SEL] = REG_FIELD(SX125X_CLK_SELECT, 0, 0),
+	/* XOSC */ /* TODO maybe make this dynamic */
+	[F_SX1257_XOSC_GM_STARTUP]  = REG_FIELD(SX1257_XOSC, 0, 3),
+	[F_SX1257_XOSC_DISABLE_CORE]  = REG_FIELD(SX1257_XOSC, 5, 5),
+};
 
 struct sx125x_priv {
 	struct regmap		*regmap;
+	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
 };
 
 static struct regmap_config __maybe_unused sx125x_regmap_config = {
@@ -44,11 +58,18 @@ static struct regmap_config __maybe_unused sx125x_regmap_config = {
 	.max_register = SX125X_MAX_REGISTER,
 };
 
+static int sx125x_field_write(struct sx125x_priv *priv,
+		enum sx125x_fields field_id, u8 val)
+{
+	return regmap_field_write(priv->regmap_fields[field_id], val);
+}
+
 static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap *regmap, unsigned int radio)
 {
 	struct sx125x_priv *priv;
 	unsigned int val;
 	int ret;
+	int i;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -56,6 +77,18 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
 
 	dev_set_drvdata(dev, priv);
 	priv->regmap = regmap;
+	for (i = 0; i < ARRAY_SIZE(sx125x_regmap_fields); i++) {
+		const struct reg_field *reg_fields = sx125x_regmap_fields;
+
+		priv->regmap_fields[i] = devm_regmap_field_alloc(dev,
+				priv->regmap,
+				reg_fields[i]);
+		if (IS_ERR(priv->regmap_fields[i])) {
+			ret = PTR_ERR(priv->regmap_fields[i]);
+			dev_err(dev, "Cannot allocate regmap field: %d\n", ret);
+			return ret;
+		}
+	}
 
 	if (true) {
 		ret = regmap_read(priv->regmap, SX1255_VERSION, &val);
@@ -66,24 +99,34 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
 		dev_info(dev, "SX125x version: %02x\n", val);
 	}
 
-	val = REG_CLK_SELECT_TX_DAC_CLK_SELECT_CLK_IN;
 	if (radio == 1) { /* HACK */
-		val |= REG_CLK_SELECT_CLK_OUT;
+		ret = sx125x_field_write(priv, F_CLK_OUT, 1);
+		if (ret) {
+			dev_err(dev, "enabling clock output failed\n");
+			return ret;
+		}
+
 		dev_info(dev, "enabling clock output\n");
 	}
 
-	ret = regmap_write(priv->regmap, SX125X_CLK_SELECT, val);
+	ret = sx125x_field_write(priv, F_TX_DAC_CLK_SEL, 1);
 	if (ret) {
-		dev_err(dev, "clk write failed\n");
+		dev_err(dev, "clock select failed\n");
 		return ret;
 	}
 
 	dev_dbg(dev, "clk written\n");
 
 	if (true) {
-		ret = regmap_write(priv->regmap, SX1257_XOSC, 13 + 2 * 16);
+		ret = sx125x_field_write(priv, F_SX1257_XOSC_DISABLE_CORE, 1);
+		if (ret) {
+			dev_err(dev, "xosc disable failed\n");
+			return ret;
+		}
+
+		ret = sx125x_field_write(priv, F_SX1257_XOSC_GM_STARTUP, 13);
 		if (ret) {
-			dev_err(dev, "xosc write failed\n");
+			dev_err(dev, "xosc startup adjust failed\n");
 			return ret;
 		}
 	}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
       [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
                   ` (3 preceding siblings ...)
  2018-10-12 16:26 ` [PATCH v3 lora-next 4/5] net: lora: sx125x: convert to regmap fields Ben Whitten
@ 2018-10-12 16:26 ` Ben Whitten
  2018-12-29 19:25   ` Andreas Färber
  4 siblings, 1 reply; 18+ messages in thread
From: Ben Whitten @ 2018-10-12 16:26 UTC (permalink / raw)
  To: afaerber
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, David S. Miller, linux-kernel

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

The 32M is run from the radio, before we just enabled it based on
the radio number but now we can use the clk framework to request the
clk is started when we need it.

The 32M clock produced from the radio is really a gated version of
tcxo which is a fixed clock provided by hardware, and isn't captured
in this patch.

The sx1301 brings the clock up prior to calibration once the radios
have probed themselves.

A sample dts showing the clk link:
	sx1301: sx1301@0 {
		...
                clocks = <&radio1 0>;
                clock-names = "clk32m";

                radio-spi {
                        radio0: radio-a@0 {
                                ...
                        };

                        radio1: radio-b@1 {
                                #clock-cells = <0>;
                                clock-output-names = "clk32m";
                        };
                };
	};

Signed-off-by: Ben Whitten <ben.whitten@gmail.com>
---
 drivers/net/lora/sx125x.c | 112 ++++++++++++++++++++++++++++++++++++++++++----
 drivers/net/lora/sx1301.c |  13 ++++++
 drivers/net/lora/sx1301.h |   2 +
 3 files changed, 119 insertions(+), 8 deletions(-)

diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index 36b61b1..b7ca782 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -9,6 +9,8 @@
  * Copyright (c) 2013 Semtech-Cycleo
  */
 
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -42,10 +44,16 @@ static const struct reg_field sx125x_regmap_fields[] = {
 };
 
 struct sx125x_priv {
+	struct clk		*clkout;
+	struct clk_hw		clkout_hw;
+
+	struct device		*dev;
 	struct regmap		*regmap;
 	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
 };
 
+#define to_clkout(_hw) container_of(_hw, struct sx125x_priv, clkout_hw)
+
 static struct regmap_config __maybe_unused sx125x_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -64,6 +72,96 @@ static int sx125x_field_write(struct sx125x_priv *priv,
 	return regmap_field_write(priv->regmap_fields[field_id], val);
 }
 
+static int sx125x_field_read(struct sx125x_priv *priv,
+		enum sx125x_fields field_id, unsigned int *val)
+{
+	return regmap_field_read(priv->regmap_fields[field_id], val);
+}
+
+static int sx125x_clkout_enable(struct clk_hw *hw)
+{
+	struct sx125x_priv *priv = to_clkout(hw);
+
+	dev_info(priv->dev, "enabling clkout\n");
+	return sx125x_field_write(priv, F_CLK_OUT, 1);
+}
+
+static void sx125x_clkout_disable(struct clk_hw *hw)
+{
+	struct sx125x_priv *priv = to_clkout(hw);
+	int ret;
+
+	dev_info(priv->dev, "disabling clkout\n");
+	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
+	if (ret)
+		dev_err(priv->dev, "error disabling clkout\n");
+}
+
+static int sx125x_clkout_is_enabled(struct clk_hw *hw)
+{
+	struct sx125x_priv *priv = to_clkout(hw);
+	unsigned int enabled;
+	int ret;
+
+	ret = sx125x_field_read(priv, F_CLK_OUT, &enabled);
+	if (ret) {
+		dev_err(priv->dev, "error reading clk enable\n");
+		return 0;
+	}
+	return enabled;
+}
+
+static const struct clk_ops sx125x_clkout_ops = {
+	.enable = sx125x_clkout_enable,
+	.disable = sx125x_clkout_disable,
+	.is_enabled = sx125x_clkout_is_enabled,
+};
+
+static int sx125x_register_clock_provider(struct sx125x_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct clk_init_data init;
+	const char *parent;
+	int ret;
+
+	/* Disable CLKOUT */
+	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
+	if (ret) {
+		dev_err(dev, "unable to disable clkout\n");
+		return ret;
+	}
+
+	/* Register clock provider if expected in DTB */
+	if (!of_find_property(dev->of_node, "#clock-cells", NULL))
+		return 0;
+
+	dev_info(dev, "registering clkout\n");
+
+	parent = of_clk_get_parent_name(dev->of_node, 0);
+	if (!parent) {
+		dev_err(dev, "Unable to find parent clk\n");
+		return -ENODEV;
+	}
+
+	init.ops = &sx125x_clkout_ops;
+	init.flags = CLK_IS_BASIC;
+	init.parent_names = &parent;
+	init.num_parents = 1;
+	priv->clkout_hw.init = &init;
+
+	of_property_read_string_index(dev->of_node, "clock-output-names", 0,
+			&init.name);
+
+	priv->clkout = devm_clk_register(dev, &priv->clkout_hw);
+	if (IS_ERR(priv->clkout)) {
+		dev_err(dev, "failed to register clkout\n");
+		return PTR_ERR(priv->clkout);
+	}
+	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
+			&priv->clkout_hw);
+	return ret;
+}
+
 static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap *regmap, unsigned int radio)
 {
 	struct sx125x_priv *priv;
@@ -76,6 +174,7 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
 		return -ENOMEM;
 
 	dev_set_drvdata(dev, priv);
+	priv->dev = dev;
 	priv->regmap = regmap;
 	for (i = 0; i < ARRAY_SIZE(sx125x_regmap_fields); i++) {
 		const struct reg_field *reg_fields = sx125x_regmap_fields;
@@ -99,16 +198,13 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
 		dev_info(dev, "SX125x version: %02x\n", val);
 	}
 
-	if (radio == 1) { /* HACK */
-		ret = sx125x_field_write(priv, F_CLK_OUT, 1);
-		if (ret) {
-			dev_err(dev, "enabling clock output failed\n");
-			return ret;
-		}
-
-		dev_info(dev, "enabling clock output\n");
+	ret = sx125x_register_clock_provider(priv);
+	if (ret) {
+		dev_err(dev, "failed to register clkout provider: %d\n", ret);
+		return ret;
 	}
 
+	/* TODO Only needs setting on radio on the TX path */
 	ret = sx125x_field_write(priv, F_TX_DAC_CLK_SEL, 1);
 	if (ret) {
 		dev_err(dev, "clock select failed\n");
diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 339f8d9..23cbddc3 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/lora.h>
@@ -378,6 +379,18 @@ static int sx130x_loradev_open(struct net_device *netdev)
 		return -ENXIO;
 	}
 
+	priv->clk32m = devm_clk_get(priv->dev, "clk32m");
+	if (IS_ERR(priv->clk32m)) {
+		dev_err(priv->dev, "failed to get clk32m\n");
+		return PTR_ERR(priv->clk32m);
+	}
+
+	ret = clk_prepare_enable(priv->clk32m);
+	if (ret) {
+		dev_err(priv->dev, "failed to enable clk32m: %d\n", ret);
+		return ret;
+	}
+
 	ret = sx1301_field_write(priv, F_GLOBAL_EN, 1);
 	if (ret) {
 		dev_err(priv->dev, "enable global clocks failed\n");
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index 0bbd948..a1a2e38 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -9,6 +9,7 @@
 #ifndef _SX1301_
 #define _SX1301_
 
+#include <linux/clk.h>
 #include <linux/regmap.h>
 #include <linux/gpio/consumer.h>
 #include <linux/lora/dev.h>
@@ -108,6 +109,7 @@ static const struct reg_field sx1301_regmap_fields[] = {
 struct sx1301_priv {
 	struct lora_dev_priv lora;
 	struct device		*dev;
+	struct clk		*clk32m;
 	struct gpio_desc *rst_gpio;
 	struct regmap		*regmap;
 	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx1301_regmap_fields)];
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API
  2018-10-12 16:26 ` [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API Ben Whitten
@ 2018-10-18 16:59   ` Andreas Färber
  2018-10-18 17:18     ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2018-10-18 16:59 UTC (permalink / raw)
  To: Ben Whitten, Mark Brown
  Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
	Stefan Rehm, linux-spi

Am 12.10.18 um 18:26 schrieb Ben Whitten:
> The regmap API had a noinc_read function added for instances where devices
> supported returning data from an internal FIFO in a single read.
> 
> This commit adds the noinc_write variant to allow writing to a non
> incrementing register, this is used in devices such as the sx1301 for
> loading firmware.
> 
> Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

Mark, please take this one through your tree - I'll rebase the LoRa
parts on linux-next then.

Thanks,
Andreas

> ---
>  drivers/base/regmap/internal.h |  3 ++
>  drivers/base/regmap/regmap.c   | 77 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/regmap.h         | 19 +++++++++++
>  3 files changed, 99 insertions(+)
> 
> diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
> index a6bf34d63..404f123 100644
> --- a/drivers/base/regmap/internal.h
> +++ b/drivers/base/regmap/internal.h
> @@ -94,11 +94,13 @@ struct regmap {
>  	bool (*readable_reg)(struct device *dev, unsigned int reg);
>  	bool (*volatile_reg)(struct device *dev, unsigned int reg);
>  	bool (*precious_reg)(struct device *dev, unsigned int reg);
> +	bool (*writeable_noinc_reg)(struct device *dev, unsigned int reg);
>  	bool (*readable_noinc_reg)(struct device *dev, unsigned int reg);
>  	const struct regmap_access_table *wr_table;
>  	const struct regmap_access_table *rd_table;
>  	const struct regmap_access_table *volatile_table;
>  	const struct regmap_access_table *precious_table;
> +	const struct regmap_access_table *wr_noinc_table;
>  	const struct regmap_access_table *rd_noinc_table;
>  
>  	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
> @@ -183,6 +185,7 @@ bool regmap_writeable(struct regmap *map, unsigned int reg);
>  bool regmap_readable(struct regmap *map, unsigned int reg);
>  bool regmap_volatile(struct regmap *map, unsigned int reg);
>  bool regmap_precious(struct regmap *map, unsigned int reg);
> +bool regmap_writeable_noinc(struct regmap *map, unsigned int reg);
>  bool regmap_readable_noinc(struct regmap *map, unsigned int reg);
>  
>  int _regmap_write(struct regmap *map, unsigned int reg,
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 0360a90..d4f1fc6 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -168,6 +168,17 @@ bool regmap_precious(struct regmap *map, unsigned int reg)
>  	return false;
>  }
>  
> +bool regmap_writeable_noinc(struct regmap *map, unsigned int reg)
> +{
> +	if (map->writeable_noinc_reg)
> +		return map->writeable_noinc_reg(map->dev, reg);
> +
> +	if (map->wr_noinc_table)
> +		return regmap_check_range_table(map, reg, map->wr_noinc_table);
> +
> +	return true;
> +}
> +
>  bool regmap_readable_noinc(struct regmap *map, unsigned int reg)
>  {
>  	if (map->readable_noinc_reg)
> @@ -777,11 +788,13 @@ struct regmap *__regmap_init(struct device *dev,
>  	map->rd_table = config->rd_table;
>  	map->volatile_table = config->volatile_table;
>  	map->precious_table = config->precious_table;
> +	map->wr_noinc_table = config->wr_noinc_table;
>  	map->rd_noinc_table = config->rd_noinc_table;
>  	map->writeable_reg = config->writeable_reg;
>  	map->readable_reg = config->readable_reg;
>  	map->volatile_reg = config->volatile_reg;
>  	map->precious_reg = config->precious_reg;
> +	map->writeable_noinc_reg = config->writeable_noinc_reg;
>  	map->readable_noinc_reg = config->readable_noinc_reg;
>  	map->cache_type = config->cache_type;
>  
> @@ -1298,6 +1311,7 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config)
>  	map->readable_reg = config->readable_reg;
>  	map->volatile_reg = config->volatile_reg;
>  	map->precious_reg = config->precious_reg;
> +	map->writeable_noinc_reg = config->writeable_noinc_reg;
>  	map->readable_noinc_reg = config->readable_noinc_reg;
>  	map->cache_type = config->cache_type;
>  
> @@ -1898,6 +1912,69 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
>  EXPORT_SYMBOL_GPL(regmap_raw_write);
>  
>  /**
> + * regmap_noinc_write(): Write data from a register without incrementing the
> + *			register number
> + *
> + * @map: Register map to write to
> + * @reg: Register to write to
> + * @val: Pointer to data buffer
> + * @val_len: Length of output buffer in bytes.
> + *
> + * The regmap API usually assumes that bulk bus write operations will write a
> + * range of registers. Some devices have certain registers for which a write
> + * operation can write to an internal FIFO.
> + *
> + * The target register must be volatile but registers after it can be
> + * completely unrelated cacheable registers.
> + *
> + * This will attempt multiple writes as required to write val_len bytes.
> + *
> + * A value of zero will be returned on success, a negative errno will be
> + * returned in error cases.
> + */
> +int regmap_noinc_write(struct regmap *map, unsigned int reg,
> +		      const void *val, size_t val_len)
> +{
> +	size_t write_len;
> +	int ret;
> +
> +	if (!map->bus)
> +		return -EINVAL;
> +	if (!map->bus->write)
> +		return -ENOTSUPP;
> +	if (val_len % map->format.val_bytes)
> +		return -EINVAL;
> +	if (!IS_ALIGNED(reg, map->reg_stride))
> +		return -EINVAL;
> +	if (val_len == 0)
> +		return -EINVAL;
> +
> +	map->lock(map->lock_arg);
> +
> +	if (!regmap_volatile(map, reg) || !regmap_writeable_noinc(map, reg)) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	while (val_len) {
> +		if (map->max_raw_write && map->max_raw_write < val_len)
> +			write_len = map->max_raw_write;
> +		else
> +			write_len = val_len;
> +		ret = _regmap_raw_write(map, reg, val, write_len);
> +		if (ret)
> +			goto out_unlock;
> +		val = ((u8 *)val) + write_len;
> +		val_len -= write_len;
> +	}
> +
> +out_unlock:
> +	map->unlock(map->lock_arg);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(regmap_noinc_write);
> +
> +/**
>   * regmap_field_update_bits_base() - Perform a read/modify/write cycle a
>   *                                   register field.
>   *
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 379505a..de04dc4 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -268,6 +268,13 @@ typedef void (*regmap_unlock)(void *);
>   *                field is NULL but precious_table (see below) is not, the
>   *                check is performed on such table (a register is precious if
>   *                it belongs to one of the ranges specified by precious_table).
> + * @writeable_noinc_reg: Optional callback returning true if the register
> + *			supports multiple write operations without incrementing
> + *			the register number. If this field is NULL but
> + *			wr_noinc_table (see below) is not, the check is
> + *			performed on such table (a register is no increment
> + *			writeable if it belongs to one of the ranges specified
> + *			by wr_noinc_table).
>   * @readable_noinc_reg: Optional callback returning true if the register
>   *			supports multiple read operations without incrementing
>   *			the register number. If this field is NULL but
> @@ -302,6 +309,7 @@ typedef void (*regmap_unlock)(void *);
>   * @rd_table:     As above, for read access.
>   * @volatile_table: As above, for volatile registers.
>   * @precious_table: As above, for precious registers.
> + * @wr_noinc_table: As above, for no increment writeable registers.
>   * @rd_noinc_table: As above, for no increment readable registers.
>   * @reg_defaults: Power on reset values for registers (for use with
>   *                register cache support).
> @@ -352,6 +360,7 @@ struct regmap_config {
>  	bool (*readable_reg)(struct device *dev, unsigned int reg);
>  	bool (*volatile_reg)(struct device *dev, unsigned int reg);
>  	bool (*precious_reg)(struct device *dev, unsigned int reg);
> +	bool (*writeable_noinc_reg)(struct device *dev, unsigned int reg);
>  	bool (*readable_noinc_reg)(struct device *dev, unsigned int reg);
>  
>  	bool disable_locking;
> @@ -369,6 +378,7 @@ struct regmap_config {
>  	const struct regmap_access_table *rd_table;
>  	const struct regmap_access_table *volatile_table;
>  	const struct regmap_access_table *precious_table;
> +	const struct regmap_access_table *wr_noinc_table;
>  	const struct regmap_access_table *rd_noinc_table;
>  	const struct reg_default *reg_defaults;
>  	unsigned int num_reg_defaults;
> @@ -979,6 +989,8 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
>  int regmap_write_async(struct regmap *map, unsigned int reg, unsigned int val);
>  int regmap_raw_write(struct regmap *map, unsigned int reg,
>  		     const void *val, size_t val_len);
> +int regmap_noinc_write(struct regmap *map, unsigned int reg,
> +		     const void *val, size_t val_len);
>  int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
>  			size_t val_count);
>  int regmap_multi_reg_write(struct regmap *map, const struct reg_sequence *regs,
> @@ -1222,6 +1234,13 @@ static inline int regmap_raw_write_async(struct regmap *map, unsigned int reg,
>  	return -EINVAL;
>  }
>  
> +static inline int regmap_noinc_write(struct regmap *map, unsigned int reg,
> +				    const void *val, size_t val_len)
> +{
> +	WARN_ONCE(1, "regmap API is disabled");
> +	return -EINVAL;
> +}
> +
>  static inline int regmap_bulk_write(struct regmap *map, unsigned int reg,
>  				    const void *val, size_t val_count)
>  {
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API
  2018-10-18 16:59   ` Andreas Färber
@ 2018-10-18 17:18     ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2018-10-18 17:18 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Ben Whitten, starnight, hasnain.virk, netdev, liuxuenetmail,
	shess, Ben Whitten, Greg Kroah-Hartman, Rafael J. Wysocki,
	linux-kernel, Stefan Rehm, linux-spi

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Thu, Oct 18, 2018 at 06:59:52PM +0200, Andreas Färber wrote:

> Mark, please take this one through your tree - I'll rebase the LoRa
> parts on linux-next then.

I don't have a copy of it, if I didn't apply it already and send a pull
request I probably got confused and thought I'd done that already, sorry
- can someone resend please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-10-12 16:26 ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Ben Whitten
@ 2018-12-29 19:25   ` Andreas Färber
  2018-12-29 20:16     ` Andreas Färber
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2018-12-29 19:25 UTC (permalink / raw)
  To: Ben Whitten
  Cc: starnight, netdev, David S. Miller, linux-kernel, linux-lpwan,
	Stephen Boyd, Michael Turquette, linux-clk, devicetree

Hi Ben,

+ linux-lpwan, linux-clk, devicetree

Am 12.10.18 um 18:26 schrieb Ben Whitten:
> From: Ben Whitten <ben.whitten@gmail.com>
> 
> The 32M is run from the radio, before we just enabled it based on
> the radio number but now we can use the clk framework to request the
> clk is started when we need it.
> 
> The 32M clock produced from the radio is really a gated version of
> tcxo which is a fixed clock provided by hardware, and isn't captured
> in this patch.
> 
> The sx1301 brings the clock up prior to calibration once the radios
> have probed themselves.
> 
> A sample dts showing the clk link:
> 	sx1301: sx1301@0 {

Nit: Node names should not duplicate model from compatible or label.
I've been using "lora" for both concentrator and radios.

> 		...
>                 clocks = <&radio1 0>;

Since you use #clock-cells of zero below, you are specifying two clocks
here, surely unintentional.

>                 clock-names = "clk32m";
> 
>                 radio-spi {
>                         radio0: radio-a@0 {

-a/-b in node names duplicates the unit address. I've been using "lora",
but we might also go for just "radio" or "transceiver"?

>                                 ...
>                         };
> 
>                         radio1: radio-b@1 {

Should add "..." here, too, for clarity.

>                                 #clock-cells = <0>;
>                                 clock-output-names = "clk32m";

Personally I'd reorder those two lines to have #foo-cells last.

But more importantly, in my testing this is lacking, e.g.,
clocks = <&clk32m>;
and an appropriate fixed-clock node in the root node. See inline.

>                         };
>                 };
> 	};

This issue is making me think we should start properly documenting our
dt-bindings in a preceding patch, even if just for my linux-lora staging
tree. Be it in the old .txt format or Rob's newly proposed YAML.

The more cooks are working on SX130x, the better we need to explain what
changes people need to make to their DT to keep things working - if I
stumble as original author, then new testers are even more likely to!

> 
> Signed-off-by: Ben Whitten <ben.whitten@gmail.com>
> ---
>  drivers/net/lora/sx125x.c | 112 ++++++++++++++++++++++++++++++++++++++++++----
>  drivers/net/lora/sx1301.c |  13 ++++++
>  drivers/net/lora/sx1301.h |   2 +
>  3 files changed, 119 insertions(+), 8 deletions(-)

In general I'd appreciate if you would prepare, e.g., the clock output
in a preceding sx125x-only patch. Exposing (and consuming) clocks in the
radio driver should be independent of consuming them in the concentrator
driver. That'll make things easier to review for us and also helps avoid
the unusual "sx125x sx1301" in the subject, here and elsewhere.

> 
> diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
> index 36b61b1..b7ca782 100644
> --- a/drivers/net/lora/sx125x.c
> +++ b/drivers/net/lora/sx125x.c
> @@ -9,6 +9,8 @@
>   * Copyright (c) 2013 Semtech-Cycleo
>   */
>  
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -42,10 +44,16 @@ static const struct reg_field sx125x_regmap_fields[] = {
>  };
>  
>  struct sx125x_priv {
> +	struct clk		*clkout;
> +	struct clk_hw		clkout_hw;

The 0-day bots have reported this to break on sparc64, as you've seen.
We'll need to squash a Kconfig or .c based solution here.

> +
> +	struct device		*dev;
>  	struct regmap		*regmap;
>  	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
>  };
>  
> +#define to_clkout(_hw) container_of(_hw, struct sx125x_priv, clkout_hw)
> +
>  static struct regmap_config __maybe_unused sx125x_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
> @@ -64,6 +72,96 @@ static int sx125x_field_write(struct sx125x_priv *priv,
>  	return regmap_field_write(priv->regmap_fields[field_id], val);
>  }
>  
> +static int sx125x_field_read(struct sx125x_priv *priv,
> +		enum sx125x_fields field_id, unsigned int *val)
> +{
> +	return regmap_field_read(priv->regmap_fields[field_id], val);
> +}

Nit: Given how trivial this is, we might make it static inline?

> +
> +static int sx125x_clkout_enable(struct clk_hw *hw)
> +{
> +	struct sx125x_priv *priv = to_clkout(hw);
> +
> +	dev_info(priv->dev, "enabling clkout\n");
> +	return sx125x_field_write(priv, F_CLK_OUT, 1);
> +}
> +
> +static void sx125x_clkout_disable(struct clk_hw *hw)
> +{
> +	struct sx125x_priv *priv = to_clkout(hw);
> +	int ret;
> +
> +	dev_info(priv->dev, "disabling clkout\n");
> +	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
> +	if (ret)
> +		dev_err(priv->dev, "error disabling clkout\n");
> +}
> +
> +static int sx125x_clkout_is_enabled(struct clk_hw *hw)
> +{
> +	struct sx125x_priv *priv = to_clkout(hw);
> +	unsigned int enabled;
> +	int ret;
> +
> +	ret = sx125x_field_read(priv, F_CLK_OUT, &enabled);
> +	if (ret) {
> +		dev_err(priv->dev, "error reading clk enable\n");
> +		return 0;
> +	}
> +	return enabled;
> +}
> +
> +static const struct clk_ops sx125x_clkout_ops = {
> +	.enable = sx125x_clkout_enable,
> +	.disable = sx125x_clkout_disable,
> +	.is_enabled = sx125x_clkout_is_enabled,
> +};
> +
> +static int sx125x_register_clock_provider(struct sx125x_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct clk_init_data init;
> +	const char *parent;
> +	int ret;
> +
> +	/* Disable CLKOUT */
> +	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
> +	if (ret) {
> +		dev_err(dev, "unable to disable clkout\n");
> +		return ret;
> +	}
> +
> +	/* Register clock provider if expected in DTB */
> +	if (!of_find_property(dev->of_node, "#clock-cells", NULL))
> +		return 0;
> +
> +	dev_info(dev, "registering clkout\n");
> +
> +	parent = of_clk_get_parent_name(dev->of_node, 0);
> +	if (!parent) {
> +		dev_err(dev, "Unable to find parent clk\n");
> +		return -ENODEV;

I got stuck here testing:

[233875.731268] sx1301 spi0.0: SX1301 module probed
[233876.520801] sx1301 spi1.0: SX1301 module probed
[233876.543460] sx125x_con spi0.0-b: SX125x version: 21
[233876.550866] sx125x_con spi0.0-b: registering clkout
[233876.555852] sx125x_con spi0.0-b: Unable to find parent clk
[233876.561491] sx125x_con spi0.0-b: failed to register clkout provider: -19
[233876.569914] sx125x_con spi0.0-a: SX125x version: 21
[233876.582915] sx125x_con spi0.0-a: SX125x module probed
[233876.589100] sx125x_con spi1.0-b: SX125x version: 21
[233876.595981] sx125x_con spi1.0-b: registering clkout
[233876.600986] sx125x_con spi1.0-b: Unable to find parent clk
[233876.606557] sx125x_con spi1.0-b: failed to register clkout provider: -19
[233876.614559] sx125x_con spi1.0-a: SX125x version: 21
[233876.625047] sx125x_con spi1.0-a: SX125x module probed

Your DT example above does not use any parent clock for the radios. It
seems adding any clocks = <> reference helps resolve that error.

I don't spot any code dealing with enabling that parent either. With a
fixed-clock we appear to get around that, except for reference counting:

# cat /sys/kernel/debug/clk/clk_summary
[...]
 rak831-clk32m                        0        0        0    32000000
       0     0  50000
    rak831_clk32m                     0        0        0    32000000
       0     0  50000
 rg186-clk32m                         0        0        0    32000000
       0     0  50000
    rg186_clk32m                      0        0        0    32000000
       0     0  50000

But it might just as well be a gpio-gate-clock or some PMIC providing
it, needing prepare+enable ops.

> +	}
> +
> +	init.ops = &sx125x_clkout_ops;
> +	init.flags = CLK_IS_BASIC;

I don't think that's really true.

> +	init.parent_names = &parent;
> +	init.num_parents = 1;
> +	priv->clkout_hw.init = &init;
> +
> +	of_property_read_string_index(dev->of_node, "clock-output-names", 0,
> +			&init.name);

Error handling for this was in your style cleanup patch. I'd prefer to
squash that hunk here.

However, clock-output-names is documented as an optional property, so we
shouldn't rely on it here, please.

> +
> +	priv->clkout = devm_clk_register(dev, &priv->clkout_hw);
> +	if (IS_ERR(priv->clkout)) {
> +		dev_err(dev, "failed to register clkout\n");

Would be nice to output the error code, but then again the caller does.

> +		return PTR_ERR(priv->clkout);
> +	}
> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
> +			&priv->clkout_hw);


Don't we need to unregister the provider on remove? Using the devm_
variant would seem to handle that for us.

> +	return ret;
> +}

I wonder whether we may want to split (some of) this off into a _clk.c
for Makefile-based optional compilation? We could then provide a static
inline version of sx125x_register_clock_provider() as alternative and
have this file not care.

> +
>  static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap *regmap, unsigned int radio)
>  {
>  	struct sx125x_priv *priv;
> @@ -76,6 +174,7 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
>  		return -ENOMEM;
>  
>  	dev_set_drvdata(dev, priv);
> +	priv->dev = dev;
>  	priv->regmap = regmap;
>  	for (i = 0; i < ARRAY_SIZE(sx125x_regmap_fields); i++) {
>  		const struct reg_field *reg_fields = sx125x_regmap_fields;
> @@ -99,16 +198,13 @@ static int __maybe_unused sx125x_regmap_probe(struct device *dev, struct regmap
>  		dev_info(dev, "SX125x version: %02x\n", val);
>  	}
>  
> -	if (radio == 1) { /* HACK */
> -		ret = sx125x_field_write(priv, F_CLK_OUT, 1);
> -		if (ret) {
> -			dev_err(dev, "enabling clock output failed\n");
> -			return ret;
> -		}
> -
> -		dev_info(dev, "enabling clock output\n");
> +	ret = sx125x_register_clock_provider(priv);
> +	if (ret) {
> +		dev_err(dev, "failed to register clkout provider: %d\n", ret);
> +		return ret;
>  	}
>  
> +	/* TODO Only needs setting on radio on the TX path */

Slightly unrelated, but thanks for documenting. :)

>  	ret = sx125x_field_write(priv, F_TX_DAC_CLK_SEL, 1);
>  	if (ret) {
>  		dev_err(dev, "clock select failed\n");
> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> index 339f8d9..23cbddc3 100644
> --- a/drivers/net/lora/sx1301.c
> +++ b/drivers/net/lora/sx1301.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/firmware.h>
>  #include <linux/lora.h>
> @@ -378,6 +379,18 @@ static int sx130x_loradev_open(struct net_device *netdev)
>  		return -ENXIO;
>  	}
>  
> +	priv->clk32m = devm_clk_get(priv->dev, "clk32m");
> +	if (IS_ERR(priv->clk32m)) {
> +		dev_err(priv->dev, "failed to get clk32m\n");
> +		return PTR_ERR(priv->clk32m);
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk32m);

Does this cope with an absent/NULL clock?

> +	if (ret) {
> +		dev_err(priv->dev, "failed to enable clk32m: %d\n", ret);
> +		return ret;
> +	}
> +
>  	ret = sx1301_field_write(priv, F_GLOBAL_EN, 1);
>  	if (ret) {
>  		dev_err(priv->dev, "enable global clocks failed\n");
> diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
> index 0bbd948..a1a2e38 100644
> --- a/drivers/net/lora/sx1301.h
> +++ b/drivers/net/lora/sx1301.h
> @@ -9,6 +9,7 @@
>  #ifndef _SX1301_
>  #define _SX1301_
>  
> +#include <linux/clk.h>
>  #include <linux/regmap.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/lora/dev.h>
> @@ -108,6 +109,7 @@ static const struct reg_field sx1301_regmap_fields[] = {
>  struct sx1301_priv {
>  	struct lora_dev_priv lora;
>  	struct device		*dev;
> +	struct clk		*clk32m;
>  	struct gpio_desc *rst_gpio;
>  	struct regmap		*regmap;
>  	struct regmap_field     *regmap_fields[ARRAY_SIZE(sx1301_regmap_fields)];

Thanks again for contributing this cleanup series!
(already staged in linux-lora.git)

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-29 19:25   ` Andreas Färber
@ 2018-12-29 20:16     ` Andreas Färber
  2018-12-30 10:55       ` Andreas Färber
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2018-12-29 20:16 UTC (permalink / raw)
  To: Ben Whitten
  Cc: devicetree, linux-clk, netdev, Michael Turquette, Stephen Boyd,
	linux-lpwan, linux-kernel, starnight, David S. Miller

Am 29.12.18 um 20:25 schrieb Andreas Färber:
> Am 12.10.18 um 18:26 schrieb Ben Whitten:
>> +static int sx125x_register_clock_provider(struct sx125x_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	struct clk_init_data init;
>> +	const char *parent;
>> +	int ret;
>> +
>> +	/* Disable CLKOUT */
>> +	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
>> +	if (ret) {
>> +		dev_err(dev, "unable to disable clkout\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Register clock provider if expected in DTB */
>> +	if (!of_find_property(dev->of_node, "#clock-cells", NULL))
>> +		return 0;
>> +
>> +	dev_info(dev, "registering clkout\n");
>> +
>> +	parent = of_clk_get_parent_name(dev->of_node, 0);
>> +	if (!parent) {
>> +		dev_err(dev, "Unable to find parent clk\n");
>> +		return -ENODEV;
> 
> I got stuck here testing:
> 
> [233875.731268] sx1301 spi0.0: SX1301 module probed
> [233876.520801] sx1301 spi1.0: SX1301 module probed
> [233876.543460] sx125x_con spi0.0-b: SX125x version: 21
> [233876.550866] sx125x_con spi0.0-b: registering clkout
> [233876.555852] sx125x_con spi0.0-b: Unable to find parent clk
> [233876.561491] sx125x_con spi0.0-b: failed to register clkout provider: -19
> [233876.569914] sx125x_con spi0.0-a: SX125x version: 21
> [233876.582915] sx125x_con spi0.0-a: SX125x module probed
> [233876.589100] sx125x_con spi1.0-b: SX125x version: 21
> [233876.595981] sx125x_con spi1.0-b: registering clkout
> [233876.600986] sx125x_con spi1.0-b: Unable to find parent clk
> [233876.606557] sx125x_con spi1.0-b: failed to register clkout provider: -19
> [233876.614559] sx125x_con spi1.0-a: SX125x version: 21
> [233876.625047] sx125x_con spi1.0-a: SX125x module probed
> 
> Your DT example above does not use any parent clock for the radios. It
> seems adding any clocks = <> reference helps resolve that error.
> 
> I don't spot any code dealing with enabling that parent either. With a
> fixed-clock we appear to get around that, except for reference counting:
> 

This was before `sudo ip link set lora1 up`...

> # cat /sys/kernel/debug/clk/clk_summary
> [...]
>  rak831-clk32m                        0        0        0    32000000
>        0     0  50000
>     rak831_clk32m                     0        0        0    32000000
>        0     0  50000
>  rg186-clk32m                         0        0        0    32000000
>        0     0  50000
>     rg186_clk32m                      0        0        0    32000000
>        0     0  50000

Afterwards things seemed okay, but I didn't capture clk_summary again.

> 
> But it might just as well be a gpio-gate-clock or some PMIC providing
> it, needing prepare+enable ops.
> 
>> +	}
>> +
>> +	init.ops = &sx125x_clkout_ops;
>> +	init.flags = CLK_IS_BASIC;
> 
> I don't think that's really true.
> 
>> +	init.parent_names = &parent;
>> +	init.num_parents = 1;
>> +	priv->clkout_hw.init = &init;
>> +
>> +	of_property_read_string_index(dev->of_node, "clock-output-names", 0,
>> +			&init.name);
> 
> Error handling for this was in your style cleanup patch. I'd prefer to
> squash that hunk here.
> 
> However, clock-output-names is documented as an optional property, so we
> shouldn't rely on it here, please.
> 
>> +
>> +	priv->clkout = devm_clk_register(dev, &priv->clkout_hw);
>> +	if (IS_ERR(priv->clkout)) {
>> +		dev_err(dev, "failed to register clkout\n");
> 
> Would be nice to output the error code, but then again the caller does.
> 
>> +		return PTR_ERR(priv->clkout);
>> +	}
>> +	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
>> +			&priv->clkout_hw);
> 
> 
> Don't we need to unregister the provider on remove? Using the devm_
> variant would seem to handle that for us.

Possibly related, `sudo ip link set lora2 up` froze my system, with both
lora1 and lora2 being sx1301. (I should script bringing all of them up!)

Investigating...

And thanks go to the multiple vendors who donated SX130x hardware for
test coverage, helping catch non-standard bugs like these early!

Regards,
Andreas

>> +	return ret;
>> +}
[snip]

[  473.605058] sx1301 spi0.0: SX1301 module probed
[  474.394760] sx1301 spi1.0: SX1301 module probed
[  485.637333] sx125x_con spi0.0-b: SX125x version: 21
[  485.645801] sx125x_con spi0.0-b: registering clkout
[  485.656684] sx125x_con spi0.0-b: SX125x module probed
[  485.663789] sx125x_con spi0.0-a: SX125x version: 21
[  485.677570] sx125x_con spi0.0-a: SX125x module probed
[  485.685713] sx125x_con spi1.0-b: SX125x version: 21
[  485.692210] sx125x_con spi1.0-b: registering clkout
[  485.701712] sx125x_con spi1.0-b: SX125x module probed
[  485.708067] sx125x_con spi1.0-a: SX125x version: 21
[  485.718707] sx125x_con spi1.0-a: SX125x module probed
[...]
[ 4603.171814] sx125x_con spi0.0-b: enabling clkout
[ 4603.697489] sx1301 spi0.0: AGC calibration firmware version 2
[ 4603.703782] sx1301 spi0.0: starting calibration...
[ 4606.030617] sx1301 spi0.0: AGC status: 87
[ 4607.034555] sx1301 spi0.0: AGC firmware version 4
[ 4607.039666] sx1301 spi0.0: ARB firmware version 1
[...]
[ 4628.196884] sx125x_con spi1.0-b: enabling clkout
[ 4968.763990] systemd[1]: systemd-udevd.service: State 'stop-sigterm'
timed out. Killing.
[ 4968.772167] systemd[1]: systemd-udevd.service: Killing process 455
(systemd-udevd) with signal SIGKILL.
[ 4968.782479] systemd[1]: systemd-logind.service: State 'stop-sigterm'
timed out. Killing.
[ 4968.790732] systemd[1]: systemd-logind.service: Killing process 642
(systemd-logind) with signal SIGKILL.
[ 5058.761523] systemd[1]: systemd-journald.service: State
'stop-sigabrt' timed out. Terminating.
[ 5059.011507] systemd[1]: systemd-udevd.service: Processes still around
after SIGKILL. Ignoring.
[ 5059.021610] systemd[1]: systemd-logind.service: Processes still
around after SIGKILL. Ignoring.
[ 5149.009055] systemd[1]: systemd-journald.service: State
'stop-sigterm' timed out. Killing.
[ 5149.017475] systemd[1]: systemd-journald.service: Killing process 440
(systemd-journal) with signal SIGKILL.
[ 5149.259029] systemd[1]: systemd-udevd.service: State
'stop-final-sigterm' timed out. Killing.
[ 5259.871913] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
[(md-udevd):4380]
[ 5259.871918] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
[(d-logind):4379]
[ 5259.871926] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5259.879226] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5259.886531] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
     4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5259.967929] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
     4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5260.049319] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5260.061466] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5260.073615] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5260.080389] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5260.087174] pc : smp_call_function_many+0x31c/0x370
[ 5260.091944] pc : smp_call_function_many+0x31c/0x370
[ 5260.096722] lr : smp_call_function_many+0x2d8/0x370
[ 5260.101589] lr : smp_call_function_many+0x2d8/0x370
[ 5260.106453] sp : ffff00000f11bc10
[ 5260.111320] sp : ffff00000f123c10
[ 5260.116187] x29: ffff00000f11bc10 x28: 0000000000000180
[ 5260.119494] x29: ffff00000f123c10 x28: 0000000000000180
[ 5260.122801] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
[ 5260.128101] x27: ffff0000092cb380 x26: ffff80007ffc13f8
[ 5260.133400] x25: 0000000000000000 x24: ffff000008188608
[ 5260.138701] x25: 0000000000000000 x24: ffff000008188608
[ 5260.144001] x23: 0000000000000001 x22: ffff00000967a614
[ 5260.149301] x23: 0000000000000001 x22: ffff00000967a614
[ 5260.154601] x21: ffff000009679738 x20: ffff80007ff8f3c8
[ 5260.159901] x21: ffff000009679738 x20: ffff80007ffc13c8
[ 5260.165201] x19: ffff80007ff8f3c0 x18: 0000000000000000
[ 5260.170501] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
[ 5260.175800] x17: 0000000000000000 x16: 0000000000000000
[ 5260.181100] x17: 0000000000000000 x16: ffff80007ffb1fe0
[ 5260.186400] x15: 0000000000000000 x14: 0005006100000006
[ 5260.191700] x15: 000000008287d0ce x14: 0000000026c4ee0b
[ 5260.197000] x13: 0000000100010035 x12: 0140000000000000
[ 5260.202299] x13: 000000003dddfe6f x12: 0000000021da966e
[ 5260.207600] x11: 0040000000000001 x10: ffff0000017d2000
[ 5260.212899] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
[ 5260.218199] x9 : 0000000000000000 x8 : 0000000000000006
[ 5260.223499] x9 : 00000000644f2575 x8 : 0000000000000006
[ 5260.228798] x7 : 0000000000000000 x6 : ffff80007ffcafe0
[ 5260.234098] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
[ 5260.239399] x5 : ffff80007ffcafe0 x4 : 000000000000000d
[ 5260.244698] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
[ 5260.249998] x3 : 0000000000000000 x2 : ffff80007ff7fff8
[ 5260.255298] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
[ 5260.260598] x1 : 0000000000000003 x0 : 0000000000000000
[ 5260.265897] x1 : 0000000000000003 x0 : 0000000000000000
[ 5260.271197] Call trace:
[ 5260.276496] Call trace:
[ 5260.281799]  smp_call_function_many+0x31c/0x370
[ 5260.284239]  smp_call_function_many+0x31c/0x370
[ 5260.286680]  kick_all_cpus_sync+0x30/0x38
[ 5260.291199]  kick_all_cpus_sync+0x30/0x38
[ 5260.295722]  bpf_int_jit_compile+0x14c/0x420
[ 5260.299720]  bpf_int_jit_compile+0x14c/0x420
[ 5260.303721]  bpf_prog_select_runtime+0xec/0x138
[ 5260.307979]  bpf_prog_select_runtime+0xec/0x138
[ 5260.312243]  bpf_prepare_filter+0x468/0x520
[ 5260.316760]  bpf_prepare_filter+0x468/0x520
[ 5260.321280]  bpf_prog_create_from_user+0xe0/0x188
[ 5260.325453]  bpf_prog_create_from_user+0xe0/0x188
[ 5260.329628]  do_seccomp+0x2a0/0x6a0
[ 5260.334319]  do_seccomp+0x2a0/0x6a0
[ 5260.339012]  __arm64_sys_seccomp+0x28/0x38
[ 5260.342492]  __arm64_sys_seccomp+0x28/0x38
[ 5260.345974]  el0_svc_common+0x98/0x100
[ 5260.350060]  el0_svc_common+0x98/0x100
[ 5260.354145]  el0_svc_handler+0x38/0x78
[ 5260.357885]  el0_svc_handler+0x38/0x78
[ 5260.361625]  el0_svc+0x8/0xc
[ 5260.365364]  el0_svc+0x8/0xc
[ 5287.871131] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
[(md-udevd):4380]
[ 5287.871135] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
[(d-logind):4379]
[ 5287.871139] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5287.878433] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5287.885735] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5287.967133] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5288.048523] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5288.060670] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5288.072819] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5288.079592] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5288.086367] pc : smp_call_function_many+0x31c/0x370
[ 5288.091147] pc : smp_call_function_many+0x31c/0x370
[ 5288.095925] lr : smp_call_function_many+0x2d8/0x370
[ 5288.100791] lr : smp_call_function_many+0x2d8/0x370
[ 5288.105656] sp : ffff00000f11bc10
[ 5288.110522] sp : ffff00000f123c10
[ 5288.115389] x29: ffff00000f11bc10 x28: 0000000000000180
[ 5288.118696] x29: ffff00000f123c10 x28: 0000000000000180
[ 5288.122003] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
[ 5288.127303] x27: ffff0000092cb380 x26: ffff80007ffc13f8
[ 5288.132602] x25: 0000000000000000 x24: ffff000008188608
[ 5288.137902] x25: 0000000000000000 x24: ffff000008188608
[ 5288.143202] x23: 0000000000000001 x22: ffff00000967a614
[ 5288.148501] x23: 0000000000000001 x22: ffff00000967a614
[ 5288.153802] x21: ffff000009679738 x20: ffff80007ff8f3c8
[ 5288.159101] x21: ffff000009679738 x20: ffff80007ffc13c8
[ 5288.164401] x19: ffff80007ff8f3c0 x18: 0000000000000000
[ 5288.169701] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
[ 5288.175000] x17: 0000000000000000 x16: 0000000000000000
[ 5288.180300] x17: 0000000000000000 x16: ffff80007ffb1fe0
[ 5288.185600] x15: 0000000000000000 x14: 0005006100000006
[ 5288.190899] x15: 000000008287d0ce x14: 0000000026c4ee0b
[ 5288.196200] x13: 0000000100010035 x12: 0140000000000000
[ 5288.201500] x13: 000000003dddfe6f x12: 0000000021da966e
[ 5288.206801] x11: 0040000000000001 x10: ffff0000017d2000
[ 5288.212100] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
[ 5288.217400] x9 : 0000000000000000 x8 : 0000000000000006
[ 5288.222700] x9 : 00000000644f2575 x8 : 0000000000000006
[ 5288.228001] x7 : 0000000000000000 x6 : ffff80007ffcafe0
[ 5288.233301] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
[ 5288.238601] x5 : ffff80007ffcafe0 x4 : 000000000000000d
[ 5288.243901] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
[ 5288.249200] x3 : 0000000000000000 x2 : ffff80007ff7fff8
[ 5288.254501] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
[ 5288.259800] x1 : 0000000000000003 x0 : 0000000000000000
[ 5288.265100] x1 : 0000000000000003 x0 : 0000000000000000
[ 5288.270399] Call trace:
[ 5288.275699] Call trace:
[ 5288.281002]  smp_call_function_many+0x31c/0x370
[ 5288.283442]  smp_call_function_many+0x31c/0x370
[ 5288.285881]  kick_all_cpus_sync+0x30/0x38
[ 5288.290401]  kick_all_cpus_sync+0x30/0x38
[ 5288.294921]  bpf_int_jit_compile+0x14c/0x420
[ 5288.298921]  bpf_int_jit_compile+0x14c/0x420
[ 5288.302920]  bpf_prog_select_runtime+0xec/0x138
[ 5288.307180]  bpf_prog_select_runtime+0xec/0x138
[ 5288.311441]  bpf_prepare_filter+0x468/0x520
[ 5288.315961]  bpf_prepare_filter+0x468/0x520
[ 5288.320481]  bpf_prog_create_from_user+0xe0/0x188
[ 5288.324654]  bpf_prog_create_from_user+0xe0/0x188
[ 5288.328827]  do_seccomp+0x2a0/0x6a0
[ 5288.333520]  do_seccomp+0x2a0/0x6a0
[ 5288.338213]  __arm64_sys_seccomp+0x28/0x38
[ 5288.341694]  __arm64_sys_seccomp+0x28/0x38
[ 5288.345175]  el0_svc_common+0x98/0x100
[ 5288.349262]  el0_svc_common+0x98/0x100
[ 5288.353349]  el0_svc_handler+0x38/0x78
[ 5288.357089]  el0_svc_handler+0x38/0x78
[ 5288.360827]  el0_svc+0x8/0xc
[ 5288.364567]  el0_svc+0x8/0xc
[ 5315.870363] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
[(md-udevd):4380]
[ 5315.870367] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
[(d-logind):4379]
[ 5315.870371] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5315.877665] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5315.884967] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5315.966366] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5316.047756] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5316.059904] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5316.072052] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5316.078826] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5316.085600] pc : smp_call_function_many+0x31c/0x370
[ 5316.090379] pc : smp_call_function_many+0x31c/0x370
[ 5316.095158] lr : smp_call_function_many+0x2d8/0x370
[ 5316.100025] lr : smp_call_function_many+0x2d8/0x370
[ 5316.104889] sp : ffff00000f11bc10
[ 5316.109756] sp : ffff00000f123c10
[ 5316.114623] x29: ffff00000f11bc10 x28: 0000000000000180
[ 5316.117929] x29: ffff00000f123c10 x28: 0000000000000180
[ 5316.121236] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
[ 5316.126536] x27: ffff0000092cb380 x26: ffff80007ffc13f8
[ 5316.131835] x25: 0000000000000000 x24: ffff000008188608
[ 5316.137135] x25: 0000000000000000 x24: ffff000008188608
[ 5316.142435] x23: 0000000000000001 x22: ffff00000967a614
[ 5316.147735] x23: 0000000000000001 x22: ffff00000967a614
[ 5316.153035] x21: ffff000009679738 x20: ffff80007ff8f3c8
[ 5316.158334] x21: ffff000009679738 x20: ffff80007ffc13c8
[ 5316.163634] x19: ffff80007ff8f3c0 x18: 0000000000000000
[ 5316.168934] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
[ 5316.174234] x17: 0000000000000000 x16: 0000000000000000
[ 5316.179533] x17: 0000000000000000 x16: ffff80007ffb1fe0
[ 5316.184833] x15: 0000000000000000 x14: 0005006100000006
[ 5316.190132] x15: 000000008287d0ce x14: 0000000026c4ee0b
[ 5316.195432] x13: 0000000100010035 x12: 0140000000000000
[ 5316.200732] x13: 000000003dddfe6f x12: 0000000021da966e
[ 5316.206031] x11: 0040000000000001 x10: ffff0000017d2000
[ 5316.211331] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
[ 5316.216630] x9 : 0000000000000000 x8 : 0000000000000006
[ 5316.221930] x9 : 00000000644f2575 x8 : 0000000000000006
[ 5316.227229] x7 : 0000000000000000 x6 : ffff80007ffcafe0
[ 5316.232529] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
[ 5316.237829] x5 : ffff80007ffcafe0 x4 : 000000000000000d
[ 5316.243128] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
[ 5316.248428] x3 : 0000000000000000 x2 : ffff80007ff7fff8
[ 5316.253727] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
[ 5316.259027] x1 : 0000000000000003 x0 : 0000000000000000
[ 5316.264327] x1 : 0000000000000003 x0 : 0000000000000000
[ 5316.269626] Call trace:
[ 5316.274926] Call trace:
[ 5316.280228]  smp_call_function_many+0x31c/0x370
[ 5316.282668]  smp_call_function_many+0x31c/0x370
[ 5316.285107]  kick_all_cpus_sync+0x30/0x38
[ 5316.289626]  kick_all_cpus_sync+0x30/0x38
[ 5316.294146]  bpf_int_jit_compile+0x14c/0x420
[ 5316.298145]  bpf_int_jit_compile+0x14c/0x420
[ 5316.302144]  bpf_prog_select_runtime+0xec/0x138
[ 5316.306405]  bpf_prog_select_runtime+0xec/0x138
[ 5316.310665]  bpf_prepare_filter+0x468/0x520
[ 5316.315185]  bpf_prepare_filter+0x468/0x520
[ 5316.319705]  bpf_prog_create_from_user+0xe0/0x188
[ 5316.323879]  bpf_prog_create_from_user+0xe0/0x188
[ 5316.328051]  do_seccomp+0x2a0/0x6a0
[ 5316.332744]  do_seccomp+0x2a0/0x6a0
[ 5316.337438]  __arm64_sys_seccomp+0x28/0x38
[ 5316.340918]  __arm64_sys_seccomp+0x28/0x38
[ 5316.344400]  el0_svc_common+0x98/0x100
[ 5316.348487]  el0_svc_common+0x98/0x100
[ 5316.352573]  el0_svc_handler+0x38/0x78
[ 5316.356313]  el0_svc_handler+0x38/0x78
[ 5316.360051]  el0_svc+0x8/0xc
[ 5316.363791]  el0_svc+0x8/0xc
[ 5343.869595] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
[(md-udevd):4380]
[ 5343.869599] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
[(d-logind):4379]
[ 5343.869603] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5343.876897] Modules linked in: lora_sx125x(O) lora_sx1301(O)
lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
[ 5343.884200] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5343.965599] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
[ 5344.046990] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5344.059138] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
[ 5344.071286] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5344.078059] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 5344.084834] pc : smp_call_function_many+0x31c/0x370
[ 5344.089614] pc : smp_call_function_many+0x31c/0x370
[ 5344.094393] lr : smp_call_function_many+0x2d8/0x370
[ 5344.099260] lr : smp_call_function_many+0x2d8/0x370
[ 5344.104124] sp : ffff00000f11bc10
[ 5344.108990] sp : ffff00000f123c10
[ 5344.113858] x29: ffff00000f11bc10 x28: 0000000000000180
[ 5344.117164] x29: ffff00000f123c10 x28: 0000000000000180
[ 5344.120471] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
[ 5344.125771] x27: ffff0000092cb380 x26: ffff80007ffc13f8
[ 5344.131071] x25: 0000000000000000 x24: ffff000008188608
[ 5344.136371] x25: 0000000000000000 x24: ffff000008188608
[ 5344.141670] x23: 0000000000000001 x22: ffff00000967a614
[ 5344.146970] x23: 0000000000000001 x22: ffff00000967a614
[ 5344.152270] x21: ffff000009679738 x20: ffff80007ff8f3c8
[ 5344.157569] x21: ffff000009679738 x20: ffff80007ffc13c8
[ 5344.162869] x19: ffff80007ff8f3c0 x18: 0000000000000000
[ 5344.168169] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
[ 5344.173469] x17: 0000000000000000 x16: 0000000000000000
[ 5344.178768] x17: 0000000000000000 x16: ffff80007ffb1fe0
[ 5344.184068] x15: 0000000000000000 x14: 0005006100000006
[ 5344.189368] x15: 000000008287d0ce x14: 0000000026c4ee0b
[ 5344.194668] x13: 0000000100010035 x12: 0140000000000000
[ 5344.199967] x13: 000000003dddfe6f x12: 0000000021da966e
[ 5344.205267] x11: 0040000000000001 x10: ffff0000017d2000
[ 5344.210566] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
[ 5344.215866] x9 : 0000000000000000 x8 : 0000000000000006
[ 5344.221165] x9 : 00000000644f2575 x8 : 0000000000000006
[ 5344.226466] x7 : 0000000000000000 x6 : ffff80007ffcafe0
[ 5344.231765] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
[ 5344.237065] x5 : ffff80007ffcafe0 x4 : 000000000000000d
[ 5344.242365] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
[ 5344.247664] x3 : 0000000000000000 x2 : ffff80007ff7fff8
[ 5344.252964] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
[ 5344.258265] x1 : 0000000000000003 x0 : 0000000000000000
[ 5344.263564] x1 : 0000000000000003 x0 : 0000000000000000
[ 5344.268863] Call trace:
[ 5344.274164] Call trace:
[ 5344.279467]  smp_call_function_many+0x31c/0x370
[ 5344.281907]  smp_call_function_many+0x31c/0x370
[ 5344.284346]  kick_all_cpus_sync+0x30/0x38
[ 5344.288865]  kick_all_cpus_sync+0x30/0x38
[ 5344.293386]  bpf_int_jit_compile+0x14c/0x420
[ 5344.297385]  bpf_int_jit_compile+0x14c/0x420
[ 5344.301384]  bpf_prog_select_runtime+0xec/0x138
[ 5344.305644]  bpf_prog_select_runtime+0xec/0x138
[ 5344.309905]  bpf_prepare_filter+0x468/0x520
[ 5344.314426]  bpf_prepare_filter+0x468/0x520
[ 5344.318946]  bpf_prog_create_from_user+0xe0/0x188
[ 5344.323118]  bpf_prog_create_from_user+0xe0/0x188
[ 5344.327291]  do_seccomp+0x2a0/0x6a0
[ 5344.331985]  do_seccomp+0x2a0/0x6a0
[ 5344.336678]  __arm64_sys_seccomp+0x28/0x38
[ 5344.340158]  __arm64_sys_seccomp+0x28/0x38
[ 5344.343638]  el0_svc_common+0x98/0x100
[ 5344.347725]  el0_svc_common+0x98/0x100
[ 5344.351812]  el0_svc_handler+0x38/0x78
[ 5344.355551]  el0_svc_handler+0x38/0x78
[ 5344.359290]  el0_svc+0x8/0xc
[ 5344.363030]  el0_svc+0x8/0xc

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-29 20:16     ` Andreas Färber
@ 2018-12-30 10:55       ` Andreas Färber
  2018-12-30 19:40         ` [PATCH lora-next] net: lora: sx125x: Add error handling for clock-output-names Andreas Färber
                           ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Andreas Färber @ 2018-12-30 10:55 UTC (permalink / raw)
  To: Ben Whitten
  Cc: devicetree, David S. Miller, netdev, Michael Turquette,
	Stephen Boyd, linux-lpwan, linux-kernel, starnight, linux-clk,
	Mark Brown, linux-spi, Maxime Ripard, linux-arm-kernel

+ linux-spi, LAKML

Am 29.12.18 um 21:16 schrieb Andreas Färber:
> Am 29.12.18 um 20:25 schrieb Andreas Färber:
>> Am 12.10.18 um 18:26 schrieb Ben Whitten:
>>> +static int sx125x_register_clock_provider(struct sx125x_priv *priv)
>>> +{
>>> +	struct device *dev = priv->dev;
>>> +	struct clk_init_data init;
>>> +	const char *parent;
>>> +	int ret;
>>> +
>>> +	/* Disable CLKOUT */
>>> +	ret = sx125x_field_write(priv, F_CLK_OUT, 0);
>>> +	if (ret) {
>>> +		dev_err(dev, "unable to disable clkout\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	/* Register clock provider if expected in DTB */
>>> +	if (!of_find_property(dev->of_node, "#clock-cells", NULL))
>>> +		return 0;
>>> +
>>> +	dev_info(dev, "registering clkout\n");
>>> +
>>> +	parent = of_clk_get_parent_name(dev->of_node, 0);
>>> +	if (!parent) {
>>> +		dev_err(dev, "Unable to find parent clk\n");
>>> +		return -ENODEV;
[snip]
> `sudo ip link set lora2 up` froze my system, with both
> lora1 and lora2 being sx1301. [...]
> 
> Investigating...

I've bisected and confirmed that it was indeed this patch that regresses
for one of my SX1301 concentrators.

I noticed that this patch changed the behavior from writing the enable
bit only on one radio to also writing zero to the other. Default value
for the bit was enabled, so a functional change in theory. I therefore
tried enabling radio A clock output on the hanging one. However ...

I've posted a fix for our regmap_bus that lead to the sx125x writes not
taking effect. I.e., no clk_out was ever disabled. Otherwise only the
XOSC register writes were reordered compared to clock output enable, so
I spot no change to SX125x or SX130x registers that could cause this.

I've amended error handling and debug output (cf. below) and verified:

We never return from the sx125x_clkout_enable() performing the
regmap_field_write() on our regmap_bus, which in turn uses a SPI regmap
in sx1301_regmap_bus_read().

A notable difference between my two concentrators is that the working
one is using spi-gpio driver, the regressing one spi-sun6i.

Two things stood out in spi-sun6i: It uses a completion (I do not run
into its timeout warning!), and it uses clk_{get,set}_rate().

Given that observed symptoms were CPU stalls, workqueue hangs and RCU
problems, requiring a power-cycle to recover, I wonder whether we are
running into some atomic/locking issue with clk_enable()? Is it valid at
all to use SPI/regmap for clk_enable()? If it is, is there a known issue
specific to spi-sun6i (A64) in 4.20.0?
I already tried setting .disable_locking = true in both regmap_configs.
Any suggestions how to further debug?

Thanks,
Andreas

> [  473.605058] sx1301 spi0.0: SX1301 module probed
> [  474.394760] sx1301 spi1.0: SX1301 module probed
> [  485.637333] sx125x_con spi0.0-b: SX125x version: 21
> [  485.645801] sx125x_con spi0.0-b: registering clkout
> [  485.656684] sx125x_con spi0.0-b: SX125x module probed
> [  485.663789] sx125x_con spi0.0-a: SX125x version: 21
> [  485.677570] sx125x_con spi0.0-a: SX125x module probed
> [  485.685713] sx125x_con spi1.0-b: SX125x version: 21
> [  485.692210] sx125x_con spi1.0-b: registering clkout
> [  485.701712] sx125x_con spi1.0-b: SX125x module probed
> [  485.708067] sx125x_con spi1.0-a: SX125x version: 21
> [  485.718707] sx125x_con spi1.0-a: SX125x module probed
> [...]
> [ 4603.171814] sx125x_con spi0.0-b: enabling clkout
> [ 4603.697489] sx1301 spi0.0: AGC calibration firmware version 2
> [ 4603.703782] sx1301 spi0.0: starting calibration...
> [ 4606.030617] sx1301 spi0.0: AGC status: 87
> [ 4607.034555] sx1301 spi0.0: AGC firmware version 4
> [ 4607.039666] sx1301 spi0.0: ARB firmware version 1
> [...]
> [ 4628.196884] sx125x_con spi1.0-b: enabling clkout

[ 4096.183451] sx1301 spi1.0: sx1301_regmap_bus_read: radio B addr 0x10

There it does not return from its first SPI-backed regmap_write().

> [ 4968.763990] systemd[1]: systemd-udevd.service: State 'stop-sigterm'
> timed out. Killing.
> [ 4968.772167] systemd[1]: systemd-udevd.service: Killing process 455
> (systemd-udevd) with signal SIGKILL.
> [ 4968.782479] systemd[1]: systemd-logind.service: State 'stop-sigterm'
> timed out. Killing.
> [ 4968.790732] systemd[1]: systemd-logind.service: Killing process 642
> (systemd-logind) with signal SIGKILL.
> [ 5058.761523] systemd[1]: systemd-journald.service: State
> 'stop-sigabrt' timed out. Terminating.
> [ 5059.011507] systemd[1]: systemd-udevd.service: Processes still around
> after SIGKILL. Ignoring.
> [ 5059.021610] systemd[1]: systemd-logind.service: Processes still
> around after SIGKILL. Ignoring.
> [ 5149.009055] systemd[1]: systemd-journald.service: State
> 'stop-sigterm' timed out. Killing.
> [ 5149.017475] systemd[1]: systemd-journald.service: Killing process 440
> (systemd-journal) with signal SIGKILL.
> [ 5149.259029] systemd[1]: systemd-udevd.service: State
> 'stop-final-sigterm' timed out. Killing.
> [ 5259.871913] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
> [(md-udevd):4380]
> [ 5259.871918] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
> [(d-logind):4379]
> [ 5259.871926] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5259.879226] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5259.886531] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
>      4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5259.967929] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
>      4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5260.049319] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5260.061466] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5260.073615] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5260.080389] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5260.087174] pc : smp_call_function_many+0x31c/0x370
> [ 5260.091944] pc : smp_call_function_many+0x31c/0x370
> [ 5260.096722] lr : smp_call_function_many+0x2d8/0x370
> [ 5260.101589] lr : smp_call_function_many+0x2d8/0x370
> [ 5260.106453] sp : ffff00000f11bc10
> [ 5260.111320] sp : ffff00000f123c10
> [ 5260.116187] x29: ffff00000f11bc10 x28: 0000000000000180
> [ 5260.119494] x29: ffff00000f123c10 x28: 0000000000000180
> [ 5260.122801] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
> [ 5260.128101] x27: ffff0000092cb380 x26: ffff80007ffc13f8
> [ 5260.133400] x25: 0000000000000000 x24: ffff000008188608
> [ 5260.138701] x25: 0000000000000000 x24: ffff000008188608
> [ 5260.144001] x23: 0000000000000001 x22: ffff00000967a614
> [ 5260.149301] x23: 0000000000000001 x22: ffff00000967a614
> [ 5260.154601] x21: ffff000009679738 x20: ffff80007ff8f3c8
> [ 5260.159901] x21: ffff000009679738 x20: ffff80007ffc13c8
> [ 5260.165201] x19: ffff80007ff8f3c0 x18: 0000000000000000
> [ 5260.170501] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
> [ 5260.175800] x17: 0000000000000000 x16: 0000000000000000
> [ 5260.181100] x17: 0000000000000000 x16: ffff80007ffb1fe0
> [ 5260.186400] x15: 0000000000000000 x14: 0005006100000006
> [ 5260.191700] x15: 000000008287d0ce x14: 0000000026c4ee0b
> [ 5260.197000] x13: 0000000100010035 x12: 0140000000000000
> [ 5260.202299] x13: 000000003dddfe6f x12: 0000000021da966e
> [ 5260.207600] x11: 0040000000000001 x10: ffff0000017d2000
> [ 5260.212899] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
> [ 5260.218199] x9 : 0000000000000000 x8 : 0000000000000006
> [ 5260.223499] x9 : 00000000644f2575 x8 : 0000000000000006
> [ 5260.228798] x7 : 0000000000000000 x6 : ffff80007ffcafe0
> [ 5260.234098] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
> [ 5260.239399] x5 : ffff80007ffcafe0 x4 : 000000000000000d
> [ 5260.244698] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
> [ 5260.249998] x3 : 0000000000000000 x2 : ffff80007ff7fff8
> [ 5260.255298] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
> [ 5260.260598] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5260.265897] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5260.271197] Call trace:
> [ 5260.276496] Call trace:
> [ 5260.281799]  smp_call_function_many+0x31c/0x370
> [ 5260.284239]  smp_call_function_many+0x31c/0x370
> [ 5260.286680]  kick_all_cpus_sync+0x30/0x38
> [ 5260.291199]  kick_all_cpus_sync+0x30/0x38
> [ 5260.295722]  bpf_int_jit_compile+0x14c/0x420
> [ 5260.299720]  bpf_int_jit_compile+0x14c/0x420
> [ 5260.303721]  bpf_prog_select_runtime+0xec/0x138
> [ 5260.307979]  bpf_prog_select_runtime+0xec/0x138
> [ 5260.312243]  bpf_prepare_filter+0x468/0x520
> [ 5260.316760]  bpf_prepare_filter+0x468/0x520
> [ 5260.321280]  bpf_prog_create_from_user+0xe0/0x188
> [ 5260.325453]  bpf_prog_create_from_user+0xe0/0x188
> [ 5260.329628]  do_seccomp+0x2a0/0x6a0
> [ 5260.334319]  do_seccomp+0x2a0/0x6a0
> [ 5260.339012]  __arm64_sys_seccomp+0x28/0x38
> [ 5260.342492]  __arm64_sys_seccomp+0x28/0x38
> [ 5260.345974]  el0_svc_common+0x98/0x100
> [ 5260.350060]  el0_svc_common+0x98/0x100
> [ 5260.354145]  el0_svc_handler+0x38/0x78
> [ 5260.357885]  el0_svc_handler+0x38/0x78
> [ 5260.361625]  el0_svc+0x8/0xc
> [ 5260.365364]  el0_svc+0x8/0xc
> [ 5287.871131] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
> [(md-udevd):4380]
> [ 5287.871135] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
> [(d-logind):4379]
> [ 5287.871139] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5287.878433] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5287.885735] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5287.967133] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5288.048523] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5288.060670] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5288.072819] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5288.079592] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5288.086367] pc : smp_call_function_many+0x31c/0x370
> [ 5288.091147] pc : smp_call_function_many+0x31c/0x370
> [ 5288.095925] lr : smp_call_function_many+0x2d8/0x370
> [ 5288.100791] lr : smp_call_function_many+0x2d8/0x370
> [ 5288.105656] sp : ffff00000f11bc10
> [ 5288.110522] sp : ffff00000f123c10
> [ 5288.115389] x29: ffff00000f11bc10 x28: 0000000000000180
> [ 5288.118696] x29: ffff00000f123c10 x28: 0000000000000180
> [ 5288.122003] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
> [ 5288.127303] x27: ffff0000092cb380 x26: ffff80007ffc13f8
> [ 5288.132602] x25: 0000000000000000 x24: ffff000008188608
> [ 5288.137902] x25: 0000000000000000 x24: ffff000008188608
> [ 5288.143202] x23: 0000000000000001 x22: ffff00000967a614
> [ 5288.148501] x23: 0000000000000001 x22: ffff00000967a614
> [ 5288.153802] x21: ffff000009679738 x20: ffff80007ff8f3c8
> [ 5288.159101] x21: ffff000009679738 x20: ffff80007ffc13c8
> [ 5288.164401] x19: ffff80007ff8f3c0 x18: 0000000000000000
> [ 5288.169701] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
> [ 5288.175000] x17: 0000000000000000 x16: 0000000000000000
> [ 5288.180300] x17: 0000000000000000 x16: ffff80007ffb1fe0
> [ 5288.185600] x15: 0000000000000000 x14: 0005006100000006
> [ 5288.190899] x15: 000000008287d0ce x14: 0000000026c4ee0b
> [ 5288.196200] x13: 0000000100010035 x12: 0140000000000000
> [ 5288.201500] x13: 000000003dddfe6f x12: 0000000021da966e
> [ 5288.206801] x11: 0040000000000001 x10: ffff0000017d2000
> [ 5288.212100] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
> [ 5288.217400] x9 : 0000000000000000 x8 : 0000000000000006
> [ 5288.222700] x9 : 00000000644f2575 x8 : 0000000000000006
> [ 5288.228001] x7 : 0000000000000000 x6 : ffff80007ffcafe0
> [ 5288.233301] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
> [ 5288.238601] x5 : ffff80007ffcafe0 x4 : 000000000000000d
> [ 5288.243901] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
> [ 5288.249200] x3 : 0000000000000000 x2 : ffff80007ff7fff8
> [ 5288.254501] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
> [ 5288.259800] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5288.265100] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5288.270399] Call trace:
> [ 5288.275699] Call trace:
> [ 5288.281002]  smp_call_function_many+0x31c/0x370
> [ 5288.283442]  smp_call_function_many+0x31c/0x370
> [ 5288.285881]  kick_all_cpus_sync+0x30/0x38
> [ 5288.290401]  kick_all_cpus_sync+0x30/0x38
> [ 5288.294921]  bpf_int_jit_compile+0x14c/0x420
> [ 5288.298921]  bpf_int_jit_compile+0x14c/0x420
> [ 5288.302920]  bpf_prog_select_runtime+0xec/0x138
> [ 5288.307180]  bpf_prog_select_runtime+0xec/0x138
> [ 5288.311441]  bpf_prepare_filter+0x468/0x520
> [ 5288.315961]  bpf_prepare_filter+0x468/0x520
> [ 5288.320481]  bpf_prog_create_from_user+0xe0/0x188
> [ 5288.324654]  bpf_prog_create_from_user+0xe0/0x188
> [ 5288.328827]  do_seccomp+0x2a0/0x6a0
> [ 5288.333520]  do_seccomp+0x2a0/0x6a0
> [ 5288.338213]  __arm64_sys_seccomp+0x28/0x38
> [ 5288.341694]  __arm64_sys_seccomp+0x28/0x38
> [ 5288.345175]  el0_svc_common+0x98/0x100
> [ 5288.349262]  el0_svc_common+0x98/0x100
> [ 5288.353349]  el0_svc_handler+0x38/0x78
> [ 5288.357089]  el0_svc_handler+0x38/0x78
> [ 5288.360827]  el0_svc+0x8/0xc
> [ 5288.364567]  el0_svc+0x8/0xc
> [ 5315.870363] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
> [(md-udevd):4380]
> [ 5315.870367] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
> [(d-logind):4379]
> [ 5315.870371] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5315.877665] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5315.884967] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5315.966366] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5316.047756] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5316.059904] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5316.072052] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5316.078826] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5316.085600] pc : smp_call_function_many+0x31c/0x370
> [ 5316.090379] pc : smp_call_function_many+0x31c/0x370
> [ 5316.095158] lr : smp_call_function_many+0x2d8/0x370
> [ 5316.100025] lr : smp_call_function_many+0x2d8/0x370
> [ 5316.104889] sp : ffff00000f11bc10
> [ 5316.109756] sp : ffff00000f123c10
> [ 5316.114623] x29: ffff00000f11bc10 x28: 0000000000000180
> [ 5316.117929] x29: ffff00000f123c10 x28: 0000000000000180
> [ 5316.121236] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
> [ 5316.126536] x27: ffff0000092cb380 x26: ffff80007ffc13f8
> [ 5316.131835] x25: 0000000000000000 x24: ffff000008188608
> [ 5316.137135] x25: 0000000000000000 x24: ffff000008188608
> [ 5316.142435] x23: 0000000000000001 x22: ffff00000967a614
> [ 5316.147735] x23: 0000000000000001 x22: ffff00000967a614
> [ 5316.153035] x21: ffff000009679738 x20: ffff80007ff8f3c8
> [ 5316.158334] x21: ffff000009679738 x20: ffff80007ffc13c8
> [ 5316.163634] x19: ffff80007ff8f3c0 x18: 0000000000000000
> [ 5316.168934] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
> [ 5316.174234] x17: 0000000000000000 x16: 0000000000000000
> [ 5316.179533] x17: 0000000000000000 x16: ffff80007ffb1fe0
> [ 5316.184833] x15: 0000000000000000 x14: 0005006100000006
> [ 5316.190132] x15: 000000008287d0ce x14: 0000000026c4ee0b
> [ 5316.195432] x13: 0000000100010035 x12: 0140000000000000
> [ 5316.200732] x13: 000000003dddfe6f x12: 0000000021da966e
> [ 5316.206031] x11: 0040000000000001 x10: ffff0000017d2000
> [ 5316.211331] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
> [ 5316.216630] x9 : 0000000000000000 x8 : 0000000000000006
> [ 5316.221930] x9 : 00000000644f2575 x8 : 0000000000000006
> [ 5316.227229] x7 : 0000000000000000 x6 : ffff80007ffcafe0
> [ 5316.232529] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
> [ 5316.237829] x5 : ffff80007ffcafe0 x4 : 000000000000000d
> [ 5316.243128] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
> [ 5316.248428] x3 : 0000000000000000 x2 : ffff80007ff7fff8
> [ 5316.253727] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
> [ 5316.259027] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5316.264327] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5316.269626] Call trace:
> [ 5316.274926] Call trace:
> [ 5316.280228]  smp_call_function_many+0x31c/0x370
> [ 5316.282668]  smp_call_function_many+0x31c/0x370
> [ 5316.285107]  kick_all_cpus_sync+0x30/0x38
> [ 5316.289626]  kick_all_cpus_sync+0x30/0x38
> [ 5316.294146]  bpf_int_jit_compile+0x14c/0x420
> [ 5316.298145]  bpf_int_jit_compile+0x14c/0x420
> [ 5316.302144]  bpf_prog_select_runtime+0xec/0x138
> [ 5316.306405]  bpf_prog_select_runtime+0xec/0x138
> [ 5316.310665]  bpf_prepare_filter+0x468/0x520
> [ 5316.315185]  bpf_prepare_filter+0x468/0x520
> [ 5316.319705]  bpf_prog_create_from_user+0xe0/0x188
> [ 5316.323879]  bpf_prog_create_from_user+0xe0/0x188
> [ 5316.328051]  do_seccomp+0x2a0/0x6a0
> [ 5316.332744]  do_seccomp+0x2a0/0x6a0
> [ 5316.337438]  __arm64_sys_seccomp+0x28/0x38
> [ 5316.340918]  __arm64_sys_seccomp+0x28/0x38
> [ 5316.344400]  el0_svc_common+0x98/0x100
> [ 5316.348487]  el0_svc_common+0x98/0x100
> [ 5316.352573]  el0_svc_handler+0x38/0x78
> [ 5316.356313]  el0_svc_handler+0x38/0x78
> [ 5316.360051]  el0_svc+0x8/0xc
> [ 5316.363791]  el0_svc+0x8/0xc
> [ 5343.869595] watchdog: BUG: soft lockup - CPU#3 stuck for 22s!
> [(md-udevd):4380]
> [ 5343.869599] watchdog: BUG: soft lockup - CPU#1 stuck for 22s!
> [(d-logind):4379]
> [ 5343.869603] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5343.876897] Modules linked in: lora_sx125x(O) lora_sx1301(O)
> lora_sx128x(O) lora_sx1276(O) lora_rf1276ts(O) lora_mm002(O)
> lora_ting01m(O) lora_rak811(O) lora_usi(O) lora_wimod(O) lora_rn2483(O)
> lora_dev(O) nllora(O) lora(O) af_packet nls_iso8859_1 nls_cp437 vfat fat
> realtek aes_ce_blk crypto_simd cryptd dwmac_sun8i stmmac_platform
> snd_soc_simple_card snd_soc_spdif_tx snd_soc_simple_card_utils stmmac
> aes_ce_cipher snd_soc_core spi_sun6i mdio_mux ac97_bus snd_pcm_dmaengine
> snd_pcm sunxi_wdt snd_timer crct10dif_ce snd ghash_ce aes_arm64
> soundcore spi_gpio spi_bitbang uio_pdrv_genirq sha2_ce uio sha256_arm64
> sha1_ce btrfs libcrc32c xor zlib_deflate raid6_pq mmc_block
> ohci_platform ehci_platform ohci_hcd sunxi phy_generic ehci_hcd
> musb_hdrc i2c_mv64xxx udc_core usbcore phy_sun4i_usb sunxi_mmc mmc_core
> sun6i_dma axp20x_regulator axp20x_pek axp20x_rsb sunxi_rsb axp20x sg
> dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua
> [ 5343.884200] CPU: 1 PID: 4379 Comm: (d-logind) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5343.965599] CPU: 3 PID: 4380 Comm: (md-udevd) Tainted: G           O
> L    4.20.0-1.gba5c149-default #1 openSUSE Tumbleweed (unreleased)
> [ 5344.046990] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5344.059138] Hardware name: sunxi sunxi/sunxi, BIOS 2019.01-rc1 12/11/2018
> [ 5344.071286] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5344.078059] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [ 5344.084834] pc : smp_call_function_many+0x31c/0x370
> [ 5344.089614] pc : smp_call_function_many+0x31c/0x370
> [ 5344.094393] lr : smp_call_function_many+0x2d8/0x370
> [ 5344.099260] lr : smp_call_function_many+0x2d8/0x370
> [ 5344.104124] sp : ffff00000f11bc10
> [ 5344.108990] sp : ffff00000f123c10
> [ 5344.113858] x29: ffff00000f11bc10 x28: 0000000000000180
> [ 5344.117164] x29: ffff00000f123c10 x28: 0000000000000180
> [ 5344.120471] x27: ffff0000092cb380 x26: ffff80007ff8f3f8
> [ 5344.125771] x27: ffff0000092cb380 x26: ffff80007ffc13f8
> [ 5344.131071] x25: 0000000000000000 x24: ffff000008188608
> [ 5344.136371] x25: 0000000000000000 x24: ffff000008188608
> [ 5344.141670] x23: 0000000000000001 x22: ffff00000967a614
> [ 5344.146970] x23: 0000000000000001 x22: ffff00000967a614
> [ 5344.152270] x21: ffff000009679738 x20: ffff80007ff8f3c8
> [ 5344.157569] x21: ffff000009679738 x20: ffff80007ffc13c8
> [ 5344.162869] x19: ffff80007ff8f3c0 x18: 0000000000000000
> [ 5344.168169] x19: ffff80007ffc13c0 x18: ffff80007ffbdb80
> [ 5344.173469] x17: 0000000000000000 x16: 0000000000000000
> [ 5344.178768] x17: 0000000000000000 x16: ffff80007ffb1fe0
> [ 5344.184068] x15: 0000000000000000 x14: 0005006100000006
> [ 5344.189368] x15: 000000008287d0ce x14: 0000000026c4ee0b
> [ 5344.194668] x13: 0000000100010035 x12: 0140000000000000
> [ 5344.199967] x13: 000000003dddfe6f x12: 0000000021da966e
> [ 5344.205267] x11: 0040000000000001 x10: ffff0000017d2000
> [ 5344.210566] x11: 00000000c5ebf508 x10: 000000009b8ba2f7
> [ 5344.215866] x9 : 0000000000000000 x8 : 0000000000000006
> [ 5344.221165] x9 : 00000000644f2575 x8 : 0000000000000006
> [ 5344.226466] x7 : 0000000000000000 x6 : ffff80007ffcafe0
> [ 5344.231765] x7 : 0000000000000000 x6 : ffff7dffbffbfc40
> [ 5344.237065] x5 : ffff80007ffcafe0 x4 : 000000000000000d
> [ 5344.242365] x5 : ffff7dffbffbfc40 x4 : 0000000000000007
> [ 5344.247664] x3 : 0000000000000000 x2 : ffff80007ff7fff8
> [ 5344.252964] x3 : 0000000000000000 x2 : ffff7dffbff8dc58
> [ 5344.258265] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5344.263564] x1 : 0000000000000003 x0 : 0000000000000000
> [ 5344.268863] Call trace:
> [ 5344.274164] Call trace:
> [ 5344.279467]  smp_call_function_many+0x31c/0x370
> [ 5344.281907]  smp_call_function_many+0x31c/0x370
> [ 5344.284346]  kick_all_cpus_sync+0x30/0x38
> [ 5344.288865]  kick_all_cpus_sync+0x30/0x38
> [ 5344.293386]  bpf_int_jit_compile+0x14c/0x420
> [ 5344.297385]  bpf_int_jit_compile+0x14c/0x420
> [ 5344.301384]  bpf_prog_select_runtime+0xec/0x138
> [ 5344.305644]  bpf_prog_select_runtime+0xec/0x138
> [ 5344.309905]  bpf_prepare_filter+0x468/0x520
> [ 5344.314426]  bpf_prepare_filter+0x468/0x520
> [ 5344.318946]  bpf_prog_create_from_user+0xe0/0x188
> [ 5344.323118]  bpf_prog_create_from_user+0xe0/0x188
> [ 5344.327291]  do_seccomp+0x2a0/0x6a0
> [ 5344.331985]  do_seccomp+0x2a0/0x6a0
> [ 5344.336678]  __arm64_sys_seccomp+0x28/0x38
> [ 5344.340158]  __arm64_sys_seccomp+0x28/0x38
> [ 5344.343638]  el0_svc_common+0x98/0x100
> [ 5344.347725]  el0_svc_common+0x98/0x100
> [ 5344.351812]  el0_svc_handler+0x38/0x78
> [ 5344.355551]  el0_svc_handler+0x38/0x78
> [ 5344.359290]  el0_svc+0x8/0xc
> [ 5344.363030]  el0_svc+0x8/0xc
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH lora-next] net: lora: sx125x: Add error handling for clock-output-names
  2018-12-30 10:55       ` Andreas Färber
@ 2018-12-30 19:40         ` Andreas Färber
  2018-12-30 19:44         ` [PATCH lora-next] net: lora: sx1301: Fix clk32m handling Andreas Färber
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Andreas Färber @ 2018-12-30 19:40 UTC (permalink / raw)
  To: linux-lpwan
  Cc: netdev, linux-kernel, Ben Whitten, Ben Whitten, Andreas Färber

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

Split off from style changes.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/net/lora/sx125x.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index b7ca782b9386..2c48301a12ab 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -149,8 +149,12 @@ static int sx125x_register_clock_provider(struct sx125x_priv *priv)
 	init.num_parents = 1;
 	priv->clkout_hw.init = &init;
 
-	of_property_read_string_index(dev->of_node, "clock-output-names", 0,
-			&init.name);
+	ret = of_property_read_string_index(dev->of_node, "clock-output-names", 0,
+					    &init.name);
+	if (ret) {
+		dev_err(dev, "unable to find output name\n");
+		return ret;
+	}
 
 	priv->clkout = devm_clk_register(dev, &priv->clkout_hw);
 	if (IS_ERR(priv->clkout)) {
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH lora-next] net: lora: sx1301: Fix clk32m handling
  2018-12-30 10:55       ` Andreas Färber
  2018-12-30 19:40         ` [PATCH lora-next] net: lora: sx125x: Add error handling for clock-output-names Andreas Färber
@ 2018-12-30 19:44         ` Andreas Färber
  2018-12-30 19:47         ` [PATCH lora-next] net: lora: sx125x: Clean up clock provider Andreas Färber
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Andreas Färber @ 2018-12-30 19:44 UTC (permalink / raw)
  To: linux-lpwan
  Cc: netdev, linux-kernel, Ben Whitten, Ben Whitten, Andreas Färber

We can't get the clk32m during probe because the radio is not yet probed then.
When doing it during netdev open we also need to undo that during netdev stop.
Revamp the error handling for open and tidy our debug output while at it.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/net/lora/sx1301.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 23cbddc364e5..533fd37c350d 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -379,58 +379,73 @@ static int sx130x_loradev_open(struct net_device *netdev)
 		return -ENXIO;
 	}
 
-	priv->clk32m = devm_clk_get(priv->dev, "clk32m");
+	priv->clk32m = clk_get(priv->dev, "clk32m");
 	if (IS_ERR(priv->clk32m)) {
-		dev_err(priv->dev, "failed to get clk32m\n");
+		dev_err(priv->dev, "failed to get clk32m (%ld)\n", PTR_ERR(priv->clk32m));
 		return PTR_ERR(priv->clk32m);
 	}
 
 	ret = clk_prepare_enable(priv->clk32m);
 	if (ret) {
-		dev_err(priv->dev, "failed to enable clk32m: %d\n", ret);
-		return ret;
+		dev_err(priv->dev, "failed to enable clk32m (%d)\n", ret);
+		goto err_clk_enable;
 	}
 
 	ret = sx1301_field_write(priv, F_GLOBAL_EN, 1);
 	if (ret) {
-		dev_err(priv->dev, "enable global clocks failed\n");
-		return ret;
+		dev_err(priv->dev, "enable global clocks failed (%d)\n", ret);
+		goto err_reg;
 	}
 
 	ret = sx1301_field_write(priv, F_CLK32M_EN, 1);
 	if (ret) {
-		dev_err(priv->dev, "enable 32M clock failed\n");
-		return ret;
+		dev_err(priv->dev, "enable 32M clock failed (%d)\n", ret);
+		goto err_reg;
 	}
 
 	/* calibration */
 
 	ret = sx1301_agc_calibrate(priv);
 	if (ret)
-		return ret;
+		goto err_calibrate;
 
 	/* TODO */
 
 	ret = sx1301_load_all_firmware(priv);
 	if (ret)
-		return ret;
+		goto err_firmware;
 
 	ret = open_loradev(netdev);
 	if (ret)
-		return ret;
+		goto err_open;
 
 	netif_start_queue(netdev);
 
 	return 0;
+
+err_open:
+err_firmware:
+err_calibrate:
+err_reg:
+	clk_disable_unprepare(priv->clk32m);
+err_clk_enable:
+	clk_put(priv->clk32m);
+	return ret;
 }
 
 static int sx130x_loradev_stop(struct net_device *netdev)
 {
+	struct sx1301_priv *priv = netdev_priv(netdev);
+
 	netdev_dbg(netdev, "%s", __func__);
 
 	netif_stop_queue(netdev);
 	close_loradev(netdev);
 
+	clk_disable_unprepare(priv->clk32m);
+	clk_put(priv->clk32m);
+	priv->clk32m = NULL;
+
 	return 0;
 }
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH lora-next] net: lora: sx125x: Clean up clock provider
  2018-12-30 10:55       ` Andreas Färber
  2018-12-30 19:40         ` [PATCH lora-next] net: lora: sx125x: Add error handling for clock-output-names Andreas Färber
  2018-12-30 19:44         ` [PATCH lora-next] net: lora: sx1301: Fix clk32m handling Andreas Färber
@ 2018-12-30 19:47         ` Andreas Färber
  2018-12-31 13:27         ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Andreas Färber
  2018-12-31 17:50         ` Mark Brown
  4 siblings, 0 replies; 18+ messages in thread
From: Andreas Färber @ 2018-12-30 19:47 UTC (permalink / raw)
  To: linux-lpwan
  Cc: netdev, linux-kernel, Ben Whitten, Ben Whitten, linux-clk,
	Andreas Färber

Use devm_ variant to have the clock provider deleted again.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 drivers/net/lora/sx125x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index 2c48301a12ab..68d8afed48ba 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -161,7 +161,7 @@ static int sx125x_register_clock_provider(struct sx125x_priv *priv)
 		dev_err(dev, "failed to register clkout\n");
 		return PTR_ERR(priv->clkout);
 	}
-	ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
 			&priv->clkout_hw);
 	return ret;
 }
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-30 10:55       ` Andreas Färber
                           ` (2 preceding siblings ...)
  2018-12-30 19:47         ` [PATCH lora-next] net: lora: sx125x: Clean up clock provider Andreas Färber
@ 2018-12-31 13:27         ` Andreas Färber
  2018-12-31 17:50         ` Mark Brown
  4 siblings, 0 replies; 18+ messages in thread
From: Andreas Färber @ 2018-12-31 13:27 UTC (permalink / raw)
  To: Ben Whitten, linux-clk
  Cc: devicetree, Maxime Ripard, netdev, Michael Turquette,
	Stephen Boyd, linux-lpwan, linux-kernel, Mark Brown, linux-spi,
	David S. Miller, linux-arm-kernel, Russell King

Am 30.12.18 um 11:55 schrieb Andreas Färber:
> Am 29.12.18 um 21:16 schrieb Andreas Färber:
>> `sudo ip link set lora2 up` froze my system, with both
>> lora1 and lora2 being sx1301. [...]
>>
>> Investigating...
> 
> I've bisected and confirmed that it was indeed this patch that regresses
> for one of my SX1301 concentrators.
[...]
> We never return from the sx125x_clkout_enable() performing the
> regmap_field_write() on our regmap_bus, which in turn uses a SPI regmap
> in sx1301_regmap_bus_read().
> 
> A notable difference between my two concentrators is that the working
> one is using spi-gpio driver, the regressing one spi-sun6i.
> 
> Two things stood out in spi-sun6i: It uses a completion (I do not run
> into its timeout warning!), and it uses clk_{get,set}_rate().
> 
> Given that observed symptoms were CPU stalls, workqueue [freezes] and RCU
> problems, requiring a power-cycle to recover, I wonder whether we are
> running into some atomic/locking issue with clk_enable()? Is it valid at
> all to use SPI/regmap for clk_enable()? If it is, is there a known issue
> specific to spi-sun6i (A64) in 4.20.0?

I've now hacked together a test case: delaying the regmap operation to a
work queue (violating the .enable contract of a stable clk on return!)
and having our caller poll afterwards for the operation to finish. Guess
what, below gross hack makes it work again on both cards... :/

Is this hinting at an issue with spi-sun6i clk_get_rate()'s prepare_lock
vs. our clk_enable()'s enable_lock? I grep'ed around and spi-sun6i is
not the only SPI driver using clk_get_rate() in transfer_one...

Thanks for any hints,

Andreas


diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
index 597b882379ac..095ca40e5de7 100644
--- a/drivers/net/lora/sx125x.c
+++ b/drivers/net/lora/sx125x.c
@@ -11,6 +11,7 @@

 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -49,6 +50,9 @@ struct sx125x_priv {
        struct device           *dev;
        struct regmap           *regmap;
        struct regmap_field
*regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
+
+       struct workqueue_struct *clk_wq;
+       struct work_struct      clk_out_enable_work;
 };

 #define to_clkout(_hw) container_of(_hw, struct sx125x_priv, clkout_hw)
@@ -81,8 +85,17 @@ static int sx125x_clkout_enable(struct clk_hw *hw)
 {
        struct sx125x_priv *priv = to_clkout(hw);

+       dev_info(priv->dev, "enabling clkout...\n");
+       queue_work(priv->clk_wq, &priv->clk_out_enable_work);
+       return 0;
+}
+
+static void sx125x_clk_out_enable_work_handler(struct work_struct *ws)
+{
+       struct sx125x_priv *priv = container_of(ws, struct sx125x_priv,
clk_out_enable_work);
+
        dev_info(priv->dev, "enabling clkout\n");
-       return sx125x_field_write(priv, F_CLK_OUT, 1);
+       sx125x_field_write(priv, F_CLK_OUT, 1);
 }

 static void sx125x_clkout_disable(struct clk_hw *hw)
@@ -230,6 +243,9 @@ static int __maybe_unused sx125x_regmap_probe(struct
device *dev, struct regmap
                }
        }

+       priv->clk_wq = alloc_workqueue("sx127x_wq", WQ_FREEZABLE |
WQ_MEM_RECLAIM, 0);
+       INIT_WORK(&priv->clk_out_enable_work,
sx125x_clk_out_enable_work_handler);
+
        dev_info(dev, "SX125x module probed\n");

        return 0;
@@ -237,6 +253,10 @@ static int __maybe_unused
sx125x_regmap_probe(struct device *dev, struct regmap

 static int __maybe_unused sx125x_regmap_remove(struct device *dev)
 {
+       struct sx125x_priv *priv = dev_get_drvdata(dev);
+
+       destroy_workqueue(priv->clk_wq);
+
        dev_info(dev, "SX125x module removed\n");

        return 0;
diff --git a/drivers/net/lora/sx130x.c b/drivers/net/lora/sx130x.c
index 7ac7de9eda46..4ae6699d38ad 100644
--- a/drivers/net/lora/sx130x.c
+++ b/drivers/net/lora/sx130x.c
@@ -11,6 +11,7 @@

 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/lora.h>
@@ -391,6 +392,10 @@ static int sx130x_loradev_open(struct net_device
*netdev)
                goto err_clk_enable;
        }

+       do {
+               usleep_range(100, 1000);
+       } while (!__clk_is_enabled(priv->clk32m));
+
        ret = sx130x_field_write(priv, F_GLOBAL_EN, 1);
        if (ret) {
                dev_err(priv->dev, "enable global clocks failed (%d)\n",
ret);


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-30 10:55       ` Andreas Färber
                           ` (3 preceding siblings ...)
  2018-12-31 13:27         ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Andreas Färber
@ 2018-12-31 17:50         ` Mark Brown
  2018-12-31 22:56           ` Andreas Färber
  4 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2018-12-31 17:50 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Ben Whitten, devicetree, David S. Miller, netdev,
	Michael Turquette, Stephen Boyd, linux-lpwan, linux-kernel,
	starnight, linux-clk, linux-spi, Maxime Ripard, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

On Sun, Dec 30, 2018 at 11:55:46AM +0100, Andreas Färber wrote:
> + linux-spi, LAKML

> Given that observed symptoms were CPU stalls, workqueue hangs and RCU
> problems, requiring a power-cycle to recover, I wonder whether we are
> running into some atomic/locking issue with clk_enable()? Is it valid at
> all to use SPI/regmap for clk_enable()? If it is, is there a known issue
> specific to spi-sun6i (A64) in 4.20.0?
> I already tried setting .disable_locking = true in both regmap_configs.
> Any suggestions how to further debug?

You can't use SPI for clk_enable(), clk_enable() needs to be doable in
atomic context since we need to wait for the bus operations to complete
(you can start SPI transfers in atomic context but you still need to
wait for them to complete).  Any clocks that are only accessible via a
slow bus like I2C or SPI need to do the enable/disable in the
prepare/unprepare operations which aren't done in atomic context.

regmap can be used in atomic contexts, though you need to configure it
to use spinlocks instead of mutexes and ensure that no register cache
allocations happen during I/O (eg, by providing defaults for all
registers or by not using a cache).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-31 17:50         ` Mark Brown
@ 2018-12-31 22:56           ` Andreas Färber
  2019-01-02  0:44             ` Andreas Färber
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2018-12-31 22:56 UTC (permalink / raw)
  To: Mark Brown
  Cc: Ben Whitten, devicetree, David S. Miller, netdev,
	Michael Turquette, Stephen Boyd, linux-lpwan, linux-kernel,
	starnight, linux-clk, linux-spi, Maxime Ripard, linux-arm-kernel,
	Russell King

Am 31.12.18 um 18:50 schrieb Mark Brown:
> On Sun, Dec 30, 2018 at 11:55:46AM +0100, Andreas Färber wrote:
>> Given that observed symptoms were CPU stalls, workqueue hangs and RCU
>> problems, requiring a power-cycle to recover, I wonder whether we are
>> running into some atomic/locking issue with clk_enable()? Is it valid at
>> all to use SPI/regmap for clk_enable()? If it is, is there a known issue
>> specific to spi-sun6i (A64) in 4.20.0?
>> I already tried setting .disable_locking = true in both regmap_configs.
>> Any suggestions how to further debug?
> 
> You can't use SPI for clk_enable(), clk_enable() needs to be doable in
> atomic context since we need to wait for the bus operations to complete
> (you can start SPI transfers in atomic context but you still need to
> wait for them to complete).  Any clocks that are only accessible via a
> slow bus like I2C or SPI need to do the enable/disable in the
> prepare/unprepare operations which aren't done in atomic context.
> 
> regmap can be used in atomic contexts, though you need to configure it
> to use spinlocks instead of mutexes and ensure that no register cache
> allocations happen during I/O (eg, by providing defaults for all
> registers or by not using a cache).

We have .cache_type = REGCACHE_NONE on both bus and spi regmap_configs.

I moved the regmap_field_write() from .enable to .prepare and set
.fast_io = true on both regmap_configs to force using spinlocks, but
same hang as in .enable before...

And same if I set .disable_locking = true on both.

Given that it works with one SPI driver and not with the other,
independent of the locking options applied, I assume my symptoms are not
a regmap-layer issue.

Is it allowed during a .prepare operation to call the mentioned
clk_get_rate(), which ends up calling clk_prepare_lock()?

According to my debug output in spi-sun6i.c our hanging
regmap_field_write() ends up calling sun6i_transfer_one() three times,
the first two look okay, but the third one doesn't make it past the
clk_get_rate() or following if block. [But for now some fireworks...]

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2018-12-31 22:56           ` Andreas Färber
@ 2019-01-02  0:44             ` Andreas Färber
  2019-01-03 12:37               ` Mark Brown
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2019-01-02  0:44 UTC (permalink / raw)
  To: Mark Brown, linux-spi
  Cc: Ben Whitten, devicetree, linux-clk, Maxime Ripard, netdev,
	Michael Turquette, Stephen Boyd, linux-lpwan, linux-kernel,
	Russell King, starnight, David S. Miller, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 3713 bytes --]

Am 31.12.18 um 23:56 schrieb Andreas Färber:
> Am 31.12.18 um 18:50 schrieb Mark Brown:
>> On Sun, Dec 30, 2018 at 11:55:46AM +0100, Andreas Färber wrote:
>>> Given that observed symptoms were CPU stalls, workqueue hangs and RCU
>>> problems, requiring a power-cycle to recover, I wonder whether we are
>>> running into some atomic/locking issue with clk_enable()? Is it valid at
>>> all to use SPI/regmap for clk_enable()? If it is, is there a known issue
>>> specific to spi-sun6i (A64) in 4.20.0?
>>> I already tried setting .disable_locking = true in both regmap_configs.
>>> Any suggestions how to further debug?
>>
>> You can't use SPI for clk_enable(), clk_enable() needs to be doable in
>> atomic context since we need to wait for the bus operations to complete
>> (you can start SPI transfers in atomic context but you still need to
>> wait for them to complete).  Any clocks that are only accessible via a
>> slow bus like I2C or SPI need to do the enable/disable in the
>> prepare/unprepare operations which aren't done in atomic context.
>>
>> regmap can be used in atomic contexts, though you need to configure it
>> to use spinlocks instead of mutexes and ensure that no register cache
>> allocations happen during I/O (eg, by providing defaults for all
>> registers or by not using a cache).
> 
> We have .cache_type = REGCACHE_NONE on both bus and spi regmap_configs.
> 
> I moved the regmap_field_write() from .enable to .prepare and set
> .fast_io = true on both regmap_configs to force using spinlocks, but
> same hang as in .enable before...
> 
> And same if I set .disable_locking = true on both.
> 
> Given that it works with one SPI driver and not with the other,
> independent of the locking options applied, I assume my symptoms are not
> a regmap-layer issue.
> 
> Is it allowed during a .prepare operation to call the mentioned
> clk_get_rate(), which ends up calling clk_prepare_lock()?
> 
> According to my debug output in spi-sun6i.c our hanging
> regmap_field_write() ends up calling sun6i_transfer_one() three times,
> the first two look okay, but the third one doesn't make it past the
> clk_get_rate() [...].

SysRq still works in that state! Attached is SysRq-w output.
(still with .disable_locking = true in both regmap_configs)

In the very bottom you see the "ip" task, at wait_for_completion() from
__spi_sync().
I trigger this issue with `ip link set lora2 up`, so that looks okay.

Then there's a "spi1" task at clk_prepare_lock()' mutex_lock() coming
from spi_pump_messages().
The reason for that will be that clk_prepare_lock()'s mutex_trylock()
failed (because we're holding the prepare_lock from clk_prepare_enable()
in the "ip" task) and that the prepare_owner == current check fails for
this separate task_struct, too.

So, the third invocation of sun6i_transfer_one() calling clk_get_rate()
hangs at the prepare_lock instead of reference-counting, because it runs
from a separate kthread, unlike the two previous calls?

Besides, there's also an mmc_rescan workqueue task at clk_prepare_lock()
coming from sunxi_mmc_enable() due to pm_generic_runtime_resume().
My rootfs is on microSD card.

I did not find any *regmap_init_spi() based example in drivers/clk/, and
all other "spi" mentions in drivers/clk/ appeared to be clock names.
The closest was devm_regmap_init_i2c() based clk-cdce706.c, which uses
the prepare/unprepare ops, as suggested by Mark, and does
regmap_update_bits() from there.

A quick grep in drivers/i2c/ does not find any mention of "kthread", so
probably that's the breaking difference?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

[-- Attachment #2: pinie-sysrq-w.txt --]
[-- Type: text/plain, Size: 12645 bytes --]

[39292.911444] sysrq: SysRq : Show Blocked State
[39292.915819]   task                        PC stack   pid father
[39292.921784] kworker/0:2     D    0   170      2 0x00000028
[39292.927282] Workqueue: ipv6_addrconf addrconf_verify_work
[39292.932676] Call trace:
[39292.935127]  __switch_to+0x9c/0xd8
[39292.938530]  __schedule+0x2a0/0x888
[39292.942016]  schedule+0x30/0x88
[39292.945157]  schedule_preempt_disabled+0x14/0x20
[39292.949770]  __mutex_lock.isra.1+0x2c0/0x4b8
[39292.954036]  __mutex_lock_slowpath+0x24/0x30
[39292.958302]  mutex_lock+0x48/0x50
[39292.961615]  rtnl_lock+0x1c/0x28
[39292.964841]  addrconf_verify_work+0x14/0x28
[39292.969022]  process_one_work+0x1f4/0x428
[39292.973028]  worker_thread+0x44/0x4a8
[39292.976686]  kthread+0x130/0x138
[39292.979912]  ret_from_fork+0x10/0x18
[39292.983518] btrfs-transacti D    0   383      2 0x00000028
[39292.988998] Call trace:
[39292.991444]  __switch_to+0x9c/0xd8
[39292.994845]  __schedule+0x2a0/0x888
[39292.998330]  schedule+0x30/0x88
[39293.001472]  io_schedule+0x20/0x40
[39293.004874]  wbt_wait+0x1bc/0x2f8
[39293.008188]  rq_qos_throttle+0x4c/0x68
[39293.011936]  blk_mq_make_request+0xc4/0x4f0
[39293.016118]  generic_make_request+0xf4/0x308
[39293.020384]  submit_bio+0x40/0x198
[39293.023977]  btrfs_map_bio+0x370/0x3f8 [btrfs]
[39293.028550]  btrfs_submit_bio_hook+0xa4/0x1c0 [btrfs]
[39293.033727]  submit_one_bio+0x78/0xb8 [btrfs]
[39293.038211]  flush_write_bio.isra.12+0x2c/0x48 [btrfs]
[39293.043477]  extent_writepages+0x60/0x70 [btrfs]
[39293.048217]  btrfs_writepages+0x28/0x38 [btrfs]
[39293.052747]  do_writepages+0x3c/0xe0
[39293.056323]  __filemap_fdatawrite_range+0xc0/0x118
[39293.061109]  filemap_fdatawrite_range+0x38/0x48
[39293.065764]  btrfs_fdatawrite_range+0x34/0x78 [btrfs]
[39293.070942]  __btrfs_write_out_cache+0x400/0x450 [btrfs]
[39293.076382]  btrfs_write_out_cache+0xc0/0x160 [btrfs]
[39293.081551]  btrfs_start_dirty_block_groups+0x2dc/0x4d0 [btrfs]
[39293.087591]  btrfs_commit_transaction+0x554/0x940 [btrfs]
[39293.093111]  transaction_kthread+0x18c/0x1a8 [btrfs]
[39293.098072]  kthread+0x130/0x138
[39293.101298]  ret_from_fork+0x10/0x18
[39293.104877] systemd-journal D    0   448      1 0x00000801
[39293.110358] Call trace:
[39293.112803]  __switch_to+0x9c/0xd8
[39293.116204]  __schedule+0x2a0/0x888
[39293.119690]  schedule+0x30/0x88
[39293.122830]  io_schedule+0x20/0x40
[39293.126230]  wait_on_page_bit+0x134/0x240
[39293.130366]  btrfs_page_mkwrite+0x1e0/0x450 [btrfs]
[39293.135240]  do_page_mkwrite+0x40/0xb0
[39293.138985]  do_wp_page+0x3d0/0x578
[39293.142471]  __handle_mm_fault+0x484/0x528
[39293.146563]  handle_mm_fault+0x10c/0x1e8
[39293.150484]  do_page_fault+0x174/0x480
[39293.154229]  do_mem_abort+0x50/0xa8
[39293.157713]  el0_da+0x20/0x24
[39293.160676] journal-offline D    0 23928      1 0x00000801
[39293.166156] Call trace:
[39293.168601]  __switch_to+0x9c/0xd8
[39293.172002]  __schedule+0x2a0/0x888
[39293.175487]  schedule+0x30/0x88
[39293.178626]  io_schedule+0x20/0x40
[39293.182025]  wbt_wait+0x1bc/0x2f8
[39293.185338]  rq_qos_throttle+0x4c/0x68
[39293.189085]  blk_mq_make_request+0xc4/0x4f0
[39293.193266]  generic_make_request+0xf4/0x308
[39293.197533]  submit_bio+0x40/0x198
[39293.201064]  btrfs_map_bio+0x370/0x3f8 [btrfs]
[39293.205632]  btrfs_submit_bio_hook+0xa4/0x1c0 [btrfs]
[39293.210808]  submit_one_bio+0x78/0xb8 [btrfs]
[39293.215292]  submit_extent_page+0xcc/0x218 [btrfs]
[39293.220209]  __extent_writepage_io+0x230/0x3e8 [btrfs]
[39293.225472]  __extent_writepage+0x114/0x2e0 [btrfs]
[39293.230476]  extent_write_cache_pages+0xec/0x328 [btrfs]
[39293.235912]  extent_writepages+0x54/0x70 [btrfs]
[39293.240651]  btrfs_writepages+0x28/0x38 [btrfs]
[39293.245179]  do_writepages+0x3c/0xe0
[39293.248751]  __filemap_fdatawrite_range+0xc0/0x118
[39293.253538]  filemap_fdatawrite_range+0x38/0x48
[39293.258194]  btrfs_fdatawrite_range+0x34/0x78 [btrfs]
[39293.263368]  start_ordered_ops+0x4c/0x80 [btrfs]
[39293.268110]  btrfs_sync_file+0x88/0x3e0 [btrfs]
[39293.272640]  vfs_fsync_range+0x4c/0x88
[39293.276385]  do_fsync+0x48/0x88
[39293.279525]  __arm64_sys_fsync+0x24/0x38
[39293.283446]  el0_svc_common+0x98/0x100
[39293.287192]  el0_svc_handler+0x38/0x78
[39293.290936]  el0_svc+0x8/0xc
[39293.293829] wickedd         D    0   713      1 0x00000000
[39293.299309] Call trace:
[39293.301755]  __switch_to+0x9c/0xd8
[39293.305155]  __schedule+0x2a0/0x888
[39293.308641]  schedule+0x30/0x88
[39293.311781]  schedule_preempt_disabled+0x14/0x20
[39293.316394]  __mutex_lock.isra.1+0x2c0/0x4b8
[39293.320661]  __mutex_lock_slowpath+0x24/0x30
[39293.324927]  mutex_lock+0x48/0x50
[39293.328243]  __netlink_dump_start+0x7c/0x1d8
[39293.332511]  rtnetlink_rcv_msg+0x280/0x2c0
[39293.336604]  netlink_rcv_skb+0x60/0x120
[39293.340437]  rtnetlink_rcv+0x28/0x38
[39293.344010]  netlink_unicast+0x1a8/0x280
[39293.347928]  netlink_sendmsg+0x188/0x338
[39293.351848]  sock_sendmsg+0x4c/0x68
[39293.355333]  ___sys_sendmsg+0x26c/0x2a0
[39293.359165]  __sys_sendmsg+0x64/0xa0
[39293.362736]  __arm64_sys_sendmsg+0x2c/0x38
[39293.366829]  el0_svc_common+0x98/0x100
[39293.370575]  el0_svc_handler+0x38/0x78
[39293.374320]  el0_svc+0x8/0xc
[39293.377201] ntpd            D    0  1354      1 0x00000001
[39293.382681] Call trace:
[39293.385126]  __switch_to+0x9c/0xd8
[39293.388526]  __schedule+0x2a0/0x888
[39293.392012]  schedule+0x30/0x88
[39293.395151]  schedule_preempt_disabled+0x14/0x20
[39293.399764]  __mutex_lock.isra.1+0x2c0/0x4b8
[39293.404029]  __mutex_lock_slowpath+0x24/0x30
[39293.408296]  mutex_lock+0x48/0x50
[39293.411610]  __netlink_dump_start+0x7c/0x1d8
[39293.415876]  rtnetlink_rcv_msg+0x280/0x2c0
[39293.419969]  netlink_rcv_skb+0x60/0x120
[39293.423801]  rtnetlink_rcv+0x28/0x38
[39293.427374]  netlink_unicast+0x1a8/0x280
[39293.431294]  netlink_sendmsg+0x188/0x338
[39293.435211]  sock_sendmsg+0x4c/0x68
[39293.438694]  __sys_sendto+0xe8/0x150
[39293.442266]  __arm64_sys_sendto+0x30/0x40
[39293.446272]  el0_svc_common+0x98/0x100
[39293.450018]  el0_svc_handler+0x38/0x78
[39293.453762]  el0_svc+0x8/0xc
[39293.456670] kworker/2:1     D    0 23762      2 0x00000028
[39293.462197] Workqueue: events_freezable mmc_rescan [mmc_core]
[39293.467936] Call trace:
[39293.470381]  __switch_to+0x9c/0xd8
[39293.473781]  __schedule+0x2a0/0x888
[39293.477267]  schedule+0x30/0x88
[39293.480406]  schedule_preempt_disabled+0x14/0x20
[39293.485019]  __mutex_lock.isra.1+0x2c0/0x4b8
[39293.489285]  __mutex_lock_slowpath+0x24/0x30
[39293.493551]  mutex_lock+0x48/0x50
[39293.496865]  clk_prepare_lock+0x48/0xa0
[39293.500699]  clk_prepare+0x24/0x58
[39293.504105]  sunxi_mmc_enable+0x40/0x220 [sunxi_mmc]
[39293.509067]  sunxi_mmc_runtime_resume+0x2c/0x70 [sunxi_mmc]
[39293.514637]  pm_generic_runtime_resume+0x3c/0x58
[39293.519251]  __rpm_callback+0x118/0x208
[39293.523082]  rpm_callback+0x70/0x98
[39293.526567]  rpm_resume+0x65c/0x840
[39293.530052]  __pm_runtime_resume+0x68/0xc8
[39293.534182]  __mmc_claim_host+0x200/0x260 [mmc_core]
[39293.539178]  mmc_get_card+0x38/0x48 [mmc_core]
[39293.543656]  mmc_sd_detect+0x24/0x90 [mmc_core]
[39293.548219]  mmc_rescan+0x3d0/0x540 [mmc_core]
[39293.552659]  process_one_work+0x1f4/0x428
[39293.556665]  worker_thread+0x44/0x4a8
[39293.560323]  kthread+0x130/0x138
[39293.563549]  ret_from_fork+0x10/0x18
[39293.567122] kworker/u8:5    D    0 23778      2 0x00000028
[39293.572608] Workqueue: writeback wb_workfn (flush-btrfs-1)
[39293.578087] Call trace:
[39293.580533]  __switch_to+0x9c/0xd8
[39293.583933]  __schedule+0x2a0/0x888
[39293.587419]  schedule+0x30/0x88
[39293.590557]  io_schedule+0x20/0x40
[39293.593956]  wbt_wait+0x1bc/0x2f8
[39293.597269]  rq_qos_throttle+0x4c/0x68
[39293.601016]  blk_mq_make_request+0xc4/0x4f0
[39293.605196]  generic_make_request+0xf4/0x308
[39293.609461]  submit_bio+0x40/0x198
[39293.612995]  btrfs_map_bio+0x370/0x3f8 [btrfs]
[39293.617563]  btrfs_submit_bio_hook+0xa4/0x1c0 [btrfs]
[39293.622743]  submit_one_bio+0x78/0xb8 [btrfs]
[39293.627227]  submit_extent_page+0xcc/0x218 [btrfs]
[39293.632142]  __extent_writepage_io+0x230/0x3e8 [btrfs]
[39293.637405]  __extent_writepage+0x114/0x2e0 [btrfs]
[39293.642411]  extent_write_cache_pages+0xec/0x328 [btrfs]
[39293.647849]  extent_writepages+0x54/0x70 [btrfs]
[39293.652590]  btrfs_writepages+0x28/0x38 [btrfs]
[39293.657118]  do_writepages+0x3c/0xe0
[39293.660691]  __writeback_single_inode+0x48/0x438
[39293.665304]  writeback_sb_inodes+0x1c8/0x478
[39293.669570]  __writeback_inodes_wb+0x78/0xc8
[39293.673835]  wb_writeback+0x264/0x388
[39293.677494]  wb_workfn+0x304/0x4a8
[39293.680893]  process_one_work+0x1f4/0x428
[39293.684898]  worker_thread+0x44/0x4a8
[39293.688555]  kthread+0x130/0x138
[39293.691781]  ret_from_fork+0x10/0x18
[39293.695354] spi1            D    0 23857      2 0x00000028
[39293.700834] Call trace:
[39293.703280]  __switch_to+0x9c/0xd8
[39293.706680]  __schedule+0x2a0/0x888
[39293.710165]  schedule+0x30/0x88
[39293.713305]  schedule_preempt_disabled+0x14/0x20
[39293.717918]  __mutex_lock.isra.1+0x2c0/0x4b8
[39293.722183]  __mutex_lock_slowpath+0x24/0x30
[39293.726450]  mutex_lock+0x48/0x50
[39293.729763]  clk_prepare_lock+0x48/0xa0
[39293.733597]  clk_core_get_rate+0x1c/0x78
[39293.737515]  clk_get_rate+0x28/0x38
[39293.741006]  sun6i_spi_transfer_one+0x150/0x4a0 [spi_sun6i]
[39293.746575]  spi_transfer_one_message+0x150/0x648
[39293.751274]  __spi_pump_messages+0x424/0x6f0
[39293.755541]  spi_pump_messages+0x24/0x30
[39293.759460]  kthread_worker_fn+0xf0/0x1f8
[39293.763465]  kthread+0x130/0x138
[39293.766691]  ret_from_fork+0x10/0x18
[39293.770266] kworker/u8:8    D    0 23923      2 0x00000028
[39293.775882] Workqueue: btrfs-submit btrfs_submit_helper [btrfs]
[39293.781793] Call trace:
[39293.784240]  __switch_to+0x9c/0xd8
[39293.787640]  __schedule+0x2a0/0x888
[39293.791126]  schedule+0x30/0x88
[39293.794301]  __mmc_claim_host+0xa8/0x260 [mmc_core]
[39293.799210]  mmc_get_card+0x38/0x48 [mmc_core]
[39293.803658]  mmc_mq_queue_rq+0x220/0x308 [mmc_block]
[39293.808619]  __blk_mq_try_issue_directly+0x120/0x190
[39293.813580]  blk_mq_request_issue_directly+0x94/0xc0
[39293.818539]  blk_mq_try_issue_list_directly+0x40/0x98
[39293.823585]  blk_mq_sched_insert_requests+0x98/0xc0
[39293.828458]  blk_mq_flush_plug_list+0x184/0x298
[39293.832984]  blk_flush_plug_list+0xd8/0x250
[39293.837164]  blk_finish_plug+0x3c/0x4c
[39293.841042]  run_scheduled_bios+0x2e4/0x408 [btrfs]
[39293.846045]  pending_bios_fn+0x20/0x30 [btrfs]
[39293.850615]  normal_work_helper+0x200/0x470 [btrfs]
[39293.855619]  btrfs_submit_helper+0x20/0x30 [btrfs]
[39293.860406]  process_one_work+0x1f4/0x428
[39293.864411]  worker_thread+0x44/0x4a8
[39293.868068]  kthread+0x130/0x138
[39293.871294]  ret_from_fork+0x10/0x18
[39293.874866] ip              D    0 23925  23924 0x00000000
[39293.880346] Call trace:
[39293.882792]  __switch_to+0x9c/0xd8
[39293.886193]  __schedule+0x2a0/0x888
[39293.889679]  schedule+0x30/0x88
[39293.892817]  schedule_timeout+0x2c4/0x408
[39293.896823]  wait_for_common+0xdc/0x1a8
[39293.900656]  wait_for_completion+0x28/0x38
[39293.904750]  __spi_sync+0x148/0x388
[39293.908235]  spi_sync+0x34/0x58
[39293.911375]  regmap_spi_write+0x84/0x98
[39293.915210]  _regmap_raw_write_impl+0x794/0x8d8
[39293.919737]  _regmap_bus_raw_write+0x70/0x90
[39293.924003]  _regmap_write+0x70/0x128
[39293.927662]  regmap_write+0x50/0x78
[39293.931155]  sx130x_radio_read+0x94/0x270 [lora_sx130x]
[39293.936375]  _regmap_raw_read+0xd0/0x248
[39293.940295]  _regmap_bus_read+0x50/0x88
[39293.944127]  _regmap_read+0x6c/0x170
[39293.947700]  _regmap_update_bits+0xa4/0xf0
[39293.951793]  regmap_update_bits_base+0x68/0x98
[39293.956233]  regmap_field_update_bits_base+0x5c/0x70
[39293.961195]  sx125x_clkout_prepare+0x44/0x50 [lora_sx125x]
[39293.966674]  clk_core_prepare+0x64/0x1d8
[39293.970593]  clk_prepare+0x2c/0x58
[39293.973995]  sx130x_loradev_open+0x58/0x738 [lora_sx130x]
[39293.979388]  __dev_open+0xd8/0x178
[39293.982788]  __dev_change_flags+0x134/0x198
[39293.986968]  dev_change_flags+0x34/0x70
[39293.990800]  do_setlink+0x2a8/0xbb8
[39293.994287]  rtnl_newlink+0x408/0x768
[39293.997945]  rtnetlink_rcv_msg+0x20c/0x2c0
[39294.002038]  netlink_rcv_skb+0x60/0x120
[39294.005870]  rtnetlink_rcv+0x28/0x38
[39294.009443]  netlink_unicast+0x1a8/0x280
[39294.013363]  netlink_sendmsg+0x188/0x338
[39294.017281]  sock_sendmsg+0x4c/0x68
[39294.020766]  ___sys_sendmsg+0x26c/0x2a0
[39294.024598]  __sys_sendmsg+0x64/0xa0
[39294.028170]  __arm64_sys_sendmsg+0x2c/0x38
[39294.032263]  el0_svc_common+0x98/0x100
[39294.036009]  el0_svc_handler+0x38/0x78
[39294.039753]  el0_svc+0x8/0xc


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
  2019-01-02  0:44             ` Andreas Färber
@ 2019-01-03 12:37               ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2019-01-03 12:37 UTC (permalink / raw)
  To: Andreas Färber
  Cc: linux-spi, Ben Whitten, devicetree, linux-clk, Maxime Ripard,
	netdev, Michael Turquette, Stephen Boyd, linux-lpwan,
	linux-kernel, Russell King, starnight, David S. Miller,
	linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

On Wed, Jan 02, 2019 at 01:44:40AM +0100, Andreas Färber wrote:

> So, the third invocation of sun6i_transfer_one() calling clk_get_rate()
> hangs at the prepare_lock instead of reference-counting, because it runs
> from a separate kthread, unlike the two previous calls?

If there's any contestation for the bus we push all the I/O through a
separate thread to maintain ordering and improve performance.

> I did not find any *regmap_init_spi() based example in drivers/clk/, and
> all other "spi" mentions in drivers/clk/ appeared to be clock names.
> The closest was devm_regmap_init_i2c() based clk-cdce706.c, which uses
> the prepare/unprepare ops, as suggested by Mark, and does
> regmap_update_bits() from there.

You'll find a bunch of the MFDs have SPI options as well.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-01-03 12:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1539361567-3602-1-git-send-email-ben.whitten@lairdtech.com>
2018-10-12 16:26 ` [PATCH v3 lora-next 1/5] regmap: Add regmap_noinc_write API Ben Whitten
2018-10-18 16:59   ` Andreas Färber
2018-10-18 17:18     ` Mark Brown
2018-10-12 16:26 ` [PATCH v3 lora-next 2/5] net: lora: sx1301: replace burst spi functions with regmap_noinc Ben Whitten
2018-10-12 16:26 ` [PATCH v3 lora-next 3/5] net: lora: sx1301: convert to using regmap fields for bit ops Ben Whitten
2018-10-12 16:26 ` [PATCH v3 lora-next 4/5] net: lora: sx125x: convert to regmap fields Ben Whitten
2018-10-12 16:26 ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Ben Whitten
2018-12-29 19:25   ` Andreas Färber
2018-12-29 20:16     ` Andreas Färber
2018-12-30 10:55       ` Andreas Färber
2018-12-30 19:40         ` [PATCH lora-next] net: lora: sx125x: Add error handling for clock-output-names Andreas Färber
2018-12-30 19:44         ` [PATCH lora-next] net: lora: sx1301: Fix clk32m handling Andreas Färber
2018-12-30 19:47         ` [PATCH lora-next] net: lora: sx125x: Clean up clock provider Andreas Färber
2018-12-31 13:27         ` [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider Andreas Färber
2018-12-31 17:50         ` Mark Brown
2018-12-31 22:56           ` Andreas Färber
2019-01-02  0:44             ` Andreas Färber
2019-01-03 12:37               ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).