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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZZgw5SNZiG4VlhZD@abdel>
Date: Fri, 5 Jan 2024 11:40:05 -0500
From: Abdel Alkuor <alkuor@...il.com>
To: Jai Luthra <j-luthra@...com>,
	Javier Carrasco <javier.carrasco@...fvision.net>
Cc: Abdel Alkuor <abdelalkuor@...tab.com>, rogerq@...nel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	vigneshr@...com, d-gole@...com, nm@...com
Subject: Re: [PATCH v2 0/4] usb: typec: tipd: add patch update support for
 tps6598x

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