Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH <[EMAIL PROTECTED]> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <[EMAIL PROTECTED]> wrote:
> > > I have tested this patch and agree that using msleep is the right.  Please
> > > apply this patch to the tpm driver.  One hunk might fail b/c the
> > > typo has been fixed already.
> >
> > Would you like me to respin the patch, Greg? Or is the failed hunk ok?
> 
> I'm sorry, but I am not in charge of accepting patches for the tpm
> driver.  Why not go through the listed maintainer for this process, they
> should know how to get it into the mainline kernel tree properly.  If
> not, why would they be listed as the maintainer?  :)

Kylene, there is no entry in MAINTAINERS as of 2.6.12-rc2 for the TPM
driver? Should there be?

I am assuming tpmdd_devel can take care of merging the patch and
pushing to mainline? Please do so.

Thanks,
Nish
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH <[EMAIL PROTECTED]> wrote:
> On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> > On 4/15/05, Kylene Hall <[EMAIL PROTECTED]> wrote:
> > > I have tested this patch and agree that using msleep is the right.  Please
> > > apply this patch to the tpm driver.  One hunk might fail b/c the
> > > typo has been fixed already.
> >
> > Would you like me to respin the patch, Greg? Or is the failed hunk ok?
> 
> I'm sorry, but I am not in charge of accepting patches for the tpm
> driver.  Why not go through the listed maintainer for this process, they
> should know how to get it into the mainline kernel tree properly.  If
> not, why would they be listed as the maintainer?  :)

Sorry about that Greg, I was just mixed-up since you had pushed the
original patch in; my fault. Will remove you from my other reply.

Thanks,
Nish
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Greg KH]

On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
> On 4/15/05, Kylene Hall <[EMAIL PROTECTED]> wrote:
> > I have tested this patch and agree that using msleep is the right.  Please
> > apply this patch to the tpm driver.  One hunk might fail b/c the
> > typo has been fixed already.
> 
> Would you like me to respin the patch, Greg? Or is the failed hunk ok?

I'm sorry, but I am not in charge of accepting patches for the tpm
driver.  Why not go through the listed maintainer for this process, they
should know how to get it into the mainline kernel tree properly.  If
not, why would they be listed as the maintainer?  :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Kylene Hall <[EMAIL PROTECTED]> wrote:
> I have tested this patch and agree that using msleep is the right.  Please
> apply this patch to the tpm driver.  One hunk might fail b/c the
> typo has been fixed already.

Would you like me to respin the patch, Greg? Or is the failed hunk ok?

Thanks,
Nish
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Kylene Hall]

I have tested this patch and agree that using msleep is the right.  Please 
apply this patch to the tpm driver.  One hunk might fail b/c the 
typo has been fixed already.

Thanks,
Kylie Hall

On Fri, 11 Mar 2005, Nishanth Aravamudan wrote:

