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;
MIME-Version: 1.0
References: <20210630080807.12600-1-kiran.k@intel.com> <20210630080807.12600-10-kiran.k@intel.com>
 <CABBYNZK-bE6vxikRHhxBM_7ucyZqMjxUBY6VEyw_e8C_xxAigA@mail.gmail.com>
 <B7EC95F8-FFDF-40CE-83B6-85861D530FD5@holtmann.org> <CABBYNZKMrq_6Rb9rJKhMU-w=WjznQsVc=yYfDVuxKtEjvtNLdw@mail.gmail.com>
 <87EBBD58-71B1-4712-A5BC-8B53E739DAFE@holtmann.org>
In-Reply-To: <87EBBD58-71B1-4712-A5BC-8B53E739DAFE@holtmann.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 22 Jul 2021 11:07:52 -0700
Message-ID: <CABBYNZJjjxzrSzLHwyzT-KyMOXYArrwVffHd6s=Kn-OLpTZhXQ@mail.gmail.com>
Subject: Re: [PATCH v10 10/10] Bluetooth: Add offload feature under
 experimental flag
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Kiran K <kiran.k@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Marcel,

On Thu, Jul 22, 2021 at 10:59 AM Marcel Holtmann <marcel@holtmann.org> wrot=
e:
>
> Hi Luiz,
>
> >>>> Allow user level process to enable / disable codec offload
> >>>> feature through mgmt interface. By default offload codec feature
> >>>> is disabled.
> >>>>
> >>>> Signed-off-by: Kiran K <kiran.k@intel.com>
> >>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> >>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> >>>> ---
> >>>> * changes in v10:
> >>>> - new patch added to place offload codec feature under experimental =
flag
> >>>>
> >>>> include/net/bluetooth/hci.h |   4 ++
> >>>> net/bluetooth/mgmt.c        | 106 ++++++++++++++++++++++++++++++++++=
+-
> >>>> net/bluetooth/sco.c         |  10 ++++
> >>>> 3 files changed, 119 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci=
.h
> >>>> index e20318854900..5ca98d9f64dd 100644
> >>>> --- a/include/net/bluetooth/hci.h
> >>>> +++ b/include/net/bluetooth/hci.h
> >>>> @@ -331,6 +331,10 @@ enum {
> >>>>       HCI_CMD_PENDING,
> >>>>       HCI_FORCE_NO_MITM,
> >>>>
> >>>> +#if IS_ENABLED(CONFIG_BT_OFFLOAD_CODECS)
> >>>> +       HCI_OFFLOAD_CODECS_ENABLED,
> >>>> +#endif
> >>>
> >>> That is probably a bad idea as it could lead the enum to assume
> >>> different values based on what is enabled, besides we don't gain
> >>> anything by not having the symbol defined all the time.
> >>
> >> While this would work with dev_flags which are internal and not API, I=
 still don=E2=80=99t like it.
> >>
> >> There is really no benefit to make this a compile time option. And as =
far as I remember I never said this needs to be compile time. Actually I ra=
ther have this as an experimental setting so that it can be switched on at =
runtime. Nobody is going to recompile their kernels to test codec offload.
> >
> > Initially I was with the same opinion, but the problem is the codecs
> > are read at init sequence and the experimental flags are set at a
> > later stage thus why I suggested a KConfig option until the feature is
> > more mature and we can remove the option altogether.
>
> I am fine with the codec options being read all the time. I mean having a=
n experimental option to control the use of offload.

Alright, then we don't need the Kconfig after all, the experimental
flag will only control the use of the codecs e.g. socketopts would not
work if the flag is not enabled I assume?

> Regards
>
> Marcel
>


--=20
Luiz Augusto von Dentz