Messages in this thread | | | Date | Thu, 25 Sep 2014 16:56:06 +0300 | From | Jarkko Sakkinen <> | Subject | Re: [PATCH v1 11/12] tpm: Driver for TPM 2.0 CRB Interface |
| |
On Wed, Sep 24, 2014 at 11:05:51AM -0600, Jason Gunthorpe wrote: > On Wed, Sep 24, 2014 at 12:06:01PM +0300, Jarkko Sakkinen wrote: > > > + offset = cca->rsp_pa - priv->cca_pa; > > + resp = (u8 *) ((unsigned long) cca + offset); > > + memcpy(buf, resp, 6); > > + expected = be32_to_cpu(*(__be32 *) (buf + 2)); > > be32_to_cpup? > > > +static void crb_release(void *data) > > +{ > > + struct tpm_chip *chip = (struct tpm_chip *) data; > > + tpm_remove_hardware(chip->dev); > > +} > > Please use a proper remove function on the device driver, not a devm > function like this. 'tpm_remove_hardware' is the wrong name for a new > API, it must be 'tpm_chip_unregister' (ie the undo of 'tpm_chip_register') > > > +static int crb_acpi_add(struct acpi_device *device) > > +{ > > + struct tpm_chip *chip; > > + struct acpi_tpm2 *buf; > > + struct crb_priv *priv; > > + struct device *dev = &device->dev; > > + acpi_status status; > > + u32 sm; > > + int rc; > > + > > + chip = tpm_chip_alloc(dev, &tpm_crb); > > + if (!chip) > > + return -ENODEV; > > Lets use ERRPTR here > > > + chip->tpm2 = true; > > + > > + rc = tpm_chip_register(chip); > > This is in the wrong place, it needs to be the last call in the probe > function - the driver must be fully operational when register is > called, that is one of the bugs the new interface must be fixing. > > > + rc = tpm_do_selftest(chip); > > + if (rc) { > > + rc = -ENODEV; > > + goto out_err; > > + } > > The common TPM command startup sequence should be in > tpm_chip_register(), so move this into there. > > > + rc = devm_add_action(dev, crb_release, chip); > > + if (rc) > > + goto out_err; > > + > > + return 0; > > +out_err: > > + tpm_remove_hardware(chip->dev); > > + return rc; > > +} > > + > > +static struct acpi_device_id crb_device_ids[] = { > > const? Not sure
Cannot be because acpi_bus_register_driver() does not take a const pointer.
> Jason
/Jarkko
| |