* [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
@ 2013-07-10 4:10 Shuah Khan
2013-07-10 22:51 ` [tpmdd-devel] " Peter Hüwe
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2013-07-10 4:10 UTC (permalink / raw)
To: rafael.j.wysocki, bhelgaas, a.zummo, key, mail, tpmdd, tpmdd,
matthew.garrett
Cc: Shuah Khan, linux-kernel, rtc-linux, tpmdd-devel,
platform-driver-x86, shuahkhan
Convert drivers/char/tpm/tpm_tis.c to use dev_pm_ops instead of legacy pm_ops.
This patch depends on pnp driver bus ops change to invoke pnp_driver
dev_pm_ops.
Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
drivers/char/tpm/tpm_tis.c | 60 ++++++++++++++++++--------------------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 4519cb3..5796d01 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -766,6 +766,25 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
}
#endif
+#ifdef CONFIG_PM_SLEEP
+static int tpm_tis_resume(struct device *dev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ int ret;
+
+ if (chip->vendor.irq)
+ tpm_tis_reenable_interrupts(chip);
+
+ ret = tpm_pm_resume(dev);
+ if (!ret)
+ tpm_do_selftest(chip);
+
+ return ret;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
+
#ifdef CONFIG_PNP
static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
const struct pnp_device_id *pnp_id)
@@ -787,26 +806,6 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
return tpm_tis_init(&pnp_dev->dev, start, len, irq);
}
-static int tpm_tis_pnp_suspend(struct pnp_dev *dev, pm_message_t msg)
-{
- return tpm_pm_suspend(&dev->dev);
-}
-
-static int tpm_tis_pnp_resume(struct pnp_dev *dev)
-{
- struct tpm_chip *chip = pnp_get_drvdata(dev);
- int ret;
-
- if (chip->vendor.irq)
- tpm_tis_reenable_interrupts(chip);
-
- ret = tpm_pm_resume(&dev->dev);
- if (!ret)
- tpm_do_selftest(chip);
-
- return ret;
-}
-
static struct pnp_device_id tpm_pnp_tbl[] = {
{"PNP0C31", 0}, /* TPM */
{"ATM1200", 0}, /* Atmel */
@@ -835,9 +834,12 @@ static struct pnp_driver tis_pnp_driver = {
.name = "tpm_tis",
.id_table = tpm_pnp_tbl,
.probe = tpm_tis_pnp_init,
- .suspend = tpm_tis_pnp_suspend,
- .resume = tpm_tis_pnp_resume,
.remove = tpm_tis_pnp_remove,
+#ifdef CONFIG_PM_SLEEP
+ .driver = {
+ .pm = &tpm_tis_pm,
+ },
+#endif
};
#define TIS_HID_USR_IDX sizeof(tpm_pnp_tbl)/sizeof(struct pnp_device_id) -2
@@ -846,20 +848,6 @@ module_param_string(hid, tpm_pnp_tbl[TIS_HID_USR_IDX].id,
MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
#endif
-#ifdef CONFIG_PM_SLEEP
-static int tpm_tis_resume(struct device *dev)
-{
- struct tpm_chip *chip = dev_get_drvdata(dev);
-
- if (chip->vendor.irq)
- tpm_tis_reenable_interrupts(chip);
-
- return tpm_pm_resume(dev);
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume);
-
static struct platform_driver tis_drv = {
.driver = {
.name = "tpm_tis",
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
2013-07-10 4:10 [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops Shuah Khan
@ 2013-07-10 22:51 ` Peter Hüwe
2013-07-10 23:02 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Peter Hüwe @ 2013-07-10 22:51 UTC (permalink / raw)
To: tpmdd-devel
Cc: Shuah Khan, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan
Hi,
thanks for your patch
> static struct pnp_device_id tpm_pnp_tbl[] = {
> {"PNP0C31", 0}, /* TPM */
> {"ATM1200", 0}, /* Atmel */
> @@ -835,9 +834,12 @@ static struct pnp_driver tis_pnp_driver = {
> .name = "tpm_tis",
> .id_table = tpm_pnp_tbl,
> .probe = tpm_tis_pnp_init,
> - .suspend = tpm_tis_pnp_suspend,
> - .resume = tpm_tis_pnp_resume,
> .remove = tpm_tis_pnp_remove,
> +#ifdef CONFIG_PM_SLEEP
> + .driver = {
> + .pm = &tpm_tis_pm,
> + },
> +#endif
> };
I don't think the #if CONFIG_PM_SLEEP is required here.
Thanks,
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
2013-07-10 22:51 ` [tpmdd-devel] " Peter Hüwe
@ 2013-07-10 23:02 ` Shuah Khan
0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2013-07-10 23:02 UTC (permalink / raw)
To: Peter Hüwe
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Shuah Khan
On 07/10/2013 04:43 PM, Peter Hüwe wrote:
> Hi,
>
> thanks for your patch
>> static struct pnp_device_id tpm_pnp_tbl[] = {
>> {"PNP0C31", 0}, /* TPM */
>> {"ATM1200", 0}, /* Atmel */
>> @@ -835,9 +834,12 @@ static struct pnp_driver tis_pnp_driver = {
>> .name = "tpm_tis",
>> .id_table = tpm_pnp_tbl,
>> .probe = tpm_tis_pnp_init,
>> - .suspend = tpm_tis_pnp_suspend,
>> - .resume = tpm_tis_pnp_resume,
>> .remove = tpm_tis_pnp_remove,
>> +#ifdef CONFIG_PM_SLEEP
>> + .driver = {
>> + .pm = &tpm_tis_pm,
>> + },
>> +#endif
>> };
>
>
> I don't think the #if CONFIG_PM_SLEEP is required here.
>
> Thanks,
> Peter
>
tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
and remove this CONFIG_PM_SLEEP when defining .pm. That does make sense
looking at tpm_pm_suspend() and tpm_pm_resume() which are defined
without CONFIG_PM_SLEEP scope. Sounds like the right approach? I will
redo the patch and send v2.
I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
are not very consistent. :)
-- Shuah
Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
@ 2013-07-10 23:02 ` Shuah Khan
0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2013-07-10 23:02 UTC (permalink / raw)
To: Peter Hüwe
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Shuah Khan
On 07/10/2013 04:43 PM, Peter Hüwe wrote:
> Hi,
>
> thanks for your patch
>> static struct pnp_device_id tpm_pnp_tbl[] = {
>> {"PNP0C31", 0}, /* TPM */
>> {"ATM1200", 0}, /* Atmel */
>> @@ -835,9 +834,12 @@ static struct pnp_driver tis_pnp_driver = {
>> .name = "tpm_tis",
>> .id_table = tpm_pnp_tbl,
>> .probe = tpm_tis_pnp_init,
>> - .suspend = tpm_tis_pnp_suspend,
>> - .resume = tpm_tis_pnp_resume,
>> .remove = tpm_tis_pnp_remove,
>> +#ifdef CONFIG_PM_SLEEP
>> + .driver = {
>> + .pm = &tpm_tis_pm,
>> + },
>> +#endif
>> };
>
>
> I don't think the #if CONFIG_PM_SLEEP is required here.
>
> Thanks,
> Peter
>
tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
and remove this CONFIG_PM_SLEEP when defining .pm. That does make sense
looking at tpm_pm_suspend() and tpm_pm_resume() which are defined
without CONFIG_PM_SLEEP scope. Sounds like the right approach? I will
redo the patch and send v2.
I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
are not very consistent. :)
-- Shuah
Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
2013-07-10 23:02 ` Shuah Khan
@ 2013-07-10 23:37 ` Peter Hüwe
-1 siblings, 0 replies; 8+ messages in thread
From: Peter Hüwe @ 2013-07-10 23:37 UTC (permalink / raw)
To: Shuah Khan
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Rafael J. Wysocki
Hi Shuah,
thanks for your reply.
> >> +#ifdef CONFIG_PM_SLEEP
> >> + .driver = {
> >> + .pm = &tpm_tis_pm,
> >> + },
> >> +#endif
> >>
> >> };
> >
> > I don't think the #if CONFIG_PM_SLEEP is required here.
In this case, the SIMPLE_DEV_PM_OPS macro handles the case internally - i.e.
no matter whether CONFIG_PM_SLEEP is set or not, the correct structure is set
up and thus no ifdef needed.
>
> tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
> make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
> and remove this CONFIG_PM_SLEEP when defining .pm.
> That does make sense looking at tpm_pm_suspend() and tpm_pm_resume() which
> are defined ithout CONFIG_PM_SLEEP scope. Sounds like the right approach?
> I will redo the patch and send v2.
Hmm,
at first I thought that would be a good idea, however scrolling to the git
history I found:
commit 07368d32f1a67e797def08cf2ee3ea1647b204b6
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date: Thu Aug 9 23:00:35 2012 +0200
tpm_tis / PM: Fix unused function warning for CONFIG_PM_SLEEP
According to a compiler warning, the tpm_tis_resume() function is not
used for CONFIG_PM_SLEEP unset, so add a #ifdef to prevent it from
being built in that case.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
So removing it there would effectively revert the patch and re-enable the
warning.
> I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
> are not very consistent. :)
Yes.
Maybe the better idea is to add the correct CONFIG_PM ifdefs for all code
paths related to PM.
Or leave the CONFIG_PM for tpm_tis_resume as it is.
Thanks,
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
@ 2013-07-10 23:37 ` Peter Hüwe
0 siblings, 0 replies; 8+ messages in thread
From: Peter Hüwe @ 2013-07-10 23:37 UTC (permalink / raw)
To: Shuah Khan
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Rafael J. Wysocki
Hi Shuah,
thanks for your reply.
> >> +#ifdef CONFIG_PM_SLEEP
> >> + .driver = {
> >> + .pm = &tpm_tis_pm,
> >> + },
> >> +#endif
> >>
> >> };
> >
> > I don't think the #if CONFIG_PM_SLEEP is required here.
In this case, the SIMPLE_DEV_PM_OPS macro handles the case internally - i.e.
no matter whether CONFIG_PM_SLEEP is set or not, the correct structure is set
up and thus no ifdef needed.
>
> tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
> make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
> and remove this CONFIG_PM_SLEEP when defining .pm.
> That does make sense looking at tpm_pm_suspend() and tpm_pm_resume() which
> are defined ithout CONFIG_PM_SLEEP scope. Sounds like the right approach?
> I will redo the patch and send v2.
Hmm,
at first I thought that would be a good idea, however scrolling to the git
history I found:
commit 07368d32f1a67e797def08cf2ee3ea1647b204b6
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date: Thu Aug 9 23:00:35 2012 +0200
tpm_tis / PM: Fix unused function warning for CONFIG_PM_SLEEP
According to a compiler warning, the tpm_tis_resume() function is not
used for CONFIG_PM_SLEEP unset, so add a #ifdef to prevent it from
being built in that case.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
So removing it there would effectively revert the patch and re-enable the
warning.
> I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
> are not very consistent. :)
Yes.
Maybe the better idea is to add the correct CONFIG_PM ifdefs for all code
paths related to PM.
Or leave the CONFIG_PM for tpm_tis_resume as it is.
Thanks,
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
2013-07-10 23:37 ` Peter Hüwe
@ 2013-07-11 1:30 ` Shuah Khan
-1 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2013-07-11 1:30 UTC (permalink / raw)
To: Peter Hüwe
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Rafael J. Wysocki, Shuah Khan
Hi Peter,
On 07/10/2013 05:30 PM, Peter Hüwe wrote:
>>
>> tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
>> make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
>> and remove this CONFIG_PM_SLEEP when defining .pm.
>> That does make sense looking at tpm_pm_suspend() and tpm_pm_resume() which
>> are defined ithout CONFIG_PM_SLEEP scope. Sounds like the right approach?
>> I will redo the patch and send v2.
>
> Hmm,
> at first I thought that would be a good idea, however scrolling to the git
> history I found:
>
> commit 07368d32f1a67e797def08cf2ee3ea1647b204b6
> Author: Rafael J. Wysocki <rjw@sisk.pl>
> Date: Thu Aug 9 23:00:35 2012 +0200
>
> tpm_tis / PM: Fix unused function warning for CONFIG_PM_SLEEP
>
> According to a compiler warning, the tpm_tis_resume() function is not
> used for CONFIG_PM_SLEEP unset, so add a #ifdef to prevent it from
> being built in that case.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> So removing it there would effectively revert the patch and re-enable the
> warning.
>
>
>
>> I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
>> are not very consistent. :)
> Yes.
>
> Maybe the better idea is to add the correct CONFIG_PM ifdefs for all code
> paths related to PM.
> Or leave the CONFIG_PM for tpm_tis_resume as it is.
>
>
For now, leaving tpm_tis_resume() is better to keep this change simpler.
I am seeing this type of inconsistency in several drivers as I am going
around making changes to convert from legacy pm_ops to dev_pm_ops. At
some point, it might be worth while looking at the usage of these
defines and set some clear guidelines.
-- Shuah
Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tpmdd-devel] [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops
@ 2013-07-11 1:30 ` Shuah Khan
0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2013-07-11 1:30 UTC (permalink / raw)
To: Peter Hüwe
Cc: tpmdd-devel, rafael.j.wysocki, bhelgaas, a.zummo, key, mail,
tpmdd, tpmdd, matthew.garrett, rtc-linux, linux-kernel,
platform-driver-x86, shuahkhan, Rafael J. Wysocki, Shuah Khan
Hi Peter,
On 07/10/2013 05:30 PM, Peter Hüwe wrote:
>>
>> tpm_tis_resume() is defined originally in CONFIG_PM_SLEEP scope. I can
>> make the change to have tpm_tis_resume() not be in CONFIG_PM_SLEEP scope
>> and remove this CONFIG_PM_SLEEP when defining .pm.
>> That does make sense looking at tpm_pm_suspend() and tpm_pm_resume() which
>> are defined ithout CONFIG_PM_SLEEP scope. Sounds like the right approach?
>> I will redo the patch and send v2.
>
> Hmm,
> at first I thought that would be a good idea, however scrolling to the git
> history I found:
>
> commit 07368d32f1a67e797def08cf2ee3ea1647b204b6
> Author: Rafael J. Wysocki <rjw@sisk.pl>
> Date: Thu Aug 9 23:00:35 2012 +0200
>
> tpm_tis / PM: Fix unused function warning for CONFIG_PM_SLEEP
>
> According to a compiler warning, the tpm_tis_resume() function is not
> used for CONFIG_PM_SLEEP unset, so add a #ifdef to prevent it from
> being built in that case.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> So removing it there would effectively revert the patch and re-enable the
> warning.
>
>
>
>> I find that the use of CONFIG_PM, CONFIG_PM_SLEEP, and CONFIG_PM_RUNTIME
>> are not very consistent. :)
> Yes.
>
> Maybe the better idea is to add the correct CONFIG_PM ifdefs for all code
> paths related to PM.
> Or leave the CONFIG_PM for tpm_tis_resume as it is.
>
>
For now, leaving tpm_tis_resume() is better to keep this change simpler.
I am seeing this type of inconsistency in several drivers as I am going
around making changes to convert from legacy pm_ops to dev_pm_ops. At
some point, it might be worth while looking at the usage of these
defines and set some clear guidelines.
-- Shuah
Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research
America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-07-11 1:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-10 4:10 [PATCH 3/4] tpm: Convert tpm_tis driver to use dev_pm_ops from legacy pm_ops Shuah Khan
2013-07-10 22:51 ` [tpmdd-devel] " Peter Hüwe
2013-07-10 23:02 ` Shuah Khan
2013-07-10 23:02 ` Shuah Khan
2013-07-10 23:37 ` Peter Hüwe
2013-07-10 23:37 ` Peter Hüwe
2013-07-11 1:30 ` Shuah Khan
2013-07-11 1:30 ` Shuah Khan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.