Message-ID: <dcf0b4bc-1b4c-eeca-42a1-6ac92a0e1275@redhat.com>
Date:   Thu, 13 Jan 2022 16:22:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH] Bluetooth: hci_bcm: Add the Asus TF103C to the
 bcm_broken_irq_dmi_table
Content-Language: en-US
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        linux-bluetooth@vger.kernel.org
References: <20211231115055.115988-1-hdegoede@redhat.com>
 <FE6FDB9A-2188-45EE-9605-CF5A5B7E43A1@holtmann.org>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <FE6FDB9A-2188-45EE-9605-CF5A5B7E43A1@holtmann.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
>