Re: [PATCH v2 0/4] usb: typec: tipd: add patch update support for tps6598x

[Abdel Alkuor]

On Fri, Jan 05, 2024 at 03:40:54PM +0530, Jai Luthra wrote:
Hi Jai and Jvaier,
> > My biggest concern is that we are sending GAID for the tps25750 under
> > the same circumstances. Could we not have the same problem with that
> > device? We would be resetting the PD controller and the SoC would stop
> > getting power as well, right? Or is there anything device-specific that
> > would avoid that?
> > 
> 
> Yes I would guess same problem can happen depending on probe order of 
> the remote-endpoint node, but I don't see any upstream platform using 
> ti,tps25750 compatible, so I have no way to confirm.
> 
> Maybe Abdel can comment on how it works, as he added the GAID reset for 
> tps25750.
> 
Ops, that's an oversight from my side. In our case, fwnode_usb_role_switch_get()
returns NULL but if it does return -EPROBE_DEFER, we will end up with
the same issue you're seeing.

The purpose of the reset is to remove any applied patch so we don't
leave USB-C PD controller in some kind of operable state when the device
fails to be probed. GO2P command forces PD controller to retrun to PTCH
mode but unfortunately that doesn't work in all cases unless ADCINx pins
configurations are set in "NegotiateHighVoltage" option, so I opted into 
using the hard reset instead regardless of ADCINx configurations.

> > > If you have a better architecture in mind that can reset only when PTCH 
> > > has been applied and not for other probe defers, feel free to send it on 
> > > top of it.
> > > 
> > 
> > I added the cold reset to have the same behavior upon probe failures for
> > both devices, given that they use the same command. But if that can
> > cause problems, let's leave the reset alone...
> > 
I think in this case, we might want to apply the patch as the last
thing in the probe or check for EPROBE_DEFER before issuing a hard
reset.

Also, I think if the patch is being applied using EEPROM, I don't
believe we need to issue a hard reset ever as the patch would be applied
automatically after that.

Thanks,
Abdel