On Sun, Jun 19, 2016 at 5:12 AM, Jarkko Sakkinen <
jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

> I think inside tpm_get_timeouts() I'd rather something along the lines
> (with error handling and such details taken away):
>
> rc = tpm_getcap(...);
>
> if (rc == TPM_ERR_INVALID_POSTINIT) {
>         tpm_startup(...);
>         tpm_getca(...);
> }
>

Agreed.

> +     if (rc) {
> > +             dev_err(chip->pdev,
> > +                     "Error %zd reading %s\n", rc, desc);
> > +             return rc;
> > +     }
> > +
> > +     if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
> > +         be32_to_cpu(tpm_cmd.header.out.length)
> > +         != sizeof(tpm_cmd.header.out) + sizeof(u32) + size *
> sizeof(u32)) {
> > +             dev_err(chip->pdev,
> > +                     "Bad return code or length reading %s\n", desc);
> > +             return -EINVAL;
> > +     }
>
> This is bogus code. All this kind of checks should be contained in
> tpm_transmit_cmd(). This is easily "fixed" by moving tpm_getcap() :)
>

tpm_transmit_cmd() already checks the return_code, so that's easy. It
doesn't know the expected length, but tpm_getcap() does (or can, based on
the subcap_id).

--Ed