linux-kernel - Re: [PATCH] usb: typec: tcpci_rt1711h: Unmask alert interrupts to fix functionality

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2e49e0ca-3542-4b6f-b92f-cd5631b14f97@collabora.com>
Date: Wed, 19 Feb 2025 12:40:35 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: heikki.krogerus@...ux.intel.com, andre.draszik@...aro.org,
 linux@...ck-us.net, shufan_lee@...htek.com, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel@...labora.com, pablo.sun@...iatek.com
Subject: Re: [PATCH] usb: typec: tcpci_rt1711h: Unmask alert interrupts to fix
 functionality

Il 14/02/25 09:10, Greg KH ha scritto:
> On Wed, Feb 12, 2025 at 11:40:40AM +0100, AngeloGioacchino Del Regno wrote:
>> During probe, the TCPC alert interrupts are getting masked to
>> avoid unwanted interrupts during chip setup: this is ok to do
>> but there is no unmasking happening at any later time, which
>> means that the chip will not raise any interrupt, essentially
>> making it not functional as, while internally it does perform
>> all of the intended functions, it won't signal anything to the
>> outside.
>>
>> Unmask the alert interrupts to fix functionality.
>>
>> Fixes: ce08eaeb6388 ("staging: typec: rt1711h typec chip driver")
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> ---
>>   drivers/usb/typec/tcpm/tcpci_rt1711h.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
>> index 64f6dd0dc660..c71b213b2441 100644
>> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
>> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
>> @@ -334,6 +334,11 @@ static int rt1711h_probe(struct i2c_client *client)
>>   {
>>   	int ret;
>>   	struct rt1711h_chip *chip;
>> +	const u16 alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED |
>> +			       TCPC_ALERT_TX_FAILED | TCPC_ALERT_RX_HARD_RST |
>> +			       TCPC_ALERT_RX_STATUS | TCPC_ALERT_POWER_STATUS |
>> +			       TCPC_ALERT_CC_STATUS | TCPC_ALERT_RX_BUF_OVF |
>> +			       TCPC_ALERT_FAULT;
>>   
>>   	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
>>   	if (!chip)
>> @@ -384,6 +389,11 @@ static int rt1711h_probe(struct i2c_client *client)
>>   		return ret;
>>   	enable_irq_wake(client->irq);
>>   
>> +	/* Enable alert interrupts */
>> +	ret = rt1711h_write16(chip, TCPC_ALERT_MASK, alert_mask);
>> +	if (ret < 0)
>> +		return ret;
> 
> If this fails, don't you then need to disable the wake irq you just
> enabled?  If not, why not?
> 

Right, that was naive on my side, I was relying on devm_request_threaded_irq
but then the wakeup interrupt stays there, so that needs to be addressed.

Expect a v2 today.

Cheers,
Angelo

> thanks,
> 
> greg k-h