[tpmdd-devel,1/4] tpm: add kdoc for tpm_transmit and tpm_transmit_cmd
diff mbox

Message ID 1479315821-27349-1-git-send-email-tomas.winkler@intel.com
State New
Headers show

Commit Message

Winkler, Tomas Nov. 16, 2016, 5:03 p.m. UTC
Functions tpm_transmit and transmit_cmd are referenced
from other functions kdoc hence deserve documentation.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/char/tpm/tpm-interface.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

Comments

Jarkko Sakkinen Nov. 16, 2016, 11 p.m. UTC | #1
On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> Functions tpm_transmit and transmit_cmd are referenced
> from other functions kdoc hence deserve documentation.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Do you know how to make "make htmldocs" to generate documentation for
the source tree? I do not except the ones that I imported when I created
an RST version of the Stefans documentation.

I'm mainly concerned having one line per error code. My guess it that
Sphinx will remove the newline characters.

The way I sorted that out in Stefans documentation was '|' prefix on the
beginning of line, which does newline enforcement.

So you would have something along the lines:

* | 0 when the operation is successful
* | negative number for system errors (errno)
* | A positive number for a TPM error.

It's uglier in unrendered form but still kind of sustainable and in the
rendered output things won't fall apart...

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a2688ac2b48f..56c238a6e005 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -328,8 +328,17 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  }
>  EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>  
> -/*
> - * Internal kernel interface to transmit TPM commands
> +/**
> + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + *
> + * @chip: TPM chip to use
> + * @buf: TPM command buffer
> + * @bufsiz: length of the TPM command buffer
> + * @flags: tpm transmit flags - bitmap
> + *
> + * Return:
> + *     0 when the operation is successful
> + *     A negative number for system errors (errno)
>   */
>  ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  		     unsigned int flags)
> @@ -409,9 +418,21 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  	return rc;
>  }
>  
> -#define TPM_DIGEST_SIZE 20
> -#define TPM_RET_CODE_IDX 6
> -
> +/**
> + * tmp_transmit_cmd - send a tpm command to the device
> + *    The function extracts tpm out header return code
> + *
> + * @chip: TPM chip to use
> + * @cmd: TPM command buffer
> + * @len: length of the TPM command
> + * @flags: tpm transmit flags - bitmap
> + * @desc: command description used in the error message
> + *
> + * Return:
> + *     0 when the operation is successful
> + *     A negative number for system errors (errno)
> + *     A positive number for a TPM error.
> + */
>  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>  			 int len, unsigned int flags, const char *desc)
>  {
> @@ -434,6 +455,8 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>  	return err;
>  }
>  
> +#define TPM_DIGEST_SIZE 20
> +#define TPM_RET_CODE_IDX 6
>  #define TPM_INTERNAL_RESULT_SIZE 200
>  #define TPM_ORD_GET_CAP cpu_to_be32(101)
>  #define TPM_ORD_GET_RANDOM cpu_to_be32(70)
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------
Winkler, Tomas Nov. 22, 2016, 9:44 a.m. UTC | #2
> 
> On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> > Functions tpm_transmit and transmit_cmd are referenced from other
> > functions kdoc hence deserve documentation.
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> 
> Do you know how to make "make htmldocs" to generate documentation for the
> source tree? I do not except the ones that I imported when I created an RST
> version of the Stefans documentation.

