>-----Original Message----- >From: linux-integrity-owner@xxxxxxxxxxxxxxx [mailto:linux-integrity- >owner@xxxxxxxxxxxxxxx] On Behalf Of Shaikh, Azhar >Sent: Monday, December 18, 2017 11:34 AM >To: Javier Martinez Canillas <javierm@xxxxxxxxxx>; Jason Gunthorpe ><jgg@xxxxxxxx> >Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>; James Ettle ><james@xxxxxxxxxxxx>; linux-integrity@xxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx; james.l.morris@xxxxxxxxxx >Subject: RE: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on >Braswell system > > > >>-----Original Message----- >>From: Javier Martinez Canillas [mailto:javierm@xxxxxxxxxx] >>Sent: Monday, December 18, 2017 11:30 AM >>To: Jason Gunthorpe <jgg@xxxxxxxx> >>Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>; James Ettle >><james@xxxxxxxxxxxx>; linux-integrity@xxxxxxxxxxxxxxx; Shaikh, Azhar >><azhar.shaikh@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; >>james.l.morris@xxxxxxxxxx >>Subject: Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on >>Braswell system >> >>Hello Jason, >> >>Thanks a lot for your feedback. >> >>On 12/18/2017 06:55 PM, Jason Gunthorpe wrote: >>> On Mon, Dec 18, 2017 at 01:29:01PM +0100, Javier Martinez Canillas wrote: >>>> On 12/18/2017 01:22 PM, Javier Martinez Canillas wrote: >>>> >>>> [snip] >>>> >>>>> >>>>> James, >>>>> >>>>> Can you please test the following (untested) patch on top of the >>>>> other two mentioned patches to see if it makes a difference for you? >>>>> >>>> >>>> I should had tried to at least compile the patch :) >>> >>> I think this is backwards.. >>> >>> If CLKRUN_EN is on (eg power management is NOT enabled on LPC) then >>> TPM shouldn't do anything at all. >>> >>> If CLKRUN_EN is off, then it should try to turn it on/off to save >>> power. >>> >> >>My knowledge of LPC is near to non-existent so I please forgive me if >>I'm wrong, but my understanding is that it's the opposite of what you said. >> >>When CLKRUN_EN is SET, power management is ENABLED on the LPC bus >and >>the host LCLK clock may be stopped when entering in some low-power >states. >> >>The CLKRUN# signal is then used by peripherals to restart the LCLK >>clock after resuming from low-power states to be able to transmit again. >> >>When CLKRUN_EN is CLEAR, power management is DISABLED on the LPC bus >>and the host LCLK clock is never stopped even when entering in some >>low- power states. >> > >Hi Javier, > >Yes you are correct with the above understanding of the CLKRUN. > >>IIUC, if CLKRUN_EN is enabled, then all the devices attached to the LPC >>bus have to support the CLKRUN protocol. My guess is that on some >>Braswell systems LPC power management is enabled but the TPM device >>doesn't have CLKRUN support. >> > >I think this is what might be happening here. > >Since I do not have an end product to test this on, I managed to get one BSW >Reference Verification Platform(RVP). I flashed the latest Ubuntu (17.10) >which is on 4.13.13 kernel version and does have the patch. > The correct kernel version is 4.13.0-19 >I was able to use the PS/2 mouse and keyboard immediately after bootup and >also after suspend/resume cycle. The system does have a TPM.(/dev/tpm0 >and /dev/tpmrm0 exist) > > The RVP has a Nuvoton TPM >>And that was the motivation of commit 5e572cab92f0 ("tpm: Enable CLKRUN >>protocol for Braswell systems") that introduced the regression. >> >>> Perhaps the best work around is to just delete the turning off of >>> CLKRUN_EN ? Uses more power but keeps the clock running which should >>> keep both TPM and superio happy. >>> >> >>But that's exactly the goal of the commit 5e572cab92f0, to disable the >>CLKRUN protocol (probably for what I mentioned before). So doing so >>will cause issues for those systems again. >> >>What the patch I shared with James does is to avoid disabling the >>CLKRUN_EN if this is already disabled. Which should be a no-op anyways >>but the problem is that latter it's enabled even when the driver found >disabled to start with. >> >>I still don't like the overall solution since it's too error prone. >>Touching a global bus configuration in a driver for a single peripheral isn't >safe to say the least. >> >>But regardless of that, the patch I shared has merits on its own since >>is wrong to keep the bus configuration in a different state that was >>before the driver was probed. And in fact, James reported that it makes >>his system to work again. >> >>> Jason >>> >> >>Best regards, >>-- >>Javier Martinez Canillas >>Software Engineer - Desktop Hardware Enablement Red Hat
