Re: [PATCH] tpm: use clear_bit

[Andrew Morton]

Kylene Jo Hall <kjhall@xxxxxxxxxx> wrote:
>
> On Mon, 2006-04-10 at 14:50 -0700, Andrew Morton wrote:
> > Kylene Jo Hall <kjhall@xxxxxxxxxx> wrote:
> > >
> > > +	dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES] &=
> > >  +	    ~(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
> > 
> > If you were to make dev_mask[] an array of longs, this could perhaps become
> > 
> > 	clear_bit(dev_mask, chip->dev_num);
> > 
> 
> Use set_bit and clear_bit for dev_mask manipulation.
> 
> Signed-off-by: Kylie Hall <kjhall@xxxxxxxxxx>
> ---
>  drivers/char/tpm/tpm.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
> 
> --- linux-2.6.17-rc1-mm2/drivers/char/tpm/tpm.c	2006-04-11 17:30:57.612009250 -0500
> +++ linux-2.6.17-rc1/drivers/char/tpm/tpm.c	2006-04-11 17:47:23.097598250 -0500
> @@ -32,7 +32,7 @@ enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
>  	TPM_BUFSIZE = 2048,
>  	TPM_NUM_DEVICES = 256,
> -	TPM_NUM_MASK_ENTRIES = TPM_NUM_DEVICES / (8 * sizeof(int))
> +	TPM_NUM_MASK_ENTRIES = TPM_NUM_DEVICES / (8 * sizeof(unsigned long))
>  };
>  
>  enum tpm_duration {
> @@ -48,7 +48,7 @@ enum tpm_duration {
>  
>  static LIST_HEAD(tpm_chip_list);
>  static DEFINE_SPINLOCK(driver_lock);
> -static int dev_mask[TPM_NUM_MASK_ENTRIES];
> +static unsigned long dev_mask[TPM_NUM_MASK_ENTRIES];
>  
>  /*
>   * Array with one entry per ordinal defining the maximum amount
> @@ -1033,8 +1033,7 @@ void tpm_remove_hardware(struct device *
>  	sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
>  	tpm_bios_log_teardown(chip->bios_dir);
>  
> -	dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES] &=
> -	    ~(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
> +	clear_bit(chip->dev_num , dev_mask);
>  
>  	kfree(chip);
>  
> @@ -1118,7 +1117,7 @@ struct tpm_chip *tpm_register_hardware(s
>  			if ((dev_mask[i] & (1 << j)) == 0) {
>  				chip->dev_num =
>  				    i * TPM_NUM_MASK_ENTRIES + j;
> -				dev_mask[i] |= 1 << j;
> +				set_bit(chip->dev_num, dev_mask);
>  				goto dev_num_search_complete;
>  			}
>  

It should use test_bit() as well.    How's this?



--- 25/drivers/char/tpm/tpm.c~tpm-use-clear_bit-fix	Tue Apr 11 15:52:55 2006
+++ 25-akpm/drivers/char/tpm/tpm.c	Tue Apr 11 16:01:38 2006
@@ -32,7 +32,6 @@ enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 2048,
 	TPM_NUM_DEVICES = 256,
-	TPM_NUM_MASK_ENTRIES = TPM_NUM_DEVICES / (8 * sizeof(unsigned long))
 };
 
 enum tpm_duration {
@@ -48,7 +47,8 @@ enum tpm_duration {
 
 static LIST_HEAD(tpm_chip_list);
 static DEFINE_SPINLOCK(driver_lock);
-static unsigned long dev_mask[TPM_NUM_MASK_ENTRIES];
+static unsigned long dev_mask[(TPM_NUM_DEVICES + BITS_PER_LONG - 1) /
+				BITS_PER_LONG];
 
 /*
  * Array with one entry per ordinal defining the maximum amount
@@ -1032,7 +1032,7 @@ void tpm_remove_hardware(struct device *
 	sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
 	tpm_bios_log_teardown(chip->bios_dir);
 
-	clear_bit(chip->dev_num , dev_mask);
+	clear_bit(chip->dev_num, dev_mask);
 
 	kfree(chip);
 
@@ -1090,7 +1090,7 @@ struct tpm_chip *tpm_register_hardware(s
 
 	char *devname;
 	struct tpm_chip *chip;
-	int i, j;
+	int dev_num;
 
 	/* Driver specific per-device data */
 	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
@@ -1111,16 +1111,15 @@ struct tpm_chip *tpm_register_hardware(s
 
 	chip->dev_num = -1;
 
-	for (i = 0; i < TPM_NUM_MASK_ENTRIES; i++)
-		for (j = 0; j < 8 * sizeof(int); j++)
-			if ((dev_mask[i] & (1 << j)) == 0) {
-				chip->dev_num =
-				    i * TPM_NUM_MASK_ENTRIES + j;
-				set_bit(chip->dev_num, dev_mask);
-				goto dev_num_search_complete;
-			}
+	/* This should use find_first_zero_bit() */
+	for (dev_num = 0; dev_num < TPM_NUM_DEVICES; dev_num++) {
+		if (!test_bit(dev_num, dev_mask)) {
+			chip->dev_num = dev_num;
+			set_bit(dev_num, dev_mask);
+			break;
+		}
+	}
 
-dev_num_search_complete:
 	if (chip->dev_num < 0) {
 		dev_err(dev, "No available tpm device numbers\n");
 		kfree(chip);
@@ -1144,7 +1143,7 @@ dev_num_search_complete:
 			chip->vendor.miscdev.minor);
 		put_device(dev);
 		kfree(chip);
-		dev_mask[i] &= !(1 << j);
+		clear_bit(dev_num, dev_mask);
 		return NULL;
 	}
 
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/