Hi,

On 16.02.21 at 13:53, Jason Gunthorpe wrote:

On Tue, Feb 16, 2021 at 01:31:00AM +0100, Lino Sanfilippo wrote:

quoted

+static int tpm_add_tpm2_char_device(struct tpm_chip *chip)
+{
+	int rc;
+
+	device_initialize(&chip->devs);
+	chip->devs.parent = chip->dev.parent;
+	chip->devs.class = tpmrm_class;
+
+	rc = dev_set_name(&chip->devs, "tpmrm%d", chip->dev_num);
+	if (rc)
+		goto out_put_devs;
+	/*
+	 * get extra reference on main device to hold on behalf of devs.
+	 * This holds the chip structure while cdevs is in use. The
+	 * corresponding put is in the tpm_devs_release.
+	 */
+	get_device(&chip->dev);
+	chip->devs.release = tpm_devs_release;
+	chip->devs.devt =
+		MKDEV(MAJOR(tpm_devt), chip->dev_num + TPM_NUM_DEVICES);
+	cdev_init(&chip->cdevs, &tpmrm_fops);
+	chip->cdevs.owner = THIS_MODULE;
+
+	rc = cdev_device_add(&chip->cdevs, &chip->devs);
+	if (rc) {
+		dev_err(&chip->devs,
+			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devs), MAJOR(chip->devs.devt),
+			MINOR(chip->devs.devt), rc);
+		goto out_put_devs;
+	}
+
+	return 0;
+
+out_put_devs:
+	put_device(&chip->devs);

I'd rather you organize this so chip->devs.release and the get_device
is always sent instead of having the possiblity for a put_device that
doesn't call release

Agreed, I will change it. It should not make a difference in terms of correctness
but I see that it is less confusing if both error cases are handled similarly (plus its
only a minimal change).


Best regards,
Lino