2017-10-06 07:34:30

by Ian W MORRISON

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_bcm: Correct context of IRQ polarity message

As the overwriting of IRQ polarity to active low occurs during the driver
probe using 'bt_dev_warn' to display the warning results in '(null)'
being displayed for the device. This patch uses the underlying 'BT_INFO'
to directly display the warning instead.

Signed-off-by: ianwmorrison <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 77326eeb6146..17c036599407 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -839,7 +839,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)

dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
if (dmi_id) {
- bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
+ BT_INFO("%s: Overwriting IRQ polarity to active low",
dmi_id->ident);
dev->irq_active_low = true;
}
--
2.11.0


2017-10-07 06:16:15

by Ian W MORRISON

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Correct context of IRQ polarity message

On 10/7/17 5:26 AM, Marcel Holtmann wrote:
<snip>
>
> This should have been a dev_warn(dev->dev, ..) call actually.
>
> Regards
>
> Marcel
>

Hi Marcel,

I've changed the call to 'dev_warn' and issued as a revised patch.

Testing shows the following improvements using an extract from dmesg of the affected message:

Without patch:
[ 5.512175] Bluetooth: (null): MINIX Z83-4: Overwriting IRQ polarity to active low

v1 (superseded) patch:
[ 5.592238] Bluetooth: MINIX Z83-4: Overwriting IRQ polarity to active low

v2 patch (with a kernel excluding serdev ACPI support patches):
[ 6.112265] hci_bcm BCM2EA4:00: MINIX Z83-4: Overwriting IRQ polarity to active low

v2 patch (with a kernel including serdev ACPI support patches):
[ 6.367829] hci_uart_bcm serial1-0: MINIX Z83-4: Overwriting IRQ polarity to active low

Regards,
Ian

2017-10-06 18:26:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Correct context of IRQ polarity message

Hi Fred,

>> As the overwriting of IRQ polarity to active low occurs during the driver
>> probe using 'bt_dev_warn' to display the warning results in '(null)'
>> being displayed for the device. This patch uses the underlying 'BT_INFO'
>> to directly display the warning instead.
>>
>> Signed-off-by: ianwmorrison <[email protected]>
>> ---
>> drivers/bluetooth/hci_bcm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index 77326eeb6146..17c036599407 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -839,7 +839,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>> dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
>> if (dmi_id) {
>> - bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
>> + BT_INFO("%s: Overwriting IRQ polarity to active low",
>> dmi_id->ident);
>> dev->irq_active_low = true;
>> }
>
> We can also set the device name before calling bcm_acpi_probe(), i.e. moving "dev->name = dev_name(dev->dev);" from bcm_get_resources() to bcm_probe() and bcm_serdev_probe().

frankly I have no idea on how this works at all. We are using bt_dev_warn on a bcm_device struct. It is meant to be used on a hci_dev struct. So by pure luck bcm_device->name actually exists.

This should have been a dev_warn(dev->dev, ..) call actually.

Regards

Marcel


2017-10-06 18:15:36

by Frédéric Danis

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Correct context of IRQ polarity message

Hi Ian,

Le 06/10/2017 à 09:34, Ian W MORRISON a écrit :
> As the overwriting of IRQ polarity to active low occurs during the driver
> probe using 'bt_dev_warn' to display the warning results in '(null)'
> being displayed for the device. This patch uses the underlying 'BT_INFO'
> to directly display the warning instead.
>
> Signed-off-by: ianwmorrison <[email protected]>
> ---
> drivers/bluetooth/hci_bcm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 77326eeb6146..17c036599407 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -839,7 +839,7 @@ static int bcm_acpi_probe(struct bcm_device *dev)
>
> dmi_id = dmi_first_match(bcm_active_low_irq_dmi_table);
> if (dmi_id) {
> - bt_dev_warn(dev, "%s: Overwriting IRQ polarity to active low",
> + BT_INFO("%s: Overwriting IRQ polarity to active low",
> dmi_id->ident);
> dev->irq_active_low = true;
> }

We can also set the device name before calling bcm_acpi_probe(), i.e.
moving "dev->name = dev_name(dev->dev);" from bcm_get_resources() to
bcm_probe() and bcm_serdev_probe().

Regards,

Fred