| Message ID | 153367366462.18015.12915462188877575543.stgit@tstruk-mobl1.jf.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm: add support for nonblocking operation | expand |
On Tue, Aug 07, 2018 at 01:27:44PM -0700, Tadeusz Struk wrote: > Add a ptr to struct tpm_space to the file_priv to have an easy > access to it in the async job without the need to allocate memory. > This also allows to consolidate of the write operations for > the two interfaces. I think the 2nd premise should be the priority and the first premise should be removed as it is not needed in any possible way to justify the change. /Jarkko
On 08/10/2018 10:27 AM, Jarkko Sakkinen wrote: > On Tue, Aug 07, 2018 at 01:27:44PM -0700, Tadeusz Struk wrote: >> Add a ptr to struct tpm_space to the file_priv to have an easy >> access to it in the async job without the need to allocate memory. >> This also allows to consolidate of the write operations for >> the two interfaces. > > I think the 2nd premise should be the priority and the first premise should > be removed as it is not needed in any possible way to justify the > change. Jarkko, The main reason why the pointer to tpm_space struct was added to the file_priv was to have access to space in the async job when it is enqueued via the /dev/tpm<N> interface. Currently it is only available in tpmrm_priv. Otherwise I would need to introduce yet another struct what would consist of a ptr file_priv and a ptr to tpm_space and allocate it on every enqueue. Much easier was to to it this way. The consolidation was only a side effect of this so I think the description is correct. Thanks,
On Fri, Aug 10, 2018 at 11:05:17AM -0700, Tadeusz Struk wrote: > On 08/10/2018 10:27 AM, Jarkko Sakkinen wrote: > > On Tue, Aug 07, 2018 at 01:27:44PM -0700, Tadeusz Struk wrote: > >> Add a ptr to struct tpm_space to the file_priv to have an easy > >> access to it in the async job without the need to allocate memory. > >> This also allows to consolidate of the write operations for > >> the two interfaces. > > > > I think the 2nd premise should be the priority and the first premise should > > be removed as it is not needed in any possible way to justify the > > change. > > Jarkko, > The main reason why the pointer to tpm_space struct was added to the > file_priv was to have access to space in the async job when it is enqueued > via the /dev/tpm<N> interface. Currently it is only available in tpmrm_priv. > Otherwise I would need to introduce yet another struct what would consist of > a ptr file_priv and a ptr to tpm_space and allocate it on every enqueue. > Much easier was to to it this way. The consolidation was only a side effect > of this so I think the description is correct. > Thanks, > -- > Tadeusz The less dependencies a commit has more easy it is for me pick up. Here the consodilation is enough to accept the commit but with the current commit message I cannot apply it without accepting the whole patch set. /Jarkko
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c index e4a04b2d3c32..f0c033b69b62 100644 --- a/drivers/char/tpm/tpm-dev-common.c +++ b/drivers/char/tpm/tpm-dev-common.c @@ -43,9 +43,11 @@ static void timeout_work(struct work_struct *work) } void tpm_common_open(struct file *file, struct tpm_chip *chip, - struct file_priv *priv) + struct file_priv *priv, struct tpm_space *space) { priv->chip = chip; + priv->space = space; + mutex_init(&priv->buffer_mutex); timer_setup(&priv->user_read_timer, user_reader_timeout, 0); INIT_WORK(&priv->work, timeout_work); @@ -79,7 +81,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf, } ssize_t tpm_common_write(struct file *file, const char __user *buf, - size_t size, loff_t *off, struct tpm_space *space) + size_t size, loff_t *off) { struct file_priv *priv = file->private_data; size_t in_size = size; @@ -119,7 +121,7 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf, mutex_unlock(&priv->buffer_mutex); return -EPIPE; } - out_size = tpm_transmit(priv->chip, space, priv->data_buffer, + out_size = tpm_transmit(priv->chip, priv->space, priv->data_buffer, sizeof(priv->data_buffer), 0); tpm_put_ops(priv->chip); diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c index ebd74ab5abef..98b9630c3a36 100644 --- a/drivers/char/tpm/tpm-dev.c +++ b/drivers/char/tpm/tpm-dev.c @@ -39,7 +39,7 @@ static int tpm_open(struct inode *inode, struct file *file) if (priv == NULL) goto out; - tpm_common_open(file, chip, priv); + tpm_common_open(file, chip, priv, NULL); return 0; @@ -48,12 +48,6 @@ static int tpm_open(struct inode *inode, struct file *file) return -ENOMEM; } -static ssize_t tpm_write(struct file *file, const char __user *buf, - size_t size, loff_t *off) -{ - return tpm_common_write(file, buf, size, off, NULL); -} - /* * Called on file close */ @@ -73,6 +67,6 @@ const struct file_operations tpm_fops = { .llseek = no_llseek, .open = tpm_open, .read = tpm_common_read, - .write = tpm_write, + .write = tpm_common_write, .release = tpm_release, }; diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h index b24cfb4d3ee1..4048677bbd78 100644 --- a/drivers/char/tpm/tpm-dev.h +++ b/drivers/char/tpm/tpm-dev.h @@ -6,6 +6,7 @@ struct file_priv { struct tpm_chip *chip; + struct tpm_space *space; /* Data passed to and from the tpm via the read/write calls */ size_t data_pending; @@ -18,11 +19,11 @@ struct file_priv { }; void tpm_common_open(struct file *file, struct tpm_chip *chip, - struct file_priv *priv); + struct file_priv *priv, struct tpm_space *space); ssize_t tpm_common_read(struct file *file, char __user *buf, size_t size, loff_t *off); ssize_t tpm_common_write(struct file *file, const char __user *buf, - size_t size, loff_t *off, struct tpm_space *space); + size_t size, loff_t *off); void tpm_common_release(struct file *file, struct file_priv *priv); #endif diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c index 1a0e97a5da5a..96006c6b9696 100644 --- a/drivers/char/tpm/tpmrm-dev.c +++ b/drivers/char/tpm/tpmrm-dev.c @@ -28,7 +28,7 @@ static int tpmrm_open(struct inode *inode, struct file *file) return -ENOMEM; } - tpm_common_open(file, chip, &priv->priv); + tpm_common_open(file, chip, &priv->priv, &priv->space); return 0; } @@ -45,21 +45,11 @@ static int tpmrm_release(struct inode *inode, struct file *file) return 0; } -static ssize_t tpmrm_write(struct file *file, const char __user *buf, - size_t size, loff_t *off) -{ - struct file_priv *fpriv = file->private_data; - struct tpmrm_priv *priv = container_of(fpriv, struct tpmrm_priv, priv); - - return tpm_common_write(file, buf, size, off, &priv->space); -} - const struct file_operations tpmrm_fops = { .owner = THIS_MODULE, .llseek = no_llseek, .open = tpmrm_open, .read = tpm_common_read, - .write = tpmrm_write, + .write = tpm_common_write, .release = tpmrm_release, }; -