Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1281118 > unrolled thread

[PATCH v2 0/3] tpm_tis: Clean up force module parameter

Started byJason Gunthorpe <jgunthorpe@obsidianresearch.com>
First post2015-12-01 20:00 +0100
Last post2015-12-02 19:20 +0100
Articles 20 on this page of 47 — 7 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:00 +0100
    [PATCH v2 3/3] tpm_tis: Clean up the force=1 module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:00 +0100
      Re: [PATCH v2 3/3] tpm_tis: Clean up the force=1 module parameter Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-01 20:40 +0100
        Re: [PATCH v2 3/3] tpm_tis: Clean up the force=1 module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 21:00 +0100
    [PATCH v2 1/3] tpm_tis: Disable interrupt auto probing on a per-device basis Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:00 +0100
      Re: [PATCH v2 1/3] tpm_tis: Disable interrupt auto probing on a  per-device basis Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-01 20:20 +0100
        Re: [PATCH v2 1/3] tpm_tis: Disable interrupt auto probing on a  per-device basis Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:40 +0100
    [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:00 +0100
      Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-01 20:30 +0100
        Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 20:50 +0100
          Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-01 21:00 +0100
            Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-01 22:00 +0100
    Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-01 22:20 +0100
    Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-01 22:40 +0100
      Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-02 00:10 +0100
        Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Peter Huewe <peterhuewe@gmx.de> - 2015-12-02 02:20 +0100
          Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-02 09:20 +0100
            Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Peter Huewe <peterhuewe@gmx.de> - 2015-12-02 10:20 +0100
        Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-02 09:20 +0100
          Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-02 09:30 +0100
            Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-12-02 18:00 +0100
              Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-02 18:10 +0100
              [PATCH v3] base/platform: fix binding for drivers without probe callback martin.wilck@ts.fujitsu.com - 2015-12-03 10:00 +0100
                Re: [PATCH v3] base/platform: fix binding for drivers without probe  callback Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2015-12-03 10:10 +0100
                Re: [tpmdd-devel] [PATCH v3] base/platform: fix binding for drivers  without probe callback Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-03 10:40 +0100
      Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-02 19:30 +0100
        Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-02 20:20 +0100
          Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-03 07:10 +0100
            Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-03 19:20 +0100
              Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-06 05:10 +0100
                Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-06 05:20 +0100
                  Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-06 05:30 +0100
                  Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-07 07:20 +0100
                  Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-07 09:10 +0100
                    Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-07 10:00 +0100
                      Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-07 11:00 +0100
                        Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-07 11:20 +0100
          Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-03 09:40 +0100
            Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-03 18:10 +0100
              Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-04 09:40 +0100
              Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-04 10:20 +0100
                Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-04 19:10 +0100
                  Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-07 11:00 +0100
                    Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module  parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-07 18:40 +0100
        Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-12-03 07:00 +0100
    Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter "Wilck, Martin" <martin.wilck@ts.fujitsu.com> - 2015-12-02 13:40 +0100
      Re: [PATCH v2 0/3] tpm_tis: Clean up force module parameter Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2015-12-02 19:20 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1282015

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2015-12-02 18:00 +0100
Message-ID<qBiV4-1mH-15@gated-at.bofh.it>
In reply to#1281520
On Wed, Dec 02, 2015 at 09:21:47AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> Cc += gregkh
> 
> On Wed, Dec 02, 2015 at 10:11:14AM +0200, Jarkko Sakkinen wrote:
> > On Tue, Dec 01, 2015 at 03:22:23PM -0700, Jason Gunthorpe wrote:
> > > On Tue, Dec 01, 2015 at 11:33:51PM +0200, Jarkko Sakkinen wrote:
> > > 
> > > > I went through the patches and didn't see anything that would shock me
> > > > enough not to apply the patches in the current if they also work when
> > > > tested *but* are these release critical for Linux v4.4?
> > > > 
> > > > I got a bit confused about the discussion that was going on about "where
> > > > to fix the probe" crash whether or not both it should be fixed in both
> > > > places.
> > > 
> > > I'm also confused by that..
> > > 
> > > It sounds like force=1 is broken in 4.4 right now - do we care? Should
> > > we fix this by using Martin's patch?
> > > 
> > > These changes are complex enough they really shouldn't go into 4.4
> > > unless absolutely necessary.
> > 
> > The reasons I'm asking this are:
> > 
> > * I'm planning to do v4.5 pull request soon.
> > * If this need to be get this into v4.4, we should act fast. Given the
> >   complexity of the changes I'd not recommend that unless it is a life
> >   and death question.
> 
> I'd say we should repair b8b2c7d845d5 ("base/platform: assert that
> dev_pm_domain callbacks are called unconditionally") for 4.4-rc$next and
> live with the problem that the tpm driver had since long another
> release.

I was going to queue up
	Subject: [PATCH] base/platform: fix panic when probe function is NULL

for 4.4-final, unless you all object to that.

thanks,

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

[toc] | [prev] | [next] | [standalone]


#1282060

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2015-12-02 18:10 +0100
Message-ID<qBj4N-1FO-91@gated-at.bofh.it>
In reply to#1282015
Hello Greg,

On Wed, Dec 02, 2015 at 08:53:38AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:21:47AM +0100, Uwe Kleine-König wrote:
> > > > These changes are complex enough they really shouldn't go into 4.4
> > > > unless absolutely necessary.
> > > 
> > > The reasons I'm asking this are:
> > > 
> > > * I'm planning to do v4.5 pull request soon.
> > > * If this need to be get this into v4.4, we should act fast. Given the
> > >   complexity of the changes I'd not recommend that unless it is a life
> > >   and death question.
> > 
> > I'd say we should repair b8b2c7d845d5 ("base/platform: assert that
> > dev_pm_domain callbacks are called unconditionally") for 4.4-rc$next and
> > live with the problem that the tpm driver had since long another
> > release.
> 
> I was going to queue up
> 	Subject: [PATCH] base/platform: fix panic when probe function is NULL
> 
> for 4.4-final, unless you all object to that.

Martin is about to send a v3 of this patch, please pick up this v3
instead.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1282813 — [PATCH v3] base/platform: fix binding for drivers without probe callback

Frommartin.wilck@ts.fujitsu.com
Date2015-12-03 10:00 +0100
Subject[PATCH v3] base/platform: fix binding for drivers without probe callback
Message-ID<qBxU6-2Ek-19@gated-at.bofh.it>
In reply to#1282015
From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>

Since b8b2c7d845d5, platform_drv_probe() is called for all platform
devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
platform_drv_probe() will return the error code from dev_pm_domain_attach().

This causes real_probe() to enter the "probe_failed" path and set
dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
success if both dev->bus->probe and drv->probe were missing. As a result,
a device and driver could be "bound" together just by matching their names;
this doesn't work any more after b8b2c7d845d5.

This change broke the assumptions of certain drivers; for example, the TPM
code has long assumed that platform driver and device with matching name
could be bound in this way. That assumption may cause such drivers to
fail with Oops during initialization after applying this change. Failure
in suspend/resume tests under qemu has also been reported.

This patch restores the previous (4.3.0 and earlier) behavior of
platform_drv_probe() in the case when the associated platform driver has
no "probe" function.

Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain callbacks are called unconditionally")
Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
---
 v2: fixed style issues, rephrased commit message.
 v3: rephrased commit message and subject again.

 drivers/base/platform.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 1dd6d3b..176b59f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -513,10 +513,15 @@ static int platform_drv_probe(struct device *_dev)
 		return ret;
 
 	ret = dev_pm_domain_attach(_dev, true);
-	if (ret != -EPROBE_DEFER && drv->probe) {
-		ret = drv->probe(dev);
-		if (ret)
-			dev_pm_domain_detach(_dev, true);
+	if (ret != -EPROBE_DEFER) {
+		if (drv->probe) {
+			ret = drv->probe(dev);
+			if (ret)
+				dev_pm_domain_detach(_dev, true);
+		} else {
+			/* don't fail if just dev_pm_domain_attach failed */
+			ret = 0;
+		}
 	}
 
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
-- 
1.8.3.1

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

[toc] | [prev] | [next] | [standalone]


#1282818 — Re: [PATCH v3] base/platform: fix binding for drivers without probe callback

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2015-12-03 10:10 +0100
SubjectRe: [PATCH v3] base/platform: fix binding for drivers without probe callback
Message-ID<qBy3L-2WY-9@gated-at.bofh.it>
In reply to#1282813
Hello Martin,

On Thu, Dec 03, 2015 at 09:51:44AM +0100, martin.wilck@ts.fujitsu.com wrote:
> From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> platform_drv_probe() will return the error code from dev_pm_domain_attach().
> 
> This causes real_probe() to enter the "probe_failed" path and set
> dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> success if both dev->bus->probe and drv->probe were missing. As a result,
> a device and driver could be "bound" together just by matching their names;
> this doesn't work any more after b8b2c7d845d5.
> 
> This change broke the assumptions of certain drivers; for example, the TPM
> code has long assumed that platform driver and device with matching name
> could be bound in this way. That assumption may cause such drivers to
> fail with Oops during initialization after applying this change. Failure
> in suspend/resume tests under qemu has also been reported.
> 
> This patch restores the previous (4.3.0 and earlier) behavior of
> platform_drv_probe() in the case when the associated platform driver has
> no "probe" function.
> 
> Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain callbacks are called unconditionally")
> Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1282839 — Re: [tpmdd-devel] [PATCH v3] base/platform: fix binding for drivers without probe callback

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-03 10:40 +0100
SubjectRe: [tpmdd-devel] [PATCH v3] base/platform: fix binding for drivers without probe callback
Message-ID<qBywN-38I-1@gated-at.bofh.it>
In reply to#1282813
On Thu, Dec 03, 2015 at 09:51:44AM +0100, martin.wilck@ts.fujitsu.com wrote:
> From: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> 
> Since b8b2c7d845d5, platform_drv_probe() is called for all platform
> devices. If drv->probe is NULL, and dev_pm_domain_attach() fails,
> platform_drv_probe() will return the error code from dev_pm_domain_attach().
> 
> This causes real_probe() to enter the "probe_failed" path and set
> dev->driver to NULL. Before b8b2c7d845d5, real_probe() would assume
> success if both dev->bus->probe and drv->probe were missing. As a result,
> a device and driver could be "bound" together just by matching their names;
> this doesn't work any more after b8b2c7d845d5.
> 
> This change broke the assumptions of certain drivers; for example, the TPM
> code has long assumed that platform driver and device with matching name
> could be bound in this way. That assumption may cause such drivers to
> fail with Oops during initialization after applying this change. Failure
> in suspend/resume tests under qemu has also been reported.
> 
> This patch restores the previous (4.3.0 and earlier) behavior of
> platform_drv_probe() in the case when the associated platform driver has
> no "probe" function.
> 
> Fixes: b8b2c7d845d5 ("base/platform: assert that dev_pm_domain callbacks are called unconditionally")
> Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com>
> ---
>  v2: fixed style issues, rephrased commit message.
>  v3: rephrased commit message and subject again.
> 
>  drivers/base/platform.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 1dd6d3b..176b59f 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -513,10 +513,15 @@ static int platform_drv_probe(struct device *_dev)
>  		return ret;
>  
>  	ret = dev_pm_domain_attach(_dev, true);
> -	if (ret != -EPROBE_DEFER && drv->probe) {
> -		ret = drv->probe(dev);
> -		if (ret)
> -			dev_pm_domain_detach(_dev, true);
> +	if (ret != -EPROBE_DEFER) {
> +		if (drv->probe) {
> +			ret = drv->probe(dev);
> +			if (ret)
> +				dev_pm_domain_detach(_dev, true);
> +		} else {
> +			/* don't fail if just dev_pm_domain_attach failed */
> +			ret = 0;
> +		}
>  	}
>  
>  	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> -- 
> 1.8.3.1

Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

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

[toc] | [prev] | [next] | [standalone]


#1282255

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2015-12-02 19:30 +0100
Message-ID<qBkka-2qa-23@gated-at.bofh.it>
In reply to#1281233
On Tue, Dec 01, 2015 at 11:33:51PM +0200, Jarkko Sakkinen wrote:
> On Tue, Dec 01, 2015 at 11:58:26AM -0700, Jason Gunthorpe wrote:

> I went through the patches and didn't see anything that would shock me
> enough not to apply the patches in the current if they also work when
> tested *but* are these release critical for Linux v4.4?

Jarkko,

Can you explain how

commit 399235dc6e95400a1322a9999e92073bc572f0c8
Author: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date:   Tue Sep 29 00:32:19 2015 +0300

    tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0

Is supposed to work? I get the jist of the idea, but I'm not seeing
how it can work reliably..

The idea is to pass off TPM2_START_FIFO to tpm_tis?

I'm guessing that if the driver probe order is tpm_crb,tpm_tis then
things work because tpm_crb will claim the device first? Otherwise
tpm_tis claims these things unconditionally? If the probe order is
reversed things become broken?

What is the address tpm_tis should be using? I see two things, it
either uses the x86 default address or it expects the ACPI to have a
MEM resource. AFAIK ACPI should never rely on hard wired addresses, so
I removed that code in this series. Perhaps tpm_tis should be using
control_area_pa ? Will ACPI ever present a struct resource? (if yes,
why isn't tpm_crb using one?)

There is also something wrong with the endianness in the acpi
stuff. I don't see endianness conversions in other acpi places, so I
wonder if the ones in tpm_crb are correct. If they are correct then
the struct needs le/be notations and there are some missing
conversions.

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

[toc] | [prev] | [next] | [standalone]


#1282286 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2015-12-02 20:20 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBl6y-2Xu-15@gated-at.bofh.it>
In reply to#1282255
On Wed, Dec 02, 2015 at 11:27:27AM -0700, Jason Gunthorpe wrote:

> I'm guessing that if the driver probe order is tpm_crb,tpm_tis then
> things work because tpm_crb will claim the device first? Otherwise
> tpm_tis claims these things unconditionally? If the probe order is
> reversed things become broken?

Okay, I didn't find the is_fifo before, so that make sense

But this:

> What is the address tpm_tis should be using? I see two things, it
> either uses the x86 default address or it expects the ACPI to have a
> MEM resource. AFAIK ACPI should never rely on hard wired addresses, so
> I removed that code in this series. Perhaps tpm_tis should be using
> control_area_pa ? Will ACPI ever present a struct resource? (if yes,
> why isn't tpm_crb using one?)

Is then still a problem. On Martin's system the MSFT0101 device does
not have a struct resource attached to it. Does any system, or is this
just dead code?

Should the control_area_pa be used?

Martin: could you try this (along with the other hunk to prevent the
oops):

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 8a3509cb10da..6824a00ba513 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -138,6 +138,8 @@ static inline int is_fifo(struct acpi_device *dev)
 	if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO)
 		return 0;
 
+	dev_err(&dev->dev, "control area pa is %x\n", tbl->control_area_pa);
+
 	/* TPM 2.0 FIFO */
 	return 1;
 }

Hoping to see it print 0xFED40000

> There is also something wrong with the endianness in the acpi
> stuff. I don't see endianness conversions in other acpi places, so I
> wonder if the ones in tpm_crb are correct. If they are correct then
> the struct needs le/be notations and there are some missing
> conversions.

I've made a patch to take care of this and move every thing to the
include/acpi/actbl2.h definitions, which is why I didn't notice
is_fifo in the first place...

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

[toc] | [prev] | [next] | [standalone]


#1282723 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-03 07:10 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBvfz-19g-1@gated-at.bofh.it>
In reply to#1282286
On Wed, Dec 02, 2015 at 12:11:55PM -0700, Jason Gunthorpe wrote:
> On Wed, Dec 02, 2015 at 11:27:27AM -0700, Jason Gunthorpe wrote:
> 
> > I'm guessing that if the driver probe order is tpm_crb,tpm_tis then
> > things work because tpm_crb will claim the device first? Otherwise
> > tpm_tis claims these things unconditionally? If the probe order is
> > reversed things become broken?
> 
> Okay, I didn't find the is_fifo before, so that make sense
> 
> But this:
> 
> > What is the address tpm_tis should be using? I see two things, it
> > either uses the x86 default address or it expects the ACPI to have a
> > MEM resource. AFAIK ACPI should never rely on hard wired addresses, so
> > I removed that code in this series. Perhaps tpm_tis should be using
> > control_area_pa ? Will ACPI ever present a struct resource? (if yes,
> > why isn't tpm_crb using one?)
> 
> Is then still a problem. On Martin's system the MSFT0101 device does
> not have a struct resource attached to it. Does any system, or is this
> just dead code?
> 
> Should the control_area_pa be used?

I guess it'd be more realiable. In my NUC the current fix works and the
people who tested it. If you supply me a fix that changes it to use that
I can test it and this will give also coverage to the people who tested
my original fix.

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

[toc] | [prev] | [next] | [standalone]


#1283235 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2015-12-03 19:20 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBGE2-cG-5@gated-at.bofh.it>
In reply to#1282723
On Thu, Dec 03, 2015 at 08:00:42AM +0200, Jarkko Sakkinen wrote:

> I guess it'd be more realiable. In my NUC the current fix works and the
> people who tested it. If you supply me a fix that changes it to use that
> I can test it and this will give also coverage to the people who tested
> my original fix.

Here is the updated series:

https://github.com/jgunthorpe/linux/commits/for-jarkko

What does your dmesg say?

It really isn't OK to hardwire an address for acpi devices, so I've
added something like this. Just completely guessing that control_pa is
where the BIOS is hiding the base address. Maybe it is cca->cmd_pa ?

From c9f7c0465008657f7fc7880496f68f4a1b3b4a26 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Thu, 3 Dec 2015 10:58:56 -0700
Subject: [PATCH 3/5] tpm_tis: Do not fall back to a hardcoded address for TPM2

If the ACPI tables do not declare a memory resource for the TPM2
then do not just fall back to the x86 default base address.

WIP: Guess that the control_address is the base address for the
TIS 1.2 memory mapped interface.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm_tis.c | 50 +++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index fecd27b45fd1..6b28f8003425 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
 {
 	return has_hid(dev, "INTC0102");
 }
-
-static inline int is_fifo(struct acpi_device *dev)
-{
-	struct acpi_table_tpm2 *tbl;
-	acpi_status st;
-
-	/* TPM 1.2 FIFO */
-	if (!has_hid(dev, "MSFT0101"))
-		return 1;
-
-	st = acpi_get_table(ACPI_SIG_TPM2, 1,
-			    (struct acpi_table_header **) &tbl);
-	if (ACPI_FAILURE(st)) {
-		dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
-		return 0;
-	}
-
-	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
-		return 0;
-
-	/* TPM 2.0 FIFO */
-	return 1;
-}
 #else
 static inline int is_itpm(struct acpi_device *dev)
 {
 	return 0;
 }
-
-static inline int is_fifo(struct acpi_device *dev)
-{
-	return 1;
-}
 #endif
 
 /* Before we attempt to access the TPM we must see that the valid bit is set.
@@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
 
 static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
 {
+	struct acpi_table_tpm2 *tbl;
+	acpi_status st;
 	struct list_head resources;
-	struct tpm_info tpm_info = tis_default_info;
+	struct tpm_info tpm_info = {};
 	int ret;
 
-	if (!is_fifo(acpi_dev))
+	st = acpi_get_table(ACPI_SIG_TPM2, 1,
+			    (struct acpi_table_header **) &tbl);
+	if (ACPI_FAILURE(st)) {
+		dev_err(&acpi_dev->dev,
+			FW_BUG "failed to get TPM2 ACPI table\n");
+		return -ENODEV;
+	}
+
+	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
 		return -ENODEV;
 
 	INIT_LIST_HEAD(&resources);
@@ -996,6 +978,14 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
 
 	acpi_dev_free_resource_list(&resources);
 
+	if (tpm_info.start == 0 && tpm_info.len == 0) {
+		tpm_info.start = tbl->control_address;
+		tpm_info.len = TIS_MEM_LEN;
+		dev_err(&acpi_dev->dev,
+			FW_BUG "TPM2 ACPI table does not define a memory resource, using 0x%lx instead\n",
+			tpm_info.start);
+	}
+
 	if (is_itpm(acpi_dev))
 		itpm = true;
 
-- 
2.1.4

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

[toc] | [prev] | [next] | [standalone]


#1284773 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-06 05:10 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCyO6-1WG-25@gated-at.bofh.it>
In reply to#1283235
On Thu, Dec 03, 2015 at 11:19:32AM -0700, Jason Gunthorpe wrote:
> On Thu, Dec 03, 2015 at 08:00:42AM +0200, Jarkko Sakkinen wrote:
> 
> > I guess it'd be more realiable. In my NUC the current fix works and the
> > people who tested it. If you supply me a fix that changes it to use that
> > I can test it and this will give also coverage to the people who tested
> > my original fix.
> 
> Here is the updated series:
> 
> https://github.com/jgunthorpe/linux/commits/for-jarkko
> 
> What does your dmesg say?
> 
> It really isn't OK to hardwire an address for acpi devices, so I've
> added something like this. Just completely guessing that control_pa is
> where the BIOS is hiding the base address. Maybe it is cca->cmd_pa ?

I'm a bit confused about the discussion because Martin replied that
tpm_tis used to get the address range before applying this series.

And pnp_driver in the backend for TPM 1.x devices grabs the address
range from DSDT.

/Jarkko

> From c9f7c0465008657f7fc7880496f68f4a1b3b4a26 Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Date: Thu, 3 Dec 2015 10:58:56 -0700
> Subject: [PATCH 3/5] tpm_tis: Do not fall back to a hardcoded address for TPM2
> 
> If the ACPI tables do not declare a memory resource for the TPM2
> then do not just fall back to the x86 default base address.
> 
> WIP: Guess that the control_address is the base address for the
> TIS 1.2 memory mapped interface.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm_tis.c | 50 +++++++++++++++++++---------------------------
>  1 file changed, 20 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index fecd27b45fd1..6b28f8003425 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -122,39 +122,11 @@ static inline int is_itpm(struct acpi_device *dev)
>  {
>  	return has_hid(dev, "INTC0102");
>  }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> -	struct acpi_table_tpm2 *tbl;
> -	acpi_status st;
> -
> -	/* TPM 1.2 FIFO */
> -	if (!has_hid(dev, "MSFT0101"))
> -		return 1;
> -
> -	st = acpi_get_table(ACPI_SIG_TPM2, 1,
> -			    (struct acpi_table_header **) &tbl);
> -	if (ACPI_FAILURE(st)) {
> -		dev_err(&dev->dev, "failed to get TPM2 ACPI table\n");
> -		return 0;
> -	}
> -
> -	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> -		return 0;
> -
> -	/* TPM 2.0 FIFO */
> -	return 1;
> -}
>  #else
>  static inline int is_itpm(struct acpi_device *dev)
>  {
>  	return 0;
>  }
> -
> -static inline int is_fifo(struct acpi_device *dev)
> -{
> -	return 1;
> -}
>  #endif
>  
>  /* Before we attempt to access the TPM we must see that the valid bit is set.
> @@ -980,11 +952,21 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data)
>  
>  static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
>  {
> +	struct acpi_table_tpm2 *tbl;
> +	acpi_status st;
>  	struct list_head resources;
> -	struct tpm_info tpm_info = tis_default_info;
> +	struct tpm_info tpm_info = {};
>  	int ret;
>  
> -	if (!is_fifo(acpi_dev))
> +	st = acpi_get_table(ACPI_SIG_TPM2, 1,
> +			    (struct acpi_table_header **) &tbl);
> +	if (ACPI_FAILURE(st)) {
> +		dev_err(&acpi_dev->dev,
> +			FW_BUG "failed to get TPM2 ACPI table\n");
> +		return -ENODEV;
> +	}
> +
> +	if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
>  		return -ENODEV;
>  
>  	INIT_LIST_HEAD(&resources);
> @@ -996,6 +978,14 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev)
>  
>  	acpi_dev_free_resource_list(&resources);
>  
> +	if (tpm_info.start == 0 && tpm_info.len == 0) {
> +		tpm_info.start = tbl->control_address;
> +		tpm_info.len = TIS_MEM_LEN;
> +		dev_err(&acpi_dev->dev,
> +			FW_BUG "TPM2 ACPI table does not define a memory resource, using 0x%lx instead\n",
> +			tpm_info.start);
> +	}
> +
>  	if (is_itpm(acpi_dev))
>  		itpm = true;
>  
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284787 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-06 05:20 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCyXL-20P-3@gated-at.bofh.it>
In reply to#1284773
On Sun, Dec 06, 2015 at 06:02:26AM +0200, Jarkko Sakkinen wrote:
> On Thu, Dec 03, 2015 at 11:19:32AM -0700, Jason Gunthorpe wrote:
> > On Thu, Dec 03, 2015 at 08:00:42AM +0200, Jarkko Sakkinen wrote:
> > 
> > > I guess it'd be more realiable. In my NUC the current fix works and the
> > > people who tested it. If you supply me a fix that changes it to use that
> > > I can test it and this will give also coverage to the people who tested
> > > my original fix.
> > 
> > Here is the updated series:
> > 
> > https://github.com/jgunthorpe/linux/commits/for-jarkko
> > 
> > What does your dmesg say?
> > 
> > It really isn't OK to hardwire an address for acpi devices, so I've
> > added something like this. Just completely guessing that control_pa is
> > where the BIOS is hiding the base address. Maybe it is cca->cmd_pa ?
> 
> I'm a bit confused about the discussion because Martin replied that
> tpm_tis used to get the address range before applying this series.
> 
> And pnp_driver in the backend for TPM 1.x devices grabs the address
> range from DSDT.

You can completely ignore this question. I saw Martins reply with a fix for
"tpm_tis: Use devm_ioremap_resource" that you should squash into that
change. So it's proved that TPM ACPI device objects do not always have a
memory resource. Good.

I think these changes are important but there's no really reason to rush
them. Maybe, since there's been a lot of commentary, it'd be better to
resubmit a new revision of the series to the mailing list so that it can
be peer-reviewed once again.

> /Jarkko

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

[toc] | [prev] | [next] | [standalone]


#1284789 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-06 05:30 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCz7r-24p-1@gated-at.bofh.it>
In reply to#1284787
On Sun, Dec 06, 2015 at 06:15:44AM +0200, Jarkko Sakkinen wrote:
> On Sun, Dec 06, 2015 at 06:02:26AM +0200, Jarkko Sakkinen wrote:
> > On Thu, Dec 03, 2015 at 11:19:32AM -0700, Jason Gunthorpe wrote:
> > > On Thu, Dec 03, 2015 at 08:00:42AM +0200, Jarkko Sakkinen wrote:
> > > 
> > > > I guess it'd be more realiable. In my NUC the current fix works and the
> > > > people who tested it. If you supply me a fix that changes it to use that
> > > > I can test it and this will give also coverage to the people who tested
> > > > my original fix.
> > > 
> > > Here is the updated series:
> > > 
> > > https://github.com/jgunthorpe/linux/commits/for-jarkko
> > > 
> > > What does your dmesg say?
> > > 
> > > It really isn't OK to hardwire an address for acpi devices, so I've
> > > added something like this. Just completely guessing that control_pa is
> > > where the BIOS is hiding the base address. Maybe it is cca->cmd_pa ?
> > 
> > I'm a bit confused about the discussion because Martin replied that
> > tpm_tis used to get the address range before applying this series.
> > 
> > And pnp_driver in the backend for TPM 1.x devices grabs the address
> > range from DSDT.
> 
> You can completely ignore this question. I saw Martins reply with a fix for
> "tpm_tis: Use devm_ioremap_resource" that you should squash into that
> change. So it's proved that TPM ACPI device objects do not always have a
> memory resource. Good.
> 
> I think these changes are important but there's no really reason to rush
> them. Maybe, since there's been a lot of commentary, it'd be better to
> resubmit a new revision of the series to the mailing list so that it can
> be peer-reviewed once again.

Maybe even there could be a common tpm_tcg driver once the common code
has been factored out (at some point, lets take this step by step and
fix the issues first). Transmit functions are not heavy and ACPI stuff
is mostly the same.

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

[toc] | [prev] | [next] | [standalone]


#1285018 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2015-12-07 07:20 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCXjs-Mw-15@gated-at.bofh.it>
In reply to#1284787
On Sun, Dec 06, 2015 at 06:15:44AM +0200, Jarkko Sakkinen wrote:
> You can completely ignore this question. I saw Martins reply with a fix for
> "tpm_tis: Use devm_ioremap_resource" that you should squash into that
> change.

It isn't quite the right fix - but I've added something that should be OK
now that Martin has debugged it.

> So it's proved that TPM ACPI device objects do not always have a
> memory resource. Good.

Hopefully.. It is so confusing because tpm_tis had that wrong
fallback, and tpm_crb incorrectly doesn't use the resource
structure. So we've never actually had anything that requires the
resource struct for 2.0 until now.

> resubmit a new revision of the series to the mailing list so that it can
> be peer-reviewed once again.

If Martin is happy I will send them to the list, the latest patches
are on my github:

https://github.com/jgunthorpe/linux/commits/for-jarkko

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

[toc] | [prev] | [next] | [standalone]


#1285070 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

From"Wilck, Martin" <martin.wilck@ts.fujitsu.com>
Date2015-12-07 09:10 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCZ1U-1Tf-19@gated-at.bofh.it>
In reply to#1284787
PiA+IEknbSBhIGJpdCBjb25mdXNlZCBhYm91dCB0aGUgZGlzY3Vzc2lvbiBiZWNhdXNlIE1hcnRp
biByZXBsaWVkIHRoYXQKPiA+IHRwbV90aXMgdXNlZCB0byBnZXQgdGhlIGFkZHJlc3MgcmFuZ2Ug
YmVmb3JlIGFwcGx5aW5nIHRoaXMgc2VyaWVzLgo+ID4gCj4gPiBBbmQgcG5wX2RyaXZlciBpbiB0
aGUgYmFja2VuZCBmb3IgVFBNIDEueCBkZXZpY2VzIGdyYWJzIHRoZSBhZGRyZXNzCj4gPiByYW5n
ZSBmcm9tIERTRFQuCj4gCj4gWW91IGNhbiBjb21wbGV0ZWx5IGlnbm9yZSB0aGlzIHF1ZXN0aW9u
LiBJIHNhdyBNYXJ0aW5zIHJlcGx5IHdpdGggYSBmaXggZm9yCj4gInRwbV90aXM6IFVzZSBkZXZt
X2lvcmVtYXBfcmVzb3VyY2UiIHRoYXQgeW91IHNob3VsZCBzcXVhc2ggaW50byB0aGF0Cj4gY2hh
bmdlLiBTbyBpdCdzIHByb3ZlZCB0aGF0IFRQTSBBQ1BJIGRldmljZSBvYmplY3RzIGRvIG5vdCBh
bHdheXMgaGF2ZSBhCj4gbWVtb3J5IHJlc291cmNlLiBHb29kLgoKUmVwZWF0LCB0aGUgbWVtb3J5
IHJlc291cmNlIERPRVMgZXhpc3Qgb24gbXkgc3lzdGVtLiBOb3Qgc3VyZSB3aGF0IHByb29mCnlv
dSBzYXcgdGhlcmUuCgpNYXJ0aW4KCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285097 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-07 10:00 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qCZOi-2cf-15@gated-at.bofh.it>
In reply to#1285070
On Mon, Dec 07, 2015 at 09:06:50AM +0100, Wilck, Martin wrote:
> > > I'm a bit confused about the discussion because Martin replied that
> > > tpm_tis used to get the address range before applying this series.
> > > 
> > > And pnp_driver in the backend for TPM 1.x devices grabs the address
> > > range from DSDT.
> > 
> > You can completely ignore this question. I saw Martins reply with a fix for
> > "tpm_tis: Use devm_ioremap_resource" that you should squash into that
> > change. So it's proved that TPM ACPI device objects do not always have a
> > memory resource. Good.
> 
> Repeat, the memory resource DOES exist on my system. Not sure what proof
> you saw there.

Ok, lets go this through.

I deduced this from two facts:

* It used to have memory resource as conditional and as a fallback use
  fixed value.
* Your workaround reverted the situation to this.

Did I understand something incorrectly?

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

[toc] | [prev] | [next] | [standalone]


#1285162 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

From"Wilck, Martin" <martin.wilck@ts.fujitsu.com>
Date2015-12-07 11:00 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qD0Km-2NJ-11@gated-at.bofh.it>
In reply to#1285097
PiA+ID4gWW91IGNhbiBjb21wbGV0ZWx5IGlnbm9yZSB0aGlzIHF1ZXN0aW9uLiBJIHNhdyBNYXJ0
aW5zIHJlcGx5IHdpdGggYSBmaXggZm9yCj4gPiA+ICJ0cG1fdGlzOiBVc2UgZGV2bV9pb3JlbWFw
X3Jlc291cmNlIiB0aGF0IHlvdSBzaG91bGQgc3F1YXNoIGludG8gdGhhdAo+ID4gPiBjaGFuZ2Uu
IFNvIGl0J3MgcHJvdmVkIHRoYXQgVFBNIEFDUEkgZGV2aWNlIG9iamVjdHMgZG8gbm90IGFsd2F5
cyBoYXZlIGEKPiA+ID4gbWVtb3J5IHJlc291cmNlLiBHb29kLgo+ID4gCj4gPiBSZXBlYXQsIHRo
ZSBtZW1vcnkgcmVzb3VyY2UgRE9FUyBleGlzdCBvbiBteSBzeXN0ZW0uIE5vdCBzdXJlIHdoYXQg
cHJvb2YKPiA+IHlvdSBzYXcgdGhlcmUuCj4gCj4gT2ssIGxldHMgZ28gdGhpcyB0aHJvdWdoLgo+
IAo+IEkgZGVkdWNlZCB0aGlzIGZyb20gdHdvIGZhY3RzOgo+IAo+ICogSXQgdXNlZCB0byBoYXZl
IG1lbW9yeSByZXNvdXJjZSBhcyBjb25kaXRpb25hbCBhbmQgYXMgYSBmYWxsYmFjayB1c2UKPiAg
IGZpeGVkIHZhbHVlLgo+ICogWW91ciB3b3JrYXJvdW5kIHJldmVydGVkIHRoZSBzaXR1YXRpb24g
dG8gdGhpcy4KPiAKPiBEaWQgSSB1bmRlcnN0YW5kIHNvbWV0aGluZyBpbmNvcnJlY3RseT8KClRo
ZSBwcm9ibGVtIGluIG15IGNhc2UgZGlkbid0IG9jY3VyIGJlY2F1c2UgQUNQSSB3YXMgbGFja2lu
ZyBhIHJlc291cmNlLgpJdCBoYXMgb25lICJleHRyYSIgcmVzb3VyY2UgdGhhdCBKYXNvbidzIG9y
aWdpbmFsIGNvZGUgZGlkbid0CnJlY29nbml6ZS4gCgpKYXNvbidzIGNvZGUgd2FzIHdyb25nbHkg
YXNzdW1pbmcgdGhhdCBhIHJlc291cmNlIHRoYXQgaXNuJ3Qgb2YgdHlwZQoiSVJRIiBoYXMgdG8g
YmUgb2YgdHlwZSAiTUVNT1JZIi4gSWYgSSBwcmludCBvdXQgdGhlIHJlc291cmNlIHR5cGVzCmVu
Y291bnRlcmVkIGluIHRwbV9jaGVja19yZXNvdXJjZSgpLCBJIGdldApBQ1BJX1JFU09VUkNFX1RZ
UEVfRklYRURfTUVNT1JZMzIgICgweDBhKSBmaXJzdCwgZm9sbG93ZWQgYnkKQUNQSV9SRVNPVVJD
RV9UWVBFX0VORF9UQUcgKDB4MDcpLiBUaGUgbGF0dGVyIHdhcyBtaXN0YWtlbmx5IHVzZWQgYnkK
SmFzb24ndCBjb2RlIGFzIGEgbWVtb3J5IHJlc291cmNlLiBUaGlzIGlzIGhvdyBBQ1BJIFJlc291
cmNlVGVtcGxhdGVzCndvcmsgKGEgbGlzdCB3aXRoIGFuIGVuZCBtYXJrZXIpLiBUaGUgY29ycmVj
dCBzb2x1dGlvbiBpcyB0byBhbHdheXMKY2hlY2sgdGhlIHJldHVybiB2YWx1ZSBvZiBhY3BpX2Rl
dl9yZXNvdXJjZV9tZW1vcnkoKSwgYXMgaXQncyBjdXJyZW50bHkKaW1wbGVtZW50ZWQgaW4gSmFz
b24ndCBjdXJyZW50ICJmb3ItamFya2tvIiBicmFuY2guCgpNYXJ0aW4KCgo+IAo+IC9KYXJra28K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285176 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date2015-12-07 11:20 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qD13I-3bd-15@gated-at.bofh.it>
In reply to#1285162
On Mon, Dec 07, 2015 at 10:52:51AM +0100, Wilck, Martin wrote:
> > > > You can completely ignore this question. I saw Martins reply with a fix for
> > > > "tpm_tis: Use devm_ioremap_resource" that you should squash into that
> > > > change. So it's proved that TPM ACPI device objects do not always have a
> > > > memory resource. Good.
> > > 
> > > Repeat, the memory resource DOES exist on my system. Not sure what proof
> > > you saw there.
> > 
> > Ok, lets go this through.
> > 
> > I deduced this from two facts:
> > 
> > * It used to have memory resource as conditional and as a fallback use
> >   fixed value.
> > * Your workaround reverted the situation to this.
> > 
> > Did I understand something incorrectly?
> 
> The problem in my case didn't occur because ACPI was lacking a resource.
> It has one "extra" resource that Jason's original code didn't
> recognize. 
> 
> Jason's code was wrongly assuming that a resource that isn't of type
> "IRQ" has to be of type "MEMORY". If I print out the resource types
> encountered in tpm_check_resource(), I get
> ACPI_RESOURCE_TYPE_FIXED_MEMORY32  (0x0a) first, followed by
> ACPI_RESOURCE_TYPE_END_TAG (0x07). The latter was mistakenly used by
> Jason't code as a memory resource. This is how ACPI ResourceTemplates
> work (a list with an end marker). The correct solution is to always
> check the return value of acpi_dev_resource_memory(), as it's currently
> implemented in Jason't current "for-jarkko" branch.

Aah. Right.

> Martin

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

[toc] | [prev] | [next] | [standalone]


#1282796 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

From"Wilck, Martin" <martin.wilck@ts.fujitsu.com>
Date2015-12-03 09:40 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBxAL-2vK-31@gated-at.bofh.it>
In reply to#1282286
T24gTWksIDIwMTUtMTItMDIgYXQgMTI6MTEgLTA3MDAsIEphc29uIEd1bnRob3JwZSB3cm90ZToK
Cj4gPiBXaGF0IGlzIHRoZSBhZGRyZXNzIHRwbV90aXMgc2hvdWxkIGJlIHVzaW5nPyBJIHNlZSB0
d28gdGhpbmdzLCBpdAo+ID4gZWl0aGVyIHVzZXMgdGhlIHg4NiBkZWZhdWx0IGFkZHJlc3Mgb3Ig
aXQgZXhwZWN0cyB0aGUgQUNQSSB0byBoYXZlIGEKPiA+IE1FTSByZXNvdXJjZS4gQUZBSUsgQUNQ
SSBzaG91bGQgbmV2ZXIgcmVseSBvbiBoYXJkIHdpcmVkIGFkZHJlc3Nlcywgc28KPiA+IEkgcmVt
b3ZlZCB0aGF0IGNvZGUgaW4gdGhpcyBzZXJpZXMuIFBlcmhhcHMgdHBtX3RpcyBzaG91bGQgYmUg
dXNpbmcKPiA+IGNvbnRyb2xfYXJlYV9wYSA/IFdpbGwgQUNQSSBldmVyIHByZXNlbnQgYSBzdHJ1
Y3QgcmVzb3VyY2U/IChpZiB5ZXMsCj4gPiB3aHkgaXNuJ3QgdHBtX2NyYiB1c2luZyBvbmU/KQo+
IAo+IElzIHRoZW4gc3RpbGwgYSBwcm9ibGVtLiBPbiBNYXJ0aW4ncyBzeXN0ZW0gdGhlIE1TRlQw
MTAxIGRldmljZSBkb2VzCj4gbm90IGhhdmUgYSBzdHJ1Y3QgcmVzb3VyY2UgYXR0YWNoZWQgdG8g
aXQuIERvZXMgYW55IHN5c3RlbSwgb3IgaXMgdGhpcwo+IGp1c3QgZGVhZCBjb2RlPwoKQUNQSSBk
ZWZpbmVzIGEgbWVtIHJlc291cmNlIGNvcnJlc3BvbmRpbmcgdG8gdGhlIHN0YW5kYXJkIFRJUyBt
ZW1vcnkKYXJlYSBvbiBteSBzeXN0ZW0sIGFuZCBpdCB1c2VkIHRvIGJlIGRldGVjdGVkIGZpbmUg
d2l0aCBKYXJra28ncyBwYXRjaC4KU29tZWhvdyB5b3VyIGxhdGVzdCBjaGFuZ2VzIGJyb2tlIGl0
LCBub3Qgc3VyZSB3aHkuCgpNYXJ0aW4KCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1283196 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

FromJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date2015-12-03 18:10 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBFyi-80z-25@gated-at.bofh.it>
In reply to#1282796
On Thu, Dec 03, 2015 at 09:30:30AM +0100, Wilck, Martin wrote:
> On Mi, 2015-12-02 at 12:11 -0700, Jason Gunthorpe wrote:
> 
> > > What is the address tpm_tis should be using? I see two things, it
> > > either uses the x86 default address or it expects the ACPI to have a
> > > MEM resource. AFAIK ACPI should never rely on hard wired addresses, so
> > > I removed that code in this series. Perhaps tpm_tis should be using
> > > control_area_pa ? Will ACPI ever present a struct resource? (if yes,
> > > why isn't tpm_crb using one?)
> > 
> > Is then still a problem. On Martin's system the MSFT0101 device does
> > not have a struct resource attached to it. Does any system, or is this
> > just dead code?
> 
> ACPI defines a mem resource corresponding to the standard TIS memory
> area on my system, and it used to be detected fine with Jarkko's patch.
> Somehow your latest changes broke it, not sure why.

Are you certain? Based on what you sent me, that output is only
possible if there is no mem resource.

With the prior arrangement no mem resource means the x86 default
address is used, which is the only way I can see how your system
works.

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

[toc] | [prev] | [next] | [standalone]


#1283646 — Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter

From"Wilck, Martin" <martin.wilck@ts.fujitsu.com>
Date2015-12-04 09:40 +0100
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
Message-ID<qBU4k-h7-43@gated-at.bofh.it>
In reply to#1283196
CgoKT24gRG8sIDIwMTUtMTItMDMgYXQgMTA6MDAgLTA3MDAsIEphc29uIEd1bnRob3JwZSB3cm90
ZToKPiBPbiBUaHUsIERlYyAwMywgMjAxNSBhdCAwOTozMDozMEFNICswMTAwLCBXaWxjaywgTWFy
dGluIHdyb3RlOgo+ID4gT24gTWksIDIwMTUtMTItMDIgYXQgMTI6MTEgLTA3MDAsIEphc29uIEd1
bnRob3JwZSB3cm90ZToKPiA+IAo+ID4gPiA+IFdoYXQgaXMgdGhlIGFkZHJlc3MgdHBtX3RpcyBz
aG91bGQgYmUgdXNpbmc/IEkgc2VlIHR3byB0aGluZ3MsIGl0Cj4gPiA+ID4gZWl0aGVyIHVzZXMg
dGhlIHg4NiBkZWZhdWx0IGFkZHJlc3Mgb3IgaXQgZXhwZWN0cyB0aGUgQUNQSSB0byBoYXZlIGEK
PiA+ID4gPiBNRU0gcmVzb3VyY2UuIEFGQUlLIEFDUEkgc2hvdWxkIG5ldmVyIHJlbHkgb24gaGFy
ZCB3aXJlZCBhZGRyZXNzZXMsIHNvCj4gPiA+ID4gSSByZW1vdmVkIHRoYXQgY29kZSBpbiB0aGlz
IHNlcmllcy4gUGVyaGFwcyB0cG1fdGlzIHNob3VsZCBiZSB1c2luZwo+ID4gPiA+IGNvbnRyb2xf
YXJlYV9wYSA/IFdpbGwgQUNQSSBldmVyIHByZXNlbnQgYSBzdHJ1Y3QgcmVzb3VyY2U/IChpZiB5
ZXMsCj4gPiA+ID4gd2h5IGlzbid0IHRwbV9jcmIgdXNpbmcgb25lPykKPiA+ID4gCj4gPiA+IElz
IHRoZW4gc3RpbGwgYSBwcm9ibGVtLiBPbiBNYXJ0aW4ncyBzeXN0ZW0gdGhlIE1TRlQwMTAxIGRl
dmljZSBkb2VzCj4gPiA+IG5vdCBoYXZlIGEgc3RydWN0IHJlc291cmNlIGF0dGFjaGVkIHRvIGl0
LiBEb2VzIGFueSBzeXN0ZW0sIG9yIGlzIHRoaXMKPiA+ID4ganVzdCBkZWFkIGNvZGU/Cj4gPiAK
PiA+IEFDUEkgZGVmaW5lcyBhIG1lbSByZXNvdXJjZSBjb3JyZXNwb25kaW5nIHRvIHRoZSBzdGFu
ZGFyZCBUSVMgbWVtb3J5Cj4gPiBhcmVhIG9uIG15IHN5c3RlbSwgYW5kIGl0IHVzZWQgdG8gYmUg
ZGV0ZWN0ZWQgZmluZSB3aXRoIEphcmtrbydzIHBhdGNoLgo+ID4gU29tZWhvdyB5b3VyIGxhdGVz
dCBjaGFuZ2VzIGJyb2tlIGl0LCBub3Qgc3VyZSB3aHkuCj4gCj4gQXJlIHlvdSBjZXJ0YWluPyBC
YXNlZCBvbiB3aGF0IHlvdSBzZW50IG1lLCB0aGF0IG91dHB1dCBpcyBvbmx5Cj4gcG9zc2libGUg
aWYgdGhlcmUgaXMgbm8gbWVtIHJlc291cmNlLgoKWWVzLCBJIGFtIGNlcnRhaW4uIEkgY2hlY2tl
ZCB0aGUgRFNEVCwgYW5kIEkgcHV0IGEgZGVidWcgc3RhdGVtZW50IHJpZ2h0CmFmdGVyIHRoZSBy
ZXNvdXJjZSBkZXRlY3Rpb24gaW4gdHBtX3Rpcy4KCk1hcnRpbgoKCj4gV2l0aCB0aGUgcHJpb3Ig
YXJyYW5nZW1lbnQgbm8gbWVtIHJlc291cmNlIG1lYW5zIHRoZSB4ODYgZGVmYXVsdAo+IGFkZHJl
c3MgaXMgdXNlZCwgd2hpY2ggaXMgdGhlIG9ubHkgd2F5IEkgY2FuIHNlZSBob3cgeW91ciBzeXN0
ZW0KPiB3b3Jrcy4KCgoKPiAKPiBKYXNvbgo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web