All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Alain Michaud <alainm@chromium.org>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [BlueZ PATCH v3 2/4] adapter:set default system configuration if available
Date: Fri, 29 May 2020 10:05:19 -0700	[thread overview]
Message-ID: <CABBYNZ+DmKdwepmYAaKjEJ5qWsK6NnTMp9JEamej0BgUGJcRCg@mail.gmail.com> (raw)
In-Reply-To: <20200529153814.213125-3-alainm@chromium.org>

Hi Alain,

On Fri, May 29, 2020 at 8:41 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This change loads any specified system configuration if provided and
> supported by the kernel.
>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  src/adapter.c | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/hcid.h    |  39 ++++++++
>  2 files changed, 290 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 972d88772..da3777cba 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -120,6 +120,8 @@ static bool kernel_conn_control = false;
>
>  static bool kernel_blocked_keys_supported = false;
>
> +static bool kernel_set_system_params = false;
> +
>  static GList *adapter_list = NULL;
>  static unsigned int adapter_remaining = 0;
>  static bool powering_down = false;
> @@ -4157,6 +4159,250 @@ static void probe_devices(void *user_data)
>         device_probe_profiles(device, btd_device_get_uuids(device));
>  }
>
> +static void load_default_system_params(struct btd_adapter *adapter)
> +{
> +       /* note: for now all params are 2 bytes, if varying size params are
> +        * added, calculating the right size of buffer will be necessary first.
> +        */
> +       struct controller_params_tlv {
> +               uint16_t parameter_type;
> +               uint8_t length;
> +               uint16_t value;
> +       } __packed;

Don't we have a definition of the tlv entry on mgmt.h? Id use that
instead of redefining it here.

> +       struct controller_params_tlv *params = NULL;
> +       uint16_t i = 0;
> +       size_t cp_size;
> +
> +       if (!main_opts.default_params.num_set_params ||
> +               !kernel_set_system_params)
> +               return;
> +
> +       cp_size = sizeof(struct controller_params_tlv) *
> +                               main_opts.default_params.num_set_params;
> +       params = g_try_malloc0(cp_size);

For new code we prefer to use new0 instead of glib allocation.

> +       if (!params)
> +               return;
> +
> +       if (main_opts.default_params.br_page_scan_type != 0xFFFF) {
> +               params[i].parameter_type = 0x0000;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.br_page_scan_type;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_page_scan_interval) {
> +               params[i].parameter_type = 0x0001;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                               main_opts.default_params.br_page_scan_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_page_scan_window) {
> +               params[i].parameter_type = 0x0002;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.br_page_scan_window;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_inquiry_scan_type != 0xFFFF) {
> +               params[i].parameter_type = 0x0003;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.br_inquiry_scan_type;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_inquiry_scan_interval) {
> +               params[i].parameter_type = 0x0004;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.br_inquiry_scan_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_inquiry_scan_window) {
> +               params[i].parameter_type = 0x0005;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                               main_opts.default_params.br_inquiry_scan_window;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_link_supervision_timeout) {
> +               params[i].parameter_type = 0x0006;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.br_link_supervision_timeout;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_page_timeout) {
> +               params[i].parameter_type = 0x0007;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.br_page_timeout;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_min_sniff_interval) {
> +               params[i].parameter_type = 0x0008;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                               main_opts.default_params.br_min_sniff_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.br_max_sniff_interval) {
> +               params[i].parameter_type = 0x0009;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                               main_opts.default_params.br_max_sniff_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_min_adv_interval) {
> +               params[i].parameter_type = 0x000a;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.le_min_adv_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_max_adv_interval) {
> +               params[i].parameter_type = 0x000b;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.le_max_adv_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_multi_adv_rotation_interval) {
> +               params[i].parameter_type = 0x000c;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_multi_adv_rotation_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_interval_autoconnect) {
> +               params[i].parameter_type = 0x000d;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_interval_autoconnect;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_window_autoconnect) {
> +               params[i].parameter_type = 0x000e;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_window_autoconnect;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_interval_suspend) {
> +               params[i].parameter_type = 0x000f;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_interval_suspend;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_window_suspend) {
> +               params[i].parameter_type = 0x0010;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_window_suspend;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_interval_discovery) {
> +               params[i].parameter_type = 0x0011;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_interval_discovery;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_window_discovery) {
> +               params[i].parameter_type = 0x0012;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_window_discovery;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_interval_adv_monitor) {
> +               params[i].parameter_type = 0x0013;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_interval_adv_monitor;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_window_adv_monitor) {
> +               params[i].parameter_type = 0x0014;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_window_adv_monitor;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_interval_connect) {
> +               params[i].parameter_type = 0x0015;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_scan_interval_connect;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_scan_window_connect) {
> +               params[i].parameter_type = 0x0016;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                               main_opts.default_params.le_scan_window_connect;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_min_connection_interval) {
> +               params[i].parameter_type = 0x0017;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_min_connection_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_max_connection_interval) {
> +               params[i].parameter_type = 0x0018;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_max_connection_interval;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_connection_latency) {
> +               params[i].parameter_type = 0x0019;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value =
> +                       main_opts.default_params.le_connection_latency;
> +               ++i;
> +       }
> +
> +       if (main_opts.default_params.le_connection_lsto) {
> +               params[i].parameter_type = 0x001a;
> +               params[i].length = sizeof(params[i].value);
> +               params[i].value = main_opts.default_params.le_connection_lsto;
> +               ++i;
> +       }
> +
> +       mgmt_send(adapter->mgmt, MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS,
> +                       adapter->dev_id, cp_size, params, NULL, NULL, NULL);
> +
> +       btd_error(adapter->dev_id,
> +                               "Failed to set default system params for hci%u",
> +                               adapter->dev_id);

I guess we are missing a if statement above otherwise this will always
be printing an error.

> +
> +       g_free(params);
> +}
> +
>  static void load_devices(struct btd_adapter *adapter)
>  {
>         char dirname[PATH_MAX];
> @@ -8265,6 +8511,7 @@ load:
>         load_drivers(adapter);
>         btd_profile_foreach(probe_profile, adapter);
>         clear_blocked(adapter);
> +       load_default_system_params(adapter);
>         load_devices(adapter);
>
>         /* restore Service Changed CCC value for bonded devices */
> @@ -9158,6 +9405,10 @@ static void read_commands_complete(uint8_t status, uint16_t length,
>                         DBG("kernel supports the set_blocked_keys op");
>                         kernel_blocked_keys_supported = true;
>                         break;
> +               case MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS:
> +                       DBG("kernel supports set system params");
> +                       kernel_set_system_params = true;
> +                       break;
>                 default:
>                         break;
>                 }
> diff --git a/src/hcid.h b/src/hcid.h
> index 1bf93784d..e6d966c58 100644
> --- a/src/hcid.h
> +++ b/src/hcid.h
> @@ -49,6 +49,45 @@ struct main_opts {
>         uint32_t        discovto;
>         uint8_t         privacy;
>
> +       struct {
> +               uint16_t        num_set_params;
> +
> +               uint16_t        br_page_scan_type;
> +               uint16_t        br_page_scan_interval;
> +               uint16_t        br_page_scan_window;
> +
> +               uint16_t        br_inquiry_scan_type;
> +               uint16_t        br_inquiry_scan_interval;
> +               uint16_t        br_inquiry_scan_window;
> +
> +               uint16_t        br_link_supervision_timeout;
> +               uint16_t        br_page_timeout;
> +
> +               uint16_t        br_min_sniff_interval;
> +               uint16_t        br_max_sniff_interval;
> +
> +               uint16_t        le_min_adv_interval;
> +               uint16_t        le_max_adv_interval;
> +               uint16_t        le_multi_adv_rotation_interval;
> +
> +               uint16_t        le_scan_interval_autoconnect;
> +               uint16_t        le_scan_window_autoconnect;
> +               uint16_t        le_scan_interval_suspend;
> +               uint16_t        le_scan_window_suspend;
> +               uint16_t        le_scan_interval_discovery;
> +               uint16_t        le_scan_window_discovery;
> +               uint16_t        le_scan_interval_adv_monitor;
> +               uint16_t        le_scan_window_adv_monitor;
> +               uint16_t        le_scan_interval_connect;
> +               uint16_t        le_scan_window_connect;
> +
> +               uint16_t        le_min_connection_interval;
> +               uint16_t        le_max_connection_interval;
> +               uint16_t        le_connection_latency;
> +               uint16_t        le_connection_lsto;
> +       } default_params;
> +
> +
>         gboolean        reverse_discovery;
>         gboolean        name_resolv;
>         gboolean        debug_keys;
> --
> 2.27.0.rc0.183.gde8f92d652-goog
>


-- 
Luiz Augusto von Dentz

  parent reply	other threads:[~2020-05-29 17:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 15:38 [BlueZ PATCH v3 0/4] Load default system configuration from file Alain Michaud
2020-05-29 15:38 ` [BlueZ PATCH v3 1/4] mgmt:adding load default system configuration definitions Alain Michaud
2020-06-04 22:58   ` Luiz Augusto von Dentz
2020-06-05 14:03     ` Alain Michaud
2020-05-29 15:38 ` [BlueZ PATCH v3 2/4] adapter:set default system configuration if available Alain Michaud
2020-05-29 16:10   ` [BlueZ,v3,2/4] " bluez.test.bot
2020-05-29 17:05   ` Luiz Augusto von Dentz [this message]
2020-05-30  0:21     ` [BlueZ PATCH v3 2/4] " Luiz Augusto von Dentz
2020-05-29 15:38 ` [BlueZ PATCH v3 3/4] main:read default system configuration from the conf file Alain Michaud
2020-05-29 16:10   ` [BlueZ,v3,3/4] " bluez.test.bot
2020-05-29 15:38 ` [BlueZ PATCH v3 4/4] fixing const decoration warnins on options Alain Michaud
2020-05-29 16:10   ` [BlueZ,v3,4/4] " bluez.test.bot
2020-05-29 17:16   ` [BlueZ PATCH v3 4/4] " Luiz Augusto von Dentz
2020-06-01 15:18     ` Alain Michaud
2020-06-10  8:55       ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABBYNZ+DmKdwepmYAaKjEJ5qWsK6NnTMp9JEamej0BgUGJcRCg@mail.gmail.com \
    --to=luiz.dentz@gmail.com \
    --cc=alainm@chromium.org \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.