RE: [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding

[Bhatt, Avinash]

Hi Krzysztof,

Thank you for the detailed review. Please find our responses inline below.
We are actively addressing all comments and plan to send v3 shortly.

On Mon, May 04, 2026 at 12:53:25PM +0300, Krzysztof Kozlowski wrote:
> On Mon, May 04, 2026 at 12:53:25PM +0300, Avinash Bhatt wrote:
> > Add a devicetree schema binding for Intel discrete Wi-Fi 7 BE200 PCIe
> > adapters.
>
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> Just describe hardware.

Fixed in v3. Subject is now:
  "dt-bindings: net: wireless: Add Intel Wi-Fi 7 BE200 PCIe adapter"

> > The binding documents OEM platform configuration properties for
>
> No, describe hardware.

Fixed in v3. Commit body now describes the BE200 hardware directly.

> > +++ b/Documentation/devicetree/bindings/net/wireless/intel,iwlwifi.yaml
>
> Filename should match actual device compatibles. iwlwifi is driver, so
> not really relevant here.

Thank you for the guidance. Looking at the other bindings in the same
directory, we see two naming conventions coexist:

  - vendor,chip   - e.g. brcm,bcm4329-fmac.yaml, marvell,sd8787.yaml
  - vendor,driver - e.g. qca,ath9k.yaml, qcom,ath10k.yaml,
                    qcom,ath11k-pci.yaml, qcom,ath12k.yaml
                    (where ath9k, ath10k, ath11k, ath12k are driver
                    names, not chip names)

None of the existing wireless bindings use pciVVVV,DDDD.yaml as a
filename. Our intel,iwlwifi.yaml follows the vendor,driver pattern used
by the Qualcomm bindings. We also anticipate that future Intel Wi-Fi
devices - supported by the same iwlwifi driver - will use this same
binding, making the driver-aligned name forward-compatible.

Could you please point us to any guidance document that specifies
pciVVVV,DDDD.yaml as the required convention for PCIe device bindings,
or clarify whether intel,iwlwifi.yaml is acceptable given the above
precedent?

> > +title: Intel iwlwifi PCIe Wi-Fi devices
>
> There is no such thing as "iwlwifi" device. Again, describe hardware.

Fixed in v3. Title is now "Intel Wi-Fi 7 BE200 PCIe adapter".

> > +      Revision 3 layout: 4 chains x 12 subbands = 50 cells total.
> > +      Header (2 cells): ...
> > +      Subband index to frequency range mapping: ...
>
> Don't invent your own syntax. Drop. There are no other bindings using
> it, so why are you coming with something completely new?

Fixed in v3. The revision header cell and its const: 0 constraint have
been removed from all properties. Property descriptions remain as plain
prose bit and subband listings - the structured table headers with
revision fields and internal layout annotations are gone.

> > +        [0] revision - structure revision, must be 0x00
>
> What? If it is 0 then just drop it. Same comments everywhere else.

Fixed in v3. Revision cells removed from all properties. Each property
is now a scalar uint32 or a plain fixed-size array with no revision
header.

> > +        [1] value    - channel configuration:
> > +                       0 = active scan permitted
> > +                       1 = passive scan only
> > +                       2 = SRD channels fully disabled
>
> I have doubts this is regulatory/certification property.

intel,srd controls the operating mode of the ETSI 5.8 GHz Short Range
Device (SRD) band - whether the adapter performs active scanning,
passive scanning, or disables operation in that band entirely. This is a
platform-level OEM configuration: ETSI EN 301 893 and its country-level
transpositions impose per-country restrictions on the 5.8 GHz SRD
sub-band, and OEMs must set this property once per product to reflect
the regulatory certification of the platform. It is not a runtime driver
feature toggle - it is a system integrator constraint that determines
which regulatory mode the radio operates in for that band.

> > +      - items:
> > +          - const: 0
>
> Drop

Fixed in v3. Revision cell dropped. intel,srd is now a plain string
property.

> > +          - enum: [0, 1, 2]
>
> Use string

Fixed in v3. Changed to:
  enum: [active-scan, passive-scan, disabled]

> > +                       bit 26:    reserved, must be 0
>
> Bindings CANNOT be reserved.
>
> > +                       bit 28:    reserved, must be 0
>
> NAK

Fixed in v3. Removed all "reserved, must be 0" annotations. Bits 26 and
28 have no assigned meaning and are simply absent from the binding - the
binding only documents bits that are defined.

> > +        [0] revision - structure revision, must be 0x00  (regulatory-special)
>
> NAK

Fixed in v3. Revision cell removed.

> > +      - items:
> > +          - const: 0
>
> No. If it is const, then it is implied by compatible.

Fixed in v3. Removed const: 0 from all allOf/items across all
properties.

> > +required:
> > +  - compatible
> > +  - reg
>
> Missing ref to ieee80211 schema.

Fixed in v3. Added:
  allOf:
    - $ref: /schemas/net/wireless/ieee80211.yaml#

Best Regards,
Avinash

-----Original Message-----
From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> 
Sent: 05 May 2026 14:46
To: Bhatt, Avinash <avinash.bhatt@xxxxxxxxx>
Cc: devicetree@xxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; johannes@xxxxxxxxxxxxxxxx; Korenblit, Miriam Rachel <miriam.rachel.korenblit@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Guetta, Kobi <kobi.guetta@xxxxxxxxx>; Grumbach, Emmanuel <emmanuel.grumbach@xxxxxxxxx>
Subject: Re: [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding

On Mon, May 04, 2026 at 12:53:25PM +0300, Avinash Bhatt wrote:
> Add a devicetree schema binding for Intel discrete Wi-Fi 7 BE200 PCIe 
> adapters.

A nit, subject: drop second/last, redundant "binding". The "dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Just describe hardware.

> 
> The binding documents OEM platform configuration properties for

No, describe hardware.

> platforms that use Device Tree instead of platform firmware methods. 
> All properties mirror the existing equivalents in structure and 
> semantics, covering SAR power limits (intel,wrds),
> 6 GHz AP type support (intel,uats), static power limit (intel,splc), 
> channel puncturing (intel,wcpe), 320 MHz per-MCC enablement 
> (intel,wbem), ETSI SRD channel configuration (intel,srd), 6-7 GHz UHB 
> country enable bitmask (intel,6e-uhb), and additional regulatory 
> override properties.
> 
> 
> Signed-off-by: Avinash Bhatt <avinash.bhatt@xxxxxxxxx>
> ---
>  .../bindings/net/wireless/intel,iwlwifi.yaml  | 430 
> ++++++++++++++++++
>  1 file changed, 430 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/intel,iwlwifi.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/net/wireless/intel,iwlwifi.yaml 
> b/Documentation/devicetree/bindings/net/wireless/intel,iwlwifi.yaml
> new file mode 100644
> index 000000000000..210063c6183d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/intel,iwlwifi.yam
> +++ l

Filename should match actual device compatibles. iwlwifi is driver, so not really relevant here.

> @@ -0,0 +1,430 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright 
> +(c) 2026 Intel Corporation %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/intel,iwlwifi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel iwlwifi PCIe Wi-Fi devices


There is no such thing as "iwlwifi" device. Google for it - I see "Intel(r) Wireless Wi-Fi Drivers for Linux".

Again, describe hardware.


> +
> +maintainers:
> +  - Avinash Bhatt <avinash.bhatt@xxxxxxxxx>
> +
> +description:
> +  Intel iwlwifi IEEE 802.11be discrete Wi-Fi adapters connected over PCIe.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci8086,272b
> +
> +  reg:
> +    maxItems: 1
> +
> +  intel,wrds:
> +    description: |
> +      Wi-Fi Regulatory Domain Settings (WRDS). SAR (Specific Absorption Rate)
> +      transmit power limits per antenna chain and frequency subband. Values
> +      are 8-bit unsigned in units of 0.125 dBm.
> +
> +      Revision 3 layout: 4 chains x 12 subbands = 50 cells total.
> +      Chain A and Chain B are the two physical antenna paths; CDB Chain A
> +      and CDB Chain B carry separate limits for simultaneous dual-band
> +      operation.
> +
> +      Header (2 cells):
> +        [0] revision - structure revision, must be 0x03
> +        [1] mode     - bit 0: 0 = SAR disabled, 1 = SAR enabled;
> +                       bits [8:1]: set to 0
> +
> +      Followed by 4 chains in order: chain_a, chain_b, cdb_chain_a,
> +      cdb_chain_b, each containing 12 subband values:
> +
> +      Subband index to frequency range mapping:
> +        [0]  2.4 GHz  ch  1-13   (2412-2472 MHz)
> +        [1]  5 GHz    ch 36-64   (5180-5320 MHz, UNII-1/2)
> +        [2]  5 GHz    ch 68-96   (5340-5480 MHz, UNII-2)
> +        [3]  5 GHz    ch 100-144 (5500-5720 MHz, UNII-2e)
> +        [4]  5 GHz    ch 149-188 (5745-5940 MHz, UNII-3/4)
> +        [5]  6 GHz    ch  1-45   (5955-6175 MHz, UNII-5 lower)
> +        [6]  6 GHz    ch 49-93   (6195-6415 MHz, UNII-5 upper)
> +        [7]  6 GHz    ch 97-115  (6435-6525 MHz, UNII-6)
> +        [8]  6 GHz    ch 117-151 (6535-6705 MHz, UNII-7 lower)
> +        [9]  6 GHz    ch 153-183 (6715-6865 MHz, UNII-7 upper)
> +        [10] 6 GHz    ch 185-233 (6875-7115 MHz, UNII-8)
> +        [11] 6 GHz    ch 237-253 (7135-7215 MHz, UNII-9)
> +    allOf:

Don't invent your own syntax. Drop. There are no other bindings using it, so why are you coming with something completely new?


> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - minItems: 50
> +        maxItems: 50
> +
> +  intel,uats:
> +    description: |
> +      UHB (Ultra High Band / 6 GHz) AP Type Support (UATS). Per-country
> +      enablement matrix for 6 GHz AP types. Uses byte array format
> +      (DTS [ ... ] notation).
> +
> +      In the 6 GHz band, regulatory rules differ per country and per AP
> +      type: AFC (Standard Power), LPI (Low Power Indoor), and VLP (Very
> +      Low Power). This matrix encodes which AP types are permitted to
> +      operate in each country.
> +
> +      Revision 1 layout (339 bytes total):
> +        [0]   revision    - structure revision, must be 0x01
> +        [1+]  country_map - 338-byte matrix encoding AP type allowances
> +                            per country.
> +
> +                            Countries are identified by their ISO 3166-1
> +                            alpha-2 code (two letters, A-Z each). The
> +                            matrix covers all 26x26 = 676 possible
> +                            two-letter combinations (AA..ZZ), most of
> +                            which are unused (set to 0x0).
> +
> +                            Each country entry is 4 bits (a nibble). Two
> +                            entries are packed per byte: the low nibble
> +                            holds the even-indexed entry, the high nibble
> +                            holds the odd-indexed entry. For example,
> +                            byte value 0x53 means: entry[even]=0x3,
> +                            entry[odd]=0x5.
> +
> +                            The matrix is stored column-major by first
> +                            letter: all 26 second-letter variants for
> +                            first letter 'A' occupy bytes [0..12], then
> +                            first letter 'B' occupies bytes [13..25],
> +                            and so on for all 26 first letters.
> +                            26 columns x 13 bytes = 338 bytes total.
> +
> +                            Each 4-bit nibble encodes AP type allowances
> +                            for one country:
> +                              bit 0: AFC (Standard Power AP) allowed
> +                              bit 1: VLP (Very Low Power AP) allowed
> +                              bit 2: LPI (Low Power Indoor AP) allowed
> +                              bit 3: reserved, must be 0
> +
> +                            Note: each bit is only effective when the
> +                            corresponding control bit in intel,6e-uhb
> +                            is also set (bit 30 for AFC, bit 29
> +                            for VLP, bit 31 for LPI country-by-country
> +                            mode).
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint8-array
> +      - minItems: 339
> +        maxItems: 339
> +
> +  intel,srd:
> +    description: |
> +      ETSI 5.8 GHz SRD (Short Range Device) channel configuration.
> +      Controls how the driver handles the 5725-5875 MHz (5.8 GHz) SRD
> +      channels in ETSI regulatory domains.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00

What? If it is 0 then just drop it. Same comments everywhere else.

> +        [1] value    - channel configuration:
> +                       0 = active scan permitted (default behaviour)
> +                       1 = passive scan only; device may associate and
> +                           transfer data but must not transmit probe
> +                           requests on SRD channels
> +                       2 = SRD channels fully disabled; the device must
> +                           not scan, associate, or operate on any of the
> +                           5725-5875 MHz SRD channels

I have doubts this is regulatory/certification property.

> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0

Drop

> +          - enum: [0, 1, 2]

Use string

> +
> +  intel,6e-uhb:
> +    description: |
> +      6-7 GHz Ultra-High Band (UHB) per-country enable bitmask.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00
> +        [1] bitmap   - UHB enablement control:
> +                       bit 0:     override control; 0 = use device defaults,
> +                                  1 = force-disable all countries not
> +                                  explicitly enabled in bits 1-25
> +                       bits 1-25: per-country/region enable flags:
> +                                  bit  1 = USA
> +                                  bit  2 = Rest of World (ROW)
> +                                  bit  3 = EU
> +                                  bit  4 = South Korea
> +                                  bit  5 = Brazil
> +                                  bit  6 = Chile
> +                                  bit  7 = Japan
> +                                  bit  8 = Canada
> +                                  bit  9 = Morocco
> +                                  bit 10 = Mongolia
> +                                  bit 11 = Malaysia
> +                                  bit 12 = Saudi Arabia
> +                                  bit 13 = Mexico
> +                                  bit 14 = Nigeria
> +                                  bit 15 = Thailand
> +                                  bit 16 = Singapore
> +                                  bit 17 = Taiwan
> +                                  bit 18 = South Africa
> +                                  bit 19 = Philippines
> +                                  bit 20 = Serbia
> +                                  bit 21 = Indonesia
> +                                  bit 22 = Azerbaijan
> +                                  bit 23 = Paraguay
> +                                  bit 24 = Vietnam
> +                                  bit 25 = India
> +                       bit 26:    reserved, must be 0

Bindings CANNOT be reserved.

> +                       bit 27:    enable VLP active scan, SoftAP, and
> +                                  P2P-GO operation in Japan
> +                       bit 28:    reserved, must be 0

NAK

> +                       bit 29:    enable VLP (Very Low Power) mode per
> +                                  country-by-country table
> +                       bit 30:    enable AFC (Standard Power) mode per
> +                                  country-by-country table
> +                       bit 31:    LPI override mode; 0 = use grouping
> +                                  mechanism, 1 = use country-by-country table
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0

No. If it is const, then it is implied by compatible.

> +          - {}
> +
> +  intel,regulatory-special:
> +    description: |
> +      Regulatory Special Configurations.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00

NAK

> +        [1] bitmap   - configuration flags:
> +                       bits 0-3: reserved, must be 0
> +                       bit 4 = Australia UHB extension
> +                       bits 5-31: reserved, must be 0
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +  intel,activate-channel:
> +    description: |
> +      Indoor channel activation bitmask. Sets specific frequency bands to
> +      active (rather than passive or disabled) when the platform is
> +      confirmed to be operating indoors.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00
> +        [1] bitmap   - per-region indoor activation flags:
> +                       bit 0 = enable EU U-NII-1 (5.2 GHz) for indoors only
> +                       bit 1 = enable Japan U-NII-1 (5.2 GHz) for indoors only
> +                       bit 2 = enable China Mainland U-NII-1 (5.2 GHz)
> +                               for indoors only
> +                       bit 3 = enable USA U-NII-4 (5.9 GHz) for indoors only
> +                       bit 4 = enable WW U-NII-1 (5.2 GHz) for indoors in any
> +                               country where the band is permitted
> +                       bit 5 = enable Canada U-NII-4 (5.9 GHz) for indoors only
> +                       bit 6 = enable USA + Canada + WW U-NII-4 (5.9 GHz) for
> +                               indoors only
> +                       bits 7-31: reserved, must be 0
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +  intel,force-disable-channels:
> +    description: |
> +      Selective Wi-Fi band force-disable bitmask. Allows the platform to
> +      permanently disable specific frequency bands regardless of regulatory
> +      domain.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00
> +        [1] bitmap   - per-band force-disable flags:
> +                       bit 0  = force disable 2.4 GHz (channels 1-13)
> +                       bit 1  = force disable 5.2 GHz (channels 36-48)
> +                       bit 2  = force disable 5.3 GHz (channels 52-64)
> +                       bit 3  = force disable 5.5 GHz (channels 100-144)
> +                       bit 4  = force disable 5.8 GHz (channels 149-165)
> +                       bit 5  = force disable 5.9 GHz (channels 169-177)
> +                       bit 6  = force disable 6.2 GHz (channels 1-93)
> +                       bit 7  = force disable 6.5 GHz (channels 97-113)
> +                       bit 8  = force disable 6.6 GHz (channels 117-153)
> +                       bit 9  = force disable 6.8 GHz (channels 157-185)
> +                       bit 10 = force disable 7.0 GHz (channels 185-233)
> +                       bits 11-31: reserved, must be 0
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +  intel,11be:
> +    description: |
> +      802.11be (Wi-Fi 7) per-country enable bitmask. Controls whether
> +      802.11be operation is permitted in specific countries.
> +
> +      Layout (2 cells):
> +        [0] revision - structure revision, must be 0x00
> +        [1] bitmap   - per-country enable flags:
> +                       bit 0 = enable 802.11be in China (CB/CN)
> +                       bit 1 = enable 802.11be in South Korea
> +                       bits 2-31: reserved, must be 0
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +  intel,splc:
> +    description: |
> +      Wi-Fi Static Power Limit Capabilities (SPLC). Sets the platform thermal
> +      power limit for the Wi-Fi core in mW. Omit this property entirely if
> +      no platform power limit applies; the device will use its certified
> +      maximum in that case.
> +
> +      Layout (2 cells):
> +        [0] revision    - structure revision, must be 0x00
> +        [1] power_limit - maximum platform power budget in mW, must be
> +                          non-zero (a zero value is equivalent to omitting
> +                          the property)
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - minimum: 1
> +
> +  intel,wcpe:
> +    description: |
> +      Wi-Fi Channel Puncturing Enablement (WCPE). Enables 802.11be channel
> +      puncturing for specific regulatory domains.
> +
> +      Layout (2 cells):
> +        [0] revision   - structure revision, must be 0x00
> +        [1] puncturing - per-country enable bitmask:
> +                         bit 0: 1 = channel puncturing enabled for USA
> +                         bit 1: 1 = channel puncturing enabled for Canada
> +                         bits 2-31: reserved, must be 0
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +  intel,wbem:
> +    description: |
> +      Wi-Fi 320 MHz Bandwidth Enablement per MCC (WBEM). Controls whether
> +      320 MHz operation is permitted in specific countries.
> +
> +      Layout (2 cells):
> +        [0] revision       - structure revision, must be 0x00
> +        [1] wifi320mhz_mcc - per-country enable bitmask:
> +                             bit 0: 1 = 320 MHz enabled for Japan
> +                             bit 1: 1 = 320 MHz enabled for South Korea
> +                             bits 2-31: reserved, must be 0
> +
> +                             Each bit takes effect only if the installed
> +                             module is certified for 320 MHz in that country.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          - const: 0
> +          - {}
> +
> +required:
> +  - compatible
> +  - reg
> +

Missing ref to ieee80211 schema.


> +additionalProperties: false

Best regards,
Krzysztof