Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: kvalo=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.codeaurora.org 6B244C4338F
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Cc:     linux-wireless@vger.kernel.org
Subject: Re: [PATCH v6 5/7] nl80211: vendor-cmd: add Intel vendor commands for iwlmei usage
References: <20210811105541.43426-1-emmanuel.grumbach@intel.com>
        <20210811105541.43426-5-emmanuel.grumbach@intel.com>
Date:   Mon, 18 Oct 2021 14:16:18 +0300
In-Reply-To: <20210811105541.43426-5-emmanuel.grumbach@intel.com> (Emmanuel
        Grumbach's message of "Wed, 11 Aug 2021 13:55:39 +0300")
Message-ID: <87h7devb0d.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Emmanuel Grumbach <emmanuel.grumbach@intel.com> writes:

> iwlmei allows to integrate with the CSME firmware. There are
> flows that are prioprietary for this purpose:
>
> * Get the information of the AP the CSME firmware is connected
>   to. This is useful when we need to speed up the connection
>   process in case the CSME firmware has a TCP connection
>   that must be kept alive across the ownership transition.
> * Forbid roaming, which will happen when the CSME firmware
>   wants to tell the user space not disrupt the connection.
> * Request ownership, upon driver boot when the CSME firmware
>   owns the device. This is a notification sent by the kernel.
>
> All those commands are expected to be used by any software
> managing the connection (mainly NetworkManager). Those commands
> are expected to be used only in case the CSME firmware owns
> the device and doesn't want to release the device unless the
> host made sure that it can keep the connectivity.

[...]

> --- /dev/null
> +++ b/include/uapi/linux/nl80211-vnd-intel.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2012-2014, 2018-2021 Intel Corporation
> + * Copyright (C) 2013-2015 Intel Mobile Communications GmbH
> + * Copyright (C) 2016-2017 Intel Deutschland GmbH
> + */
> +#ifndef __VENDOR_CMD_INTEL_H__
> +#define __VENDOR_CMD_INTEL_H__
> +
> +#define INTEL_OUI	0x001735
> +
> +/**
> + * enum iwl_mvm_vendor_cmd - supported vendor commands
> + * @IWL_MVM_VENDOR_CMD_GET_CSME_CONN_INFO: reports CSME connection info.
> + * @IWL_MVM_VENDOR_CMD_HOST_GET_OWNERSHIP: asks for ownership on the device.
> + * @IWL_MVM_VENDOR_CMD_ROAMING_FORBIDDEN_EVENT: notifies if roaming is allowed.
> + *	It contains a &IWL_MVM_VENDOR_ATTR_ROAMING_FORBIDDEN and a
> + *	&IWL_MVM_VENDOR_ATTR_VIF_ADDR attributes.
> + */
> +
> +enum iwl_mvm_vendor_cmd {
> +	IWL_MVM_VENDOR_CMD_GET_CSME_CONN_INFO			= 0x2d,
> +	IWL_MVM_VENDOR_CMD_HOST_GET_OWNERSHIP			= 0x30,
> +	IWL_MVM_VENDOR_CMD_ROAMING_FORBIDDEN_EVENT		= 0x32,
> +};

If I were trying to implement a user space application based on the
documentation in nl80211-vnd-intel.h I would not get far. For example,
how is IWL_MVM_VENDOR_CMD_HOST_GET_OWNERSHIP supposed to be used? The
documentation just repeats what the command already implies.

You had some good info in the commit log, I recommend at least copying
the most important part to the .h file.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches