Hi Heikki, On Mon, Sep 09, 2024 at 12:20:47PM +0300, Heikki Krogerus wrote: > Hi Saranya, Christian, > > On Fri, Sep 06, 2024 at 11:47:42AM +0000, Gopal, Saranya wrote: > > Hi Heikki, Christian, > > > > > -----Original Message----- > > > From: Christian A. Ehrhardt <lk@xxxxxxx> > > > Sent: Saturday, August 31, 2024 3:40 AM > > > To: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > > Cc: Gopal, Saranya <saranya.gopal@xxxxxxxxx>; linux- > > > usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Regupathy, > > > Rajaram <rajaram.regupathy@xxxxxxxxx> > > > Subject: Re: [PATCH] usb: typec: ucsi: Do not call ACPI _DSM method > > > for UCSI read operations > > > > > > > > > Hi Heikki, Hi Saranya, > > > > > > On Fri, Aug 30, 2024 at 11:44:33AM +0300, Heikki Krogerus wrote: > > > > On Fri, Aug 30, 2024 at 02:13:42PM +0530, Saranya Gopal wrote: > > > > > ACPI _DSM methods are needed only for UCSI write operations > > > and for reading > > > > > CCI during RESET_PPM operation. So, remove _DSM calls from > > > other places. > > > > > While there, remove the Zenbook quirk also since the default > > > behavior > > > > > now aligns with the Zenbook quirk. With this change, > > > GET_CONNECTOR_STATUS > > > > > returns at least 6 seconds faster than before in Arrowlake-S > > > platforms. > > > > > > > > > > Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > > > > Signed-off-by: Saranya Gopal <saranya.gopal@xxxxxxxxx> > > > > > > > > Maybe this should be marked as a fix. I think this covers: > > > > https://lore.kernel.org/linux-usb/20240829100109.562429-2- > > > lk@xxxxxxx/ > > > > > > > > Heikki, > > I see that Christian's other patch is marked as a fix already (https://lore.kernel.org/linux-usb/20240906065853.637205-1-lk@xxxxxxx/T/#u). > > The other part still needs a fix. Well technically not. I've established with the reporter of https://bugzilla.kernel.org/show_bug.cgi?id=219108 that the immediate regression (keyboard on ASUS laptop not working) is fixed with the ucsi.c patch (that got your Reviewed-By today) alone. UCSI on the ASUS laptop is still broken but it always was, AFAICT. Thus I'd like to push the above mentioned patch as the fix for the regression. The reporter was very helpful and responsive in testing and I intend to look into the reason why UCSI does not work after that with the reporter's help. > On Thu, 5 Sept 2024 at 20:00, Christian A. Ehrhardt <lk@xxxxxxx> wrote: > > > > > Hi again, > > > > attached is version 4 of the patch. This will not fix the error > > messages we talked about (I have to think about this some more). > > > > It should fix your keyboard issues, though. > > > > Heikki had another request to change the patch and it would be > > cool if you could test this version to make sure that it really > > fixes your immediate problem. > > > > Best regards, > > Christian > > > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 1 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 2 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 3 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 4 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 5 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU8: Topology domain 6 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 1 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 2 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 3 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 4 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 5 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU9: Topology domain 6 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 1 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 2 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 3 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 4 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 5 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU10: Topology domain 6 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 1 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 2 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 3 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 4 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 5 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU11: Topology domain 6 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU12: Topology domain 1 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU12: Topology domain 2 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU12: Topology domain 3 shift 7 != 6 > [ 0.019168] [Firmware Bug]: CPU12: Topology domain 4 shift 7 != 6 > > > So, can this patch go in as it is? > > Please let me know if I need to resubmit with any changes. > > If you prefer that we go with Christian's patch to fix the issue > - which is fine by me - you need to rebase this on top of his patch in > any case. So you will need to resend this either way. > > Christian would you mind resending that second patch after all where > you take the Zenbook quirk into use on that ASUS system? See above. The immediate regression is fixed with the already reviewed patch alone. The remaining issue with UCSI on the ASUS laptop not working can be fixed separately. I'd rather base a fix for UCSI on the ASUS laptop onto Saranya's patch because I think that patch is the correct thing to do. Unfortunately, testing by the original reporter was inconclusive wrt. this. I have one report of a test run with the (classical) ASUS quirk (and the other patch) where UCSI on the ASUS laptop did work. Patch version v1 was the result of this. With Saranya's patch and my patch to ucsi.c the regression was gone but UCSI did _not_ work. As this does not make sense because Saranya's patch should be equivalent to the ASUS zenbook quirk. Thus this needs more investigation and dropping the zenbook quirk patch looks like the correct thing to do. > Let's make that as the actual fix for the issue. Maybe it's more clear > that way. Please let me know if you disagree and I can resend the ASUS quirk patch. Best regards, Christian
