Messages in this thread | | | Subject | Re: [BISECTED] tpm CLKRUN breaks PS/2 keyboard and touchpad on Braswell system | From | Javier Martinez Canillas <> | Date | Tue, 19 Dec 2017 14:10:25 +0100 |
| |
Hello Jarkko,
On 12/19/2017 01:59 PM, Jarkko Sakkinen wrote: > James, Javier, thank you for sorting this out. I'll just have couple of > minor comments on the patch. > > On Mon, Dec 18, 2017 at 01:22:28PM +0100, Javier Martinez Canillas wrote: >> + u32 vendor, intfcaps, intmask, clkrun_val; > > Could these split into four lines (one declaration per line)? >
Yes, I didn't like that either but did this way for consistency with the driver.
>> u8 rid; >> int rc, probe; >> struct tpm_chip *chip; >> @@ -772,6 +772,15 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, >> ILB_REMAP_SIZE); >> if (!priv->ilb_base_addr) >> return -ENOMEM; >> + >> + clkrun_val = ioread32(priv->ilb_base_addr + LPC_CNTRL_OFFSET); >> + /* Check if CLKRUN# is already not enabled in the LPC bus */ > > /* > >> + if (!(clkrun_val & LPC_CLKRUN_EN)) { >> + priv->flags |= TPM_TIS_CLK_ENABLE; > > Is the flag added just so surpress those WARN()'s? >
I believe so. I just included here again for consistency, but I think the flag and the warns can just go away.
> I've forgot why the WARN()'s even exist assuming that driver is > functioning correctly. > > /Jarkko >
If you agree with the patch, I can post it as a formal patch on a series that do these cleanups as preparatory work. I've also noticed a NULL pointer deref bug in an error path so I'll also fix that too.
Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
| |