The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding
       [not found] ` <20260504095327.30892-2-avinash.bhatt@intel.com>
@ 2026-05-05  9:15   ` Krzysztof Kozlowski
  2026-05-12  9:59     ` Bhatt, Avinash
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-05  9:15 UTC (permalink / raw)
  To: Avinash Bhatt
  Cc: devicetree, linux-wireless, robh, krzk+dt, conor+dt, johannes,
	miriam.rachel.korenblit, linux-kernel, kobi.guetta,
	emmanuel.grumbach

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@intel.com>
> ---
>  .../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.yaml

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® Wireless Wi-Fi Drivers for Linux".

Again, describe hardware.


> +
> +maintainers:
> +  - Avinash Bhatt <avinash.bhatt@intel.com>
> +
> +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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure
       [not found] ` <20260504095327.30892-3-avinash.bhatt@intel.com>
@ 2026-05-05  9:18   ` Krzysztof Kozlowski
  2026-05-12 10:03     ` Bhatt, Avinash
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-05  9:18 UTC (permalink / raw)
  To: Avinash Bhatt
  Cc: devicetree, linux-wireless, robh, krzk+dt, conor+dt, johannes,
	miriam.rachel.korenblit, linux-kernel, kobi.guetta,
	emmanuel.grumbach

