Received-SPF: pass (google.com: domain of linux-kernel+bounces-132494-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 5 Apr 2024 09:49:51 +0300
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
Message-ID: <Zg+fD6w1MykCsEe6@kuha.fi.intel.com>
References: <20240328-tps6598x_fix_event_handling-v1-0-502721ff705b@wolfvision.net>
 <20240328-tps6598x_fix_event_handling-v1-2-502721ff705b@wolfvision.net>
 <ZgveG5Ly3mw0O0eo@kuha.fi.intel.com>
 <b6bf7f8e-7d46-4b70-930c-9483f13fd80a@wolfvision.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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