linux-kernel - Re: [PATCH RESEND 2/2] usb: typec: tipd: fix event checking 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zg+fD6w1MykCsEe6@kuha.fi.intel.com>
Date: Fri, 5 Apr 2024 09:49:51 +0300
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Javier Carrasco <javier.carrasco@...fvision.net>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Abdel Alkuor <abdelalkuor@...tab.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH RESEND 2/2] usb: typec: tipd: fix event checking for
 tps6598x

On Wed, Apr 03, 2024 at 10:55:29AM +0200, Javier Carrasco wrote:
> >> -	ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1);
> >> -	ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2);
> >> +	ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT1, event1, 11);
> > 
> > This is not going to work with the older TI PD controllers.
> > 
> > The lenght of these registers is 8 bytes on the older TI PD
> > controllers (TPS65981, TPS65982, etc.). I think we need to split this
> > function.
> > 
> 
> That is a good point. I had a look at the older TI PD controllers and I
> agree with you that we should split the function to cover both register
> lengths separately.
> 
> I was thinking about adding a new compatible for the newer PD
> controllers (tps65987 and tps65988), keeping the current tps6598x for
> the older ones as well as backwards compatibility. But backwards
> compatibility would also mean that flags beyond the first 8 bytes would
> be ignored.
> 
> On the other hand, the upper flags are only relevant for firmware
> updates, so we could check those (i.e. read 11 bytes) if a firmware was
> provided via "firmware-name", and ignore them (i.e. read 8 bytes) otherwise.
> 
> Other ideas or improvements to mine are more than welcome.

I don't have any good ideas. On ACPI platforms the same device ID may
be used with all of these, so we should actually try to figure out the
version from registers like VID, DID and Version (if they are
available).

thanks,

-- 
heikki