Hi Marcel,
On Wed, May 13, 2020 at 3:49 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> 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.
> > I wanted to avoid using "controller" in the name as some of these are
> > not really "controller" parameters but instead just general
> > parameters.
>
> fair enough. Now the question is if we might want to split between controller defaults and host defaults.
>
> >>> +
> >>> + 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.
> > The goal was to try to keep BR and LE parameters together, but if you
> > prefer, I don't mind collapsing them. Note that we intend to use all
> > of these, so I'd prefer not break this into a smaller subset.
>
> My OCD wants to do that as well, but my experience has been that this never actually works out. And we will have parameters that apply to both transports. Like for example the minimum encryption key size.
>
> >>> +
> >>> + 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.
> > Things like connection intervals and scan defaults actually apply the
> > next time the activity is started. We need to be able to change these
> > without completely powering down the controller and tearing down all
> > connections etc. Things that are obviously only used during
> > initialization, then I would expect they would require a power cycles.
>
> Now we might actually better split this into a command for system default parameters and one for runtime default parameters. I think it would make the code a lot simpler since we can have clear error codes. Thoughts?
That's a good idea. Any recommendation on naming for the runtime
ones? I guess it would also need to carry the name "update" of some
sort.
>
>
> Regards
>
> Marcel
>
