* [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform
@ 2020-09-03 18:48 Andy Shevchenko
2020-09-03 18:48 ` [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values Andy Shevchenko
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Andy Shevchenko @ 2020-09-03 18:48 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, linux-bluetooth; +Cc: Andy Shevchenko
On new Intel platform the device is provided with INT33E3 ID.
Append it to the list.
This will require ACPI_GPIO_QUIRK_ONLY_GPIOIO to be enabled because
the relevant ASL looks like:
UartSerialBusV2 ( ... )
GpioInt ( ... ) { ... }
GpioIo ( ... ) { ... }
which means that first GPIO resource is an interrupt, while we are expecting it
to be reset one (output). Do the same for host-wake because in case of
GpioInt() the platform_get_irq() will do the job and should return correct
Linux IRQ number. That said, host-wake GPIO can only be GpioIo() resource.
While here, drop commas in terminator lines.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/bluetooth/hci_intel.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index f1299da6eed8..703d774be5a6 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -1076,7 +1076,8 @@ static const struct hci_uart_proto intel_proto = {
#ifdef CONFIG_ACPI
static const struct acpi_device_id intel_acpi_match[] = {
{ "INT33E1", 0 },
- { },
+ { "INT33E3", 0 },
+ { }
};
MODULE_DEVICE_TABLE(acpi, intel_acpi_match);
#endif
@@ -1138,9 +1139,9 @@ static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
static const struct acpi_gpio_params host_wake_gpios = { 1, 0, false };
static const struct acpi_gpio_mapping acpi_hci_intel_gpios[] = {
- { "reset-gpios", &reset_gpios, 1 },
- { "host-wake-gpios", &host_wake_gpios, 1 },
- { },
+ { "reset-gpios", &reset_gpios, 1, ACPI_GPIO_QUIRK_ONLY_GPIOIO },
+ { "host-wake-gpios", &host_wake_gpios, 1, ACPI_GPIO_QUIRK_ONLY_GPIOIO },
+ { }
};
static int intel_probe(struct platform_device *pdev)
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
@ 2020-09-03 18:48 ` Andy Shevchenko
2020-09-11 6:51 ` Marcel Holtmann
2020-09-03 18:48 ` [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry() Andy Shevchenko
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2020-09-03 18:48 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, linux-bluetooth; +Cc: Andy Shevchenko
Sparse rightfully complains:
hci_intel.c:696:26: warning: cast to restricted __le16
hci_intel.c:701:26: warning: cast to restricted __le16
hci_intel.c:702:26: warning: cast to restricted __le16
hci_intel.c:703:26: warning: cast to restricted __le16
hci_intel.c:725:26: warning: cast to restricted __le16
hci_intel.c:730:26: warning: cast to restricted __le16
hci_intel.c:731:26: warning: cast to restricted __le16
hci_intel.c:732:26: warning: cast to restricted __le16
because we access non-restricted types with le16_to_cpu().
More confusion is added by using above against u8. On big-endian
architecture we will get all zeroes. I bet it's not what should be
in such case.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/bluetooth/hci_intel.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 703d774be5a6..50e4fc6813c2 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -693,14 +693,11 @@ static int intel_setup(struct hci_uart *hu)
case 0x0b: /* SfP */
case 0x0c: /* WsP */
snprintf(fwname, sizeof(fwname), "intel/ibt-%u-%u.sfi",
- le16_to_cpu(ver.hw_variant),
- le16_to_cpu(params.dev_revid));
+ ver.hw_variant, le16_to_cpu(params.dev_revid));
break;
case 0x12: /* ThP */
snprintf(fwname, sizeof(fwname), "intel/ibt-%u-%u-%u.sfi",
- le16_to_cpu(ver.hw_variant),
- le16_to_cpu(ver.hw_revision),
- le16_to_cpu(ver.fw_revision));
+ ver.hw_variant, ver.hw_revision, ver.fw_revision);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hardware variant (%u)",
@@ -722,14 +719,11 @@ static int intel_setup(struct hci_uart *hu)
case 0x0b: /* SfP */
case 0x0c: /* WsP */
snprintf(fwname, sizeof(fwname), "intel/ibt-%u-%u.ddc",
- le16_to_cpu(ver.hw_variant),
- le16_to_cpu(params.dev_revid));
+ ver.hw_variant, le16_to_cpu(params.dev_revid));
break;
case 0x12: /* ThP */
snprintf(fwname, sizeof(fwname), "intel/ibt-%u-%u-%u.ddc",
- le16_to_cpu(ver.hw_variant),
- le16_to_cpu(ver.hw_revision),
- le16_to_cpu(ver.fw_revision));
+ ver.hw_variant, ver.hw_revision, ver.fw_revision);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hardware variant (%u)",
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry()
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
2020-09-03 18:48 ` [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values Andy Shevchenko
@ 2020-09-03 18:48 ` Andy Shevchenko
2020-09-11 6:51 ` Marcel Holtmann
2020-09-03 18:48 ` [PATCH v1 4/4] Bluetooth: hci_intel: sort headers alphabetically Andy Shevchenko
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2020-09-03 18:48 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, linux-bluetooth; +Cc: Andy Shevchenko
There is no need to have list_for_each() followed by list_entry()
when we simply may use list_for_each_entry() directly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/bluetooth/hci_intel.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 50e4fc6813c2..b20a40fab83e 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -288,7 +288,7 @@ static irqreturn_t intel_irq(int irq, void *dev_id)
static int intel_set_power(struct hci_uart *hu, bool powered)
{
- struct list_head *p;
+ struct intel_device *idev;
int err = -ENODEV;
if (!hu->tty->dev)
@@ -296,10 +296,7 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
mutex_lock(&intel_device_list_lock);
- list_for_each(p, &intel_device_list) {
- struct intel_device *idev = list_entry(p, struct intel_device,
- list);
-
+ list_for_each_entry(idev, &intel_device_list, list) {
/* tty device and pdev device should share the same parent
* which is the UART port.
*/
@@ -362,19 +359,16 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
static void intel_busy_work(struct work_struct *work)
{
- struct list_head *p;
struct intel_data *intel = container_of(work, struct intel_data,
busy_work);
+ struct intel_device *idev;
if (!intel->hu->tty->dev)
return;
/* Link is busy, delay the suspend */
mutex_lock(&intel_device_list_lock);
- list_for_each(p, &intel_device_list) {
- struct intel_device *idev = list_entry(p, struct intel_device,
- list);
-
+ list_for_each_entry(idev, &intel_device_list, list) {
if (intel->hu->tty->dev->parent == idev->pdev->dev.parent) {
pm_runtime_get(&idev->pdev->dev);
pm_runtime_mark_last_busy(&idev->pdev->dev);
@@ -533,7 +527,7 @@ static int intel_setup(struct hci_uart *hu)
struct sk_buff *skb;
struct intel_version ver;
struct intel_boot_params params;
- struct list_head *p;
+ struct intel_device *idev;
const struct firmware *fw;
char fwname[64];
u32 boot_param;
@@ -833,13 +827,11 @@ static int intel_setup(struct hci_uart *hu)
* until further LPM TX notification.
*/
mutex_lock(&intel_device_list_lock);
- list_for_each(p, &intel_device_list) {
- struct intel_device *dev = list_entry(p, struct intel_device,
- list);
+ list_for_each_entry(idev, &intel_device_list, list) {
if (!hu->tty->dev)
break;
- if (hu->tty->dev->parent == dev->pdev->dev.parent) {
- if (device_may_wakeup(&dev->pdev->dev)) {
+ if (hu->tty->dev->parent == idev->pdev->dev.parent) {
+ if (device_may_wakeup(&idev->pdev->dev)) {
set_bit(STATE_LPM_ENABLED, &intel->flags);
set_bit(STATE_TX_ACTIVE, &intel->flags);
}
@@ -993,7 +985,7 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
{
struct intel_data *intel = hu->priv;
- struct list_head *p;
+ struct intel_device *idev;
BT_DBG("hu %p skb %p", hu, skb);
@@ -1004,10 +996,7 @@ static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
* completed before enqueuing any packet.
*/
mutex_lock(&intel_device_list_lock);
- list_for_each(p, &intel_device_list) {
- struct intel_device *idev = list_entry(p, struct intel_device,
- list);
-
+ list_for_each_entry(idev, &intel_device_list, list) {
if (hu->tty->dev->parent == idev->pdev->dev.parent) {
pm_runtime_get_sync(&idev->pdev->dev);
pm_runtime_mark_last_busy(&idev->pdev->dev);
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 4/4] Bluetooth: hci_intel: sort headers alphabetically
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
2020-09-03 18:48 ` [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values Andy Shevchenko
2020-09-03 18:48 ` [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry() Andy Shevchenko
@ 2020-09-03 18:48 ` Andy Shevchenko
2020-09-09 15:58 ` [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
2020-09-11 7:04 ` Marcel Holtmann
4 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2020-09-03 18:48 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, linux-bluetooth; +Cc: Andy Shevchenko
Sort headers alphabetically to increase readability
and make maintenance easier.
While here, update copyright year.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/bluetooth/hci_intel.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index b20a40fab83e..17f51bc17bb5 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -3,21 +3,21 @@
*
* Bluetooth HCI UART driver for Intel devices
*
- * Copyright (C) 2015 Intel Corporation
+ * Copyright (C) 2015,2020 Intel Corporation
*/
-#include <linux/kernel.h>
+#include <linux/acpi.h>
#include <linux/errno.h>
-#include <linux/skbuff.h>
#include <linux/firmware.h>
-#include <linux/module.h>
-#include <linux/wait.h>
-#include <linux/tty.h>
-#include <linux/platform_device.h>
#include <linux/gpio/consumer.h>
-#include <linux/acpi.h>
#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/skbuff.h>
+#include <linux/tty.h>
+#include <linux/wait.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
` (2 preceding siblings ...)
2020-09-03 18:48 ` [PATCH v1 4/4] Bluetooth: hci_intel: sort headers alphabetically Andy Shevchenko
@ 2020-09-09 15:58 ` Andy Shevchenko
2020-09-11 7:04 ` Marcel Holtmann
4 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2020-09-09 15:58 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, linux-bluetooth
On Thu, Sep 03, 2020 at 09:48:47PM +0300, Andy Shevchenko wrote:
> On new Intel platform the device is provided with INT33E3 ID.
> Append it to the list.
>
> This will require ACPI_GPIO_QUIRK_ONLY_GPIOIO to be enabled because
> the relevant ASL looks like:
>
> UartSerialBusV2 ( ... )
> GpioInt ( ... ) { ... }
> GpioIo ( ... ) { ... }
>
> which means that first GPIO resource is an interrupt, while we are expecting it
> to be reset one (output). Do the same for host-wake because in case of
> GpioInt() the platform_get_irq() will do the job and should return correct
> Linux IRQ number. That said, host-wake GPIO can only be GpioIo() resource.
>
> While here, drop commas in terminator lines.
Any comments?
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/bluetooth/hci_intel.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> index f1299da6eed8..703d774be5a6 100644
> --- a/drivers/bluetooth/hci_intel.c
> +++ b/drivers/bluetooth/hci_intel.c
> @@ -1076,7 +1076,8 @@ static const struct hci_uart_proto intel_proto = {
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id intel_acpi_match[] = {
> { "INT33E1", 0 },
> - { },
> + { "INT33E3", 0 },
> + { }
> };
> MODULE_DEVICE_TABLE(acpi, intel_acpi_match);
> #endif
> @@ -1138,9 +1139,9 @@ static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> static const struct acpi_gpio_params host_wake_gpios = { 1, 0, false };
>
> static const struct acpi_gpio_mapping acpi_hci_intel_gpios[] = {
> - { "reset-gpios", &reset_gpios, 1 },
> - { "host-wake-gpios", &host_wake_gpios, 1 },
> - { },
> + { "reset-gpios", &reset_gpios, 1, ACPI_GPIO_QUIRK_ONLY_GPIOIO },
> + { "host-wake-gpios", &host_wake_gpios, 1, ACPI_GPIO_QUIRK_ONLY_GPIOIO },
> + { }
> };
>
> static int intel_probe(struct platform_device *pdev)
> --
> 2.28.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry()
2020-09-03 18:48 ` [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry() Andy Shevchenko
@ 2020-09-11 6:51 ` Marcel Holtmann
0 siblings, 0 replies; 8+ messages in thread
From: Marcel Holtmann @ 2020-09-11 6:51 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Johan Hedberg, linux-bluetooth
Hi Andy,
> There is no need to have list_for_each() followed by list_entry()
> when we simply may use list_for_each_entry() directly.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/bluetooth/hci_intel.c | 31 ++++++++++---------------------
> 1 file changed, 10 insertions(+), 21 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values
2020-09-03 18:48 ` [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values Andy Shevchenko
@ 2020-09-11 6:51 ` Marcel Holtmann
0 siblings, 0 replies; 8+ messages in thread
From: Marcel Holtmann @ 2020-09-11 6:51 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Johan Hedberg, linux-bluetooth
Hi Andy,
> Sparse rightfully complains:
>
> hci_intel.c:696:26: warning: cast to restricted __le16
> hci_intel.c:701:26: warning: cast to restricted __le16
> hci_intel.c:702:26: warning: cast to restricted __le16
> hci_intel.c:703:26: warning: cast to restricted __le16
> hci_intel.c:725:26: warning: cast to restricted __le16
> hci_intel.c:730:26: warning: cast to restricted __le16
> hci_intel.c:731:26: warning: cast to restricted __le16
> hci_intel.c:732:26: warning: cast to restricted __le16
>
> because we access non-restricted types with le16_to_cpu().
> More confusion is added by using above against u8. On big-endian
> architecture we will get all zeroes. I bet it's not what should be
> in such case.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/bluetooth/hci_intel.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
` (3 preceding siblings ...)
2020-09-09 15:58 ` [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
@ 2020-09-11 7:04 ` Marcel Holtmann
4 siblings, 0 replies; 8+ messages in thread
From: Marcel Holtmann @ 2020-09-11 7:04 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Johan Hedberg, linux-bluetooth
Hi Andy,
> On new Intel platform the device is provided with INT33E3 ID.
> Append it to the list.
>
> This will require ACPI_GPIO_QUIRK_ONLY_GPIOIO to be enabled because
> the relevant ASL looks like:
>
> UartSerialBusV2 ( ... )
> GpioInt ( ... ) { ... }
> GpioIo ( ... ) { ... }
>
> which means that first GPIO resource is an interrupt, while we are expecting it
> to be reset one (output). Do the same for host-wake because in case of
> GpioInt() the platform_get_irq() will do the job and should return correct
> Linux IRQ number. That said, host-wake GPIO can only be GpioIo() resource.
>
> While here, drop commas in terminator lines.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/bluetooth/hci_intel.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-11 7:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 18:48 [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
2020-09-03 18:48 ` [PATCH v1 2/4] Bluetooth: hci_intel: drop strange le16_to_cpu() against u8 values Andy Shevchenko
2020-09-11 6:51 ` Marcel Holtmann
2020-09-03 18:48 ` [PATCH v1 3/4] Bluetooth: hci_intel: switch to list_for_each_entry() Andy Shevchenko
2020-09-11 6:51 ` Marcel Holtmann
2020-09-03 18:48 ` [PATCH v1 4/4] Bluetooth: hci_intel: sort headers alphabetically Andy Shevchenko
2020-09-09 15:58 ` [PATCH v1 1/4] Bluetooth: hci_intel: enable on new platform Andy Shevchenko
2020-09-11 7:04 ` Marcel Holtmann
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.