2021-12-31 11:51:09

by Hans de Goede

[permalink] [raw]
Subject: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table

The DSDT for the Asus TF103C specifies a IOAPIC IRQ for the HCI -> host IRQ
but this is not correct. Unlike the previous entries in the table, this
time the correct GPIO to use instead is known; and the TF103C is battery
powered making runtime-pm support more important.

Extend the bcm_broken_irq_dmi_table mechanism to allow specifying the right
GPIO instead of just always disabling runtime-pm and add an entry to it for
the Asus TF103C.

Signed-off-by: Hans de Goede <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index ef54afa29357..c6ac4aa994af 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -20,6 +20,7 @@
#include <linux/regulator/consumer.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
#include <linux/tty.h>
#include <linux/interrupt.h>
#include <linux/dmi.h>
@@ -870,7 +871,23 @@ static int bcm_resume(struct device *dev)
#endif

/* Some firmware reports an IRQ which does not work (wrong pin in fw table?) */
+static struct gpiod_lookup_table asus_tf103c_irq_gpios = {
+ .dev_id = "serial0-0",
+ .table = {
+ GPIO_LOOKUP("INT33FC:02", 17, "host-wakeup-alt", GPIO_ACTIVE_HIGH),
+ { }
+ },
+};
+
static const struct dmi_system_id bcm_broken_irq_dmi_table[] = {
+ {
+ .ident = "Asus TF103C",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
+ },
+ .driver_data = &asus_tf103c_irq_gpios,
+ },
{
.ident = "Meegopad T08",
.matches = {
@@ -1027,7 +1044,8 @@ static struct clk *bcm_get_txco(struct device *dev)

static int bcm_get_resources(struct bcm_device *dev)
{
- const struct dmi_system_id *dmi_id;
+ const struct dmi_system_id *broken_irq_dmi_id;
+ const char *irq_con_id = "host-wakeup";
int err;

dev->name = dev_name(dev->dev);
@@ -1083,23 +1101,33 @@ static int bcm_get_resources(struct bcm_device *dev)
if (err)
return err;

+ broken_irq_dmi_id = dmi_first_match(bcm_broken_irq_dmi_table);
+ if (broken_irq_dmi_id && broken_irq_dmi_id->driver_data) {
+ gpiod_add_lookup_table(broken_irq_dmi_id->driver_data);
+ irq_con_id = "host-wakeup-alt";
+ dev->irq_active_low = false;
+ dev->irq = 0;
+ }
+
/* IRQ can be declared in ACPI table as Interrupt or GpioInt */
if (dev->irq <= 0) {
struct gpio_desc *gpio;

- gpio = devm_gpiod_get_optional(dev->dev, "host-wakeup",
- GPIOD_IN);
+ gpio = devm_gpiod_get_optional(dev->dev, irq_con_id, GPIOD_IN);
if (IS_ERR(gpio))
return PTR_ERR(gpio);

dev->irq = gpiod_to_irq(gpio);
}

- dmi_id = dmi_first_match(bcm_broken_irq_dmi_table);
- if (dmi_id) {
- dev_info(dev->dev, "%s: Has a broken IRQ config, disabling IRQ support / runtime-pm\n",
- dmi_id->ident);
- dev->irq = 0;
+ if (broken_irq_dmi_id) {
+ if (broken_irq_dmi_id->driver_data) {
+ gpiod_remove_lookup_table(broken_irq_dmi_id->driver_data);
+ } else {
+ dev_info(dev->dev, "%s: Has a broken IRQ config, disabling IRQ support / runtime-pm\n",
+ broken_irq_dmi_id->ident);
+ dev->irq = 0;
+ }
}

dev_dbg(dev->dev, "BCM irq: %d\n", dev->irq);
--
2.33.1



2022-01-06 20:27:47

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table

Hi Hans,

> The DSDT for the Asus TF103C specifies a IOAPIC IRQ for the HCI -> host IRQ
> but this is not correct. Unlike the previous entries in the table, this
> time the correct GPIO to use instead is known; and the TF103C is battery
> powered making runtime-pm support more important.
>
> Extend the bcm_broken_irq_dmi_table mechanism to allow specifying the right
> GPIO instead of just always disabling runtime-pm and add an entry to it for
> the Asus TF103C.
>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index ef54afa29357..c6ac4aa994af 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -20,6 +20,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/clk.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> #include <linux/tty.h>
> #include <linux/interrupt.h>
> #include <linux/dmi.h>
> @@ -870,7 +871,23 @@ static int bcm_resume(struct device *dev)
> #endif
>
> /* Some firmware reports an IRQ which does not work (wrong pin in fw table?) */
> +static struct gpiod_lookup_table asus_tf103c_irq_gpios = {
> + .dev_id = "serial0-0",

do you need this one? I assume it could be easily enumerated as serial1-0 if you are unlucky.

> + .table = {
> + GPIO_LOOKUP("INT33FC:02", 17, "host-wakeup-alt", GPIO_ACTIVE_HIGH),
> + { }
> + },
> +};
> +
> static const struct dmi_system_id bcm_broken_irq_dmi_table[] = {
> + {
> + .ident = "Asus TF103C",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
> + },
> + .driver_data = &asus_tf103c_irq_gpios,
> + },
> {
> .ident = "Meegopad T08",
> .matches = {

Regards

Marcel


2022-01-13 15:22:50

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table

Hi Marcel,

Thank you for the review.

On 1/6/22 21:27, Marcel Holtmann wrote:
> Hi Hans,
>
>> The DSDT for the Asus TF103C specifies a IOAPIC IRQ for the HCI -> host IRQ
>> but this is not correct. Unlike the previous entries in the table, this
>> time the correct GPIO to use instead is known; and the TF103C is battery
>> powered making runtime-pm support more important.
>>
>> Extend the bcm_broken_irq_dmi_table mechanism to allow specifying the right
>> GPIO instead of just always disabling runtime-pm and add an entry to it for
>> the Asus TF103C.
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>> drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++-------
>> 1 file changed, 36 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>> index ef54afa29357..c6ac4aa994af 100644
>> --- a/drivers/bluetooth/hci_bcm.c
>> +++ b/drivers/bluetooth/hci_bcm.c
>> @@ -20,6 +20,7 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/clk.h>
>> #include <linux/gpio/consumer.h>
>> +#include <linux/gpio/machine.h>
>> #include <linux/tty.h>
>> #include <linux/interrupt.h>
>> #include <linux/dmi.h>
>> @@ -870,7 +871,23 @@ static int bcm_resume(struct device *dev)
>> #endif
>>
>> /* Some firmware reports an IRQ which does not work (wrong pin in fw table?) */
>> +static struct gpiod_lookup_table asus_tf103c_irq_gpios = {
>> + .dev_id = "serial0-0",
>
> do you need this one? I assume it could be easily enumerated as serial1-0 if you are unlucky.

Yes there can be multiple global gpiod_lookup_table-s registered
and the gpiolib code finds the one to use be matching this field
to the dev_name() for the device passed to gpiod_get().

I'm not worried about this getting enumerated with another dev_name(),
this is a tablet with no ways to add extra serial-bus devices and
there is only the 1 serial-bus device.

Regards,

Hans



>
>> + .table = {
>> + GPIO_LOOKUP("INT33FC:02", 17, "host-wakeup-alt", GPIO_ACTIVE_HIGH),
>> + { }
>> + },
>> +};
>> +
>> static const struct dmi_system_id bcm_broken_irq_dmi_table[] = {
>> + {
>> + .ident = "Asus TF103C",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
>> + },
>> + .driver_data = &asus_tf103c_irq_gpios,
>> + },
>> {
>> .ident = "Meegopad T08",
>> .matches = {
>
> Regards
>
> Marcel
>


2022-01-21 20:01:06

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table

Hi Hans,

>>> The DSDT for the Asus TF103C specifies a IOAPIC IRQ for the HCI -> host IRQ
>>> but this is not correct. Unlike the previous entries in the table, this
>>> time the correct GPIO to use instead is known; and the TF103C is battery
>>> powered making runtime-pm support more important.
>>>
>>> Extend the bcm_broken_irq_dmi_table mechanism to allow specifying the right
>>> GPIO instead of just always disabling runtime-pm and add an entry to it for
>>> the Asus TF103C.
>>>
>>> Signed-off-by: Hans de Goede <[email protected]>
>>> ---
>>> drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++-------
>>> 1 file changed, 36 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>>> index ef54afa29357..c6ac4aa994af 100644
>>> --- a/drivers/bluetooth/hci_bcm.c
>>> +++ b/drivers/bluetooth/hci_bcm.c
>>> @@ -20,6 +20,7 @@
>>> #include <linux/regulator/consumer.h>
>>> #include <linux/clk.h>
>>> #include <linux/gpio/consumer.h>
>>> +#include <linux/gpio/machine.h>
>>> #include <linux/tty.h>
>>> #include <linux/interrupt.h>
>>> #include <linux/dmi.h>
>>> @@ -870,7 +871,23 @@ static int bcm_resume(struct device *dev)
>>> #endif
>>>
>>> /* Some firmware reports an IRQ which does not work (wrong pin in fw table?) */
>>> +static struct gpiod_lookup_table asus_tf103c_irq_gpios = {
>>> + .dev_id = "serial0-0",
>>
>> do you need this one? I assume it could be easily enumerated as serial1-0 if you are unlucky.
>
> Yes there can be multiple global gpiod_lookup_table-s registered
> and the gpiolib code finds the one to use be matching this field
> to the dev_name() for the device passed to gpiod_get().
>
> I'm not worried about this getting enumerated with another dev_name(),
> this is a tablet with no ways to add extra serial-bus devices and
> there is only the 1 serial-bus device.

is there no other way to match this device?

Regards

Marcel

2022-01-21 20:02:42

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table

Hi Marcel,

On 1/19/22 20:39, Marcel Holtmann wrote:
> Hi Hans,
>
>>>> The DSDT for the Asus TF103C specifies a IOAPIC IRQ for the HCI -> host IRQ
>>>> but this is not correct. Unlike the previous entries in the table, this
>>>> time the correct GPIO to use instead is known; and the TF103C is battery
>>>> powered making runtime-pm support more important.
>>>>
>>>> Extend the bcm_broken_irq_dmi_table mechanism to allow specifying the right
>>>> GPIO instead of just always disabling runtime-pm and add an entry to it for
>>>> the Asus TF103C.
>>>>
>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>> ---
>>>> drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++-------
>>>> 1 file changed, 36 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
>>>> index ef54afa29357..c6ac4aa994af 100644
>>>> --- a/drivers/bluetooth/hci_bcm.c
>>>> +++ b/drivers/bluetooth/hci_bcm.c
>>>> @@ -20,6 +20,7 @@
>>>> #include <linux/regulator/consumer.h>
>>>> #include <linux/clk.h>
>>>> #include <linux/gpio/consumer.h>
>>>> +#include <linux/gpio/machine.h>
>>>> #include <linux/tty.h>
>>>> #include <linux/interrupt.h>
>>>> #include <linux/dmi.h>
>>>> @@ -870,7 +871,23 @@ static int bcm_resume(struct device *dev)
>>>> #endif
>>>>
>>>> /* Some firmware reports an IRQ which does not work (wrong pin in fw table?) */
>>>> +static struct gpiod_lookup_table asus_tf103c_irq_gpios = {
>>>> + .dev_id = "serial0-0",
>>>
>>> do you need this one? I assume it could be easily enumerated as serial1-0 if you are unlucky.
>>
>> Yes there can be multiple global gpiod_lookup_table-s registered
>> and the gpiolib code finds the one to use be matching this field
>> to the dev_name() for the device passed to gpiod_get().
>>
>> I'm not worried about this getting enumerated with another dev_name(),
>> this is a tablet with no ways to add extra serial-bus devices and
>> there is only the 1 serial-bus device.
>
> is there no other way to match this device?

I'm not sure what you mean ny "matching this device" ? Since the ACPI
tables on this tablet contain the wrong GPIO, we need a gpio-lookup
table override and the gpio cores checks for a lookup table to use with
a specific struct device * based on dev_name() and only based on that,
there are no other matching options in the gpio-lookup mechanism.

Regards,

Hans