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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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@collabora.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