Re: [PATCH lora-next v2 7/8] net: lora: sx1301: add initial registration for regmap

2018-08-09 Thread Andreas Färber
Am 09.08.2018 um 14:33 schrieb Ben Whitten:
> The register and bit-field definitions are taken from the SX1301
> datasheet version 2.01 dated June 2014 with the revision information
> 'First released version'.
> 
> The reset state and RW capability of each field is not reflected in this
> patch however from the datasheet:
> "Bits and registers that are not documented are reserved. They may
> include calibration values. It is important not to modify these bits and
> registers. If specific bits must be changed in a register with reserved
> bits, the register must be read first, specific bits modified while
> masking reserved bits and then the register can be written."
> 
> Then goes on to state:
> "Reserved bits should be written with their reset state, they may be
> read different states."
> 
> Caching is currently disabled.
> 
> The version is read back using regmap_read to verify regmap operation,
> in doing so needs to be moved after priv and regmap allocation.
> 
> Further registers or fields are added as they are required in conversion.
> 
> Signed-off-by: Ben Whitten 
> ---
>  drivers/net/lora/Kconfig  |  1 +
>  drivers/net/lora/sx1301.c | 46 ++
>  drivers/net/lora/sx1301.h | 10 ++
>  3 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
> index bb57a01..79d23f2 100644
> --- a/drivers/net/lora/Kconfig
> +++ b/drivers/net/lora/Kconfig
> @@ -49,6 +49,7 @@ config LORA_SX1301
>   tristate "Semtech SX1301 SPI driver"
>   default y
>   depends on SPI
> + select REGMAP_SPI
>   help
> Semtech SX1301
>  
> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> index 8e81179..766df06 100644
> --- a/drivers/net/lora/sx1301.c
> +++ b/drivers/net/lora/sx1301.c
> @@ -20,11 +20,11 @@
>  #include 
>  #include 
>  #include 
> +#include 

Misordered.

