Linux-WPAN Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH 1/1] net: lora: add sysfs entry for LoRa system
       [not found]     ` <CAJuUDws-Ob_W4U83jnR3hH0Q=R45qRAQbyht11EbbPfL5CrD3A@mail.gmail.com>
@ 2019-01-29 15:26       ` Andreas Färber
  2019-02-01  8:56         ` Xue Liu
  0 siblings, 1 reply; 2+ messages in thread
From: Andreas Färber @ 2019-01-29 15:26 UTC (permalink / raw)
  To: Xue Liu; +Cc: linux-lpwan, linux-wpan - ML, Alexander Aring, Stefan Schmidt

Hello,

Am 29.01.19 um 13:58 schrieb Xue Liu:
> On Tue, 29 Jan 2019 at 06:53, Andreas Färber <afaerber@suse.de> wrote:
>> Am 28.01.19 um 23:09 schrieb Xue Liu:
>>> Signed-off-by: Xue Liu <liuxuenetmail@gmail.com>
>>> ---
>>>  net/lora/Makefile |  1 +
>>>  net/lora/cfg.c    | 45 +++++++++++++++++++++++++++-
>>>  net/lora/cfg.h    |  4 +++
>>>  net/lora/sysfs.c  | 76 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  net/lora/sysfs.h  | 10 +++++++
>>>  5 files changed, 135 insertions(+), 1 deletion(-)
>>>  create mode 100644 net/lora/sysfs.c
>>>  create mode 100644 net/lora/sysfs.h
[...]
>> Please don't copy code from 802.15.4 (including its "ugh" comments!) to
>> lora, I tried to do a minimal sane implementation here. Why does it need
>> to be a device?
> which device do you mean ? struct device ?

Yes, you're doing a device_initialize() and lora_class dance there,
without mentioning it in your commit message or cover letter!

>> I also didn't understand why a single 802.15.4 PHY would
>> be associated with multiple net_devices; when we don't allow that here,
> AFAIK net_deivce (wpan_dev) is acting as a virtual interface.
> Multi interfaces can share a single PHY. The advantage is that you can use
> only one PHY to simulate many virtual devices with different parameters,
> e.g. address. Not only 802.15.4 but also 802.11 are using this strategy.

We don't have any address at the PHY layer, neither for LoRa nor FSK.
The only things we have at the PHY layer are freq/SF/bw and Sync Word.
And I have not found any LoRa or FSK PHY so far that could listen to
multiple Sync Words in parallel. Therefore it'll need to be configured
via netlink for the one PHY, with no obvious use case for multiple
net_devices.

>> that also simplifies our reference counting, as it is then bound to the
>> lifetime of the parent device.
>>
>> Note also how I added a net_device pointer to the phy, whereas 802.15.4
>> has a pointer in net_device (which would rule out me testing this on our
>> distro kernels).
>>
> I did not touch net_device.
>> In case of SX1276 our sx127x SPI driver would have three different PHYs.
>> Still not sure whether we would want ten PHYs for SX1301 or just two -
>> the former would rule out going by ifindex and require a netlink
>> interface to enumerate PHYs to obtain an identifier for reference,
>> unless we restart the old "joke" of how many netdevs we need for sx130x.
>>
> Why do you want three different PHYs ?. Can SX1276 working in three
> different channel at the same time ? I think multiple PHYs for one chip is
> reasonable for the chip which can transmitting and receiving packets from
> different channels at the same time, such as SX1301 and MCR20A.

It's not about different channels, it's about different modulation modes
and therefore different configuration and driver frameworks. Just like a
Wifi driver won't use the ieee802154 or whatever other PHY struct.

We can't tie FSK into a LoRa PHY, as I have demonstrated with nrf24l01p
and si443x drivers that there are FSK transceivers unrelated to LoRa. So
we'll end up with drivers/net/lora/ using LoRa, FSK and ASK/OOK PHYs,
drivers/fsk/ using FSK and ASK/OOK PHYs. Plus LoRaWAN MAC layer. Yes,
it's a mess, and it gets worse if you consider modules with BLE/Sigfox
in addition. I see no sane way to use mfd framework there.

My original LoRa RFC received zero feedback on this topic so far, so I
am planning for a single lora0 net_device with three alternative PHYs,
switched via netlink or something; the alternative would've been three
net_devices with one PHY each, of which only one can be up at a time.

Hoping we can discuss that at Netdevconf 0x13 in Stefan's IoT workshop.

>> Finally, why do we need a sysfs entry when we already have netlink? Not
>> saying we can't have sysfs, you just haven't explained your motivation.
>> Reasons (benefits) convince me more than precedence.
>>
> Not every programing language has support for netlink, such as Bash.
> A good example is the script used in OpenWRT[1].
[...]
> [1] https://github.com/openwrt/openwrt/blob/master/package/kernel/mac80211/files/lib/wifi/mac80211.sh

Exposing something in sysfs could also be done via the main driver
though, like I've done for debugfs in sx127x driver.

What's your use case of using a bash script with the LoRa PHY? I'm not
convinced that OpenWRT scripts are the best design example here...

As long as a struct is internal to our drivers, we can refactor it as
much as we want. Once we expose things to userspace like you do here,
changes become much more problematic. Therefore it needs to be a
conscious decision and not hidden in a larger patch.

Regards,
Andreas

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/1] net: lora: add sysfs entry for LoRa system
  2019-01-29 15:26       ` [PATCH 1/1] net: lora: add sysfs entry for LoRa system Andreas Färber