I'm using a quick script like that
RES=tpm-kdoc.html
rm -r $RES kdoc.err
files=$(git ls-files drivers/char/tpm/*.[ch] includ/linux/tpm.h includ/linux/tpm_command.h)

echo '<html>' > $RES
for f in ${files}; do
        ./scripts/kernel-doc -html $f >> $RES 2>> kdoc.err
done
echo '</html>' >> $RES
cat kdoc.err | grep -v 'warning: no structured comments found'

> 
> I'm mainly concerned having one line per error code. My guess it that Sphinx
> will remove the newline characters.

Why do you find it tragic that the new line is removed, we just need to make sure it's readable in both cases.

> The way I sorted that out in Stefans documentation was '|' prefix on the
> beginning of line, which does newline enforcement.
> 
> So you would have something along the lines:
> 
> * | 0 when the operation is successful
> * | negative number for system errors (errno)
> * | A positive number for a TPM error.
> 
> It's uglier in unrendered form but still kind of sustainable and in the rendered
> output things won't fall apart...

I'm not sure it matters much regarding the readability of the document but the old html format will have then those strange 'I' rendered. 

Let me know which why you won't to go frankly I don't have strong opinion either way,  I will respin the patch anyhow as I've missed some '.' 

Thanks
Tomas


 
> /Jarkko
> 
> > ---
> >  drivers/char/tpm/tpm-interface.c | 33
> > ++++++++++++++++++++++++++++-----
> >  1 file changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index a2688ac2b48f..56c238a6e005 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -328,8 +328,17 @@ unsigned long tpm_calc_ordinal_duration(struct
> > tpm_chip *chip,  }  EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
> >
> > -/*
> > - * Internal kernel interface to transmit TPM commands
> > +/**
> > + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> > + *
> > + * @chip: TPM chip to use
> > + * @buf: TPM command buffer
> > + * @bufsiz: length of the TPM command buffer
> > + * @flags: tpm transmit flags - bitmap
> > + *
> > + * Return:
> > + *     0 when the operation is successful
> > + *     A negative number for system errors (errno)
> >   */
> >  ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
> >  		     unsigned int flags)
> > @@ -409,9 +418,21 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const
> u8 *buf, size_t bufsiz,
> >  	return rc;
> >  }
> >
> > -#define TPM_DIGEST_SIZE 20
> > -#define TPM_RET_CODE_IDX 6
> > -
> > +/**
> > + * tmp_transmit_cmd - send a tpm command to the device
> > + *    The function extracts tpm out header return code
> > + *
> > + * @chip: TPM chip to use
> > + * @cmd: TPM command buffer
> > + * @len: length of the TPM command
> > + * @flags: tpm transmit flags - bitmap
> > + * @desc: command description used in the error message
> > + *
> > + * Return:
> > + *     0 when the operation is successful
> > + *     A negative number for system errors (errno)
> > + *     A positive number for a TPM error.
> > + */
> >  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
> >  			 int len, unsigned int flags, const char *desc)  { @@ -
> 434,6
> > +455,8 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void
> *cmd,
> >  	return err;
> >  }
> >
> > +#define TPM_DIGEST_SIZE 20
> > +#define TPM_RET_CODE_IDX 6
> >  #define TPM_INTERNAL_RESULT_SIZE 200
> >  #define TPM_ORD_GET_CAP cpu_to_be32(101)  #define
> TPM_ORD_GET_RANDOM
> > cpu_to_be32(70)
> > --
> > 2.7.4
> >