>  
>  #include "sx1301.h"
>  
>  #define REG_PAGE_RESET   0
> -#define REG_VERSION  1
>  #define REG_MCU_PROM_ADDR9
>  #define REG_MCU_PROM_DATA10
>  #define REG_GPIO_SELECT_INPUT27
> @@ -68,6 +68,35 @@
>  
>  #define REG_EMERGENCY_FORCE_HOST_CTRLBIT(0)
>  
> +static const struct regmap_range_cfg sx1301_ranges[] = {

Let's rename to _regmap_ranges for consistency.

> @@ -81,6 +110,7 @@ struct sx1301_priv {
>   struct gpio_desc *rst_gpio;
>   u8 cur_page;
>   struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
> + struct regmap   *regmap;

Note: We need a consistent style. Either whitespace or tabs, not both
depending on author. Same in an earlier patch. Problem with tabs is that
at some point it's always one tab too little, but we can try it.

Applied.

Thanks,
Andreas

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


[PATCH lora-next v2 7/8] net: lora: sx1301: add initial registration for regmap

2018-08-09 Thread Ben Whitten
The register and bit-field definitions are taken from the SX1301
datasheet version 2.01 dated June 2014 with the revision information
'First released version'.

The reset state and RW capability of each field is not reflected in this
patch however from the datasheet:
"Bits and registers that are not documented are reserved. They may
include calibration values. It is important not to modify these bits and
registers. If specific bits must be changed in a register with reserved
bits, the register must be read first, specific bits modified while
masking reserved bits and then the register can be written."

Then goes on to state:
"Reserved bits should be written with their reset state, they may be
read different states."

Caching is currently disabled.

The version is read back using regmap_read to verify regmap operation,
in doing so needs to be moved after priv and regmap allocation.

Further registers or fields are added as they are required in conversion.

Signed-off-by: Ben Whitten 
---
 drivers/net/lora/Kconfig  |  1 +
 drivers/net/lora/sx1301.c | 46 ++
 drivers/net/lora/sx1301.h | 10 ++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
index bb57a01..79d23f2 100644
--- a/drivers/net/lora/Kconfig
+++ b/drivers/net/lora/Kconfig
@@ -49,6 +49,7 @@ config LORA_SX1301
tristate "Semtech SX1301 SPI driver"
default y
depends on SPI
+   select REGMAP_SPI
help
  Semtech SX1301
 
diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 8e81179..766df06 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -20,11 +20,11 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "sx1301.h"
 
 #define REG_PAGE_RESET 0
-#define REG_VERSION1
 #define REG_MCU_PROM_ADDR  9
 #define REG_MCU_PROM_DATA  10
 #define REG_GPIO_SELECT_INPUT  27
@@ -68,6 +68,35 @@
 
 #define REG_EMERGENCY_FORCE_HOST_CTRL  BIT(0)
 
+static const struct regmap_range_cfg sx1301_ranges[] = {
+   {
+   .name = "Pages",
+
+   .range_min = SX1301_VIRT_BASE,
+   .range_max = SX1301_MAX_REGISTER,
+
+   .selector_reg = SX1301_PAGE,
+   .selector_mask = 0x3,
+
+   .window_start = 0,
+   .window_len = SX1301_PAGE_LEN,
+   },
+};
+
+static struct regmap_config sx1301_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+
+   .cache_type = REGCACHE_NONE,
+
+   .read_flag_mask = 0,
+   .write_flag_mask = BIT(7),
+
+   .ranges = sx1301_ranges,
+   .num_ranges = ARRAY_SIZE(sx1301_ranges),
+   .max_register = SX1301_MAX_REGISTER,
+};
+
 struct spi_sx1301 {
struct spi_device *parent;
u8 page;
@@ -81,6 +110,7 @@ struct sx1301_priv {
struct gpio_desc *rst_gpio;
u8 cur_page;
struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
+   struct regmap   *regmap;
 };
 
 static int sx1301_read_burst(struct sx1301_priv *priv, u8 reg, u8 *val, size_t 
len)
@@ -614,6 +644,7 @@ static int sx1301_probe(struct spi_device *spi)
struct spi_sx1301 *radio;
struct gpio_desc *rst;
int ret;
+   unsigned int ver;
u8 val;
 
rst = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_LOW);
@@ -641,14 +672,21 @@ static int sx1301_probe(struct spi_device *spi)
priv->spi = spi;
SET_NETDEV_DEV(netdev, >dev);
 
-   ret = sx1301_read(priv, REG_VERSION, );
+   priv->regmap = devm_regmap_init_spi(spi, _regmap_config);
+   if (IS_ERR(priv->regmap)) {
+   ret = PTR_ERR(priv->regmap);
+   dev_err(>dev, "Regmap allocation failed: %d\n", ret);
+   return ret;
+   }
+
+   ret = regmap_read(priv->regmap, SX1301_VER, );
if (ret) {
dev_err(>dev, "version read failed\n");
return ret;
}
 
-   if (val != SX1301_CHIP_VERSION) {
-   dev_err(>dev, "unexpected version: %u\n", val);
+   if (ver != SX1301_CHIP_VERSION) {
+   dev_err(>dev, "unexpected version: %u\n", ver);
return -ENXIO;
}
 
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index b37ac56..2fc283f 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -15,4 +15,14 @@
 #define SX1301_MCU_AGC_FW_VERSION 4
 #define SX1301_MCU_AGC_CAL_FW_VERSION 2
 
+/* Page independent */
+#define SX1301_PAGE 0x00
+#define SX1301_VER  0x01
+
+#define SX1301_VIRT_BASE0x100
+#define SX1301_PAGE_LEN 0x80
+#define SX1301_PAGE_BASE(n) (SX1301_VIRT_BASE + (SX1301_PAGE_LEN * n))
+
+#define SX1301_MAX_REGISTER (SX1301_PAGE_BASE(3) + 0x7F)
+
 #endif
-- 
2.7.4