AW: [PATCH] usb: typec: tcpci: Add FAULT_ALERT handling

Commit Message

Yanik Fuchs Oct. 24, 2024, 10:29 p.m. UTC

From 031c09076392422235fc79b982cc4ab052a565b9 Mon Sep 17 00:00:00 2001
From: Yanik Fuchs <[e-mail]>
Date: Thu, 24 Oct 2024 23:36:25 +0200
Subject: [PATCH v2] usb: typec: tcpci: Add FAULT_ALERT handling
 message


Good evening

Thank you very much for the feedback! I have now
updated the patch to register the fault pin.

Changes since v1:
- added TCPC_ALERT_FAULT to pins to unmask in tcpci_init
- Added missing "{}" in if statement
- checked patch using ./scripts/checkpatch.pl

Conversation:

>________________________________________
>Von: Yanik Fuchs <[e-mail]>
>Gesendet: Dienstag, 22. Oktober 2024 21:13
>An: RD Babiera <[e-mail]>
> 
>Thank you for your fast response!
>
>I agree, since the Chip seems to need Fault alert acknowledged to work properly, it would, in my opinion, make sense to unmask the fault_alert at the init >phase prior IRQ, where the others alerts get unmasked. But I do not know if all, or just some faults trigger the fault_alert pin even if it is masked and if that >then would be an issue (like uneeded traffic on i2c). 
>
>Best regards 
>Yanik Fuchs
>
>________________________________________
>From: RD Babiera <[e-mail]>
>Sent: Tuesday, October 22, 2024 8:01:38 PM
>To: Yanik Fuchs <[e-mail]>
>
>Hi Yanik,
>
>> +   /*
>> +   * some chips asert fault alert, even if it is masked.
> >+   * The FAULT_STATUS is read and
> >+   */
> >+   if (status & TCPC_ALERT_FAULT)
> >+       regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, &raw);
> >+       regmap_write(tcpci->regmap, TCPC_FAULT_STATUS, raw);
>
>Would it make sense to register TCPC_ALERT_FAULT to the alert mask as well?
>If TCPC_ALERT_FAULT would be the only alert to trigger an IRQ, will tcpci_irq
>still run if it is masked? i.e., can this patch only read/clear the
>fault status because
>it piggybacks off of another alert?
>
>Best,
>RD

I am looking forward to hear more feedback :)

Best regards
Yanik Fuchs

Signed-off-by: Yanik Fuchs <[e-mail]>
---
 drivers/usb/typec/tcpm/tcpci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Patch

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index ed32583829be..eb6dee2f0c53 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -686,7 +686,8 @@  static int tcpci_init(struct tcpc_dev *tcpc)
 
 	reg = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_FAILED |
 		TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_RX_STATUS |
-		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
+		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS |
+		TCPC_ALERT_FAULT;
 	if (tcpci->controls_vbus)
 		reg |= TCPC_ALERT_POWER_STATUS;
 	/* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */
@@ -714,6 +715,14 @@  irqreturn_t tcpci_irq(struct tcpci *tcpci)
 	irq_ret = status & tcpci->alert_mask;
 
 process_status:
+	/*
+	 * some chips asert fault alert, even if it is masked.
+	 * The FAULT_STATUS is read and written after to acknolige
+	 */
+	if (status & TCPC_ALERT_FAULT) {
+		regmap_read(tcpci->regmap, TCPC_FAULT_STATUS, &raw);
+		regmap_write(tcpci->regmap, TCPC_FAULT_STATUS, raw);
+	}
 	/*
 	 * Clear alert status for everything except RX_STATUS, which shouldn't
 	 * be cleared until we have successfully retrieved message.