Hi Marcel,
On Thu, Jul 22, 2021 at 10:59 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
> >>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
> >>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@xxxxxxxxx>
> >>>> ---
> >>>> * 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’t 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 rather 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 an 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
>
--
Luiz Augusto von Dentz
