Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3693-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <1710914907-30012-1-git-send-email-quic_zijuhu@quicinc.com>
 <1713358336-29619-1-git-send-email-quic_zijuhu@quicinc.com> <1713358336-29619-4-git-send-email-quic_zijuhu@quicinc.com>
In-Reply-To: <1713358336-29619-4-git-send-email-quic_zijuhu@quicinc.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Wed, 17 Apr 2024 17:27:35 -0400
Message-ID: <CABBYNZKPK8d2yKKDnpNrr0vYj1u4wygvL-bSuDva31mY-4BwEw@mail.gmail.com>
Subject: Re: [PATCH v2 3/4] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA
To: Zijun Hu <quic_zijuhu@quicinc.com>
Cc: luiz.von.dentz@intel.com, marcel@holtmann.org, 
	linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Zijun,

On Wed, Apr 17, 2024 at 8:53=E2=80=AFAM Zijun Hu <quic_zijuhu@quicinc.com> =
wrote:
>
> HCIUARTSETPROTODATA is introduced to specify protocol specific settings
> prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
> to specify soc type.
>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
>  drivers/bluetooth/hci_uart.h  |  3 +++
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.=
c
> index a26367e9fb19..4be09c61bae5 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
>         /* disable alignment support by default */
>         hu->alignment =3D 1;
>         hu->padding =3D 0;
> +       hu->proto_data =3D 0;
>
>         INIT_WORK(&hu->init_ready, hci_uart_init_work);
>         INIT_WORK(&hu->write_work, hci_uart_write_work);
> @@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty=
, unsigned int cmd,
>                 err =3D hu->hdev_flags;
>                 break;
>
> +       case HCIUARTSETPROTODATA:
> +               if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
> +                       err =3D -EBUSY;
> +               } else {
> +                       hu->proto_data =3D arg;
> +                       BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
> +               }
> +               break;
> +
>         default:
>                 err =3D n_tty_ioctl_helper(tty, cmd, arg);
>                 break;
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 68c8c7e95d64..fc35e9bd4398 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -18,6 +18,8 @@
>  #define HCIUARTGETDEVICE       _IOR('U', 202, int)
>  #define HCIUARTSETFLAGS                _IOW('U', 203, int)
>  #define HCIUARTGETFLAGS                _IOR('U', 204, int)
> +/* Used prior to HCIUARTSETPROTO */
> +#define HCIUARTSETPROTODATA    _IOW('U', 205, unsigned long)

Don't think this will gonna fly, Im not going to introduce vendor
specific like this, besides if the kernel is not able to discover this
data why would userspace be?

>  /* UART protocols */
>  #define HCI_UART_MAX_PROTO     12
> @@ -71,6 +73,7 @@ struct hci_uart {
>         struct work_struct      init_ready;
>         struct work_struct      write_work;
>
> +       unsigned long proto_data;
>         const struct hci_uart_proto *proto;
>         struct percpu_rw_semaphore proto_lock;  /* Stop work for proto cl=
ose */
>         void                    *priv;
> --
> 2.7.4
>


--=20
Luiz Augusto von Dentz