| Message ID | 20211231115055.115988-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Queued, archived |
| Headers | show |
| Series | Bluetooth: hci_bcm: Add the Asus TF103C to the bcm_broken_irq_dmi_table | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/incremental_build | success | Pass |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
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 <hdegoede@redhat.com> > --- > 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
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 <hdegoede@redhat.com> >> --- >> 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 >
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 <hdegoede@redhat.com> >>> --- >>> 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
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 <hdegoede@redhat.com> >>>> --- >>>> 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
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);
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 <hdegoede@redhat.com> --- drivers/bluetooth/hci_bcm.c | 44 ++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-)