| Message ID | 20200609140351.153833-2-alainm@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | Support reading and setting default system parameters | expand |
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@chromium.org> > Reviewed-by: Yu Liu <yudiliu@google.com> > > Signed-off-by: Alain Michaud <alainm@chromium.org> > --- > > 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
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@holtmann.org> 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@chromium.org> > > Reviewed-by: Yu Liu <yudiliu@google.com> > > > > Signed-off-by: Alain Michaud <alainm@chromium.org> > > --- > > > > 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 >
Hi Alain, > 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. we can fix this up easily. And I can do that if needed. > 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. That is fine, then I will add it for you. It would be just great if you can review it though. Regards Marcel
Hi Marcel, I'll be happy to review :) Thanks, Alain On Wed, Jun 10, 2020 at 12:02 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Alain, > > > 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. > > we can fix this up easily. And I can do that if needed. > > > 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. > > That is fine, then I will add it for you. It would be just great if you can review it though. > > Regards > > Marcel >
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 + +struct mgmt_system_parameter_tlv { + __u16 type; + __u8 length; + __u8 value[]; +} __packed; + +struct mgmt_rp_read_default_system_parameters { + __u8 parameters[0]; /* mgmt_system_parameter_tlv */ +} __packed; + +#define MGMT_OP_SET_DEFAULT_SYSTEM_PARAMETERS 0x004c + +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;