| Message ID | 1458502483-16887-6-git-send-email-christophe-h.ricard@st.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: > Remove TPM_VPRIV macro and replace with tpm_set_vendordata and > tpm_get_vendordata. NAK. These unnecessary wrapper functions make code just harder to read. I'd rather accept a patch that would drop TPM_VPRIV and not introduce new clutter. /Jarkko > Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> > --- > drivers/char/tpm/st33zp24/st33zp24.c | 26 +++++++++++++------------- > drivers/char/tpm/tpm.h | 12 ++++++++++-- > drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++---- > drivers/char/tpm/tpm_tis.c | 2 +- > drivers/char/tpm/xen-tpmfront.c | 14 +++++++------- > 5 files changed, 35 insertions(+), 27 deletions(-) > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c > index 9e91ca7..dc2d0f3 100644 > --- a/drivers/char/tpm/st33zp24/st33zp24.c > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > @@ -105,7 +105,7 @@ static void st33zp24_cancel(struct tpm_chip *chip) > struct st33zp24_dev *tpm_dev; > u8 data; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > data = TPM_STS_COMMAND_READY; > tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1); > @@ -121,7 +121,7 @@ static u8 st33zp24_status(struct tpm_chip *chip) > struct st33zp24_dev *tpm_dev; > u8 data; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1); > return data; > @@ -138,7 +138,7 @@ static int check_locality(struct tpm_chip *chip) > u8 data; > u8 status; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > if (status && (data & > @@ -164,7 +164,7 @@ static int request_locality(struct tpm_chip *chip) > if (check_locality(chip) == chip->vendor.locality) > return chip->vendor.locality; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > data = TPM_ACCESS_REQUEST_USE; > ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > @@ -193,7 +193,7 @@ static void release_locality(struct tpm_chip *chip) > struct st33zp24_dev *tpm_dev; > u8 data; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > data = TPM_ACCESS_ACTIVE_LOCALITY; > > tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > @@ -211,7 +211,7 @@ static int get_burstcount(struct tpm_chip *chip) > u8 temp; > struct st33zp24_dev *tpm_dev; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > stop = jiffies + chip->vendor.timeout_d; > do { > @@ -279,7 +279,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout, > u8 status; > struct st33zp24_dev *tpm_dev; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > /* check current status */ > status = st33zp24_status(chip); > @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count) > int size = 0, burstcnt, len, ret; > struct st33zp24_dev *tpm_dev; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > while (size < count && > wait_for_stat(chip, > @@ -372,7 +372,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, void *dev_id) > struct tpm_chip *chip = dev_id; > struct st33zp24_dev *tpm_dev; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > tpm_dev->intrs++; > wake_up_interruptible(&chip->vendor.read_queue); > @@ -404,7 +404,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf, > if (len < TPM_HEADER_SIZE) > return -EBUSY; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > ret = request_locality(chip); > if (ret < 0) > @@ -565,7 +565,7 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, > if (!tpm_dev) > return -ENOMEM; > > - TPM_VPRIV(chip) = tpm_dev; > + tpm_set_vendordata(chip, tpm_dev); > tpm_dev->phy_id = phy_id; > tpm_dev->ops = ops; > > @@ -653,7 +653,7 @@ int st33zp24_pm_suspend(struct device *dev) > struct st33zp24_dev *tpm_dev; > int ret = 0; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > if (gpio_is_valid(tpm_dev->io_lpcpd)) > gpio_set_value(tpm_dev->io_lpcpd, 0); > @@ -675,7 +675,7 @@ int st33zp24_pm_resume(struct device *dev) > struct st33zp24_dev *tpm_dev; > int ret = 0; > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > if (gpio_is_valid(tpm_dev->io_lpcpd)) { > gpio_set_value(tpm_dev->io_lpcpd, 1); > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 28a0c17..6a973b9 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -157,8 +157,6 @@ struct tpm_vendor_specific { > u16 manufacturer_id; > }; > > -#define TPM_VPRIV(c) ((c)->vendor.priv) > - > #define TPM_VID_INTEL 0x8086 > #define TPM_VID_WINBOND 0x1050 > #define TPM_VID_STM 0x104A > @@ -203,6 +201,16 @@ struct tpm_chip { > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > > +static inline void tpm_set_vendordata(struct tpm_chip *chip, void *data) > +{ > + chip->vendor.priv = data; > +} > + > +static inline void *tpm_get_vendordata(struct tpm_chip *chip) > +{ > + return chip->vendor.priv; > +} > + > static inline int tpm_read_index(int base, int index) > { > outb(index, base); > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index b0a9a9e..c8b1643 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -64,7 +64,7 @@ static struct ibmvtpm_dev *ibmvtpm_get_data(const struct device *dev) > { > struct tpm_chip *chip = dev_get_drvdata(dev); > if (chip) > - return (struct ibmvtpm_dev *)TPM_VPRIV(chip); > + return (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > return NULL; > } > > @@ -83,7 +83,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > u16 len; > int sig; > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > if (!ibmvtpm->rtce_buf) { > dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); > @@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > __be64 *word = (__be64 *)&crq; > int rc, sig; > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > if (!ibmvtpm->rtce_buf) { > dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); > @@ -643,7 +643,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > > crq_q->index = 0; > > - TPM_VPRIV(chip) = (void *)ibmvtpm; > + tpm_set_vendordata(chip, (void *)ibmvtpm); > > spin_lock_init(&ibmvtpm->rtce_lock); > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index eed3bf5..607fa3f 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -678,7 +678,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > if (IS_ERR(chip)) > return PTR_ERR(chip); > > - chip->vendor.priv = priv; > + tpm_set_vendordata(chip, priv); > #ifdef CONFIG_ACPI > chip->acpi_dev_handle = acpi_dev_handle; > #endif > diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c > index 3111f27..c472fd8 100644 > --- a/drivers/char/tpm/xen-tpmfront.c > +++ b/drivers/char/tpm/xen-tpmfront.c > @@ -39,7 +39,7 @@ enum status_bits { > > static u8 vtpm_status(struct tpm_chip *chip) > { > - struct tpm_private *priv = TPM_VPRIV(chip); > + struct tpm_private *priv = tpm_get_vendordata(chip); > switch (priv->shr->state) { > case VTPM_STATE_IDLE: > return VTPM_STATUS_IDLE | VTPM_STATUS_CANCELED; > @@ -60,7 +60,7 @@ static bool vtpm_req_canceled(struct tpm_chip *chip, u8 status) > > static void vtpm_cancel(struct tpm_chip *chip) > { > - struct tpm_private *priv = TPM_VPRIV(chip); > + struct tpm_private *priv = tpm_get_vendordata(chip); > priv->shr->state = VTPM_STATE_CANCEL; > wmb(); > notify_remote_via_evtchn(priv->evtchn); > @@ -73,7 +73,7 @@ static unsigned int shr_data_offset(struct vtpm_shared_page *shr) > > static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > { > - struct tpm_private *priv = TPM_VPRIV(chip); > + struct tpm_private *priv = tpm_get_vendordata(chip); > struct vtpm_shared_page *shr = priv->shr; > unsigned int offset = shr_data_offset(shr); > > @@ -115,7 +115,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > > static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > { > - struct tpm_private *priv = TPM_VPRIV(chip); > + struct tpm_private *priv = tpm_get_vendordata(chip); > struct vtpm_shared_page *shr = priv->shr; > unsigned int offset = shr_data_offset(shr); > size_t length = shr->length; > @@ -182,7 +182,7 @@ static int setup_chip(struct device *dev, struct tpm_private *priv) > init_waitqueue_head(&chip->vendor.read_queue); > > priv->chip = chip; > - TPM_VPRIV(chip) = priv; > + tpm_set_vendordata(chip, priv); > > return 0; > } > @@ -318,10 +318,10 @@ static int tpmfront_probe(struct xenbus_device *dev, > static int tpmfront_remove(struct xenbus_device *dev) > { > struct tpm_chip *chip = dev_get_drvdata(&dev->dev); > - struct tpm_private *priv = TPM_VPRIV(chip); > + struct tpm_private *priv = tpm_get_vendordata(chip); > tpm_chip_unregister(chip); > ring_free(priv); > - TPM_VPRIV(chip) = NULL; > + tpm_set_vendordata(chip, NULL); > return 0; > } > > -- > 2.5.0 > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On Tue, Mar 22, 2016 at 08:20:01AM +0200, Jarkko Sakkinen wrote: > On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: > > Remove TPM_VPRIV macro and replace with tpm_set_vendordata and > > tpm_get_vendordata. > > NAK. These unnecessary wrapper functions make code just harder to read. > I'd rather accept a patch that would drop TPM_VPRIV and not introduce > new clutter. We should be getting pretty close to being able to use drvdata on the chip->dev for this sort of stuff... Jason ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
Hi Jason, Jarkko, I think i found some way to get to it. I will share something as soon as it is ready. Best Regards Christophe On 22/03/2016 16:58, Jason Gunthorpe wrote: > On Tue, Mar 22, 2016 at 08:20:01AM +0200, Jarkko Sakkinen wrote: >> On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: >>> Remove TPM_VPRIV macro and replace with tpm_set_vendordata and >>> tpm_get_vendordata. >> NAK. These unnecessary wrapper functions make code just harder to read. >> I'd rather accept a patch that would drop TPM_VPRIV and not introduce >> new clutter. > We should be getting pretty close to being able to use drvdata on the > chip->dev for this sort of stuff... > > Jason ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
Hi Jarkko, Just coming back to you on this one. My intention was mainly align the "code style" from my eyes with other similar functions from other subsystems. For example and coming to my mind: i2c: i2c_get_clientdata/i2c_set_clientdata spi: spi_get_drvdata/spi_set_drvdata nfc: nfc_get_drvdata/nfc_set_drvdata In the mean time when trying to cleanup the code i found helpers with the exact same purpose in tpm_atmel.h (atmel_get_priv) or tpm_nsc.c (tpm_nsc_get_priv). When working on the tpm_vendor_specific cleanup, i came also through the conclusion that moving priv from tpm_vendor_specific to tpm_chip structure would the most simple and appropriate solution in short term. ...priv field may be renamed data... Do you see anything against a serie removing atmel_get_priv and tpm_nsc_get_priv and with a common helper replacing TPM_VPRIV ? Best Regards Christophe 2016-03-22 7:20 GMT+01:00 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>: > On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: > > Remove TPM_VPRIV macro and replace with tpm_set_vendordata and > > tpm_get_vendordata. > > NAK. These unnecessary wrapper functions make code just harder to read. > I'd rather accept a patch that would drop TPM_VPRIV and not introduce > new clutter. > > /Jarkko > > > Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> > > --- > > drivers/char/tpm/st33zp24/st33zp24.c | 26 +++++++++++++------------- > > drivers/char/tpm/tpm.h | 12 ++++++++++-- > > drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++---- > > drivers/char/tpm/tpm_tis.c | 2 +- > > drivers/char/tpm/xen-tpmfront.c | 14 +++++++------- > > 5 files changed, 35 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c > b/drivers/char/tpm/st33zp24/st33zp24.c > > index 9e91ca7..dc2d0f3 100644 > > --- a/drivers/char/tpm/st33zp24/st33zp24.c > > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > > @@ -105,7 +105,7 @@ static void st33zp24_cancel(struct tpm_chip *chip) > > struct st33zp24_dev *tpm_dev; > > u8 data; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > data = TPM_STS_COMMAND_READY; > > tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1); > > @@ -121,7 +121,7 @@ static u8 st33zp24_status(struct tpm_chip *chip) > > struct st33zp24_dev *tpm_dev; > > u8 data; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1); > > return data; > > @@ -138,7 +138,7 @@ static int check_locality(struct tpm_chip *chip) > > u8 data; > > u8 status; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > > if (status && (data & > > @@ -164,7 +164,7 @@ static int request_locality(struct tpm_chip *chip) > > if (check_locality(chip) == chip->vendor.locality) > > return chip->vendor.locality; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > data = TPM_ACCESS_REQUEST_USE; > > ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > > @@ -193,7 +193,7 @@ static void release_locality(struct tpm_chip *chip) > > struct st33zp24_dev *tpm_dev; > > u8 data; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > data = TPM_ACCESS_ACTIVE_LOCALITY; > > > > tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > > @@ -211,7 +211,7 @@ static int get_burstcount(struct tpm_chip *chip) > > u8 temp; > > struct st33zp24_dev *tpm_dev; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > stop = jiffies + chip->vendor.timeout_d; > > do { > > @@ -279,7 +279,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 > mask, unsigned long timeout, > > u8 status; > > struct st33zp24_dev *tpm_dev; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > /* check current status */ > > status = st33zp24_status(chip); > > @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, > size_t count) > > int size = 0, burstcnt, len, ret; > > struct st33zp24_dev *tpm_dev; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > while (size < count && > > wait_for_stat(chip, > > @@ -372,7 +372,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, > void *dev_id) > > struct tpm_chip *chip = dev_id; > > struct st33zp24_dev *tpm_dev; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > tpm_dev->intrs++; > > wake_up_interruptible(&chip->vendor.read_queue); > > @@ -404,7 +404,7 @@ static int st33zp24_send(struct tpm_chip *chip, > unsigned char *buf, > > if (len < TPM_HEADER_SIZE) > > return -EBUSY; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > ret = request_locality(chip); > > if (ret < 0) > > @@ -565,7 +565,7 @@ int st33zp24_probe(void *phy_id, const struct > st33zp24_phy_ops *ops, > > if (!tpm_dev) > > return -ENOMEM; > > > > - TPM_VPRIV(chip) = tpm_dev; > > + tpm_set_vendordata(chip, tpm_dev); > > tpm_dev->phy_id = phy_id; > > tpm_dev->ops = ops; > > > > @@ -653,7 +653,7 @@ int st33zp24_pm_suspend(struct device *dev) > > struct st33zp24_dev *tpm_dev; > > int ret = 0; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > if (gpio_is_valid(tpm_dev->io_lpcpd)) > > gpio_set_value(tpm_dev->io_lpcpd, 0); > > @@ -675,7 +675,7 @@ int st33zp24_pm_resume(struct device *dev) > > struct st33zp24_dev *tpm_dev; > > int ret = 0; > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > if (gpio_is_valid(tpm_dev->io_lpcpd)) { > > gpio_set_value(tpm_dev->io_lpcpd, 1); > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > index 28a0c17..6a973b9 100644 > > --- a/drivers/char/tpm/tpm.h > > +++ b/drivers/char/tpm/tpm.h > > @@ -157,8 +157,6 @@ struct tpm_vendor_specific { > > u16 manufacturer_id; > > }; > > > > -#define TPM_VPRIV(c) ((c)->vendor.priv) > > - > > #define TPM_VID_INTEL 0x8086 > > #define TPM_VID_WINBOND 0x1050 > > #define TPM_VID_STM 0x104A > > @@ -203,6 +201,16 @@ struct tpm_chip { > > > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > > > > +static inline void tpm_set_vendordata(struct tpm_chip *chip, void *data) > > +{ > > + chip->vendor.priv = data; > > +} > > + > > +static inline void *tpm_get_vendordata(struct tpm_chip *chip) > > +{ > > + return chip->vendor.priv; > > +} > > + > > static inline int tpm_read_index(int base, int index) > > { > > outb(index, base); > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c > b/drivers/char/tpm/tpm_ibmvtpm.c > > index b0a9a9e..c8b1643 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > @@ -64,7 +64,7 @@ static struct ibmvtpm_dev *ibmvtpm_get_data(const > struct device *dev) > > { > > struct tpm_chip *chip = dev_get_drvdata(dev); > > if (chip) > > - return (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > + return (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > return NULL; > > } > > > > @@ -83,7 +83,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 > *buf, size_t count) > > u16 len; > > int sig; > > > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > > if (!ibmvtpm->rtce_buf) { > > dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); > > @@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, > u8 *buf, size_t count) > > __be64 *word = (__be64 *)&crq; > > int rc, sig; > > > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > > if (!ibmvtpm->rtce_buf) { > > dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); > > @@ -643,7 +643,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > > > > crq_q->index = 0; > > > > - TPM_VPRIV(chip) = (void *)ibmvtpm; > > + tpm_set_vendordata(chip, (void *)ibmvtpm); > > > > spin_lock_init(&ibmvtpm->rtce_lock); > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > index eed3bf5..607fa3f 100644 > > --- a/drivers/char/tpm/tpm_tis.c > > +++ b/drivers/char/tpm/tpm_tis.c > > @@ -678,7 +678,7 @@ static int tpm_tis_init(struct device *dev, struct > tpm_info *tpm_info, > > if (IS_ERR(chip)) > > return PTR_ERR(chip); > > > > - chip->vendor.priv = priv; > > + tpm_set_vendordata(chip, priv); > > #ifdef CONFIG_ACPI > > chip->acpi_dev_handle = acpi_dev_handle; > > #endif > > diff --git a/drivers/char/tpm/xen-tpmfront.c > b/drivers/char/tpm/xen-tpmfront.c > > index 3111f27..c472fd8 100644 > > --- a/drivers/char/tpm/xen-tpmfront.c > > +++ b/drivers/char/tpm/xen-tpmfront.c > > @@ -39,7 +39,7 @@ enum status_bits { > > > > static u8 vtpm_status(struct tpm_chip *chip) > > { > > - struct tpm_private *priv = TPM_VPRIV(chip); > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > switch (priv->shr->state) { > > case VTPM_STATE_IDLE: > > return VTPM_STATUS_IDLE | VTPM_STATUS_CANCELED; > > @@ -60,7 +60,7 @@ static bool vtpm_req_canceled(struct tpm_chip *chip, > u8 status) > > > > static void vtpm_cancel(struct tpm_chip *chip) > > { > > - struct tpm_private *priv = TPM_VPRIV(chip); > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > priv->shr->state = VTPM_STATE_CANCEL; > > wmb(); > > notify_remote_via_evtchn(priv->evtchn); > > @@ -73,7 +73,7 @@ static unsigned int shr_data_offset(struct > vtpm_shared_page *shr) > > > > static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > > { > > - struct tpm_private *priv = TPM_VPRIV(chip); > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > struct vtpm_shared_page *shr = priv->shr; > > unsigned int offset = shr_data_offset(shr); > > > > @@ -115,7 +115,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, > size_t count) > > > > static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > > { > > - struct tpm_private *priv = TPM_VPRIV(chip); > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > struct vtpm_shared_page *shr = priv->shr; > > unsigned int offset = shr_data_offset(shr); > > size_t length = shr->length; > > @@ -182,7 +182,7 @@ static int setup_chip(struct device *dev, struct > tpm_private *priv) > > init_waitqueue_head(&chip->vendor.read_queue); > > > > priv->chip = chip; > > - TPM_VPRIV(chip) = priv; > > + tpm_set_vendordata(chip, priv); > > > > return 0; > > } > > @@ -318,10 +318,10 @@ static int tpmfront_probe(struct xenbus_device > *dev, > > static int tpmfront_remove(struct xenbus_device *dev) > > { > > struct tpm_chip *chip = dev_get_drvdata(&dev->dev); > > - struct tpm_private *priv = TPM_VPRIV(chip); > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > tpm_chip_unregister(chip); > > ring_free(priv); > > - TPM_VPRIV(chip) = NULL; > > + tpm_set_vendordata(chip, NULL); > > return 0; > > } > > > > -- > > 2.5.0 > > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On Wed, Mar 23, 2016 at 02:11:26PM +0100, Christophe Ricard wrote: > When working on the tpm_vendor_specific cleanup, i came also through > the conclusion that moving priv from tpm_vendor_specific to tpm_chip > structure would the most > simple and appropriate solution in short term. Someday, but we really should just use chip->dev drvdata for 'priv' > ...priv field may be renamed data... > Do you see anything against a serie removing atmel_get_priv and > tpm_nsc_get_priv and with a common helper replacing TPM_VPRIV ? These functions include a cast, they can't be generic. Jason ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On Wed, Mar 23, 2016 at 02:11:26PM +0100, Christophe Ricard wrote: > Hi Jarkko, > > Just coming back to you on this one. My intention was mainly align the > "code style" from my eyes with other similar functions from other > subsystems. > For example and coming to my mind: > i2c: i2c_get_clientdata/i2c_set_clientdata > spi: spi_get_drvdata/spi_set_drvdata > nfc: nfc_get_drvdata/nfc_set_drvdata > > In the mean time when trying to cleanup the code i found helpers with the > exact same purpose in tpm_atmel.h (atmel_get_priv) or tpm_nsc.c > (tpm_nsc_get_priv). > > When working on the tpm_vendor_specific cleanup, i came also through the > conclusion that moving priv from tpm_vendor_specific to tpm_chip structure > would the most > simple and appropriate solution in short term. > > ...priv field may be renamed data... > Do you see anything against a serie removing atmel_get_priv and > tpm_nsc_get_priv and with a common helper replacing TPM_VPRIV ? Yes I do. The current problem is that HW specific stuff lives in the framework and not in the drivers for that HW. I would like to see HW drivers to use priv for their internal and get rid off tpm_vendor_specific completely. /Jarkko > Best Regards > Christophe > 2016-03-22 7:20 GMT+01:00 Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com>: > > On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: > > Remove TPM_VPRIV macro and replace with tpm_set_vendordata and > > tpm_get_vendordata. > > NAK. These unnecessary wrapper functions make code just harder to read. > I'd rather accept a patch that would drop TPM_VPRIV and not introduce > new clutter. > /Jarkko > > Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> > > --- > > drivers/char/tpm/st33zp24/st33zp24.c | 26 +++++++++++++------------- > > drivers/char/tpm/tpm.h        | 12 ++++++++++-- > > drivers/char/tpm/tpm_ibmvtpm.c    | 8 ++++---- > > drivers/char/tpm/tpm_tis.c      | 2 +- > > drivers/char/tpm/xen-tpmfront.c   | 14 +++++++------- > > 5 files changed, 35 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c > b/drivers/char/tpm/st33zp24/st33zp24.c > > index 9e91ca7..dc2d0f3 100644 > > --- a/drivers/char/tpm/st33zp24/st33zp24.c > > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > > @@ -105,7 +105,7 @@ static void st33zp24_cancel(struct tpm_chip *chip) > >    struct st33zp24_dev *tpm_dev; > >    u8 data; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    data = TPM_STS_COMMAND_READY; > >    tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1); > > @@ -121,7 +121,7 @@ static u8 st33zp24_status(struct tpm_chip *chip) > >    struct st33zp24_dev *tpm_dev; > >    u8 data; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1); > >    return data; > > @@ -138,7 +138,7 @@ static int check_locality(struct tpm_chip *chip) > >    u8 data; > >    u8 status; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, > &data, 1); > >    if (status && (data & > > @@ -164,7 +164,7 @@ static int request_locality(struct tpm_chip *chip) > >    if (check_locality(chip) == chip->vendor.locality) > >        return chip->vendor.locality; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    data = TPM_ACCESS_REQUEST_USE; > >    ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, > 1); > > @@ -193,7 +193,7 @@ static void release_locality(struct tpm_chip > *chip) > >    struct st33zp24_dev *tpm_dev; > >    u8 data; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > >    data = TPM_ACCESS_ACTIVE_LOCALITY; > > > >    tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > > @@ -211,7 +211,7 @@ static int get_burstcount(struct tpm_chip *chip) > >    u8 temp; > >    struct st33zp24_dev *tpm_dev; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    stop = jiffies + chip->vendor.timeout_d; > >    do { > > @@ -279,7 +279,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 > mask, unsigned long timeout, > >    u8 status; > >    struct st33zp24_dev *tpm_dev; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    /* check current status */ > >    status = st33zp24_status(chip); > > @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 > *buf, size_t count) > >    int size = 0, burstcnt, len, ret; > >    struct st33zp24_dev *tpm_dev; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    while (size < count && > >       wait_for_stat(chip, > > @@ -372,7 +372,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, > void *dev_id) > >    struct tpm_chip *chip = dev_id; > >    struct st33zp24_dev *tpm_dev; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    tpm_dev->intrs++; > >    wake_up_interruptible(&chip->vendor.read_queue); > > @@ -404,7 +404,7 @@ static int st33zp24_send(struct tpm_chip *chip, > unsigned char *buf, > >    if (len < TPM_HEADER_SIZE) > >        return -EBUSY; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    ret = request_locality(chip); > >    if (ret < 0) > > @@ -565,7 +565,7 @@ int st33zp24_probe(void *phy_id, const struct > st33zp24_phy_ops *ops, > >    if (!tpm_dev) > >        return -ENOMEM; > > > > -   TPM_VPRIV(chip) = tpm_dev; > > +   tpm_set_vendordata(chip, tpm_dev); > >    tpm_dev->phy_id = phy_id; > >    tpm_dev->ops = ops; > > > > @@ -653,7 +653,7 @@ int st33zp24_pm_suspend(struct device *dev) > >    struct st33zp24_dev *tpm_dev; > >    int ret = 0; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    if (gpio_is_valid(tpm_dev->io_lpcpd)) > >        gpio_set_value(tpm_dev->io_lpcpd, 0); > > @@ -675,7 +675,7 @@ int st33zp24_pm_resume(struct device *dev) > >    struct st33zp24_dev *tpm_dev; > >    int ret = 0; > > > > -   tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > +   tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > >    if (gpio_is_valid(tpm_dev->io_lpcpd)) { > >        gpio_set_value(tpm_dev->io_lpcpd, 1); > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > index 28a0c17..6a973b9 100644 > > --- a/drivers/char/tpm/tpm.h > > +++ b/drivers/char/tpm/tpm.h > > @@ -157,8 +157,6 @@ struct tpm_vendor_specific { > >    u16 manufacturer_id; > > }; > > > > -#define TPM_VPRIV(c)   ((c)->vendor.priv) > > - > > #define TPM_VID_INTEL  0x8086 > > #define TPM_VID_WINBOND 0x1050 > > #define TPM_VID_STM   0x104A > > @@ -203,6 +201,16 @@ struct tpm_chip { > > > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > > > > +static inline void tpm_set_vendordata(struct tpm_chip *chip, void > *data) > > +{ > > +   chip->vendor.priv = data; > > +} > > + > > +static inline void *tpm_get_vendordata(struct tpm_chip *chip) > > +{ > > +   return chip->vendor.priv; > > +} > > + > > static inline int tpm_read_index(int base, int index) > > { > >    outb(index, base); > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c > b/drivers/char/tpm/tpm_ibmvtpm.c > > index b0a9a9e..c8b1643 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > @@ -64,7 +64,7 @@ static struct ibmvtpm_dev *ibmvtpm_get_data(const > struct device *dev) > > { > >    struct tpm_chip *chip = dev_get_drvdata(dev); > >    if (chip) > > -       return (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > +       return (struct ibmvtpm_dev > *)tpm_get_vendordata(chip); > >    return NULL; > > } > > > > @@ -83,7 +83,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, > u8 *buf, size_t count) > >    u16 len; > >    int sig; > > > > -   ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > +   ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > >    if (!ibmvtpm->rtce_buf) { > >        dev_err(ibmvtpm->dev, "ibmvtpm device is not > ready\n"); > > @@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, > u8 *buf, size_t count) > >    __be64 *word = (__be64 *)&crq; > >    int rc, sig; > > > > -   ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > +   ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > >    if (!ibmvtpm->rtce_buf) { > >        dev_err(ibmvtpm->dev, "ibmvtpm device is not > ready\n"); > > @@ -643,7 +643,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev > *vio_dev, > > > >    crq_q->index = 0; > > > > -   TPM_VPRIV(chip) = (void *)ibmvtpm; > > +   tpm_set_vendordata(chip, (void *)ibmvtpm); > > > >    spin_lock_init(&ibmvtpm->rtce_lock); > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > index eed3bf5..607fa3f 100644 > > --- a/drivers/char/tpm/tpm_tis.c > > +++ b/drivers/char/tpm/tpm_tis.c > > @@ -678,7 +678,7 @@ static int tpm_tis_init(struct device *dev, struct > tpm_info *tpm_info, > >    if (IS_ERR(chip)) > >        return PTR_ERR(chip); > > > > -   chip->vendor.priv = priv; > > +   tpm_set_vendordata(chip, priv); > > #ifdef CONFIG_ACPI > >    chip->acpi_dev_handle = acpi_dev_handle; > > #endif > > diff --git a/drivers/char/tpm/xen-tpmfront.c > b/drivers/char/tpm/xen-tpmfront.c > > index 3111f27..c472fd8 100644 > > --- a/drivers/char/tpm/xen-tpmfront.c > > +++ b/drivers/char/tpm/xen-tpmfront.c > > @@ -39,7 +39,7 @@ enum status_bits { > > > > static u8 vtpm_status(struct tpm_chip *chip) > > { > > -   struct tpm_private *priv = TPM_VPRIV(chip); > > +   struct tpm_private *priv = tpm_get_vendordata(chip); > >    switch (priv->shr->state) { > >    case VTPM_STATE_IDLE: > >        return VTPM_STATUS_IDLE | VTPM_STATUS_CANCELED; > > @@ -60,7 +60,7 @@ static bool vtpm_req_canceled(struct tpm_chip *chip, > u8 status) > > > > static void vtpm_cancel(struct tpm_chip *chip) > > { > > -   struct tpm_private *priv = TPM_VPRIV(chip); > > +   struct tpm_private *priv = tpm_get_vendordata(chip); > >    priv->shr->state = VTPM_STATE_CANCEL; > >    wmb(); > >    notify_remote_via_evtchn(priv->evtchn); > > @@ -73,7 +73,7 @@ static unsigned int shr_data_offset(struct > vtpm_shared_page *shr) > > > > static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > > { > > -   struct tpm_private *priv = TPM_VPRIV(chip); > > +   struct tpm_private *priv = tpm_get_vendordata(chip); > >    struct vtpm_shared_page *shr = priv->shr; > >    unsigned int offset = shr_data_offset(shr); > > > > @@ -115,7 +115,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 > *buf, size_t count) > > > > static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > > { > > -   struct tpm_private *priv = TPM_VPRIV(chip); > > +   struct tpm_private *priv = tpm_get_vendordata(chip); > >    struct vtpm_shared_page *shr = priv->shr; > >    unsigned int offset = shr_data_offset(shr); > >    size_t length = shr->length; > > @@ -182,7 +182,7 @@ static int setup_chip(struct device *dev, struct > tpm_private *priv) > >    init_waitqueue_head(&chip->vendor.read_queue); > > > >    priv->chip = chip; > > -   TPM_VPRIV(chip) = priv; > > +   tpm_set_vendordata(chip, priv); > > > >    return 0; > > } > > @@ -318,10 +318,10 @@ static int tpmfront_probe(struct xenbus_device > *dev, > > static int tpmfront_remove(struct xenbus_device *dev) > > { > >    struct tpm_chip *chip = dev_get_drvdata(&dev->dev); > > -   struct tpm_private *priv = TPM_VPRIV(chip); > > +   struct tpm_private *priv = tpm_get_vendordata(chip); > >    tpm_chip_unregister(chip); > >    ring_free(priv); > > -   TPM_VPRIV(chip) = NULL; > > +   tpm_set_vendordata(chip, NULL); > >    return 0; > > } > > > > -- > > 2.5.0 > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
Hi Jarkko, I am currently working at removing this TPM_VPRIV and priv field as requested previously. As pointed by you or Jason, i succeeded to go the dev_get_drvdata way :). Will send a new serie asap once ready. Best Regards Christophe 2016-03-24 14:27 GMT+01:00 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> : > On Wed, Mar 23, 2016 at 02:11:26PM +0100, Christophe Ricard wrote: > > Hi Jarkko, > > > > Just coming back to you on this one. My intention was mainly align the > > "code style" from my eyes with other similar functions from other > > subsystems. > > For example and coming to my mind: > > i2c: i2c_get_clientdata/i2c_set_clientdata > > spi: spi_get_drvdata/spi_set_drvdata > > nfc: nfc_get_drvdata/nfc_set_drvdata > > > > In the mean time when trying to cleanup the code i found helpers with > the > > exact same purpose in tpm_atmel.h (atmel_get_priv) or tpm_nsc.c > > (tpm_nsc_get_priv). > > > > When working on the tpm_vendor_specific cleanup, i came also through > the > > conclusion that moving priv from tpm_vendor_specific to tpm_chip > structure > > would the most > > simple and appropriate solution in short term. > > > > ...priv field may be renamed data... > > Do you see anything against a serie removing atmel_get_priv and > > tpm_nsc_get_priv and with a common helper replacing TPM_VPRIV ? > > Yes I do. The current problem is that HW specific stuff lives in the > framework and not in the drivers for that HW. I would like to see HW > drivers to use priv for their internal and get rid off > tpm_vendor_specific completely. > > /Jarkko > > > Best Regards > > Christophe > > 2016-03-22 7:20 GMT+01:00 Jarkko Sakkinen > > <jarkko.sakkinen@linux.intel.com>: > > > > On Sun, Mar 20, 2016 at 08:34:36PM +0100, Christophe Ricard wrote: > > > Remove TPM_VPRIV macro and replace with tpm_set_vendordata and > > > tpm_get_vendordata. > > > > NAK. These unnecessary wrapper functions make code just harder to > read. > > I'd rather accept a patch that would drop TPM_VPRIV and not > introduce > > new clutter. > > /Jarkko > > > Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> > > > --- > > > drivers/char/tpm/st33zp24/st33zp24.c | 26 > +++++++++++++------------- > > > drivers/char/tpm/tpm.h | 12 ++++++++++-- > > > drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++---- > > > drivers/char/tpm/tpm_tis.c | 2 +- > > > drivers/char/tpm/xen-tpmfront.c | 14 +++++++------- > > > 5 files changed, 35 insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c > > b/drivers/char/tpm/st33zp24/st33zp24.c > > > index 9e91ca7..dc2d0f3 100644 > > > --- a/drivers/char/tpm/st33zp24/st33zp24.c > > > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > > > @@ -105,7 +105,7 @@ static void st33zp24_cancel(struct tpm_chip > *chip) > > > struct st33zp24_dev *tpm_dev; > > > u8 data; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > data = TPM_STS_COMMAND_READY; > > > tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1); > > > @@ -121,7 +121,7 @@ static u8 st33zp24_status(struct tpm_chip > *chip) > > > struct st33zp24_dev *tpm_dev; > > > u8 data; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1); > > > return data; > > > @@ -138,7 +138,7 @@ static int check_locality(struct tpm_chip > *chip) > > > u8 data; > > > u8 status; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, > > &data, 1); > > > if (status && (data & > > > @@ -164,7 +164,7 @@ static int request_locality(struct tpm_chip > *chip) > > > if (check_locality(chip) == chip->vendor.locality) > > > return chip->vendor.locality; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > data = TPM_ACCESS_REQUEST_USE; > > > ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, > > 1); > > > @@ -193,7 +193,7 @@ static void release_locality(struct tpm_chip > > *chip) > > > struct st33zp24_dev *tpm_dev; > > > u8 data; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > data = TPM_ACCESS_ACTIVE_LOCALITY; > > > > > > tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); > > > @@ -211,7 +211,7 @@ static int get_burstcount(struct tpm_chip > *chip) > > > u8 temp; > > > struct st33zp24_dev *tpm_dev; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > stop = jiffies + chip->vendor.timeout_d; > > > do { > > > @@ -279,7 +279,7 @@ static int wait_for_stat(struct tpm_chip > *chip, u8 > > mask, unsigned long timeout, > > > u8 status; > > > struct st33zp24_dev *tpm_dev; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > /* check current status */ > > > status = st33zp24_status(chip); > > > @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 > > *buf, size_t count) > > > int size = 0, burstcnt, len, ret; > > > struct st33zp24_dev *tpm_dev; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > while (size < count && > > > wait_for_stat(chip, > > > @@ -372,7 +372,7 @@ static irqreturn_t tpm_ioserirq_handler(int > irq, > > void *dev_id) > > > struct tpm_chip *chip = dev_id; > > > struct st33zp24_dev *tpm_dev; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > tpm_dev->intrs++; > > > wake_up_interruptible(&chip->vendor.read_queue); > > > @@ -404,7 +404,7 @@ static int st33zp24_send(struct tpm_chip > *chip, > > unsigned char *buf, > > > if (len < TPM_HEADER_SIZE) > > > return -EBUSY; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > ret = request_locality(chip); > > > if (ret < 0) > > > @@ -565,7 +565,7 @@ int st33zp24_probe(void *phy_id, const struct > > st33zp24_phy_ops *ops, > > > if (!tpm_dev) > > > return -ENOMEM; > > > > > > - TPM_VPRIV(chip) = tpm_dev; > > > + tpm_set_vendordata(chip, tpm_dev); > > > tpm_dev->phy_id = phy_id; > > > tpm_dev->ops = ops; > > > > > > @@ -653,7 +653,7 @@ int st33zp24_pm_suspend(struct device *dev) > > > struct st33zp24_dev *tpm_dev; > > > int ret = 0; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > if (gpio_is_valid(tpm_dev->io_lpcpd)) > > > gpio_set_value(tpm_dev->io_lpcpd, 0); > > > @@ -675,7 +675,7 @@ int st33zp24_pm_resume(struct device *dev) > > > struct st33zp24_dev *tpm_dev; > > > int ret = 0; > > > > > > - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); > > > + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); > > > > > > if (gpio_is_valid(tpm_dev->io_lpcpd)) { > > > gpio_set_value(tpm_dev->io_lpcpd, 1); > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > > index 28a0c17..6a973b9 100644 > > > --- a/drivers/char/tpm/tpm.h > > > +++ b/drivers/char/tpm/tpm.h > > > @@ -157,8 +157,6 @@ struct tpm_vendor_specific { > > > u16 manufacturer_id; > > > }; > > > > > > -#define TPM_VPRIV(c) ((c)->vendor.priv) > > > - > > > #define TPM_VID_INTEL 0x8086 > > > #define TPM_VID_WINBOND 0x1050 > > > #define TPM_VID_STM 0x104A > > > @@ -203,6 +201,16 @@ struct tpm_chip { > > > > > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > > > > > > +static inline void tpm_set_vendordata(struct tpm_chip *chip, void > > *data) > > > +{ > > > + chip->vendor.priv = data; > > > +} > > > + > > > +static inline void *tpm_get_vendordata(struct tpm_chip *chip) > > > +{ > > > + return chip->vendor.priv; > > > +} > > > + > > > static inline int tpm_read_index(int base, int index) > > > { > > > outb(index, base); > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c > > b/drivers/char/tpm/tpm_ibmvtpm.c > > > index b0a9a9e..c8b1643 100644 > > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > > @@ -64,7 +64,7 @@ static struct ibmvtpm_dev > *ibmvtpm_get_data(const > > struct device *dev) > > > { > > > struct tpm_chip *chip = dev_get_drvdata(dev); > > > if (chip) > > > - return (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > > + return (struct ibmvtpm_dev > > *)tpm_get_vendordata(chip); > > > return NULL; > > > } > > > > > > @@ -83,7 +83,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip > *chip, > > u8 *buf, size_t count) > > > u16 len; > > > int sig; > > > > > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > > > > if (!ibmvtpm->rtce_buf) { > > > dev_err(ibmvtpm->dev, "ibmvtpm device is not > > ready\n"); > > > @@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip > *chip, > > u8 *buf, size_t count) > > > __be64 *word = (__be64 *)&crq; > > > int rc, sig; > > > > > > - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > > + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); > > > > > > if (!ibmvtpm->rtce_buf) { > > > dev_err(ibmvtpm->dev, "ibmvtpm device is not > > ready\n"); > > > @@ -643,7 +643,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev > > *vio_dev, > > > > > > crq_q->index = 0; > > > > > > - TPM_VPRIV(chip) = (void *)ibmvtpm; > > > + tpm_set_vendordata(chip, (void *)ibmvtpm); > > > > > > spin_lock_init(&ibmvtpm->rtce_lock); > > > > > > diff --git a/drivers/char/tpm/tpm_tis.c > b/drivers/char/tpm/tpm_tis.c > > > index eed3bf5..607fa3f 100644 > > > --- a/drivers/char/tpm/tpm_tis.c > > > +++ b/drivers/char/tpm/tpm_tis.c > > > @@ -678,7 +678,7 @@ static int tpm_tis_init(struct device *dev, > struct > > tpm_info *tpm_info, > > > if (IS_ERR(chip)) > > > return PTR_ERR(chip); > > > > > > - chip->vendor.priv = priv; > > > + tpm_set_vendordata(chip, priv); > > > #ifdef CONFIG_ACPI > > > chip->acpi_dev_handle = acpi_dev_handle; > > > #endif > > > diff --git a/drivers/char/tpm/xen-tpmfront.c > > b/drivers/char/tpm/xen-tpmfront.c > > > index 3111f27..c472fd8 100644 > > > --- a/drivers/char/tpm/xen-tpmfront.c > > > +++ b/drivers/char/tpm/xen-tpmfront.c > > > @@ -39,7 +39,7 @@ enum status_bits { > > > > > > static u8 vtpm_status(struct tpm_chip *chip) > > > { > > > - struct tpm_private *priv = TPM_VPRIV(chip); > > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > > switch (priv->shr->state) { > > > case VTPM_STATE_IDLE: > > > return VTPM_STATUS_IDLE | VTPM_STATUS_CANCELED; > > > @@ -60,7 +60,7 @@ static bool vtpm_req_canceled(struct tpm_chip > *chip, > > u8 status) > > > > > > static void vtpm_cancel(struct tpm_chip *chip) > > > { > > > - struct tpm_private *priv = TPM_VPRIV(chip); > > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > > priv->shr->state = VTPM_STATE_CANCEL; > > > wmb(); > > > notify_remote_via_evtchn(priv->evtchn); > > > @@ -73,7 +73,7 @@ static unsigned int shr_data_offset(struct > > vtpm_shared_page *shr) > > > > > > static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t > count) > > > { > > > - struct tpm_private *priv = TPM_VPRIV(chip); > > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > > struct vtpm_shared_page *shr = priv->shr; > > > unsigned int offset = shr_data_offset(shr); > > > > > > @@ -115,7 +115,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 > > *buf, size_t count) > > > > > > static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t > count) > > > { > > > - struct tpm_private *priv = TPM_VPRIV(chip); > > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > > struct vtpm_shared_page *shr = priv->shr; > > > unsigned int offset = shr_data_offset(shr); > > > size_t length = shr->length; > > > @@ -182,7 +182,7 @@ static int setup_chip(struct device *dev, > struct > > tpm_private *priv) > > > init_waitqueue_head(&chip->vendor.read_queue); > > > > > > priv->chip = chip; > > > - TPM_VPRIV(chip) = priv; > > > + tpm_set_vendordata(chip, priv); > > > > > > return 0; > > > } > > > @@ -318,10 +318,10 @@ static int tpmfront_probe(struct > xenbus_device > > *dev, > > > static int tpmfront_remove(struct xenbus_device *dev) > > > { > > > struct tpm_chip *chip = dev_get_drvdata(&dev->dev); > > > - struct tpm_private *priv = TPM_VPRIV(chip); > > > + struct tpm_private *priv = tpm_get_vendordata(chip); > > > tpm_chip_unregister(chip); > > > ring_free(priv); > > > - TPM_VPRIV(chip) = NULL; > > > + tpm_set_vendordata(chip, NULL); > > > return 0; > > > } > > > > > > -- > > > 2.5.0 > > > > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On Thu, Mar 24, 2016 at 02:30:24PM +0100, Christophe Ricard wrote: > Hi Jarkko, > > I am currently working at removing this TPM_VPRIV and priv field as requested > previously. > As pointed by you or Jason, i succeeded to go the dev_get_drvdata way :). We may need another patch or two to allow that, I have an ugly feeling the chip->drvdata is being used for something Jason ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
I got confused like you are but when looking a little bit better at the code i think we have 2 differents "struct device" available on a tpm driver. One is coming from the physical layer driver (aka: client->dev, spi->dev, pdev->dev) and the other one from chip->dev which is initialized in tpm_chip_alloc. As a consequence, a quick trick allows to set data to physical layer driver or to the tpm_chip device. I haven't seen any place where this is already used for anything. Do you confirm ? Best Regards Christophe 2016-03-24 17:18 GMT+01:00 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> : > On Thu, Mar 24, 2016 at 02:30:24PM +0100, Christophe Ricard wrote: > > Hi Jarkko, > > > > I am currently working at removing this TPM_VPRIV and priv field as > requested > > previously. > > As pointed by you or Jason, i succeeded to go the dev_get_drvdata way :). > > We may need another patch or two to allow that, I have an ugly feeling > the chip->drvdata is being used for something > > Jason > ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On Thu, Mar 24, 2016 at 05:31:21PM +0100, Christophe Ricard wrote: > I got confused like you are but when looking a little bit better at the > code i think we have 2 differents "struct device" available on a tpm > driver. > One is coming from the physical layer driver (aka: client->dev, > spi->dev, pdev->dev) and the other one from chip->dev which is > initialized in tpm_chip_alloc. Yes, client->dev's drvdata (aka chip->dev.parent) should point to the chip. In an ideal world the TPM core will never touch this, but we are not quite there yet, sysfs in particular is broken. chip->dev's drvdata could/should point to the driver's priv, that would make sense. With Jarkko's tree today that is OK. But Stefan's vtpm driver adds a use: https://github.com/stefanberger/linux/commit/3ab4e3aabf50137068e2807910e5f867668b4e23 Which is an ugly hacky thing.. So, if you are planning a tpm_get_drvdata or whatever then we should probably patch vtpm to put priv back in or patch vtpm to not need that hack. Jason ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
On 24/03/2016 18:10, Jason Gunthorpe wrote: > On Thu, Mar 24, 2016 at 05:31:21PM +0100, Christophe Ricard wrote: >> I got confused like you are but when looking a little bit better at the >> code i think we have 2 differents "struct device" available on a tpm >> driver. >> One is coming from the physical layer driver (aka: client->dev, >> spi->dev, pdev->dev) and the other one from chip->dev which is >> initialized in tpm_chip_alloc. > Yes, client->dev's drvdata (aka chip->dev.parent) should point to the > chip. In an ideal world the TPM core will never touch this, but we are > not quite there yet, sysfs in particular is broken. > > chip->dev's drvdata could/should point to the driver's priv, that > would make sense. With Jarkko's tree today that is OK. > > But Stefan's vtpm driver adds a use: > > https://github.com/stefanberger/linux/commit/3ab4e3aabf50137068e2807910e5f867668b4e23 > > Which is an ugly hacky thing.. I ran through the changes using client->dev's drvdata pointing to the chip and &chip->dev's drvdata pointing to the driver's priv. I have looked to stefanberger githubs and i am really sceptical about the purpose of doing dev_set_drvdata(&chip->dev, chip); in tpm_sysfs_add_device. I haven't seen any dev_get_drvdata(&chip->dev) in the tree even on top of the history. Did i overlooked at it ? Shouldn't dev_set_drvdata(&chip->dev, chip); in tpm_sysfs_add_device be removed ? I plan to release the next serie about tpm_vendor_specific cleanup by tomorrow at least for further discussion :). > So, if you are planning a tpm_get_drvdata or whatever then we should > probably patch vtpm to put priv back in or patch vtpm to not need that > hack. > > Jason Best Regards Christophe ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c index 9e91ca7..dc2d0f3 100644 --- a/drivers/char/tpm/st33zp24/st33zp24.c +++ b/drivers/char/tpm/st33zp24/st33zp24.c @@ -105,7 +105,7 @@ static void st33zp24_cancel(struct tpm_chip *chip) struct st33zp24_dev *tpm_dev; u8 data; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); data = TPM_STS_COMMAND_READY; tpm_dev->ops->send(tpm_dev->phy_id, TPM_STS, &data, 1); @@ -121,7 +121,7 @@ static u8 st33zp24_status(struct tpm_chip *chip) struct st33zp24_dev *tpm_dev; u8 data; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS, &data, 1); return data; @@ -138,7 +138,7 @@ static int check_locality(struct tpm_chip *chip) u8 data; u8 status; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_ACCESS, &data, 1); if (status && (data & @@ -164,7 +164,7 @@ static int request_locality(struct tpm_chip *chip) if (check_locality(chip) == chip->vendor.locality) return chip->vendor.locality; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); data = TPM_ACCESS_REQUEST_USE; ret = tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); @@ -193,7 +193,7 @@ static void release_locality(struct tpm_chip *chip) struct st33zp24_dev *tpm_dev; u8 data; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); data = TPM_ACCESS_ACTIVE_LOCALITY; tpm_dev->ops->send(tpm_dev->phy_id, TPM_ACCESS, &data, 1); @@ -211,7 +211,7 @@ static int get_burstcount(struct tpm_chip *chip) u8 temp; struct st33zp24_dev *tpm_dev; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); stop = jiffies + chip->vendor.timeout_d; do { @@ -279,7 +279,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout, u8 status; struct st33zp24_dev *tpm_dev; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); /* check current status */ status = st33zp24_status(chip); @@ -340,7 +340,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count) int size = 0, burstcnt, len, ret; struct st33zp24_dev *tpm_dev; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); while (size < count && wait_for_stat(chip, @@ -372,7 +372,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, void *dev_id) struct tpm_chip *chip = dev_id; struct st33zp24_dev *tpm_dev; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); tpm_dev->intrs++; wake_up_interruptible(&chip->vendor.read_queue); @@ -404,7 +404,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf, if (len < TPM_HEADER_SIZE) return -EBUSY; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); ret = request_locality(chip); if (ret < 0) @@ -565,7 +565,7 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, if (!tpm_dev) return -ENOMEM; - TPM_VPRIV(chip) = tpm_dev; + tpm_set_vendordata(chip, tpm_dev); tpm_dev->phy_id = phy_id; tpm_dev->ops = ops; @@ -653,7 +653,7 @@ int st33zp24_pm_suspend(struct device *dev) struct st33zp24_dev *tpm_dev; int ret = 0; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); if (gpio_is_valid(tpm_dev->io_lpcpd)) gpio_set_value(tpm_dev->io_lpcpd, 0); @@ -675,7 +675,7 @@ int st33zp24_pm_resume(struct device *dev) struct st33zp24_dev *tpm_dev; int ret = 0; - tpm_dev = (struct st33zp24_dev *)TPM_VPRIV(chip); + tpm_dev = (struct st33zp24_dev *)tpm_get_vendordata(chip); if (gpio_is_valid(tpm_dev->io_lpcpd)) { gpio_set_value(tpm_dev->io_lpcpd, 1); diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 28a0c17..6a973b9 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -157,8 +157,6 @@ struct tpm_vendor_specific { u16 manufacturer_id; }; -#define TPM_VPRIV(c) ((c)->vendor.priv) - #define TPM_VID_INTEL 0x8086 #define TPM_VID_WINBOND 0x1050 #define TPM_VID_STM 0x104A @@ -203,6 +201,16 @@ struct tpm_chip { #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) +static inline void tpm_set_vendordata(struct tpm_chip *chip, void *data) +{ + chip->vendor.priv = data; +} + +static inline void *tpm_get_vendordata(struct tpm_chip *chip) +{ + return chip->vendor.priv; +} + static inline int tpm_read_index(int base, int index) { outb(index, base); diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index b0a9a9e..c8b1643 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -64,7 +64,7 @@ static struct ibmvtpm_dev *ibmvtpm_get_data(const struct device *dev) { struct tpm_chip *chip = dev_get_drvdata(dev); if (chip) - return (struct ibmvtpm_dev *)TPM_VPRIV(chip); + return (struct ibmvtpm_dev *)tpm_get_vendordata(chip); return NULL; } @@ -83,7 +83,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) u16 len; int sig; - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); if (!ibmvtpm->rtce_buf) { dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); @@ -127,7 +127,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) __be64 *word = (__be64 *)&crq; int rc, sig; - ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); + ibmvtpm = (struct ibmvtpm_dev *)tpm_get_vendordata(chip); if (!ibmvtpm->rtce_buf) { dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n"); @@ -643,7 +643,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, crq_q->index = 0; - TPM_VPRIV(chip) = (void *)ibmvtpm; + tpm_set_vendordata(chip, (void *)ibmvtpm); spin_lock_init(&ibmvtpm->rtce_lock); diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index eed3bf5..607fa3f 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -678,7 +678,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, if (IS_ERR(chip)) return PTR_ERR(chip); - chip->vendor.priv = priv; + tpm_set_vendordata(chip, priv); #ifdef CONFIG_ACPI chip->acpi_dev_handle = acpi_dev_handle; #endif diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c index 3111f27..c472fd8 100644 --- a/drivers/char/tpm/xen-tpmfront.c +++ b/drivers/char/tpm/xen-tpmfront.c @@ -39,7 +39,7 @@ enum status_bits { static u8 vtpm_status(struct tpm_chip *chip) { - struct tpm_private *priv = TPM_VPRIV(chip); + struct tpm_private *priv = tpm_get_vendordata(chip); switch (priv->shr->state) { case VTPM_STATE_IDLE: return VTPM_STATUS_IDLE | VTPM_STATUS_CANCELED; @@ -60,7 +60,7 @@ static bool vtpm_req_canceled(struct tpm_chip *chip, u8 status) static void vtpm_cancel(struct tpm_chip *chip) { - struct tpm_private *priv = TPM_VPRIV(chip); + struct tpm_private *priv = tpm_get_vendordata(chip); priv->shr->state = VTPM_STATE_CANCEL; wmb(); notify_remote_via_evtchn(priv->evtchn); @@ -73,7 +73,7 @@ static unsigned int shr_data_offset(struct vtpm_shared_page *shr) static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) { - struct tpm_private *priv = TPM_VPRIV(chip); + struct tpm_private *priv = tpm_get_vendordata(chip); struct vtpm_shared_page *shr = priv->shr; unsigned int offset = shr_data_offset(shr); @@ -115,7 +115,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) { - struct tpm_private *priv = TPM_VPRIV(chip); + struct tpm_private *priv = tpm_get_vendordata(chip); struct vtpm_shared_page *shr = priv->shr; unsigned int offset = shr_data_offset(shr); size_t length = shr->length; @@ -182,7 +182,7 @@ static int setup_chip(struct device *dev, struct tpm_private *priv) init_waitqueue_head(&chip->vendor.read_queue); priv->chip = chip; - TPM_VPRIV(chip) = priv; + tpm_set_vendordata(chip, priv); return 0; } @@ -318,10 +318,10 @@ static int tpmfront_probe(struct xenbus_device *dev, static int tpmfront_remove(struct xenbus_device *dev) { struct tpm_chip *chip = dev_get_drvdata(&dev->dev); - struct tpm_private *priv = TPM_VPRIV(chip); + struct tpm_private *priv = tpm_get_vendordata(chip); tpm_chip_unregister(chip); ring_free(priv); - TPM_VPRIV(chip) = NULL; + tpm_set_vendordata(chip, NULL); return 0; }
Remove TPM_VPRIV macro and replace with tpm_set_vendordata and tpm_get_vendordata. Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com> --- drivers/char/tpm/st33zp24/st33zp24.c | 26 +++++++++++++------------- drivers/char/tpm/tpm.h | 12 ++++++++++-- drivers/char/tpm/tpm_ibmvtpm.c | 8 ++++---- drivers/char/tpm/tpm_tis.c | 2 +- drivers/char/tpm/xen-tpmfront.c | 14 +++++++------- 5 files changed, 35 insertions(+), 27 deletions(-)