| Message ID | 20240812194441.3826789-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v1,1/1] dt-bindings: net: wireless: convert marvel-8xxx.txt to yaml format | expand |
Hi Frank, On Mon, Aug 12, 2024 at 03:44:40PM -0400, Frank Li wrote: > Convert binding doc marvel-8xxx.txt to yaml format. > Additional change: > - Remove marvell,caldata_00_txpwrlimit_2g_cfg_set in example. > - Remove mmc related property in example. > > Fix below warning: > arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dtb: /soc@0/bus@30800000/mmc@30b40000/wifi@1: > failed to match any schema with compatible: ['marvell,sd8997'] Can you make sure to run through `make dtbs_check` and handle any new issues? For one, I think you might want to include 'wakeup-source'? arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dtb: wifi@0,0: 'wakeup-source' does not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/net/wireless/marvell,8xxx.yaml# > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > .../bindings/net/wireless/marvell,8xxx.yaml | 96 +++++++++++++++++++ > .../bindings/net/wireless/marvell-8xxx.txt | 70 -------------- > 2 files changed, 96 insertions(+), 70 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml > delete mode 100644 Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt > > diff --git a/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml b/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml > new file mode 100644 > index 0000000000000..7b4927cdb7a01 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml > @@ -0,0 +1,96 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/wireless/marvell,8xxx.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell 8787/8897/8978/8997 (sd8787/sd8897/sd8978/sd8997/pcie8997) SDIO/PCIE devices > + > +maintainers: > + - Frank Li <Frank.Li@nxp.com> I wouldn't mind adding: - Brian Norris <briannorris@chromium.org> > + > +description: > + This node provides properties for controlling the Marvell SDIO/PCIE wireless device. Since we're essentially rewriting this doc, might as well tweak a few things: Please replace "controlling" with "describing". These bindings are for hardware description, not for software control (even though they seem like it sometimes and can be abused for that). > + The node is expected to be specified as a child node to the SDIO/PCIE controller that > + connects the device to the system. > + > +properties: > + compatible: > + enum: > + - marvell,sd8787 > + - marvell,sd8897 > + - marvell,sd8978 > + - marvell,sd8997 > + - nxp,iw416 > + - pci11ab,2b42 > + - pci1b4b,2b42 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + marvell,caldata-txpwrlimit-2g: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: calibration data Capitalize the first letter ("Calibration"). Same on all your descriptions. And maybe expand a little on what this means? Same on the other caldata properties. For example, "Calibration data for the 2GHz band." > + maxItems: 566 Non-critical question: are these numbers actually correct? The only instance I see in the upstream tree is arch/arm/boot/dts/rockchip/rk3288-veyron-jerry.dts, with 526 items. Yes, that still fits in this "max", but I just wonder whether this is an actually-correct specification, or an off-by-40 specification. Or, maybe the structure varies a lot by chip or firmware, and this max just isn't very meaningful. Like I said, it's non-critical, so maybe we leave it as-is, if it doesn't matter much. > + marvell,caldata-txpwrlimit-5g-sub0: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: calibration data Possibly, "Calibration data for sub-band 0 in the 5GHz band."? And even better if you can describe what sub-band 0 is (e.g., 5.xxx MHz - 5.yyy MHz). But I'm not familiar. > + maxItems: 502 > + > + marvell,caldata-txpwrlimit-5g-sub1: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: calibration data > + maxItems: 688 > + > + marvell,caldata-txpwrlimit-5g-sub2: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: calibration data > + maxItems: 750 > + > + marvell,caldata-txpwrlimit-5g-sub3: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: calibration data > + maxItems: 502 > + > + marvell,wakeup-pin: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + a wakeup pin number of wifi chip which will be configured > + to firmware. Firmware will wakeup the host using this pin > + during suspend/resume. Optional: this could use a bit of a rewrite to describe the hardware instead of the software. For example, "Provides the pin number for the wakeup pin from the device's point of view. The wakeup pin is used for the device to wake the host system from sleep. This property is only necessary if the wakeup pin is wired in a non-standard way, such that the default pin assignments are invalid." > + > + vmmc-supply: > + description: a phandle of a regulator, supplying VCC to the card I believe this vmmc-supply property is actually misplaced. I don't see any in-tree users, and OTOH all in-tree users specify this in the parent (e.g., the MMC controller), where it's already properly documented. > + mmc-pwrseq: > + description: > + phandle to the MMC power sequence node. See "mmc-pwrseq-*" > + for documentation of MMC power sequence bindings. Similarly, I think this is misplaced. See its introduction here, commit e3fffc1f0b47 ("devicetree: document new marvell-8xxx and pwrseq-sd8787 options"), but the controller docs (Documentation/devicetree/bindings/mmc/mmc-controller.yaml) specify these properties for the controller, not the endpoint/card. And Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.yaml is vague, but in practice, again, I think everyone uses this only in the controller. I'd consider dropping this and vmmc-supply, unless `make dtbs_check` complains. Brian > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + mmc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + wifi@1 { > + compatible = "marvell,sd8897"; > + reg = <1>; > + interrupt-parent = <&pio>; > + interrupts = <38 IRQ_TYPE_LEVEL_LOW>; > + marvell,wakeup-pin = <3>; > + }; > + }; > +
diff --git a/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml b/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml new file mode 100644 index 0000000000000..7b4927cdb7a01 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml @@ -0,0 +1,96 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/wireless/marvell,8xxx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell 8787/8897/8978/8997 (sd8787/sd8897/sd8978/sd8997/pcie8997) SDIO/PCIE devices + +maintainers: + - Frank Li <Frank.Li@nxp.com> + +description: + This node provides properties for controlling the Marvell SDIO/PCIE wireless device. + The node is expected to be specified as a child node to the SDIO/PCIE controller that + connects the device to the system. + +properties: + compatible: + enum: + - marvell,sd8787 + - marvell,sd8897 + - marvell,sd8978 + - marvell,sd8997 + - nxp,iw416 + - pci11ab,2b42 + - pci1b4b,2b42 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + marvell,caldata-txpwrlimit-2g: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: calibration data + maxItems: 566 + + marvell,caldata-txpwrlimit-5g-sub0: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: calibration data + maxItems: 502 + + marvell,caldata-txpwrlimit-5g-sub1: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: calibration data + maxItems: 688 + + marvell,caldata-txpwrlimit-5g-sub2: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: calibration data + maxItems: 750 + + marvell,caldata-txpwrlimit-5g-sub3: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: calibration data + maxItems: 502 + + marvell,wakeup-pin: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + a wakeup pin number of wifi chip which will be configured + to firmware. Firmware will wakeup the host using this pin + during suspend/resume. + + vmmc-supply: + description: a phandle of a regulator, supplying VCC to the card + + mmc-pwrseq: + description: + phandle to the MMC power sequence node. See "mmc-pwrseq-*" + for documentation of MMC power sequence bindings. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + mmc { + #address-cells = <1>; + #size-cells = <0>; + + wifi@1 { + compatible = "marvell,sd8897"; + reg = <1>; + interrupt-parent = <&pio>; + interrupts = <38 IRQ_TYPE_LEVEL_LOW>; + marvell,wakeup-pin = <3>; + }; + }; + diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt deleted file mode 100644 index cdc303caf5f45..0000000000000 --- a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt +++ /dev/null @@ -1,70 +0,0 @@ -Marvell 8787/8897/8978/8997 (sd8787/sd8897/sd8978/sd8997/pcie8997) SDIO/PCIE devices ------- - -This node provides properties for controlling the Marvell SDIO/PCIE wireless device. -The node is expected to be specified as a child node to the SDIO/PCIE controller that -connects the device to the system. - -Required properties: - - - compatible : should be one of the following: - * "marvell,sd8787" - * "marvell,sd8897" - * "marvell,sd8978" - * "marvell,sd8997" - * "nxp,iw416" - * "pci11ab,2b42" - * "pci1b4b,2b42" - -Optional properties: - - - marvell,caldata* : A series of properties with marvell,caldata prefix, - represent calibration data downloaded to the device during - initialization. This is an array of unsigned 8-bit values. - the properties should follow below property name and - corresponding array length: - "marvell,caldata-txpwrlimit-2g" (length = 566). - "marvell,caldata-txpwrlimit-5g-sub0" (length = 502). - "marvell,caldata-txpwrlimit-5g-sub1" (length = 688). - "marvell,caldata-txpwrlimit-5g-sub2" (length = 750). - "marvell,caldata-txpwrlimit-5g-sub3" (length = 502). - - marvell,wakeup-pin : a wakeup pin number of wifi chip which will be configured - to firmware. Firmware will wakeup the host using this pin - during suspend/resume. - - interrupts : interrupt pin number to the cpu. driver will request an irq based on - this interrupt number. during system suspend, the irq will be enabled - so that the wifi chip can wakeup host platform under certain condition. - during system resume, the irq will be disabled to make sure - unnecessary interrupt is not received. - - vmmc-supply: a phandle of a regulator, supplying VCC to the card - - mmc-pwrseq: phandle to the MMC power sequence node. See "mmc-pwrseq-*" - for documentation of MMC power sequence bindings. - -Example: - -Tx power limit calibration data is configured in below example. -The calibration data is an array of unsigned values, the length -can vary between hw versions. -IRQ pin 38 is used as system wakeup source interrupt. wakeup pin 3 is configured -so that firmware can wakeup host using this device side pin. - -&mmc3 { - vmmc-supply = <&wlan_en_reg>; - mmc-pwrseq = <&wifi_pwrseq>; - bus-width = <4>; - cap-power-off-card; - keep-power-in-suspend; - - #address-cells = <1>; - #size-cells = <0>; - mwifiex: wifi@1 { - compatible = "marvell,sd8897"; - reg = <1>; - interrupt-parent = <&pio>; - interrupts = <38 IRQ_TYPE_LEVEL_LOW>; - - marvell,caldata_00_txpwrlimit_2g_cfg_set = /bits/ 8 < - 0x01 0x00 0x06 0x00 0x08 0x02 0x89 0x01>; - marvell,wakeup-pin = <3>; - }; -};
Convert binding doc marvel-8xxx.txt to yaml format. Additional change: - Remove marvell,caldata_00_txpwrlimit_2g_cfg_set in example. - Remove mmc related property in example. Fix below warning: arch/arm64/boot/dts/freescale/imx8mp-beacon-kit.dtb: /soc@0/bus@30800000/mmc@30b40000/wifi@1: failed to match any schema with compatible: ['marvell,sd8997'] Signed-off-by: Frank Li <Frank.Li@nxp.com> --- .../bindings/net/wireless/marvell,8xxx.yaml | 96 +++++++++++++++++++ .../bindings/net/wireless/marvell-8xxx.txt | 70 -------------- 2 files changed, 96 insertions(+), 70 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/wireless/marvell,8xxx.yaml delete mode 100644 Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt