On 2019-05-08 10:22, Guenter Roeck wrote:
On Wed, May 08, 2019 at 07:48:43AM -0600, Angus Ainslie wrote:
Hi Guenter
On 2019-05-07 23:18, Guenter Roeck wrote:
>On 5/7/19 7:49 PM, Angus Ainslie wrote:
>>On 2019-05-07 20:03, Guenter Roeck wrote:
>>>On 5/7/19 5:27 PM, Angus Ainslie (Purism) wrote:
>>>>If the fault status register doesn't get cleared then
>>>>the ptn5110 interrupt gets stuck on. As the fault register gets
>>>>set everytime the ptn5110 powers on the interrupt is always stuck.
>>>>
>>>>Fixes: fault status register stuck
>>>>Signed-off-by: Angus Ainslie (Purism) <angus@xxxxxxxx>
>>>>---
>>>> drivers/usb/typec/tcpm/tcpci.c | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>>diff --git a/drivers/usb/typec/tcpm/tcpci.c
>>>>b/drivers/usb/typec/tcpm/tcpci.c
>>>>index c1f7073a56de..a5746657b190 100644
>>>>--- a/drivers/usb/typec/tcpm/tcpci.c
>>>>+++ b/drivers/usb/typec/tcpm/tcpci.c
>>>>@@ -463,6 +463,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
>>>> else if (status & TCPC_ALERT_TX_FAILED)
>>>> tcpm_pd_transmit_complete(tcpci->port, TCPC_TX_FAILED);
>>>> + if (status & TCPC_ALERT_FAULT) {
>>>
>>>Wait - the driver doesn't set TCPC_ALERT_FAULT in the alert mask
>>>register. How can the chip report it if fault alerts are not enabled ?
>>
>>Well that I didn't check. But I know this code gets executed so
>>something must be turning it on.
>>
>>Also if I don't clear it I get an unlimited number of interrupts.
>>
>>>What am I missing here ?
>>
>>Can the power on fault be masked ?
>>
>
>There is a TCPC_ALERT_FAULT mask bit, so I would think so.
>Can you dump register contents in the irq function and at the end of
>tcpci_init() ?
>
Ok so this seems to be related to imx8mq errata e7805:
I2C: When the I2C clock speed is configured for 400 kHz, the SCL low
period
violates the I2C spec of
1.3 uS min
The work around suggested by NXP is to set the clock to 384 kHz so
that is
what I did and this is the output:
[ 4.091512] device: 'tcpm-source-psy-0-0052': device_add
[ 4.091581] PM: Adding info for No Bus:tcpm-source-psy-0-0052
[ 4.091596] device: 'tcpm-source-psy-0-0052': dev_uevent: class
uevent()
returned -11
[ 4.094774] tcpci 0-0052: ALERT MASK 0x7f
[ 4.107869] driver: 'tcpci': driver_bound: bound to device '0-0052'
[ 4.107935] bus: 'i2c': really_probe: bound device 0-0052 to driver
tcpci
[ 4.110994] tcpci 0-0052: ALERT MASK 0x7f
[ 4.115511] tcpci 0-0052: FAULT ALERT status 0x80
[ 4.126332] tcpci 0-0052: ALERT MASK 0x7f
[ 4.130784] tcpci 0-0052: FAULT ALERT status 0x0
The first "ALERT MASK" is in the init function immediately after
setting
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;
if (tcpci->controls_vbus)
reg |= TCPC_ALERT_POWER_STATUS;
ret = tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
So it looks like the register is correct but the fault interrupt still
fires. At 200 kHz I get the following output.
[ 4.136845] device: 'tcpm-source-psy-0-0052': device_add
[ 4.136943] PM: Adding info for No Bus:tcpm-source-psy-0-0052
[ 4.136966] device: 'tcpm-source-psy-0-0052': dev_uevent: class
uevent()
returned -11
[ 4.178510] tcpci 0-0052: ALERT MASK 0x7f
[ 4.217197] driver: 'tcpci': driver_bound: bound to device '0-0052'
[ 4.217371] bus: 'i2c': really_probe: bound device 0-0052 to driver
tcpci
So this is what is expected no fault interrupt.
Maybe errata e7805 needs an update.
Sorry for the noise.
Let's not jump to conclusions; I don't think this is noise. It is more
likely that the i2c problem uncovers a race condition in tcpci_init().
In tcpci_init(), we first clear TCPC_ALERT by writing 0xffff into it.
Subsequently, we set TCPC_ALERT_MASK. I suspect what may happen is
that the chip has FAULT_ALERT enabled, and that a fault was logged.
We don't clear the FAULT_STATUS register in tcpci_init(), thus
FAULT_ALERT is immediately set again, before we clear the FAULT_ALERT
mask bit.
Ok but wouldn't slowing down the bus speed make this more likely to
happen than less ?
The standard says that the alert pin shall not be set if the respective
interrupt is masked, but maybe the chip doesn't follow that. Either
case,
the standard does say that masked alerts are still reported in the
status
registers, so it is not surprising that the fault status is reported.
What we should probably do in tcpci_init() is to change the register
initialization order, and to clear the fault status register.
- Set TCPC_ALERT_MASK
- Set FAULT_STATUS_MASK (to 0)
- Clear TCPC_FAULT_STATUS
- Clear TCPC_ALERT
I suspect that will fix your problem.
I'll try and get time to give that a shot.
Another question is if TCPC_ALERT_FAULT (together with appropriate
FAULT_STATUS_MASK bits) should be enabled, and if faults should be
logged. But that would be a separate patch or patch series.
I was thinking this too but it also falls into the if I can find time
category.
Angus
Thanks,
Guenter
