Re: [PATCH 0/1] tpm_tis_i2c: Fix -Wdeclaration-after-statement

[Peter Delevoryas <peter@pjd.dev>]

On Fri, Jul 29, 2022 at 12:21:31AM +0000, Joel Stanley wrote:
> Hi Peter,
> 
> On Mon, 25 Jul 2022 at 22:51, Peter Delevoryas <peter@pjd.dev> wrote:
> >
> > Hey Joel,
> >
> > I've been trying to build fby35 from https://github.com/openbmc/openbmc, and I
> > noticed that the TPM TIS I2C file seems to emit a warning that causes a
> > compilation error:
> >
> > ../drivers/char/tpm/tpm_tis_i2c.c: In function ‘tpm_tis_i2c_write_bytes’:
> > ../drivers/char/tpm/tpm_tis_i2c.c:114:17: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
> >   114 |                 struct i2c_msg msgs[] = {
> >       |                 ^~~~~~
> >
> > I'm sending this patch as a fix for the warning, to avoid the compilation error.
> 
> Thanks for trying to fix this.
> 
> >
> > Normally I think I would send this to the upstream kernel mailing list, but
> > this file doesn't actually seem to be in the upstream at any point:
> >
> >     drivers/char/tpm/tpm_tis_i2c.c
> >
> > It looks like it was added by Nuvoton and never upstreamed successfully? Perhaps
> > we should get rid of it at this point? fby35 doesn't actually use the TPM TIS
> > I2C interface, it uses the SPI one. Bletchley doesn't use the I2C one either.
> > Only older FB platforms did.
> 
> The i2c driver was added for the IBM P10 platforms.

I see, thanks for pointing that out, I didn't do a proper survey.

> 
> > Actually, doing a quick search on lore.kernel.org: It looks like maybe there's
> > a new version submitted by Infineon in June:
> >
> >     https://lore.kernel.org/all/20220608173113.9232-1-Alexander.Steffen@infineon.com/
> >
> > If fby35 wants to avoid this error, should we fix the driver, remove it from the
> > kernel, pull in the new driver from upstream, or maybe just disable it in the
> > linux-aspeed tpm2 distro feature Kconfig?
> 
> The upstream maintainer has been reluctant to merge this code. Nuvoton
> told me off-list they had abandoned plans to upstream it. Since then
> Infineon have picked it up and made some submissions. It looks like
> it's been queued for merging in v5.20.

Oh great, nice that Infineon did that.

> 
> I'd be happy moving to the latest version of the patch in the openbmc
> tree. Either as a revert+new driver, or as a diff, whichever you think
> makes sense. They inexplicably removed the compatible strings in the
> latest submission, so we would need to add them back:

That's great! I think revert + new driver would make most sense to me,
are you planning on just pulling the diffs from that mailing list thread
and applying the compatible string change on top of that? Let me know if
I can help somehow.

Thanks,
Peter

> 
>  static const struct of_device_id of_tis_i2c_match[] = {
> -       { .compatible = "nuvoton,npct75x", },
> -       { .compatible = "tcg,tpm-tis-i2c", },
> +       { .compatible = "infineon,slb9673", },
> 
> 
> Cheers,
> 
> Joel