Re: [PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
On Tue, Jan 08, 2019 at 05:41:29PM +0900, Ben Whitten wrote: > Add basic documentation in YAML format for the sx130x series concentrators > from Semtech. > Required is; the location on the SPI bus, the reset gpio and the node for > downstream IQ radios, typically sx125x. > > Signed-off-by: Ben Whitten > --- > .../bindings/lora/semtech,sx130x.yaml | 87 +++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > new file mode 100644 > index ..ad263bc4e60d > --- /dev/null > +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/lora/semtech,sx130x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Semtech LoRa concentrator > + > +maintainers: > + - Andreas Färber > + - Ben Whitten > + > +description: | > + Semtech LoRa concentrator sx130x digital baseband chip is capable of > + demodulating LoRa signals on 8 channels simultaneously. > + > + It is typically paired with two sx125x IQ radios controlled over an > + SPI directly from the concentrator. > + > + The concentrator itself it controlled over SPI. > + > +properties: > + compatible: > +items: > + - enum: > +- semtech,sx1301 > +- semtech,sx1308 > + > + reg: > +maxItems: 1 > +description: The chip select on the SPI bus. > + > + reset-gpios: > +maxItems: 1 > +description: A connection of the reset gpio line. > + > + spi-max-frequency: > +maximum: 1000 > +default: 800 > +description: The frequency of the SPI communication to the concentrator, > + in Hz. Maximum SPI frequency is 10MHz although 8MHz is typically used > + on a number of cards. > + > + radio-spi: > +description: The concentrator has two radios connected which are > contained > + within the following node. > + > +properties: > + '#address-cells': > +const: 1 > + > + '#size-cells': > +const: 0 > + > +required: > + - '#address-cells' > + - '#size-cells' > + > +required: > + - compatible > + - reg > + - reset-gpios > + - radio-spi > + > +examples: > + - | One more comment. Make sure you "build" the schema with 'dt_binding_check' target. That builds the examples. It is also checked by a CI job and the status is added to patchwork[1]. (Though it didn't work correctly for this one and I just fixed it.) > +concentrator0: lora@0 { This example doesn't build because there's an expectation of the top level nodes being MMIO if there's a reg prop. You just need to wrap this in a parent node representing a spi controller: spi { #address-cells = <1>; #size-cells = <0>; ... }; > + compatible = "semtech,sx1301"; > + reg = <0>; > + reset-gpios = <&pioB 27 GPIO_ACTIVE_HIGH>; > + spi-max-frequency = <800>; > + > + radio-spi { > +#address-cells = <1>; > +#size-cells = <0>; > + > +radio0: lora@0 { > + compatible = "semtech,sx1257"; > + reg = <0>; > +}; > + > +radio1: lora@1 { > + compatible = "semtech,sx1257"; > + reg = <1>; > +}; > + }; > +}; [1] https://patchwork.ozlabs.org/patch/1021773/
Re: [PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
On Mon, Jan 21, 2019 at 01:14:20PM -0600, Rob Herring wrote: > On Tue, Jan 08, 2019 at 05:41:29PM +0900, Ben Whitten wrote: > > Add basic documentation in YAML format for the sx130x series concentrators > > from Semtech. > > Required is; the location on the SPI bus, the reset gpio and the node for > > downstream IQ radios, typically sx125x. > > > > Signed-off-by: Ben Whitten > > --- > > .../bindings/lora/semtech,sx130x.yaml | 87 +++ > > 1 file changed, 87 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > A schema binding. Yay! > > > > > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > new file mode 100644 > > index ..ad263bc4e60d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > @@ -0,0 +1,87 @@ > > +# SPDX-License-Identifier: GPL-2.0 Also, I'd like new bindings to be dual GPL-2.0 and BSD-2-Clause if you don't mind. This will make it easier to move the schema out of the kernel if/when we ever do that. Rob
Re: [PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
On Tue, Jan 08, 2019 at 05:41:29PM +0900, Ben Whitten wrote: > Add basic documentation in YAML format for the sx130x series concentrators > from Semtech. > Required is; the location on the SPI bus, the reset gpio and the node for > downstream IQ radios, typically sx125x. > > Signed-off-by: Ben Whitten > --- > .../bindings/lora/semtech,sx130x.yaml | 87 +++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/lora/semtech,sx130x.yaml A schema binding. Yay! > > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > new file mode 100644 > index ..ad263bc4e60d > --- /dev/null > +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/lora/semtech,sx130x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Semtech LoRa concentrator > + > +maintainers: > + - Andreas Färber > + - Ben Whitten > + > +description: | > + Semtech LoRa concentrator sx130x digital baseband chip is capable of > + demodulating LoRa signals on 8 channels simultaneously. > + > + It is typically paired with two sx125x IQ radios controlled over an > + SPI directly from the concentrator. > + > + The concentrator itself it controlled over SPI. s/it/is/ > + > +properties: > + compatible: > +items: > + - enum: > +- semtech,sx1301 > +- semtech,sx1308 > + > + reg: > +maxItems: 1 > +description: The chip select on the SPI bus. > + > + reset-gpios: > +maxItems: 1 > +description: A connection of the reset gpio line. > + > + spi-max-frequency: > +maximum: 1000 > +default: 800 > +description: The frequency of the SPI communication to the concentrator, > + in Hz. Maximum SPI frequency is 10MHz although 8MHz is typically used > + on a number of cards. > + > + radio-spi: child nodes should have 'type: object' > +description: The concentrator has two radios connected which are > contained > + within the following node. > + > +properties: > + '#address-cells': > +const: 1 > + > + '#size-cells': > +const: 0 You need the grandchildren here too to define 'reg' should be 0-8. You'll need to use 'patternProperties' since there's a unit-address. You could maybe get rid of the radio-spi node. You don't need it unless you wanted to add other types of child nodes. > + > +required: > + - '#address-cells' > + - '#size-cells' > + > +required: > + - compatible > + - reg > + - reset-gpios > + - radio-spi > + > +examples: > + - | > +concentrator0: lora@0 { > + compatible = "semtech,sx1301"; > + reg = <0>; > + reset-gpios = <&pioB 27 GPIO_ACTIVE_HIGH>; > + spi-max-frequency = <800>; > + > + radio-spi { > +#address-cells = <1>; > +#size-cells = <0>; > + > +radio0: lora@0 { > + compatible = "semtech,sx1257"; > + reg = <0>; > +}; > + > +radio1: lora@1 { > + compatible = "semtech,sx1257"; > + reg = <1>; > +}; > + }; > +}; > -- > 2.17.1 >
RE: [PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
Hi Andreas, > Am 08.01.19 um 09:41 schrieb Ben Whitten: > > Add basic documentation in YAML format for the sx130x series concentrators > > from Semtech. > > Required is; the location on the SPI bus, the reset gpio and the node for > > downstream IQ radios, typically sx125x. > > > > Signed-off-by: Ben Whitten > > --- > > .../bindings/lora/semtech,sx130x.yaml | 87 +++ > > 1 file changed, 87 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > Patch 3/4 moves this to net/lora/, which I think is more appropriate. Agreed, I think it was a change merged into the wrong commits by mistake > > > > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > new file mode 100644 > > index ..ad263bc4e60d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > > @@ -0,0 +1,87 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/lora/semtech,sx130x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Semtech LoRa concentrator > > + > > +maintainers: > > + - Andreas Färber > > + - Ben Whitten > > + > > +description: | > > + Semtech LoRa concentrator sx130x digital baseband chip is capable of > > SX130x or SX1301/SX1308, to distinguish from driver name sx130x and to > avoid ambiguities of which x is a wildcard. > > > + demodulating LoRa signals on 8 channels simultaneously. > > + > > + It is typically paired with two sx125x IQ radios controlled over an > > Ditto, SX125x > > > + SPI directly from the concentrator. > > + > > + The concentrator itself it controlled over SPI. > > + > > +properties: > > + compatible: > > +items: > > + - enum: > > +- semtech,sx1301 > > +- semtech,sx1308 > > We should only list both if we expect differences between the two > models. Otherwise SX1308 can reuse the SX1301 compatible. If we want to > mark it up just in case then rearranging the above to be a sequence of > "semtech,sx1308", "semtech,sx1301" would be an alternative. It was my understanding that we should name each device that is compatible, avoiding wildcard 'x' in compatible names. This allows the device tree to be more accurate to the hardware that it is describing. I do not expect there to be much difference, but there may be some that I am unaware of. Not sure I follow here, do you wish for the order to be flipped if we do want to state every device? I see that example-schema does indeed have entries in reverse. > > > + > > + reg: > > +maxItems: 1 > > +description: The chip select on the SPI bus. > > Is this mandatory now or not with maxItems? min/maxItems is implied if you have a list but for our chipselect we have no list. I followed child-node-example.yaml in yaml-bindings and trivial-devices.yaml in being explicit and stating it be one element and making reg required. > > > + > > + reset-gpios: > > +maxItems: 1 > > +description: A connection of the reset gpio line. > > This needs to be optional, which I think the maxItems syntax says unlike > the commit message? > On an mPCIe card you won't have such a GPIO, for instance. We do a Soft > Reset, so it's not functionally mandatory. I'll drop this from required and from the commit message. > > + > > + spi-max-frequency: > > +maximum: 1000 > > +default: 800 > > +description: The frequency of the SPI communication to the > > concentrator, > > + in Hz. Maximum SPI frequency is 10MHz although 8MHz is typically used > > + on a number of cards. > > Do we really need to describe this here? It should be covered in the > common SPI bindings, and only applies to SPI bus, not USB picoGW. True, I'll drop this. > > > + > > + radio-spi: > > +description: The concentrator has two radios connected which are > contained > > + within the following node. > > "can have" > > > + > > +properties: > > + '#address-cells': > > +const: 1 > > + > > + '#size-cells': > > +const: 0 > > + > > +required: > > + - '#address-cells' > > + - '#size-cells' > > I'm pretty sure that Rob would like to have a compatible here, even if > unneeded in our Linux driver? > > BTW if someone has better naming suggestions than "radio-spi"... I just > wanted to avoid having it in the main node directly, in case we need > other sub-nodes, too. Just like ahb and apb it makes sense to separate the bus from the device An alternative could be "transceiver-bus" or "radio-bus" as the fact its spi is masked away anyway. What would it's compatible string be, match the node name? Would the sx130x_radio bus type match against it? > > > + > > +required: > > + - compatible > > + - reg > > + - reset-gpios > > Must be optional. > > > + - radio-spi > > Should be optional. (Driver needs it t
Re: [PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
Hi Ben, Am 08.01.19 um 09:41 schrieb Ben Whitten: > Add basic documentation in YAML format for the sx130x series concentrators > from Semtech. > Required is; the location on the SPI bus, the reset gpio and the node for > downstream IQ radios, typically sx125x. > > Signed-off-by: Ben Whitten > --- > .../bindings/lora/semtech,sx130x.yaml | 87 +++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/lora/semtech,sx130x.yaml Patch 3/4 moves this to net/lora/, which I think is more appropriate. > > diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > new file mode 100644 > index ..ad263bc4e60d > --- /dev/null > +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/lora/semtech,sx130x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Semtech LoRa concentrator > + > +maintainers: > + - Andreas Färber > + - Ben Whitten > + > +description: | > + Semtech LoRa concentrator sx130x digital baseband chip is capable of SX130x or SX1301/SX1308, to distinguish from driver name sx130x and to avoid ambiguities of which x is a wildcard. > + demodulating LoRa signals on 8 channels simultaneously. > + > + It is typically paired with two sx125x IQ radios controlled over an Ditto, SX125x > + SPI directly from the concentrator. > + > + The concentrator itself it controlled over SPI. > + > +properties: > + compatible: > +items: > + - enum: > +- semtech,sx1301 > +- semtech,sx1308 We should only list both if we expect differences between the two models. Otherwise SX1308 can reuse the SX1301 compatible. If we want to mark it up just in case then rearranging the above to be a sequence of "semtech,sx1308", "semtech,sx1301" would be an alternative. > + > + reg: > +maxItems: 1 > +description: The chip select on the SPI bus. Is this mandatory now or not with maxItems? > + > + reset-gpios: > +maxItems: 1 > +description: A connection of the reset gpio line. This needs to be optional, which I think the maxItems syntax says unlike the commit message? On an mPCIe card you won't have such a GPIO, for instance. We do a Soft Reset, so it's not functionally mandatory. > + > + spi-max-frequency: > +maximum: 1000 > +default: 800 > +description: The frequency of the SPI communication to the concentrator, > + in Hz. Maximum SPI frequency is 10MHz although 8MHz is typically used > + on a number of cards. Do we really need to describe this here? It should be covered in the common SPI bindings, and only applies to SPI bus, not USB picoGW. > + > + radio-spi: > +description: The concentrator has two radios connected which are > contained > + within the following node. "can have" > + > +properties: > + '#address-cells': > +const: 1 > + > + '#size-cells': > +const: 0 > + > +required: > + - '#address-cells' > + - '#size-cells' I'm pretty sure that Rob would like to have a compatible here, even if unneeded in our Linux driver? BTW if someone has better naming suggestions than "radio-spi"... I just wanted to avoid having it in the main node directly, in case we need other sub-nodes, too. > + > +required: > + - compatible > + - reg > + - reset-gpios Must be optional. > + - radio-spi Should be optional. (Driver needs it today, but that's another problem.) > + > +examples: > + - | > +concentrator0: lora@0 { > + compatible = "semtech,sx1301"; > + reg = <0>; > + reset-gpios = <&pioB 27 GPIO_ACTIVE_HIGH>; > + spi-max-frequency = <800>; > + > + radio-spi { > +#address-cells = <1>; > +#size-cells = <0>; > + > +radio0: lora@0 { > + compatible = "semtech,sx1257"; > + reg = <0>; > +}; > + > +radio1: lora@1 { > + compatible = "semtech,sx1257"; > + reg = <1>; > +}; > + }; > +}; Thanks for looking into this, Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
[PATCH lora-next 1/4] dt-bindings: lora: sx130x: add basic documentation
Add basic documentation in YAML format for the sx130x series concentrators from Semtech. Required is; the location on the SPI bus, the reset gpio and the node for downstream IQ radios, typically sx125x. Signed-off-by: Ben Whitten --- .../bindings/lora/semtech,sx130x.yaml | 87 +++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/lora/semtech,sx130x.yaml diff --git a/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml new file mode 100644 index ..ad263bc4e60d --- /dev/null +++ b/Documentation/devicetree/bindings/lora/semtech,sx130x.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/lora/semtech,sx130x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Semtech LoRa concentrator + +maintainers: + - Andreas Färber + - Ben Whitten + +description: | + Semtech LoRa concentrator sx130x digital baseband chip is capable of + demodulating LoRa signals on 8 channels simultaneously. + + It is typically paired with two sx125x IQ radios controlled over an + SPI directly from the concentrator. + + The concentrator itself it controlled over SPI. + +properties: + compatible: +items: + - enum: +- semtech,sx1301 +- semtech,sx1308 + + reg: +maxItems: 1 +description: The chip select on the SPI bus. + + reset-gpios: +maxItems: 1 +description: A connection of the reset gpio line. + + spi-max-frequency: +maximum: 1000 +default: 800 +description: The frequency of the SPI communication to the concentrator, + in Hz. Maximum SPI frequency is 10MHz although 8MHz is typically used + on a number of cards. + + radio-spi: +description: The concentrator has two radios connected which are contained + within the following node. + +properties: + '#address-cells': +const: 1 + + '#size-cells': +const: 0 + +required: + - '#address-cells' + - '#size-cells' + +required: + - compatible + - reg + - reset-gpios + - radio-spi + +examples: + - | +concentrator0: lora@0 { + compatible = "semtech,sx1301"; + reg = <0>; + reset-gpios = <&pioB 27 GPIO_ACTIVE_HIGH>; + spi-max-frequency = <800>; + + radio-spi { +#address-cells = <1>; +#size-cells = <0>; + +radio0: lora@0 { + compatible = "semtech,sx1257"; + reg = <0>; +}; + +radio1: lora@1 { + compatible = "semtech,sx1257"; + reg = <1>; +}; + }; +}; -- 2.17.1