All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller
@ 2018-10-30 10:50 Chethan T N
  2018-10-30 10:50 ` [PATCH 2/2] Bluetooth: Add build configuration option for Intel bluetooth rfkill driver Chethan T N
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Chethan T N @ 2018-10-30 10:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: amit.k.bag, Raghuram Hegde, Chethan T N, Sukumar Ghorai

From: Raghuram Hegde <raghuram.hegde@intel.com>

Register ACPI object INTL6205 as platform Bluetooth RfKill
driver to attach/detach Intel Bluetooth controller from
platform USB bus.

Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
---
 drivers/platform/x86/intel_bt_rfkill.c | 133 +++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 drivers/platform/x86/intel_bt_rfkill.c

diff --git a/drivers/platform/x86/intel_bt_rfkill.c b/drivers/platform/x86/intel_bt_rfkill.c
new file mode 100644
index 000000000000..89a3d7f4b32c
--- /dev/null
+++ b/drivers/platform/x86/intel_bt_rfkill.c
@@ -0,0 +1,133 @@
+/*
+ *  Intel Bluetooth Rfkill Driver
+ */
+
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/rfkill.h>
+#include <linux/gpio/consumer.h>
+
+struct btintel_rfkill_dev {
+	struct rfkill *rfk;
+	struct gpio_desc *reset_gpio_handler;
+};
+
+static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
+static const struct acpi_gpio_mapping acpi_btintel_gpios[] = {
+	{ "reset-gpios", &reset_gpios, 1 },
+	{ },
+};
+
+static const struct acpi_device_id btintel_acpi_match[] = {
+	{ "INTL6205", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, btintel_acpi_match);
+
+static int btintel_disable_bt(struct gpio_desc *reset_gpio)
+{
+	if (!reset_gpio)
+		return -EINVAL;
+
+	/* This will detach the Intel BT controller */
+	gpiod_set_value(reset_gpio, 0);
+	return 0;
+}
+
+static int btintel_enable_bt(struct gpio_desc *reset_gpio)
+{
+	if (!reset_gpio)
+		return -EINVAL;
+
+	/* This will re-attach the Intel BT controller */
+	gpiod_set_value(reset_gpio, 1);
+	return 0;
+}
+
+/* RFKill handlers */
+static int bt_rfkill_set_block(void *data, bool blocked)
+{
+	struct btintel_rfkill_dev *bt_dev = data;
+	int ret;
+
+	if (blocked)
+		ret = btintel_disable_bt(bt_dev->reset_gpio_handler);
+	else
+		ret = btintel_enable_bt(bt_dev->reset_gpio_handler);
+
+	return ret;
+}
+static const struct rfkill_ops rfk_ops = {
+	.set_block = bt_rfkill_set_block,
+};
+
+/* ACPI object probe */
+static int btintel_probe(struct platform_device *pdev)
+{
+	struct btintel_rfkill_dev *bt_dev;
+	int result;
+
+	bt_dev = kzalloc(sizeof(*bt_dev), GFP_KERNEL);
+	if (!bt_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(&pdev->dev, bt_dev);
+
+	if (acpi_dev_add_driver_gpios(ACPI_COMPANION(&pdev->dev),
+				     acpi_btintel_gpios)) {
+		kfree(bt_dev);
+		return -EINVAL;
+	}
+
+	bt_dev->reset_gpio_handler = devm_gpiod_get_optional(&pdev->dev,
+					"reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(bt_dev->reset_gpio_handler)) {
+		kfree(bt_dev);
+		return PTR_ERR(bt_dev->reset_gpio_handler);
+	}
+
+	bt_dev->rfk = rfkill_alloc("intel_bluetooth",
+				   &pdev->dev,
+				   RFKILL_TYPE_BLUETOOTH,
+				   &rfk_ops,
+				   bt_dev);
+	if (!bt_dev->rfk) {
+		kfree(bt_dev);
+		return -ENOMEM;
+	}
+
+	result = rfkill_register(bt_dev->rfk);
+	if (result) {
+		rfkill_destroy(bt_dev->rfk);
+		kfree(bt_dev);
+	}
+
+	return result;
+}
+
+static int btintel_remove(struct platform_device *pdev)
+{
+	struct btintel_rfkill_dev *bt_dev = dev_get_drvdata(&pdev->dev);
+
+	if (bt_dev->rfk) {
+		rfkill_unregister(bt_dev->rfk);
+		rfkill_destroy(bt_dev->rfk);
+	}
+
+	kfree(bt_dev);
+
+	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
+	return 0;
+}
+
+static struct platform_driver btintel_driver = {
+	.probe = btintel_probe,
+	.remove = btintel_remove,
+	.driver = {
+		.name = "btintel",
+		.acpi_match_table = ACPI_PTR(btintel_acpi_match),
+	},
+};
+module_platform_driver(btintel_driver);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] Bluetooth: Add build configuration option for Intel bluetooth rfkill driver
  2018-10-30 10:50 [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Chethan T N
@ 2018-10-30 10:50 ` Chethan T N
  2018-10-30 11:08 ` [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Hegde, Raghuram
  2018-10-30 11:08 ` Marcel Holtmann
  2 siblings, 0 replies; 5+ messages in thread
