RE: [PATCH v2 2/3] wifi: iwlwifi: dt: add Device Tree BIOS configuration infrastructure

[Bhatt, Avinash]

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@xxxxxxxxxx> 
Sent: 05 May 2026 14:48
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 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