Please review and upstream to bluetooth-next

Please review and upstream to bluetooth-next

[G, Jaya P]

[-- Attachment #1: Type: text/plain, Size: 5991 bytes --]

Hi,

Patch is for creating a kobject file from btusb which creates a file under kobject
Which exposes HW Variant, FW Variant and Patch number for Intel specific SKU's
Which is required to differentiate different chips from intel manufacturer.

Please review and upstream the Patch to bluetooth-next.

>From 01e904148f4de6d8d48d61b01b08da7a771fe998 Mon Sep 17 00:00:00 2001
From: Jaya Praveen G <jaya.p.g@intel.com>
Date: Fri, 15 Jul 2016 17:25:31 +0530
Subject: [PATCH] btusb: Export the Intel HW/FW/Patch variants to /sysfs

Intel will have different SKU and version with the same
VID/PID. Different SKU use different BT version this means
different HW and FW. To check this, we have read version
for the intel SKU which is unique, the same data is put in
a file create with kobject in /sys/kernel/intel_hw_version_kobject/
with the name /intel_hw_version where it shows the HW variant,
FW variant and Patch number which will different for each SKU.

Signed-off-by: Jaya Praveen G <jaya.p.g@intel.com>
---
drivers/bluetooth/btusb.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 811f9b9..720f14f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,10 @@
#include <linux/module.h>
#include <linux/usb.h>
#include <linux/firmware.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/fs.h>
+#include <linux/string.h>
#include <asm/unaligned.h>
 #include <net/bluetooth/bluetooth.h>
@@ -42,6 +46,30 @@ static bool reset = true;
 static struct usb_driver btusb_driver;
+static struct kobject *intel_hw_version_kobject;
+struct intel_version ver;
+
+/*
+ * This file which is under /sys/kernel/intel_hw_version_kobject kobject which has the file name as intel_hw_version
+ * contains the intel FW version in terms of HW variant, FW variant and Patch number which are different for
+ * different SKU's.
+ */
+static ssize_t intel_hw_showrev(struct kobject *k, struct attribute *a, char *buf)
+{
+        return sprintf(buf, "HW_Variant: 0x%02x%02x%02x\nFW_variant: 0x%02x%02x%02x%02x%02x\nPatch Num: 0x%02x\n",ver.hw_platform, ver.hw_variant, ver.hw_revision,
+                            ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
+                            ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
+}
+
+/* Sysfs attributes made as user readable and writable*/
+static const struct {
+             struct attribute attr;
+             ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
+} intel_hw_version_attr = {
+             .attr = { .name = "intel_hw_version", .mode = S_IWUSR | S_IRUSR},
+             .show = intel_hw_showrev,
+};
+
#define BTUSB_IGNORE                              0x01
#define BTUSB_DIGIANSWER      0x02
#define BTUSB_CSR                       0x04
@@ -1655,7 +1683,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
              const struct firmware *fw;
              const u8 *fw_ptr;
              int disable_patch, err;
-              struct intel_version ver;
+        int error = 0;
               BT_DBG("%s", hdev->name);
@@ -1690,6 +1718,19 @@ static int btusb_setup_intel(struct hci_dev *hdev)
                             ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
                             ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
+        /* Create a kobject with the name "intel_hw_version_kobject", located
+         * under /sys/kernel/.
+         */
+        intel_hw_version_kobject = kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);
+        if(!intel_hw_version_kobject)
+                  return -ENOMEM;
+
+        /* Create a file associated with the kobject */
+        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
+        if(error) {
+              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
+        }
+
              /* fw_patch_num indicates the version of patch the device currently
               * have. If there is no patch data in the device, it is always 0x00.
               * So, if it is other than 0x00, no need to patch the device again.
@@ -1974,7 +2015,6 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
                                                                            0x00, 0x08, 0x04, 0x00 };
              struct btusb_data *data = hci_get_drvdata(hdev);
              struct sk_buff *skb;
-              struct intel_version ver;
              struct intel_boot_params *params;
              const struct firmware *fw;
              const u8 *fw_ptr;
@@ -1983,6 +2023,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
              ktime_t calltime, delta, rettime;
              unsigned long long duration;
              int err;
+        int error = 0;
               BT_DBG("%s", hdev->name);
@@ -1996,6 +2037,18 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
              if (err)
                             return err;
+        /* Create a kobject with the name "intel_hw_version_kobject", located
+         * under /sys/kernel/.
+         */
+        intel_hw_version_kobject = kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);
+        if(!intel_hw_version_kobject)
+                  return -ENOMEM;
+
+        /* Create a file associated with the kobject */
+        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
+        if(error) {
+              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
+        }
              /* The hardware platform number has a fixed value of 0x37 and
               * for now only accept this single value.
               */
