Hi, On 17.02.21 at 23:18, Jarkko Sakkinen wrote: >> + > > /* > * Please describe what the heck the function does. No need for full on > * kdoc. > */ Ok. >> +int tpm2_add_device(struct tpm_chip *chip) > > Please, rename as tpm_devs_add for coherency sake. > Sorry I confused this and renamed it wrongly. I will fix it in the next patch version. >> +{ >> + int rc; >> + >> + device_initialize(&chip->devs); >> + chip->devs.parent = chip->dev.parent; >> + chip->devs.class = tpmrm_class; > > Empty line. Cannot recall, if I stated before. >> + /* + * 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); > > NAK, same comment as before. > Thx for the review, I will fix these issues. Regards, Lino