> Not sure what happened to the original mail, but I'm not seeing it
> yet...
> 
> On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
> > ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [EMAIL PROTECTED]
> > 
> > [PATCH] Add TPM hardware enablement driver
> > 
> > This patch is a device driver to enable new hardware.  The new hardware is
> > the TPM chip as described by specifications at
> > .  The TPM chip will enable you to
> > use hardware to securely store and protect your keys and personal data.
> > To use the chip according to the specification, you will need the Trusted
> > Software Stack (TSS) of which an implementation for Linux is available at:
> > .
> 
> Here is a patch that removes all callers of schedule_timeout() as I
> previously mentioned:
> 
> Description: The TPM driver unnecessarily uses timers when it simply
> needs to maintain a maximum delay via time_before(). msleep() is used
> instead of schedule_timeout() to guarantee the task delays as expected.
> While compile-testing, I found a typo in the driver, using tpm_chp
> instead of tpm_chip. Remove the now unused timer callback function and
> change TPM_TIMEOUT's units to milliseconds. Patch is compile-tested.
> 
> Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]>
> 
> diff -urpN 2.6.11-v/drivers/char/tpm/tpm.c 2.6.11/drivers/char/tpm/tpm.c
> --- 2.6.11-v/drivers/char/tpm/tpm.c   2005-03-10 10:50:00.0 -0800
> +++ 2.6.11/drivers/char/tpm/tpm.c 2005-03-10 11:00:50.0 -0800
> @@ -19,7 +19,7 @@
>   * 
>   * Note, the TPM chip is not interrupt driven (only polling)
>   * and can have very long timeouts (minutes!). Hence the unusual
> - * calls to schedule_timeout.
> + * calls to msleep.
>   *
>   */
>  
> @@ -52,14 +52,6 @@ static void user_reader_timeout(unsigned
>   up(>buffer_mutex);
>  }
>  
> -void tpm_time_expired(unsigned long ptr)
> -{
> - int *exp = (int *) ptr;
> - *exp = 1;
> -}
> -
> -EXPORT_SYMBOL_GPL(tpm_time_expired);
> -
>  /*
>   * Initialize the LPC bus and enable the TPM ports
>   */
> @@ -135,6 +127,7 @@ static ssize_t tpm_transmit(struct tpm_c
>   ssize_t len;
>   u32 count;
>   __be32 *native_size;
> + unsigned long stop;
>  
>   native_size = (__force __be32 *) (buf + 2);
>   count = be32_to_cpu(*native_size);
> @@ -155,28 +148,16 @@ static ssize_t tpm_transmit(struct tpm_c
>   return len;
>   }
>  
> - down(>timer_manipulation_mutex);
> - chip->time_expired = 0;
> - init_timer(>device_timer);
> - chip->device_timer.function = tpm_time_expired;
> - chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> - chip->device_timer.data = (unsigned long) >time_expired;
> - add_timer(>device_timer);
> - up(>timer_manipulation_mutex);
> -
> + stop = jiffies + 2 * 60 * HZ;
>   do {
>   u8 status = inb(chip->vendor->base + 1);
>   if ((status & chip->vendor->req_complete_mask) ==
>   chip->vendor->req_complete_val) {
> - down(>timer_manipulation_mutex);
> - del_singleshot_timer_sync(>device_timer);
> - up(>timer_manipulation_mutex);
>   goto out_recv;
>   }
> - set_current_state(TASK_UNINTERRUPTIBLE);
> - schedule_timeout(TPM_TIMEOUT);
> + msleep(TPM_TIMEOUT); /* CHECK */
>   rmb();
> - } while (!chip->time_expired);
> + } while (time_before(jiffies, stop));
>  
>  
>   chip->vendor->cancel(chip);
> @@ -219,7 +200,7 @@ static ssize_t show_pcrs(struct device *
>   int i, j, index, num_pcrs;
>   char *str = buf;
>  
> - struct tpm_chp *chip =
> + struct tpm_chip *chip =
>   pci_get_drvdata(container_of(dev, struct pci_dev, dev));
>   if (chip == NULL)
>   return -ENODEV;
> @@ -450,10 +431,8 @@ ssize_t tpm_write(struct file * file, co
>  
>   /* cannot perform a write until the read has cleared
>  either via tpm_read or a user_read_timer timeout */
> - while (atomic_read(>data_pending) != 0) {
> - set_current_state(TASK_UNINTERRUPTIBLE);
> - schedule_timeout(TPM_TIMEOUT);
> - }
> + while (atomic_read(>data_pending) != 0)
> + msleep(TPM_TIMEOUT);
>  
>   down(>buffer_mutex);
>  
> diff -urpN 2.6.11-v/drivers/char/tpm/tpm.h 2.6.11/drivers/char/tpm/tpm.h
> --- 2.6.11-v/drivers/char/tpm/tpm.h   2005-03-10 10:50:00.0 -0800
> +++ 2.6.11/drivers/char/tpm/tpm.h 2005-03-10 10:58:12.0 -0800
> @@ -24,7 +24,7 @@
>  #include 
>  #include 
>  
> -#define TPM_TIMEOUT msecs_to_jiffies(5)
> +#define 

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Kylene Hall]

I have tested this patch and agree that using msleep is the right.  Please 
apply this patch to the tpm driver.  One hunk might fail b/c the 
typo has been fixed already.

Thanks,
Kylie Hall

On Fri, 11 Mar 2005, Nishanth Aravamudan wrote:

 Not sure what happened to the original mail, but I'm not seeing it
 yet...
 
 On Wed, Mar 09, 2005 at 04:42:01PM -0800, Greg KH wrote:
  ChangeSet 1.2035, 2005/03/09 10:12:19-08:00, [EMAIL PROTECTED]
  
  [PATCH] Add TPM hardware enablement driver
  
  This patch is a device driver to enable new hardware.  The new hardware is
  the TPM chip as described by specifications at
  http://www.trustedcomputinggroup.org.  The TPM chip will enable you to
  use hardware to securely store and protect your keys and personal data.
  To use the chip according to the specification, you will need the Trusted
  Software Stack (TSS) of which an implementation for Linux is available at:
  http://sourceforge.net/projects/trousers.
 
 Here is a patch that removes all callers of schedule_timeout() as I
 previously mentioned:
 
 Description: The TPM driver unnecessarily uses timers when it simply
 needs to maintain a maximum delay via time_before(). msleep() is used
 instead of schedule_timeout() to guarantee the task delays as expected.
 While compile-testing, I found a typo in the driver, using tpm_chp
 instead of tpm_chip. Remove the now unused timer callback function and
 change TPM_TIMEOUT's units to milliseconds. Patch is compile-tested.
 
 Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED]
 
 diff -urpN 2.6.11-v/drivers/char/tpm/tpm.c 2.6.11/drivers/char/tpm/tpm.c
 --- 2.6.11-v/drivers/char/tpm/tpm.c   2005-03-10 10:50:00.0 -0800
 +++ 2.6.11/drivers/char/tpm/tpm.c 2005-03-10 11:00:50.0 -0800
 @@ -19,7 +19,7 @@
   * 
   * Note, the TPM chip is not interrupt driven (only polling)
   * and can have very long timeouts (minutes!). Hence the unusual
 - * calls to schedule_timeout.
 + * calls to msleep.
   *
   */
  
 @@ -52,14 +52,6 @@ static void user_reader_timeout(unsigned
   up(chip-buffer_mutex);
  }
  
 -void tpm_time_expired(unsigned long ptr)
 -{
 - int *exp = (int *) ptr;
 - *exp = 1;
 -}
 -
 -EXPORT_SYMBOL_GPL(tpm_time_expired);
 -
  /*
   * Initialize the LPC bus and enable the TPM ports
   */
 @@ -135,6 +127,7 @@ static ssize_t tpm_transmit(struct tpm_c
   ssize_t len;
   u32 count;
   __be32 *native_size;
 + unsigned long stop;
  
   native_size = (__force __be32 *) (buf + 2);
   count = be32_to_cpu(*native_size);
 @@ -155,28 +148,16 @@ static ssize_t tpm_transmit(struct tpm_c
   return len;
   }
  
 - down(chip-timer_manipulation_mutex);
 - chip-time_expired = 0;
 - init_timer(chip-device_timer);
 - chip-device_timer.function = tpm_time_expired;
 - chip-device_timer.expires = jiffies + 2 * 60 * HZ;
 - chip-device_timer.data = (unsigned long) chip-time_expired;
 - add_timer(chip-device_timer);
 - up(chip-timer_manipulation_mutex);
 -
 + stop = jiffies + 2 * 60 * HZ;
   do {
   u8 status = inb(chip-vendor-base + 1);
   if ((status  chip-vendor-req_complete_mask) ==
   chip-vendor-req_complete_val) {
 - down(chip-timer_manipulation_mutex);
 - del_singleshot_timer_sync(chip-device_timer);
 - up(chip-timer_manipulation_mutex);
   goto out_recv;
   }
 - set_current_state(TASK_UNINTERRUPTIBLE);
 - schedule_timeout(TPM_TIMEOUT);
 + msleep(TPM_TIMEOUT); /* CHECK */
   rmb();
 - } while (!chip-time_expired);
 + } while (time_before(jiffies, stop));
  
  
   chip-vendor-cancel(chip);
 @@ -219,7 +200,7 @@ static ssize_t show_pcrs(struct device *
   int i, j, index, num_pcrs;
   char *str = buf;
  
 - struct tpm_chp *chip =
 + struct tpm_chip *chip =
   pci_get_drvdata(container_of(dev, struct pci_dev, dev));
   if (chip == NULL)
   return -ENODEV;
 @@ -450,10 +431,8 @@ ssize_t tpm_write(struct file * file, co
  
   /* cannot perform a write until the read has cleared
  either via tpm_read or a user_read_timer timeout */
 - while (atomic_read(chip-data_pending) != 0) {
 - set_current_state(TASK_UNINTERRUPTIBLE);
 - schedule_timeout(TPM_TIMEOUT);
 - }
 + while (atomic_read(chip-data_pending) != 0)
 + msleep(TPM_TIMEOUT);
  
   down(chip-buffer_mutex);
  
 diff -urpN 2.6.11-v/drivers/char/tpm/tpm.h 2.6.11/drivers/char/tpm/tpm.h
 --- 2.6.11-v/drivers/char/tpm/tpm.h   2005-03-10 10:50:00.0 -0800
 +++ 2.6.11/drivers/char/tpm/tpm.h 2005-03-10 10:58:12.0 -0800
 @@ -24,7 +24,7 @@
  #include linux/delay.h
  #include linux/miscdevice.h
  
 -#define TPM_TIMEOUT msecs_to_jiffies(5)
 +#define TPM_TIMEOUT  5   /* msecs */
  
  /* TPM addresses */
  #define  TPM_ADDR

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Kylene Hall [EMAIL PROTECTED] wrote:
 I have tested this patch and agree that using msleep is the right.  Please
 apply this patch to the tpm driver.  One hunk might fail b/c the
 typo has been fixed already.

Would you like me to respin the patch, Greg? Or is the failed hunk ok?

Thanks,
Nish
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Greg KH]

On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
 On 4/15/05, Kylene Hall [EMAIL PROTECTED] wrote:
  I have tested this patch and agree that using msleep is the right.  Please
  apply this patch to the tpm driver.  One hunk might fail b/c the
  typo has been fixed already.
 
 Would you like me to respin the patch, Greg? Or is the failed hunk ok?

I'm sorry, but I am not in charge of accepting patches for the tpm
driver.  Why not go through the listed maintainer for this process, they
should know how to get it into the mainline kernel tree properly.  If
not, why would they be listed as the maintainer?  :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH [EMAIL PROTECTED] wrote:
 On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
  On 4/15/05, Kylene Hall [EMAIL PROTECTED] wrote:
   I have tested this patch and agree that using msleep is the right.  Please
   apply this patch to the tpm driver.  One hunk might fail b/c the
   typo has been fixed already.
 
  Would you like me to respin the patch, Greg? Or is the failed hunk ok?
 
 I'm sorry, but I am not in charge of accepting patches for the tpm
 driver.  Why not go through the listed maintainer for this process, they
 should know how to get it into the mainline kernel tree properly.  If
 not, why would they be listed as the maintainer?  :)

Sorry about that Greg, I was just mixed-up since you had pushed the
original patch in; my fault. Will remove you from my other reply.

Thanks,
Nish
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] char/tpm: use msleep(), clean-up timers, fix typo

[Nish Aravamudan]

On 4/15/05, Greg KH [EMAIL PROTECTED] wrote:
 On Fri, Apr 15, 2005 at 01:44:55PM -0700, Nish Aravamudan wrote:
  On 4/15/05, Kylene Hall [EMAIL PROTECTED] wrote:
   I have tested this patch and agree that using msleep is the right.  Please
   apply this patch to the tpm driver.  One hunk might fail b/c the
   typo has been fixed already.
 
  Would you like me to respin the patch, Greg? Or is the failed hunk ok?
 
 I'm sorry, but I am not in charge of accepting patches for the tpm
 driver.  Why not go through the listed maintainer for this process, they
 should know how to get it into the mainline kernel tree properly.  If
 not, why would they be listed as the maintainer?  :)

Kylene, there is no entry in MAINTAINERS as of 2.6.12-rc2 for the TPM
driver? Should there be?

I am assuming tpmdd_devel can take care of merging the patch and
pushing to mainline? Please do so.

Thanks,
Nish
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/