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 12:32:52 +0000
From:   Ben Whitten <Ben.Whitten@...rdtech.com>
To:     Andreas Färber <afaerber@...e.de>,
        Ben Whitten <ben.whitten@...il.com>
CC:     "starnight@...cu.edu.tw" <starnight@...cu.edu.tw>,
        "hasnain.virk@....com" <hasnain.virk@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH lora-next 01/10] net: lora: sx1301: add register,
 bit-fields, and helpers for regmap

> 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?

Sure, I'll strip the reg_fields down to those only required for
loading firmware and setting clocks that will be used in the
immediate term. This does clutter the driver a bit
unnecessarily at the moment.
I would like to keep the full register dump in the header file
though as its self-contained and gives a full picture of the chip.

Thanks,
Ben