On Mon, May 04, 2026 at 12:53:26PM +0300, Avinash Bhatt wrote:
> +
> +/*
> + * Mapping from DSM function index to Device Tree property name.
> + * Returns the DT property name for a given DSM function, or NULL if the
> + * function has no Device Tree representation.
> + */
> +static const char *dsm_func_to_prop_name(enum iwl_dsm_funcs func)
> +{
> +	switch (func) {
> +	case DSM_FUNC_DISABLE_SRD:          return IWL_DT_PROP_SRD;
> +	case DSM_FUNC_ENABLE_6E:            return IWL_DT_PROP_6E_UHB;
> +	case DSM_FUNC_REGULATORY_CONFIG:    return IWL_DT_PROP_REG_SPECIAL;
> +	case DSM_FUNC_ACTIVATE_CHANNEL:     return IWL_DT_PROP_ACTIVATE_CH;
> +	case DSM_FUNC_FORCE_DISABLE_CHANNELS:
> +		return IWL_DT_PROP_FORCE_DISABLE_CH;
> +	case DSM_FUNC_ENABLE_11BE:          return IWL_DT_PROP_11BE;
> +	default:                            return NULL;

Pointless function and only making DT ABI checks difficult. Drop.

Don't invent API wrappers or other HAL over simple calls to OF or device
API.

Drop also ALL defines and use OF API like EVERY other driver. This is
not a special place.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information
       [not found] <20260504095327.30892-1-avinash.bhatt@intel.com>
       [not found] ` <20260504095327.30892-2-avinash.bhatt@intel.com>
       [not found] ` <20260504095327.30892-3-avinash.bhatt@intel.com>
@ 2026-05-05  9:19 ` Krzysztof Kozlowski
  2026-05-12 10:07   ` Bhatt, Avinash
  2 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-05  9:19 UTC (permalink / raw)
  To: Avinash Bhatt
  Cc: devicetree, linux-wireless, robh, krzk+dt, conor+dt, johannes,
	miriam.rachel.korenblit, linux-kernel, kobi.guetta,
	emmanuel.grumbach

On Mon, May 04, 2026 at 12:53:24PM +0300, Avinash Bhatt wrote:
> Add Device Tree support for Intel Wi-Fi hardware integration information
> on platforms that do not provide UEFI variables or ACPI methods.
> 
> Patch 1/3 adds the DT binding schema for the Intel iwlwifi compatible
> node. Patches 2/3 and 3/3 add the driver infrastructure and integrate DT
> as the lowest-priority fallback after UEFI and ACPI.

Please provide link to any upstream DTS user of this binding, either
complete or work in progress.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding
  2026-05-05  9:15   ` [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding Krzysztof Kozlowski
@ 2026-05-12  9:59     ` Bhatt, Avinash
  0 siblings, 0 replies; 8+ messages in thread
From: Bhatt, Avinash @ 2026-05-12  9:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree@vger.kernel.org, linux-wireless@vger.kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	johannes@sipsolutions.net, Korenblit, Miriam Rachel,
	linux-kernel@vger.kernel.org, Guetta, Kobi, Grumbach, Emmanuel

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@kernel.org> 
Sent: 05 May 2026 14:46
To: Bhatt, Avinash <avinash.bhatt@intel.com>
Cc: devicetree@vger.kernel.org; linux-wireless@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; johannes@sipsolutions.net; Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux-kernel@vger.kernel.org; Guetta, Kobi <kobi.guetta@intel.com>; Grumbach, Emmanuel <emmanuel.grumbach@intel.com>
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@intel.com>
> ---
>  .../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@intel.com>
> +
> +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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure
  2026-05-05  9:18   ` [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure Krzysztof Kozlowski
@ 2026-05-12 10:03     ` Bhatt, Avinash
  2026-05-13 18:16       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Bhatt, Avinash @ 2026-05-12 10:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree@vger.kernel.org, linux-wireless@vger.kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	johannes@sipsolutions.net, Korenblit, Miriam Rachel,
	linux-kernel@vger.kernel.org, Guetta, Kobi, Grumbach, Emmanuel

Hi Krzysztof,

Thank you for the review. Please find our response inline below.
We have made several changes in the driver as part of v3 and will be
sending it shortly.

On Mon, May 04, 2026 at 12:53:26PM +0300, Krzysztof Kozlowski wrote:
> +static const char *dsm_func_to_prop_name(enum iwl_dsm_funcs func) {
> +	switch (func) {
> +	case DSM_FUNC_DISABLE_SRD:          return IWL_DT_PROP_SRD;
> +	case DSM_FUNC_ENABLE_6E:            return IWL_DT_PROP_6E_UHB;
> ...
>
> Pointless function and only making DT ABI checks difficult. Drop.
> Don't invent API wrappers or other HAL over simple calls to OF or
> device API.
> Drop also ALL defines and use OF API like EVERY other driver.

Fixed in v3. Removed dsm_func_to_prop_name() and all IWL_DT_PROP_*
defines. iwl_dt_get_dsm() now dispatches via a direct switch on the
function index to individual per-property functions (iwl_dt_get_srd(),
iwl_dt_get_6e_uhb(), etc.), each calling of_property_read_*() directly
with the literal property string — consistent with how every other
driver uses the OF API.

Best Regards,
Avinash

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org> 
Sent: 05 May 2026 14:48
To: Bhatt, Avinash <avinash.bhatt@intel.com>
Cc: devicetree@vger.kernel.org; linux-wireless@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; johannes@sipsolutions.net; Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux-kernel@vger.kernel.org; Guetta, Kobi <kobi.guetta@intel.com>; Grumbach, Emmanuel <emmanuel.grumbach@intel.com>
Subject: Re: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure

On Mon, May 04, 2026 at 12:53:26PM +0300, Avinash Bhatt wrote:
> +
> +/*
> + * Mapping from DSM function index to Device Tree property name.
> + * Returns the DT property name for a given DSM function, or NULL if 
> +the
> + * function has no Device Tree representation.
> + */
> +static const char *dsm_func_to_prop_name(enum iwl_dsm_funcs func) {
> +	switch (func) {
> +	case DSM_FUNC_DISABLE_SRD:          return IWL_DT_PROP_SRD;
> +	case DSM_FUNC_ENABLE_6E:            return IWL_DT_PROP_6E_UHB;
> +	case DSM_FUNC_REGULATORY_CONFIG:    return IWL_DT_PROP_REG_SPECIAL;
> +	case DSM_FUNC_ACTIVATE_CHANNEL:     return IWL_DT_PROP_ACTIVATE_CH;
> +	case DSM_FUNC_FORCE_DISABLE_CHANNELS:
> +		return IWL_DT_PROP_FORCE_DISABLE_CH;
> +	case DSM_FUNC_ENABLE_11BE:          return IWL_DT_PROP_11BE;
> +	default:                            return NULL;

Pointless function and only making DT ABI checks difficult. Drop.

Don't invent API wrappers or other HAL over simple calls to OF or device API.

Drop also ALL defines and use OF API like EVERY other driver. This is not a special place.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information
  2026-05-05  9:19 ` [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information Krzysztof Kozlowski
@ 2026-05-12 10:07   ` Bhatt, Avinash
  2026-05-13 18:18     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 8+ messages in thread
From: Bhatt, Avinash @ 2026-05-12 10:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: devicetree@vger.kernel.org, linux-wireless@vger.kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	johannes@sipsolutions.net, Korenblit, Miriam Rachel,
	linux-kernel@vger.kernel.org, Guetta, Kobi, Grumbach, Emmanuel

Hi Krzysztof,

Thank you for the review. Please find our response inline below.

On Mon, May 04, 2026 at 12:53:24PM +0300, Krzysztof Kozlowski wrote:
> Please provide link to any upstream DTS user of this binding, either
> complete or work in progress.

We have OEM partners waiting for this binding layout to be finalized
before they proceed with their DTS work. Our intent is to have the
schema reviewed and accepted by the DT maintainers first — sharing a
binding that may still undergo structural changes with OEMs would create
unnecessary churn on their side.

OEM partners are targeting platforms that depend on this binding. They
are waiting for the schema to be finalized before proceeding with their
DTS work, and whether they upstream that DTS is ultimately their
decision.

If a DTS user is strictly required for the binding to be merged, we
understand and accept that requirement — however, since OEM upstreaming
is outside our control, we would greatly appreciate an early indication
of whether the schema direction is acceptable before we distribute it
further. Any feedback or provisional acceptance at this stage would be
very helpful.

We are actively addressing all the review comments from v2 and plan to
send v3 shortly.

Best Regards,
Avinash

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org> 
Sent: 05 May 2026 14:50
To: Bhatt, Avinash <avinash.bhatt@intel.com>
Cc: devicetree@vger.kernel.org; linux-wireless@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; johannes@sipsolutions.net; Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux-kernel@vger.kernel.org; Guetta, Kobi <kobi.guetta@intel.com>; Grumbach, Emmanuel <emmanuel.grumbach@intel.com>
Subject: Re: [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information

On Mon, May 04, 2026 at 12:53:24PM +0300, Avinash Bhatt wrote:
> Add Device Tree support for Intel Wi-Fi hardware integration 
> information on platforms that do not provide UEFI variables or ACPI methods.
> 
> Patch 1/3 adds the DT binding schema for the Intel iwlwifi compatible 
> node. Patches 2/3 and 3/3 add the driver infrastructure and integrate 
> DT as the lowest-priority fallback after UEFI and ACPI.

Please provide link to any upstream DTS user of this binding, either complete or work in progress.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure
  2026-05-12 10:03     ` Bhatt, Avinash
@ 2026-05-13 18:16       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-13 18:16 UTC (permalink / raw)
  To: Bhatt, Avinash
  Cc: devicetree@vger.kernel.org, linux-wireless@vger.kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	johannes@sipsolutions.net, Korenblit, Miriam Rachel,
	linux-kernel@vger.kernel.org, Guetta, Kobi, Grumbach, Emmanuel

On 12/05/2026 12:03, Bhatt, Avinash wrote:
> Hi Krzysztof,
> 
> Thank you for the review. Please find our response inline below.
> We have made several changes in the driver as part of v3 and will be
> sending it shortly.
> 
> On Mon, May 04, 2026 at 12:53:26PM +0300, Krzysztof Kozlowski wrote:
>> +static const char *dsm_func_to_prop_name(enum iwl_dsm_funcs func) {
>> +	switch (func) {
>> +	case DSM_FUNC_DISABLE_SRD:          return IWL_DT_PROP_SRD;
>> +	case DSM_FUNC_ENABLE_6E:            return IWL_DT_PROP_6E_UHB;
>> ...
>>
>> Pointless function and only making DT ABI checks difficult. Drop.
>> Don't invent API wrappers or other HAL over simple calls to OF or
>> device API.
>> Drop also ALL defines and use OF API like EVERY other driver.
> 
> Fixed in v3. Removed dsm_func_to_prop_name() and all IWL_DT_PROP_*
> defines. iwl_dt_get_dsm() now dispatches via a direct switch on the
> function index to individual per-property functions (iwl_dt_get_srd(),
> iwl_dt_get_6e_uhb(), etc.), each calling of_property_read_*() directly
> with the literal property string — consistent with how every other
> driver uses the OF API.
> 
> Best Regards,
> Avinash
> 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org> 
> Sent: 05 May 2026 14:48
> To: Bhatt, Avinash <avinash.bhatt@intel.com>
> Cc: devicetree@vger.kernel.org; linux-wireless@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; johannes@sipsolutions.net; Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux-kernel@vger.kernel.org; Guetta, Kobi <kobi.guetta@intel.com>; Grumbach, Emmanuel <emmanuel.grumbach@intel.com>
> Subject: Re: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure
> 
> On Mon, May 04, 2026 at 12:53:26PM +0300, Avinash Bhatt wrote:
>> +
>> +/*
>> + * Mapping from DSM function index to Device Tree property name.
>> + * Returns the DT property name for a given DSM function, or NULL if 
>> +the
>> + * function has no Device Tree representation.
>> + */
>> +static const char *dsm_func_to_prop_name(enum iwl_dsm_funcs func) {
>> +	switch (func) {
>> +	case DSM_FUNC_DISABLE_SRD:          return IWL_DT_PROP_SRD;
>> +	case DSM_FUNC_ENABLE_6E:            return IWL_DT_PROP_6E_UHB;
>> +	case DSM_FUNC_REGULATORY_CONFIG:    return IWL_DT_PROP_REG_SPECIAL;
>> +	case DSM_FUNC_ACTIVATE_CHANNEL:     return IWL_DT_PROP_ACTIVATE_CH;
>> +	case DSM_FUNC_FORCE_DISABLE_CHANNELS:
>> +		return IWL_DT_PROP_FORCE_DISABLE_CH;
>> +	case DSM_FUNC_ENABLE_11BE:          return IWL_DT_PROP_11BE;
>> +	default:                            return NULL;
> 
> Pointless function and only making DT ABI checks difficult. Drop.
> 
> Don't invent API wrappers or other HAL over simple calls to OF or device API.
> 
> Drop also ALL defines and use OF API like EVERY other driver. This is not a special place.
> 

What is this style of answering? Am I replying now to my own email?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information
  2026-05-12 10:07   ` Bhatt, Avinash
@ 2026-05-13 18:18     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-13 18:18 UTC (permalink / raw)
  To: Bhatt, Avinash
  Cc: devicetree@vger.kernel.org, linux-wireless@vger.kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	johannes@sipsolutions.net, Korenblit, Miriam Rachel,
	linux-kernel@vger.kernel.org, Guetta, Kobi, Grumbach, Emmanuel

On 12/05/2026 12:07, Bhatt, Avinash wrote:
> Hi Krzysztof,
> 
> Thank you for the review. Please find our response inline below.
> 
> On Mon, May 04, 2026 at 12:53:24PM +0300, Krzysztof Kozlowski wrote:
>> Please provide link to any upstream DTS user of this binding, either
>> complete or work in progress.
> 
> We have OEM partners waiting for this binding layout to be finalized
> before they proceed with their DTS work. Our intent is to have the
> schema reviewed and accepted by the DT maintainers first — sharing a
> binding that may still undergo structural changes with OEMs would create
> unnecessary churn on their side.
> 
> OEM partners are targeting platforms that depend on this binding. They
> are waiting for the schema to be finalized before proceeding with their
> DTS work, and whether they upstream that DTS is ultimately their
> decision.
> 
> If a DTS user is strictly required for the binding to be merged, we
> understand and accept that requirement — however, since OEM upstreaming
> is outside our control, we would greatly appreciate an early indication
> of whether the schema direction is acceptable before we distribute it
> further. Any feedback or provisional acceptance at this stage would be
> very helpful.
> 
> We are actively addressing all the review comments from v2 and plan to
> send v3 shortly.
> 
> Best Regards,
> Avinash
> 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org> 
> Sent: 05 May 2026 14:50
> To: Bhatt, Avinash <avinash.bhatt@intel.com>
> Cc: devicetree@vger.kernel.org; linux-wireless@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; johannes@sipsolutions.net; Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux-kernel@vger.kernel.org; Guetta, Kobi <kobi.guetta@intel.com>; Grumbach, Emmanuel <emmanuel.grumbach@intel.com>
> Subject: Re: [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information
> 
> On Mon, May 04, 2026 at 12:53:24PM +0300, Avinash Bhatt wrote:
>> Add Device Tree support for Intel Wi-Fi hardware integration 
>> information on platforms that do not provide UEFI variables or ACPI methods.
>>
>> Patch 1/3 adds the DT binding schema for the Intel iwlwifi compatible 
>> node. Patches 2/3 and 3/3 add the driver infrastructure and integrate 
>> DT as the lowest-priority fallback after UEFI and ACPI.
> 
> Please provide link to any upstream DTS user of this binding, either complete or work in progress.
> 
> Best regards,
> Krzysztof

Do not top post.

If you are not willing to post DTS, then you are not helping us to
understand the completeness of this binding and hardware description.
IOW, the bigger picture is missing, so IMO I would remove most of the
properties.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-13 18:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260504095327.30892-1-avinash.bhatt@intel.com>
     [not found] ` <20260504095327.30892-2-avinash.bhatt@intel.com>
2026-05-05  9:15   ` [PATCH v2 1/3] dt-bindings: net: wireless: intel,iwlwifi: add binding Krzysztof Kozlowski
2026-05-12  9:59     ` Bhatt, Avinash
     [not found] ` <20260504095327.30892-3-avinash.bhatt@intel.com>
2026-05-05  9:18   ` [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure Krzysztof Kozlowski
2026-05-12 10:03     ` Bhatt, Avinash
2026-05-13 18:16       ` Krzysztof Kozlowski
2026-05-05  9:19 ` [PATCH v2 0/3] wifi: iwlwifi: add Device Tree hardware integration information Krzysztof Kozlowski
2026-05-12 10:07   ` Bhatt, Avinash
2026-05-13 18:18     ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox