netdev - Re: [PATCH lora-next 01/10] net: lora: sx1301: add register, bit-fields, and helpers 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]

Date:   Wed, 8 Aug 2018 10:57:41 +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, Ben Whitten <ben.whitten@...rdtech.com>
Subject: Re: [PATCH lora-next 01/10] net: lora: sx1301: add register,
 bit-fields, and helpers for regmap

Am 07.08.2018 um 19:32 schrieb Ben Whitten:
> From: Ben Whitten <ben.whitten@...rdtech.com>
> 
> 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.
> 
> Signed-off-by: Ben Whitten <ben.whitten@...rdtech.com>
> ---
>  drivers/net/lora/Kconfig  |   1 +
>  drivers/net/lora/sx1301.c | 282 +++++++++++++++++++++++++++++++++++++++++++---
>  drivers/net/lora/sx1301.h | 169 +++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/net/lora/sx1301.h

My main concern about this patch is its sheer size. Normally for
#defines the rule is not to add unused ones. Here I see for example FSK
RSSI fields that we're surely not using yet. Any chance to strip this
down some more?

Regards,
Andreas

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