This patch adds macro for exposing quirks through debugfs. Exposing
quirks would be useful for making quirk dependent BlueZ tests using
vhci. Currently there is no way to test that. It might be also
useful for manual testing.
This patch also uses this macro to expose first two quirks.
Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/hci_debugfs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
index 0818fab..a45a0c8 100644
--- a/net/bluetooth/hci_debugfs.c
+++ b/net/bluetooth/hci_debugfs.c
@@ -28,6 +28,54 @@
#include "hci_debugfs.h"
+#define DEFINE_QUIRK_ATTRIBUTE(ATTRIBUTE_NAME, QUIRK_NAME) \
+static ssize_t ATTRIBUTE_NAME##_read(struct file *file, \
+ char __user *user_buf, \
+ size_t count, loff_t *ppos) \
+{ \
+ struct hci_dev *hdev = file->private_data; \
+ char buf[3]; \
+ \
+ buf[0] = test_bit(QUIRK_NAME, &hdev->quirks) ? 'Y' : 'N'; \
+ buf[1] = '\n'; \
+ buf[2] = '\0'; \
+ return simple_read_from_buffer(user_buf, count, ppos, buf, 2); \
+} \
+ \
+static ssize_t ATTRIBUTE_NAME##_write(struct file *file, \
+ const char __user *user_buf, \
+ size_t count, loff_t *ppos) \
+{ \
+ struct hci_dev *hdev = file->private_data; \
+ char buf[32]; \
+ size_t buf_size = min(count, (sizeof(buf) - 1)); \
+ bool enable; \
+ \
+ if (test_bit(HCI_UP, &hdev->flags)) \
+ return -EBUSY; \
+ \
+ if (copy_from_user(buf, user_buf, buf_size)) \
+ return -EFAULT; \
+ \
+ buf[buf_size] = '\0'; \
+ if (strtobool(buf, &enable)) \
+ return -EINVAL; \
+ \
+ if (enable == test_bit(QUIRK_NAME, &hdev->quirks)) \
+ return -EALREADY; \
+ \
+ change_bit(QUIRK_NAME, &hdev->quirks); \
+ \
+ return count; \
+} \
+ \
+static const struct file_operations ATTRIBUTE_NAME##_fops = { \
+ .open = simple_open, \
+ .read = ATTRIBUTE_NAME##_read, \
+ .write = ATTRIBUTE_NAME##_write, \
+ .llseek = default_llseek, \
+} \
+
static int features_show(struct seq_file *f, void *ptr)
{
struct hci_dev *hdev = f->private;
@@ -277,6 +325,11 @@ static const struct file_operations sc_only_mode_fops = {
.llseek = default_llseek,
};
+DEFINE_QUIRK_ATTRIBUTE(quirk_strict_duplicate_filter,
+ HCI_QUIRK_STRICT_DUPLICATE_FILTER);
+DEFINE_QUIRK_ATTRIBUTE(quirk_simulteanous_discovery,
+ HCI_QUIRK_SIMULTANEOUS_DISCOVERY);
+
void hci_debugfs_create_common(struct hci_dev *hdev)
{
debugfs_create_file("features", 0444, hdev->debugfs, hdev,
@@ -308,6 +361,12 @@ void hci_debugfs_create_common(struct hci_dev *hdev)
if (lmp_sc_capable(hdev) || lmp_le_capable(hdev))
debugfs_create_file("sc_only_mode", 0444, hdev->debugfs,
hdev, &sc_only_mode_fops);
+ debugfs_create_file("quirk_strict_duplicate_filter", 0644,
+ hdev->debugfs, hdev,
+ &quirk_strict_duplicate_filter_fops);
+ debugfs_create_file("quirk_simulteanous_discovery", 0644,
+ hdev->debugfs, hdev,
+ &quirk_simulteanous_discovery_fops);
}
static int inquiry_cache_show(struct seq_file *f, void *p)
--
2.2.0.rc0.207.ga3a616c
On Fri, Mar 20, 2015 at 10:09 AM, Marcel Holtmann <[email protected]> wrote:
>
> Hi Jakub,
>
> > This patch adds macro for exposing quirks through debugfs. Exposing
> > quirks would be useful for making quirk dependent BlueZ tests using
> > vhci. Currently there is no way to test that. It might be also
> > useful for manual testing.
> >
> > This patch also uses this macro to expose first two quirks.
> >
> > Signed-off-by: Jakub Pawlowski <[email protected]>
> > ---
> > net/bluetooth/hci_debugfs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 59 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> > index 0818fab..a45a0c8 100644
> > --- a/net/bluetooth/hci_debugfs.c
> > +++ b/net/bluetooth/hci_debugfs.c
> > @@ -28,6 +28,54 @@
> >
> > #include "hci_debugfs.h"
>
> this looks good to me, but I think we need to make some cosmetic changes to have it fit cleaner with how other parts of the kernel do these helper macros.
>
> > +#define DEFINE_QUIRK_ATTRIBUTE(ATTRIBUTE_NAME, QUIRK_NAME) \
>
> Lets name these _name and _quirk instead. I did not find a good reference where they have been used in uppercase.
I've looked at DEFINE_SIMPLE_ATTRIBUTE and decided to use "__" instead
of single _
>
> > +static ssize_t ATTRIBUTE_NAME##_read(struct file *file, \
> > + char __user *user_buf, \
> > + size_t count, loff_t *ppos) \
>
> You might want to do _name ## _read with the proper spaces in between. That seems to be a bit more common.
>
> > +{ \
> > + struct hci_dev *hdev = file->private_data; \
> > + char buf[3]; \
> > + \
> > + buf[0] = test_bit(QUIRK_NAME, &hdev->quirks) ? 'Y' : 'N'; \
> > + buf[1] = '\n'; \
> > + buf[2] = '\0'; \
> > + return simple_read_from_buffer(user_buf, count, ppos, buf, 2); \
> > +} \
> > + \
> > +static ssize_t ATTRIBUTE_NAME##_write(struct file *file, \
> > + const char __user *user_buf, \
> > + size_t count, loff_t *ppos) \
> > +{ \
> > + struct hci_dev *hdev = file->private_data; \
> > + char buf[32]; \
> > + size_t buf_size = min(count, (sizeof(buf) - 1)); \
> > + bool enable; \
> > + \
> > + if (test_bit(HCI_UP, &hdev->flags)) \
> > + return -EBUSY; \
> > + \
> > + if (copy_from_user(buf, user_buf, buf_size)) \
> > + return -EFAULT; \
> > + \
> > + buf[buf_size] = '\0'; \
> > + if (strtobool(buf, &enable)) \
> > + return -EINVAL; \
> > + \
> > + if (enable == test_bit(QUIRK_NAME, &hdev->quirks)) \
> > + return -EALREADY; \
> > + \
> > + change_bit(QUIRK_NAME, &hdev->quirks); \
> > + \
> > + return count; \
> > +} \
> > + \
> > +static const struct file_operations ATTRIBUTE_NAME##_fops = { \
> > + .open = simple_open, \
> > + .read = ATTRIBUTE_NAME##_read, \
> > + .write = ATTRIBUTE_NAME##_write, \
>
> And I would do the same here, _name ## _read.
>
> > + .llseek = default_llseek, \
> > +} \
> > +
> > static int features_show(struct seq_file *f, void *ptr)
> > {
> > struct hci_dev *hdev = f->private;
> > @@ -277,6 +325,11 @@ static const struct file_operations sc_only_mode_fops = {
> > .llseek = default_llseek,
> > };
> >
> > +DEFINE_QUIRK_ATTRIBUTE(quirk_strict_duplicate_filter,
> > + HCI_QUIRK_STRICT_DUPLICATE_FILTER);
> > +DEFINE_QUIRK_ATTRIBUTE(quirk_simulteanous_discovery,
> > + HCI_QUIRK_SIMULTANEOUS_DISCOVERY);
> > +
> > void hci_debugfs_create_common(struct hci_dev *hdev)
> > {
> > debugfs_create_file("features", 0444, hdev->debugfs, hdev,
> > @@ -308,6 +361,12 @@ void hci_debugfs_create_common(struct hci_dev *hdev)
> > if (lmp_sc_capable(hdev) || lmp_le_capable(hdev))
> > debugfs_create_file("sc_only_mode", 0444, hdev->debugfs,
> > hdev, &sc_only_mode_fops);
> > + debugfs_create_file("quirk_strict_duplicate_filter", 0644,
> > + hdev->debugfs, hdev,
> > + &quirk_strict_duplicate_filter_fops);
> > + debugfs_create_file("quirk_simulteanous_discovery", 0644,
> > + hdev->debugfs, hdev,
> > + &quirk_simulteanous_discovery_fops);
>
> If you want to, you could create a second macro for these.
>
> debugfs_create_quirk(hdev, strict_duplicate_filter);
>
> All the string magic etc. can be done by a define. Could be also your second patch to make this even simple and we the first one in first after the cosmetic cleanup.
I'll look into that.
Thanks for quick review!
>
> Regards
>
> Marcel
>
Hi Jakub,
> This patch adds macro for exposing quirks through debugfs. Exposing
> quirks would be useful for making quirk dependent BlueZ tests using
> vhci. Currently there is no way to test that. It might be also
> useful for manual testing.
>
> This patch also uses this macro to expose first two quirks.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> net/bluetooth/hci_debugfs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/net/bluetooth/hci_debugfs.c b/net/bluetooth/hci_debugfs.c
> index 0818fab..a45a0c8 100644
> --- a/net/bluetooth/hci_debugfs.c
> +++ b/net/bluetooth/hci_debugfs.c
> @@ -28,6 +28,54 @@
>
> #include "hci_debugfs.h"
this looks good to me, but I think we need to make some cosmetic changes to have it fit cleaner with how other parts of the kernel do these helper macros.
> +#define DEFINE_QUIRK_ATTRIBUTE(ATTRIBUTE_NAME, QUIRK_NAME) \
Lets name these _name and _quirk instead. I did not find a good reference where they have been used in uppercase.
> +static ssize_t ATTRIBUTE_NAME##_read(struct file *file, \
> + char __user *user_buf, \
> + size_t count, loff_t *ppos) \
You might want to do _name ## _read with the proper spaces in between. That seems to be a bit more common.
> +{ \
> + struct hci_dev *hdev = file->private_data; \
> + char buf[3]; \
> + \
> + buf[0] = test_bit(QUIRK_NAME, &hdev->quirks) ? 'Y' : 'N'; \
> + buf[1] = '\n'; \
> + buf[2] = '\0'; \
> + return simple_read_from_buffer(user_buf, count, ppos, buf, 2); \
> +} \
> + \
> +static ssize_t ATTRIBUTE_NAME##_write(struct file *file, \
> + const char __user *user_buf, \
> + size_t count, loff_t *ppos) \
> +{ \
> + struct hci_dev *hdev = file->private_data; \
> + char buf[32]; \
> + size_t buf_size = min(count, (sizeof(buf) - 1)); \
> + bool enable; \
> + \
> + if (test_bit(HCI_UP, &hdev->flags)) \
> + return -EBUSY; \
> + \
> + if (copy_from_user(buf, user_buf, buf_size)) \
> + return -EFAULT; \
> + \
> + buf[buf_size] = '\0'; \
> + if (strtobool(buf, &enable)) \
> + return -EINVAL; \
> + \
> + if (enable == test_bit(QUIRK_NAME, &hdev->quirks)) \
> + return -EALREADY; \
> + \
> + change_bit(QUIRK_NAME, &hdev->quirks); \
> + \
> + return count; \
> +} \
> + \
> +static const struct file_operations ATTRIBUTE_NAME##_fops = { \
> + .open = simple_open, \
> + .read = ATTRIBUTE_NAME##_read, \
> + .write = ATTRIBUTE_NAME##_write, \
And I would do the same here, _name ## _read.
> + .llseek = default_llseek, \
> +} \
> +
> static int features_show(struct seq_file *f, void *ptr)
> {
> struct hci_dev *hdev = f->private;
> @@ -277,6 +325,11 @@ static const struct file_operations sc_only_mode_fops = {
> .llseek = default_llseek,
> };
>
> +DEFINE_QUIRK_ATTRIBUTE(quirk_strict_duplicate_filter,
> + HCI_QUIRK_STRICT_DUPLICATE_FILTER);
> +DEFINE_QUIRK_ATTRIBUTE(quirk_simulteanous_discovery,
> + HCI_QUIRK_SIMULTANEOUS_DISCOVERY);
> +
> void hci_debugfs_create_common(struct hci_dev *hdev)
> {
> debugfs_create_file("features", 0444, hdev->debugfs, hdev,
> @@ -308,6 +361,12 @@ void hci_debugfs_create_common(struct hci_dev *hdev)
> if (lmp_sc_capable(hdev) || lmp_le_capable(hdev))
> debugfs_create_file("sc_only_mode", 0444, hdev->debugfs,
> hdev, &sc_only_mode_fops);
> + debugfs_create_file("quirk_strict_duplicate_filter", 0644,
> + hdev->debugfs, hdev,
> + &quirk_strict_duplicate_filter_fops);
> + debugfs_create_file("quirk_simulteanous_discovery", 0644,
> + hdev->debugfs, hdev,
> + &quirk_simulteanous_discovery_fops);
If you want to, you could create a second macro for these.
debugfs_create_quirk(hdev, strict_duplicate_filter);
All the string magic etc. can be done by a define. Could be also your second patch to make this even simple and we the first one in first after the cosmetic cleanup.
Regards
Marcel