Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\))
Subject: Re: [PATCH] Add Support for LE Ping feature
From:   Johan Hedberg <johan.hedberg@gmail.com>
In-Reply-To: <1553661793-10304-1-git-send-email-spoorthix.k@intel.com>
Date:   Wed, 27 Mar 2019 18:29:47 +0100
Cc:     linux-bluetooth@vger.kernel.org,
        linux-bluetooth-owner@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <A7F336FF-1D53-4DC9-94ED-392011649A5E@gmail.com>
References: <1553661793-10304-1-git-send-email-spoorthix.k@intel.com>
To:     SpoorthiX K <spoorthix.k@intel.com>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Spoorthi,

On 27 Mar 2019, at 5.43, SpoorthiX K <spoorthix.k@intel.com> wrote:
> +#define HCI_OP_WRITE_AUTH_PAYLOAD_TO    0x0c7c
> +struct hci_cp_write_auth_payload_to {
> +	__le16	handle;
> +	__u16	timeout;

As Marcel (and now also the build test robot) pointed out this should be =
__le16.

> #if IS_ENABLED(CONFIG_BT_LEDS)
> 	struct led_trigger	*power_led;
> @@ -500,6 +501,7 @@ struct hci_conn {
> 	__u8		remote_id;
>=20
> 	unsigned int	sent;
> +	bool		auth_payload_timeout_set;

Would it make sense to add this to the flags instead of increasing the =
hci_conn struct size?

> +	/* Set the default Authenticated Payload Timeout after
> +	 * an LE Link is established.

The test further below also includes BR/EDR links, i.e. the above =
comment doesn=E2=80=99t match that.

> +	 * Section 3.3, the HCI command WRITE_AUTH_PAYLOAD_TIMEOUT =
should be
> +	 * sent when the link is active and Encryption is enabled, the =
conn
> +	 * type can be either LE or ACL and controller must support LMP =
Ping.
> +	 * Ensure for AES-CCM encryption as well.
> +	 */
> +	if ((conn->type =3D=3D LE_LINK || conn->type =3D=3D ACL_LINK) &

That =E2=80=98&=E2=80=99 the end should probably be a =E2=80=98&&=E2=80=99=
, shouldn=E2=80=99t it?

> +	} else {
> +		conn->state =3D BT_CONNECTED;
> +		hci_connect_cfm(conn, ev->status);
> +	}

Why is this being added? Such a transition to BT_CONNECTED state at this =
point in the code didn=E2=80=99t exist previously, and this doesn=E2=80=99=
t seem to be related to the new feature.

Johan=