Hi Marcel,
Since this has already been committed in user space, could we agree to
keep it as is? The alternative is that we'd need to re-patch all the
userspace implementation through a seperate patch. Up to you.
I won't have time to implement the runtime config ones in the next few
weeks, feel free to post it separately, or I can get to it in July.
Thanks,
Alain
On Wed, Jun 10, 2020 at 10:16 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> Hi Alain,
>
> > This patch submits the corresponding kernel definitions to mgmt.h.
> > This is submitted before the implementation to avoid any conflicts in
> > values allocations.
> >
> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> > Reviewed-by: Yu Liu <yudiliu@xxxxxxxxxx>
> >
> > Signed-off-by: Alain Michaud <alainm@xxxxxxxxxxxx>
> > ---
> >
> > include/net/bluetooth/mgmt.h | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index 16e0d87bd8fa..1081e371f03d 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -702,6 +702,24 @@ struct mgmt_rp_set_exp_feature {
> > __le32 flags;
> > } __packed;
> >
> > +#define MGMT_OP_READ_DEFAULT_SYSTEM_PARAMETERS 0x004b
> > +
>
> I would go for MGMT_OP_READ_DEF_SYSTEM_CONFIG or MGMT_OP_READ_DEFAULT_SYSTEM_CONFIG to match the name in the mgmt-api.txt more closely.
>
> > +struct mgmt_system_parameter_tlv {
> > + __u16 type;
> > + __u8 length;
> > + __u8 value[];
> > +} __packed;
> > +
>
> Can we just introduce a generic mgmt_tlv {} struct. I think we could use it more broadly. However I wonder if we need it actually since have the EIR parsing support. Maybe just extend that one.
>
> > +struct mgmt_rp_read_default_system_parameters {
> > + __u8 parameters[0]; /* mgmt_system_parameter_tlv */
> > +} __packed;
> > +
> > +#define MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS 0x004c
>
> Similar to the comment above.
>
> > +
> > +struct mgmt_cp_set_default_system_parameters {
> > + __u8 parameters[0]; /* mgmt_system_parameter_tlv */
> > +} __packed;
> > +
> > #define MGMT_EV_CMD_COMPLETE 0x0001
> > struct mgmt_ev_cmd_complete {
> > __le16 opcode;
>
> If you have a chance, please also add MGMT_OP_{READ,SET}_DEF_RUNTIME_CONFIG as well. If not, then I am going to send out a patch for that by myself.
>
> Regards
>
> Marcel
>