------------------------------------------------------------------------------
Jarkko Sakkinen Nov. 22, 2016, 10:19 a.m. UTC | #3
On Tue, Nov 22, 2016 at 09:44:20AM +0000, Winkler, Tomas wrote:
> > 
> > On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> > > Functions tpm_transmit and transmit_cmd are referenced from other
> > > functions kdoc hence deserve documentation.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > 
> > Do you know how to make "make htmldocs" to generate documentation for the
> > source tree? I do not except the ones that I imported when I created an RST
> > version of the Stefans documentation.
> 
> I'm using a quick script like that
> RES=tpm-kdoc.html
> rm -r $RES kdoc.err
> files=$(git ls-files drivers/char/tpm/*.[ch] includ/linux/tpm.h includ/linux/tpm_command.h)
> 
> echo '<html>' > $RES
> for f in ${files}; do
>         ./scripts/kernel-doc -html $f >> $RES 2>> kdoc.err
> done
> echo '</html>' >> $RES
> cat kdoc.err | grep -v 'warning: no structured comments found'

Thanks. I'll try this as soon as the release chaos is over.

/Jarkko

------------------------------------------------------------------------------
Jarkko Sakkinen Nov. 24, 2016, 1:11 p.m. UTC | #4
On Wed, Nov 16, 2016 at 07:03:38PM +0200, Tomas Winkler wrote:
> Functions tpm_transmit and transmit_cmd are referenced
> from other functions kdoc hence deserve documentation.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/char/tpm/tpm-interface.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a2688ac2b48f..56c238a6e005 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -328,8 +328,17 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  }
>  EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>  
> -/*
> - * Internal kernel interface to transmit TPM commands
> +/**
> + * tmp_transmit - Internal kernel interface to transmit TPM commands.
> + *
> + * @chip: TPM chip to use
> + * @buf: TPM command buffer
> + * @bufsiz: length of the TPM command buffer
> + * @flags: tpm transmit flags - bitmap
> + *
> + * Return:
> + *     0 when the operation is successful
> + *     A negative number for system errors (errno)
>   */
>  ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  		     unsigned int flags)
> @@ -409,9 +418,21 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  	return rc;
>  }
>  
> -#define TPM_DIGEST_SIZE 20
> -#define TPM_RET_CODE_IDX 6
> -
> +/**
> + * tmp_transmit_cmd - send a tpm command to the device
> + *    The function extracts tpm out header return code
> + *
> + * @chip: TPM chip to use
> + * @cmd: TPM command buffer
> + * @len: length of the TPM command
> + * @flags: tpm transmit flags - bitmap
> + * @desc: command description used in the error message
> + *
> + * Return:
> + *     0 when the operation is successful
> + *     A negative number for system errors (errno)
> + *     A positive number for a TPM error.
> + */
>  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>  			 int len, unsigned int flags, const char *desc)
>  {
> @@ -434,6 +455,8 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>  	return err;
>  }
>  
> +#define TPM_DIGEST_SIZE 20
> +#define TPM_RET_CODE_IDX 6

I think we should scrap this stupid constant away but its not this
patches fault :-) Would be better to just use SHA1_DIGEST_SIZE.

>  #define TPM_INTERNAL_RESULT_SIZE 200
>  #define TPM_ORD_GET_CAP cpu_to_be32(101)
>  #define TPM_ORD_GET_RANDOM cpu_to_be32(70)
> -- 
> 2.7.4

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

I'm applying this and other patches in this series post pull requeest as
it is not release critical.

/Jarkko

------------------------------------------------------------------------------

Patch
diff mbox

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a2688ac2b48f..56c238a6e005 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,8 +328,17 @@  unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
-/*
- * Internal kernel interface to transmit TPM commands
+/**
+ * tmp_transmit - Internal kernel interface to transmit TPM commands.
+ *
+ * @chip: TPM chip to use
+ * @buf: TPM command buffer
+ * @bufsiz: length of the TPM command buffer
+ * @flags: tpm transmit flags - bitmap
+ *
+ * Return:
+ *     0 when the operation is successful
+ *     A negative number for system errors (errno)
  */
 ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 		     unsigned int flags)
@@ -409,9 +418,21 @@  ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 	return rc;
 }
 
-#define TPM_DIGEST_SIZE 20
-#define TPM_RET_CODE_IDX 6
-
+/**
+ * tmp_transmit_cmd - send a tpm command to the device
+ *    The function extracts tpm out header return code
+ *
+ * @chip: TPM chip to use
+ * @cmd: TPM command buffer
+ * @len: length of the TPM command
+ * @flags: tpm transmit flags - bitmap
+ * @desc: command description used in the error message
+ *
+ * Return:
+ *     0 when the operation is successful
+ *     A negative number for system errors (errno)
+ *     A positive number for a TPM error.
+ */
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
 			 int len, unsigned int flags, const char *desc)
 {
@@ -434,6 +455,8 @@  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
 	return err;
 }
 
+#define TPM_DIGEST_SIZE 20
+#define TPM_RET_CODE_IDX 6
 #define TPM_INTERNAL_RESULT_SIZE 200
 #define TPM_ORD_GET_CAP cpu_to_be32(101)
 #define TPM_ORD_GET_RANDOM cpu_to_be32(70)