linux-kernel - RE: [PATCH lora-next 2/4] dt-bindings: lora: sx125x: add basic documentation

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, 16 Jan 2019 17:22:37 +0000
From:   Ben Whitten <Ben.Whitten@...rdtech.com>
To:     Andreas Färber <afaerber@...e.de>,
        Ben Whitten <ben.whitten@...il.com>
CC:     "linux-lpwan@...ts.infradead.org" <linux-lpwan@...ts.infradead.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>
Subject: RE: [PATCH lora-next 2/4] dt-bindings: lora: sx125x: add basic
 documentation

Hi Andreas,

> Am 08.01.19 um 09:41 schrieb Ben Whitten:
> > The sx125x family are IQ radio transceivers from Semtech configured over
> > SPI, they are typically connected to an sx130x series concentrator however
> > may be connected to a host directly.
> 
> "SX125x" and "SX130x"
> 
> >
> > Required properties include the radio number of the host or concentrator
> > bus.
> >
> > Signed-off-by: Ben Whitten <ben.whitten@...il.com>
> > ---
> >  .../bindings/lora/semtech,sx125x.yaml         | 45 +++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/lora/semtech,sx125x.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx125x.yaml
> b/Documentation/devicetree/bindings/lora/semtech,sx125x.yaml
> > new file mode 100644
> > index 000000000000..5eadec860b70
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/lora/semtech,sx125x.yaml
> > @@ -0,0 +1,45 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/lora/semtech,sx125x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Semtech IQ modulator/de-modulator transeiver
> > +
> > +maintainers:
> > +  - Andreas Färber <afaerber@...e.de>
> > +  - Ben Whitten <ben.whitten@...il.com>
> > +
> > +description: |
> > +  The sx125x family are highly integrated RF front-end to digital I and Q
> > +  modulator/demodulator Multi-PHY mode transceiver capable of
> supporting
> > +  multiple constant and non-constant envelope modulation schemes.
> 
> "SX125x"
> 
> "are ... transceivers"
> 
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +        - semtech,sx1255
> > +        - semtech,sx1257
> > +        - semtech,sx1258
> 
> Do we need all three? Probably yes for the concentrator to make decisions?

We can probe the device version from the concentrator or a host, but if I recall
from a previous review we should not rely on the version register for making
decisions.
The version register of the SX1257 has been missed from the public datasheet
(although exists) and the existing concentrator HAL does not make decisions
based on the version register value.
Better to be explicit in the DT and let the driver use that.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: The chip select on the SPI bus or radio number in
> concentrator.
> 
> ", with radio A = 0 and radio B = 1."
> 
> > +
> > +  spi-max-frequency:
> > +    maximum: 10000000
> > +    default: 8000000
> > +    description: The frequency of the SPI communication to the radio,
> > +      in Hz. Maximum SPI frequency is 10MHz.
> 
> If on the concentrator's SPI bus then this is unused. You don't mark it
> required below, so maybe add a textual note?

As with SX130x comment, this should be covered in common SPI bindings
anyone connecting directly to spi will inherit that, and then anyone using
concentrator will not require it so we can drop this.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    radio0: lora@0 {
> > +      compatible = "semtech,sx1257";
> > +      reg = <0>;
> > +    };
> 
> If you compare my sx127x implementation, should we prepare a
> "radio-frequency" (or "radio-center-frequency") property for the
> antenna, to preconfigure the driver during probe? Contents would then be
> <868000000>, for example.

I don’t know the policy of configuration in DT.
Is the value a hardware property of the antenna? Could we call it
"antenna-frequency" or "bandwidth" and give it a high low point?
Whilst considering antenna properties we should include a gain value as
that affects the radiated power at a given transmit power, food for though
when we come to regulatory db.

Thanks,
Ben Whitten