| Message ID | 1510090328-153106-3-git-send-email-azhar.shaikh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: > +#ifdef CONFIG_X86 > + if (is_bsw()) > + iounmap(phy->ilb_base_addr); > +#endif This whole thing would be much better if is_bsw was just bool is_bsw(void) { if (!IS_ENABLED(CONFIG_X86)) return false; [..] } Then drop every single one of these #ifdef CONFIG_X86 Jason
On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: > On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: > > > +#ifdef CONFIG_X86 > > + if (is_bsw()) > > + iounmap(phy->ilb_base_addr); > > +#endif > > This whole thing would be much better if is_bsw was just > > bool is_bsw(void) > { > if (!IS_ENABLED(CONFIG_X86)) > return false; > [..] > } > > Then drop every single one of these #ifdef CONFIG_X86 > > Jason +1 /Jarkko
>-----Original Message----- >From: Sakkinen, Jarkko >Sent: Wednesday, November 15, 2017 12:14 AM >To: Jason Gunthorpe <jgg@ziepe.ca> >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; linux-integrity@vger.kernel.org >Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > >On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: >> On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: >> >> > +#ifdef CONFIG_X86 >> > + if (is_bsw()) >> > + iounmap(phy->ilb_base_addr); >> > +#endif >> >> This whole thing would be much better if is_bsw was just >> >> bool is_bsw(void) >> { >> if (!IS_ENABLED(CONFIG_X86)) >> return false; >> [..] >> } >> >> Then drop every single one of these #ifdef CONFIG_X86 >> >> Jason > >+1 > Thank you for the suggestion. Will incorporate this in next patch series. >/Jarkko
>-----Original Message----- >From: Shaikh, Azhar >Sent: Wednesday, November 15, 2017 9:59 AM >To: Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; Jason Gunthorpe ><jgg@ziepe.ca> >Cc: linux-integrity@vger.kernel.org >Subject: RE: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > > > >>-----Original Message----- >>From: Sakkinen, Jarkko >>Sent: Wednesday, November 15, 2017 12:14 AM >>To: Jason Gunthorpe <jgg@ziepe.ca> >>Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; >>linux-integrity@vger.kernel.org >>Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >>tpm_tis_tcg_phy >> >>On Tue, Nov 14, 2017 at 04:28:12PM -0700, Jason Gunthorpe wrote: >>> On Tue, Nov 07, 2017 at 01:32:08PM -0800, Azhar Shaikh wrote: >>> >>> > +#ifdef CONFIG_X86 >>> > + if (is_bsw()) >>> > + iounmap(phy->ilb_base_addr); >>> > +#endif >>> >>> This whole thing would be much better if is_bsw was just >>> >>> bool is_bsw(void) >>> { >>> if (!IS_ENABLED(CONFIG_X86)) >>> return false; >>> [..] >>> } >>> >>> Then drop every single one of these #ifdef CONFIG_X86 >>> >>> Jason >> >>+1 >> > >Thank you for the suggestion. >Will incorporate this in next patch series. > If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef CONFIG_X86, I will still get compilation errors for non-x86 platforms, since INTEL_FAM6_ATOM_AIRMONT will be undefined. bool is_bsw(void) { if (!IS_ENABLED(CONFIG_X86)) return false; return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); } I think I will have to keep is_bsw() implementation unchanged? >>/Jarkko
On Wed, Nov 15, 2017 at 07:36:47PM +0000, Shaikh, Azhar wrote: > If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef CONFIG_X86, I will still get compilation errors for non-x86 platforms, since INTEL_FAM6_ATOM_AIRMONT will be undefined. > > bool is_bsw(void) > { > if (!IS_ENABLED(CONFIG_X86)) > return false; > return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); > } > > I think I will have to keep is_bsw() implementation unchanged? No, still move the #ifdef to is_bsw, just like this instead: bool is_bsw(void) { #ifdef CONFIG_X86 return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); #else return false; #endif } Jason
>-----Original Message----- >From: Jason Gunthorpe [mailto:jgg@ziepe.ca] >Sent: Wednesday, November 15, 2017 11:40 AM >To: Shaikh, Azhar <azhar.shaikh@intel.com> >Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>; linux- >integrity@vger.kernel.org >Subject: Re: [PATCH RFC v2 2/2] tpm_tis: Move ilb_base_addr to >tpm_tis_tcg_phy > >On Wed, Nov 15, 2017 at 07:36:47PM +0000, Shaikh, Azhar wrote: > >> If I implement is_bsw() as below and move it(is_bsw()) outside the #ifdef >CONFIG_X86, I will still get compilation errors for non-x86 platforms, since >INTEL_FAM6_ATOM_AIRMONT will be undefined. >> >> bool is_bsw(void) >> { >> if (!IS_ENABLED(CONFIG_X86)) >> return false; >> return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 >: 0); >> } >> >> I think I will have to keep is_bsw() implementation unchanged? > >No, still move the #ifdef to is_bsw, just like this instead: > >bool is_bsw(void) >{ >#ifdef CONFIG_X86 > return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 > : 0); >#else > return false; >#endif >} > Ok, sure. Should have thought myself about this... :( >Jason
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 76a7b64195c8..5f49e9051515 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -46,6 +46,7 @@ struct tpm_info { struct tpm_tis_tcg_phy { struct tpm_tis_data priv; void __iomem *iobase; + void __iomem *ilb_base_addr; bool begin_xfer_done; }; @@ -140,8 +141,6 @@ static int check_acpi_tpm2(struct device *dev) #define LPC_CNTRL_REG_OFFSET 0x84 #define LPC_CLKRUN_EN (1 << 2) -static void __iomem *ilb_base_addr; - static inline bool is_bsw(void) { return ((boot_cpu_data.x86_model == INTEL_FAM6_ATOM_AIRMONT) ? 1 : 0); @@ -160,11 +159,11 @@ static void tpm_platform_begin_xfer(struct tpm_tis_data *data) phy->begin_xfer_done)) return; - clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET); + clkrun_val = ioread32(phy->ilb_base_addr + LPC_CNTRL_REG_OFFSET); /* Disable LPC CLKRUN# */ clkrun_val &= ~LPC_CLKRUN_EN; - iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET); + iowrite32(clkrun_val, phy->ilb_base_addr + LPC_CNTRL_REG_OFFSET); /* * Write any random value on port 0x80 which is on LPC, to make @@ -185,15 +184,16 @@ static void tpm_platform_begin_xfer(struct tpm_tis_data *data) static void tpm_platform_end_xfer(struct tpm_tis_data *data) { u32 clkrun_val; + struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); if (!is_bsw() || (data->flags & TPM_TIS_CLK_ENABLE)) return; - clkrun_val = ioread32(ilb_base_addr + LPC_CNTRL_REG_OFFSET); + clkrun_val = ioread32(phy->ilb_base_addr + LPC_CNTRL_REG_OFFSET); /* Enable LPC CLKRUN# */ clkrun_val |= LPC_CLKRUN_EN; - iowrite32(clkrun_val, ilb_base_addr + LPC_CNTRL_REG_OFFSET); + iowrite32(clkrun_val, phy->ilb_base_addr + LPC_CNTRL_REG_OFFSET); /* * Write any random value on port 0x80 which is on LPC, to make @@ -311,14 +311,31 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) if (IS_ERR(phy->iobase)) return PTR_ERR(phy->iobase); +#ifdef CONFIG_X86 + if (is_bsw()) { + phy->ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR, + ILB_REMAP_SIZE); + if (!phy->ilb_base_addr) + return -ENOMEM; + } +#endif + if (interrupts) irq = tpm_info->irq; if (itpm || is_itpm(ACPI_COMPANION(dev))) phy->priv.flags |= TPM_TIS_ITPM_WORKAROUND; - return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg, + rc = tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg, ACPI_HANDLE(dev)); + if (rc) { +#ifdef CONFIG_X86 + if (is_bsw()) + iounmap(phy->ilb_base_addr); +#endif + } + + return rc; } static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume); @@ -359,9 +376,16 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, static void tpm_tis_pnp_remove(struct pnp_dev *dev) { struct tpm_chip *chip = pnp_get_drvdata(dev); + struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); + struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(priv); tpm_chip_unregister(chip); tpm_tis_remove(chip); + +#ifdef CONFIG_X86 + if (is_bsw()) + iounmap(phy->ilb_base_addr); +#endif } static struct pnp_driver tis_pnp_driver = { @@ -408,10 +432,17 @@ static int tpm_tis_plat_probe(struct platform_device *pdev) static int tpm_tis_plat_remove(struct platform_device *pdev) { struct tpm_chip *chip = dev_get_drvdata(&pdev->dev); + struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); + struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(priv); tpm_chip_unregister(chip); tpm_tis_remove(chip); +#ifdef CONFIG_X86 + if (is_bsw()) + iounmap(phy->ilb_base_addr); +#endif + return 0; } @@ -469,11 +500,6 @@ static int __init init_tis(void) if (rc) goto err_force; -#ifdef CONFIG_X86 - if (is_bsw()) - ilb_base_addr = ioremap(INTEL_LEGACY_BLK_BASE_ADDR, - ILB_REMAP_SIZE); -#endif rc = platform_driver_register(&tis_drv); if (rc) goto err_platform; @@ -492,10 +518,6 @@ static int __init init_tis(void) err_platform: if (force_pdev) platform_device_unregister(force_pdev); -#ifdef CONFIG_X86 - if (is_bsw()) - iounmap(ilb_base_addr); -#endif err_force: return rc; } @@ -505,10 +527,6 @@ static void __exit cleanup_tis(void) pnp_unregister_driver(&tis_pnp_driver); platform_driver_unregister(&tis_drv); -#ifdef CONFIG_X86 - if (is_bsw()) - iounmap(ilb_base_addr); -#endif if (force_pdev) platform_device_unregister(force_pdev); }
Move the static variable ilb_base_addr to tpm_tis_tcg_phy. Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> --- drivers/char/tpm/tpm_tis.c | 58 ++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 20 deletions(-)