Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\))
Subject: Re: [BlueZ PATCH v1] doc:adding definitions for load default params
 mgmt op
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <20200504142625.213143-1-alainm@chromium.org>
Date:   Wed, 6 May 2020 13:22:18 +0200
Cc:     linux-bluetooth@vger.kernel.org
Content-Transfer-Encoding: 8BIT
Message-Id: <7610937E-424D-4968-B8D9-378E44773088@holtmann.org>
References: <20200504142625.213143-1-alainm@chromium.org>
To:     Alain Michaud <alainm@chromium.org>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Alain,

> This change adds the definition for the load default parameter command.
> In particular, this command is used to load default parameters for
> various operations in the kernel. This mechanism is also expandable to
> future knobs that may be necessary.
> 
> This will allow bluetoothd to load parameters from a conf file that may
> be customized for the specific requirements of each platforms.
> 
> ---
> 
> doc/mgmt-api.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 39f23c456..71d528706 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3137,6 +3137,65 @@ Read Security Information Command
> 	Possible errors:	Invalid Parameters
> 				Invalid Index
> 
> +Load Default Parameter Command
> +=============================

I think that Load Controller Default Parameters is a better name.

> +
> +	Command Code:		0x0049
> +	Controller Index:	<controller id>
> +	Command Parameters:	Parameter_Count (2 Octets)
> +				Parameter1 {
> +					Parameter_Type (2 Octet)
> +					Value_Length (1 Octet)
> +					Value (0-255 Octets)
> +				}
> +				Parameter2 { }
> +				...
> +	Return Parameters:
> +
> +	This command is used to feed the kernel a list of default parameters.
> +
> +	Currently defined Parameter_Type values are:
> +
> +			0x0000	BR/EDR Page Scan Type
> +			0x0001	BR/EDR Page Scan Interval
> +			0x0002	BR/EDR Page Scan Window
> +			0x0003	BR/EDR Inquiry Scan Type
> +			0x0004	BR/EDR Inquiry Scan Interval
> +			0x0005	BR/EDR Inquiry Scan Window
> +			0x0006	BR/EDR Link Supervision Timeout
> +			0x0007	BR/EDR Page Timeout
> +			0x0008	BR/EDR Min Sniff Interval
> +			0x0009	BR/EDR Max Sniff Interval
> +			0x0080	LE Advertisement Min Interval
> +			0x0081	LE Advertisement Max Interval
> +			0x0082	LE Multi Advertisement Rotation Interval
> +			0x0083	LE Scanning Interval for auto connect
> +			0x0084	LE Scanning Window for auto connect
> +			0x0085	LE Scanning Interval for HID only
> +			0x0086	LE Scanning Window for HID only
> +			0x0087	LE Scanning Interval for wake scenarios
> +			0x0088	LE Scanning Window for wake scenarios
> +			0x0089	LE Scanning Interval for discovery
> +			0x008a	LE Scanning Window for discovery
> +			0x008b	LE Scanning Interval for adv monitoring
> +			0x008c	LE Scanning Window for adv monitoring
> +			0x008d	LE Scanning Interval for connect
> +			0x008e	LE Scanning Window for connect
> +			0x008f	LE Min Connection Interval
> +			0x0090	LE Max Connection Interval
> +			0x0091	LE Connection Connection Latency
> +			0x0092	LE Connection Supervision Timeout

I would just enumerate these and not try to split between 0x000 and 0x008 for BR/EDR and LE. Also I would just start with a few values that we currently want to change.

> +
> +	This command can be used any time, but will not take effect until the
> +	next activity requiring the parameters.  In the case the parameters are
> +	used during initialization of the adapter, it is expected that the
> +	parameters be set before the adapter is powered.

Hmmm. This one, I might want to limit this controllers powered down. Otherwise you get into the conflict case that we would expect parameters taking affect and I think that is just too complicated.

> +
> +	This command generates a Command Complete event on success or
> +	a Command Status event on failure.
> +
> +	Possible errors:	Invalid Parameters
> +				Invalid Index
> 

Regards

Marcel