Re: [PATCH RESEND 2/2] usb: typec: tipd: fix event checking for tps6598x

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Javier Carrasco <javier.carrasco@wolfvision.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Abdel Alkuor <abdelalkuor@geotab.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH RESEND 2/2] usb: typec: tipd: fix event checking for tps6598x
Date: Fri, 5 Apr 2024 09:49:51 +0300	[thread overview]
Message-ID: <Zg+fD6w1MykCsEe6@kuha.fi.intel.com> (raw)
In-Reply-To: <b6bf7f8e-7d46-4b70-930c-9483f13fd80a@wolfvision.net>

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