@ 2019-02-01  8:56         ` Xue Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Xue Liu @ 2019-02-01  8:56 UTC (permalink / raw)
  To: Andreas Färber
  Cc: linux-lpwan, linux-wpan - ML, Alexander Aring, Stefan Schmidt

Hi Andreas,

On Tue, 29 Jan 2019 at 16:26, Andreas Färber <afaerber@suse.de> wrote:
>
> Hello,
>
> Am 29.01.19 um 13:58 schrieb Xue Liu:
> > On Tue, 29 Jan 2019 at 06:53, Andreas Färber <afaerber@suse.de> wrote:
> >> Am 28.01.19 um 23:09 schrieb Xue Liu:
> >>> Signed-off-by: Xue Liu <liuxuenetmail@gmail.com>
> >>> ---
> >>>  net/lora/Makefile |  1 +
> >>>  net/lora/cfg.c    | 45 +++++++++++++++++++++++++++-
> >>>  net/lora/cfg.h    |  4 +++
> >>>  net/lora/sysfs.c  | 76 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>  net/lora/sysfs.h  | 10 +++++++
> >>>  5 files changed, 135 insertions(+), 1 deletion(-)
> >>>  create mode 100644 net/lora/sysfs.c
> >>>  create mode 100644 net/lora/sysfs.h
> [...]
> >> Please don't copy code from 802.15.4 (including its "ugh" comments!) to
> >> lora, I tried to do a minimal sane implementation here. Why does it need
> >> to be a device?
> > which device do you mean ? struct device ?
>
> Yes, you're doing a device_initialize() and lora_class dance there,
> without mentioning it in your commit message or cover letter!
>
Yes. My fault.
> >> I also didn't understand why a single 802.15.4 PHY would
> >> be associated with multiple net_devices; when we don't allow that here,
> > AFAIK net_deivce (wpan_dev) is acting as a virtual interface.
> > Multi interfaces can share a single PHY. The advantage is that you can use
> > only one PHY to simulate many virtual devices with different parameters,
> > e.g. address. Not only 802.15.4 but also 802.11 are using this strategy.
>
> We don't have any address at the PHY layer, neither for LoRa nor FSK.
> The only things we have at the PHY layer are freq/SF/bw and Sync Word.
> And I have not found any LoRa or FSK PHY so far that could listen to
> multiple Sync Words in parallel. Therefore it'll need to be configured
> via netlink for the one PHY, with no obvious use case for multiple
> net_devices.
>
> >> that also simplifies our reference counting, as it is then bound to the
> >> lifetime of the parent device.
> >>
> >> Note also how I added a net_device pointer to the phy, whereas 802.15.4
> >> has a pointer in net_device (which would rule out me testing this on our
> >> distro kernels).
> >>
> > I did not touch net_device.
> >> In case of SX1276 our sx127x SPI driver would have three different PHYs.
> >> Still not sure whether we would want ten PHYs for SX1301 or just two -
> >> the former would rule out going by ifindex and require a netlink
> >> interface to enumerate PHYs to obtain an identifier for reference,
> >> unless we restart the old "joke" of how many netdevs we need for sx130x.
> >>
> > Why do you want three different PHYs ?. Can SX1276 working in three
> > different channel at the same time ? I think multiple PHYs for one chip is
> > reasonable for the chip which can transmitting and receiving packets from
> > different channels at the same time, such as SX1301 and MCR20A.
>
> It's not about different channels, it's about different modulation modes
> and therefore different configuration and driver frameworks. Just like a
> Wifi driver won't use the ieee802154 or whatever other PHY struct.
>
> We can't tie FSK into a LoRa PHY, as I have demonstrated with nrf24l01p
> and si443x drivers that there are FSK transceivers unrelated to LoRa. So
> we'll end up with drivers/net/lora/ using LoRa, FSK and ASK/OOK PHYs,
> drivers/fsk/ using FSK and ASK/OOK PHYs. Plus LoRaWAN MAC layer. Yes,
> it's a mess, and it gets worse if you consider modules with BLE/Sigfox
> in addition. I see no sane way to use mfd framework there.
>
> My original LoRa RFC received zero feedback on this topic so far, so I
> am planning for a single lora0 net_device with three alternative PHYs,
> switched via netlink or something; the alternative would've been three
> net_devices with one PHY each, of which only one can be up at a time.
>
Sorry. I am not a fulltime developer for the open source community. Sometimes
I can open the maillist in the weekend.
> Hoping we can discuss that at Netdevconf 0x13 in Stefan's IoT workshop.
>
Sorry. I have to move my home. Maybe next time.
> >> Finally, why do we need a sysfs entry when we already have netlink? Not
> >> saying we can't have sysfs, you just haven't explained your motivation.
> >> Reasons (benefits) convince me more than precedence.
> >>
> > Not every programing language has support for netlink, such as Bash.
> > A good example is the script used in OpenWRT[1].
> [...]
> > [1] https://github.com/openwrt/openwrt/blob/master/package/kernel/mac80211/files/lib/wifi/mac80211.sh
>
> Exposing something in sysfs could also be done via the main driver
> though, like I've done for debugfs in sx127x driver.
>
> What's your use case of using a bash script with the LoRa PHY? I'm not
> convinced that OpenWRT scripts are the best design example here...
>
In the company we use OpenWRT as the base system for some industry products.
I have a proposal to exploit the LoRa for IIoT together with OPC UA
for wireless remote control system.
So I am looking forward a simlar way/script like mac80211.sh to
communicate with this LoRa system,
so that it can easily cooperate with OpenWRT system components. A
seperate sysfs patch in OpenWRT
only may a solution for me.
> As long as a struct is internal to our drivers, we can refactor it as
> much as we want. Once we expose things to userspace like you do here,
> changes become much more problematic. Therefore it needs to be a
> conscious decision and not hidden in a larger patch.
>
Could you please propose your LoRa system roadmap here or links to
previous discussion, which I may miss before. So I can know what
we need and what I can contribute.

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

Regards,

Xue Liu

--

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190128220930.23366-1-liuxuenetmail@gmail.com>
     [not found] ` <20190128220930.23366-2-liuxuenetmail@gmail.com>
     [not found]   ` <43477075-3df1-0437-dc57-51715271a323@suse.de>
     [not found]     ` <CAJuUDws-Ob_W4U83jnR3hH0Q=R45qRAQbyht11EbbPfL5CrD3A@mail.gmail.com>
2019-01-29 15:26       ` [PATCH 1/1] net: lora: add sysfs entry for LoRa system Andreas Färber
2019-02-01  8:56         ` Xue Liu

Linux-WPAN Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wpan/0 linux-wpan/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wpan linux-wpan/ https://lore.kernel.org/linux-wpan \
		linux-wpan@vger.kernel.org
	public-inbox-index linux-wpan

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wpan


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git