--
1.9.1

Regards,
Jaya Praveen G

[-- Attachment #2: Type: text/html, Size: 23123 bytes --]

Re: Please review and upstream to bluetooth-next

[Marcel Holtmann]

Hi Jaya,

please do not post HTML emails to this mailing list. The mailing list system will most likely reject them anyway.

> Patch is for creating a kobject file from btusb which creates a file under kobject
> Which exposes HW Variant, FW Variant and Patch number for Intel specific SKU’s
> Which is required to differentiate different chips from intel manufacturer.
>  
> Please review and upstream the Patch to bluetooth-next.

Please send patches according to the submitting patches guidelines. I can not apply patches randomly inserted into email bodies.

> From 01e904148f4de6d8d48d61b01b08da7a771fe998 Mon Sep 17 00:00:00 2001
> From: Jaya Praveen G <jaya.p.g@intel.com>
> Date: Fri, 15 Jul 2016 17:25:31 +0530
> Subject: [PATCH] btusb: Export the Intel HW/FW/Patch variants to /sysfs
>  
> Intel will have different SKU and version with the same
> VID/PID. Different SKU use different BT version this means
> different HW and FW. To check this, we have read version
> for the intel SKU which is unique, the same data is put in
> a file create with kobject in /sys/kernel/intel_hw_version_kobject/
> with the name /intel_hw_version where it shows the HW variant,
> FW variant and Patch number which will different for each SKU.

I am failing to understand what these informations are used for. The bluemoon utility can read out these information easily already.

It is also something that no other driver is doing. Why would Intel need such a thing in the first place. Also exposing random kobjects from a driver is the wrong approach to this. I mean no Bluetooth driver is exposing things in sysfs.

>  
> Signed-off-by: Jaya Praveen G <jaya.p.g@intel.com>
> ---
> drivers/bluetooth/btusb.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 2 deletions(-)
>  
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 811f9b9..720f14f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,10 @@
> #include <linux/module.h>
> #include <linux/usb.h>
> #include <linux/firmware.h>
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +#include <linux/fs.h>
> +#include <linux/string.h>
> #include <asm/unaligned.h>
>  #include <net/bluetooth/bluetooth.h>
> @@ -42,6 +46,30 @@ static bool reset = true;
>  static struct usb_driver btusb_driver;
> +static struct kobject *intel_hw_version_kobject;
> +struct intel_version ver;
> +
> +/*
> + * This file which is under /sys/kernel/intel_hw_version_kobject kobject which has the file name as intel_hw_version
> + * contains the intel FW version in terms of HW variant, FW variant and Patch number which are different for
> + * different SKU's.
> + */
> +static ssize_t intel_hw_showrev(struct kobject *k, struct attribute *a, char *buf)
> +{
> +        return sprintf(buf, "HW_Variant: 0x%02x%02x%02x\nFW_variant: 0x%02x%02x%02x%02x%02x\nPatch Num: 0x%02x\n",ver.hw_platform, ver.hw_variant, ver.hw_revision,
> +                            ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
> +                            ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
> +}

We have a line size limit of 78 characters that should be considered whenever possible to break things into multiple lines. Only a few exceptions are breaking this coding style rules.

> +
> +/* Sysfs attributes made as user readable and writable*/
> +static const struct {
> +             struct attribute attr;
> +             ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
> +} intel_hw_version_attr = {
> +             .attr = { .name = "intel_hw_version", .mode = S_IWUSR | S_IRUSR},
> +             .show = intel_hw_showrev,
> +};
> +
> #define BTUSB_IGNORE                              0x01
> #define BTUSB_DIGIANSWER      0x02
> #define BTUSB_CSR                       0x04
> @@ -1655,7 +1683,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>               const struct firmware *fw;
>               const u8 *fw_ptr;
>               int disable_patch, err;
> -              struct intel_version ver;
> +        int error = 0;
>                BT_DBG("%s", hdev->name);
> @@ -1690,6 +1718,19 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>                              ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
>                              ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
> +        /* Create a kobject with the name "intel_hw_version_kobject", located
> +         * under /sys/kernel/.
> +         */
> +        intel_hw_version_kobject = kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);

This is broken by design. Linux supports multiple controllers. So what happens if you attach a second Intel controller to the same system.

> +        if(!intel_hw_version_kobject)
> +                  return -ENOMEM;
> +
> +        /* Create a file associated with the kobject */
> +        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
> +        if(error) {
> +              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
> +        }
> +

The whole section is wrongly intended and is not following the network subsystem coding style.

Regards

Marcel

RE: Please review and upstream to bluetooth-next

[G, Jaya P]

[-- Attachment #1: Type: text/plain, Size: 6829 bytes --]

Hi Marcel,

I am sending the first patch to Bluetooth-next.
Regarding the coding guidelines, Attached a new patchset with the coding guidelines, except for removing the kobject changes.

And regarding this requirement, this came for google chrome  to check the 
Different SKU's version on the go rather than issuing read version command for the controller.
And so, the design proposed that we expose a kobject file for this from btusb	. 
And for your understanding, the Intel FW file pushed for the controller will have these 3 components 
HW Variant : 0x000000
FW Variant: 0x0000000000
Patch number: 0x00
Which is not the same for all the vendors, this information required to check which controller is 
Inserted in our linux pc, so an additional information along VID, PID information which is same different Intel Controllers.
This is required because, Intel SKU's have same VID,PID for different chips i.e for XX B0, XX B1 will have the same VID, PID.

And the Linux PC or the OEM that we are supporting can have only one Bluetooth controller, so the
Issue that you mention never happens.

Regards,
Jaya Praveen G
-----Original Message-----
From: Marcel Holtmann [mailto:marcel@holtmann.org] 
Sent: Friday, July 15, 2016 12:18 PM
To: G, Jaya P <jaya.p.g@intel.com>
Cc: linux-bluetooth@vger.kernel.org; An, Tedd <tedd.an@intel.com>
Subject: Re: Please review and upstream to bluetooth-next

Hi Jaya,

please do not post HTML emails to this mailing list. The mailing list system will most likely reject them anyway.

> Patch is for creating a kobject file from btusb which creates a file 
> under kobject Which exposes HW Variant, FW Variant and Patch number 
> for Intel specific SKU’s Which is required to differentiate different chips from intel manufacturer.
>  
> Please review and upstream the Patch to bluetooth-next.

Please send patches according to the submitting patches guidelines. I can not apply patches randomly inserted into email bodies.

> From 01e904148f4de6d8d48d61b01b08da7a771fe998 Mon Sep 17 00:00:00 2001
> From: Jaya Praveen G <jaya.p.g@intel.com>
> Date: Fri, 15 Jul 2016 17:25:31 +0530
> Subject: [PATCH] btusb: Export the Intel HW/FW/Patch variants to 
> /sysfs
>  
> Intel will have different SKU and version with the same VID/PID. 
> Different SKU use different BT version this means different HW and FW. 
> To check this, we have read version for the intel SKU which is unique, 
> the same data is put in a file create with kobject in 
> /sys/kernel/intel_hw_version_kobject/
> with the name /intel_hw_version where it shows the HW variant, FW 
> variant and Patch number which will different for each SKU.

I am failing to understand what these informations are used for. The bluemoon utility can read out these information easily already.

It is also something that no other driver is doing. Why would Intel need such a thing in the first place. Also exposing random kobjects from a driver is the wrong approach to this. I mean no Bluetooth driver is exposing things in sysfs.

>  
> Signed-off-by: Jaya Praveen G <jaya.p.g@intel.com>
> ---
> drivers/bluetooth/btusb.c | 57 
> +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 55 insertions(+), 2 deletions(-)
>  
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c 
> index 811f9b9..720f14f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,10 @@
> #include <linux/module.h>
> #include <linux/usb.h>
> #include <linux/firmware.h>
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +#include <linux/fs.h>
> +#include <linux/string.h>
> #include <asm/unaligned.h>
>  #include <net/bluetooth/bluetooth.h>
> @@ -42,6 +46,30 @@ static bool reset = true;  static struct usb_driver 
> btusb_driver;
> +static struct kobject *intel_hw_version_kobject; struct intel_version 
> +ver;
> +
> +/*
> + * This file which is under /sys/kernel/intel_hw_version_kobject 
> +kobject which has the file name as intel_hw_version
> + * contains the intel FW version in terms of HW variant, FW variant 
> +and Patch number which are different for
> + * different SKU's.
> + */
> +static ssize_t intel_hw_showrev(struct kobject *k, struct attribute 
> +*a, char *buf) {
> +        return sprintf(buf, "HW_Variant: 0x%02x%02x%02x\nFW_variant: 0x%02x%02x%02x%02x%02x\nPatch Num: 0x%02x\n",ver.hw_platform, ver.hw_variant, ver.hw_revision,
> +                            ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
> +                            ver.fw_build_ww, ver.fw_build_yy, 
> +ver.fw_patch_num); }

We have a line size limit of 78 characters that should be considered whenever possible to break things into multiple lines. Only a few exceptions are breaking this coding style rules.

> +
> +/* Sysfs attributes made as user readable and writable*/ static const 
> +struct {
> +             struct attribute attr;
> +             ssize_t (*show)(struct kobject *k, struct attribute *a, 
> +char *buf); } intel_hw_version_attr = {
> +             .attr = { .name = "intel_hw_version", .mode = S_IWUSR | S_IRUSR},
> +             .show = intel_hw_showrev, };
> +
> #define BTUSB_IGNORE                              0x01
> #define BTUSB_DIGIANSWER      0x02
> #define BTUSB_CSR                       0x04
> @@ -1655,7 +1683,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>               const struct firmware *fw;
>               const u8 *fw_ptr;
>               int disable_patch, err;
> -              struct intel_version ver;
> +        int error = 0;
>                BT_DBG("%s", hdev->name); @@ -1690,6 +1718,19 @@ static 
> int btusb_setup_intel(struct hci_dev *hdev)
>                              ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
>                              ver.fw_build_ww, ver.fw_build_yy, 
> ver.fw_patch_num);
> +        /* Create a kobject with the name "intel_hw_version_kobject", located
> +         * under /sys/kernel/.
> +         */
> +        intel_hw_version_kobject = 
> + kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);