From: Chethan T N @ 2018-10-30 10:50 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: amit.k.bag, Raghuram Hegde, Chethan T N, Sukumar Ghorai

From: Raghuram Hegde <raghuram.hegde@intel.com>

CONFIG_INTEL_BT_RFKILL option enables the support for Rfkill
switch on Intel bluetooth controllers. If you have a PC with
Intel bluetooth controller,choose Y for this option.

Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
---
 drivers/platform/x86/Kconfig  | 9 +++++++++
 drivers/platform/x86/Makefile | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bf050ba644c6 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1229,6 +1229,15 @@ config I2C_MULTI_INSTANTIATE
 	  To compile this driver as a module, choose M here: the module
 	  will be called i2c-multi-instantiate.
 
+config INTEL_BT_RFKILL
+	tristate "Intel bluetooth platform rfkill support"
+	depends on ACPI
+	depends on RFKILL || !RFKILL
+	---help---
+	  This option adds support for rfkill switch on Intel bluetooth
+	  controllers.
+	  If you have a PC with Intel Bluetooth controller, choose Y.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e6d1becf81ce..af9ac3cd6151 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -92,3 +92,4 @@ obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
 obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
 obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
+obj-$(CONFIG_INTEL_BT_RFKILL)   += intel_bt_rfkill.o
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller
  2018-10-30 10:50 [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Chethan T N
  2018-10-30 10:50 ` [PATCH 2/2] Bluetooth: Add build configuration option for Intel bluetooth rfkill driver Chethan T N
@ 2018-10-30 11:08 ` Hegde, Raghuram
  2018-10-30 11:08 ` Marcel Holtmann
  2 siblings, 0 replies; 5+ messages in thread
From: Hegde, Raghuram @ 2018-10-30 11:08 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bag, Amit K, Ghorai, Sukumar, Tumkur Narayan, Chethan

Hello All,

In this patch, we have implemented the Rfkill driver for Intel Bluetooth controllers as suggested by Marcel. 
We believe this approach handles the scenario of multiple Bluetooth Controllers.

We need suggestion on how we can link this rfkill object with the corresponding hdev object, so that we can invoke the set_block function from hci core if the BT controller goes to a un-recoverable state.

Thanks
Raghuram

-----Original Message-----
From: Tumkur Narayan, Chethan 
Sent: Tuesday, October 30, 2018 4:21 PM
To: linux-bluetooth@vger.kernel.org
Cc: Bag, Amit K <amit.k.bag@intel.com>; Hegde, Raghuram <raghuram.hegde@intel.com>; Tumkur Narayan, Chethan <chethan.tumkur.narayan@intel.com>; Ghorai, Sukumar <sukumar.ghorai@intel.com>
Subject: [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller

From: Raghuram Hegde <raghuram.hegde@intel.com>

Register ACPI object INTL6205 as platform Bluetooth RfKill driver to attach/detach Intel Bluetooth controller from platform USB bus.

Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
---
 drivers/platform/x86/intel_bt_rfkill.c | 133 +++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 drivers/platform/x86/intel_bt_rfkill.c

diff --git a/drivers/platform/x86/intel_bt_rfkill.c b/drivers/platform/x86/intel_bt_rfkill.c
new file mode 100644
index 000000000000..89a3d7f4b32c
--- /dev/null
+++ b/drivers/platform/x86/intel_bt_rfkill.c
@@ -0,0 +1,133 @@
+/*
+ *  Intel Bluetooth Rfkill Driver
+ */
+
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/rfkill.h>
+#include <linux/gpio/consumer.h>
+
+struct btintel_rfkill_dev {
+	struct rfkill *rfk;
+	struct gpio_desc *reset_gpio_handler;
+};
+
+static const struct acpi_gpio_params reset_gpios = { 0, 0, false }; 
+static const struct acpi_gpio_mapping acpi_btintel_gpios[] = {
+	{ "reset-gpios", &reset_gpios, 1 },
+	{ },
+};
+
+static const struct acpi_device_id btintel_acpi_match[] = {
+	{ "INTL6205", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, btintel_acpi_match);
+
+static int btintel_disable_bt(struct gpio_desc *reset_gpio) {
+	if (!reset_gpio)
+		return -EINVAL;
+
+	/* This will detach the Intel BT controller */
+	gpiod_set_value(reset_gpio, 0);
+	return 0;
+}
+
+static int btintel_enable_bt(struct gpio_desc *reset_gpio) {
+	if (!reset_gpio)
+		return -EINVAL;
+
+	/* This will re-attach the Intel BT controller */
+	gpiod_set_value(reset_gpio, 1);
+	return 0;
+}
+
+/* RFKill handlers */
+static int bt_rfkill_set_block(void *data, bool blocked) {
+	struct btintel_rfkill_dev *bt_dev = data;
+	int ret;
+
+	if (blocked)
+		ret = btintel_disable_bt(bt_dev->reset_gpio_handler);
+	else
+		ret = btintel_enable_bt(bt_dev->reset_gpio_handler);
+
+	return ret;
+}
+static const struct rfkill_ops rfk_ops = {
+	.set_block = bt_rfkill_set_block,
+};
+
+/* ACPI object probe */
+static int btintel_probe(struct platform_device *pdev) {
+	struct btintel_rfkill_dev *bt_dev;
+	int result;
+
+	bt_dev = kzalloc(sizeof(*bt_dev), GFP_KERNEL);
+	if (!bt_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(&pdev->dev, bt_dev);
+
+	if (acpi_dev_add_driver_gpios(ACPI_COMPANION(&pdev->dev),
+				     acpi_btintel_gpios)) {
+		kfree(bt_dev);
+		return -EINVAL;
+	}
+
+	bt_dev->reset_gpio_handler = devm_gpiod_get_optional(&pdev->dev,
+					"reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(bt_dev->reset_gpio_handler)) {
+		kfree(bt_dev);
+		return PTR_ERR(bt_dev->reset_gpio_handler);
+	}
+
+	bt_dev->rfk = rfkill_alloc("intel_bluetooth",
+				   &pdev->dev,
+				   RFKILL_TYPE_BLUETOOTH,
+				   &rfk_ops,
+				   bt_dev);
+	if (!bt_dev->rfk) {
+		kfree(bt_dev);
+		return -ENOMEM;
+	}
+
+	result = rfkill_register(bt_dev->rfk);
+	if (result) {
+		rfkill_destroy(bt_dev->rfk);
+		kfree(bt_dev);
+	}
+
+	return result;
+}
+
+static int btintel_remove(struct platform_device *pdev) {
+	struct btintel_rfkill_dev *bt_dev = dev_get_drvdata(&pdev->dev);
+
+	if (bt_dev->rfk) {
+		rfkill_unregister(bt_dev->rfk);
+		rfkill_destroy(bt_dev->rfk);
+	}
+
+	kfree(bt_dev);
+
+	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
+	return 0;
+}
+
+static struct platform_driver btintel_driver = {
+	.probe = btintel_probe,
+	.remove = btintel_remove,
+	.driver = {
+		.name = "btintel",
+		.acpi_match_table = ACPI_PTR(btintel_acpi_match),
+	},
+};
+module_platform_driver(btintel_driver);
--
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller
  2018-10-30 10:50 [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Chethan T N
  2018-10-30 10:50 ` [PATCH 2/2] Bluetooth: Add build configuration option for Intel bluetooth rfkill driver Chethan T N
  2018-10-30 11:08 ` [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Hegde, Raghuram
@ 2018-10-30 11:08 ` Marcel Holtmann
  2018-10-30 13:42   ` Tumkur Narayan, Chethan
  2 siblings, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2018-10-30 11:08 UTC (permalink / raw)
  To: Chethan T N; +Cc: linux-bluetooth, amit.k.bag, Raghuram Hegde, Sukumar Ghorai

Hi Chethan,

> Register ACPI object INTL6205 as platform Bluetooth RfKill
> driver to attach/detach Intel Bluetooth controller from
> platform USB bus.
> 
> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> ---
> drivers/platform/x86/intel_bt_rfkill.c | 133 +++++++++++++++++++++++++++++++++
> 1 file changed, 133 insertions(+)
> create mode 100644 drivers/platform/x86/intel_bt_rfkill.c

I would include the Kconfig and Makefile changes here since it is a small driver.

> diff --git a/drivers/platform/x86/intel_bt_rfkill.c b/drivers/platform/x86/intel_bt_rfkill.c
> new file mode 100644
> index 000000000000..89a3d7f4b32c
> --- /dev/null
> +++ b/drivers/platform/x86/intel_bt_rfkill.c
> @@ -0,0 +1,133 @@
> +/*
> + *  Intel Bluetooth Rfkill Driver
> + */
> +
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/rfkill.h>
> +#include <linux/gpio/consumer.h>
> +
> +struct btintel_rfkill_dev {
> +	struct rfkill *rfk;

Most drivers used rfkill here instead of rfk. I only found one that used this abbreviation.

> +	struct gpio_desc *reset_gpio_handler;
> +};
> +
> +static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> +static const struct acpi_gpio_mapping acpi_btintel_gpios[] = {
> +	{ "reset-gpios", &reset_gpios, 1 },
> +	{ },
> +};
> +
> +static const struct acpi_device_id btintel_acpi_match[] = {
> +	{ "INTL6205", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, btintel_acpi_match);

Personally I put these above the platform_driver struct right before it is used.

> +
> +static int btintel_disable_bt(struct gpio_desc *reset_gpio)
> +{
> +	if (!reset_gpio)
> +		return -EINVAL;
> +
> +	/* This will detach the Intel BT controller */
> +	gpiod_set_value(reset_gpio, 0);
> +	return 0;
> +}
> +
> +static int btintel_enable_bt(struct gpio_desc *reset_gpio)
> +{
> +	if (!reset_gpio)
> +		return -EINVAL;
> +
> +	/* This will re-attach the Intel BT controller */
> +	gpiod_set_value(reset_gpio, 1);
> +	return 0;
> +}
> +
> +/* RFKill handlers */
> +static int bt_rfkill_set_block(void *data, bool blocked)
> +{
> +	struct btintel_rfkill_dev *bt_dev = data;
> +	int ret;

This driver needs a bit more consistency. Either btintel_rfkill or intel_bt_rfkill, but lets not get this confused with actual btintel_ code that we already have.

> +
> +	if (blocked)
> +		ret = btintel_disable_bt(bt_dev->reset_gpio_handler);
> +	else
> +		ret = btintel_enable_bt(bt_dev->reset_gpio_handler);
> +
> +	return ret;
> +}
> +static const struct rfkill_ops rfk_ops = {
> +	.set_block = bt_rfkill_set_block,
> +};
> +
> +/* ACPI object probe */
> +static int btintel_probe(struct platform_device *pdev)
> +{
> +	struct btintel_rfkill_dev *bt_dev;
> +	int result;
> +
> +	bt_dev = kzalloc(sizeof(*bt_dev), GFP_KERNEL);
> +	if (!bt_dev)
> +		return -ENOMEM;

Would’t it better to just use the devm_ version here and let it handle the lifetime tracking of the associated memory.

> +
> +	dev_set_drvdata(&pdev->dev, bt_dev);
> +
> +	if (acpi_dev_add_driver_gpios(ACPI_COMPANION(&pdev->dev),
> +				     acpi_btintel_gpios)) {
> +		kfree(bt_dev);
> +		return -EINVAL;
> +	}
> +
> +	bt_dev->reset_gpio_handler = devm_gpiod_get_optional(&pdev->dev,
> +					"reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(bt_dev->reset_gpio_handler)) {
> +		kfree(bt_dev);
> +		return PTR_ERR(bt_dev->reset_gpio_handler);
> +	}
> +
> +	bt_dev->rfk = rfkill_alloc("intel_bluetooth",
> +				   &pdev->dev,
> +				   RFKILL_TYPE_BLUETOOTH,
> +				   &rfk_ops,
> +				   bt_dev);
> +	if (!bt_dev->rfk) {
> +		kfree(bt_dev);
> +		return -ENOMEM;
> +	}
> +
> +	result = rfkill_register(bt_dev->rfk);
> +	if (result) {
> +		rfkill_destroy(bt_dev->rfk);
> +		kfree(bt_dev);
> +	}
> +
> +	return result;
> +}
> +
> +static int btintel_remove(struct platform_device *pdev)
> +{
> +	struct btintel_rfkill_dev *bt_dev = dev_get_drvdata(&pdev->dev);
> +
> +	if (bt_dev->rfk) {
> +		rfkill_unregister(bt_dev->rfk);
> +		rfkill_destroy(bt_dev->rfk);
> +	}
> +
> +	kfree(bt_dev);
> +
> +	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
> +
> +	return 0;
> +}
> +
> +static struct platform_driver btintel_driver = {
> +	.probe = btintel_probe,
> +	.remove = btintel_remove,
> +	.driver = {
> +		.name = "btintel”,

I don’t like this name since it is too generic. Call it btintel_rfkill or intel_bt_rfkill.

> +		.acpi_match_table = ACPI_PTR(btintel_acpi_match),
> +	},
> +};
> +module_platform_driver(btintel_driver);

Regards

Marcel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller
  2018-10-30 11:08 ` Marcel Holtmann
@ 2018-10-30 13:42   ` Tumkur Narayan, Chethan
  0 siblings, 0 replies; 5+ messages in thread
From: Tumkur Narayan, Chethan @ 2018-10-30 13:42 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Bag, Amit K, Hegde, Raghuram, Ghorai, Sukumar

Hi Marcel,

Thanks for your inputs, I will create v1 patch with the modification and sent it again.
Best  Regards
Chethan


-----Original Message-----
From: Marcel Holtmann [mailto:marcel@holtmann.org] 
Sent: Tuesday, October 30, 2018 4:38 PM
To: Tumkur Narayan, Chethan <chethan.tumkur.narayan@intel.com>
Cc: linux-bluetooth@vger.kernel.org; Bag, Amit K <amit.k.bag@intel.com>; Hegde, Raghuram <raghuram.hegde@intel.com>; Ghorai, Sukumar <sukumar.ghorai@intel.com>
Subject: Re: [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller

Hi Chethan,

> Register ACPI object INTL6205 as platform Bluetooth RfKill driver to 
> attach/detach Intel Bluetooth controller from platform USB bus.
> 
> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Sukumar Ghorai <sukumar.ghorai@intel.com>
> ---
> drivers/platform/x86/intel_bt_rfkill.c | 133 
> +++++++++++++++++++++++++++++++++
> 1 file changed, 133 insertions(+)
> create mode 100644 drivers/platform/x86/intel_bt_rfkill.c

I would include the Kconfig and Makefile changes here since it is a small driver.

> diff --git a/drivers/platform/x86/intel_bt_rfkill.c 
> b/drivers/platform/x86/intel_bt_rfkill.c
> new file mode 100644
> index 000000000000..89a3d7f4b32c
> --- /dev/null
> +++ b/drivers/platform/x86/intel_bt_rfkill.c
> @@ -0,0 +1,133 @@
> +/*
> + *  Intel Bluetooth Rfkill Driver
> + */
> +
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/rfkill.h>
> +#include <linux/gpio/consumer.h>
> +
> +struct btintel_rfkill_dev {
> +	struct rfkill *rfk;

Most drivers used rfkill here instead of rfk. I only found one that used this abbreviation.

> +	struct gpio_desc *reset_gpio_handler; };
> +
> +static const struct acpi_gpio_params reset_gpios = { 0, 0, false }; 
> +static const struct acpi_gpio_mapping acpi_btintel_gpios[] = {
> +	{ "reset-gpios", &reset_gpios, 1 },
> +	{ },
> +};
> +
> +static const struct acpi_device_id btintel_acpi_match[] = {
> +	{ "INTL6205", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, btintel_acpi_match);

Personally I put these above the platform_driver struct right before it is used.

> +
> +static int btintel_disable_bt(struct gpio_desc *reset_gpio) {
> +	if (!reset_gpio)
> +		return -EINVAL;
> +
> +	/* This will detach the Intel BT controller */
> +	gpiod_set_value(reset_gpio, 0);
> +	return 0;
> +}
> +
> +static int btintel_enable_bt(struct gpio_desc *reset_gpio) {
> +	if (!reset_gpio)
> +		return -EINVAL;
> +
> +	/* This will re-attach the Intel BT controller */
> +	gpiod_set_value(reset_gpio, 1);
> +	return 0;
> +}
> +
> +/* RFKill handlers */
> +static int bt_rfkill_set_block(void *data, bool blocked) {
> +	struct btintel_rfkill_dev *bt_dev = data;
> +	int ret;

This driver needs a bit more consistency. Either btintel_rfkill or intel_bt_rfkill, but lets not get this confused with actual btintel_ code that we already have.

> +
> +	if (blocked)
> +		ret = btintel_disable_bt(bt_dev->reset_gpio_handler);
> +	else
> +		ret = btintel_enable_bt(bt_dev->reset_gpio_handler);
> +
> +	return ret;
> +}
> +static const struct rfkill_ops rfk_ops = {
> +	.set_block = bt_rfkill_set_block,
> +};
> +
> +/* ACPI object probe */
> +static int btintel_probe(struct platform_device *pdev) {
> +	struct btintel_rfkill_dev *bt_dev;
> +	int result;
> +
> +	bt_dev = kzalloc(sizeof(*bt_dev), GFP_KERNEL);
> +	if (!bt_dev)
> +		return -ENOMEM;

Would’t it better to just use the devm_ version here and let it handle the lifetime tracking of the associated memory.

> +
> +	dev_set_drvdata(&pdev->dev, bt_dev);
> +
> +	if (acpi_dev_add_driver_gpios(ACPI_COMPANION(&pdev->dev),
> +				     acpi_btintel_gpios)) {
> +		kfree(bt_dev);
> +		return -EINVAL;
> +	}
> +
> +	bt_dev->reset_gpio_handler = devm_gpiod_get_optional(&pdev->dev,
> +					"reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(bt_dev->reset_gpio_handler)) {
> +		kfree(bt_dev);
> +		return PTR_ERR(bt_dev->reset_gpio_handler);
> +	}
> +
> +	bt_dev->rfk = rfkill_alloc("intel_bluetooth",
> +				   &pdev->dev,
> +				   RFKILL_TYPE_BLUETOOTH,
> +				   &rfk_ops,
> +				   bt_dev);
> +	if (!bt_dev->rfk) {
> +		kfree(bt_dev);
> +		return -ENOMEM;
> +	}
> +
> +	result = rfkill_register(bt_dev->rfk);
> +	if (result) {
> +		rfkill_destroy(bt_dev->rfk);
> +		kfree(bt_dev);
> +	}
> +
> +	return result;
> +}
> +
> +static int btintel_remove(struct platform_device *pdev) {
> +	struct btintel_rfkill_dev *bt_dev = dev_get_drvdata(&pdev->dev);
> +
> +	if (bt_dev->rfk) {
> +		rfkill_unregister(bt_dev->rfk);
> +		rfkill_destroy(bt_dev->rfk);
> +	}
> +
> +	kfree(bt_dev);
> +
> +	acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
> +
> +	return 0;
> +}
> +
> +static struct platform_driver btintel_driver = {
> +	.probe = btintel_probe,
> +	.remove = btintel_remove,
> +	.driver = {
> +		.name = "btintel”,

I don’t like this name since it is too generic. Call it btintel_rfkill or intel_bt_rfkill.

> +		.acpi_match_table = ACPI_PTR(btintel_acpi_match),
> +	},
> +};
> +module_platform_driver(btintel_driver);

Regards

Marcel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-30 13:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 10:50 [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Chethan T N
2018-10-30 10:50 ` [PATCH 2/2] Bluetooth: Add build configuration option for Intel bluetooth rfkill driver Chethan T N
2018-10-30 11:08 ` [PATCH 1/2] Bluetooth: Add Rfkill driver for Intel Bluetooth controller Hegde, Raghuram
2018-10-30 11:08 ` Marcel Holtmann
2018-10-30 13:42   ` Tumkur Narayan, Chethan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.