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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6c8404e3-467e-bdd5-e855-c5ad8c03de7b@suse.de>
Date:   Thu, 9 Aug 2018 23:58:52 +0200
From:   Andreas Färber <afaerber@...e.de>
To:     Ben Whitten <ben.whitten@...il.com>
Cc:     starnight@...cu.edu.tw, hasnain.virk@....com,
        netdev@...r.kernel.org, liuxuenetmail@...il.com, shess@...sware.de,
        Ben Whitten <ben.whitten@...rdtech.com>
Subject: Re: [PATCH lora-next v2 7/8] net: lora: sx1301: add initial
 registration for regmap

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 <ben.whitten@...rdtech.com>
> ---
>  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 <linux/of_gpio.h>
>  #include <linux/lora/dev.h>
>  #include <linux/spi/spi.h>
> +#include <linux/regmap.h>

Misordered.

>  
>  #include "sx1301.h"
>  
>  #define REG_PAGE_RESET			0
> -#define REG_VERSION			1
>  #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[] = {

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)