This is broken by design. Linux supports multiple controllers. So what happens if you attach a second Intel controller to the same system.

> +        if(!intel_hw_version_kobject)
> +                  return -ENOMEM;
> +
> +        /* Create a file associated with the kobject */
> +        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
> +        if(error) {
> +              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
> +        }
> +

The whole section is wrongly intended and is not following the network subsystem coding style.

Regards

Marcel


[-- Attachment #2: 0001-btusb-Export-the-Intel-HW-FW-Patch-variants-to-sysfs.patch --]
[-- Type: application/octet-stream, Size: 4993 bytes --]

From 01e904148f4de6d8d48d61b01b08da7a771fe998 Mon Sep 17 00:00:00 2001
From: Jaya Praveen G <jaya.p.g@intel.com>
Date: Fri, 15 Jul 2016 17:25:31 +0530
Subject: [PATCH] btusb: Export the Intel HW/FW/Patch variants to /sysfs

Intel will have different SKU and version with the same
VID/PID. Different SKU use different BT version this means
different HW and FW. To check this, we have read version
for the intel SKU which is unique, the same data is put in
a file create with kobject in /sys/kernel/intel_hw_version_kobject/
with the name /intel_hw_version where it shows the HW variant,
FW variant and Patch number which will different for each SKU.

Signed-off-by: Jaya Praveen G <jaya.p.g@intel.com>
---
 drivers/bluetooth/btusb.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 811f9b9..720f14f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,10 @@
 #include <linux/module.h>
 #include <linux/usb.h>
 #include <linux/firmware.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/fs.h>
+#include <linux/string.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -42,6 +46,30 @@ static bool reset = true;
 
 static struct usb_driver btusb_driver;
 
+static struct kobject *intel_hw_version_kobject;
+struct intel_version ver;
+
+/*
+ * This file which is under /sys/kernel/intel_hw_version_kobject kobject which has the file name as intel_hw_version
+ * contains the intel FW version in terms of HW variant, FW variant and Patch number which are different for
+ * different SKU's.
+ */
+static ssize_t intel_hw_showrev(struct kobject *k, struct attribute *a, char *buf)
+{
+        return sprintf(buf, "HW_Variant: 0x%02x%02x%02x\nFW_variant: 0x%02x%02x%02x%02x%02x\nPatch Num: 0x%02x\n",ver.hw_platform, ver.hw_variant, ver.hw_revision,
+		ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
+		ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
+}
+
+/* Sysfs attributes made as user readable and writable*/
+static const struct {
+	struct attribute attr;
+	ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
+} intel_hw_version_attr = {
+	.attr = { .name = "intel_hw_version", .mode = S_IWUSR | S_IRUSR},
+	.show = intel_hw_showrev,
+};
+
 #define BTUSB_IGNORE		0x01
 #define BTUSB_DIGIANSWER	0x02
 #define BTUSB_CSR		0x04
@@ -1655,7 +1683,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
 	const struct firmware *fw;
 	const u8 *fw_ptr;
 	int disable_patch, err;
-	struct intel_version ver;
+        int error = 0;
 
 	BT_DBG("%s", hdev->name);
 
@@ -1690,6 +1718,19 @@ static int btusb_setup_intel(struct hci_dev *hdev)
 		ver.fw_variant,  ver.fw_revision, ver.fw_build_num,
 		ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
 
+        /* Create a kobject with the name "intel_hw_version_kobject", located
+         * under /sys/kernel/.
+         */
+        intel_hw_version_kobject = kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);
+        if(!intel_hw_version_kobject)
+                  return -ENOMEM;
+
+        /* Create a file associated with the kobject */
+        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
+        if(error) {
+              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
+        }
+
 	/* fw_patch_num indicates the version of patch the device currently
 	 * have. If there is no patch data in the device, it is always 0x00.
 	 * So, if it is other than 0x00, no need to patch the device again.
@@ -1974,7 +2015,6 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 					  0x00, 0x08, 0x04, 0x00 };
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct sk_buff *skb;
-	struct intel_version ver;
 	struct intel_boot_params *params;
 	const struct firmware *fw;
 	const u8 *fw_ptr;
@@ -1983,6 +2023,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 	int err;
+        int error = 0;
 
 	BT_DBG("%s", hdev->name);
 
@@ -1996,6 +2037,18 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 	if (err)
 		return err;
 
+        /* Create a kobject with the name "intel_hw_version_kobject", located
+         * under /sys/kernel/.
+         */
+        intel_hw_version_kobject = kobject_create_and_add("intel_hw_version_kobject", kernel_kobj);
+        if(!intel_hw_version_kobject)
+                  return -ENOMEM;
+
+        /* Create a file associated with the kobject */
+        error = sysfs_create_file(intel_hw_version_kobject, &intel_hw_version_attr.attr);
+        if(error) {
+              BT_DBG("failed to create the version file in /sys/kernel/intel_hw_version_kobject \n");
+        }
 	/* The hardware platform number has a fixed value of 0x37 and
 	 * for now only accept this single value.
 	 */
-- 
1.9.1

Re: Please review and upstream to bluetooth-next

[Marcel Holtmann]

Hi Jaya,

please do not top-post on this mailing list. It is against its etiquette.

> I am sending the first patch to Bluetooth-next.
> Regarding the coding guidelines, Attached a new patchset with the coding guidelines, except for removing the kobject changes.

The kobject exposing from the driver is not acceptable. That is something that is just a plain hack. It is not a clean solution and can not be accepted upstream.

> And regarding this requirement, this came for google chrome  to check the 
> Different SKU's version on the go rather than issuing read version command for the controller.

What is the difference? I do not get it. It is specific to Intel anyway. How is this done for Broadcom or Marvell controllers? Someone needs to shed some light into this requirement. Either this is a common way for all controllers to expose a vendor revision string or it is not useful in the broader sense at all.

> And so, the design proposed that we expose a kobject file for this from btusb	. 
> And for your understanding, the Intel FW file pushed for the controller will have these 3 components 
> HW Variant : 0x000000
> FW Variant: 0x0000000000
> Patch number: 0x00
> Which is not the same for all the vendors, this information required to check which controller is 
> Inserted in our linux pc, so an additional information along VID, PID information which is same different Intel Controllers.
> This is required because, Intel SKU's have same VID,PID for different chips i.e for XX B0, XX B1 will have the same VID, PID.

I know how our SKUs work, but exposing the raw numbers is still not doing much in the end. You want to decode the numbers into something real. That is what bluemoon actually does.

> And the Linux PC or the OEM that we are supporting can have only one Bluetooth controller, so the
> Issue that you mention never happens.

Please forget about this kind of reasoning right away. That does not work with upstream. This argument alone would be a reason to reject the patch. This Linux kernel runs on multiple systems and if I attach two Intel controllers to my desktop system, I want them to work. Full stop here. You can not break other peoples machines only because this is out of scope for you. This means any design has to take this into account.

Regards

Marcel