>-----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. 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) >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
