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

AngeloGioacchino Del Regno
2025-02-19T11:40:38Z

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