Message-ID: <076ce794-036a-291a-0167-5f302f67f74f@suse.de>
Date:   Mon, 7 Jan 2019 12:29:32 +0100
From:   Andreas Färber <afaerber@...e.de>
To:     Heinrich Schuchardt <xypron.glpk@....de>
Cc:     netdev@...r.kernel.org, Marcel Holtmann <marcel@...tmann.org>,
        linux-kernel@...r.kernel.org, Jon Ortego <Jon.Ortego@...t.de>,
        "David S . Miller" <davem@...emloft.net>,
        linux-arm-kernel@...ts.infradead.org,
        "linux-lpwan@...ts.infradead.org" <linux-lpwan@...ts.infradead.org>
Subject: Re: [RFC net-next 11/15] net: lora: Add IMST WiMOD

Hello Heinrich,

+ linux-lpwan, - other vendors

Am 06.01.19 um 15:57 schrieb Heinrich Schuchardt:
> On 7/1/18 1:08 PM, Andreas Färber wrote:
>> The IMST WiMOD uses a SLIP based binary UART protocol. Two separate
>> firmwares are available. By default it ships with a LoRaWAN firmware.
>> The alternative firmware is a custom P2P addressing mode based on LoRa.
>>
>> Cc: Jon Ortego <Jon.Ortego@...t.de>
>> Signed-off-by: Andreas Färber <afaerber@...e.de>
>> ---
>>  drivers/net/lora/Kconfig  |   8 +
>>  drivers/net/lora/Makefile |   3 +
>>  drivers/net/lora/wimod.c  | 597 ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 608 insertions(+)
>>  create mode 100644 drivers/net/lora/wimod.c
>>
>> diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
>> index 940bd2cbe106..2e05caef8645 100644
>> --- a/drivers/net/lora/Kconfig
>> +++ b/drivers/net/lora/Kconfig
>> @@ -31,6 +31,14 @@ config LORA_SX1276
>>  	help
>>  	  Semtech SX1272/1276/1278
>>  
>> +config LORA_WIMOD
>> +	tristate "IMST WiMOD driver"
> 
> scripts/checkpatch.pl throws this warning:
> WARNING: please write a paragraph that describes the config symbol fully
> 
> IMST has multiple products related to "WiMOD":
> * WiMOD iC880A
> * WiMOD module iM871A
> * WSA01-iM880B - WiMOD Shield for Arduino
> * iM880B-L - Long Range Radio Module

True, and I notice that my wimod.c doesn't explain it either.

Similarly, in patch 12/15 USI is the name of its vendor (but that one
does mention the model in commit message, Kconfig help and header).

> 
> And IMST is not very consistent about what is called "WiMOD". So this
> leaves me clueless concerning what this Kconfig option is about. Please,
> provide a description.

So, do you actually have any of them? :) Otherwise the Kconfig symbol,
patch subject and cover letter all clearly indicate LoRa.

* iM871A is rather a Wireless M-Bus module - if you have it and are
interested in collaborating on a driver, let me know. Its HCI message
format looks slightly different, so it would need a separate driver.
Currently I only have access to multiple chipsets with pure FSK support
that one would need a Linux userspace/kernel implementation of wM-Bus
for, plus something to test against. Some LoRa gateway vendors have
expressed a need to consume the FSK support in SX1301 for wM-Bus; sx127x
might still be the easiest path to testing that, but FSK has not been an
implementation priority for me yet, more a design consideration.
Side note: A serdev driver like this might be used to implement also
(non-Wireless) M-Bus in kernel space, if desired.

* iC880A uses the sx130x SPI driver directly. You'll have a hard time
using this serdev driver with it... Please don't waste time commenting
on patch 15/15, it has largely been rewritten since and is under active
work on linux-lpwan list, still in preparation of an RFC v2 patchset.
https://lists.infradead.org/pipermail/linux-lpwan/
The sx130x driver is being tested with iC880A (and on RPi3B+ suffers
from the same clk_prepare() issue as RAK831 on Pine64+).

* iM880B, iM880B-L, iM881A and iM980A should hopefully be pretty much
compatible, and they were the only LoRa modules at the time of posting.
This driver had been tested with the Arduino Shield, using ArPi-600
adapter to Raspberry Pi pinout, connected to a Pine64+ board.

Sadly my Pine64+ board that this Arduino Shield was connected to had
stopped booting (and annoyingly not for the first time). Looks like it
damaged U-Boot somehow... (no serial output anymore)
Writing a new U-Boot onto the card or starting with a new image, I ran
into a U-Boot/TF-A issue related to UEFI that I've now reported:
https://bugzilla.opensuse.org/show_bug.cgi?id=1120869
I now have a new booting image again to continue this driver but need to
set up my test environment with DT Overlays and kernel tree again.

* I hope the new iM282A can work with the same driver - it uses LR-Base+
firmware, whereas iM880B ships with LR-LoRaWAN and has an optional
LR-Base firmware. I am hopeful to cover both with one self-detecting
driver, somehow. A difficulty with LR-Base/LR-Base+ is that it prepends
some custom addressing fields to the user-supplied data, so is not
really PF_LORA / ETH_P_LORA in the sense of this series.

=> This driver covers more than one model. At the moment only iM880B is
verified, iM282A TBD. If you have better naming suggestions let us know.

And if you have experience flashing firmware onto the Arduino Shield
using Linux/stm32flash, do let me know. I'll need that to test LR-Base.

> There are dozens of warnings given by scripts/checkpatch. Please, have a
> look at them.

Please take a look at the date of these patches: It is not helpful to
nitpick about early RFC patches from July now. If my linux-lora.git repo
still has that issue (which I'm sure it does, but you didn't mention you
checked!) then you're welcome to send a fix-up patch against that repo
to linux-lpwan list. Otherwise this is not a priority for me, given
current clk_prepare() lockups and other bigger issues under discussion!

The cover letter mentioned a number of big open socket questions, and so
did my ELCE 2018 presentation:

https://events.linuxfoundation.org/wp-content/uploads/2017/12/ELCE2018_LoRa_final_Andreas-Farber.pdf
https://www.youtube.com/watch?v=Jjel65sZO9M

The purpose of this RFC series was to discuss the _socket layer_ design
and how it would interface with different drivers. This driver didn't
expose any netdev yet, focusing on a PoC of its binary as opposed to AT
protocol. Therefore it is not ready for merging or for unfamiliar users.
Back then there was no LoRaWAN patchset yet (discussing how Jian-Hong's
and my work could be combined was one purpose of this RFC!), now we have
some preliminary constants and an early PF_LORAWAN socket implementation
staged that we can revisit this driver with.

It still carries two implementations for sending the HCI commands that
I'd be interested in feedback for deciding on which to pursue. Also the
receive path of all my serdev drivers could use some review and help.
This driver as-is is nowhere near merging, so please understand that
line length etc. warnings are the least of my concern for this WIP code.

Please don't forget to CC the new linux-lpwan mailing list on any reply
- it contains people for whom the netdev traffic simply is too much.

Thanks,
Andreas

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

Powered by blists - more mailing lists