* [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
@ 2005-04-27 22:19 Kylene Hall
2005-04-28 4:19 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Kylene Hall @ 2005-04-27 22:19 UTC (permalink / raw)
To: linux-kernel
In the current driver all sysfs files end up owned by the base driver
module rather than the module that actually owns the device this is a
problem if the module is unloaded and the file is open. This patch fixes
all that.
Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:58:48.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:36:54.000000000 -0500
@@ -133,7 +133,8 @@ static struct tpm_vendor_specific tpm_at
.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
.req_complete_val = ATML_STATUS_DATA_AVAIL,
.base = TPM_ATML_BASE,
- .miscdev = { .fops = &atmel_ops, },
+ .attr = TPM_DEVICE_ATTRS,
+ .miscdev.fops = &atmel_ops,
};
static int __devinit tpm_atml_init(struct pci_dev *pci_dev,
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-20 19:07:43.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-20 18:57:28.000000000 -0500
@@ -207,7 +207,7 @@ static const u8 pcrread[] = {
0, 0, 0, 0 /* PCR index */
};
-static ssize_t show_pcrs(struct device *dev, char *buf)
+ssize_t tpm_show_pcrs(struct device *dev, char *buf)
{
u8 data[READ_PCR_RESULT_SIZE];
ssize_t len;
@@ -242,7 +242,7 @@ static ssize_t show_pcrs(struct device *
return str - buf;
}
-static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pcrs);
#define READ_PUBEK_RESULT_SIZE 314
static const u8 readpubek[] = {
@@ -251,7 +251,7 @@ static const u8 readpubek[] = {
0, 0, 0, 124, /* TPM_ORD_ReadPubek */
};
-static ssize_t show_pubek(struct device *dev, char *buf)
+ssize_t tpm_show_pubek(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -301,7 +301,7 @@ static ssize_t show_pubek(struct device
return str - buf;
}
-static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pubek);
#define CAP_VER_RESULT_SIZE 18
static const u8 cap_version[] = {
@@ -322,7 +322,7 @@ static const u8 cap_manufacturer[] = {
0, 0, 1, 3
};
-static ssize_t show_caps(struct device *dev, char *buf)
+ssize_t tpm_show_caps(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -356,7 +356,21 @@ static ssize_t show_caps(struct device *
return str - buf;
}
-static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_caps);
+
+ssize_t tpm_store_cancel(struct device * dev, const char *buf,
+ size_t count)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ if (chip == NULL)
+ return 0;
+
+ chip->vendor->cancel(chip);
+ return count;
+}
+
+EXPORT_SYMBOL_GPL(tpm_store_cancel);
+
/*
* Device file system interface to the TPM
@@ -532,9 +546,8 @@ void __devexit tpm_remove(struct pci_dev
pci_set_drvdata(pci_dev, NULL);
misc_deregister(&chip->vendor->miscdev);
- device_remove_file(&pci_dev->dev, &dev_attr_pubek);
- device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
- device_remove_file(&pci_dev->dev, &dev_attr_caps);
+ for (i = 0; i < TPM_NUM_ATTR; i++)
+ device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
pci_disable_device(pci_dev);
@@ -659,9 +672,8 @@ dev_num_search_complete:
list_add(&chip->list, &tpm_chip_list);
- device_create_file(&pci_dev->dev, &dev_attr_pubek);
- device_create_file(&pci_dev->dev, &dev_attr_pcrs);
- device_create_file(&pci_dev->dev, &dev_attr_caps);
+ for (i = 0; i < TPM_NUM_ATTR; i++)
+ device_create_file(&pci_dev->dev, &chip->vendor->attr[i]);
return 0;
}
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-21 16:54:56.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-21 17:05:21.000000000 -0500
@@ -531,6 +531,7 @@ EXPORT_SYMBOL_GPL(tpm_read);
void __devexit tpm_remove(struct pci_dev *pci_dev)
{
struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+ int i;
if (chip == NULL) {
dev_err(&pci_dev->dev, "No device data found\n");
--- linux-2.6.12-rc3/drivers/char/tpm/tpm_nsc.c 2005-04-27 10:40:16.000000000 -0500
+++ linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c 2005-04-25 18:49:08.000000000 -0500
@@ -223,8 +231,9 @@ static struct tpm_vendor_specific tpm_ns
.req_complete_mask = NSC_STATUS_OBF,
.req_complete_val = NSC_STATUS_OBF,
.base = TPM_NSC_BASE,
- .miscdev = { .fops = &nsc_ops, },
-
+ .attr = TPM_DEVICE_ATTRS,
+ .miscdev.fops = &nsc_ops,
+
};
static int __devinit tpm_nsc_init(struct pci_dev *pci_dev,
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h 2005-04-27 11:03:26.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/chat/tpm/tpm.h 2005-04-27 11:04:11.000000000 -0500
@@ -36,6 +36,16 @@ enum {
TPM_DATA = 0x4F
};
+extern ssize_t tpm_show_pubek(struct device *, char *);
+extern ssize_t tpm_show_pcrs(struct device *, char *);
+extern ssize_t tpm_show_caps(struct device *, char *);
+extern ssize_t tpm_store_cancel(struct device *, const char *, size_t);
+
+#define TPM_DEVICE_ATTRS { \
+ __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
+ __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
+ __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
+ __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
struct tpm_chip;
@@ -48,6 +58,7 @@ struct tpm_vendor_specific {
int (*send) (struct tpm_chip *, u8 *, size_t);
void (*cancel) (struct tpm_chip *);
struct miscdevice miscdev;
+ struct device_attribute attr[TPM_NUM_ATTR];
};
struct tpm_chip {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
2005-04-27 22:19 [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes Kylene Hall
@ 2005-04-28 4:19 ` Greg KH
2005-04-28 20:40 ` Kylene Hall
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2005-04-28 4:19 UTC (permalink / raw)
To: Kylene Hall; +Cc: linux-kernel
On Wed, Apr 27, 2005 at 05:19:03PM -0500, Kylene Hall wrote:
> - device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> - device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> - device_remove_file(&pci_dev->dev, &dev_attr_caps);
> + for (i = 0; i < TPM_NUM_ATTR; i++)
> + device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
Use an attribute group, instead of this. That will allow you to get
rid of the TPM_NUM_ATTR value, and this looney macro:
> +#define TPM_DEVICE_ATTRS { \
> + __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
> + __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
> + __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
> + __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
2005-04-28 4:19 ` Greg KH
@ 2005-04-28 20:40 ` Kylene Hall
2005-04-29 4:28 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Kylene Hall @ 2005-04-28 20:40 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel
On Wed, 27 Apr 2005, Greg KH wrote:
> On Wed, Apr 27, 2005 at 05:19:03PM -0500, Kylene Hall wrote:
> > - device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> > - device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> > - device_remove_file(&pci_dev->dev, &dev_attr_caps);
> > + for (i = 0; i < TPM_NUM_ATTR; i++)
> > + device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
>
> Use an attribute group, instead of this. That will allow you to get
> rid of the TPM_NUM_ATTR value, and this looney macro:
>
> > +#define TPM_DEVICE_ATTRS { \
> > + __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
> > + __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
> > + __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
> > + __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
>
> thanks,
>
> greg k-h
>
>
Ok, the patch below has the same functionality as the previous patch but
gets rid of the macro and implements an attribute_group. Is there any way
to avoid the repeated code in every tpm_specific file to setup the
attribute_group and still ensure the files are owned by the tpm_specific
module? The only thing I can come up with is either not using the
TPM_DEVICE macro at all or creating with TPM_DEVICE macro and then
changing the owner field.
Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:58:48.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:36:54.000000000 -0500
@@ -133,7 +133,8 @@ static struct tpm_vendor_specific tpm_at
.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
.req_complete_val = ATML_STATUS_DATA_AVAIL,
.base = TPM_ATML_BASE,
- .miscdev = { .fops = &atmel_ops, },
+ .attr_group = &atmel_attr_grp,
+ .miscdev.fops = &atmel_ops,
};
static int __devinit tpm_atml_init(struct pci_dev *pci_dev,
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-20 19:07:43.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-20 18:57:28.000000000 -0500
@@ -207,7 +207,7 @@ static const u8 pcrread[] = {
0, 0, 0, 0 /* PCR index */
};
-static ssize_t show_pcrs(struct device *dev, char *buf)
+ssize_t tpm_show_pcrs(struct device *dev, char *buf)
{
u8 data[READ_PCR_RESULT_SIZE];
ssize_t len;
@@ -242,7 +242,7 @@ static ssize_t show_pcrs(struct device *
return str - buf;
}
-static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pcrs);
#define READ_PUBEK_RESULT_SIZE 314
static const u8 readpubek[] = {
@@ -251,7 +251,7 @@ static const u8 readpubek[] = {
0, 0, 0, 124, /* TPM_ORD_ReadPubek */
};
-static ssize_t show_pubek(struct device *dev, char *buf)
+ssize_t tpm_show_pubek(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -301,7 +301,7 @@ static ssize_t show_pubek(struct device
return str - buf;
}
-static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pubek);
#define CAP_VER_RESULT_SIZE 18
static const u8 cap_version[] = {
@@ -322,7 +322,7 @@ static const u8 cap_manufacturer[] = {
0, 0, 1, 3
};
-static ssize_t show_caps(struct device *dev, char *buf)
+ssize_t tpm_show_caps(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -356,7 +356,21 @@ static ssize_t show_caps(struct device *
return str - buf;
}
-static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_caps);
+
+ssize_t tpm_store_cancel(struct device * dev, const char *buf,
+ size_t count)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ if (chip == NULL)
+ return 0;
+
+ chip->vendor->cancel(chip);
+ return count;
+}
+
+EXPORT_SYMBOL_GPL(tpm_store_cancel);
+
/*
* Device file system interface to the TPM
@@ -532,9 +546,7 @@ void __devexit tpm_remove(struct pci_dev
pci_set_drvdata(pci_dev, NULL);
misc_deregister(&chip->vendor->miscdev);
- device_remove_file(&pci_dev->dev, &dev_attr_pubek);
- device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
- device_remove_file(&pci_dev->dev, &dev_attr_caps);
+ sysfs_remove_group(&pci_dev->dev.kobj, chip->vendor->attr_group);
pci_disable_device(pci_dev);
@@ -659,9 +672,7 @@ dev_num_search_complete:
list_add(&chip->list, &tpm_chip_list);
- device_create_file(&pci_dev->dev, &dev_attr_pubek);
- device_create_file(&pci_dev->dev, &dev_attr_pcrs);
- device_create_file(&pci_dev->dev, &dev_attr_caps);
+ sysfs_create_group(&pci_dev->dev.kobj, chip->vendor->attr_group);
return 0;
}
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-21 16:54:56.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-21 17:05:21.000000000 -0500
@@ -531,6 +531,7 @@ EXPORT_SYMBOL_GPL(tpm_read);
void __devexit tpm_remove(struct pci_dev *pci_dev)
{
struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+ int i;
if (chip == NULL) {
dev_err(&pci_dev->dev, "No device data found\n");
--- linux-2.6.12-rc3/drivers/char/tpm/tpm_nsc.c 2005-04-27 10:40:16.000000000 -0500
+++ linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c 2005-04-25 18:49:08.000000000 -0500
@@ -223,8 +231,9 @@ static struct tpm_vendor_specific tpm_ns
.req_complete_mask = NSC_STATUS_OBF,
.req_complete_val = NSC_STATUS_OBF,
.base = TPM_NSC_BASE,
- .miscdev = { .fops = &nsc_ops, },
-
+ .attr_group = &nsc_attr_grp,
+ .miscdev.fops = &nsc_ops,
+
};
static int __devinit tpm_nsc_init(struct pci_dev *pci_dev,
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h 2005-04-27 11:03:26.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/chat/tpm/tpm.h 2005-04-27 11:04:11.000000000 -0500
@@ -36,6 +36,11 @@ enum {
TPM_DATA = 0x4F
};
+extern ssize_t tpm_show_pubek(struct device *, char *);
+extern ssize_t tpm_show_pcrs(struct device *, char *);
+extern ssize_t tpm_show_caps(struct device *, char *);
+extern ssize_t tpm_store_cancel(struct device *, const char *, size_t);
+
struct tpm_chip;
@@ -48,6 +58,7 @@ struct tpm_vendor_specific {
int (*send) (struct tpm_chip *, u8 *, size_t);
void (*cancel) (struct tpm_chip *);
struct miscdevice miscdev;
+ struct attribute_group *attr_group;
};
struct tpm_chip {
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-28 15:15:24.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-28 15:33:23.000000000 -0500
@@ -536,7 +536,6 @@ EXPORT_SYMBOL_GPL(tpm_read);
void __devexit tpm_remove(struct pci_dev *pci_dev)
{
struct tpm_chip *chip = pci_get_drvdata(pci_dev);
- int i;
if (chip == NULL) {
dev_err(&pci_dev->dev, "No device data found\n");
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c.orig 2005-04-28 14:55:11.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_nsc.c 2005-04-28 16:23:10.000000000 -0500
@@ -224,6 +224,21 @@ static struct file_operations nsc_ops =
.release = tpm_release,
};
+static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
+static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
+static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
+static DEVICE_ATTR(cancel, S_IWUSR|S_IWGRP, NULL, tpm_store_cancel);
+
+static struct attribute * nsc_attrs[] = {
+ &dev_attr_pubek.attr,
+ &dev_attr_pcrs.attr,
+ &dev_attr_caps.attr,
+ &dev_attr_cancel.attr,
+ 0,
+};
+
+static struct attribute_group nsc_attr_grp = { .attrs = nsc_attrs };
+
static struct tpm_vendor_specific tpm_nsc = {
.recv = tpm_nsc_recv,
.send = tpm_nsc_send,
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c.orig 2005-04-28 14:55:04.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c 2005-04-28 16:16:41.000000000 -0500
@@ -126,6 +126,21 @@ static struct file_operations atmel_ops
.release = tpm_release,
};
+static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
+static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
+static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
+static DEVICE_ATTR(cancel, S_IWUSR |S_IWGRP, NULL, tpm_store_cancel);
+
+static struct attribute* atmel_attrs[] = {
+ &dev_attr_pubek.attr,
+ &dev_attr_pcrs.attr,
+ &dev_attr_caps.attr,
+ &dev_attr_cancel.attr,
+ 0,
+};
+
+static struct attribute_group atmel_attr_grp = { .attrs = atmel_attrs };
+
static struct tpm_vendor_specific tpm_atmel = {
.recv = tpm_atml_recv,
.send = tpm_atml_send,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
2005-04-28 20:40 ` Kylene Hall
@ 2005-04-29 4:28 ` Greg KH
2005-04-29 16:30 ` Kylene Jo Hall
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2005-04-29 4:28 UTC (permalink / raw)
To: Kylene Hall; +Cc: linux-kernel
On Thu, Apr 28, 2005 at 03:40:16PM -0500, Kylene Hall wrote:
> On Wed, 27 Apr 2005, Greg KH wrote:
> > On Wed, Apr 27, 2005 at 05:19:03PM -0500, Kylene Hall wrote:
> > > - device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> > > - device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> > > - device_remove_file(&pci_dev->dev, &dev_attr_caps);
> > > + for (i = 0; i < TPM_NUM_ATTR; i++)
> > > + device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
> >
> > Use an attribute group, instead of this. That will allow you to get
> > rid of the TPM_NUM_ATTR value, and this looney macro:
> >
> > > +#define TPM_DEVICE_ATTRS { \
> > > + __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
> > > + __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
> > > + __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
> > > + __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
> >
> > thanks,
> >
> > greg k-h
> >
> >
>
> Ok, the patch below has the same functionality as the previous patch but
> gets rid of the macro and implements an attribute_group. Is there any way
> to avoid the repeated code in every tpm_specific file to setup the
> attribute_group and still ensure the files are owned by the tpm_specific
> module? The only thing I can come up with is either not using the
> TPM_DEVICE macro at all or creating with TPM_DEVICE macro and then
> changing the owner field.
Why are you trying to split this driver up into such tiny pieces?
What's wrong with one driver for all devices?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
2005-04-29 4:28 ` Greg KH
@ 2005-04-29 16:30 ` Kylene Jo Hall
2005-04-29 16:38 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Kylene Jo Hall @ 2005-04-29 16:30 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel
On Thu, 2005-04-28 at 21:28 -0700, Greg KH wrote:
> On Thu, Apr 28, 2005 at 03:40:16PM -0500, Kylene Hall wrote:
> > On Wed, 27 Apr 2005, Greg KH wrote:
> > > On Wed, Apr 27, 2005 at 05:19:03PM -0500, Kylene Hall wrote:
> > > > - device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> > > > - device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> > > > - device_remove_file(&pci_dev->dev, &dev_attr_caps);
> > > > + for (i = 0; i < TPM_NUM_ATTR; i++)
> > > > + device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
> > >
> > > Use an attribute group, instead of this. That will allow you to get
> > > rid of the TPM_NUM_ATTR value, and this looney macro:
> > >
> > > > +#define TPM_DEVICE_ATTRS { \
> > > > + __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
> > > > + __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
> > > > + __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
> > > > + __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
> > >
> > > thanks,
> > >
> > > greg k-h
> > >
> > >
> >
> > Ok, the patch below has the same functionality as the previous patch but
> > gets rid of the macro and implements an attribute_group. Is there any way
> > to avoid the repeated code in every tpm_specific file to setup the
> > attribute_group and still ensure the files are owned by the tpm_specific
> > module? The only thing I can come up with is either not using the
> > TPM_DEVICE macro at all or creating with TPM_DEVICE macro and then
> > changing the owner field.
>
> Why are you trying to split this driver up into such tiny pieces?
> What's wrong with one driver for all devices?
The driver was orginally all one piece and was split at the suggestion
of Ian Campbell on this list.
<snip>
On Fri, 2004-12-10 at 10:56 +0000, Ian Campbell wrote:
> Hi,
>
> On Thu, 2004-12-09 at 09:25 -0600, Kylene Hall wrote:
> > + /* Determine chip type */
> > + if (tpm_nsc_init(chip) == 0) {
> > + chip->recv = tpm_nsc_recv;
> > + chip->send = tpm_nsc_send;
> > + chip->cancel = tpm_nsc_cancel;
> > + chip->req_complete_mask = NSC_STATUS_OBF;
> > + chip->req_complete_val = NSC_STATUS_OBF;
> > + } else if (tpm_atml_init(chip) == 0) {
> > + chip->recv = tpm_atml_recv;
> > + chip->send = tpm_atml_send;
> > + chip->cancel = tpm_atml_cancel;
> > + chip->req_complete_mask =
> > + ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL;
> > + chip->req_complete_val = ATML_STATUS_DATA_AVAIL;
> > + } else {
> > + rc = -ENODEV;
> > + goto out_release;
> > + }
>
> The atmel part at least also comes as an I2C variant.
>
> We could continue to add to the ifelse here but perhaps it might be
> beneficial to split the individual chip specific stuff into separate
> files now and perhaps register them via some sort of
> register_tpm_hardware(struct tpm_chip_ops *) type interface?
>
> Ian.
>
<snip>
TPM vendor specific code can better determine whether the chip belongs
to them or not. Since the pci_probe process takes care of checking the
possible drivers to claim a device it was decided to split the logic up.
Here is a link to the thread:
http://www.ussg.iu.edu/hypermail/linux/kernel/0412.1/0550.html
Kylie
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
2005-04-29 16:30 ` Kylene Jo Hall
@ 2005-04-29 16:38 ` Greg KH
0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2005-04-29 16:38 UTC (permalink / raw)
To: Kylene Jo Hall; +Cc: linux-kernel
On Fri, Apr 29, 2005 at 11:30:12AM -0500, Kylene Jo Hall wrote:
> On Thu, 2005-04-28 at 21:28 -0700, Greg KH wrote:
> > On Thu, Apr 28, 2005 at 03:40:16PM -0500, Kylene Hall wrote:
> > > On Wed, 27 Apr 2005, Greg KH wrote:
> > > > On Wed, Apr 27, 2005 at 05:19:03PM -0500, Kylene Hall wrote:
> > > > > - device_remove_file(&pci_dev->dev, &dev_attr_pubek);
> > > > > - device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
> > > > > - device_remove_file(&pci_dev->dev, &dev_attr_caps);
> > > > > + for (i = 0; i < TPM_NUM_ATTR; i++)
> > > > > + device_remove_file(&pci_dev->dev, &chip->vendor->attr[i]);
> > > >
> > > > Use an attribute group, instead of this. That will allow you to get
> > > > rid of the TPM_NUM_ATTR value, and this looney macro:
> > > >
> > > > > +#define TPM_DEVICE_ATTRS { \
> > > > > + __ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL), \
> > > > > + __ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL), \
> > > > > + __ATTR(caps, S_IRUGO, tpm_show_caps, NULL), \
> > > > > + __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel) }
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > > >
> > > >
> > >
> > > Ok, the patch below has the same functionality as the previous patch but
> > > gets rid of the macro and implements an attribute_group. Is there any way
> > > to avoid the repeated code in every tpm_specific file to setup the
> > > attribute_group and still ensure the files are owned by the tpm_specific
> > > module? The only thing I can come up with is either not using the
> > > TPM_DEVICE macro at all or creating with TPM_DEVICE macro and then
> > > changing the owner field.
> >
> > Why are you trying to split this driver up into such tiny pieces?
> > What's wrong with one driver for all devices?
>
> The driver was orginally all one piece and was split at the suggestion
> of Ian Campbell on this list.
Oh, I remember that. But if it turns out to be impractical...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes
@ 2005-05-05 19:11 Kylene Hall
0 siblings, 0 replies; 7+ messages in thread
From: Kylene Hall @ 2005-05-05 19:11 UTC (permalink / raw)
To: akpm; +Cc: greg, linux-kernel
Please apply these fixes to the Tpm driver. I am resubmitting the entire
patch set that was orginally sent to LKML on April 27 with the changes
that were requested fixed.
Fixed in this patch is how the sysfs attributes are created and the files
have been lumped into a attribute_group.
Thanks,
Kylie
In the current driver all sysfs files end up owned by the base driver
module rather than the module that actually owns the device this is a
problem if the module is unloaded and the file is open. This patch fixes
all that.
Signed-off-by: Kylene Hall <kjhall@us.ibm.com>
---
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:58:48.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c 2005-04-20 18:36:54.000000000 -0500
@@ -133,7 +133,8 @@ static struct tpm_vendor_specific tpm_at
.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
.req_complete_val = ATML_STATUS_DATA_AVAIL,
.base = TPM_ATML_BASE,
- .miscdev = { .fops = &atmel_ops, },
+ .attr_group = &atmel_attr_grp,
+ .miscdev.fops = &atmel_ops,
};
static int __devinit tpm_atml_init(struct pci_dev *pci_dev,
diff -uprN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-20 19:07:43.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-20 18:57:28.000000000 -0500
@@ -207,7 +207,7 @@ static const u8 pcrread[] = {
0, 0, 0, 0 /* PCR index */
};
-static ssize_t show_pcrs(struct device *dev, char *buf)
+ssize_t tpm_show_pcrs(struct device *dev, char *buf)
{
u8 data[READ_PCR_RESULT_SIZE];
ssize_t len;
@@ -242,7 +242,7 @@ static ssize_t show_pcrs(struct device *
return str - buf;
}
-static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pcrs);
#define READ_PUBEK_RESULT_SIZE 314
static const u8 readpubek[] = {
@@ -251,7 +251,7 @@ static const u8 readpubek[] = {
0, 0, 0, 124, /* TPM_ORD_ReadPubek */
};
-static ssize_t show_pubek(struct device *dev, char *buf)
+ssize_t tpm_show_pubek(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -301,7 +301,7 @@ static ssize_t show_pubek(struct device
return str - buf;
}
-static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_pubek);
#define CAP_VER_RESULT_SIZE 18
static const u8 cap_version[] = {
@@ -322,7 +322,7 @@ static const u8 cap_manufacturer[] = {
0, 0, 1, 3
};
-static ssize_t show_caps(struct device *dev, char *buf)
+ssize_t tpm_show_caps(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -356,7 +356,21 @@ static ssize_t show_caps(struct device *
return str - buf;
}
-static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
+EXPORT_SYMBOL_GPL(tpm_show_caps);
+
+ssize_t tpm_store_cancel(struct device * dev, const char *buf,
+ size_t count)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ if (chip == NULL)
+ return 0;
+
+ chip->vendor->cancel(chip);
+ return count;
+}
+
+EXPORT_SYMBOL_GPL(tpm_store_cancel);
+
/*
* Device file system interface to the TPM
@@ -532,9 +546,7 @@ void __devexit tpm_remove(struct pci_dev
pci_set_drvdata(pci_dev, NULL);
misc_deregister(&chip->vendor->miscdev);
- device_remove_file(&pci_dev->dev, &dev_attr_pubek);
- device_remove_file(&pci_dev->dev, &dev_attr_pcrs);
- device_remove_file(&pci_dev->dev, &dev_attr_caps);
+ sysfs_remove_group(&pci_dev->dev.kobj, chip->vendor->attr_group);
pci_disable_device(pci_dev);
@@ -659,9 +672,7 @@ dev_num_search_complete:
list_add(&chip->list, &tpm_chip_list);
- device_create_file(&pci_dev->dev, &dev_attr_pubek);
- device_create_file(&pci_dev->dev, &dev_attr_pcrs);
- device_create_file(&pci_dev->dev, &dev_attr_caps);
+ sysfs_create_group(&pci_dev->dev.kobj, chip->vendor->attr_group);
return 0;
}
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-21 16:54:56.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-21 17:05:21.000000000 -0500
@@ -531,6 +531,7 @@ EXPORT_SYMBOL_GPL(tpm_read);
void __devexit tpm_remove(struct pci_dev *pci_dev)
{
struct tpm_chip *chip = pci_get_drvdata(pci_dev);
+ int i;
if (chip == NULL) {
dev_err(&pci_dev->dev, "No device data found\n");
--- linux-2.6.12-rc3/drivers/char/tpm/tpm_nsc.c 2005-04-27 10:40:16.000000000 -0500
+++ linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c 2005-04-25 18:49:08.000000000 -0500
@@ -223,8 +231,9 @@ static struct tpm_vendor_specific tpm_ns
.req_complete_mask = NSC_STATUS_OBF,
.req_complete_val = NSC_STATUS_OBF,
.base = TPM_NSC_BASE,
- .miscdev = { .fops = &nsc_ops, },
-
+ .attr_group = &nsc_attr_grp,
+ .miscdev.fops = &nsc_ops,
+
};
static int __devinit tpm_nsc_init(struct pci_dev *pci_dev,
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h 2005-04-27 11:03:26.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/chat/tpm/tpm.h 2005-04-27 11:04:11.000000000 -0500
@@ -36,6 +36,11 @@ enum {
TPM_DATA = 0x4F
};
+extern ssize_t tpm_show_pubek(struct device *, char *);
+extern ssize_t tpm_show_pcrs(struct device *, char *);
+extern ssize_t tpm_show_caps(struct device *, char *);
+extern ssize_t tpm_store_cancel(struct device *, const char *, size_t);
+
struct tpm_chip;
@@ -48,6 +58,7 @@ struct tpm_vendor_specific {
int (*send) (struct tpm_chip *, u8 *, size_t);
void (*cancel) (struct tpm_chip *);
struct miscdevice miscdev;
+ struct attribute_group *attr_group;
};
struct tpm_chip {
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-28 15:15:24.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-28 15:33:23.000000000 -0500
@@ -536,7 +536,6 @@ EXPORT_SYMBOL_GPL(tpm_read);
void __devexit tpm_remove(struct pci_dev *pci_dev)
{
struct tpm_chip *chip = pci_get_drvdata(pci_dev);
- int i;
if (chip == NULL) {
dev_err(&pci_dev->dev, "No device data found\n");
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_nsc.c.orig 2005-04-28 14:55:11.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_nsc.c 2005-04-28 16:23:10.000000000 -0500
@@ -224,6 +224,21 @@ static struct file_operations nsc_ops =
.release = tpm_release,
};
+static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
+static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
+static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
+static DEVICE_ATTR(cancel, S_IWUSR|S_IWGRP, NULL, tpm_store_cancel);
+
+static struct attribute * nsc_attrs[] = {
+ &dev_attr_pubek.attr,
+ &dev_attr_pcrs.attr,
+ &dev_attr_caps.attr,
+ &dev_attr_cancel.attr,
+ 0,
+};
+
+static struct attribute_group nsc_attr_grp = { .attrs = nsc_attrs };
+
static struct tpm_vendor_specific tpm_nsc = {
.recv = tpm_nsc_recv,
.send = tpm_nsc_send,
--- linux-2.6.12-rc2/drivers/char/tpm/tpm_atmel.c.orig 2005-04-28 14:55:04.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm_atmel.c 2005-04-28 16:16:41.000000000 -0500
@@ -126,6 +126,21 @@ static struct file_operations atmel_ops
.release = tpm_release,
};
+static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
+static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
+static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
+static DEVICE_ATTR(cancel, S_IWUSR |S_IWGRP, NULL, tpm_store_cancel);
+
+static struct attribute* atmel_attrs[] = {
+ &dev_attr_pubek.attr,
+ &dev_attr_pcrs.attr,
+ &dev_attr_caps.attr,
+ &dev_attr_cancel.attr,
+ 0,
+};
+
+static struct attribute_group atmel_attr_grp = { .attrs = atmel_attrs };
+
static struct tpm_vendor_specific tpm_atmel = {
.recv = tpm_atml_recv,
.send = tpm_atml_send,
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2005-05-05 19:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-04-27 22:19 [PATCH 10 of 12] Fix Tpm driver -- sysfs owernship changes Kylene Hall
2005-04-28 4:19 ` Greg KH
2005-04-28 20:40 ` Kylene Hall
2005-04-29 4:28 ` Greg KH
2005-04-29 16:30 ` Kylene Jo Hall
2005-04-29 16:38 ` Greg KH
2005-05-05 19:11 Kylene Hall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).