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

Ben Whitten
2019-01-16T17:22:45Z

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@gmail.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@suse.de>
> > + - Ben Whitten <ben.whitten@gmail.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