| Message ID | 1455885728-10315-7-git-send-email-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 19, 2016 at 07:42:03AM -0500, Stefan Berger wrote: > - dev_set_drvdata(dev, chip); > + device_initialize(&chip->dev); Ah, there is the rebase mistake, this hunk needs to be shifted to the dev_set_name patch. Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Fri, Feb 19, 2016 at 07:42:03AM -0500, Stefan Berger wrote: > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > tpm_chip_alloc becomes a typical subsystem allocate call. Maybe a more verbose commit message? > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/char/tpm/tpm-chip.c | 57 +++++++++++++++++++++++++++++++-------------- > drivers/char/tpm/tpm.h | 4 +++- > 2 files changed, 42 insertions(+), 19 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index fe15637..2270e47 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -120,17 +120,17 @@ static void tpm_dev_release(struct device *dev) > } > > /** > - * tpmm_chip_alloc() - allocate a new struct tpm_chip instance > - * @dev: device to which the chip is associated > + * tpm_chip_alloc() - allocate a new struct tpm_chip instance > + * @pdev: device to which the chip is associated > + * At this point pdev mst be initialized, but does not have to > + * be registered > * @ops: struct tpm_class_ops instance > * > * Allocates a new struct tpm_chip instance and assigns a free > - * device number for it. Caller does not have to worry about > - * freeing the allocated resources. When the devices is removed > - * devres calls tpmm_chip_remove() to do the job. > + * device number for it. Must be paired with put_device(&chip->dev). > */ > -struct tpm_chip *tpmm_chip_alloc(struct device *dev, > - const struct tpm_class_ops *ops) > +struct tpm_chip *tpm_chip_alloc(struct device *dev, > + const struct tpm_class_ops *ops) > { > struct tpm_chip *chip; > int err; > @@ -143,10 +143,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, > mutex_init(&chip->tpm_mutex); > INIT_LIST_HEAD(&chip->list); > > - chip->ops = ops; > - > spin_lock(&driver_lock); > chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); > + if (chip->dev_num < TPM_NUM_DEVICES) > + set_bit(chip->dev_num, dev_mask); I guess this change should a separate patch. > spin_unlock(&driver_lock); > > if (chip->dev_num >= TPM_NUM_DEVICES) { > @@ -155,9 +155,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, > return ERR_PTR(-ENOMEM); > } > > - set_bit(chip->dev_num, dev_mask); > + chip->ops = ops; > > - dev_set_drvdata(dev, chip); > + device_initialize(&chip->dev); > > chip->dev.class = tpm_class; > chip->dev.release = tpm_dev_release; > @@ -175,23 +175,44 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, > if (err) > goto out; > > - device_initialize(&chip->dev); > - > cdev_init(&chip->cdev, &tpm_fops); > chip->cdev.owner = dev->driver->owner; > chip->cdev.kobj.parent = &chip->dev.kobj; > > - err = devm_add_action(dev, (void (*)(void *)) put_device, &chip->dev); > + return chip; > + > +out: > + put_device(&chip->dev); > + return ERR_PTR(err); > +} > +EXPORT_SYMBOL_GPL(tpm_chip_alloc); > + > +/** > + * tpmm_chip_alloc() - allocate a new struct tpm_chip instance > + * @pdev: parent device to which the chip is associated > + * @ops: struct tpm_class_ops instance > + * > + * Same as tpm_chip_alloc except devm is used to do the put_device > + */ > +struct tpm_chip *tpmm_chip_alloc(struct device *pdev, > + const struct tpm_class_ops *ops) > +{ > + struct tpm_chip *chip; > + int err; > + > + chip = tpm_chip_alloc(pdev, ops); > + if (IS_ERR(chip)) > + return chip; > + > + err = devm_add_action(pdev, (void (*)(void *)) put_device, &chip->dev); > if (err) { > put_device(&chip->dev); > return ERR_PTR(err); > } > > - return chip; > + dev_set_drvdata(pdev, chip); > > -out: > - put_device(&chip->dev); > - return ERR_PTR(err); > + return chip; > } > EXPORT_SYMBOL_GPL(tpmm_chip_alloc); > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 2a8373e..25efe8f 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -515,7 +515,9 @@ struct tpm_chip *tpm_chip_find_get(int chip_num); > __must_check int tpm_try_get_ops(struct tpm_chip *chip); > void tpm_put_ops(struct tpm_chip *chip); > > -extern struct tpm_chip *tpmm_chip_alloc(struct device *dev, > +extern struct tpm_chip *tpm_chip_alloc(struct device *dev, > + const struct tpm_class_ops *ops); > +extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev, > const struct tpm_class_ops *ops); > extern int tpm_chip_register(struct tpm_chip *chip); > extern void tpm_chip_unregister(struct tpm_chip *chip); > -- > 2.4.3 > /Jarkko ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 11:14:14PM +0200, Jarkko Sakkinen wrote: > On Fri, Feb 19, 2016 at 07:42:03AM -0500, Stefan Berger wrote: > > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > tpm_chip_alloc becomes a typical subsystem allocate call. > > Maybe a more verbose commit message? What more do you want to say? > > spin_lock(&driver_lock); > > chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); > > + if (chip->dev_num < TPM_NUM_DEVICES) > > + set_bit(chip->dev_num, dev_mask); > > I guess this change should a separate patch. Hurm, Stefan? This was not in the patch I sent you? This hunk looks incomplete to me, and replaced by the IDR stuff anyhow. Drop it? You need to put your Signed-off-by on patches you make notable changes to, this one would qualify Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 05:13:28 PM: > > On Mon, Feb 22, 2016 at 11:14:14PM +0200, Jarkko Sakkinen wrote: > > On Fri, Feb 19, 2016 at 07:42:03AM -0500, Stefan Berger wrote: > > > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > > > tpm_chip_alloc becomes a typical subsystem allocate call. > > > > Maybe a more verbose commit message? > > What more do you want to say? > > > > spin_lock(&driver_lock); > > > chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); > > > + if (chip->dev_num < TPM_NUM_DEVICES) > > > + set_bit(chip->dev_num, dev_mask); > > > > I guess this change should a separate patch. > > Hurm, Stefan? This was not in the patch I sent you? This hunk looks > incomplete to me, and replaced by the IDR stuff anyhow. Drop it? Dropped. Stefan ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 03:13:28PM -0700, Jason Gunthorpe wrote: > On Mon, Feb 22, 2016 at 11:14:14PM +0200, Jarkko Sakkinen wrote: > > On Fri, Feb 19, 2016 at 07:42:03AM -0500, Stefan Berger wrote: > > > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > > > tpm_chip_alloc becomes a typical subsystem allocate call. > > > > Maybe a more verbose commit message? > > What more do you want to say? I would just add that: "This is needed for virtual devices because they are not associated to any parent." Of course when you read the patch set in reverse order it is clear but this one sentence would make it easier to read it when you iterate it from beginning :) /Jarkko > > > spin_lock(&driver_lock); > > > chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); > > > + if (chip->dev_num < TPM_NUM_DEVICES) > > > + set_bit(chip->dev_num, dev_mask); > > > > I guess this change should a separate patch. > > Hurm, Stefan? This was not in the patch I sent you? This hunk looks > incomplete to me, and replaced by the IDR stuff anyhow. Drop it? > > You need to put your Signed-off-by on patches you make notable changes > to, this one would qualify > > Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index fe15637..2270e47 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -120,17 +120,17 @@ static void tpm_dev_release(struct device *dev) } /** - * tpmm_chip_alloc() - allocate a new struct tpm_chip instance - * @dev: device to which the chip is associated + * tpm_chip_alloc() - allocate a new struct tpm_chip instance + * @pdev: device to which the chip is associated + * At this point pdev mst be initialized, but does not have to + * be registered * @ops: struct tpm_class_ops instance * * Allocates a new struct tpm_chip instance and assigns a free - * device number for it. Caller does not have to worry about - * freeing the allocated resources. When the devices is removed - * devres calls tpmm_chip_remove() to do the job. + * device number for it. Must be paired with put_device(&chip->dev). */ -struct tpm_chip *tpmm_chip_alloc(struct device *dev, - const struct tpm_class_ops *ops) +struct tpm_chip *tpm_chip_alloc(struct device *dev, + const struct tpm_class_ops *ops) { struct tpm_chip *chip; int err; @@ -143,10 +143,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, mutex_init(&chip->tpm_mutex); INIT_LIST_HEAD(&chip->list); - chip->ops = ops; - spin_lock(&driver_lock); chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES); + if (chip->dev_num < TPM_NUM_DEVICES) + set_bit(chip->dev_num, dev_mask); spin_unlock(&driver_lock); if (chip->dev_num >= TPM_NUM_DEVICES) { @@ -155,9 +155,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, return ERR_PTR(-ENOMEM); } - set_bit(chip->dev_num, dev_mask); + chip->ops = ops; - dev_set_drvdata(dev, chip); + device_initialize(&chip->dev); chip->dev.class = tpm_class; chip->dev.release = tpm_dev_release; @@ -175,23 +175,44 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, if (err) goto out; - device_initialize(&chip->dev); - cdev_init(&chip->cdev, &tpm_fops); chip->cdev.owner = dev->driver->owner; chip->cdev.kobj.parent = &chip->dev.kobj; - err = devm_add_action(dev, (void (*)(void *)) put_device, &chip->dev); + return chip; + +out: + put_device(&chip->dev); + return ERR_PTR(err); +} +EXPORT_SYMBOL_GPL(tpm_chip_alloc); + +/** + * tpmm_chip_alloc() - allocate a new struct tpm_chip instance + * @pdev: parent device to which the chip is associated + * @ops: struct tpm_class_ops instance + * + * Same as tpm_chip_alloc except devm is used to do the put_device + */ +struct tpm_chip *tpmm_chip_alloc(struct device *pdev, + const struct tpm_class_ops *ops) +{ + struct tpm_chip *chip; + int err; + + chip = tpm_chip_alloc(pdev, ops); + if (IS_ERR(chip)) + return chip; + + err = devm_add_action(pdev, (void (*)(void *)) put_device, &chip->dev); if (err) { put_device(&chip->dev); return ERR_PTR(err); } - return chip; + dev_set_drvdata(pdev, chip); -out: - put_device(&chip->dev); - return ERR_PTR(err); + return chip; } EXPORT_SYMBOL_GPL(tpmm_chip_alloc); diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 2a8373e..25efe8f 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -515,7 +515,9 @@ struct tpm_chip *tpm_chip_find_get(int chip_num); __must_check int tpm_try_get_ops(struct tpm_chip *chip); void tpm_put_ops(struct tpm_chip *chip); -extern struct tpm_chip *tpmm_chip_alloc(struct device *dev, +extern struct tpm_chip *tpm_chip_alloc(struct device *dev, + const struct tpm_class_ops *ops); +extern struct tpm_chip *tpmm_chip_alloc(struct device *pdev, const struct tpm_class_ops *ops); extern int tpm_chip_register(struct tpm_chip *chip); extern void tpm_chip_unregister(struct tpm_chip *chip);