linux-kernel - Re: [PATCH CFT] usb: ucsi_ccg: Fix command completion handling

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]

Date: Thu, 29 Feb 2024 21:13:25 +0100
From: "Christian A. Ehrhardt" <lk@...e.de>
To: HaoTien Hsu <haotienh@...dia.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
	Sing-Han Chen <singhanc@...dia.com>,
	Utkarsh Patel <utkarsh.h.patel@...el.com>,
	Jon Hunter <jonathanh@...dia.com>, Wayne Chang <waynec@...dia.com>,
	WK Tsai <wtsai@...dia.com>
Subject: Re: [PATCH CFT] usb: ucsi_ccg: Fix command completion handling


Hi Haotien,

On Thu, Feb 29, 2024 at 07:18:44AM +0000, HaoTien Hsu wrote:
> On 2/15/24 20:03, Christian A. Ehrhardt wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > Hi Greg,
> > 
> > On Thu, Feb 15, 2024 at 12:07:20PM +0100, Greg Kroah-Hartman wrote:
> >> On Thu, Feb 15, 2024 at 11:10:24AM +0100, Christian A. Ehrhardt wrote:
> >>> In case of a spurious or otherwise delayed interrupt
> >>> it is possible that CCI still reports the previous completion.
> >>> For this reason the UCSI spec provides different completion
> >>> bits for normal commands and for UCSI_ACK_CC_CI.
> >>>
> >>> Only complete a sync command if the correct completion bit
> >>> is set.
> >>>
> >>> This should avoid the need to clear out CCI before starting
> >>> a command. Thus remove this code.
> >>>
> >>> Signed-off-by: Christian A. Ehrhardt <lk@...e.de>
> >>> Fixes: e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
> >>
> >> What does "CFT" in your subject line mean?
> > 
> > It's supposed to mean "Call For Testers". More info in the
> > "Additional Information" section of the original mail.
> > 
> > I think the change is necessary and good but I do not have the HW
> > to test it.
> > 
> > I did test a similar change for ucsi_acpi.c that got merged and this
> > is the ping for ucsi_ccg.c people that they probably need this, too.
> > 
> >     regards   Christian
> > 
> > 
> 
> Hi Christian,
> 
> If we don't clean the CCI cache in ucsi_ccg_async_write(), there might 
> be a potential problem when the driver is polling the results.
> 
> In ucsi_init(), we may get EPROBE_DEFER from ucsi_register_port().
> Then it does ucsi_reset_ppm() before returning the error code, and we 
> will get  UCSI_CCI_RESET_COMPLETE and store it in the CCI cache.
> If we don't clean the cache, when the UCSI driver calls ucsi_init() 
> again, then in ucsi_reset_ppm(), it will get UCSI_CCI_RESET_COMPLETE 
> from the CCI cache instantly.
> Then the driver will run the next UCSI commands when the HW is not 
> completely reset.

Thanks, I indeed did not think the reset case completely through.
However, the real bugfix is in the other hunk of the diff and this
is a genuine bugfix on its own. I found that the corresponding
diff was neccessary for ucsi_acpi.c. Should I resend without the
CCI cleaning?

Thanks   Christian